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

Bazel 6.0.0 Java 8 Coverage Broken #17165

Closed
cjohnstoniv opened this issue Jan 8, 2023 · 21 comments
Closed

Bazel 6.0.0 Java 8 Coverage Broken #17165

cjohnstoniv opened this issue Jan 8, 2023 · 21 comments
Labels

Comments

@cjohnstoniv
Copy link

Description of the bug:

When building with Bazel 6.0.0 and building a Java 8 (compile + runtime, with a local JDK setup) workspace fails to calculate any coverage.

There error I see in the test logs is:

Exception in thread "Thread-0" java.lang.NoSuchMethodError: java.util.Optional.isEmpty()Z
at com.google.testing.coverage.JacocoLCOVFormatter$1.getExecPathForEntryName(JacocoLCOVFormatter.java:73)
at com.google.testing.coverage.JacocoLCOVFormatter$1.visitBundle(JacocoLCOVFormatter.java:117)
at com.google.testing.coverage.JacocoCoverageRunner.createReport(JacocoCoverageRunner.java:156)
at com.google.testing.coverage.JacocoCoverageRunner.create(JacocoCoverageRunner.java:132)
at com.google.testing.coverage.JacocoCoverageRunner$2.run(JacocoCoverageRunner.java:568)

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Execute bazel coverage on any Java 8 (with a local OpenJDK 8 setup) workspace and look at the coverage files, they are empty.

Which operating system are you running Bazel on?

Mac & Linux

What is the output of bazel info release?

release 6.0.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

Is JacocoLCOVFormatter only Java 11 supported now? Is there any way that maybe we can just change it to use !optional.isPresent() to make it Java 8 compatible?

@sgowroji
Copy link
Member

sgowroji commented Jan 9, 2023

Hello @cjohnstoniv, Can you provide an example code to reproduce the above error. Thanks!

@cjohnstoniv
Copy link
Author

Unfortunately not easily as the code base we build this on is private and I cannot share it. Also it's built using a localjdk workspace config so none of that would be repeatable on anyone else's machine (unless they have java in the same path).

The issue is quite straightforward / obvious though. The Optional.isEmpty method was added in Java 9+ so it would (as expected) fail in any Java 8 JVMs. My ask is can we just make this 1 line Java 8 compliant by making it !optional.isPresent() instead.

If Bazel still intends to support Java 8+ then this would be a requirement to make sure that the coverage tools remain Java 8 compliant. Unless the intention is Java 8+ compilation and not runtime.

This all works just fine on Bazel 5.4.0 and if you look at the diffs between 5.4.0 and 6.0.0, you can see this class updated to Java 9+ only complaint code.

@fmeum
Copy link
Collaborator

fmeum commented Jan 9, 2023

I personally don't see a reason not to make this one line compatible with Java 8. But even if you are targeting Java 8, I would expect the coverage tool to be built in the exec configuration and (by default) use a remote JDK 11. Using a more modern JDK just for tools could help you in the meantime.

@cjohnstoniv
Copy link
Author

We want to run our runtime on Java 8 too as unfortunately our org still has a lot of software still running on Oracle Java 8 JDKs (gross, I know). So from a Bazel build/test perspective we are trying to keep it all as consistent as possible. Testing with Java 11 runtime but with Java 8 target, isn't the same as compiling and testing on the same JDK version. There are plenty of things like needing to add-opens/exports to make Java 8 compiled code run in Java 11 runtimes.

If the answer is the devs don't feel making this Java 8 compatible is the right thing to do, then we should update the docs to make it clear that Java 8 is supported purely from compilation and not from runtime.

@fmeum
Copy link
Collaborator

fmeum commented Jan 9, 2023

I don't mean "compile with Java 11, run with Java 8". What I am referring to is using something like --java_runtime_version=local_jdk_8 --tool_java_runtime_version=remotejdk_11 to:

  • build and test all regular targets with JDK 8
  • build and execute tools run as part of the build itself (for example the coverage tools) with JDK 11

@cjohnstoniv Could you share at least the parts of your .bazelrc and WORKSPACE that configure Java toolchains?

@cjohnstoniv
Copy link
Author

