Avoid mixing -source and --release

javac doesn't allow both to be set, so when defaulting to target
Java 8 use --release if the javacopts already include a
--release flag.

PiperOrigin-RevId: 181777670

2 files changed, 76 insertions, 36 deletions

diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java
index 190164a904..3bf8348280 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java
@@ -123,36 +123,55 @@ public class JavacTurbine implements AutoCloseable {
     this.turbineOptions = turbineOptions;
   }
 
-  Result compile() throws IOException {
-    ImmutableList.Builder<String> argbuilder = ImmutableList.builder();
+  /** Creates the compilation javacopts from {@link TurbineOptions}. */
+  @VisibleForTesting
+  static ImmutableList<String> processJavacopts(TurbineOptions turbineOptions) {
+    ImmutableList<String> javacopts =
+        JavacOptions.removeBazelSpecificFlags(turbineOptions.javacOpts());
 
-    argbuilder.addAll(JavacOptions.removeBazelSpecificFlags(turbineOptions.javacOpts()));
+    ImmutableList.Builder<String> builder = ImmutableList.builder();
+    builder.addAll(javacopts);
 
     // Disable compilation of implicit source files.
     // This is insurance: the sourcepath is empty, so we don't expect implicit sources.
-    argbuilder.add("-implicit:none");
+    builder.add("-implicit:none");
 
     // Disable debug info
-    argbuilder.add("-g:none");
+    builder.add("-g:none");
 
     // Enable MethodParameters
-    argbuilder.add("-parameters");
+    builder.add("-parameters");
 
     // Compile-time jars always use Java 8
-    argbuilder.add("-source");
-    argbuilder.add("8");
-    argbuilder.add("-target");
-    argbuilder.add("8");
+    if (javacopts.contains("--release")) {
+      // javac doesn't allow mixing -source and --release, so use --release if it's already present
+      // in javacopts.
+      builder.add("--release");
+      builder.add("8");
+    } else {
+      builder.add("-source");
+      builder.add("8");
+      builder.add("-target");
+      builder.add("8");
+    }
 
-    ImmutableList<Path> processorpath;
     if (!turbineOptions.processors().isEmpty()) {
-      argbuilder.add("-processor");
-      argbuilder.add(Joiner.on(',').join(turbineOptions.processors()));
-      processorpath = asPaths(turbineOptions.processorPath());
-    } else {
-      processorpath = ImmutableList.of();
+      builder.add("-processor");
+      builder.add(Joiner.on(',').join(turbineOptions.processors()));
     }
 
+    return builder.build();
+  }
+
+  Result compile() throws IOException {
+
+    ImmutableList<String> javacopts = processJavacopts(turbineOptions);
+
+    ImmutableList<Path> processorpath =
+        !turbineOptions.processors().isEmpty()
+            ? asPaths(turbineOptions.processorPath())
+            : ImmutableList.of();
+
     ImmutableList<Path> sources =
         ImmutableList.<Path>builder()
             .addAll(asPaths(turbineOptions.sources()))
@@ -162,7 +181,7 @@ public class JavacTurbine implements AutoCloseable {
     JavacTurbineCompileRequest.Builder requestBuilder =
         JavacTurbineCompileRequest.builder()
             .setSources(sources)
-            .setJavacOptions(argbuilder.build())
+            .setJavacOptions(javacopts)
             .setBootClassPath(asPaths(turbineOptions.bootClassPath()))
             .setProcessorClassPath(processorpath);
 
diff --git a/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java b/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java
index 3d730d9793..5ceb0bdf46 100644
--- a/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java
+++ b/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java
@@ -27,6 +27,7 @@ import com.google.common.io.ByteStreams;
 import com.google.devtools.build.java.turbine.javac.JavacTurbine.Result;
 import com.google.devtools.build.lib.view.proto.Deps;
 import com.google.devtools.build.lib.view.proto.Deps.Dependency;
+import com.google.turbine.options.TurbineOptions;
 import com.sun.source.tree.LiteralTree;
 import com.sun.source.util.JavacTask;
 import com.sun.source.util.TaskEvent;
@@ -52,7 +53,6 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -306,13 +306,14 @@ public class JavacTurbineTest extends AbstractJavacTurbineCompilationTest {
     compileLib(
         libC,
         Collections.<Path>emptyList(),
-        Arrays.asList(new StringJavaFileObject("C.java", "interface C { String getString(); }")));
+        ImmutableList.of(
+            new StringJavaFileObject("C.java", "interface C { String getString(); }")));
 
     Path libA = temp.newFile("liba.jar").toPath();
     compileLib(
         libA,
         Collections.singleton(libC),
-        Arrays.asList(new StringJavaFileObject("A.java", "interface A { C getC(); }")));
+        ImmutableList.of(new StringJavaFileObject("A.java", "interface A { C getC(); }")));
 
     Path depsA =
         writedeps(
@@ -330,7 +331,7 @@ public class JavacTurbineTest extends AbstractJavacTurbineCompilationTest {
     compileLib(
         libB,
         Collections.<Path>emptyList(),
-        Arrays.asList(new StringJavaFileObject("B.java", "interface B {}")));
+        ImmutableList.of(new StringJavaFileObject("B.java", "interface B {}")));
 
     optionsBuilder.addClassPathEntries(
         ImmutableList.of(libA.toString(), libB.toString(), libC.toString()));
@@ -391,7 +392,7 @@ public class JavacTurbineTest extends AbstractJavacTurbineCompilationTest {
     JavacFileManager fm = new JavacFileManager(new Context(), false, UTF_8);
     fm.setLocationFromPaths(StandardLocation.CLASS_OUTPUT, Collections.singleton(outdir));
     fm.setLocationFromPaths(StandardLocation.CLASS_PATH, classpath);
-    List<String> options = Arrays.asList("-d", outdir.toString());
+    List<String> options = ImmutableList.of("-d", outdir.toString());
     JavacTool tool = JavacTool.create();
 
     JavacTask task =
@@ -442,25 +443,25 @@ public class JavacTurbineTest extends AbstractJavacTurbineCompilationTest {
     compileLib(
         libD,
         Collections.<Path>emptyList(),
-        Arrays.asList(new StringJavaFileObject("D.java", "public class D {}")));
+        ImmutableList.of(new StringJavaFileObject("D.java", "public class D {}")));
 
     Path libC = temp.newFile("libc.jar").toPath();
     compileLib(
         libC,
         Collections.singleton(libD),
-        Arrays.asList(new StringJavaFileObject("C.java", "class C { static D d; }")));
+        ImmutableList.of(new StringJavaFileObject("C.java", "class C { static D d; }")));
 
     Path libB = temp.newFile("libb.jar").toPath();
     compileLib(
         libB,
-        Arrays.asList(libC, libD),
-        Arrays.asList(new StringJavaFileObject("B.java", "class B { static C c; }")));
+        ImmutableList.of(libC, libD),
+        ImmutableList.of(new StringJavaFileObject("B.java", "class B { static C c; }")));
 
     Path libA = temp.newFile("liba.jar").toPath();
     compileLib(
         libA,
-        Arrays.asList(libB, libC, libD),
-        Arrays.asList(new StringJavaFileObject("A.java", "class A { static B b; }")));
+        ImmutableList.of(libB, libC, libD),
+        ImmutableList.of(new StringJavaFileObject("A.java", "class A { static B b; }")));
     Path depsA =
         writedeps(
             "liba.jdeps",
@@ -535,26 +536,26 @@ public class JavacTurbineTest extends AbstractJavacTurbineCompilationTest {
     compileLib(
         libD,
         Collections.<Path>emptyList(),
-        Arrays.asList(
+        ImmutableList.of(
             new StringJavaFileObject("D.java", "public class D { static final int CONST = 42; }")));
 
     Path libC = temp.newFile("libc.jar").toPath();
     compileLib(
         libC,
         Collections.singleton(libD),
-        Arrays.asList(new StringJavaFileObject("C.java", "class C extends D {}")));
+        ImmutableList.of(new StringJavaFileObject("C.java", "class C extends D {}")));
 
     Path libB = temp.newFile("libb.jar").toPath();
     compileLib(
         libB,
-        Arrays.asList(libC, libD),
-        Arrays.asList(new StringJavaFileObject("B.java", "class B extends C {}")));
+        ImmutableList.of(libC, libD),
+        ImmutableList.of(new StringJavaFileObject("B.java", "class B extends C {}")));
 
     Path libA = temp.newFile("liba.jar").toPath();
     compileLib(
         libA,
-        Arrays.asList(libB, libC, libD),
-        Arrays.asList(new StringJavaFileObject("A.java", "class A extends B {}")));
+        ImmutableList.of(libB, libC, libD),
+        ImmutableList.of(new StringJavaFileObject("A.java", "class A extends B {}")));
     Path depsA =
         writedeps(
             "liba.jdeps",
@@ -899,7 +900,7 @@ public class JavacTurbineTest extends AbstractJavacTurbineCompilationTest {
   @Test
   public void invalidJavacopts() throws Exception {
     addSourceLines("Hello.java", "class Hello {}");
-    optionsBuilder.addAllJavacOpts(Arrays.asList("-NOT_AN_OPTION"));
+    optionsBuilder.addAllJavacOpts(ImmutableList.of("-NOT_AN_OPTION"));
     optionsBuilder.addSources(ImmutableList.copyOf(Iterables.transform(sources, TO_STRING)));
     StringWriter errOutput = new StringWriter();
     try (JavacTurbine turbine =
@@ -1279,7 +1280,7 @@ public class JavacTurbineTest extends AbstractJavacTurbineCompilationTest {
 
     optionsBuilder.addProcessors(ImmutableList.of(SimpleProcessor.class.getName()));
     optionsBuilder.addProcessorPathEntries(HOST_CLASSPATH);
-    optionsBuilder.addAllJavacOpts(Arrays.asList("-Xlint:deprecation"));
+    optionsBuilder.addAllJavacOpts(ImmutableList.of("-Xlint:deprecation"));
     optionsBuilder.addSources(ImmutableList.copyOf(Iterables.transform(sources, TO_STRING)));
 
     StringWriter output = new StringWriter();
@@ -1292,5 +1293,25 @@ public class JavacTurbineTest extends AbstractJavacTurbineCompilationTest {
     assertThat(output.toString()).isEmpty();
     assertThat(result).isEqualTo(Result.OK_WITH_REDUCED_CLASSPATH);
   }
+
+  @Test
+  public void processJavacopts_useSourceByDefault() {
+    TurbineOptions options = TurbineOptions.builder().setOutput("/out").setTempDir("/tmp").build();
+    ImmutableList<String> javacopts = JavacTurbine.processJavacopts(options);
+    assertThat(javacopts).contains("-source");
+    assertThat(javacopts).doesNotContain("--release");
+  }
+
+  @Test
+  public void processJavacopts_releaseDefault() {
+    TurbineOptions options =
+        TurbineOptions.builder()
+            .setOutput("/out")
+            .setTempDir("/tmp")
+            .addAllJavacOpts(ImmutableList.of("--release", "9"))
+            .build();
+    ImmutableList<String> javacopts = JavacTurbine.processJavacopts(options);
+    assertThat(javacopts).doesNotContain("-source");
+  }
 }