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

4.0 breaking change: Java coverage #12793

Closed
alexeagle opened this issue Jan 9, 2021 · 4 comments
Closed

4.0 breaking change: Java coverage #12793

alexeagle opened this issue Jan 9, 2021 · 4 comments
Assignees
Labels
area-java-toolchains javabase, java_toolchain flags, JDK selection, java_toolchain rules, java_tools repository more data needed P1 I'll work on this now. (Assignee required) team-Rules-Java Issues for Java rules

Comments

@alexeagle
Copy link
Contributor

I'm not sure if there is a bug, or if this is just missing documentation (and if such documentation would be in the 4.0 release)

It seems that 4.0 introduces a breaking change for bazel coverage on Java targets, where the jacocorunner must now be configured in a toolchain. The default auto-discovered toolchain does not have one set, and after some digging I have not been able to create my own.

Every attempt to make a default_java_toolchain gives me an error like this:

(16:42:43) ERROR: Misconfigured toolchains: @remote_java_tools_darwin//:toolchain is declared as a toolchain but has inappropriate dependencies. Declared toolchains should be created with the 'toolchain' rule and should not have dependencies that themselves require toolchains.
.-> RegisteredToolchains
|   @remote_java_tools_darwin//:toolchain
|   @bazel_tools//tools/jdk:proguard_whitelister
|   toolchain types @bazel_tools//tools/python:toolchain_type, @bazel_tools//tools/cpp:toolchain_type
|   toolchain type @bazel_tools//tools/cpp:toolchain_type
`-- RegisteredToolchains

Is it expected that users need to create their own java_toolchain to keep coverage working? If so, how?

@comius comius self-assigned this Jan 11, 2021
@comius comius added area-java-toolchains javabase, java_toolchain flags, JDK selection, java_toolchain rules, java_tools repository P1 I'll work on this now. (Assignee required) release blocker team-Rules-Java Issues for Java rules labels Jan 11, 2021
@comius
Copy link
Contributor

comius commented Jan 11, 2021

Toolchains defined in java_tools all have jacoco set.
default_java_toolchain macro does not set it, so I will prepare a fix for it (unfortunately the code on master already diverged too much to cherry pick it)

@alexeagle could you provide more details about autoconfiguration that you're mentioning? Are you using bazel-toolchains?

Just using default_java_toolchain target I cannot reproduce the error.
From the error message it seems there is a call of register_toolchains or --extra_toolchains flag on a java_toolchain target, whereas this should be called on toolchain target.

comius added a commit to comius/bazel that referenced this issue Jan 11, 2021
In such case jacoco runner was not set on the java_toolchain.

Issue bazelbuild#12793
comius added a commit to comius/bazel that referenced this issue Jan 11, 2021
bazel-io pushed a commit that referenced this issue Jan 11, 2021
@alexeagle
Copy link
Contributor Author

@comius thanks for looking, my repro is pretty simple:

.bazelrc:

build --javacopt="-source 8"
build --javacopt="-target 8"
build --host_javabase=@bazel_tools//tools/jdk:remote_jdk11
build --javabase=@bazel_tools//tools/jdk:remote_jdk11
build --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_java11
build --java_toolchain=@bazel_tools//tools/jdk:toolchain_java11

.bazelversion:

4.0.0rc7

WORKSPACE:

workspace(name = "repro")

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive", "http_file", "http_jar")


http_archive(
    name = "rules_java",
    sha256 = "f3f75b52e1a70c0e2c6f8c4387feca5dbbb9f9d9cc9f64031994411291516877",
    strip_prefix = "rules_java-d43b0aa2d3b2173fcc54d3c194e89fa66cdc6e1a",
    urls = [
        "https://mirror.bazel.build/github.com/bazelbuild/rules_java/archive/d43b0aa2d3b2173fcc54d3c194e89fa66cdc6e1a.tar.gz",
        "https://github.com/bazelbuild/rules_java/archive/d43b0aa2d3b2173fcc54d3c194e89fa66cdc6e1a.tar.gz",
    ],
)

load("@rules_java//java:repositories.bzl", "rules_java_dependencies", "rules_java_toolchains")

rules_java_dependencies()

rules_java_toolchains()

BUILD.bazel:

load("@rules_java//java:defs.bzl", "java_binary", "java_test")

java_binary(
    name = "bin",
    srcs = ["Main.java"],
    main_class = "Main",
)
% bazel coverage :all
ERROR: /Users/alex.eagle/repros/java_coverage/BUILD.bazel:3:12: in java_binary rule //:bin: jacocorunner not set in java_toolchain:@bazel_tools//tools/jdk:toolchain_java11

I see that if I comment out --java_toolchain in the .bazelrc then it's not reproducible anymore. That line was added in our repo for java_proto_library FWIW

@alexeagle
Copy link
Contributor Author

AFAICT we don't need these java_toolchain override lines in the .bazelrc for this repo. I'll confirm that our upgrade is unblocked, and then I think the severity here is probably low (unless you know of a reason other Bazel users might need to select those toolchains from bazel_tools)
/cc @redisliu

@comius
Copy link
Contributor

comius commented Jan 12, 2021

Thanks for reproduction. I'm confirming the bug as well as that the PR fixes it.

@alexeagle Do you also have reproduction for the ERROR: Misconfigured toolchains? I'd like to make sure there isn't another problem.

comius added a commit that referenced this issue Jan 12, 2021
* Fix coverage support when using default_java_toolchain.

In such case jacoco runner was not set on the java_toolchain.

Issue #12793

* Additional coverage test for predefined toolchains.

* Revert "Additional coverage test for predefined toolchains."

This reverts commit da3f7fd.
meisterT pushed a commit that referenced this issue Jan 12, 2021
* Fix coverage support when using default_java_toolchain.

In such case jacoco runner was not set on the java_toolchain.

Issue #12793

* Additional coverage test for predefined toolchains.

* Revert "Additional coverage test for predefined toolchains."

This reverts commit da3f7fd.
@comius comius closed this as completed Jan 13, 2021
ulfjack pushed a commit to EngFlow/bazel that referenced this issue Mar 5, 2021
…2801)

* Fix coverage support when using default_java_toolchain.

In such case jacoco runner was not set on the java_toolchain.

Issue bazelbuild#12793

* Additional coverage test for predefined toolchains.

* Revert "Additional coverage test for predefined toolchains."

This reverts commit da3f7fd.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-java-toolchains javabase, java_toolchain flags, JDK selection, java_toolchain rules, java_tools repository more data needed P1 I'll work on this now. (Assignee required) team-Rules-Java Issues for Java rules
Projects
None yet
Development

No branches or pull requests

2 participants