Plumb some information about the location of the JVM through JavaRuntimeProvider instead of Jvm if a java_runtime rule is used.

There are a few things standing in the way of removing package loading from JvmConfigurationLoader:

- The JAVABASE/JAVA Make variables, which are a function of the contents of the package with the JVM. The plan is to make JavaRuntime a MakeVariableProvider and add an attribute to rules that do Make variable evaluation to tell which Make variables they need

-  The path to the Java binary is exported to Skylark through the configuration fragment. We'll need to export an attribute to Skylark that can be used to depend on the JVM, then do the three-step dance to use that instead of the configuration and figure out what to do if --javabase is not a label.

- Jvm#getJavaExecutable() has a bunch of call sites. They need to be adjusted individually.

RELNOTES: None.
PiperOrigin-RevId: 161176462

10 files changed, 90 insertions, 34 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 5c8248d0c2..b3d8940598 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -1053,7 +1053,7 @@ public final class RuleContext extends TargetContext
 
   public ImmutableMap<String, String> getMakeVariables(Iterable<String> attributeNames) {
     // Using an ImmutableBuilder to complain about duplicate keys. This traversal order of
-    // getPrerequisites isn't well-defined, so this makes sure providers don't seceretly stomp on
+    // getPrerequisites isn't well-defined, so this makes sure providers don't secretly stomp on
     // each other.
     ImmutableMap.Builder<String, String> makeVariableBuilder = ImmutableMap.builder();
     ImmutableSet.Builder<MakeVariableProvider> makeVariableProvidersBuilder =
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
index b562eac1ea..6e764477b5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
@@ -186,7 +186,8 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor
         getJvmFlags(ruleContext, testClass),
         executable,
         mainClass,
-        JavaCommon.getJavaBinSubstitution(ruleContext, launcher));
+        JavaCommon.getJavaBinSubstitution(
+            ruleContext, JavaHelper.getJavaRuntime(ruleContext), launcher));
 
     Artifact deployJar =
         ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_BINARY_DEPLOY_JAR);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
index 4a927b8c8b..8142ce6c3b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
@@ -24,7 +24,6 @@ import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.FileProvider;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
@@ -233,6 +232,8 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
         CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext)
             .getDynamicRuntimeLinkInputs();
 
