tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Barker" <billwbar...@verizon.net>
Subject Re: svn commit: r966882 - in /tomcat/trunk: build.properties.default build.xml checkstyle.xml webapps/docs/changelog.xml
Date Wed, 28 Jul 2010 02:38:17 GMT


"Bill Barker" <billwbarker@verizon.net> wrote in message 
news:i2dl9b$i62$1@dough.gmane.org...
>
>
> "Bill Barker" <billwbarker@verizon.net> wrote in message 
> news:i2dje8$es0$1@dough.gmane.org...
>>
>>
>> <markt@apache.org> wrote in message 
>> news:20100722223130.A183D23889E0@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message