Skip to content

Commit

Permalink
Fix jacoco-toolchain dependency.
Browse files Browse the repository at this point in the history
Bazel 0.26 added the `jacocorunner` attribute on `java_toolchain`. Java targets now fail when using the new host java_toolchain because of a dependency cycle:

```
@bazel_tools//tools/jdk:JacocoCoverageRunner
@bazel_tools//tools/jdk:current_java_toolchain
@bazel_tools//tools/jdk:legacy_current_java_toolchain
@bazel_tools//tools/jdk:remote_toolchain
@remote_java_tools_linux//:toolchain
.-> @remote_java_tools_linux//:jacoco_coverage_runner (host)
|   @bazel_tools//tools/jdk:current_java_toolchain (host)
|   @bazel_tools//tools/jdk:legacy_current_java_toolchain (host)
|   @bazel_tools//tools/jdk:remote_toolchain (host)
|   @remote_java_tools_linux//:toolchain (host)
`-- @remote_java_tools_linux//:jacoco_coverage_runner (host)
```

The dependency is happening because `jacocorunner` in `java_toolchain` was declared as a `java_import` which depends on the host `java_toolchain`. The issue was not caught because we only run the java integration tests with `--java_toolchain` built at head. If we had run the tests also with `--host_java_toolchain` the issue would have been caught (see #8530).

This PR does the following:
* Makes `jacocorunner` attribute a `filegroup` instead of a `java_import` to avoid the dependency cycle.
* Converts the type of `jacocoRunner` in `JavaToolchain` from a `TransitiveInfoCollection` to `FilesToRunProvider` to be able to retrieve the executable jar from the `filegroup`.
* Adds testing with `--host_java_toolchain` of the java_tools built as head

This is a blocker for the new java_tools release (bazelbuild/java_tools#8).

Closes #8529.

PiperOrigin-RevId: 250876908
  • Loading branch information
iirina authored and copybara-github committed May 31, 2019
1 parent f34458b commit 3dc59e4
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -269,13 +269,17 @@ 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;
FilesToRunProvider jacocoRunner = javaToolchain.getJacocoRunner();
if (jacocoRunner == null) {
return false;
}
Artifact jacocoRunnerJar = jacocoRunner.getExecutable();
if (isStrict()) {
attributes.addDirectJar(jacocoRunnerJar);
}
return false;
attributes.addCompileTimeClassPathEntry(jacocoRunnerJar);
attributes.addRuntimeClassPathEntry(jacocoRunnerJar);
return true;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,12 @@ public Builder addRuntimeClassPathEntries(NestedSet<Artifact> classPathEntries)
return this;
}

public Builder addCompileTimeClassPathEntry(Artifact entry) {
Preconditions.checkArgument(!built);
compileTimeClassPath.add(entry);
return this;
}

public Builder addCompileTimeClassPathEntries(NestedSet<Artifact> entries) {
Preconditions.checkArgument(!built);
compileTimeClassPath.addTransitive(entries);
Expand Down Expand Up @@ -233,6 +239,12 @@ public Builder addDirectJars(NestedSet<Artifact> directJars) {
return this;
}

public Builder addDirectJar(Artifact directJar) {
Preconditions.checkArgument(!built);
this.directJars.add(directJar);
return this;
}

public Builder addCompileTimeDependencyArtifacts(NestedSet<Artifact> dependencyArtifacts) {
Preconditions.checkArgument(!built);
compileTimeDependencyArtifacts.addTransitive(dependencyArtifacts);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ public ConfiguredTarget create(RuleContext ruleContext)
ruleContext.getPrerequisites(
"package_configuration", Mode.HOST, JavaPackageConfigurationProvider.class));

TransitiveInfoCollection jacocoRunner = ruleContext.getPrerequisite("jacocorunner", Mode.HOST);
FilesToRunProvider jacocoRunner =
ruleContext.getExecutablePrerequisite("jacocorunner", Mode.HOST);

JavaToolchainProvider provider =
JavaToolchainProvider.create(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public static JavaToolchainProvider create(
FilesToRunProvider ijar,
ImmutableListMultimap<String, String> compatibleJavacOptions,
ImmutableList<JavaPackageConfigurationProvider> packageConfiguration,
TransitiveInfoCollection jacocoRunner,
FilesToRunProvider jacocoRunner,
JavaSemantics javaSemantics) {
return new JavaToolchainProvider(
label,
Expand Down Expand Up @@ -142,7 +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 FilesToRunProvider jacocoRunner;
private final JavaSemantics javaSemantics;

@VisibleForSerialization
Expand All @@ -169,7 +169,7 @@ public static JavaToolchainProvider create(
ImmutableList<String> javabuilderJvmOptions,
boolean javacSupportsWorkers,
ImmutableList<JavaPackageConfigurationProvider> packageConfiguration,
TransitiveInfoCollection jacocoRunner,
FilesToRunProvider jacocoRunner,
JavaSemantics javaSemantics) {
super(ImmutableMap.of(), Location.BUILTIN);

Expand Down Expand Up @@ -339,7 +339,7 @@ public ImmutableList<JavaPackageConfigurationProvider> packageConfiguration() {
return packageConfiguration;
}

public TransitiveInfoCollection getJacocoRunner() {
public FilesToRunProvider getJacocoRunner() {
return jacocoRunner;
}

Expand Down
2 changes: 2 additions & 0 deletions src/test/shell/bazel/bazel_coverage_java_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ source "${CURRENT_DIR}/../integration_test_setup.sh" \

JAVA_TOOLCHAIN="$1"; shift
add_to_bazelrc "build --java_toolchain=${JAVA_TOOLCHAIN}"
add_to_bazelrc "build --host_java_toolchain=${JAVA_TOOLCHAIN}"

JAVA_TOOLS_ZIP="$1"; shift
if [[ "${JAVA_TOOLS_ZIP}" != "released" ]]; then
Expand All @@ -37,6 +38,7 @@ JAVA_TOOLS_ZIP_FILE_URL=${JAVA_TOOLS_ZIP_FILE_URL:-}
if [[ $# -gt 0 ]]; then
JAVABASE_VALUE="$1"; shift
add_to_bazelrc "build --javabase=${JAVABASE_VALUE}"
add_to_bazelrc "build --host_javabase=${JAVABASE_VALUE}"
fi

function set_up() {
Expand Down
2 changes: 2 additions & 0 deletions src/test/shell/bazel/bazel_java_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ fi

JAVA_TOOLCHAIN="$1"; shift
add_to_bazelrc "build --java_toolchain=${JAVA_TOOLCHAIN}"
add_to_bazelrc "build --host_java_toolchain=${JAVA_TOOLCHAIN}"

JAVA_TOOLS_ZIP="$1"; shift
if [[ "${JAVA_TOOLS_ZIP}" != "released" ]]; then
Expand All @@ -73,6 +74,7 @@ JAVA_TOOLS_ZIP_FILE_URL=${JAVA_TOOLS_ZIP_FILE_URL:-}
if [[ $# -gt 0 ]]; then
JAVABASE_VALUE="$1"; shift
add_to_bazelrc "build --javabase=${JAVABASE_VALUE}"
add_to_bazelrc "build --host_javabase=${JAVABASE_VALUE}"
fi

function set_up() {
Expand Down
7 changes: 6 additions & 1 deletion tools/jdk/BUILD.java_tools
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ java_toolchain(
":java_compiler_jar",
":jdk_compiler_jar",
],
jacocorunner = ":jacoco_coverage_runner"
jacocorunner = ":jacoco_coverage_runner_filegroup"
)

filegroup(
Expand All @@ -72,6 +72,11 @@ filegroup(
srcs = ["java_tools/GenClass_deploy.jar"],
)

filegroup(
name = "jacoco_coverage_runner_filegroup",
srcs = ["java_tools/JacocoCoverage_jarjar_deploy.jar"],
)

java_import(
name = "jacoco_coverage_runner",
jars = ["java_tools/JacocoCoverage_jarjar_deploy.jar"],
Expand Down

0 comments on commit 3dc59e4

Please sign in to comment.