From 29362026e2e3c1a2cdbbdd549176e8395426bdb4 Mon Sep 17 00:00:00 2001 From: ilist Date: Tue, 3 Nov 2020 08:33:33 -0800 Subject: [PATCH] Use jacocorunner from java_toolchain in Bazel. Before jacocorunner from @bazel_tools/tools/jdk:jacocorunner was used, which is implemented with remote_java_tools. remote_java_tools uses select function that reconstructs which java_tools archive is actually used and then point to jacocorunner from the archive. Since jacocorunner is already deployed in Bazel it is nicer to attach it to java_toolchain. Also it doesn't use fragile selects from remote_java_tools. PiperOrigin-RevId: 340450013 --- .../bazel/rules/java/BazelJavaBinaryRule.java | 4 ---- .../bazel/rules/java/BazelJavaSemantics.java | 19 +------------------ .../lib/rules/java/JavaCompilationHelper.java | 7 ++++--- .../lib/analysis/mock/BazelAnalysisMock.java | 4 +++- 4 files changed, 8 insertions(+), 26 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaBinaryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaBinaryRule.java index 412c44558df510..d051402a623b25 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaBinaryRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaBinaryRule.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.bazel.rules.java; import static com.google.devtools.build.lib.packages.Attribute.attr; -import static com.google.devtools.build.lib.packages.BuildType.LABEL; import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; import static com.google.devtools.build.lib.packages.Type.BOOLEAN; @@ -102,9 +101,6 @@ public Object getDefault(AttributeMap rule) { return rule.get("create_executable", BOOLEAN); } })) - .add( - attr("$jacocorunner", LABEL) - .value(env.getToolsLabel("//tools/jdk:JacocoCoverageRunner"))) .addRequiredToolchains(CppRuleClasses.ccToolchainTypeAttribute(env)) .useToolchainTransition(true) .build(); 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 index 1f0a06075074dd..179240e88e95ae 100644 --- 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 @@ -57,7 +57,6 @@ import com.google.devtools.build.lib.rules.java.JavaConfiguration; import com.google.devtools.build.lib.rules.java.JavaConfiguration.OneVersionEnforcementLevel; import com.google.devtools.build.lib.rules.java.JavaHelper; -import com.google.devtools.build.lib.rules.java.JavaInfo; import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider; import com.google.devtools.build.lib.rules.java.JavaSemantics; import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider; @@ -611,23 +610,7 @@ public Iterable getJvmFlags( public String addCoverageSupport(JavaCompilationHelper helper, Artifact executable) { // This method can be called only for *_binary/*_test targets. Preconditions.checkNotNull(executable); - 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"); - 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"); - } - } + helper.addCoverageSupport(); // We do not add the instrumented jar to the runtime classpath, but provide it in the shell // script via an environment variable. 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 26e073c8fdbde0..cedb6f836c9243 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 @@ -372,10 +372,12 @@ public BootClassPathInfo getBootclasspathOrDefault() { } } - public boolean addCoverageSupport() { + /** Adds coverage support from java_toolchain. */ + public void addCoverageSupport() { FilesToRunProvider jacocoRunner = javaToolchain.getJacocoRunner(); if (jacocoRunner == null) { - return false; + ruleContext.ruleError( + "jacocorunner not set in java_toolchain:" + javaToolchain.getToolchainLabel()); } Artifact jacocoRunnerJar = jacocoRunner.getExecutable(); if (isStrict()) { @@ -383,7 +385,6 @@ public boolean addCoverageSupport() { } attributes.addCompileTimeClassPathEntry(jacocoRunnerJar); attributes.addRuntimeClassPathEntry(jacocoRunnerJar); - return true; } /** diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java index 291881e8775944..c936378db54b10 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java @@ -142,6 +142,7 @@ public void setupMockClient(MockToolsConfig config, List workspaceConten " extclasspath = [':extclasspath'],", " javac = [':langtools'],", " javabuilder = ['JavaBuilder_deploy.jar'],", + " jacocorunner = ':JacocoCoverage',", " header_compiler = ['turbine_deploy.jar'],", " header_compiler_direct = ['TurbineDirect_deploy.jar'],", " singlejar = ['SingleJar_deploy.jar'],", @@ -156,6 +157,7 @@ public void setupMockClient(MockToolsConfig config, List workspaceConten " extclasspath = [':extclasspath'],", " javac = [':langtools'],", " javabuilder = ['JavaBuilder_deploy.jar'],", + " jacocorunner = ':JacocoCoverage',", " header_compiler = ['turbine_deploy.jar'],", " header_compiler_direct = ['TurbineDirect_deploy.jar'],", " singlejar = ['SingleJar_deploy.jar'],", @@ -191,7 +193,7 @@ public void setupMockClient(MockToolsConfig config, List workspaceConten "filegroup(name='bootclasspath', srcs=['jdk/jre/lib/rt.jar'])", "filegroup(name='extdir', srcs=glob(['jdk/jre/lib/ext/*']))", "filegroup(name='java', srcs = ['jdk/jre/bin/java'])", - "filegroup(name='JacocoCoverage', srcs = [])", + "filegroup(name='JacocoCoverage', srcs = ['JacocoCoverage_deploy.jar'])", "exports_files([", " 'JavaBuilder_deploy.jar',", " 'SingleJar_deploy.jar',",