Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix jacoco-toolchain dependency. #8529

Closed
wants to merge 7 commits into from

Conversation

iirina
Copy link
Contributor

@iirina iirina commented May 31, 2019

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).

@iirina iirina requested a review from hlopko as a code owner May 31, 2019 08:52
@iirina iirina removed the request for review from hlopko May 31, 2019 08:56
@iirina
Copy link
Contributor Author

iirina commented May 31, 2019

Not ready for review.

@iirina iirina closed this May 31, 2019
@iirina iirina deleted the fix-java-toolchain-jacoco branch May 31, 2019 08:56
@iirina iirina restored the fix-java-toolchain-jacoco branch May 31, 2019 08:56
@iirina iirina reopened this May 31, 2019
@iirina iirina requested a review from hlopko May 31, 2019 11:15
@iirina
Copy link
Contributor Author

iirina commented May 31, 2019

@hlopko This PR is ready for review now.

@bazel-io bazel-io closed this in 3dc59e4 May 31, 2019
irengrig pushed a commit to irengrig/bazel that referenced this pull request Jun 18, 2019
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 bazelbuild#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 bazelbuild#8529.

PiperOrigin-RevId: 250876908
irengrig pushed a commit to irengrig/bazel that referenced this pull request Jul 15, 2019
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 bazelbuild#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 bazelbuild#8529.

PiperOrigin-RevId: 250876908
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants