Bug 47003

Summary: Add to ant lib classpath from within project file
Product: Ant Reporter: Martin von Gagern <Martin.vGagern>
Component: Optional TasksAssignee: Ant Notifications List <notifications>
Status: NEW ---    
Severity: enhancement Keywords: PatchAvailable
Priority: P2    
Version: 1.7.1   
Target Milestone: ---   
Hardware: PC   
OS: Linux   
URL: https://github.com/apache/ant/pull/4
Attachments: Imrpove <classloader> task, always provide core loader

Description Martin von Gagern 2009-04-08 14:25:32 UTC
I'm trying to write ant build files that only require developers to have ant core installed, and resolve any optional tasks through ivy. For some tasks that works fairly well by <taskdef>ing them with a proper nested classpath argument. At least in the case of <junitreport>, however, this approach fails when trying to format the combined report, due to missing TraXLiaison. I just filed bug #47002 about this.

On the whole, a cleaner solution would be preferable, though. I would imagine some expandable class loader within ant itself. Then there could be some task which adds a given resource collection to the internal ant classpath, making it available whenever an UnknownElement is instantiated or a task is executed.

This way, I could declare all optional tasks in ivy, could tell ivy to retrieve them, then could ant to add them to its classpath, and start using the tasks without any need to redefine tasks or specify bulky classpaths. Especially now that ivy is an official ant project, I would consider such scenarios very useful.
Comment 1 Stefan Bodewig 2009-04-09 06:09:35 UTC
I think you are looking for the (undocumented) classloader task.
Comment 2 Martin von Gagern 2009-04-09 09:56:29 UTC
Created attachment 23476 [details]
Imrpove <classloader> task, always provide core loader

