Use commandline argument file for all options, but -J, for case javac.forked=true

2 files changed, 196 insertions, 17 deletions

diff --git a/src/main/org/apache/tools/ant/taskdefs/compilers/JavacExternal.java b/src/main/org/apache/tools/ant/taskdefs/compilers/JavacExternal.java
index 7c8354558..5f2b9d5bb 100644
--- a/src/main/org/apache/tools/ant/taskdefs/compilers/JavacExternal.java
+++ b/src/main/org/apache/tools/ant/taskdefs/compilers/JavacExternal.java
@@ -51,36 +51,54 @@ public class JavacExternal extends DefaultCompilerAdapter {
         } else {
             setupJavacCommandlineSwitches(cmd, true);
         }
-        int firstFileName = assumeJava11() ? -1 : cmd.size();
+        int openVmsFirstFileName = assumeJava11() ? -1 : cmd.size();
         logAndAddFilesToCompile(cmd);
         //On VMS platform, we need to create a special java options file
         //containing the arguments and classpath for the javac command.
         //The special file is supported by the "-V" switch on the VMS JVM.
         if (Os.isFamily("openvms")) {
-            return execOnVMS(cmd, firstFileName);
+            return execOnVMS(cmd, openVmsFirstFileName);
         }
-        return
-                executeExternalCompile(cmd.getCommandline(),
-                        assumeJava11() ? -1 : firstNonJOption(cmd),
-                        true)
-                        == 0;
+
+        String[] commandLine = cmd.getCommandline();
+        int firstFileName;
+        if (assumeJava11()) {
+            firstFileName = -1;
+        } else {
+            firstFileName = moveJOptionsToBeginning(commandLine);
+        }
+
+        return executeExternalCompile(commandLine, firstFileName,
+                true)
+                == 0;
     }
 
     /**
-     * Finds first non -J argument, so that all, but -J options are written to file
+     * Moves all -J arguments to the beginning
+     * So that all command line arguments could be written to file, but -J
      * As per javac documentation:
-     * you can specify one or more files that contain arguments to the javac command (except -J options)
-     * @param cmd Commandline
-     * @return int
+     *      you can specify one or more files that contain arguments to the javac command (except -J options)
+     * @param commandLine command line to process
+     * @return int index of first non -J argument
      */
