Skip to content

Commit

Permalink
Fix Incompatible Target Skipping for test args
Browse files Browse the repository at this point in the history
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 bazelbuild#13219.

PiperOrigin-RevId: 363233765
  • Loading branch information
philsc authored and Copybara-Service committed Mar 16, 2021
1 parent 586de95 commit 807f2a1
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 2 deletions.
Expand Up @@ -453,6 +453,22 @@ 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 <a
* href="https://docs.bazel.build/versions/master/platforms.html#skipping-incompatible-targets">Incompatible
* Target Skipping</a>.
*/
public static RunfilesSupport withExecutableButNoArgs(
RuleContext ruleContext, Runfiles runfiles, Artifact executable) {
return RunfilesSupport.create(
ruleContext,
executable,
runfiles,
CommandLine.EMPTY,
computeActionEnvironment(ruleContext));
}

private static CommandLine computeArgs(RuleContext ruleContext, CommandLine additionalArgs) {
if (!ruleContext.getRule().isAttrDefined("args", Type.STRING_LIST)) {
// Some non-_binary rules create RunfilesSupport instances; it is fine to not have an args
Expand Down
Expand Up @@ -1024,7 +1024,7 @@ private static ConfiguredTarget createIncompatibleConfiguredTarget(
if (!outputArtifacts.isEmpty()) {
Artifact executable = outputArtifacts.get(0);
RunfilesSupport runfilesSupport =
RunfilesSupport.withExecutable(ruleContext, runfiles, executable);
RunfilesSupport.withExecutableButNoArgs(ruleContext, runfiles, executable);
builder.setRunfilesSupport(runfilesSupport, executable);

ruleContext.registerAction(
Expand Down
22 changes: 21 additions & 1 deletion src/test/shell/integration/target_compatible_with_test.sh
Expand Up @@ -378,7 +378,10 @@ function atest_build_event_protocol() {
# incompatible targets are themselves deemed incompatible and should therefore
# not be built.
function test_non_top_level_skipping() {
cat >> target_skipping/BUILD <<EOF
touch target_skipping/foo_test.sh
chmod +x target_skipping/foo_test.sh
cat >> target_skipping/BUILD <<'EOF'
genrule(
name = "genrule_foo1",
target_compatible_with = [":foo1"],
Expand All @@ -391,6 +394,15 @@ sh_binary(
srcs = ["foo1.sh"],
target_compatible_with = [":foo2"],
)
# Make sure that using an incompatible target in Make variable substitution
# doesn't produce an unexpected error.
sh_test(
name = "foo_test",
srcs = ["foo_test.sh"],
data = [":some_foo3_target"],
args = ["$(location :some_foo3_target)"],
)
EOF
cd target_skipping || fail "couldn't cd into workspace"
Expand All @@ -402,6 +414,14 @@ EOF
//target_skipping:sh_foo2 &> "${TEST_log}" && fail "Bazel passed unexpectedly."
expect_log 'ERROR: Target //target_skipping:sh_foo2 is incompatible and cannot be built, but was explicitly requested'
expect_log 'FAILED: Build did NOT complete successfully'
bazel build \
--show_result=10 \
--host_platform=@//target_skipping:foo2_bar1_platform \
--platforms=@//target_skipping:foo2_bar1_platform \
//target_skipping:foo_test &> "${TEST_log}" && fail "Bazel passed unexpectedly."
expect_log 'ERROR: Target //target_skipping:foo_test is incompatible and cannot be built, but was explicitly requested'
expect_log 'FAILED: Build did NOT complete successfully'
}
# Validate that targets are skipped when the implementation is in Starlark
Expand Down

0 comments on commit 807f2a1

Please sign in to comment.