You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by Curt Arnold <ca...@apache.org> on 2007/09/05 00:31:58 UTC

Rev 568096 breaks ant-contrib

Ant-contrib-test has been started failing on Gump sometime between  
2007-08-21T00:00 and 2007-08-22T00L:00 (I believe Pacific DT, but  
could be wrong) with the following test failures:

run-test:
     [junit] Testsuite: net.sf.antcontrib.design.VerifyDesignTest
     [junit] Tests run: 41, Failures: 0, Errors: 0, Time elapsed:  
15.651 sec
     [junit]
     [junit] Testsuite: net.sf.antcontrib.logic.AntCallBackTest
     [junit] Tests run: 5, Failures: 0, Errors: 0, Time elapsed:  
0.886 sec
     [junit]
     [junit] Testsuite: net.sf.antcontrib.logic.AssertTest
     [junit] Tests run: 8, Failures: 0, Errors: 2, Time elapsed:  
0.317 sec
     [junit]
     [junit] Testcase: test4(net.sf.antcontrib.logic.AssertTest):	 
Caused an ERROR
     [junit] both arg1 and arg2 are required in greater than
     [junit] /srv/gump/public/workspace/ant-contrib/test/resources/ 
logic/asserttest.xml:34: both arg1 and arg2 are required in greater than
     [junit] 	at net.sf.antcontrib.logic.condition.IsGreaterThan.eval 
(IsGreaterThan.java:64)
     [junit] 	at net.sf.antcontrib.logic.Assert.execute(Assert.java:102)
     [junit] 	at net.sf.antcontrib.logic.AssertTest.test4 
(AssertTest.java:57)
     [junit]
     [junit]
     [junit] Testcase: test5(net.sf.antcontrib.logic.AssertTest):	 
Caused an ERROR
     [junit] both arg1 and arg2 are required in less than
     [junit] /srv/gump/public/workspace/ant-contrib/test/resources/ 
logic/asserttest.xml:42: both arg1 and arg2 are required in less than
     [junit] 	at net.sf.antcontrib.logic.condition.IsLessThan.eval 
(IsLessThan.java:64)
     [junit] 	at net.sf.antcontrib.logic.Assert.execute(Assert.java:102)
     [junit] 	at net.sf.antcontrib.logic.AssertTest.test5 
