Fix Incompatible Target Skipping for test args #13219
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
|
@gregestren , FYI. |
gregestren
reviewed
Mar 15, 2021
| @@ -453,6 +453,20 @@ public static RunfilesSupport withExecutable( | |||
| computeActionEnvironment(ruleContext)); | |||
| } | |||
|
|
|||
| /** | |||
| * Creates and returns a {@link RunfilesSupport} object for the given rule and executable. This | |||
| * version discards all arguments. Only use this for Incompatible Target Skipping. | |||
There was a problem hiding this comment.
Can you link this more precisely to a specific file or source javadoc?
This comment comes from the hard-learned lesson that even specific concepts can be fuzzy to devs who haven't focused specifically on them. Precise links are always great for that.
There was a problem hiding this comment.
I can't verify that the link renders correctly, but I added a link to https://docs.bazel.build/versions/master/platforms.html#skipping-incompatible-targets
Would you prefer a link to the site/docs/platforms.md file instead?
There was a problem hiding this comment.
That's fine, thanks. Just anything to connect the breadcrumbs.
Before the fix the test in this patch would error out with the
following message:
ERROR: ...: in args attribute of sh_test rule //target_skipping:foo_test: label '//target_skipping:some_foo3_target' in $(location) expression expands to no files
The problem was that the `RunfilesSupport` class for the
`RunfilesProvider` instance was computing the arguments as the
incompatible target was being constructed. That meant that the
arguments get evaluated and it would try to resolve things like
`$(location :incompatible_dep)`. That can't really succeed so bazel
would throw an error.
This patch makes it so arguments are ignored when constructing an
incompatible target.
philsc
force-pushed
the
unreviewed/incompatible_in_args-publish
branch
from
3d0f99d
to
ea86bf5
Compare
gregestren
approved these changes
Mar 16, 2021
philwo
pushed a commit
that referenced
this pull request
Apr 19, 2021
Before the fix, the test in this patch would error out with the
following message:
ERROR: ...: in args attribute of sh_test rule //target_skipping:foo_test: label '//target_skipping:some_foo3_target' in $(location) expression expands to no files
The problem was that the `RunfilesSupport` class for the
`RunfilesProvider` instance was computing the arguments as the
incompatible target was being constructed. That meant that the
arguments get evaluated and it would try to resolve things like
`$(location :incompatible_dep)`. That can't really succeed so bazel
would throw an error.
This patch makes it so arguments are ignored when constructing an
incompatible target.
Closes #13219.
PiperOrigin-RevId: 363233765
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.
Before the fix, the test in this patch would error out with the
following message:
The problem was that the
RunfilesSupportclass for theRunfilesProviderinstance was computing the arguments as theincompatible target was being constructed. That meant that the
arguments get evaluated and it would try to resolve things like
$(location :incompatible_dep). That can't really succeed so bazelwould throw an error.
This patch makes it so arguments are ignored when constructing an
incompatible target.