Add jacocorunner to java_toolchain.

The jacoco runner was previously retrieved via an implicit attribute `$jacocorunner` on the `java_binary` and `java_test` rules. Retrieving the runner via the implicit attribute makes testing the tools in the `java_tools` release difficult and inconsistent (almost all other tools are defined via `java_toolchain`). This PR makes `jacocorunner` part of `java_toolchain`. If `jacocorunner` is not found in the toolchain it falls back to the `$jacocorunner` attribute. The fallback behavior can be removed after a bazel release with this change. Closes bazelbuild#8378. PiperOrigin-RevId: 249241175
irengrig · Jun 18, 2019 · c796da7 · c796da7
1 parent fe9a03e
commit c796da7
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 13 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
@@ -670,19 +670,22 @@ public Iterable<String> getJvmFlags(
   public String addCoverageSupport(JavaCompilationHelper helper, Artifact executable) {
     // This method can be called only for *_binary/*_test targets.
     Preconditions.checkNotNull(executable);
-
-    // Add the coverage runner to the list of dependencies when compiling in coverage mode.
-    TransitiveInfoCollection runnerTarget =
-        helper.getRuleContext().getPrerequisite("$jacocorunner", Mode.TARGET);
-    if (JavaInfo.getProvider(JavaCompilationArgsProvider.class, runnerTarget) != null) {
-      helper.addLibrariesToAttributes(ImmutableList.of(runnerTarget));
-    } else {
-      helper
-          .getRuleContext()
-          .ruleError(
-              "this rule depends on "
-                  + helper.getRuleContext().attributes().get("$jacocorunner", BuildType.LABEL)
-                  + " which is not a java_library rule, or contains errors");
+    if (!helper.addCoverageSupport()) {
+      // Fallback to $jacocorunner attribute if no jacocorunner was found in the toolchain.
+
+      // Add the coverage runner to the list of dependencies when compiling in coverage mode.
+      TransitiveInfoCollection runnerTarget =
+          helper.getRuleContext().getPrerequisite("$jacocorunner", Mode.TARGET);
+      if (JavaInfo.getProvider(JavaCompilationArgsProvider.class, runnerTarget) != null) {
+        helper.addLibrariesToAttributes(ImmutableList.of(runnerTarget));
+      } else {
+        helper
+            .getRuleContext()
+            .ruleError(
+                "this rule depends on "
+                    + helper.getRuleContext().attributes().get("$jacocorunner", BuildType.LABEL)
+                    + " which is not a java_library rule, or contains errors");
+      }
     }
 
     // We do not add the instrumented jar to the runtime classpath, but provide it in the shell

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
@@ -267,6 +267,16 @@ public ImmutableList<Artifact> getBootclasspathOrDefault() {
     }
   }
 
+  public boolean addCoverageSupport() {
+    TransitiveInfoCollection jacocoRunner = javaToolchain.getJacocoRunner();
+    if (jacocoRunner != null
+        && JavaInfo.getProvider(JavaCompilationArgsProvider.class, jacocoRunner) != null) {
+      addLibrariesToAttributes(ImmutableList.of(jacocoRunner));
+      return true;
+    }
+    return false;
+  }
+
   /**
    * Creates an {@link Artifact} needed by {@code JacocoCoverageRunner}.
    *

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchain.java
@@ -106,6 +106,8 @@ public ConfiguredTarget create(RuleContext ruleContext)
             ruleContext.getPrerequisites(
                 "package_configuration", Mode.HOST, JavaPackageConfigurationProvider.class));
 
+    TransitiveInfoCollection jacocoRunner = ruleContext.getPrerequisite("jacocorunner", Mode.HOST);
+
     JavaToolchainProvider provider =
         JavaToolchainProvider.create(
             ruleContext.getLabel(),
@@ -130,6 +132,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
             ijar,
             compatibleJavacOptions,
             packageConfiguration,
+            jacocoRunner,
             semantics);
     RuleConfiguredTargetBuilder builder =
         new RuleConfiguredTargetBuilder(ruleContext)

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java
@@ -91,6 +91,7 @@ public static JavaToolchainProvider create(
       FilesToRunProvider ijar,
       ImmutableListMultimap<String, String> compatibleJavacOptions,
       ImmutableList<JavaPackageConfigurationProvider> packageConfiguration,
+      TransitiveInfoCollection jacocoRunner,
       JavaSemantics javaSemantics) {
     return new JavaToolchainProvider(
         label,
@@ -115,6 +116,7 @@ public static JavaToolchainProvider create(
         javabuilderJvmOptions,
         javacSupportsWorkers,
         packageConfiguration,
+        jacocoRunner,
         javaSemantics);
   }
 
@@ -140,6 +142,7 @@ public static JavaToolchainProvider create(
   private final ImmutableList<String> javabuilderJvmOptions;
   private final boolean javacSupportsWorkers;
   private final ImmutableList<JavaPackageConfigurationProvider> packageConfiguration;
+  private final TransitiveInfoCollection jacocoRunner;
   private final JavaSemantics javaSemantics;
 
   @VisibleForSerialization
@@ -166,6 +169,7 @@ public static JavaToolchainProvider create(
       ImmutableList<String> javabuilderJvmOptions,
       boolean javacSupportsWorkers,
       ImmutableList<JavaPackageConfigurationProvider> packageConfiguration,
+      TransitiveInfoCollection jacocoRunner,
       JavaSemantics javaSemantics) {
     super(ImmutableMap.of(), Location.BUILTIN);
 
@@ -191,6 +195,7 @@ public static JavaToolchainProvider create(
     this.javabuilderJvmOptions = javabuilderJvmOptions;
     this.javacSupportsWorkers = javacSupportsWorkers;
     this.packageConfiguration = packageConfiguration;
+    this.jacocoRunner = jacocoRunner;
     this.javaSemantics = javaSemantics;
   }
 
@@ -334,6 +339,10 @@ public ImmutableList<JavaPackageConfigurationProvider> packageConfiguration() {
     return packageConfiguration;
   }
 
+  public TransitiveInfoCollection getJacocoRunner() {
+    return jacocoRunner;
+  }
+
   public JavaSemantics getJavaSemantics() {
     return javaSemantics;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainRule.java
@@ -232,6 +232,14 @@ The Java target version (e.g., '6' or '7'). It specifies for which Java runtime
                 .cfg(HostTransition.createFactory())
                 .allowedFileTypes()
                 .mandatoryNativeProviders(ImmutableList.of(JavaPackageConfigurationProvider.class)))
+        /* <!-- #BLAZE_RULE(java_toolchain).ATTRIBUTE(jacocorunner) -->
+        Label of the JacocoCoverageRunner deploy jar.
+        <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
+        .add(
+            attr("jacocorunner", LABEL)
+                .cfg(HostTransition.createFactory())
+                .allowedFileTypes(FileTypeSet.ANY_FILE)
+                .exec())
         .build();
   }
 

diff --git a/tools/jdk/BUILD.java_tools b/tools/jdk/BUILD.java_tools
@@ -59,6 +59,8 @@ java_toolchain(
         ":java_compiler_jar",
         ":jdk_compiler_jar",
     ],
+    # TODO(iirina): Re-enable this after #8378 is merged.
+    # jacocorunner = ":jacoco_coverage_runner"
 )
 
 filegroup(
@@ -71,6 +73,11 @@ filegroup(
     srcs = ["java_tools/GenClass_deploy.jar"],
 )
 
+java_import(
+    name = "jacoco_coverage_runner",
+    jars = ["java_tools/JacocoCoverage_jarjar_deploy.jar"],
+)
+
 filegroup(
     name = "JacocoCoverage",
     srcs = ["java_tools/JacocoCoverage_jarjar_deploy.jar"],