+    JavaRuntimeProvider javaRuntime = JavaHelper.getJavaRuntime(ruleContext);
+
     Iterables.addAll(jvmFlags,
         semantics.getJvmFlags(ruleContext, common.getSrcsArtifacts(), userJvmFlags));
     if (ruleContext.hasErrors()) {
@@ -249,7 +250,7 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
               jvmFlags,
               executableForRunfiles,
               mainClass,
-              JavaCommon.getJavaBinSubstitution(ruleContext, launcher));
+              JavaCommon.getJavaBinSubstitution(ruleContext, javaRuntime, launcher));
       if (!executableToRun.equals(executableForRunfiles)) {
         filesBuilder.add(executableToRun);
         runfilesBuilder.addArtifact(executableToRun);
@@ -508,9 +509,10 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
 
     // Add the JDK files if it comes from the source repository (see java_stub_template.txt).
     TransitiveInfoCollection javabaseTarget = ruleContext.getPrerequisite(":jvm", Mode.TARGET);
+    JavaRuntimeProvider javaRuntime = null;
     if (javabaseTarget != null) {
-      builder.addArtifacts(
-          (Iterable<Artifact>) javabaseTarget.getProvider(FileProvider.class).getFilesToBuild());
+      javaRuntime = javabaseTarget.getProvider(JavaRuntimeProvider.class);
+      builder.addTransitiveArtifacts(javaRuntime.javaBaseInputs());
 
       // Add symlinks to the C++ runtime libraries under a path that can be built
       // into the Java binary without having to embed the crosstool, gcc, and grte
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
index 8a3e93c98e..02d6f935fa 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
@@ -471,14 +471,16 @@ public class JavaCommon {
    * @param launcher if non-null, the cc_binary used to launch the Java Virtual Machine
    */
   public static String getJavaBinSubstitution(
-      RuleContext ruleContext, @Nullable Artifact launcher) {
+      RuleContext ruleContext, JavaRuntimeProvider javaRuntime, @Nullable Artifact launcher) {
     Preconditions.checkState(ruleContext.getConfiguration().hasFragment(Jvm.class));
     PathFragment javaExecutable;
 
     if (launcher != null) {
       javaExecutable = launcher.getRootRelativePath();
+    } else if (javaRuntime != null) {
+      javaExecutable = javaRuntime.javaBinaryRunfilesPath();
     } else {
-      javaExecutable = ruleContext.getFragment(Jvm.class).getRunfilesJavaExecutable();
+      javaExecutable = ruleContext.getFragment(Jvm.class).getJavaExecutable();
     }
 
     if (!javaExecutable.isAbsolute()) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHelper.java
index 5984de833e..c6e16e1911 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHelper.java
@@ -149,6 +149,19 @@ public abstract class JavaHelper {
         ruleContext, ":host_jdk" + implicitAttributesSuffix, Mode.HOST);
   }
 
+  public static JavaRuntimeProvider getJavaRuntime(RuleContext ruleContext) {
+    if (!ruleContext.attributes().has(":jvm", BuildType.LABEL)) {
+      return null;
+    }
+
+    TransitiveInfoCollection jvm = ruleContext.getPrerequisite(":jvm", Mode.TARGET);
+    if (jvm == null) {
+      return null;
+    }
+
+    return jvm.getProvider(JavaRuntimeProvider.class);
+  }
+
   /**
    * Returns true if the given Label is of the pseudo-cc_binary that tells Bazel a Java target's
    * JAVABIN is never to be replaced by the contents of --java_launcher; only the JDK's launcher
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java
index abe3c9ac00..0d15e60e9f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java
@@ -25,16 +25,40 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
+import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.util.OsUtils;
+import com.google.devtools.build.lib.vfs.PathFragment;
 
 /** Implementation for the {@code java_runtime} rule. */
 public class JavaRuntime implements RuleConfiguredTargetFactory {
+  // TODO(lberki): This is incorrect but that what the Jvm configuration fragment did. We'd have the
+  // the ability to do better if we knew what OS the BuildConfiguration refers to.
+  private static final String BIN_JAVA = "bin/java" + OsUtils.executableExtension();
+
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException {
     NestedSet<Artifact> filesToBuild =
         PrerequisiteArtifacts.nestedSet(ruleContext, "srcs", Mode.TARGET);
+    PathFragment javaHome = defaultJavaHome(ruleContext.getLabel());
+    if (ruleContext.attributes().isAttributeValueExplicitlySpecified("java_home")) {
+      PathFragment javaHomeAttribute = PathFragment.create(
+          ruleContext.attributes().get("java_home", Type.STRING));
+      if (!filesToBuild.isEmpty() && javaHomeAttribute.isAbsolute()) {
+        ruleContext.ruleError("'java_home' with an absolute path requires 'srcs' to be empty.");
+        return null;
+      }
+
+      javaHome = javaHome.getRelative(javaHomeAttribute);
+    }
+
+    PathFragment javaBinaryExecPath = javaHome.getRelative(BIN_JAVA);
+    PathFragment javaBinaryRunfilesPath = getRunfilesJavaExecutable(
+        javaHome, ruleContext.getLabel());
+
     NestedSet<Artifact> middleman =
         CompilationHelper.getAggregatingMiddleman(
             ruleContext, Actions.escapeLabel(ruleContext.getLabel()), filesToBuild);
@@ -46,8 +70,25 @@ public class JavaRuntime implements RuleConfiguredTargetFactory {
     return new RuleConfiguredTargetBuilder(ruleContext)
         .addProvider(RunfilesProvider.class, RunfilesProvider.simple(runfiles))
         .setFilesToBuild(filesToBuild)
-        .addProvider(JavaRuntimeProvider.class, JavaRuntimeProvider.create(filesToBuild))
+        .addProvider(JavaRuntimeProvider.class, JavaRuntimeProvider.create(
+            filesToBuild, javaHome, javaBinaryExecPath, javaBinaryRunfilesPath))
         .addProvider(MiddlemanProvider.class, new MiddlemanProvider(middleman))
         .build();
   }
+
+  static PathFragment defaultJavaHome(Label javabase) {
+    if (javabase.getPackageIdentifier().getRepository().isDefault()) {
+      return javabase.getPackageFragment();
+    }
+    return javabase.getPackageIdentifier().getSourceRoot();
+  }
+
+  private static PathFragment getRunfilesJavaExecutable(PathFragment javaHome, Label javabase) {
+    if (javaHome.isAbsolute() || javabase.getPackageIdentifier().getRepository().isMain()) {
+      return javaHome.getRelative(BIN_JAVA);
+    } else {
+      return javabase.getPackageIdentifier().getRepository().getRunfilesPath()
+          .getRelative(BIN_JAVA);
+    }
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeProvider.java
index 7ecd58c35e..7df3efcb7a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeProvider.java
@@ -19,16 +19,29 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.vfs.PathFragment;
 
 /** Information about the Java runtime used by the <code>java_*</code> rules. */
 @AutoValue
 @Immutable
 public abstract class JavaRuntimeProvider implements TransitiveInfoProvider {
 
-  public static JavaRuntimeProvider create(NestedSet<Artifact> javaBaseInputs) {
-    return new AutoValue_JavaRuntimeProvider(javaBaseInputs);
+  public static JavaRuntimeProvider create(
+      NestedSet<Artifact> javaBaseInputs, PathFragment javaHome,
+      PathFragment javaBinaryExecPath, PathFragment javaBinaryRunfilesPath) {
+    return new AutoValue_JavaRuntimeProvider(
+        javaBaseInputs, javaHome, javaBinaryExecPath, javaBinaryRunfilesPath);
   }
 
   /** All input artifacts in the javabase. */
   public abstract NestedSet<Artifact> javaBaseInputs();
+
+  /** The root directory of the Java installation. */
+  public abstract PathFragment javaHome();
+
+  /** The execpath of the Java binary. */
+  public abstract PathFragment javaBinaryExecPath();
+
+  /** The runfiles path of the Java binary. */
+  public abstract PathFragment javaBinaryRunfilesPath();
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeSuite.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeSuite.java
index 2a42e5e560..e214eac612 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeSuite.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeSuite.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.java;
 
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.FileProvider;
+import com.google.devtools.build.lib.analysis.MiddlemanProvider;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.RuleContext;
@@ -40,6 +41,7 @@ public class JavaRuntimeSuite implements RuleConfiguredTargetFactory {
     return new RuleConfiguredTargetBuilder(ruleContext)
         .addProvider(JavaRuntimeProvider.class, runtime.getProvider(JavaRuntimeProvider.class))
         .addProvider(RunfilesProvider.class, runtime.getProvider(RunfilesProvider.class))
+        .addProvider(MiddlemanProvider.class, runtime.getProvider(MiddlemanProvider.class))
         .setFilesToBuild(runtime.getProvider(FileProvider.class).getFilesToBuild())
         .build();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java b/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java
index 7658cd341a..1661e8dcc9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java
@@ -52,7 +52,7 @@ public final class Jvm extends BuildConfiguration.Fragment {
     Preconditions.checkArgument(javaHome.isAbsolute() || jvmLabel != null);
     this.javaHome = javaHome;
     this.jvmLabel = jvmLabel;
-    this.java = getJavaHome().getRelative(BIN_JAVA);
+    this.java = javaHome.getRelative(BIN_JAVA);
   }
 
   /**
@@ -83,20 +83,9 @@ public final class Jvm extends BuildConfiguration.Fragment {
     return jvmLabel;
   }
 
-  /**
-   * If possible, resolves java relative to the jvmLabel's repository. Otherwise, returns the
-   * same thing as getJavaExecutable().
-   */
-  public PathFragment getRunfilesJavaExecutable() {
-    if (javaHome.isAbsolute() || jvmLabel.getPackageIdentifier().getRepository().isMain()) {
-      return getJavaExecutable();
-    }
-    return jvmLabel.getPackageIdentifier().getRepository().getRunfilesPath().getRelative(BIN_JAVA);
-  }
-
   @Override
   public void addGlobalMakeVariables(Builder<String, String> globalMakeEnvBuilder) {
-    globalMakeEnvBuilder.put("JAVABASE", getJavaHome().getPathString());
-    globalMakeEnvBuilder.put("JAVA", getJavaExecutable().getPathString());
+    globalMakeEnvBuilder.put("JAVABASE", javaHome.getPathString());
+    globalMakeEnvBuilder.put("JAVA", java.getPathString());
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java
index 5f191fde78..3e1e4870a7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java
@@ -125,7 +125,7 @@ public final class JvmConfigurationLoader implements ConfigurationFragmentFactor
           String.format("Expected a java_runtime rule, was '%s'", javaRuntimeRule.getRuleClass()));
     }
     RawAttributeMapper attributes = RawAttributeMapper.of(javaRuntimeRule);
-    PathFragment javaHomePath = defaultJavaHome(javaRuntimeLabel);
+    PathFragment javaHomePath = JavaRuntime.defaultJavaHome(javaRuntimeLabel);
     if (attributes.isAttributeValueExplicitlySpecified("java_home")) {
       javaHomePath = javaHomePath.getRelative(attributes.get("java_home", Type.STRING));
       List<Label> srcs = attributes.get("srcs", BuildType.LABEL_LIST);
@@ -137,7 +137,7 @@ public final class JvmConfigurationLoader implements ConfigurationFragmentFactor
                 javaHomePath, srcs.toString()));
       }
     }
-    return new Jvm(javaHomePath, javaRuntimeLabel);
+    return new Jvm(javaHomePath, javaRuntimeSuite.getLabel());
   }
 
   private static Label selectRuntime(Rule javaRuntimeSuite, String cpu)
@@ -154,13 +154,6 @@ public final class JvmConfigurationLoader implements ConfigurationFragmentFactor
         "No JVM target found under " + javaRuntimeSuite + " that would work for " + cpu);
   }
 
-  private static PathFragment defaultJavaHome(Label javaBase) {
-    if (javaBase.getPackageIdentifier().getRepository().isDefault()) {
-      return javaBase.getPackageFragment();
-    }
-    return javaBase.getPackageIdentifier().getSourceRoot();
-  }
-
   private static Jvm createLegacy(String javaHome) throws InvalidConfigurationException {
     PathFragment javaHomePathFrag = PathFragment.create(javaHome);
     if (!javaHomePathFrag.isAbsolute()) {