From 6d4eff75fcdcfbe0cf6b92ff05de1f321dd42bbf Mon Sep 17 00:00:00 2001 From: Martin von Gagern Date: Thu, 27 Nov 2014 09:52:52 +0100 Subject: [PATCH 1/5] Improve task, always provide core loader. 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. Also 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. This patch was taken from PR #47003 and dates back to April 2009. --- src/main/org/apache/tools/ant/Main.java | 2 ++ .../org/apache/tools/ant/taskdefs/Classloader.java | 13 +++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/main/org/apache/tools/ant/Main.java b/src/main/org/apache/tools/ant/Main.java index a9c23a53a7..933bf92a99 100644 --- a/src/main/org/apache/tools/ant/Main.java +++ b/src/main/org/apache/tools/ant/Main.java @@ -764,6 +764,8 @@ private void runBuild(final ClassLoader coreLoader) throws BuildException { } final Project project = new Project(); + if (coreLoader == null || coreLoader instanceof AntClassLoader == false) + coreLoader = new AntClassLoader(coreLoader, project, null, true); project.setCoreLoader(coreLoader); Throwable error = null; diff --git a/src/main/org/apache/tools/ant/taskdefs/Classloader.java b/src/main/org/apache/tools/ant/taskdefs/Classloader.java index 8a5967c701..628e99ae94 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Classloader.java +++ b/src/main/org/apache/tools/ant/taskdefs/Classloader.java @@ -169,9 +169,18 @@ public void execute() { return; } - String loaderName = (name == null) ? SYSTEM_LOADER_REF : name; + String loaderName; + Object obj; + if (name == null || SYSTEM_LOADER_REF.equals(name)) { + name = null; + loaderName = SYSTEM_LOADER_REF; + obj = getProject().getCoreLoader(); + } + else { + loaderName = name; + obj = getProject().getReference(loaderName); + } - Object obj = getProject().getReference(loaderName); if (reset) { // Are any other references held ? Can we 'close' the loader // so it removes the locks on jars ? From 8b5639ca04f840488ba5e3bb8dd098fac6b6d054 Mon Sep 17 00:00:00 2001 From: Martin von Gagern Date: Thu, 27 Nov 2014 10:11:06 +0100 Subject: [PATCH 2/5] Avoid relying on Main. Instead of introducing an AntClassLoader in Main, we now have the Project class itself ensure that the core class loader is always an AntClassLoader. This should cater for the embedded mode, where Main isn't even used. We also use the factory method to make sure we get the right subclass of the class loader class. And there is some API documentation about all of this. --- src/main/org/apache/tools/ant/Main.java | 2 -- src/main/org/apache/tools/ant/Project.java | 14 ++++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/main/org/apache/tools/ant/Main.java b/src/main/org/apache/tools/ant/Main.java index 933bf92a99..a9c23a53a7 100644 --- a/src/main/org/apache/tools/ant/Main.java +++ b/src/main/org/apache/tools/ant/Main.java @@ -764,8 +764,6 @@ private void runBuild(final ClassLoader coreLoader) throws BuildException { } final Project project = new Project(); - if (coreLoader == null || coreLoader instanceof AntClassLoader == false) - coreLoader = new AntClassLoader(coreLoader, project, null, true); project.setCoreLoader(coreLoader); Throwable error = null; diff --git a/src/main/org/apache/tools/ant/Project.java b/src/main/org/apache/tools/ant/Project.java index b7ab25bc6d..9d29a1f239 100644 --- a/src/main/org/apache/tools/ant/Project.java +++ b/src/main/org/apache/tools/ant/Project.java @@ -302,7 +302,8 @@ public void initSubProject(final Project subProject) { */ public void init() throws BuildException { initProperties(); - + if (coreLoader == null) + setCoreLoader(null); ComponentHelper.getComponentHelper(this).initDefaultDefinitions(); } @@ -361,12 +362,21 @@ public AntClassLoader createClassLoader( /** * Set the core classloader for the project. If a null * classloader is specified, the parent classloader should be used. + *

