"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 CheckstyleThe 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.defaultURL: 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.xmlURL: 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.
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).
<!-- Compile internal server components --> <javac srcdir="java" destdir="${tomcat.classes}" @@ -1888,6 +1899,17 @@ Apache Tomcat ${version} native binaries<!-- ================ Download and dependency building =================== -->+ <target name="download-validate"+ description="Download components necessary to validate source" >+ + <antcall target="downloadzip"> + <param name="sourcefile" value="${checkstyle.loc}"/> + <param name="destfile" value="${checkstyle.jar}"/> + <param name="destdir" value="${base.path}"/> + </antcall> + + </target> + <target name="download-compile"description="Download (and build) components necessary to compile" >Added: tomcat/trunk/checkstyle.xmlURL: http://svn.apache.org/viewvc/tomcat/trunk/checkstyle.xml?rev=966882&view=auto============================================================================== --- tomcat/trunk/checkstyle.xml (added) +++ tomcat/trunk/checkstyle.xml Thu Jul 22 22:31:30 2010 @@ -0,0 +1,25 @@ +<?xml version="1.0"?> +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership.+ The ASF licenses this file to You under the Apache License, Version 2.0+ (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS,+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.+ See the License for the specific language governing permissions and + limitations under the License. +--> +<!DOCTYPE module PUBLIC + "-//Puppy Crawl//DTD Check Configuration 1.2//EN" + "http://www.puppycrawl.com/dtds/configuration_1_2.dtd";> +<module name="Checker"> +<!-- + <module name="FileTabCharacter"/> +--> +</module> \ No newline at end of file Propchange: tomcat/trunk/checkstyle.xml ------------------------------------------------------------------------------ svn:eol-style = native Modified: tomcat/trunk/webapps/docs/changelog.xmlURL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=966882&r1=966881&r2=966882&view=diff============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Thu Jul 22 22:31:30 2010 @@ -294,6 +294,11 @@Re-factor unit tests to enable them to be run once with each of the HTTPconnector implementations (BIO, NIO and APR/native). (markt) </add> + <add>+ <bug>49268</bug>: Add the necessary plumbing to include CheckStyle in + the build process. Start with no checks. Additional checks will be+ added as they are agreed. (markt) + </add> </changelog> </subsection></section>
--------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
