Fix ClasspathConfiguredFragment bootclasspath collection

Previously it was only collecting custom per-rule bootclasspaths (e.g. from
android_library, which uses android.jar instead of the default Java
bootclasspath). This allows the java.compilation_info.boot_classpath Skylark
provider to work for regular Java rules.

--
MOS_MIGRATED_REVID=117031832

6 files changed, 36 insertions, 29 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
index f0e3015e4b..ed61872a7c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
@@ -611,7 +611,10 @@ public class AndroidCommon {
 
     javaCommon.setClassPathFragment(
         new ClasspathConfiguredFragment(
-            javaCommon.getJavaCompilationArtifacts(), attributes, asNeverLink));
+            javaCommon.getJavaCompilationArtifacts(),
+            attributes,
+            asNeverLink,
+            helper.getBootclasspathOrDefault()));
 
     transitiveNeverlinkLibraries = collectTransitiveNeverlinkLibraries(
         ruleContext,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/ClasspathConfiguredFragment.java b/src/main/java/com/google/devtools/build/lib/rules/java/ClasspathConfiguredFragment.java
index 89d32700ff..7a34dcc4c8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/ClasspathConfiguredFragment.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/ClasspathConfiguredFragment.java
@@ -37,15 +37,18 @@ public final class ClasspathConfiguredFragment {
    * @param attributes the processed attributes of this Java target
    * @param isNeverLink whether to leave runtimeClasspath empty
    */
-  public ClasspathConfiguredFragment(JavaCompilationArtifacts javaArtifacts,
-      JavaTargetAttributes attributes, boolean isNeverLink) {
+  public ClasspathConfiguredFragment(
+      JavaCompilationArtifacts javaArtifacts,
+      JavaTargetAttributes attributes,
+      boolean isNeverLink,
+      ImmutableList<Artifact> bootClasspath) {
     if (!isNeverLink) {
       runtimeClasspath = getRuntimeClasspathList(attributes, javaArtifacts);
     } else {
       runtimeClasspath = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER);
     }
     compileTimeClasspath = attributes.getCompileTimeClassPath();
-    bootClasspath = attributes.getBootClassPath();
+    this.bootClasspath = bootClasspath;
   }
 
   public ClasspathConfiguredFragment() {
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 24c67e3ca8..e92ba74022 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
@@ -187,8 +187,9 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
         classJar, manifestProtoOutput, genSourceJar, outputDepsProto, instrumentationMetadata);
     helper.createSourceJarAction(srcJar, genSourceJar);
 
-    common.setClassPathFragment(new ClasspathConfiguredFragment(
-        javaArtifacts, attributes, false));
+    common.setClassPathFragment(
+        new ClasspathConfiguredFragment(
+            javaArtifacts, attributes, false, helper.getBootclasspathOrDefault()));
 
     // Collect the action inputs for the runfiles collector here because we need to access the
     // analysis environment, and that may no longer be safe when the runfiles collector runs.
@@ -492,9 +493,9 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
     }
 
     @Override
-    protected ImmutableList<Artifact> collectProguardSpecsForRule(RuleContext ruleContext,
-        String mainClassName) {
-      return ImmutableList.of(generateSpecForJavaBinary(ruleContext, mainClassName));
+    protected ImmutableList<Artifact> collectProguardSpecsForRule(
+        RuleContext ruleContext, ImmutableList<Artifact> bootclasspath, String mainClassName) {
+      return ImmutableList.of(generateSpecForJavaBinary(ruleContext, bootclasspath, mainClassName));
     }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
index 53e9fe535f..08ca26841a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
@@ -162,7 +162,7 @@ public final class JavaCompilationHelper extends BaseJavaCompilationHelper {
   }
 
   /** Returns the bootclasspath explicit set in attributes if present, or else the default. */
-  private ImmutableList<Artifact> getBootclasspathOrDefault() {
+  public ImmutableList<Artifact> getBootclasspathOrDefault() {
     JavaTargetAttributes attributes = getAttributes();
     if (!attributes.getBootClassPath().isEmpty()) {
       return attributes.getBootClassPath();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibrary.java
index a631dd47a7..3b4cc3c12b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibrary.java
@@ -129,8 +129,9 @@ public class JavaLibrary implements RuleConfiguredTargetFactory {
     boolean neverLink = JavaCommon.isNeverLink(ruleContext);
     JavaCompilationArtifacts javaArtifacts = javaArtifactsBuilder.build();
     common.setJavaCompilationArtifacts(javaArtifacts);
-    common.setClassPathFragment(new ClasspathConfiguredFragment(
-        javaArtifacts, attributes, neverLink));
+    common.setClassPathFragment(
+        new ClasspathConfiguredFragment(
+            javaArtifacts, attributes, neverLink, helper.getBootclasspathOrDefault()));
     CppCompilationContext transitiveCppDeps = common.collectTransitiveCppDeps();
 
     NestedSet<Artifact> transitiveSourceJars = common.collectTransitiveSourceJars(srcJar);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/ProguardHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/ProguardHelper.java
index 98e01d1f09..55851a3e46 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/ProguardHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/ProguardHelper.java
@@ -98,15 +98,15 @@ public abstract class ProguardHelper {
       return null;
     }
 
-    Preconditions.checkArgument(bootclasspath.isEmpty(),
-        "Bootclasspath should be empty b/c not compiling for Android device: %s", bootclasspath);
+    Preconditions.checkArgument(!bootclasspath.isEmpty(), "Bootclasspath should not be empty");
     FilesToRunProvider proguard = findProguard(ruleContext);
     if (proguard == null) {
       ruleContext.ruleError("--proguard_top required for --java_optimization_mode=" + optMode);
       return null;
     }
 
-    ImmutableList<Artifact> proguardSpecs = collectProguardSpecs(ruleContext, mainClassName);
+    ImmutableList<Artifact> proguardSpecs =
+        collectProguardSpecs(ruleContext, bootclasspath, mainClassName);
     Artifact singleJar =
         ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_BINARY_MERGED_JAR);
     return createProguardAction(ruleContext, proguard, singleJar, proguardSpecs, (Artifact) null,
@@ -115,9 +115,9 @@ public abstract class ProguardHelper {
   }
 
   private ImmutableList<Artifact> collectProguardSpecs(
-      RuleContext ruleContext, String mainClassName) {
-    return ProguardHelper.collectTransitiveProguardSpecs(ruleContext,
-        collectProguardSpecsForRule(ruleContext, mainClassName));
+      RuleContext ruleContext, ImmutableList<Artifact> bootclasspath, String mainClassName) {
+    return ProguardHelper.collectTransitiveProguardSpecs(
+        ruleContext, collectProguardSpecsForRule(ruleContext, bootclasspath, mainClassName));
   }
 
   /**
@@ -136,7 +136,7 @@ public abstract class ProguardHelper {
    * so it's ok to generate files here.
    */
   protected abstract ImmutableList<Artifact> collectProguardSpecsForRule(
-      RuleContext ruleContext, String mainClassName);
+      RuleContext ruleContext, ImmutableList<Artifact> bootclasspath, String mainClassName);
 
   /**
    * Retrieves the full set of proguard specs that should be applied to this binary, including the
@@ -200,21 +200,20 @@ public abstract class ProguardHelper {
    * to how android_binary would give Android SDK's android.jar to Proguard as library jar, and
    * to keep the binary's entry point, ie., the main() method to be invoked.
    */
-  protected static Artifact generateSpecForJavaBinary(RuleContext ruleContext,
-      String mainClassName) {
-    // Add -libraryjars <java.home>/lib/rt.jar so Proguard uses JDK bootclasspath, which JavaCommon
-    // doesn't expose when building for JDK (see checkArgument in applyProguardIfRequested).
-    // Note <java.home>/lib/rt.jar refers to rt.jar that comes with JVM running Proguard, which
-    // should be identical to the JVM that will run the binary.
+  protected static Artifact generateSpecForJavaBinary(
+      RuleContext ruleContext, ImmutableList<Artifact> bootclasspath, String mainClassName) {
     Artifact result = ProguardHelper.getProguardConfigArtifact(ruleContext, "jvm");
     ruleContext.registerAction(
         new FileWriteAction(
             ruleContext.getActionOwner(),
             result,
-            String.format("-libraryjars <java.home>/lib/rt.jar%n"
-                + "-keep class %s {%n"
-                + "  public static void main(java.lang.String[]);%n"
-                + "}",
+            String.format(
+                "-libraryjars %s%n"
+                    + "-keep class %s {%n"
+                    + "  public static void main(java.lang.String[]);%n"
+                    + "}",
+                Artifact.joinExecPaths(
+                    ruleContext.getConfiguration().getHostPathSeparator(), bootclasspath),
                 mainClassName),
             /*executable*/ false));
     return result;