-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Java coverage: fix handling of external files #13377
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When java_test rules are run with coverage, then Bazel writes a file containing the runtime classpath using root-relative paths. The collect_coverage.sh script then processes that file to generate another file containing absolute paths by prefixing each of the paths with the runfiles directory and the workspace name. It then runs the singlejar tool on this list of jar files to merge them into a single file. This happens to work for all jar files in the main repository because the runfiles path is the runfiles directory plus the workspace name plus the root-relative path. However, it is broken if some of the jar files come from an external repository. It appears that singlejar errors out on the first such jar file, generating a partial output jar file without a central directory. The error is swallowed by the `collect_coverage.sh` script. Presence of coverage results may thus depend on the order of classpath entries. In order to fix this, we change the runtime classpath file to contain runfiles-relative paths, and the coverage script to prefix them with the runfiles directory. Note that we reuse SourceManifestAction here, i.e., the identical code that is also responsible for generating the runfiles directory in the first place. This is the only reliable way to get the correct paths into the test action. There are more places that use `LazyWritePathsFileAction` and I suspect that all of them are broken: - persistent test runner support in BazelJavaSemantics (AFAIK, this doesn't work in Bazel anyway) - coverage w/ metadata jars in BazelJavaSemantics (not used by JavaBinary) - coverage source list file ('-paths-for-coverage.txt'); this probably results in non-functioning coverage for *source files* in external repositories Fixes bazelbuild#13376. Change-Id: Ie9bcc92344f06e190efcb192a3b6ef9905aea352
Can I haz test case? |
katre
pushed a commit
that referenced
this pull request
Jul 12, 2021
When java_test rules are run with coverage, then Bazel writes a file containing the runtime classpath using root-relative paths. The collect_coverage.sh script then processes that file to generate another file containing absolute paths by prefixing each of the paths with the runfiles directory and the workspace name. It then runs the singlejar tool on this list of jar files to merge them into a single file. This happens to work for all jar files in the main repository because the runfiles path is the runfiles directory plus the workspace name plus the root-relative path. However, it is broken if some of the jar files come from an external repository. It appears that singlejar errors out on the first such jar file, generating a partial output jar file without a central directory. The error is swallowed by the `collect_coverage.sh` script. Presence of coverage results may thus depend on the order of classpath entries. In order to fix this, we change the runtime classpath file to contain runfiles-relative paths, and the coverage script to prefix them with the runfiles directory. Note that we reuse SourceManifestAction here, i.e., the identical code that is also responsible for generating the runfiles directory in the first place. This is the only reliable way to get the correct paths into the test action. There are more places that use `LazyWritePathsFileAction` and I suspect that all of them are broken: - persistent test runner support in BazelJavaSemantics (AFAIK, this doesn't work in Bazel anyway) - coverage w/ metadata jars in BazelJavaSemantics (not used by JavaBinary) - coverage source list file ('-paths-for-coverage.txt'); this probably results in non-functioning coverage for *source files* in external repositories Fixes #13376. Change-Id: Ie9bcc92344f06e190efcb192a3b6ef9905aea352 Closes #13377. Change-Id: Ie9bcc92344f06e190efcb192a3b6ef9905aea352 PiperOrigin-RevId: 369687409
katre
pushed a commit
to katre/bazel
that referenced
this pull request
Jul 13, 2021
When java_test rules are run with coverage, then Bazel writes a file containing the runtime classpath using root-relative paths. The collect_coverage.sh script then processes that file to generate another file containing absolute paths by prefixing each of the paths with the runfiles directory and the workspace name. It then runs the singlejar tool on this list of jar files to merge them into a single file. This happens to work for all jar files in the main repository because the runfiles path is the runfiles directory plus the workspace name plus the root-relative path. However, it is broken if some of the jar files come from an external repository. It appears that singlejar errors out on the first such jar file, generating a partial output jar file without a central directory. The error is swallowed by the `collect_coverage.sh` script. Presence of coverage results may thus depend on the order of classpath entries. In order to fix this, we change the runtime classpath file to contain runfiles-relative paths, and the coverage script to prefix them with the runfiles directory. Note that we reuse SourceManifestAction here, i.e., the identical code that is also responsible for generating the runfiles directory in the first place. This is the only reliable way to get the correct paths into the test action. There are more places that use `LazyWritePathsFileAction` and I suspect that all of them are broken: - persistent test runner support in BazelJavaSemantics (AFAIK, this doesn't work in Bazel anyway) - coverage w/ metadata jars in BazelJavaSemantics (not used by JavaBinary) - coverage source list file ('-paths-for-coverage.txt'); this probably results in non-functioning coverage for *source files* in external repositories Fixes bazelbuild#13376. Change-Id: Ie9bcc92344f06e190efcb192a3b6ef9905aea352 Closes bazelbuild#13377. Change-Id: Ie9bcc92344f06e190efcb192a3b6ef9905aea352 PiperOrigin-RevId: 369687409
katre
pushed a commit
to katre/bazel
that referenced
this pull request
Jul 13, 2021
When java_test rules are run with coverage, then Bazel writes a file containing the runtime classpath using root-relative paths. The collect_coverage.sh script then processes that file to generate another file containing absolute paths by prefixing each of the paths with the runfiles directory and the workspace name. It then runs the singlejar tool on this list of jar files to merge them into a single file. This happens to work for all jar files in the main repository because the runfiles path is the runfiles directory plus the workspace name plus the root-relative path. However, it is broken if some of the jar files come from an external repository. It appears that singlejar errors out on the first such jar file, generating a partial output jar file without a central directory. The error is swallowed by the `collect_coverage.sh` script. Presence of coverage results may thus depend on the order of classpath entries. In order to fix this, we change the runtime classpath file to contain runfiles-relative paths, and the coverage script to prefix them with the runfiles directory. Note that we reuse SourceManifestAction here, i.e., the identical code that is also responsible for generating the runfiles directory in the first place. This is the only reliable way to get the correct paths into the test action. There are more places that use `LazyWritePathsFileAction` and I suspect that all of them are broken: - persistent test runner support in BazelJavaSemantics (AFAIK, this doesn't work in Bazel anyway) - coverage w/ metadata jars in BazelJavaSemantics (not used by JavaBinary) - coverage source list file ('-paths-for-coverage.txt'); this probably results in non-functioning coverage for *source files* in external repositories Fixes bazelbuild#13376. Change-Id: Ie9bcc92344f06e190efcb192a3b6ef9905aea352 Closes bazelbuild#13377. Change-Id: Ie9bcc92344f06e190efcb192a3b6ef9905aea352 PiperOrigin-RevId: 369687409
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When java_test rules are run with coverage, then Bazel writes a file
containing the runtime classpath using root-relative paths.
The collect_coverage.sh script then processes that file to generate
another file containing absolute paths by prefixing each of the paths
with the runfiles directory and the workspace name. It then runs the
singlejar tool on this list of jar files to merge them into a single
file.
This happens to work for all jar files in the main repository because
the runfiles path is the runfiles directory plus the workspace name
plus the root-relative path.
However, it is broken if some of the jar files come from an external
repository. It appears that singlejar errors out on the first such jar
file, generating a partial output jar file without a central directory.
The error is swallowed by the
collect_coverage.sh
script. Presence ofcoverage results may thus depend on the order of classpath entries.
In order to fix this, we change the runtime classpath file to contain
runfiles-relative paths, and the coverage script to prefix them with
the runfiles directory.
Note that we reuse SourceManifestAction here, i.e., the identical code
that is also responsible for generating the runfiles directory in the
first place. This is the only reliable way to get the correct paths
into the test action.
There are more places that use
LazyWritePathsFileAction
and I suspectthat all of them are broken:
doesn't work in Bazel anyway)
JavaBinary)
results in non-functioning coverage for source files in external
repositories
Fixes #13376.
Change-Id: Ie9bcc92344f06e190efcb192a3b6ef9905aea352