Return-Path: X-Original-To: apmail-tomcat-dev-archive@www.apache.org Delivered-To: apmail-tomcat-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id D748BC530 for ; Mon, 30 Apr 2012 09:08:52 +0000 (UTC) Received: (qmail 98102 invoked by uid 500); 30 Apr 2012 09:08:52 -0000 Delivered-To: apmail-tomcat-dev-archive@tomcat.apache.org Received: (qmail 98022 invoked by uid 500); 30 Apr 2012 09:08:51 -0000 Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "Tomcat Developers List" Delivered-To: mailing list dev@tomcat.apache.org Received: (qmail 98009 invoked by uid 99); 30 Apr 2012 09:08:51 -0000 Received: from minotaur.apache.org (HELO minotaur.apache.org) (140.211.11.9) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 30 Apr 2012 09:08:51 +0000 Received: from localhost (HELO mail-yx0-f173.google.com) (127.0.0.1) (smtp-auth username olamy, mechanism plain) by minotaur.apache.org (qpsmtpd/0.29) with ESMTP; Mon, 30 Apr 2012 09:08:51 +0000 Received: by yenr5 with SMTP id r5so1484508yen.18 for ; Mon, 30 Apr 2012 02:08:50 -0700 (PDT) Received: by 10.236.92.178 with SMTP id j38mr16381484yhf.58.1335776930338; Mon, 30 Apr 2012 02:08:50 -0700 (PDT) MIME-Version: 1.0 Received: by 10.100.14.18 with HTTP; Mon, 30 Apr 2012 02:08:29 -0700 (PDT) In-Reply-To: References: <20120428211102.3E31B23889E0@eris.apache.org> From: Olivier Lamy Date: Mon, 30 Apr 2012 11:08:29 +0200 Message-ID: Subject: Re: svn commit: r1331833 - in /tomcat/maven-plugin/trunk: tomcat7-maven-plugin/src/main/java/org/apache/tomcat/maven/plugin/tomcat7/run/AbstractExecWarMojo.java tomcat7-war-runner/src/main/java/org/apache/tomcat/maven/runner/Tomcat7Runner.java To: Tomcat Developers List Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable fixed. Thanks for review. 2012/4/29 Konstantin Kolinko : > 2012/4/29 =A0: >> Author: olamy >> Date: Sat Apr 28 21:11:01 2012 >> New Revision: 1331833 >> >> URL: http://svn.apache.org/viewvc?rev=3D1331833&view=3Drev >> Log: >> for exec war extraction mode, generate a file containing the archive tim= estamp creation to force reextract when users regenerate it and miss to res= et extract folder. >> >> Modified: >> =A0 =A0tomcat/maven-plugin/trunk/tomcat7-maven-plugin/src/main/java/org/= apache/tomcat/maven/plugin/tomcat7/run/AbstractExecWarMojo.java >> =A0 =A0tomcat/maven-plugin/trunk/tomcat7-war-runner/src/main/java/org/ap= ache/tomcat/maven/runner/Tomcat7Runner.java >> >> Modified: tomcat/maven-plugin/trunk/tomcat7-maven-plugin/src/main/java/o= rg/apache/tomcat/maven/plugin/tomcat7/run/AbstractExecWarMojo.java >> URL: http://svn.apache.org/viewvc/tomcat/maven-plugin/trunk/tomcat7-mave= n-plugin/src/main/java/org/apache/tomcat/maven/plugin/tomcat7/run/AbstractE= xecWarMojo.java?rev=3D1331833&r1=3D1331832&r2=3D1331833&view=3Ddiff >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D >> --- tomcat/maven-plugin/trunk/tomcat7-maven-plugin/src/main/java/org/apa= che/tomcat/maven/plugin/tomcat7/run/AbstractExecWarMojo.java (original) >> +++ tomcat/maven-plugin/trunk/tomcat7-maven-plugin/src/main/java/org/apa= che/tomcat/maven/plugin/tomcat7/run/AbstractExecWarMojo.java Sat Apr 28 21:= 11:01 2012 >> @@ -51,6 +51,7 @@ import java.io.InputStream; >> =A0import java.io.OutputStream; >> =A0import java.io.PrintWriter; >> =A0import java.util.ArrayList; >> +import java.util.Date; >> =A0import java.util.Enumeration; >> =A0import java.util.Iterator; >> =A0import java.util.List; >> @@ -293,6 +294,7 @@ public abstract class AbstractExecWarMoj >> >> =A0 =A0 =A0 =A0 =A0 =A0 Properties properties =3D new Properties(); >> >> + =A0 =A0 =A0 =A0 =A0 =A0properties.put( Tomcat7Runner.ARCHIVE_GENERATIO= N_TIMESTAMP_KEY, Long.toString( new Date().getTime() ) ); > > > The above "new Date().getTime()" call produces the same result as > calling System.currentTimeMillis() directly, but creates an unneeded > Date object. > > >> =A0 =A0 =A0 =A0 =A0 =A0 properties.put( Tomcat7Runner.ENABLE_NAMING_KEY,= Boolean.toString( enableNaming ) ); >> =A0 =A0 =A0 =A0 =A0 =A0 properties.put( Tomcat7Runner.ACCESS_LOG_VALVE_F= ORMAT_KEY, accessLogValveFormat ); >> =A0 =A0 =A0 =A0 =A0 =A0 properties.put( Tomcat7Runner.HTTP_PROTOCOL_KEY,= connectorHttpProtocol ); >> @@ -369,7 +371,6 @@ public abstract class AbstractExecWarMoj >> =A0 =A0 =A0 =A0 =A0 =A0 IOUtils.copy( getClass().getResourceAsStream( "/= conf/web.xml" ), os ); >> =A0 =A0 =A0 =A0 =A0 =A0 os.closeArchiveEntry(); >> >> - >> =A0 =A0 =A0 =A0 =A0 =A0 properties.store( tmpPropertiesFileOutputStream,= "created by Apache Tomcat Maven plugin" ); >> >> =A0 =A0 =A0 =A0 =A0 =A0 tmpPropertiesFileOutputStream.flush(); >> >> Modified: tomcat/maven-plugin/trunk/tomcat7-war-runner/src/main/java/org= /apache/tomcat/maven/runner/Tomcat7Runner.java >> URL: http://svn.apache.org/viewvc/tomcat/maven-plugin/trunk/tomcat7-war-= runner/src/main/java/org/apache/tomcat/maven/runner/Tomcat7Runner.java?rev= =3D1331833&r1=3D1331832&r2=3D1331833&view=3Ddiff >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D >> --- tomcat/maven-plugin/trunk/tomcat7-war-runner/src/main/java/org/apach= e/tomcat/maven/runner/Tomcat7Runner.java (original) >> +++ tomcat/maven-plugin/trunk/tomcat7-war-runner/src/main/java/org/apach= e/tomcat/maven/runner/Tomcat7Runner.java Sat Apr 28 21:11:01 2012 >> @@ -33,6 +33,8 @@ import java.io.BufferedOutputStream; >> =A0import java.io.File; >> =A0import java.io.FileNotFoundException; >> =A0import java.io.FileOutputStream; >> +import java.io.FileReader; >> +import java.io.FileWriter; >> =A0import java.io.IOException; >> =A0import java.io.InputStream; >> =A0import java.lang.reflect.InvocationTargetException; >> @@ -58,6 +60,8 @@ public class Tomcat7Runner >> =A0 =A0 // contains war name wars=3Dfoo.war,bar.war >> =A0 =A0 public static final String WARS_KEY =3D "wars"; >> >> + =A0 =A0public static final String ARCHIVE_GENERATION_TIMESTAMP_KEY =3D= "generationTimestamp"; >> + >> =A0 =A0 public static final String ENABLE_NAMING_KEY =3D "enableNaming"; >> >> =A0 =A0 public static final String ACCESS_LOG_VALVE_FORMAT_KEY =3D "acce= ssLogValveFormat"; >> @@ -125,15 +129,48 @@ public class Tomcat7Runner >> >> =A0 =A0 =A0 =A0 debugMessage( "use extractDirectory:" + extractDirectory= File.getPath() ); >> >> - =A0 =A0 =A0 =A0// do we have to extract content >> - =A0 =A0 =A0 =A0if ( !extractDirectoryFile.exists() || resetExtract ) >> + =A0 =A0 =A0 =A0boolean archiveTimestampChanged =3D false; >> + >> + =A0 =A0 =A0 =A0// compare timestamp stored during previous run if exis= ts >> + =A0 =A0 =A0 =A0File timestampFile =3D new File( extractDirectoryFile, = ".tomcat_executable_archive.timestamp" ); >> + >> + =A0 =A0 =A0 =A0Properties timestampProps =3D new Properties(); >> + >> + =A0 =A0 =A0 =A0if ( timestampFile.exists() ) >> =A0 =A0 =A0 =A0 { >> - =A0 =A0 =A0 =A0 =A0 =A0extract(); >> + =A0 =A0 =A0 =A0 =A0 =A0timestampProps.load( new FileReader( timestampF= ile ) ); > > The above is bad coding. You should use FileInputStream to load a > Properties file. > > Properties file uses ISO-8859-1, not system default encoding, and > there are methods that use InputStream/OutputStream in the Properties > class to deal with that. > > >> + =A0 =A0 =A0 =A0 =A0 =A0String timestampValue =3D timestampProps.getPro= perty( Tomcat7Runner.ARCHIVE_GENERATION_TIMESTAMP_KEY ); >> + =A0 =A0 =A0 =A0 =A0 =A0if ( timestampValue !=3D null ) >> + =A0 =A0 =A0 =A0 =A0 =A0{ >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0long timestamp =3D Long.parseLong( time= stampValue ); >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0archiveTimestampChanged =3D >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Long.parseLong( runtimeProperti= es.getProperty( Tomcat7Runner.ARCHIVE_GENERATION_TIMESTAMP_KEY ) ) >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0> timestamp; >> + >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0debugMessage( "read timestamp from file= " + timestampValue + ", archiveTimestampChanged: " >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0+ a= rchiveTimestampChanged ); >> + =A0 =A0 =A0 =A0 =A0 =A0} >> + >> =A0 =A0 =A0 =A0 } >> - =A0 =A0 =A0 =A0else >> + >> + =A0 =A0 =A0 =A0// do we have to extract content >> =A0 =A0 =A0 =A0 { >> - =A0 =A0 =A0 =A0 =A0 =A0String wars =3D runtimeProperties.getProperty( = WARS_KEY ); >> - =A0 =A0 =A0 =A0 =A0 =A0populateWebAppWarPerContext( wars ); >> + =A0 =A0 =A0 =A0 =A0 =A0if ( !extractDirectoryFile.exists() || resetExt= ract ) >> + =A0 =A0 =A0 =A0 =A0 =A0{ >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0extract(); >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0// first run so create timestamp file >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if ( !timestampFile.exists() ) > > I do not get why you use the above exists() check. Shouldn't it be > unconditional? > > With your followup fix in > http://svn.apache.org/viewvc?diff_format=3Dh&view=3Drevision&revision=3D1= 331839 > you do perform extract() call if (archiveTimestampChanged) is true, > but I do not see how the timestamp file is updated. =A0I have not looked > into it, but does extract() call delete the file? That would be the > reason to remove the above exists() check. > >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0{ >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0timestampProps.put( Tomcat7Runn= er.ARCHIVE_GENERATION_TIMESTAMP_KEY, runtimeProperties.getProperty( >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Tomcat7Runner.ARCHIVE_G= ENERATION_TIMESTAMP_KEY ) ); >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0timestampProps.store( new FileW= riter( timestampFile ), "Timestamp file for executable war/jar" ); > > > It needs FileOutputStream above. > > >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} >> + =A0 =A0 =A0 =A0 =A0 =A0} >> + =A0 =A0 =A0 =A0 =A0 =A0else >> + =A0 =A0 =A0 =A0 =A0 =A0{ >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0String wars =3D runtimeProperties.getPr= operty( WARS_KEY ); >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0populateWebAppWarPerContext( wars ); >> + =A0 =A0 =A0 =A0 =A0 =A0} >> =A0 =A0 =A0 =A0 } >> >> =A0 =A0 =A0 =A0 // create tomcat various paths >> > > Best regards, > Konstantin Kolinko > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org > For additional commands, e-mail: dev-help@tomcat.apache.org > --=20 Olivier Lamy Talend: http://coders.talend.com http://twitter.com/olamy | http://linkedin.com/in/olamy --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org For additional commands, e-mail: dev-help@tomcat.apache.org