Prevent template_ctx.run from using map_directory outputs as inputs.

This has the following implications:
- Internal actions (generated by `template_ctx.run()`) cannot take both subtrees and their parent tree artifacts as inputs, which might be problematic for remote execution as artifacts are "streamed" (potentially causing the same files from the subtree to be added twice).
- Ensures encapsulation of the action graph generated by `map_directory()`. For example, internal actions generated within `map_directory(implementation)` themselves depending on the output directories of the same `map_directory()` breaks encapsulation.

PiperOrigin-RevId: 884538235
Change-Id: If6c8f4990dea353460de9bdde5f5abca61f8b5a8

Author: zhengwei143

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkTemplateContext.java

@@ -27,6 +27,7 @@
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.collect.nestedset.Depset;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.starlarkbuildapi.FileApi;
 import com.google.devtools.build.lib.starlarkbuildapi.StarlarkTemplateContextApi;
 import com.google.devtools.build.lib.supplier.InterruptibleSupplier;
@@ -88,15 +89,29 @@ public void run(
 
     StarlarkActionFactory.buildCommandLine(builder, arguments, repoMappingSupplier);
 
+    List<Artifact> inputArtifacts;
     switch (inputs) {
-      case Sequence<?> sequence ->
-          builder.addInputs(Sequence.cast(inputs, Artifact.class, "inputs"));
-      case Depset depset ->
-          builder.addTransitiveInputs(Depset.cast(depset, Artifact.class, "inputs"));
+      case Sequence<?> sequence -> {
+        inputArtifacts = Sequence.cast(inputs, Artifact.class, "inputs");
+        builder.addInputs(inputArtifacts);
+      }
+      case Depset depset -> {
+        NestedSet<Artifact> inputNestedSet = Depset.cast(depset, Artifact.class, "inputs");
+        inputArtifacts = inputNestedSet.toList();
+        builder.addTransitiveInputs(inputNestedSet);
+      }
       default -> {
         throw Starlark.errorf("Expected a list or depset but got %s", Starlark.type(inputs));
       }
     }
+
+    for (Artifact input : inputArtifacts) {
+      if (outputDirectories.contains(input)) {
+        throw Starlark.errorf(
+            "Output directory %s cannot be used as an input to template_ctx.run()", input);
+      }
+    }
+
     switch (executableUnchecked) {
       case Artifact executable -> builder.setExecutable(executable);
       case FilesToRunProvider filesToRun -> builder.setExecutable(filesToRun);

src/test/java/com/google/devtools/build/lib/starlark/StarlarkMapActionTemplateTest.java

@@ -1012,6 +1012,65 @@ def rule_impl(ctx):
             PathFragment.create("subdir_1/f2"));
   }
 
+  @Test
+  public void internalActionsCannotTakeTopLevelDirectoriesAsInputs() throws Exception {
+    SkyframeExecutorTestHelper.process(getSkyframeExecutor());
+    write(
+        "test/rule_def.bzl",
+        """
+        load(":helpers.bzl", "create_seed_dir", "create_seed_subdir")
+
+        def combined_impl(
+                template_ctx,
+                input_directories,
+                output_directories,
+                tools,
+                **kwargs):
+            output_dir1 = output_directories["output_dir1"]
+            output_dir2 = output_directories["output_dir2"]
+            output_file = template_ctx.declare_file(
+                "combined.txt",
+                directory = output_dir2,
+            )
+            template_ctx.run(
+                inputs = [output_dir1],
+                outputs = [output_file],
+                executable = tools["cat_tool"],
+                # Args don't matter, it should fail.
+                arguments = [template_ctx.args()],
+            )
+
+        def rule_impl(ctx):
+            input_dir = create_seed_dir(ctx, "input_dir", 1, 1)
+            output_dir1 = ctx.actions.declare_directory(ctx.attr.name + "_output_dir1")
+            output_dir2 = ctx.actions.declare_directory(ctx.attr.name + "_output_dir2")
+            ctx.actions.map_directory(
+                implementation = combined_impl,
+                input_directories = {
+                    "input_dir": input_dir,
+                },
+                output_directories = {
+                    "output_dir1": output_dir1,
+                    "output_dir2": output_dir2,
+                },
+                tools = {
+                    "cat_tool": ctx.attr.cat_tool.files_to_run,
+                    "gen_subdir_tool": ctx.attr.gen_subdir_tool.files_to_run,
+                },
+            )
+            return [DefaultInfo(files = depset([output_dir2]))]
+        """);
+    RecordingOutErr recordingOutErr = new RecordingOutErr();
+    this.outErr = recordingOutErr;
+    assertThrows(BuildFailedException.class, () -> buildTarget("//test:target"));
+    assertThat(recordingOutErr.errAsLatin1())
+        .containsMatch(
+            "ERROR: .*/test/BUILD:2:8: Expanding \\[File:\\[.*\\]test/target_input_dir\\] into"
+                + " actions\\. failed: Output directory"
+                + " File:\\[.*\\]test/target_output_dir1"
+                + " cannot be used as an input to template_ctx\\.run\\(\\)");
+  }
+
   @Test
   public void actionConflicts_declaredFileWithPrefixOfSubdir() throws Exception {
     SkyframeExecutorTestHelper.process(getSkyframeExecutor());