You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2016/04/06 02:25:41 UTC

[Bug 59276] New: Update Checkstyle to version 6.17 needs a configuration change

https://bz.apache.org/bugzilla/show_bug.cgi?id=59276

            Bug ID: 59276
           Summary: Update Checkstyle to version 6.17 needs a
                    configuration change
           Product: Tomcat 9
           Version: 9.0.0.M4
          Hardware: PC
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: Catalina
          Assignee: dev@tomcat.apache.org
          Reporter: knst.kolinko@gmail.com

Filing into Bugzilla to better document the issue.

As I noted in commit message for r1737833, simply dropping in Checkstyle 6.17
(6.16.1, 6.16) - any version later than 6.15 - results in notable drop in
performance of reruns.

A more detailed observation is that running Checkstyle 6.17 does not create the
cache files (output/res/checkstyle/*), so 6.17 is so slow because it is running
without a cache.

In changelog for Checkstyle 6.16 [1] there is the following item:

* Move Treewalker cache to Checker. Author: Andrei Selkin #569 [2]

[1] http://checkstyle.sourceforge.net/releasenotes.html
[2] https://github.com/checkstyle/checkstyle/issues/569

[3] Commit that fixes issue 569:
https://github.com/MEZk/checkstyle/commit/d46c2cf0e9df06bb5f424dbd7645574f082f7609


This means that "setCacheFile()" method was added to Checker class. It means
that our configurations (res/checkstyle/*) need to be updated:

The fix is to move

  <property name="cacheFile" ...>

from within <module name="TreeWalker">
into top-level <module name="Checker"> element.


It is easy to do the change for command-line users of checkstyle. A question is
whether the same configuration files are used by developers running Checkstyle
plugin in IDEs. Moving the property will remove caching from older versions of
Xhexkstyle.


A temporary backwards-compatible workaround may be to keep the property in both
places (as setting the duplicate property on TreeWalker does not cause a
runtime error with 6.17), but it is expected that the setter method will be
removed from TreeWalker in some future release [4]

[4] https://github.com/checkstyle/checkstyle/issues/2883
#2883 - Remove `cache` field from TreeWalker in Checkstyle 8.0

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 59276] Update Checkstyle to version 6.17 needs a configuration change

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59276

Konstantin Kolinko <kn...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|                            |All

--- Comment #1 from Konstantin Kolinko <kn...@gmail.com> ---
(In reply to Konstantin Kolinko from comment #0)
> 
> A temporary backwards-compatible workaround may be to keep the property in
> both places (as setting the duplicate property on TreeWalker does not cause
> a runtime error with 6.17), but it is expected that the setter method will
> be removed from TreeWalker in some future release [4]
> 
> [4] https://github.com/checkstyle/checkstyle/issues/2883
> #2883 - Remove `cache` field from TreeWalker in Checkstyle 8.0

Such workaround is not possible. Checkstyle 6.15 does not have setCacheFile()
method on Walker. An attempt to run updated configuration with 6.15 results in

BUILD FAILED
\build.xml:586: Unable to create a Checker: configLocation
 {<...>\trunk\res\checkstyle\javax-checkstyle.xml}, classpath {null}.

without any further details provided by the error message.

I changed the configuration in Tomcat 9 in r1737903.

Tomcat 8, 8.5 are TODO.

Tomcat 7 is using Checkstyle 6.1.1 as running 6.2 requires Java 7.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 59276] Update Checkstyle to version 6.17 needs a configuration change

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59276

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #2 from Mark Thomas <ma...@apache.org> ---
Fixed for 8.5.x and 8.0.x as well.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org