On Mon, Sep 8, 2008 at 10:56 AM, Scott Blum <[EMAIL PROTECTED]> wrote:
> Let me rephrase the question: couldn't we have two sentinel files instead
> of four? Right now you have two sentinel files for dev/core and two for
> distro-source; each could get by with only one sentinel file whose name
> composited gwt.version with gwt.svnrev.filename.
>
Ah. Yes, we could have two sentinels rather than four, sorry---I
misunderstood which pair(s) you wanted to fold.
> Detailed review:
>
> At a high level, I'm not sure why you went the route of making things so
> general. Why does gwt.svninfo need to take parameters to say what props to
> read the values into? Why not simply always read them into {gwt.svnrev} and
> ${gwt.svnrev.filename}? For that matter, why does gwt.revfilter need to
> take parameters for those values? I think both of these should use
> hardcoded property names; the other advantage to this is the ability to
> override these values on the command line and know you've gotten them
> globally.
>
I like macros not to have side effects. Not strongly enough to contest it,
so the new patch has hardcoded output properties, but that was the
motivation. (It also would avoid any namespace issues when, inevitably,
this gets cut-and-pasted into the "common".ant.xml of incubator,
gwt-google-apis, etc... though I know those examples have no namespace
issues today anyway.)
> Ideally, those properties could be computed at the top level without
> actually having to remember to call the macro to get them defined, but I'm
> not sure that that's worth the effort.
>
While setting the value should be pretty "fast," if you're working just in
user you may rather not have it set since it won't ever be used. (We only
have two use instances; I'd swing the other way with a third one.) So,
yeah, I passed on this one: you need to call <gwt.getsvninfo/> in any
subproject that cares. That does, however, establish common naming for the
sentinels and creates the sentinel directory, which cleans up some stuff.
The distro-source used to skip sentinels entirely, so there's also some
added exclusion there now that they're creating 'em.
> /dev/common.ant.xml
> - Why do you need to exclude About.properties from both source directories,
> since it will only exist in dev/core?
>
I'd expected common to be common-inclusive-of-core, not just
common-to-plaforms-only. ("Expected" is maybe too strong; "programmed to
allow for" is perhaps better---but if core included common, then inside core
"src" would be an alias for ${gwt.core.root}/src and would need the
exclusion.) Today, not needed, so out it goes.
> /distro-source/common.ant.xml
> - Again, 24-25 & 40-41 can be merged to a single sentinel file.
> - 21: What's with the echo here?
>
Yup, and debuggage now gone.
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
Index: common.ant.xml
===================================================================
--- common.ant.xml (revision 3642)
+++ common.ant.xml (working copy)
@@ -199,10 +199,7 @@
</sequential>
</macrodef>
- <macrodef name="gwt.revfilter" description="Filters files for versioning">
- <attribute name="todir" description="Destination for the filtered copy"/>
- <element name="src.fileset" implicit="true"
- description="Source for the filtered copy"/>
+ <macrodef name="gwt.getsvninfo" description="Identifies the SVN info of a workspace">
<sequential>
<exec outputproperty="svn.info" executable="svn"
failifexecutionfails="false" logError="true">
@@ -227,7 +224,31 @@
casesensitive="false" />
<property name="gwt.svnrev"
value="[EMAIL PROTECTED]"/>
+ <!-- Windows NTFS and FAT don't like ':' or '@', and we need a sentinel file to test
+ for out-of-dateness, so this is a filename-acceptable equivalent to svnrev -->
+ <propertyregex property="gwt.svnrev.filename"
+ input="${gwt.svnrev}"
+ regexp="[:@]"
+ replace="-"
+ defaultValue="${gwt.svnrev}"/>
+ <!-- Generally, filtering requires a sentinel file so that changes to svn rev will
+ be noticed as invalidating the previously-generated filter output. This property
+ names where such a sentinel lives; it is tested with <available/> and created
+ with <touch/> -->
+ <mkdir dir="${project.build}/sentinels" />
+ <property name="filter.sentinel"
+ location="${project.build}/sentinels/gwt-${gwt.version}-svn-${gwt.svnrev.filename}" />
+ </sequential>
+ </macrodef>
+
+ <macrodef name="gwt.revfilter" description="Filters files for versioning">
+ <attribute name="todir" description="Destination for the filtered copy"/>
+
+ <element name="src.fileset" implicit="true"
+ description="Source for the filtered copy"/>
+ <sequential>
<!-- These files must be filtered for versioning -->
+ <echo message="Branding as GWT version ${gwt.version}, SVN rev ${gwt.svnrev}"/>
<mkdir dir="@{todir}" />
<copy todir="@{todir}" overwrite="true">
<src.fileset/>
@@ -236,7 +257,6 @@
<filter token="GWT_SVNREV" value="${gwt.svnrev}" />
</filterset>
</copy>
- <echo message="Branding as GWT version ${gwt.version}, SVN rev ${gwt.svnrev}"/>
</sequential>
</macrodef>
Index: dev/common.ant.xml
===================================================================
--- dev/common.ant.xml (revision 3642)
+++ dev/common.ant.xml (working copy)
@@ -19,8 +19,11 @@
<target name="build" depends="compile" description="Build and package this project">
<mkdir dir="${gwt.build.lib}" />
<gwt.jar>
- <fileset dir="src" excludes="**/package.html" />
- <fileset dir="${gwt.core.root}/src" excludes="**/package.html" />
+ <fileset dir="src" excludes="**/package.html"/>
+ <fileset dir="${gwt.core.root}/src">
+ <exclude name="**/package.html"/>
+ <exclude name="com/google/gwt/dev/About.properties"/>
+ </fileset>
<fileset dir="${gwt.core.root}/super" excludes="**/package.html" />
<fileset dir="${javac.out}" />
<fileset dir="${gwt.core.build}/bin" />
Index: dev/core/build.xml
===================================================================
--- dev/core/build.xml (revision 3642)
+++ dev/core/build.xml (working copy)
@@ -57,12 +57,12 @@
<property name="filter.pattern" value="com/google/gwt/dev/About.properties" />
- <target name="-filter.src" description="Creates filtered copies of source files" unless="filter.uptodate">
- <delete dir="${src.filtered}" failonerror="false" />
- <gwt.revfilter todir="${src.filtered}" >
+ <target name="-filter.props" description="Creates filtered About.properties with version info"
+ unless="filter.uptodate">
+ <gwt.revfilter todir="${javac.out}">
<fileset dir="src" includes="${filter.pattern}" />
</gwt.revfilter>
- <touch file="${src.filtered}/gwt.version-${gwt.version}" />
+ <touch file="${filter.sentinel}" />
</target>
<target name="build" depends="build.alldeps.jar" description="Compiles this project">
@@ -76,22 +76,8 @@
<mkdir dir="${javac.out-dummy}" />
<gwt.javac srcdir="src-dummy" destdir="${javac.out-dummy}" />
- <!-- Files with hardcoded version information must be filtered -->
- <property name="src.filtered" location="${project.build}/src-filtered" />
- <condition property="filter.uptodate">
- <and>
- <available file="${src.filtered}/gwt.version-${gwt.version}" />
- <uptodate>
- <srcfiles dir="src" includes="${filter.pattern}" />
- <globmapper from="*" to="${src.filtered}/*" />
- </uptodate>
- </and>
- </condition>
- <antcall target="-filter.src" />
-
<mkdir dir="${javac.out}" />
<gwt.javac srcdir="super" excludes="com/google/gwt/dev/jjs/intrinsic/"/>
- <gwt.javac srcdir="${src.filtered}" />
<gwt.javac srcdir="src" excludes="${filter.pattern}">
<classpath>
<pathelement location="${javac.out-dummy}" />
@@ -102,8 +88,21 @@
</classpath>
</gwt.javac>
<copy todir="${javac.out}">
- <fileset dir="src" includes="**/*.properties"/>
- </copy>
+ <fileset dir="src" includes="**/*.properties" excludes="${filter.pattern}"/>
+ </copy>
+
+ <!-- Files with hardcoded version information must be filtered -->
+ <gwt.getsvninfo />
+ <condition property="filter.uptodate">
+ <and>
+ <available file="${filter.sentinel}" />
+ <uptodate>
+ <srcfiles dir="src" includes="${filter.pattern}" />
+ <globmapper from="*" to="${javac.out}/*" />
+ </uptodate>
+ </and>
+ </condition>
+ <antcall target="-filter.props" />
</target>
<target name="checkstyle" description="Static analysis of source">
Index: dev/core/src/com/google/gwt/dev/About.java
===================================================================
--- dev/core/src/com/google/gwt/dev/About.java (revision 3642)
+++ dev/core/src/com/google/gwt/dev/About.java (working copy)
@@ -32,13 +32,10 @@
public static String GWT_VERSION;
- {
- Class<? extends About> myClass = this.getClass();
- String propsPath = myClass.getName().replace('.', '/').concat(".properties");
+ static {
Properties props = new Properties();
try {
- InputStream instream = myClass.getClassLoader().getResourceAsStream(
- propsPath);
+ InputStream instream = About.class.getResourceAsStream("About.properties");
props.load(instream);
} catch (IOException iox) {
// okay... we use default values, then.
Index: distro-source/common.ant.xml
===================================================================
--- distro-source/common.ant.xml (revision 3642)
+++ distro-source/common.ant.xml (working copy)
@@ -17,9 +17,24 @@
</patternset>
<target name="filter" description="Filters distro files for versioning">
+ <gwt.getsvninfo />
+ <condition property="filter.uptodate">
+ <and>
+ <available file="${filter.sentinel}" />
+ <uptodate>
+ <srcfiles dir="../core/src" />
+ <globmapper from="*" to="${project.build}/*" />
+ </uptodate>
+ </and>
+ </condition>
+ <antcall target="-filter.props" />
+ </target>
+
+ <target name="-filter.props" unless="filter.uptodate">
<gwt.revfilter todir="${project.build}" >
<fileset dir="../core/src" />
</gwt.revfilter>
+ <touch file="${filter.sentinel}" />
</target>
<target name="clean" description="Cleans this project's intermediate and output files">
Index: distro-source/linux/build.xml
===================================================================
--- distro-source/linux/build.xml (revision 3642)
+++ distro-source/linux/build.xml (working copy)
@@ -25,6 +25,7 @@
</tarfileset>
<tarfileset dir="${project.build}" prefix="${project.distname}">
<patternset refid="chmod.not.executables" />
+ <exclude name="sentinels/**"/>
</tarfileset>
<tarfileset dir="src" prefix="${project.distname}" mode="755">
<patternset refid="chmod.executables" />
Index: distro-source/mac/build.xml
===================================================================
--- distro-source/mac/build.xml (revision 3642)
+++ distro-source/mac/build.xml (working copy)
@@ -27,6 +27,7 @@
</tarfileset>
<tarfileset dir="${project.build}" prefix="${project.distname}">
<patternset refid="chmod.not.executables" />
+ <exclude name="sentinels/**"/>
</tarfileset>
<tarfileset dir="src" prefix="${project.distname}" mode="755">
<patternset refid="chmod.executables" />
Index: distro-source/windows/build.xml
===================================================================
--- distro-source/windows/build.xml (revision 3642)
+++ distro-source/windows/build.xml (working copy)
@@ -20,7 +20,9 @@
</zipfileset>
<!-- raw files -->
- <zipfileset dir="${project.build}" prefix="${project.distname}" />
+ <zipfileset dir="${project.build}" prefix="${project.distname}">
+ <exclude name="sentinels/**"/>
+ </zipfileset>
<zipfileset dir="src" prefix="${project.distname}" />
<!-- doc -->