(In reply to comment #1)
> I think you are looking for the (undocumented) classloader task.

Almost, but not quite. <classloader> is a good first step, because it allows modification of an existing class loader, in theory even the core class loader.

It fails to achieve this, though, because it identified the core task loader using a reference name not used anywhere else. ComponentHelper.initTasks() comes before <classloader> is executed, and already fixes the association between standard tasks and the core class loader in use at that moment. Furthermore, when started from the command line, ant by default has null as its core class loader.

The attached patch addresses those issues. Firstly it changes Main.runBuild to always provide an AntClassLoader as the core loader, so there will always be a loader to which additional path elements can be added.

Secondly it changes the behaviour of the classloader task when name is omitted or specified as "ant.coreLoader". In that case, the loader is not looked up by reference, but instead the core loader of the project is used. This avoids having to register a reference to this loader anywhere else.

I've tested using this simple build file:

<project default="demo">
  <target name="demo" description="Demonstrate bug 47003">
    <echo message="$${ant-junit.jar}='${ant-junit.jar}'"/>
    <classloader classpath="${ant-junit.jar}"/>
    <junit/>
  </target>
</project>

I had only ant.jar inside ant.library.dir, but had ant-junit.jar point at that optional task. Before my patch, the build failed because of a missing JUnitTask. With the patch applied, it failed due to missing junit.jar, emitting an error message specific to a correctly loaded <junit> task.
Comment 3 Jesse Glick 2010-01-18 09:51:27 UTC
My preference would be for fixes to particular tasks (such as bug #47002) to ensure that any necessary libraries can be supplied using a regular <classpath> element. Injecting JARs into the main Ant classpath has at least two disadvantages:

1. It breaks modularity for large builds, parts of which may have different expectations about library versions and so on.

2. It will likely not work when Ant is run in an embedded environment. In particular, Main is not used at all when run from NetBeans and possibly other containers.
Comment 4 Martin von Gagern 2010-06-01 12:45:44 UTC
(In reply to comment #3)
> My preference would be for fixes to particular tasks (such as bug #47002) to
> ensure that any necessary libraries can be supplied using a regular
> <classpath> element.

I agree that this should be an option, for cases where tweaking the main class path are either impossible or a worse alternative. But one approach doesn't precluse the other; if we can have both ways to address the issue, why not expose them both to developers in a fairly usable way, and let them decide what works best for them?

> Injecting JARs into the main Ant classpath has at least two
> disadvantages:
> 
> 1. It breaks modularity for large builds, parts of which may have different
> expectations about library versions and so on.

In my experience, many large projects tend to ship a shell script and batch file which builds a class path to be passed to the java vm running ant, in order to ensure that all required things are available on the ant class path and able to cooperate smoothly. So in those cases, having a single target in build.xml replace the two shell scripts certainly is a benefit.

In cases where you really and truly need different library versions on the classpath for different tasks, then there should be a <classpath> element to configure this, but I consider this a rare case so developers shouldn't be forced to juggle class paths for those 98% of projects that can work quite well with a single homogenous classpath.

> 2. It will likely not work when Ant is run in an embedded environment. In
> particular, Main is not used at all when run from NetBeans and possibly other
> containers.

Good point. Do you know of some unique entry point suitable for this classpath manipulation?

I'm willing to improve my patch further, but I'd like to know if this whole approach has a chance of landing in ant core one day, before I invest any more of my free time into coding improvements. Seeing that even the simpler bug #47002 has been open for well over a year without so much as a comment, I feel a bit pessimistic there.
Comment 5 Stefan Bodewig 2014-11-27 16:54:31 UTC
I think the classloader task is a dirty little secret that we don't really want to feature prominently.  Note the lack of any manual page for it.

I personally don't see any harm with patching the task to do evil things as people who use it better know what they do.  I don't think the coreloader does what you hope for since it is part of a concept that has never been fully implemented IIRC.  You will better know then myself if this really helps your case.

In general I'm with Jesse and would prefer to patch individual tasks as needed.
Comment 6 Stefan Bodewig 2014-11-28 05:10:34 UTC
Martin has provided an updated version of the patch as pull request: https://github.com/apache/ant/pull/4
Comment 7 Martin von Gagern 2014-11-28 22:57:57 UTC
(In reply to Stefan Bodewig from comment #5)
> I think the classloader task is a dirty little secret that we don't really
> want to feature prominently.  Note the lack of any manual page for it.

Too bad, it looks like it could become a really powerful tool.

> I personally don't see any harm with patching the task to do evil things as
> people who use it better know what they do.

Does that mean you'd consider my patch, even though it patches not only the classloader task but also the project initialization? That would be great!

> I don't think the coreloader
> does what you hope for since it is part of a concept that has never been
> fully implemented IIRC.

Reading http://thread.gmane.org/gmane.comp.jakarta.ant.devel/12896 from 2002 it seems as if modifying the core class loader was Costin Manolache's primary goal of the <classloader> task. My changes would make that work as intended, I'd say.

In http://article.gmane.org/gmane.comp.jakarta.ant.devel/28676 from 2004 Rainer Noack proposed a rather big patch for the <classloader> task, which was discussed in http://thread.gmane.org/gmane.comp.jakarta.ant.devel/28881. To me it sounds like mostly positive feedback, and eventually bug #28228 was filed for this. I still haven't figured out all the stuff that patch does.

Which of these is the “concept that has never been fully implemented”? Or do you have something else in mind entirely?

> You will better know then myself if this really helps your case.

It would help in my case if it were available in official ant builds. At the moment, my workarounds using taskdefs cause a lot of code in the build file, making it harder to read.

> In general I'm with Jesse and would prefer to patch individual tasks as
> needed.

I don't think these two approaches are mutually exclusive. Yes, every aspect where a class path is consulted SHOULD be tweakable, to allow fine-grained access to all aspects of the configuration. On the other hand, a single line stating “well, just make that JAR available to all of Ant” would be very powerful and might eventually be preferred in many situations.
Comment 8 Martin von Gagern 2014-11-28 23:13:42 UTC
Looking around some more, I noticed bug #46060 which according to its title requests documentation of the <classloader> task, but actually containes a small to-do list. On that list is bug #40522 which seems to be VERY close to what I'm asking for here.

The approach taken by the patch attatched to that report is different from my own pull request, though. Where I concentrate on ensuring that there always is a core loader, and try to augment that core loader as needed, that patch aims at replacing the core loader, and upating task definitions to reflect this.

Personally I'd consider my own approach to be easier. We already have the ability to add URLs to an AntClassLoader, so making use of that feels natural. Since I don't replace an old class loader by a new one, I don't encounter problems about different definitionsof the same class. I'm also less likely to cause a class loader memory leak. But perhaps I'm missing something, so I'd welcome some discussion about the relative merits of these approaches.

The fact that essentially the same idea has cropped up so often indicates to me that there is some actual use case for this out there, not just in my mind. So I'd like to see a usable and documented <classloader>, one way or another.