Fix Incompatible Target Skipping for test args

philsc · copybara-github · commit 807f2a1929e2 · 2021-03-16T11:46:40.000-07:00
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
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
@@ -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
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/RuleContextConstraintSemantics.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/RuleContextConstraintSemantics.java
@@ -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(
diff --git a/src/test/shell/integration/target_compatible_with_test.sh b/src/test/shell/integration/target_compatible_with_test.sh
@@ -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"],
@@ -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"
@@ -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
