Add baseline coverage support to Java compilation action

[fmeum]

• JacocoInstrumentationProcessor optionally analyzes all instrumented classes with null probes and generates an LCOV baseline report.
• create_compilation_action gains a baseline_coverage_file parameter that rules_java can use to supply the desired output file for the baseline report.

Work towards #5716

[fmeum]

Stacked on #26340, please ignore the first commit.
Enabling the test requires a small change to rules_java that I will send out for review after this PR has been merged: bazelbuild/rules_java#304

[fmeum]

@c-mita @cushon No longer stacked and ready for review.

[fmeum]

@hvadehra Friendly ping, what needs to be done to get this merged?

[hvadehra]

@c-mita for coverage

[fmeum]

@c-mita Friendly ping, this would need to get into Bazel 9 so that further Starlark work can be based on it

[hvadehra]

c-mita is OOO for a while, let's get this merged.

[hvadehra]

@fmeum //src/java_tools/buildjar/java/com/google/devtools/build/buildjar:buildjar is also used internally, and adding a dep on JacocoCoverageLib breaks things because the version of jacoco we're using internally includes this breaking change jacoco/jacoco@f823d43#diff-d72a1a86ab329396109e6450e579c98ac482204ce62d0ff6aabcfb6ca353cde7R54

(this wasn't a problem before this change because JacocoCoverageLib has been Bazel-only since 2019¹)

Footnotes

1. internal ref cl 228858523 ↩

[fmeum]

Is this what @c-mita has been working on with ef614e1?

Happy to update the JaCoCo version in Bazel and modify this appropriately - what would you suggest?

[hvadehra]

Updating jacoco SGTM, if @meteorcloudy is okay with using an unreleased version.

[meteorcloudy]

We check-in prebuilt jars for jacoco: https://github.com/bazelbuild/bazel/tree/master/third_party/java/jacoco

Are we going to switch from build from source, I'm okay with that.

[fmeum]

@meteorcloudy I can send an update to the prebuilt jars, but I guess you would want a Googler to generate them? Anything I can do to help with that?

[meteorcloudy]

Hmm, we used to download those jars from https://mvnrepository.com/artifact/org.jacoco/jacoco-maven-plugin, I'm not really comfortable with checking in custom built jars. How important is this? Could we wait for the official relesae?

[fmeum]

We can wait

[fmeum]

0.8.14 is out now: https://search.maven.org/remotecontent?filepath=org/jacoco/jacoco/0.8.14/jacoco-0.8.14.zip

I've never gone through the update procedure for JaCoCo. Does it help if I send a PR for the first step or would you rather do it yourself to ensure that the jars haven't been tampered with?

[c-mita]

Is this what @c-mita has been working on with ef614e1?

Happy to update the JaCoCo version in Bazel and modify this appropriately - what would you suggest?

Jacoco changed some things that means Bazel's branch analyzer needs updating. I have already done the updates for our internal version and can easily update Bazel's when I'm back from paternity leave in a couple of weeks.

[c-mita]

Speaking to @hvadehra, I would really like to avoid adding the dependency on JacocoCoverageLib in buildjar.

Whilst I could resolve the issue now (and might do so for other reasons), this is likely to cause a problem in the future due to version skew.

I think the "simplest" solution might a separate java binary that just gets run on the output jar that just performs coverage on the uninstrumented class files.

I did an experiment with things internally a while back with a binary that consumed a java_binary's _deploy_jar to generate baseline coverage. I think something like that could be wired in without adding anything to the compile action.

[fmeum]

Whilst I could resolve the issue now (and might do so for other reasons), this is likely to cause a problem in the future due to version skew.

Could you elaborate on the kind of version skew you are worried about? JaCoCo is already a dependency of buildjar through JacocoInstrumentationProcessor and the JacocoCoverageLib is, at least for Bazel, released together with buildjar via junitrunner. Is the situation different internally?

[c-mita]

Fundamentally the issue is that we have a version of Jacoco internally that is currently not the same version as the copy Bazel uses. We also cannot maintain two versions internally. Whilst I would like to keep the Bazel's version and Google's version in sync as much as possible, it isn't practical to keep them perfectly in sync.

For JacocoInstrumentationProcessor this doesn't matter too much because that doesn't depend on any of Jacoco's internal interfaces, using only the published API. But JacocoCoverageLib does depend on internal interfaces and so is more prone to breakages (indeed, this has already happened).

[fmeum]

I see, thanks for the explanation. What makes the usage through junitrunner less problematic? Since that is preexisting, maybe I can make sure that I don't add extra complexity beyond that.

[c-mita]

What makes the usage through junitrunner less problematic?

Well, it's been forked already.

BazelTestRunner doesn't depend on Jacoco or the coverage lib and //src/java_tools/junitrunner/java/com/google/testing/coverage is forked; we have a separate implementation used internally.

Obviously whatever does the actual generation of a baseline coverage report is going to have to depend on
//src/java_tools/junitrunner/java/com/google/testing/coverage in some capacity. However, a trivial binary that calls new JacocoCoverageRunner(...).create() should not be a problem (maybe the interface to JacocoCoverageRunner could be improved to make this easier).

I would like to avoid forking a complex binary, but a trivial one (if it's even necessary) is less of an issue.