Replace crashing behaviour during execution with an error during anal…

…ysis if a middleman artifact is added to runfiles from Starlark PiperOrigin-RevId: 466619921 Change-Id: I1c2cc0349267bbb6b8644ad893f44eb47ce06b20
aiuto · Aug 10, 2022 · 04e8b63 · 04e8b63
1 parent db64be5
commit 04e8b63
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 1 deletion.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java
@@ -956,7 +956,9 @@ public Runfiles runfiles(
       builder.addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES);
     }
     if (!files.isEmpty()) {
-      builder.addArtifacts(Sequence.cast(files, Artifact.class, "files"));
+      Sequence<Artifact> artifacts = Sequence.cast(files, Artifact.class, "files");
+      checkForMiddlemanArtifacts(artifacts);
+      builder.addArtifacts(artifacts);
     }
     if (transitiveFiles != Starlark.NONE) {
       NestedSet<Artifact> transitiveArtifacts =
@@ -968,6 +970,7 @@ public Runfiles runfiles(
             "order '%s' is invalid for transitive_files",
             transitiveArtifacts.getOrder().getStarlarkName());
       }
+      checkForMiddlemanArtifacts(transitiveArtifacts.toList());
       builder.addTransitiveArtifacts(transitiveArtifacts);
     }
     if (isDepset(symlinks)) {
@@ -996,6 +999,16 @@ public Runfiles runfiles(
     return runfiles;
   }
 
+  private void checkForMiddlemanArtifacts(Iterable<Artifact> artifacts) throws EvalException {
+    for (Artifact artifact : artifacts) {
+      if (artifact.isMiddlemanArtifact()) {
+        throw Starlark.errorf(
+            "Middleman artifacts are forbidden here. Artifact: %s, owner: %s",
+            artifact, artifact.getOwner());
+      }
+    }
+  }
+
   private static boolean isNonEmptyDict(Object o) {
     return o instanceof Dict && !((Dict<?, ?>) o).isEmpty();
   }

diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java
@@ -2084,6 +2084,50 @@ public void runfiles_incompatibleTransitiveFilesOrder() throws Exception {
     assertContainsEvent("Error in runfiles: order 'preorder' is invalid for transitive_files");
   }
 
+  @Test
+  public void runfiles_failOnMiddlemanInFiles() throws Exception {
+    scratch.file(
+        "test/rule.bzl",
+        "def _impl(ctx):",
+        "  internal_output_group = ctx.attr.bin[OutputGroupInfo]._hidden_top_level_INTERNAL_",
+        "  ctx.runfiles(files = internal_output_group.to_list())",
+        "bad_runfiles = rule(",
+        "  implementation = _impl,",
+        "  attrs = {'bin' : attr.label()}",
+        ")");
+    scratch.file(
+        "test/BUILD",
+        "load(':rule.bzl', 'bad_runfiles')",
+        "cc_binary(name = 'bin')",
+        "bad_runfiles(name = 'test', bin = ':bin')");
+
+    reporter.removeHandler(failFastHandler); // Error expected.
+    assertThat(getConfiguredTarget("//test:test")).isNull();
+    assertContainsEvent("Error in runfiles: Middleman artifacts are forbidden here");
+  }
+
+  @Test
+  public void runfiles_failOnMiddlemanInTransitiveFiles() throws Exception {
+    scratch.file(
+        "test/rule.bzl",
+        "def _impl(ctx):",
+        "  internal_output_group = ctx.attr.bin[OutputGroupInfo]._hidden_top_level_INTERNAL_",
+        "  ctx.runfiles(transitive_files = internal_output_group)",
+        "bad_runfiles = rule(",
+        "  implementation = _impl,",
+        "  attrs = {'bin' : attr.label()}",
+        ")");
+    scratch.file(
+        "test/BUILD",
+        "load(':rule.bzl', 'bad_runfiles')",
+        "cc_binary(name = 'bin')",
+        "bad_runfiles(name = 'test', bin = ':bin')");
+
+    reporter.removeHandler(failFastHandler); // Error expected.
+    assertThat(getConfiguredTarget("//test:test")).isNull();
+    assertContainsEvent("Error in runfiles: Middleman artifacts are forbidden here");
+  }
+
   @Test
   public void testExternalShortPath() throws Exception {
     scratch.file("/bar/WORKSPACE");