Re: Junit Task warning about multiple versions of Ant

[Nicolas Lalevée]

I have been able to make unit test which reproduces the false warning. And I 
have fixed the way the classloader was created. Thanks for the insights.

Nicolas

> Le 13 avr. 2018 à 07:38, Stefan Bodewig  a écrit :
> 
> On 2018-04-12, Nicolas Lalevée wrote:
> 
>> As far as I can see, the classpath used by checkForkedPath is the
>> proper one. The function which manipulates the classpath to add the
>> Ant runtime [1] is called before. So I should start looking into the
>> AntClassLoader which is improperly finding the Ant classes. Maybe we
>> should « isolate » it.
> 
> Sounds reasonable, so we can ensure it really only contains what will be
> on -classpath as we..
> 
>> Or maybe that check for duplicate ant jar is only useful when
>> includeantruntime is _not_ set to « no ». Since includeantruntime is
>> true by default, it is nice that Ant is printing a warning when it
>> also find Ant classes in the provided classpath, it is a common
>> pitfall. But when includeantruntime is explicitely set to false, then
>> I would say that the user know what he's doing, thus no need for
>> special check.
> 
> I'm not sure. To be honest the check and message are there so we get
> fewer bug reports by helping people figure out their problem
> themselves. At one point in time this has been a very common problem,
> which likely predated the includeAntRuntime attribute. If we manage to
> isolate the classloader (the infrastructure should be there) then we
> shouldn't need to disable the check.
> 
> Stefan
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
> For additional commands, e-mail: dev-h...@ant.apache.org
> 


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

Re: Junit Task warning about multiple versions of Ant

[Stefan Bodewig]

On 2018-04-12, Nicolas Lalevée wrote:

> As far as I can see, the classpath used by checkForkedPath is the
> proper one. The function which manipulates the classpath to add the
> Ant runtime [1] is called before. So I should start looking into the
> AntClassLoader which is improperly finding the Ant classes. Maybe we
> should « isolate » it.

Sounds reasonable, so we can ensure it really only contains what will be
on -classpath as we..

> Or maybe that check for duplicate ant jar is only useful when
> includeantruntime is _not_ set to « no ». Since includeantruntime is
> true by default, it is nice that Ant is printing a warning when it
> also find Ant classes in the provided classpath, it is a common
> pitfall. But when includeantruntime is explicitely set to false, then
> I would say that the user know what he's doing, thus no need for
> special check.

I'm not sure. To be honest the check and message are there so we get
fewer bug reports by helping people figure out their problem
themselves. At one point in time this has been a very common problem,
which likely predated the includeAntRuntime attribute. If we manage to
isolate the classloader (the infrastructure should be there) then we
shouldn't need to disable the check.

Stefan

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

Re: Junit Task warning about multiple versions of Ant

[Nicolas Lalevée]

> Le 12 avr. 2018 à 16:57, Stefan Bodewig  a écrit :
> 
> On 2018-04-12, Nicolas Lalevée wrote:
> 
>> The Junit task is printing a warning if it finds multiple versions of
>> Ant in the classpath of the unit tests. It seems it doesn’t do
>> correctly the job if the ant runtime is explicitly removed from the
>> classpath.
> 
> Quite possible.
> 
>> Here the function which checks the classpath:
>> https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1362
>>  
>> 
> 
> this is in the forked case.
> 
>> And here is the one which build the classloader during the actual forked run:
>> https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1952
>>  
>> 
> 
> AFAICT createClassloader is not invoked for forked VMs, only in the
> non-forked case.
> 
>> Shouldn’t the classloader be built the same way in both function?
> 
> In the forked case, the classloader is not built by the task, the
> CommandLineJava instance collects the classpath and sets it as
> -classpath command line argument.

Ha yes, you’re right.

As far as I can see, the classpath used by checkForkedPath is the proper one. 
The function which manipulates the classpath to add the Ant runtime [1] is 
called before. So I should start looking into the AntClassLoader which is 
improperly finding the Ant classes. Maybe we should « isolate » it.

Or maybe that check for duplicate ant jar is only useful when includeantruntime 
is _not_ set to « no ». Since includeantruntime is true by default, it is nice 
that Ant is printing a warning when it also find Ant classes in the provided 
classpath, it is a common pitfall. But when includeantruntime is explicitely 
set to false, then I would say that the user know what he's doing, thus no need 
for special check.

Nicolas

[1] 
https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1320
 

Re: Junit Task warning about multiple versions of Ant

[Stefan Bodewig]

On 2018-04-12, Nicolas Lalevée wrote:

> The Junit task is printing a warning if it finds multiple versions of
> Ant in the classpath of the unit tests. It seems it doesn’t do
> correctly the job if the ant runtime is explicitly removed from the
> classpath.

Quite possible.

> Here the function which checks the classpath:
> https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1362
>  
> 

this is in the forked case.

> And here is the one which build the classloader during the actual forked run:
> https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1952
>  
> 

AFAICT createClassloader is not invoked for forked VMs, only in the
non-forked case.

> Shouldn’t the classloader be built the same way in both function?

In the forked case, the classloader is not built by the task, the
CommandLineJava instance collects the classpath and sets it as
-classpath command line argument.

> trying to release Ivy, finding bugs in Ant :p

:-)

Stefan

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