.bazelrc

build --embed_label=9.0.LTS-SNAPSHOT
build --java_language_version=8
build --tool_java_language_version=11
build --tool_java_runtime_version=remotejdk_11
build --java_runtime_version=localjdk_8
build --verbose_failures
build --experimental_ui_max_stdouterr_bytes=100048576
startup --max_idle_secs=300
test --test_output=errors
test --test_verbose_timeout_warnings
coverage --collect_code_coverage
coverage --combined_report=lcov
coverage --coverage_report_generator=@bazel_aladdin//sonarqube:SonarQubeCoverageGenerator
build --remote_cache=grpc://[SOME IP]:8980
build --remote_max_connections=200
coverage --experimental_split_coverage_postprocessing
coverage --experimental_fetch_all_coverage_outputs
build --disk_cache=/Users/chjohnst/mazel/.cache

WORKSPACE toolchains:

load("@bazel_tools//tools/jdk:local_java_repository.bzl", "local_java_repository")

local_java_repository(
  name = "localjdk",
  version = '8',
  java_home = "/Library/Java/JavaVirtualMachines/jdk1.8.0_202.jdk/Contents/Home"
)

@fmeum based on your comments above, I was under the impression that using --tool_java_language_version=11 --tool_java_runtime_version=remotejdk_11 would have solved the issue (before we had --tool_java_language_version=8 --tool_java_runtime_version=remotejdk_11). But I still see the same error in the test logs:

Exception in thread "Thread-0" java.lang.NoSuchMethodError: java.util.Optional.isEmpty()Z
at com.google.testing.coverage.JacocoLCOVFormatter$1.getExecPathForEntryName(JacocoLCOVFormatter.java:73)
at com.google.testing.coverage.JacocoLCOVFormatter$1.visitBundle(JacocoLCOVFormatter.java:117)
at com.google.testing.coverage.JacocoCoverageRunner.createReport(JacocoCoverageRunner.java:156)
at com.google.testing.coverage.JacocoCoverageRunner.create(JacocoCoverageRunner.java:132)
at com.google.testing.coverage.JacocoCoverageRunner$2.run(JacocoCoverageRunner.java:568)
GCov does not exist at the given path: ''

@fmeum
Copy link
Collaborator

fmeum commented Jan 10, 2023

If that also doesn't work I am personally out of ideas why you are seeing this issue. team-Rules-Java can probably help here.

@cjohnstoniv
Copy link
Author

I'll see if I can find some time later this week from my personal PC to upload a small Java project that recreates the issue so others can test.

Oddly when I think about it, I would've expected the tools compilation stage to fail when I had set --tool_java_language_version=8 as it would have tried compiling Java 11 code on Java 8; but instead it fails with a runtime NoSuchMethodError which suggests the tool is compiling on Java 11+ but for whatever reason is not running on Java 11+.

@cjohnstoniv
Copy link
Author

cjohnstoniv commented Jan 19, 2023

broken-java8-coverage.zip