+ * If the argument is not a non-null instance of + * {@link AntClassLoader}, then an ant class loader will be + * constructed around the specified class loader. This ensures + * that the core class loader is always an ant class loader which + * can be modified using the <classloader> task. * * @param coreLoader The classloader to use for the project. * May be null. */ public void setCoreLoader(final ClassLoader coreLoader) { - this.coreLoader = coreLoader; + if (coreLoader == null || coreLoader instanceof AntClassLoader == false) + this.coreLoader = createClassLoader(coreLoader, null); + else + this.coreLoader = coreLoader; } /** From f6517218534ab6467f1c74b3d7cdca9625597a0d Mon Sep 17 00:00:00 2001 From: Martin von Gagern Date: Thu, 27 Nov 2014 10:23:22 +0100 Subject: [PATCH 3/5] Document the (assumed) version for classloader changes. The documentation now specifies the version when the behavior changed. The private variable is adjusted to reflect the fact that we always want an AntClassLoader there. The public interface is left as is, though, to avoid trouble in case someone subclasses Project and overrides that method. Although I can't imagine why someone would do that. --- src/main/org/apache/tools/ant/Project.java | 25 ++++++++++++---------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/main/org/apache/tools/ant/Project.java b/src/main/org/apache/tools/ant/Project.java index 9d29a1f239..fa055279fc 100644 --- a/src/main/org/apache/tools/ant/Project.java +++ b/src/main/org/apache/tools/ant/Project.java @@ -185,10 +185,9 @@ protected Boolean initialValue() { }; /** - * The Ant core classloader--may be null if using - * parent classloader. + * The Ant core classloader */ - private ClassLoader coreLoader = null; + private AntClassLoader coreLoader = null; /** Records the latest task to be executed on a thread. */ private final Map threadTasks = @@ -363,11 +362,12 @@ public AntClassLoader createClassLoader( * Set the core classloader for the project. If a null * classloader is specified, the parent classloader should be used. *

- * If the argument is not a non-null instance of - * {@link AntClassLoader}, then an ant class loader will be - * constructed around the specified class loader. This ensures - * that the core class loader is always an ant class loader which - * can be modified using the <classloader> task. + * Since Ant 1.9.5, if the argument is not a + * non-null instance of {@link AntClassLoader}, then + * an ant class loader will be constructed around the specified + * class loader. This ensures that the core class loader is always + * an ant class loader which can be modified using the + * <classloader> task. * * @param coreLoader The classloader to use for the project. * May be null. @@ -376,13 +376,16 @@ public void setCoreLoader(final ClassLoader coreLoader) { if (coreLoader == null || coreLoader instanceof AntClassLoader == false) this.coreLoader = createClassLoader(coreLoader, null); else - this.coreLoader = coreLoader; + this.coreLoader = (AntClassLoader)coreLoader; } /** * Return the core classloader to use for this project. - * This may be null, indicating that - * the parent classloader should be used. + *

+ * Since Ant 1.9.5 the returned loader will always be a + * non-null instance of {@link AntClassLoader}. + * In older versions, this might be null, + * indicating that the parent classloader should be used. * * @return the core classloader to use for this project. * From f870b562a95ad2cfd72b28302584be8b920dd5a3 Mon Sep 17 00:00:00 2001 From: Martin von Gagern Date: Thu, 27 Nov 2014 12:21:50 +0100 Subject: [PATCH 4/5] Added antunit test for classloader modifying the core loader. --- .../antunit/taskdefs/classloader-test.xml | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 src/tests/antunit/taskdefs/classloader-test.xml diff --git a/src/tests/antunit/taskdefs/classloader-test.xml b/src/tests/antunit/taskdefs/classloader-test.xml new file mode 100644 index 0000000000..4b53823058 --- /dev/null +++ b/src/tests/antunit/taskdefs/classloader-test.xml @@ -0,0 +1,87 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 7df45757dfa727aed3624d62851c167110b15ea4 Mon Sep 17 00:00:00 2001 From: Martin von Gagern Date: Thu, 27 Nov 2014 12:28:05 +0100 Subject: [PATCH 5/5] Merge sysclasspath=only subcase into existing core loader case. --- .../apache/tools/ant/taskdefs/Classloader.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/org/apache/tools/ant/taskdefs/Classloader.java b/src/main/org/apache/tools/ant/taskdefs/Classloader.java index 628e99ae94..c8898b255a 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Classloader.java +++ b/src/main/org/apache/tools/ant/taskdefs/Classloader.java @@ -161,17 +161,17 @@ public Path createClasspath() { */ public void execute() { try { - // Gump friendly - don't mess with the core loader if only classpath - if ("only".equals(getProject().getProperty("build.sysclasspath")) - && (name == null || SYSTEM_LOADER_REF.equals(name))) { - log("Changing the system loader is disabled " - + "by build.sysclasspath=only", Project.MSG_WARN); - return; - } - String loaderName; Object obj; if (name == null || SYSTEM_LOADER_REF.equals(name)) { + if ("only".equals(getProject() + .getProperty("build.sysclasspath"))) { + // Gump friendly - don't mess with the core loader + // if only the system classpath is to be used + log("Changing the system loader is disabled " + + "by build.sysclasspath=only", Project.MSG_WARN); + return; + } name = null; loaderName = SYSTEM_LOADER_REF; obj = getProject().getCoreLoader();