Conor MacNeill wrote: > Costin Manolache wrote: >> I would like to check in the (updated) <classloader> and <import> >> tasks into the MAIN branch. >> > > Costin, > > I haven't had time to review this in-depth but I would like to provide you > with some quick feedback since I'm going offline for a while. > > 1. The use of ProjectHelper2 in the ImportTask > > As I understand it, ProjectHelper2 is an implementation of the > ProjectHelper plugin. As such, I don't think Tasks, such as ImportTask, > should have a direct dependency on this class. Someone should be able to > use another ProjectHelper without requiring ProjectHelper2 to be around. > > It seems this is there just for the AntXmlContext class. Can this be made > a top level class and moved out of the helper package.
Good point. For the moment I think it would be better to wait until things stabilise ( I assume people will want changes in import and the project helper and we may find bugs ). Then add a (minimal) subset of AntXmlContext, or find another solution to abstract this. The APIs we add in core will haunt us a very long time :-) > 2. Classloader > > For the classloader task, you stated "It'll create a new loader or add > paths to an existing loader." I'm not sure that modifying a classloader is > safe after it has started serving classes. > The scenario I'm considering involves a child loader and the following steps > 1. Class A is requested from the child loader. The child loader first > tries the parent loader which cannot find class A. The child loader then > tries to load class A and succeeds. > > 2. The parent class loader is modified to add a path containing class A. > > 3. Class B is requested from the child loader. The child loader delegates > to the parent which finds Class B. > > 4. Class B has a dependency on Class A which the VM attempts to load > through > Class B's class loader. Since this loader has been modified, Class A is > found. > > 5. Class A has now been loaded by a class loader and its child loader. > This will most likely lead to violation of loader constraints and > mysterious failures. > > Since you are proposing this to allow modification of the core loader, I > think it may cause problems. There are countless scenarios where a class ends up loaded by 2 loaders. The whole reason for adding paths to the core loader is to avoid this - for most common use case. IMO most people will use <classloader> to add jars like junit.jar and other dependencies and optional tasks ( and antlibs ) to the core loader. That means all tasks and libraries will be loaded with the same loader ( the core loader ), and will behave almost like if they would be in CLASSPATH. I think creating new loaders is for advanced users - and I doubt we can ever have a loader mechanism to completely avoid class cast/linkage errors or loader constraint violations ( especially if "reverse" loader is used ). You could specify that you want the system loader as parent - and avoid the core loader. One interesting solution is in jboss - where each jar is loaded in a different loader, and a parent loader does some tricks so it appears it loads all classes. But that is basically the same thing ( a single loader is visible ) - the only benefit is reloading, which is not yet an issue for ant. Costin -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