Attached is a sample project that recreates the issue following the following steps (assumes you downloaded the zip to an empty dir, also assumes linux OS, modify the less location for other OS's, you may also have to modify your Java 8 localjdk location in WORKSPACE file):

unzip broken-java8-coverage.zip; bazelisk coverage //...; less bazel-out/k8-fastbuild/testlogs/unit-tests/test.log

You will see that despite the tool java level and runtime being 11, the test runner still fails to calculate coverage due to the Java 9+ added Optional method.

@cjohnstoniv
Copy link
Author

Also not sure if anyone has any experience with the Bazel distribution build but I tried checking out the 6.0 tag and compiling my own version of Bazel with changes to the JacocoLCOVFormatter class; however I found that seemingly when I run compile.sh and/or bazel build //src:bazel my changes to the class aren't picked up. I've confirmed this at least by making non-compilable changes to the class (i.e bad characters) and the bazel build runs just fine.

Is there someway I can rebuild/retest the JacocoLCOVFormatter class? I was trying to test what would happen if we made the isEmpty() call !isPresent() instead.

@fmeum
Copy link
Collaborator

fmeum commented Jan 26, 2023

@cushon This may he another instance of #17281: The tool Java language level/runtime version not behaving as one may expect. Could you also provide some insight for this particular situation?

@cushon
Copy link
Contributor

cushon commented Jan 26, 2023

@comius

I can reproduce (thanks for #17165 (comment))!

Exception in thread "Thread-0" java.lang.NoSuchMethodError: java.util.Optional.isEmpty()Z
        at com.google.testing.coverage.JacocoLCOVFormatter$1.getExecPathForEntryName(JacocoLCOVFormatter.java:73)
...
        at com.google.testing.coverage.JacocoCoverageRunner$2.run(JacocoCoverageRunner.java:568)

It looks like JacocoCoverageRunner is being built in the exec configuration, and then apparently being run in the target configuration as part of test execution:

/* <!-- #BLAZE_RULE(java_toolchain).ATTRIBUTE(jacocorunner) -->
Label of the JacocoCoverageRunner deploy jar.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(
attr("jacocorunner", LABEL)
// This needs to be in the execution configuration.
.cfg(ExecutionTransitionFactory.create())
.allowedFileTypes(FileTypeSet.ANY_FILE)
.exec())

I'm not sure what the intent there was, but either it shouldn't be running on the target JDK, or it should be built in the target configuration.

@fmeum
Copy link
Collaborator

fmeum commented Jan 26, 2023

In Bazel JacocoCoverageRunner appears to be taken from remote_coverage_tools in precompiled form, but the packaging process probably uses JDK 11 with -target 8, which doesn't catch this.

@cjohnstoniv
Copy link
Author

Is there any thoughts to a solution for this? Can we not just make the coverage tool's code Java 8 compliant so we know it will run in Java 8 JVM? This is assuming the isEmpty() usage is the only place we use Java 11+ code, but I also imagine there aren't any real needs for Java 11+ code other than "nice-to-haves".

@fmeum
Copy link
Collaborator

fmeum commented Feb 6, 2023

#17338 is in review and fixes this. @cushon Can you say whether Ivo's input is truly needed? He appears to be quite busy.

It's unfortunately hard to cover with tests as that would require maintaining a JDK 8 on all CI machine. But if this breaks in the future, it can probably be fixed again as long as Java 8 is supported.

hvadehra pushed a commit that referenced this issue Feb 14, 2023
`JacocoCoverageRunner` is run with the target runtime, not the exec runtime, and thus needs to be compatible with a Java 8 runtime.

Fixes #17165

Closes #17338.

PiperOrigin-RevId: 509123073
Change-Id: I41d7255e1f3c3dd3864082899d5dc9ab2cc82027
ShreeM01 added a commit that referenced this issue Feb 15, 2023
`JacocoCoverageRunner` is run with the target runtime, not the exec runtime, and thus needs to be compatible with a Java 8 runtime.

Fixes #17165

Closes #17338.

PiperOrigin-RevId: 509123073
Change-Id: I41d7255e1f3c3dd3864082899d5dc9ab2cc82027

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
@cjohnstoniv
Copy link
Author

cjohnstoniv commented Mar 6, 2023

I still get this issue on 6.1.0, the project above still recreates this issue.

@fmeum
Copy link
Collaborator

fmeum commented Mar 6, 2023

Rules requires a Java tools update and it's possible that didn't make it into the release.

@hvadehra Is there already a release with the change?

@hvadehra
Copy link
Member

hvadehra commented Mar 7, 2023

https://github.com/bazelbuild/java_tools/releases/tag/java_v11.12 was released a few days ago which should contain c266651

@cjohnstoniv
Copy link
Author

So can we reopen this issue then? It seems that even if the patch did go out, it's not actually fixing the issue defined here/demonstrated by the example.

@fmeum
Copy link
Collaborator

fmeum commented Mar 7, 2023

The fix went into the Java tools, but the commit bumping the pinned version of these tools didn't go into 6.1.0. It does look like it will be cherry-picked into 6.2.0.

@thirtyseven
Copy link
Contributor

thirtyseven commented Mar 8, 2023

@cjohnstoniv You should be able to manually update remote_java_tools to v11.12 in your WORKSPACE too.

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 a pull request may close this issue.

6 participants