Skip to content

Conversation

@eric-milles
Copy link
Member

@eric-milles eric-milles commented Nov 11, 2019

GROOVY-8775, GROOVY-9197: Java 10+ compatibility of groovyc ant task

The design of this change is to pass separately the paths necessary for groovyc to run and the classpath of the compiled artifacts. The -classpath argument was previously populated with groovy-all, etc. Now this stuff is passed via the --classpath argument to FileSystemCompilerFacade. Thus, no special URLClassLoader interaction is required (which fails under Java 11).

  [compile] Compilation arguments:
  [compile] C:\Program Files\Java\jdk\11.0.5\bin\java
  [compile] -classpath
  [compile] <enough to execute FileSystemCompilerFacade>
  [compile] -Dfile.encoding=UTF-8
  [compile] org.codehaus.groovy.ant.FileSystemCompilerFacade
  [compile] --classpath
  [compile] <compilation classpath>
  [compile] -j
  [compile] -Fg
  [compile] -Jsource=1.8
  [compile] -Jtarget=1.8
  [compile] -d
  [compile] <output directory>
  [compile] --configscript
  [compile] config.groovy
  [compile] <source files to compile>

The trick to doing all of this is determining what the bootstrap classpath for FileSystemCompilerFacade should be. At the moment, I am getting the Ant runtime from AntClassLoader. But this should be reduced to just groovy and groovy-ant.

    private void doForkCommandLineList(List<String> commandLineList, Path classpath, String separator) {
        //commandLineList.add("java");

        // TODO: Need some help to complete this!  Is it safe to get codesource of FileSystemCompilerFacade.class?
        ClassLoader parent = getClass().getClassLoader();
        if (parent instanceof AntClassLoader) {
            commandLineList.add("-classpath");
            commandLineList.add(((AntClassLoader) parent).getClasspath());
        } else {
            // TODO: add enough classpath entries to support execution of FileSystemCompilerFacade
            throw new AssertionError("not yet implemented");
        }

@eric-milles
Copy link
Member Author

eric-milles commented Nov 11, 2019

Here is the ProtectionDomain/CodeSource idea:

    private void doForkCommandLineList(List<String> commandLineList, Path classpath, String separator) {
        //commandLineList.add("java");

        commandLineList.add("-classpath");
        commandLineList.add(
FileSystemCompiler.class.getProtectionDomain().getCodeSource().getLocation().getPath() +
File.pathSeparator +
FileSystemCompilerFacade.class.getProtectionDomain().getCodeSource().getLocation().getPath()
);

I have tested this and it does compile successfully. I'm not sure how portable or safe this is. Does anyone know the constraints/limits of using ProtectionDomain/CodeSource?

@paulk-asert
Copy link
Contributor

@eric-milles Probably worth asking your ProtectionDomain question on the dev mailing list with a pointer back here since not everyone reads each notification message.

@asfgit asfgit force-pushed the GROOVY-9197 branch 4 times, most recently from 655f30a to 0c12af8 Compare November 12, 2019 17:56
@eric-milles eric-milles changed the title GROOVY-9197: Ant: separate compiler classpath from compilation classpath GROOVY-8775, GROOVY-9197: Ant: separate JVM and compilation classpaths Nov 12, 2019
@eric-milles eric-milles marked this pull request as ready for review November 12, 2019 18:44
@eric-milles
Copy link
Member Author

This is ready to review and merge. I have tested with one of our Ant builds against Java 11.

@daniellansun daniellansun merged commit c6e50ba into master Nov 15, 2019
@daniellansun
Copy link
Contributor

Merged. Thanks!

@asfgit asfgit deleted the GROOVY-9197 branch November 16, 2019 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants