Never use @embedded_jdk for --javabase

See #6105

RELNOTES: Future versions of Bazel will require a locally installed JDK
for Java development. Previously Bazel would fall back to using the
embedded --server_javabase if no JDK as available. Pass
--incompatible_never_use_embedded_jdk_for_javabase to disable the legacy
behaviour.

Closes #6106.

PiperOrigin-RevId: 212316246

src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java

@@ -410,6 +410,20 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
   )
   public boolean internalSkylarkFlagTestCanary;
 
+  @Option(
+      name = "incompatible_never_use_embedded_jdk_for_javabase",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      metadataTags = {
+        OptionMetadataTag.INCOMPATIBLE_CHANGE,
+        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+      },
+      help =
+          "Don't use the embedded JDK as a fall-back --javabase if we can't find a locally"
+              + " installed JDK (using JAVA_HOME or `which javac`).")
+  public boolean incompatibleNeverUseEmbeddedJDKForJavabase;
+
   /** Constructs a {@link SkylarkSemantics} object corresponding to this set of option values. */
   public SkylarkSemantics toSkylarkSemantics() {
     return SkylarkSemantics.builder()
@@ -440,6 +454,7 @@ public SkylarkSemantics toSkylarkSemantics() {
         .incompatibleStaticNameResolution(incompatibleStaticNameResolution)
         .incompatibleStringIsNotIterable(incompatibleStringIsNotIterable)
         .internalSkylarkFlagTestCanary(internalSkylarkFlagTestCanary)
+        .incompatibleNeverUseEmbeddedJDKForJavabase(incompatibleNeverUseEmbeddedJDKForJavabase)
         .build();
   }
 }

src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java

@@ -102,18 +102,7 @@ public class WorkspaceFactory {
   // List of top level variable bindings
   private ImmutableMap<String, Object> variableBindings = ImmutableMap.of();
 
-  /**
-   * @param builder a builder for the Workspace
-   * @param ruleClassProvider a provider for known rule classes
-   * @param mutability the Mutability for the current evaluation context
-   */
-  public WorkspaceFactory(
-      Package.Builder builder,
-      RuleClassProvider ruleClassProvider,
-      ImmutableList<EnvironmentExtension> environmentExtensions,
-      Mutability mutability) {
-    this(builder, ruleClassProvider, environmentExtensions, mutability, true, null, null, null);
-  }
+  private final SkylarkSemantics skylarkSemantics;
 
   // TODO(bazel-team): document installDir
   /**
@@ -133,7 +122,8 @@ public WorkspaceFactory(
       boolean allowOverride,
       @Nullable Path installDir,
       @Nullable Path workspaceDir,
-      @Nullable Path defaultSystemJavabaseDir) {
+      @Nullable Path defaultSystemJavabaseDir,
+      SkylarkSemantics skylarkSemantics) {
     this.builder = builder;
     this.mutability = mutability;
     this.installDir = installDir;
@@ -144,6 +134,7 @@ public WorkspaceFactory(
     this.ruleFactory = new RuleFactory(ruleClassProvider, AttributeContainer::new);
     this.workspaceFunctions = WorkspaceFactory.createWorkspaceFunctions(
         allowOverride, ruleFactory);
+    this.skylarkSemantics = skylarkSemantics;
   }
 
   /**
@@ -596,11 +587,7 @@ private void addWorkspaceFunctions(Environment workspaceEnv, StoredEventHandler
         javaHome = javaHome.getParentFile();
       }
       workspaceEnv.update("DEFAULT_SERVER_JAVABASE", javaHome.toString());
-      workspaceEnv.update(
-          "DEFAULT_SYSTEM_JAVABASE",
-          defaultSystemJavabaseDir != null
-              ? defaultSystemJavabaseDir.toString()
-              : javaHome.toString());
+      workspaceEnv.update("DEFAULT_SYSTEM_JAVABASE", getDefaultSystemJavabase(javaHome));
 
       for (EnvironmentExtension extension : environmentExtensions) {
         extension.updateWorkspace(workspaceEnv);
@@ -613,6 +600,19 @@ private void addWorkspaceFunctions(Environment workspaceEnv, StoredEventHandler
     }
   }
 
+  private String getDefaultSystemJavabase(File embeddedJavabase) {
+    if (defaultSystemJavabaseDir != null) {
+      return defaultSystemJavabaseDir.toString();
+    }
+    if (skylarkSemantics.incompatibleNeverUseEmbeddedJDKForJavabase()) {
+      // --javabase is empty if there's no locally installed JDK
+      return "";
+    }
+    // legacy behaviour: fall back to using the embedded JDK as a --javabase
+    // TODO(cushon): delete this
+    return embeddedJavabase.toString();
+  }
+
   private static ClassObject newNativeModule(
       ImmutableMap<String, BaseFunction> workspaceFunctions, String version) {
     ImmutableMap.Builder<String, Object> builder = new ImmutableMap.Builder<>();

src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java

@@ -100,7 +100,8 @@ public SkyValue compute(SkyKey skyKey, Environment env)
               key.getIndex() == 0,
               directories.getEmbeddedBinariesRoot(),
               directories.getWorkspace(),
-              directories.getLocalJavabase());
+              directories.getLocalJavabase(),
+              skylarkSemantics);
       if (key.getIndex() > 0) {
         WorkspaceFileValue prevValue = (WorkspaceFileValue) env.getValue(
             WorkspaceFileValue.key(key.getPath(), key.getIndex() - 1));

src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java

@@ -93,6 +93,8 @@ public abstract class SkylarkSemantics {
 
   public abstract boolean internalSkylarkFlagTestCanary();
 
+  public abstract boolean incompatibleNeverUseEmbeddedJDKForJavabase();
+
   /** Returns a {@link Builder} initialized with the values of this instance. */
   public abstract Builder toBuilder();
 
@@ -134,6 +136,7 @@ public static Builder builderWithDefaults() {
           .incompatibleStaticNameResolution(false)
           .incompatibleStringIsNotIterable(false)
           .internalSkylarkFlagTestCanary(false)
+          .incompatibleNeverUseEmbeddedJDKForJavabase(false)
           .build();
 
   /** Builder for {@link SkylarkSemantics}. All fields are mandatory. */
@@ -193,6 +196,8 @@ public abstract static class Builder {
 
     public abstract Builder internalSkylarkFlagTestCanary(boolean value);
 
+    public abstract Builder incompatibleNeverUseEmbeddedJDKForJavabase(boolean value);
+
     public abstract SkylarkSemantics build();
   }
 }

src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java

@@ -146,7 +146,8 @@ private static SkylarkSemanticsOptions buildRandomOptions(Random rand) throws Ex
         "--incompatible_remove_native_http_archive=" + rand.nextBoolean(),
         "--incompatible_static_name_resolution=" + rand.nextBoolean(),
         "--incompatible_string_is_not_iterable=" + rand.nextBoolean(),
-        "--internal_skylark_flag_test_canary=" + rand.nextBoolean());
+        "--internal_skylark_flag_test_canary=" + rand.nextBoolean(),
+        "--incompatible_never_use_embedded_jdk_for_javabase=" + rand.nextBoolean());
   }
 
   /**
@@ -183,6 +184,7 @@ private static SkylarkSemantics buildRandomSemantics(Random rand) {
         .incompatibleStaticNameResolution(rand.nextBoolean())
         .incompatibleStringIsNotIterable(rand.nextBoolean())
         .internalSkylarkFlagTestCanary(rand.nextBoolean())
+        .incompatibleNeverUseEmbeddedJDKForJavabase(rand.nextBoolean())
         .build();
   }
 

src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java

@@ -73,7 +73,8 @@ void parse(Scratch scratch, String... args) {
             allowOverride,
             root,
             root,
-            /* defaultSystemJavabaseDir= */ null);
+            /* defaultSystemJavabaseDir= */ null,
+            /* skylarkSemantics= */ SkylarkSemantics.DEFAULT_SEMANTICS);
     Exception exception = null;
     try {
       byte[] bytes =

src/test/shell/integration/bazel_java_test.sh

@@ -44,11 +44,11 @@ EOF
   expect_log "java-home: .*/_embedded_binaries/embedded_tools/jdk"
 }
 
-function test_rhs_host_javabase() {
+function test_host_javabase() {
   mkdir -p foobar/bin
   cat << EOF > BUILD
 java_runtime(
-    name = "rhs_host_javabase",
+    name = "host_javabase",
     java_home = "$PWD/foobar",
     visibility = ["//visibility:public"],
 )
@@ -65,7 +65,7 @@ EOF
 
   # We expect the given host_javabase to appear in the command line of
   # java_library actions.
-  bazel aquery --output=text --host_javabase=//:rhs_host_javabase //java:javalib >& $TEST_log
+  bazel aquery --output=text --host_javabase=//:host_javabase //java:javalib >& $TEST_log
   expect_log "exec .*foobar/bin/java"
 
   # If we don't specify anything, we expect the embedded JDK to be used.
@@ -74,11 +74,11 @@ EOF
   expect_log "exec external/embedded_jdk/bin/java"
 }
 
-function test_rhs_javabase() {
+function test_javabase() {
   mkdir -p zoo/bin
   cat << EOF > BUILD
 java_runtime(
-    name = "rhs_javabase",
+    name = "javabase",
     java_home = "$PWD/zoo",
     visibility = ["//visibility:public"],
 )
@@ -96,7 +96,7 @@ public class HelloWorld {}
 EOF
 
   # Check that the RHS javabase appears in the launcher.
-  bazel build --javabase=//:rhs_javabase //java:javabin
+  bazel build --javabase=//:javabase //java:javabin
   cat bazel-bin/java/javabin >& $TEST_log
   expect_log "JAVABIN=.*/zoo/bin/java"
 
@@ -106,5 +106,63 @@ EOF
   expect_log "JAVABIN=.*/local_jdk/bin/java"
 }
 
+function write_javabase_files() {
+  mkdir -p javabase_test
+  cat << EOF > javabase_test/BUILD
+java_binary(
+    name = "a",
+    srcs = ["A.java"],
+    main_class = "A",
+)
+EOF
+
+  cat << EOF > javabase_test/A.java
+class A {
+  public static void main(String[] args) {
+    System.err.println("hello");
+  }
+}
+EOF
+}
+
+function test_no_javabase() {
+  # Only run this test when there's no locally installed JDK.
+  which javac && return
+
+  write_javabase_files
+
+  bazel build //javabase_test:a
+  bazel build --incompatible_never_use_embedded_jdk_for_javabase //javabase_test:a
+
+  ($(bazel-bin/javabase_test/a --print_javabin) -version || true) >& $TEST_log
+  expect_log "bazel-bin/javabase_test/a.runfiles/local_jdk/bin/java: No such file or directory"
+}
+
+function test_no_javabase() {
+  # Only run this test when there's no locally installed JDK.
+  which javac && return
+
+  write_javabase_files
+
+  bazel --batch build //javabase_test:a
+  bazel --batch build --incompatible_never_use_embedded_jdk_for_javabase //javabase_test:a
+
+  ($(bazel-bin/javabase_test/a --print_javabin) -version || true) >& $TEST_log
+  expect_log "bazel-bin/javabase_test/a.runfiles/local_jdk/bin/java: No such file or directory"
+}
+
+function test_no_javabase_default_embedded() {
+  # Only run this test when there's no locally installed JDK.
+  which javac && return
+
+  write_javabase_files
+
+  bazel --batch build //javabase_test:a
+
+  echo $(bazel-bin/javabase_test/a --print_javabin) >& $TEST_log
+  expect_log "bazel-bin/javabase_test/a.runfiles/local_jdk/bin/java"
+  $(bazel-bin/javabase_test/a --print_javabin) -version >& $TEST_log
+  expect_log "Zulu"
+}
 
 run_suite "Tests of specifying custom server_javabase/host_javabase and javabase."