-    private int firstNonJOption(Commandline cmd) {
-        String[] commandline = cmd.getCommandline();
-        int i = 1; // 0 is for javac executable
-        while(i < commandline.length && commandline[i].startsWith("-J")) {
-            i++;
+    private int moveJOptionsToBeginning(String[] commandLine) {
+        int nonJArgumentIdx = 1; // 0 for javac executable
+        while(nonJArgumentIdx < commandLine.length && commandLine[nonJArgumentIdx].startsWith("-J")) {
+            nonJArgumentIdx++;
+        }
+
+        for(int i = nonJArgumentIdx + 1; i < commandLine.length; i++) {
+            if (commandLine[i].startsWith("-J")) {
+                String jArgument = commandLine[i];
+                for(int j = i - 1; j >= nonJArgumentIdx; j--) {
+                    commandLine[j + 1] = commandLine[j];
+                }
+                commandLine[nonJArgumentIdx] = jArgument;
+                nonJArgumentIdx++;
+            }
         }
 
-        return i;
+        return nonJArgumentIdx;
     }
 
     /**
diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/compilers/JavacExternalTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/compilers/JavacExternalTest.java
new file mode 100644
index 000000000..53aac6377
--- /dev/null
+++ b/src/tests/junit/org/apache/tools/ant/taskdefs/compilers/JavacExternalTest.java
@@ -0,0 +1,161 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+
+package org.apache.tools.ant.taskdefs.compilers;
+
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.taskdefs.Javac;
+import org.apache.tools.ant.types.Commandline;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.util.FileUtils;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class JavacExternalTest {
+    private static class TestJavacExternal extends JavacExternal {
+        private String[] args;
+        private int firstFileName;
+
+        @Override
+        protected int executeExternalCompile(String[] args, int firstFileName, final boolean quoteFiles) {
+            this.args = args;
+            this.firstFileName = firstFileName;
+            return 0;
+        }
+
+        public String[] getArgs() {
+            return args;
+        }
+
+        public int getFirstFileName() {
+            return firstFileName;
+        }
+    }
+
+    @Test
+    public void allJavacOptionsButJAreWrittenToFile() throws Exception {
+        final File workDir = createWorkDir("testSMC");
+        try {
+            final File src = new File(workDir, "src");
+            src.mkdir();
+            createFile(src, "org/apache/ant/tests/J1.java");
+            createFile(src, "org/apache/ant/tests/J2.java");
+            final File modules = new File(workDir, "modules");
+            modules.mkdir();
+            final Project prj = new Project();
+            prj.setBaseDir(workDir);
+            final Javac javac = new Javac();
+            javac.setProject(prj);
+            final Commandline[] cmd = new Commandline[1];
+            final TestJavacExternal impl = new TestJavacExternal();
+            final Path srcPath = new Path(prj);
+            srcPath.setLocation(src);
+            javac.setSrcdir(srcPath);
+            javac.createModulepath().setLocation(modules);
+            javac.setSource("9");
+            javac.setTarget("9");
+            javac.setFork(true);
+            javac.setMemoryInitialSize("80m");
+            javac.setExecutable("javacExecutable");
+            javac.add(impl);
+            javac.execute();
+            assertEquals("javacExecutable", impl.getArgs()[0]);
+            assertEquals("-J-Xms80m", impl.getArgs()[1]);
+            assertTrue(impl.getArgs()[impl.getArgs().length - 1].endsWith("J2.java"));
+            assertEquals(2, impl.getFirstFileName());
+        } finally {
+            delete(workDir);
+        }
+    }
+
+    @Test
+    public void allJOptionsAreMovedToBeginning() throws Exception {
+        final File workDir = createWorkDir("testSMC");
+        try {
+            final File src = new File(workDir, "src");
+            src.mkdir();
+            createFile(src, "org/apache/ant/tests/J1.java");
+            createFile(src, "org/apache/ant/tests/J2.java");
+            final File modules = new File(workDir, "modules");
+            modules.mkdir();
+            final Project prj = new Project();
+            prj.setBaseDir(workDir);
+            final Javac javac = new Javac();
+            javac.setProject(prj);
+            final Commandline[] cmd = new Commandline[1];
+            final TestJavacExternal impl = new TestJavacExternal();
+            final Path srcPath = new Path(prj);
+            srcPath.setLocation(src);
+            javac.setSrcdir(srcPath);
+            javac.createModulepath().setLocation(modules);
+            javac.setSource("9");
+            javac.setTarget("9");
+            javac.setFork(true);
+            javac.setMemoryInitialSize("80m");
+            javac.setExecutable("javacExecutable");
+            javac.add(impl);
+            javac.createCompilerArg().setValue("-JDfoo=bar");
+            javac.createCompilerArg().setValue("-JDred=color");
+            javac.createCompilerArg().setLine("-JDspace line");
+            javac.execute();
+            assertEquals("javacExecutable", impl.getArgs()[0]);
+            assertEquals("-J-Xms80m", impl.getArgs()[1]);
+            assertEquals("-JDfoo=bar", impl.getArgs()[2]);
+            assertEquals("-JDred=color", impl.getArgs()[3]);
+            assertEquals("-JDspace", impl.getArgs()[4]);
+            assertTrue(impl.getArgs()[impl.getArgs().length - 1].endsWith("J2.java"));
+            assertEquals(5, impl.getFirstFileName());
+        } finally {
+            delete(workDir);
+        }
+    }
+
+    private File createWorkDir(String testName) {
+        final File tmp = new File(System.getProperty("java.io.tmpdir"));   //NOI18N
+        final File destDir = new File(tmp, String.format("%s%s%d",
+                getClass().getName(),
+                testName,
+                System.currentTimeMillis() / 1000));
+        destDir.mkdirs();
+        return destDir;
+    }
+
+    private File createFile(File folder, String relativePath) throws IOException {
+        final File file = new File(
+                folder,
+                relativePath.replace('/', File.separatorChar).replace('\\', File.separatorChar));
+        FileUtils.getFileUtils().createNewFile(file, true);
+        return file;
+    }
+
+    private void delete(File f) {
+        if (f.isDirectory()) {
+            final File[] clds = f.listFiles();
+            if (clds != null) {
+                Arrays.stream(clds).forEach(this::delete);
+            }
+        }
+        f.delete();
+    }
+}