Re: Junit Task warning about multiple versions of Ant

2018-04-13 Thread 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: ant git commit: More isEmpty()

2018-04-13 Thread Jaikiran Pai


On 11/04/18 1:27 PM, Stefan Bodewig wrote:

On 2018-04-06,  wrote:


http://git-wip-us.apache.org/repos/asf/ant/blob/c3b91f90/src/main/org/apache/tools/ant/types/ArchiveFileSet.java
--
diff --git a/src/main/org/apache/tools/ant/types/ArchiveFileSet.java 
b/src/main/org/apache/tools/ant/types/ArchiveFileSet.java
index e4b9d12..eea603d 100644
--- a/src/main/org/apache/tools/ant/types/ArchiveFileSet.java

b/src/main/org/apache/tools/ant/types/ArchiveFileSet.java

@@ -223,7 +223,7 @@ public abstract class ArchiveFileSet extends FileSet {
   */
  public void setPrefix(String prefix) {
  checkArchiveAttributesAllowed();
-if (!"".equals(prefix) && !"".equals(fullpath)) {
+if (!prefix.isEmpty() && !fullpath.isEmpty()) {
  throw new BuildException(ERROR_PATH_AND_PREFIX);
  }
  this.prefix = prefix;
@@ -250,7 +250,7 @@ public abstract class ArchiveFileSet extends FileSet {
   */
  public void setFullpath(String fullpath) {
  checkArchiveAttributesAllowed();
-if (!"".equals(prefix) && !"".equals(fullpath)) {
+if (!prefix.isEmpty() && !fullpath.isEmpty()) {
  throw new BuildException(ERROR_PATH_AND_PREFIX);
  }
  this.fullpath = fullpath;

in both hunks prefix or fullpath could be null. Obviously the old code
doesn't handle this properly either. Do we want to keep assuming nobody
invokes either method with a null argument? I'm afraid we aren't really
consistent with the attribute setters dealing with null args.

I think we could probably document that passing null to such attribute 
setters will lead to NullPointerException, which most likely will be the 
case in a lot of places other than those that just set the incoming 
value to some member variable.


-Jaikiran


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



Re: Mass changes to various projects under Ant umbrella - should we be doing it?

2018-04-13 Thread Gintautas Grigelionis
2018-04-13 5:33 GMT+00:00 Stefan Bodewig :

> On 2018-04-12, Gintautas Grigelionis wrote:
>
> > 2018-04-12 15:18 GMT+00:00 Stefan Bodewig :
>
> >> On 2018-04-12, Gintautas Grigelionis wrote:
>
> >>> 2018-04-12 14:39 GMT+00:00 Stefan Bodewig :
>
>  On 2018-04-11, Gintautas Grigelionis wrote:
>
> > XMLCatalogTest is an example of unit tests complicated by root
> >> property.
>
>  Actually, the test even passes if you remove the line using the
> property
>  as long as your current working directory is Ant's basedir :-)
>
> >>> What is the reason of having and/or keeping the property, anyway?
>
> >> Running the test when your current working directory is not the root of
> >> the Ant source tree. I don't know who does so, but it has been something
> >> somebody needed some time ago (and took the time to make it
> >> possible). Right now I don't see any reason to stop supporting that.
>
> > I'd say it's the other way around: unless I'm mistaken, I have to
> > remember to set the property in the IDEs that I use if I want to use a
> > test case for debugging. So IMHO it's an unnecessary complication for
> > no good reason.
>
> Ant test cases are not designed to be run from an IDE, this has never
> been a goal. I'm surprised this system property is the only thing you
> need to remember :-)
>

 Well, I decided to give it a spin :-) I find it useful to run debugger on
test cases from IDE.