Re: Cleanup for ClasspathUtils

2003-04-23 Thread Stefan Bodewig
On Fri, 18 Apr 2003, Marc Portier [EMAIL PROTECTED] wrote:

 1. ClasspathUtils duplicates (i.e. stole) some code from the
 o.a.t.a.taskdefs.Definer: the least I should do is refactor that one
 to now use what is in the ClasspathUtils.

This is now in CVS.

I noted at least one difference, and I'm not sure whether it is going
to have any effect right now, but we may need to address this: If no
classpath had been defined, Definer would first check the project's
core loader and use that if set.  ClasspathUtils ignores this.

 While this is not that much of code I still would like to add to
 this some Helper that does all of this for you,

It's also in.

BTW

 public void setClasspathRef(Reference r) {
  this.cpHelper.setClasspathRef(r);
 }

I wouldn't implement this and setClasspath(Path) at all if I were to
write a new task today.  I'd simply stick with the nested element.

 (what about these ./proposal things?)

Ignore them for now.

 I take it the prefered patch format is cvs -q diff -u -N ?

Yep.  The -q is your choice, off course.

 tests are currently not working in cvs head

I think some of the failures you've seen are due to the copy
changes the line-end style bug that is supposed to be fixed now.  If
things keep failing, nag this list.  ant test is supposed to pass.

Stefan


Cleanup for ClasspathUtils

2003-04-18 Thread Marc Portier
Hi there again,
(more background reading in
- http://marc.theaimsgroup.com/?l=ant-devm=105006876127852w=2
- http://issues.apache.org/bugzilla/show_bug.cgi?id=18906)
I'm finally finding the time to get into more of the proposed 
(promised) refactoring of the classloading stuff...

I know there is reworks in that area expected for 1.6 but I'm 
just assuming that work will be easier after some rationalisation 
and refactoring.

This mail is mainly there to let you know what I'm thinking off 
doing (so you get the chance to stop me :-))

1. ClasspathUtils duplicates (i.e. stole) some code from the 
o.a.t.a.taskdefs.Definer: the least I should do is refactor that 
one to now use what is in the ClasspathUtils.

2. ClasspathUtils currently advises people to use it like this:
public void setClasspathRef(Reference r) {
this.classpathId = r.getRefId();
createClasspath().setRefid(r);
}
public Path createClasspath() {
if (this.classpath == null) {
this.classpath = new Path(getProject());
}
return this.classpath.createPath();
}
public void setClassname(String fqcn) {
this.classname = fqcn;
}
when you actually need the classloading you can just:/p
ClassLoader cl = ClasspathUtils.getClassLoaderForPath(
this.classpath, this.classpathId);
Object o = ClasspathUtils.newInstance(this.classname, cl);
While this is not that much of code I still would like to add to 
this some Helper that does all of this for you, so you're custom 
task or type that needs classloading would just

ClasspathUtil.Helper cpHelper;
public void init(){
this.cpHelper = ClasspathUtil.getHelper(this);
}
public void setClasspathRef(Reference r) {
this.cpHelper.setClasspathRef(r);
}
public Path createClasspath() {
return this.cpHelper.createClasspath();
}
public void setClassname(String fqcn) {
this.cpHelper.setClassname(fqcn);
}
so when you need the particular instance you just do a
Object o = this.cpHelper.newInstance();
3. Will do a scan of where related stuff is going on
grepping for ant.reuse.loader and setClasspath(ref)
$ find -name *.java -exec grep -H ant.reuse.loader {} \;
./proposal/embed/src/java/org/apache/tools/ant/taskdefs/Taskdef2.java
./src/main/org/apache/tools/ant/taskdefs/Definer.java
$ find -name *.java -exec grep -H 'void setClasspath' {} \;
./proposal/embed/src/java/org/apache/tools/ant/taskdefs/SystemPath.java: 

./proposal/embed/src/java/org/apache/tools/ant/taskdefs/Taskdef2.java: 

./proposal/sandbox/antlib/src/main/org/apache/tools/ant/taskdefs/Antlib.java: 
   ./src/main/org/apache/tools/ant/taskdefs/Available.java: 
./src/main/org/apache/tools/ant/taskdefs/Classloader.java: 
./src/main/org/apache/tools/ant/taskdefs/Definer.java:
./src/main/org/apache/tools/ant/taskdefs/ExecuteJava.java:
./src/main/org/apache/tools/ant/taskdefs/Java.java:
./src/main/org/apache/tools/ant/taskdefs/Javac.java:
./src/main/org/apache/tools/ant/taskdefs/Javadoc.java:
./src/main/org/apache/tools/ant/taskdefs/JDBCTask.java:
./src/main/org/apache/tools/ant/taskdefs/optional/depend/Depend.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/ejb/BorlandGenerateClient.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/ejb/DDCreator.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/ejb/Ejbc.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/ejb/EjbJar.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/ejb/GenericDeploymentTool.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/ejb/IPlanetEjbcTask.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/ejb/WLRun.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/ejb/WLStop.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/IContract.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/j2ee/AbstractHotDeploymentTool.java: 
   ./src/main/org/apache/tools/ant/taskdefs/optional/Javah.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/jdepend/JDependTask.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/jsp/JspC.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/jsp/WLJspc.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/NetRexxC.java: 

./src/main/org/apache/tools/ant/taskdefs/optional/XMLValidateTask.java: 
   ./src/main/org/apache/tools/ant/taskdefs/Property.java:
./src/main/org/apache/tools/ant/taskdefs/Rmic.java:
./src/main/org/apache/tools/ant/taskdefs/WhichResource.java: 
./src/main/org/apache/tools/ant/taskdefs/XSLTProcess.java: 
./src/main/org/apache/tools/ant/types/AntFilterReader.java: 
./src/main/org/apache/tools/ant/types/Mapper.java: 
./src/main/org/apache/tools/ant/types/selectors/ExtendSelector.java: 
   ./src/main/org/apache/tools/ant/types/XMLCatalog.java:

offering me quite some work-list, but I assume not all of them 
need the same treatement...
(what about these ./proposal things?)

Finally some practical stuff:
- I take it the prefered patch format is cvs -q diff -u -N ?
- tests are currently not working in cvs head (I see a