Always include a TreeArtifactValue in the ActionInputMap if at least one of its TreeFileArtifacts is an action input.

Removes an edge case from AbstractActionInputPrefetcher.

PiperOrigin-RevId: 762350969
Change-Id: I9cbd4ac10aac19a1e997643da27632e83fb694fc

Author: tjgq

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

@@ -378,9 +378,9 @@ private ListenableFuture<Void> prefetchFile(
 
       PathFragment execPath = input.getExecPath();
 
-      // Metadata may legitimately be missing, e.g. if this is an optional test output.
       FileArtifactValue metadata = metadataSupplier.getMetadata(input);
       if (metadata == null || !canDownloadFile(execRoot.getRelative(execPath), metadata)) {
+        // Metadata may legitimately be missing, e.g. if this is an optional spawn output.
         return immediateVoidFuture();
       }
 
@@ -441,19 +441,12 @@ private PathFragment maybeGetTreeRoot(ActionInput input, MetadataSupplier metada
     }
     SpecialArtifact treeArtifact = treeFile.getParent();
     FileArtifactValue treeMetadata = metadataSupplier.getMetadata(treeArtifact);
-    if (treeMetadata == null) {
-      // There are two cases where tree metadata is legitimately not available:
-      // (1) If the file is the output of an action expanded from an action template. In this
-      //     case, the symlink optimization is intentionally not supported.
-      // (2) If the file is part of an input fileset. In this case, a symlink has already been
-      //     created, but we're currently unable to prefetch the file(s) it points to.
-      // TODO: b/401575099 - Treating fileset more like runfiles could make the tree metadata
-      //  available for case (2).
-      return null;
-    }
-    PathFragment resolvedPath = treeMetadata.getResolvedPath();
-    if (resolvedPath != null) {
-      return resolvedPath.relativeTo(execRoot.asFragment());
+    // Metadata may legitimately be missing, e.g. if this is an optional spawn output.
+    if (treeMetadata != null) {
+      PathFragment resolvedPath = treeMetadata.getResolvedPath();
+      if (resolvedPath != null) {
+        return resolvedPath.relativeTo(execRoot.asFragment());
+      }
     }
     return treeArtifact.getExecPath();
   }
@@ -468,24 +461,15 @@ private PathFragment maybeGetTreeRoot(ActionInput input, MetadataSupplier metada
    */
   @Nullable
   private Symlink maybeGetSymlink(
-      ActionInput input,
-      FileArtifactValue metadata,
-      MetadataSupplier metadataSupplier)
+      ActionInput input, FileArtifactValue metadata, MetadataSupplier metadataSupplier)
       throws IOException, InterruptedException {
     if (input instanceof TreeFileArtifact treeFile) {
       SpecialArtifact treeArtifact = treeFile.getParent();
       FileArtifactValue treeMetadata = metadataSupplier.getMetadata(treeArtifact);
-      if (treeMetadata == null) {
-        // There are two cases where tree metadata is legitimately not available:
-        // (1) If the file is the output of an action expanded from an action template. In this
-        //     case, the symlink optimization is intentionally not supported.
-        // (2) If the file is part of an input fileset. In this case, a symlink has already been
-        //     created, but we're currently unable to prefetch the file(s) it points to.
-        // TODO: b/401575099 - Treating fileset more like runfiles could make the tree metadata
-        //  available for case (2).
-        return null;
+      // Metadata may legitimately be missing, e.g. if this is an optional spawn output.
+      if (treeMetadata != null) {
+        return maybeGetSymlink(treeArtifact, treeMetadata, metadataSupplier);
       }
-      return maybeGetSymlink(treeArtifact, treeMetadata, metadataSupplier);
     }
     PathFragment execPath = input.getExecPath();
     PathFragment resolvedExecPath = execPath;

src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java

@@ -16,6 +16,7 @@
 import com.google.devtools.build.lib.actions.ActionInputMap;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
+import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
 import com.google.devtools.build.lib.actions.FileArtifactValue;
 import com.google.devtools.build.lib.actions.FilesetOutputTree;
 import com.google.devtools.build.lib.actions.RunfilesArtifactValue;
@@ -42,12 +43,14 @@ static void addToMap(
         consumer.accumulate(treeArtifactValue);
       }
       case ActionExecutionValue actionExecutionValue -> {
-        // Actions resulting from the expansion of an ActionTemplate consume only one of the files
-        // in a tree artifact. However, the input prefetcher and the Linux sandbox require access to
-        // the tree metadata to determine the prefetch location of a tree artifact materialized as a
-        // symlink to another (cf. TreeArtifactValue#getResolvedPath()).
-        if (key.isChildOfDeclaredDirectory()) {
-          SpecialArtifact treeArtifact = key.getParent();
+        if (key instanceof TreeFileArtifact treeFileArtifact) {
+          // Since input expansion hasn't yet occurred, it must be the case that the current action
+          // was generated by an action template (as otherwise it would only be allowed to depend
+          // on a tree artifact in its entirety). In this case insert the full tree metadata into
+          // the map, as some components need it to operate properly (for example, if the tree
+          // artifact must be materialized as a symlink to another artifact, the prefetcher needs it
+          // to materialize the artifact correctly).
+          SpecialArtifact treeArtifact = treeFileArtifact.getParent();
           TreeArtifactValue treeArtifactValue =
               actionExecutionValue.getTreeArtifactValue(treeArtifact);
           expandTreeArtifactAndPopulateArtifactData(