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

Add aliases for :lcov_merger and :coverage_report_generator targets #10379

Closed
wants to merge 1 commit into from

Conversation

ulfjack
Copy link
Contributor

@ulfjack ulfjack commented Dec 7, 2019

Currently, the coverage tool setup is smeared across @bazel_tools//tools/test/BUILD, @bazel_tools//tools/test/CoverageOutputGenerator/.../BUILD (both shipped inside Bazel), and @remote_coverage_tools//, which is separately uploaded as a zipped repository. The second BUILD file contains java_* rules which are actually used. The remote_coverage_tools repository contains a copy of the java_* rules, but they are never used, and the java_import in the former directly references the deploy jar in the latter.

Instead, the plan is to only ship @bazel_tools//tools/test/BUILD in Bazel, and use alias rules to point to the @remote_coverage_tools repository. There, we define two rules (:lcov_merger and :coverage_report_generator), which fully define the implementation. This allows the repository to be swapped out for another repository with a different implementation using --override_repository, which facilitates independent work on the merger and generator.

The underlying problem is that the merger currently does not work with remote execution, and there is no workaround because all the relevant parts are hard-coded in Bazel, which requires a Bazel release to fix. By making the coverage tools self-contained, we make it easier to solve such problems in the future.

This should not cause any problems for the existing workaround described in #4685.

Progress on #4685.

Change-Id: I26758db573a1966b40169314d4aec61eff83f83b

Currently, the coverage tool setup is smeared across @bazel_tools//tools/test/BUILD, @bazel_tools//tools/test/CoverageOutputGenerator/.../BUILD (both shipped inside Bazel), and @remote_coverage_tools//, which is separately uploaded as a zipped repository. The second BUILD file contains java_* rules which are actually used. The remote_coverage_tools repository contains a copy of the java_* rules, but they are never used, and the java_import in the former directly references the deploy jar in the latter.

Instead, the plan is to only ship @bazel_tools//tools/test/BUILD in Bazel, and use alias rules to point to the @remote_coverage_tools repository. There, we define two rules (:lcov_merger and :coverage_report_generator), which fully define the implementation. This allows the repository to be swapped out for another repository with a different implementation using --override_repository, which facilitates independent work on the merger and generator.

The underlying problem is that the merger currently does not work with remote execution, and there is no workaround because all the relevant parts are hard-coded in Bazel, which requires a Bazel release to fix. By making the coverage tools self-contained, we make it easier to solve such problems in the future.

This should not cause any problems for the existing workaround described in bazelbuild#4685.

Progress on bazelbuild#4685.

Change-Id: I26758db573a1966b40169314d4aec61eff83f83b
@ulfjack ulfjack requested a review from iirina as a code owner December 7, 2019 02:39
ulfjack added a commit to ulfjack/bazel that referenced this pull request Dec 7, 2019
Requires:
 - bazelbuild#10379 is merged
 - a new remote coverage tools zip pushed
 - coverage.WORKSPACE updated to the new tools

This changes the @bazel_tools//tools/test/BUILD file to fully delegate to the @remote_coverage_tools repository, which must contain rules for :lcov_merger and :coverage_report_generator.

This makes the @remote_coverage_tools reference self-contained, which allows overriding the tools using --override_repository, and allows independently replacing or fixing them.

Progress on bazelbuild#4685.

Change-Id: I321c62332f00d910f4ccfb3244d63e60627d59ad
ulfjack added a commit to ulfjack/bazel that referenced this pull request Dec 7, 2019
Requires:
 - bazelbuild#10379 is merged
 - a new remote coverage tools zip pushed
 - coverage.WORKSPACE updated to the new tools

This changes the @bazel_tools//tools/test/BUILD file to fully delegate to the @remote_coverage_tools repository, which must contain rules for :lcov_merger and :coverage_report_generator.

This makes the @remote_coverage_tools reference self-contained, which allows overriding the tools using --override_repository, and allows independently replacing or fixing them.

Progress on bazelbuild#4685.

Change-Id: I321c62332f00d910f4ccfb3244d63e60627d59ad
@bazel-io bazel-io closed this in 21298e3 Dec 9, 2019
bazel-io pushed a commit that referenced this pull request Dec 12, 2019
Requires:
 - #10379 is merged
 - a new remote coverage tools zip pushed
 - coverage.WORKSPACE updated to the new tools

This changes the @bazel_tools//tools/test/BUILD file to fully delegate to the @remote_coverage_tools repository, which must contain rules for :lcov_merger and :coverage_report_generator.

This makes the @remote_coverage_tools reference self-contained, which allows overriding the tools using --override_repository, and allows independently replacing or fixing them.

Progress on #4685.

Change-Id: I321c62332f00d910f4ccfb3244d63e60627d59ad

Closes #10383.

Change-Id: I321c62332f00d910f4ccfb3244d63e60627d59ad
PiperOrigin-RevId: 285246323
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.

2 participants