"Bill Barker" <billwbar...@verizon.net> wrote in message
news:i2dl9b$i6...@dough.gmane.org...
"Bill Barker" <billwbar...@verizon.net> wrote in message
news:i2dje8$es...@dough.gmane.org...
<ma...@apache.org> wrote in message
news:20100722223130.a183d2388...@eris.apache.org...
Author: markt
Date: Thu Jul 22 22:31:30 2010
New Revision: 966882
URL: http://svn.apache.org/viewvc?rev=966882&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49268
Add necessary plumbing to enable Checkstyle
The config file is deliberately empty. The check will be uncommented
once the source code has been fixed (~200 files contain tabs).
Added:
tomcat/trunk/checkstyle.xml (with props)
Modified:
tomcat/trunk/build.properties.default
tomcat/trunk/build.xml
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/build.properties.default
URL:
http://svn.apache.org/viewvc/tomcat/trunk/build.properties.default?rev=966882&r1=966881&r2=966882&view=diff
==============================================================================
--- tomcat/trunk/build.properties.default (original)
+++ tomcat/trunk/build.properties.default Thu Jul 22 22:31:30 2010
@@ -120,6 +120,12 @@ junit.lib=${junit.home}
junit.jar=${junit.lib}/junit.jar
junit.loc=${base-sf.loc}/junit/junit3.8.2.zip
+# ----- Checkstyle, version 5.1 or later -----
+checkstyle.version=5.1
+checkstyle.home=${base.path}/checkstyle-${checkstyle.version}
+checkstyle.loc=${base-sf.loc}/checkstyle/checkstyle-${checkstyle.version}.zip
+checkstyle.jar=${checkstyle.home}/checkstyle-all-${checkstyle.version}.jar
+
# ----- JSON Libraries (for bayeux module) -----
json-lib.home=${base.path}/json-20080701
json-lib.lib=http://repo1.maven.org/maven2/org/json/json/20080701/json-20080701.jar
Modified: tomcat/trunk/build.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/build.xml?rev=966882&r1=966881&r2=966882&view=diff
==============================================================================
--- tomcat/trunk/build.xml (original)
+++ tomcat/trunk/build.xml Thu Jul 22 22:31:30 2010
@@ -410,7 +410,18 @@
</target>
- <target name="compile" depends="build-prepare,download-compile">
+ <target name="validate" depends="download-validate">
+ <taskdef resource="checkstyletask.properties"
+ classpath="${checkstyle.jar}" />
+ <checkstyle config="checkstyle.xml">
+ <fileset dir="java">
+ <include name="**/*.java" />
+ <exclude name="org/apache/tomcat/util/bcel/**" />
+ </fileset>
+ </checkstyle>
+ </target>
+
+ <target name="compile"
depends="build-prepare,download-compile,validate">
I'm -1 (vote, not veto) on having validate be a dependency for compile.
Historically, Tomcat has been very lenient about what is required just to
build the project, and I can't see the reason to force checkstyle just to
be able to build. Mostly my concern is with Gump (which I can fix the
metadata when svn comes up again). But that puts a transitive dependency
on bcel that will get fixed who knows when.
I'm more than happy to add a tomcat-trunk-validate project to Gump if it
is split from compile.
Having tried this (yes, I should have done this first), changing to -1
veto. In the OS world of cat herding, we should allow for the fact that
somebody mistakenly violates the checkstyle rules and commits without
building. It happens all the time, and then they go off to sleep and
nobody else can build the project with a fresh checkout for hours.
This is probably cruelty against dead horses, but shouldn't the download
target be conditional as well? Since checkstyle is LGPL licensed, I really
don't think that we should be installing it by default for anyone that wants
to build Tomcat. Being an extremist on this issue, I would actually prefer
that we remove the download-validate target. This would mean that anyone
that wants to use it has to have installed it already, and so have accepted
the LGPL license. I for one, expect that when I download an Apache product,
that it will only install compatibly licensed software. At least not
without warning me otherwise.
This is a -0: vent. There is nothing wrong from a licensing stand point on
how we are using it.
As I've said, I'm happy to add this to Gump (assuming that bcel ever
builds ever again) if it a separate target to get regular automated
notices. But developers can run a separate target themselves (just like
test).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
