Fix crash on remote-only action template outputs

Fixes the following type of crash when the outputs of an action template are built with BwoB without being downloaded:

```
java.lang.RuntimeException: Unrecoverable error while evaluating node 'TargetCompletionKey{topLevelArtifactContext=com.google.devtools.build.lib.analysis.TopLevelArtifactContext@90504ed, actionLookupKey=ConfiguredTargetKey{label=//a:main, config=BuildConfigurationKey[ace1802e9ffc3691a893a40a89ff0d55ab85b5bb1a265fd1bb830b1af3f3a0e0]}, willTest=false}' (requested by nodes 'BuildDriverKey of ActionLookupKey: ConfiguredTargetKey{label=//a:main, config=BuildConfigurationKey[4e639ef1ee6b55f3683fb1281a60a624b3a6eb2ef19705d98d7e875872945deb]}')
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:551)
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:435)
	at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.compute(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.compute(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinTask$InterruptibleTask.exec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
Caused by: java.lang.IllegalStateException: generating action for artifact File:[[<execution_root>]bazel-out/darwin_arm64-fastbuild/bin]a/_objs/main/dir is not an ActionExecutionMetadata
	at com.google.common.base.Preconditions.checkState(Preconditions.java:603)
	at com.google.devtools.build.lib.remote.RemoteImportantOutputHandler.getGeneratingAction(RemoteImportantOutputHandler.java:191)
	at com.google.devtools.build.lib.remote.RemoteImportantOutputHandler.downloadArtifact(RemoteImportantOutputHandler.java:164)
	at com.google.devtools.build.lib.remote.RemoteImportantOutputHandler.ensureToplevelArtifacts(RemoteImportantOutputHandler.java:117)
	at com.google.devtools.build.lib.remote.RemoteImportantOutputHandler.processOutputsAndGetLostArtifacts(RemoteImportantOutputHandler.java:72)
	at com.google.devtools.build.lib.skyframe.CompletionFunction.informImportantOutputHandler(CompletionFunction.java:431)
	at com.google.devtools.build.lib.skyframe.CompletionFunction.compute(CompletionFunction.java:327)
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:471)
```

Closes #26143.

PiperOrigin-RevId: 776209213
Change-Id: Id17372a202cab5b57bc9fccfdba53291fd08a91b

Author: fmeum

src/main/java/com/google/devtools/build/lib/remote/RemoteImportantOutputHandler.java

@@ -18,11 +18,11 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
-import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
 import com.google.devtools.build.lib.actions.Actions;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
+import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
 import com.google.devtools.build.lib.actions.FileArtifactValue;
 import com.google.devtools.build.lib.actions.ImportantOutputHandler;
 import com.google.devtools.build.lib.actions.InputMetadataProvider;
@@ -152,7 +152,7 @@ private void downloadArtifact(
         return;
       }
 
-      var filesToDownload = new ArrayList<ActionInput>(treeArtifactValue.getChildren().size());
+      var filesToDownload = new ArrayList<TreeFileArtifact>(treeArtifactValue.getChildren().size());
       for (var entry : treeArtifactValue.getChildValues().entrySet()) {
         if (remoteOutputChecker.shouldDownloadOutput(entry.getKey(), entry.getValue())) {
           filesToDownload.add(entry.getKey());
@@ -161,7 +161,9 @@ private void downloadArtifact(
       if (!filesToDownload.isEmpty()) {
         futures.add(
             actionInputPrefetcher.prefetchFiles(
-                getGeneratingAction(derivedArtifact),
+                // derivedArtifact's generating action may be an action template, which doesn't
+                // implement the required ActionExecutionMetadata.
+                getGeneratingAction(filesToDownload.getFirst()),
                 filesToDownload,
                 metadataProvider,
                 ActionInputPrefetcher.Priority.LOW,
@@ -190,8 +192,9 @@ private ActionExecutionMetadata getGeneratingAction(DerivedArtifact artifact)
     var action = Actions.getGeneratingAction(graph, artifact);
     Preconditions.checkState(
         action instanceof ActionExecutionMetadata,
-        "generating action for artifact %s is not an ActionExecutionMetadata",
-        artifact);
+        "generating action for artifact %s is not an ActionExecutionMetadata, but %s",
+        artifact,
+        action != null ? action.getClass() : null);
     return (ActionExecutionMetadata) action;
   }
 }

src/test/shell/bazel/remote/build_without_the_bytes_test.sh

@@ -571,6 +571,57 @@ EOF
   fi
 }
 
+function test_download_minimal_templated_tree_artifact_downloaded_later() {
+  mkdir -p a
+
+  # We need the top-level output to be a tree artifact generated by a template
+  # action. This is one way to do that: generate a tree artifact of C++ source
+  # files, and then compile them with a cc_library / cc_binary rule.
+  #
+  # The default top-level output of a cc_binary is the final binary, which is
+  # not what we want. Instead, we use --output_groups=compilation_outputs to
+  # fetch the .o files as the top-level outputs.
+
+  cat > a/gentree.bzl <<'EOF'
+def _gentree(ctx):
+    out = ctx.actions.declare_directory("dir.cc")
+    ctx.actions.run_shell(
+        outputs = [out],
+        command = "echo 'int main(){return 0;}' > %s/foo.cc" % out.path,
+    )
+    return DefaultInfo(files = depset([out]))
+
+gentree = rule(implementation = _gentree)
+EOF
+
+  cat > a/BUILD <<'EOF'
+load(":gentree.bzl", "gentree")
+gentree(name = "tree")
+cc_binary(name = "main", srcs = [":tree"])
+EOF
+
+  bazel build \
+    --remote_executor=grpc://localhost:${worker_port} \
+    --remote_download_minimal \
+    --output_groups=compilation_outputs \
+    //a:main || fail "Failed to build //a:main"
+
+  bazel build \
+    --remote_executor=grpc://localhost:${worker_port} \
+    --remote_download_toplevel \
+    --output_groups=compilation_outputs \
+    //a:main || fail "Failed to build //a:main"
+
+  if [[ "$PLATFORM" == "darwin" ]]; then
+    expected_object_file=bazel-bin/a/_objs/main/dir/foo.o
+  else
+    expected_object_file=bazel-bin/a/_pic_objs/main/dir/foo.pic.o
+  fi
+  if ! [[ -f "$expected_object_file" ]]; then
+    fail "Expected toplevel output $expected_object_file to be downloaded"
+  fi
+}
+
 function test_downloads_toplevel_src_runfiles() {
   # Test that using --remote_download_toplevel with a non-generated (source)
   # runfile dependency works.