(AssertTest.java:61)
     [junit]
     [junit]
     [junit] Test net.sf.antcontrib.logic.AssertTest FAILED
     [junit] Testsuite: net.sf.antcontrib.logic.ForTest
     [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed:  
0.076 sec

...
     [junit]
     [junit] Testsuite: net.sf.antcontrib.process.LimitTest
     [junit] Tests run: 2, Failures: 0, Errors: 1, Time elapsed:  
2.072 sec
     [junit]
     [junit] Testcase: test2(net.sf.antcontrib.process.LimitTest):	 
Caused an ERROR
     [junit] both arg1 and arg2 are required in less than
     [junit] /srv/gump/public/workspace/ant-contrib/test/resources/ 
logic/limittest.xml:32: both arg1 and arg2 are required in less than
     [junit] 	at net.sf.antcontrib.logic.condition.IsLessThan.eval 
(IsLessThan.java:64)
     [junit] 	at net.sf.antcontrib.logic.Assert.execute(Assert.java:102)
     [junit] 	at net.sf.antcontrib.process.LimitTest.test2 
(LimitTest.java:35)
     [junit]
     [junit]
     [junit] Test net.sf.antcontrib.process.LimitTest FAILED
     [junit] Testsuite: net.sf.antcontrib.property.PathToFileSetTest
     [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed:  
0.112 sec

...

and continues in that state see http://vmgump.apache.org/gump/public/ 
ant-contrib/ant-contrib-test/index.html.

There were no changes to ant-contrib during that time, but there were  
many changes to Ant.  I went through and ran the ant-contrib-tests  
against various revisions and it appears that ant-contrib would pass  
with rev 568091 and failed with 568096 and every later release I tried.

To run the test, check ant-contrib out from SVN:

$ svn co https://ant-contrib.svn.sourceforge.net/svnroot/ant-contrib/ 
ant-contrib/trunk ant-contrib
$ cd ant-contrib
'
'
' will run ant-contrib against Ant 1.6.5, but will also download  
dependencies.  Should pass.
$ mvn package
'
'  will pass or fail depending on whether a pre or post rev 568096  
ant is on the path
'
$ ant -Dno-ivy=true clean gump-safe-test -lib ~/.m2/repository/junit/ 
junit/3.8.1/junit-3.8.1.jar

The offending rev does deal with Introspection and seems that it  
could interfere with proper initialization of tasks, but I don't see  
any obvious error in the patch.  It doesn't have a related bug  
report, so I don't know what it is trying to fix.

Index: src/main/org/apache/tools/ant/IntrospectionHelper.java
===================================================================
--- src/main/org/apache/tools/ant/IntrospectionHelper.java	(revision  
568091)
+++ src/main/org/apache/tools/ant/IntrospectionHelper.java	(revision  
568096)
@@ -943,6 +943,17 @@
          final Class reflectedArg = PRIMITIVE_TYPE_MAP.containsKey(arg)
              ? (Class) PRIMITIVE_TYPE_MAP.get(arg) : arg;
+        // Object.class - it gets handled differently by  
AttributeSetter
+        if (java.lang.Object.class == reflectedArg) {
+            return new AttributeSetter(m, arg) {
+                public void set(Project p, Object parent, String value)
+                        throws InvocationTargetException,
+                    IllegalAccessException {
+                    throw new BuildException(
+                        "Internal ant problem - this should not get  
called");
+                }
+            };
+        }
          // simplest case - setAttribute expects String
          if (java.lang.String.class.equals(reflectedArg)) {
              return new AttributeSetter(m, arg) {


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: Rev 568096 breaks ant-contrib

Posted by Matt Benson <gu...@yahoo.com>.
--- Stefan Bodewig <bo...@apache.org> wrote:

> On Wed, 05 Sep 2007, Stefan Bodewig
> <bo...@apache.org> wrote:
> 
> > Instead of what we have in Equals it should
> probably be
> > 
> > public void setArg1(Object a) {
> >     if (a instanceof String) {
> >         setArg1((String) a);
> >     } else {
> >         somePrivateArg1Setter(a);
> >     }
> > }
> > 
> > public void setArg1(String a) {
> >     somePrivateArg1Setter(a);
> > }
> 
> Is anything wrong with that approach?

I had to go back and fully understand what was going
on, but I see now what you're doing with this
change... yes, it does seem like this could solve the
BC issue here.

Although I actually can't see any reason why IsGreater
extends Equals anyway, but once again, that would
theoretically be a BC issue for ac if they took it out
now.  Anyway, I forgive you, Dale... ;)  Or if I've
missed some obvious reason for the inheritance, you
can forgive me!

-Matt

> 
> Stefan
> 
>
---------------------------------------------------------------------
> To unsubscribe, e-mail:
> dev-unsubscribe@ant.apache.org
> For additional commands, e-mail:
> dev-help@ant.apache.org
> 
> 



       
____________________________________________________________________________________
Boardwalk for $500? In 2007? Ha! Play Monopoly Here and Now (it's updated for today's economy) at Yahoo! Games.
http://get.games.yahoo.com/proddesc?gamekey=monopolyherenow  

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: Rev 568096 breaks ant-contrib

Posted by Stefan Bodewig <bo...@apache.org>.
On Wed, 05 Sep 2007, Stefan Bodewig <bo...@apache.org> wrote:

> Instead of what we have in Equals it should probably be
> 
> public void setArg1(Object a) {
>     if (a instanceof String) {
>         setArg1((String) a);
>     } else {
>         somePrivateArg1Setter(a);
>     }
> }
> 
> public void setArg1(String a) {
>     somePrivateArg1Setter(a);
> }

Is anything wrong with that approach?

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: Rev 568096 breaks ant-contrib

Posted by Stefan Bodewig <bo...@apache.org>.
On Wed, 05 Sep 2007, Stefan Bodewig <bo...@apache.org> wrote:

> Equals now has changed the signature of the setters to take Object
> arguments (rev 557097) so setArg1 in IsGreateThan no longer
> overrides the one in Equals.

I see the String setters haven't been removed.  In order to have
subclasses work as expected the deleagtion must be changed.

Instead of what we have in Equals it should probably be

public void setArg1(Object a) {
    if (a instanceof String) {
        setArg1((String) a);
    } else {
        somePrivateArg1Setter(a);
    }
}

public void setArg1(String a) {
    somePrivateArg1Setter(a);
}

I haven't changed the code yet, 'cause I'm waiting for feedback.  Also
Equals probably isn't the only place and I don't feel like searching
through the commit logs 8-)

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: Rev 568096 breaks ant-contrib

Posted by Curt Arnold <ca...@apache.org>.
On Sep 5, 2007, at 10:23 AM, Peter Reilly wrote:

> On 9/5/07, Steve Loughran <st...@apache.org> wrote:
>> Matt Benson wrote:
>>
>>> Great diagnosis, Stefan.  I will revert the changes to
>>> <equals> tomorrow.  Does anyone object to a new
>>> <objectsequal> task?
>>
>> we can change <equals> to compare objects, just bind it to the new
>> ObjectsEqual condition...
>>
> No we cannot!
> I was about to suggest this, but then I remembered the binding
> is done by reflection in the ConditionBase class.
>
> Peter
>

I'm coming in late, I'd like to follow what is going on, but I didn't  
find any obvious place that described the new feature that was  
motivating the original change.  How exactly would you use this new  
object comparison feature and what would it accomplish?  Could have a  
base class that could take a strategy and use it as the base for  
Equals, IsGreaterThan, etc?  Mapping to objectequals does seem  
undesirable from a user perspective if it is a superset of the  
capabilities of the existing equals task.

Also, might be good to run CLIRR, jdiff or similar to detect any  
other api changes.



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: Rev 568096 breaks ant-contrib

Posted by Peter Reilly <pe...@gmail.com>.
On 9/5/07, Steve Loughran <st...@apache.org> wrote:
> Matt Benson wrote:
>
> > Great diagnosis, Stefan.  I will revert the changes to
> > <equals> tomorrow.  Does anyone object to a new
> > <objectsequal> task?
>
> we can change <equals> to compare objects, just bind it to the new
> ObjectsEqual condition...
>
No we cannot!
I was about to suggest this, but then I remembered the binding
is done by reflection in the ConditionBase class.

Peter

> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: Rev 568096 breaks ant-contrib

Posted by Steve Loughran <st...@apache.org>.
Matt Benson wrote:

> Great diagnosis, Stefan.  I will revert the changes to
> <equals> tomorrow.  Does anyone object to a new
> <objectsequal> task?

we can change <equals> to compare objects, just bind it to the new 
ObjectsEqual condition...

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: Rev 568096 breaks ant-contrib

Posted by Matt Benson <gu...@yahoo.com>.
--- Stefan Bodewig <bo...@apache.org> wrote:

> On Tue, 4 Sep 2007, Curt Arnold <ca...@apache.org>
> wrote:
> 
> > The offending rev does deal with Introspection and
> seems that it
> > could interfere with proper initialization of
> tasks, but I don't see
> > any obvious error in the patch.  It doesn't have a
> related bug
> > report, so I don't know what it is trying to fix.
> 
> IsGreaterThan extends Equals.  It has setters for
> arg1 and arg2 that
> take String parameters which used to override the
> ones in Equals.
> Equals now has changed the signature of the setters
> to take Object
> arguments (rev 557097) so setArg1 in IsGreateThan no
> longer overrides
> the one in Equals.
> 
> So IsGreaterThan ends up with to setters for arg1
> and I the change in
> IntrospectionHelper favors the Object version which
> means the
> IsGreaterThan condition never gets its args.
> 
> Given that I haven't followed the PropertyHelper
> stuff close enough I
> don't know whether simply moving up the String
> setters could work.
> 
> Also, we shouldn't change the setter signatures in
> existing tasks,
> types, conditions and so on (at least for non-final
> methods) since
> there may always be a subclass lurking somewhere.
> 

Great diagnosis, Stefan.  I will revert the changes to
<equals> tomorrow.  Does anyone object to a new
<objectsequal> task?

-Matt

> Stefan
> 
>
---------------------------------------------------------------------
> To unsubscribe, e-mail:
> dev-unsubscribe@ant.apache.org
> For additional commands, e-mail:
> dev-help@ant.apache.org
> 
> 



       
____________________________________________________________________________________
Take the Internet to Go: Yahoo!Go puts the Internet in your pocket: mail, news, photos & more. 
http://mobile.yahoo.com/go?refer=1GNXIC

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: Rev 568096 breaks ant-contrib

Posted by Stefan Bodewig <bo...@apache.org>.
On Tue, 4 Sep 2007, Curt Arnold <ca...@apache.org> wrote:

> The offending rev does deal with Introspection and seems that it
> could interfere with proper initialization of tasks, but I don't see
> any obvious error in the patch.  It doesn't have a related bug
> report, so I don't know what it is trying to fix.

IsGreaterThan extends Equals.  It has setters for arg1 and arg2 that
take String parameters which used to override the ones in Equals.
Equals now has changed the signature of the setters to take Object
arguments (rev 557097) so setArg1 in IsGreateThan no longer overrides
the one in Equals.

So IsGreaterThan ends up with to setters for arg1 and I the change in
IntrospectionHelper favors the Object version which means the
IsGreaterThan condition never gets its args.

Given that I haven't followed the PropertyHelper stuff close enough I
don't know whether simply moving up the String setters could work.

Also, we shouldn't change the setter signatures in existing tasks,
types, conditions and so on (at least for non-final methods) since
there may always be a subclass lurking somewhere.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: Rev 568096 breaks ant-contrib

Posted by Matt Benson <gu...@yahoo.com>.
Thanks for the note.  One or more of us will be
looking into it.  :)

-Matt

--- Curt Arnold <ca...@apache.org> wrote:

> Ant-contrib-test has been started failing on Gump
> sometime between  
> 2007-08-21T00:00 and 2007-08-22T00L:00 (I believe
> Pacific DT, but  
> could be wrong) with the following test failures:
> 
> run-test:
>      [junit] Testsuite:
> net.sf.antcontrib.design.VerifyDesignTest
>      [junit] Tests run: 41, Failures: 0, Errors: 0,
> Time elapsed:  
> 15.651 sec
>      [junit]
>      [junit] Testsuite:
> net.sf.antcontrib.logic.AntCallBackTest
>      [junit] Tests run: 5, Failures: 0, Errors: 0,
> Time elapsed:  
> 0.886 sec
>      [junit]
>      [junit] Testsuite:
> net.sf.antcontrib.logic.AssertTest
>      [junit] Tests run: 8, Failures: 0, Errors: 2,
> Time elapsed:  
> 0.317 sec
>      [junit]
>      [junit] Testcase:
> test4(net.sf.antcontrib.logic.AssertTest):	 
> Caused an ERROR
>      [junit] both arg1 and arg2 are required in
> greater than
>      [junit]
>
/srv/gump/public/workspace/ant-contrib/test/resources/
> 
> logic/asserttest.xml:34: both arg1 and arg2 are
> required in greater than
>      [junit] 	at
> net.sf.antcontrib.logic.condition.IsGreaterThan.eval
> 
> (IsGreaterThan.java:64)
>      [junit] 	at
>
net.sf.antcontrib.logic.Assert.execute(Assert.java:102)
>      [junit] 	at
> net.sf.antcontrib.logic.AssertTest.test4 
> (AssertTest.java:57)
>      [junit]
>      [junit]
>      [junit] Testcase:
> test5(net.sf.antcontrib.logic.AssertTest):	 
> Caused an ERROR
>      [junit] both arg1 and arg2 are required in less
> than
>      [junit]
>
/srv/gump/public/workspace/ant-contrib/test/resources/
> 
> logic/asserttest.xml:42: both arg1 and arg2 are
> required in less than
>      [junit] 	at
> net.sf.antcontrib.logic.condition.IsLessThan.eval 
> (IsLessThan.java:64)
>      [junit] 	at
>
net.sf.antcontrib.logic.Assert.execute(Assert.java:102)
>      [junit] 	at
> net.sf.antcontrib.logic.AssertTest.test5 
> (AssertTest.java:61)
>      [junit]
>      [junit]
>      [junit] Test net.sf.antcontrib.logic.AssertTest
> FAILED
>      [junit] Testsuite:
> net.sf.antcontrib.logic.ForTest
>      [junit] Tests run: 2, Failures: 0, Errors: 0,
> Time elapsed:  
> 0.076 sec
> 
> ...
>      [junit]
>      [junit] Testsuite:
> net.sf.antcontrib.process.LimitTest
>      [junit] Tests run: 2, Failures: 0, Errors: 1,
> Time elapsed:  
> 2.072 sec
>      [junit]
>      [junit] Testcase:
> test2(net.sf.antcontrib.process.LimitTest):	 
> Caused an ERROR
>      [junit] both arg1 and arg2 are required in less
> than
>      [junit]
>
/srv/gump/public/workspace/ant-contrib/test/resources/
> 
> logic/limittest.xml:32: both arg1 and arg2 are
> required in less than
>      [junit] 	at
> net.sf.antcontrib.logic.condition.IsLessThan.eval 
> (IsLessThan.java:64)
>      [junit] 	at
>
net.sf.antcontrib.logic.Assert.execute(Assert.java:102)
>      [junit] 	at
> net.sf.antcontrib.process.LimitTest.test2 
> (LimitTest.java:35)
>      [junit]
>      [junit]
>      [junit] Test
> net.sf.antcontrib.process.LimitTest FAILED
>      [junit] Testsuite:
> net.sf.antcontrib.property.PathToFileSetTest
>      [junit] Tests run: 2, Failures: 0, Errors: 0,
> Time elapsed:  
> 0.112 sec
> 
> ...
> 
> and continues in that state see
> http://vmgump.apache.org/gump/public/ 
> ant-contrib/ant-contrib-test/index.html.
> 
> There were no changes to ant-contrib during that
> time, but there were  
> many changes to Ant.  I went through and ran the
> ant-contrib-tests  
> against various revisions and it appears that
> ant-contrib would pass  
> with rev 568091 and failed with 568096 and every
> later release I tried.
> 
> To run the test, check ant-contrib out from SVN:
> 
> $ svn co
>
https://ant-contrib.svn.sourceforge.net/svnroot/ant-contrib/
> 
> ant-contrib/trunk ant-contrib
> $ cd ant-contrib
> '
> '
> ' will run ant-contrib against Ant 1.6.5, but will
> also download  
> dependencies.  Should pass.
> $ mvn package
> '
> '  will pass or fail depending on whether a pre or
> post rev 568096  
> ant is on the path
> '
> $ ant -Dno-ivy=true clean gump-safe-test -lib
> ~/.m2/repository/junit/ 
> junit/3.8.1/junit-3.8.1.jar
> 
> The offending rev does deal with Introspection and
> seems that it  
> could interfere with proper initialization of tasks,
> but I don't see  
> any obvious error in the patch.  It doesn't have a
> related bug  
> report, so I don't know what it is trying to fix.
> 
> Index:
>
src/main/org/apache/tools/ant/IntrospectionHelper.java
>
===================================================================
> ---
>
src/main/org/apache/tools/ant/IntrospectionHelper.java
> (revision  
> 568091)
> +++
>
src/main/org/apache/tools/ant/IntrospectionHelper.java
> (revision  
> 568096)
> @@ -943,6 +943,17 @@
>           final Class reflectedArg =
> PRIMITIVE_TYPE_MAP.containsKey(arg)
>               ? (Class) PRIMITIVE_TYPE_MAP.get(arg)
> : arg;
> +        // Object.class - it gets handled
> differently by  
> AttributeSetter
> +        if (java.lang.Object.class == reflectedArg)
> {
> +            return new AttributeSetter(m, arg) {
> +                public void set(Project p, Object
> parent, String value)
> +                        throws
> InvocationTargetException,
> +                    IllegalAccessException {
> +                    throw new BuildException(
> +                        "Internal ant problem -
> this should not get  
> called");
> +                }
> +            };
> +        }
>           // simplest case - setAttribute expects
> String
>           if
> (java.lang.String.class.equals(reflectedArg)) {
>               return new AttributeSetter(m, arg) {
> 
> 
>
---------------------------------------------------------------------
> To unsubscribe, e-mail:
> dev-unsubscribe@ant.apache.org
> For additional commands, e-mail:
> dev-help@ant.apache.org
> 
> 



       
____________________________________________________________________________________Ready for the edge of your seat? 
Check out tonight's top picks on Yahoo! TV. 
http://tv.yahoo.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org