Replace explodeDirectory with visitTree for an on-line visitation of tree artifact contents.

Use this from ActionMetadataHandler to merge two tree traversals into one. We had been performing a traversal in setTreeReadOnlyAndExecutable, then repeating it in explodeDirectory. Furthermore, since the traversal uses readdir which returns type information, we can skip the stat that checks whether a path is a file before calling chmod.

RELNOTES: None.
PiperOrigin-RevId: 321215438
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
index 03df389..232d3f2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
@@ -30,17 +30,14 @@
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
 import com.google.devtools.build.lib.vfs.Dirent;
-import com.google.devtools.build.lib.vfs.Dirent.Type;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Symlinks;
 import com.google.devtools.build.skyframe.SkyValue;
 import java.io.IOException;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import javax.annotation.Nullable;
@@ -244,64 +241,90 @@
     };
   }
 
-  private static void explodeDirectory(
-      Path treeArtifactPath,
-      PathFragment pathToExplode,
-      ImmutableSet.Builder<PathFragment> valuesBuilder)
+  /** Visitor for use in {@link #visitTree}. */
+  @FunctionalInterface
+  public interface TreeArtifactVisitor {
+    /**
+     * Called for every directory entry encountered during tree traversal.
+     *
+     * <p>Symlinks are not followed during traversal and are simply reported as {@link
+     * Dirent.Type#SYMLINK} regardless of whether they point to a file, directory, or are dangling.
+     *
+     * <p>{@code type} is guaranteed never to be {@link Dirent.Type#UNKNOWN}, since if this type is
+     * encountered, {@link IOException} is immediately thrown without invoking the visitor.
+     *
+     * <p>If the implementation throws {@link IOException}, traversal is immediately halted and the
+     * exception is propagated.
+     */
+    void visit(PathFragment parentRelativePath, Dirent.Type type) throws IOException;
+  }
+
+  /**
+   * Recursively visits all descendants under a directory.
+   *
+   * <p>{@link TreeArtifactVisitor#visit} is invoked on {@code visitor} for each directory, file,
+   * and symlink under the given {@code parentDir}.
+   *
+   * <p>This method is intended to provide uniform semantics for constructing a tree artifact,
+   * including special logic that validates directory entries. Invalid directory entries include a
+   * symlink that traverses outside of the tree artifact and any entry of {@link
+   * Dirent.Type#UNKNOWN}, such as a named pipe.
+   *
+   * @throws IOException if there is any problem reading or validating outputs under the given tree
+   *     artifact directory, or if {@link TreeArtifactVisitor#visit} throws {@link IOException}
+   */
+  public static void visitTree(Path parentDir, TreeArtifactVisitor visitor) throws IOException {
+    visitTree(parentDir, PathFragment.EMPTY_FRAGMENT, Preconditions.checkNotNull(visitor));
+  }
+
+  private static void visitTree(Path parentDir, PathFragment subdir, TreeArtifactVisitor visitor)
       throws IOException {
-    Path dir = treeArtifactPath.getRelative(pathToExplode);
-    Collection<Dirent> dirents = dir.readdir(Symlinks.NOFOLLOW);
-    for (Dirent dirent : dirents) {
-      PathFragment canonicalSubpathFragment = pathToExplode.getChild(dirent.getName());
-      if (dirent.getType() == Type.DIRECTORY) {
-        explodeDirectory(treeArtifactPath, canonicalSubpathFragment, valuesBuilder);
-      } else if (dirent.getType() == Type.SYMLINK) {
-        Path subpath = dir.getRelative(dirent.getName());
-        PathFragment linkTarget = subpath.readSymbolicLinkUnchecked();
-        valuesBuilder.add(canonicalSubpathFragment);
-        if (linkTarget.isAbsolute()) {
-          // We tolerate absolute symlinks here. They will probably be dangling if any downstream
-          // consumer tries to read them, but let that be downstream's problem.
-          continue;
-        }
-        // We visit each path segment of the link target to catch any path traversal outside of the
-        // TreeArtifact root directory. For example, for TreeArtifact a/b/c, it is possible to have
-        // a symlink, a/b/c/sym_link that points to ../outside_dir/../c/link_target. Although this
-        // symlink points to a file under the TreeArtifact, the link target traverses outside of the
-        // TreeArtifact into a/b/outside_dir.
-        PathFragment intermediatePath = canonicalSubpathFragment.getParentDirectory();
-        for (String pathSegment : linkTarget.getSegments()) {
-          intermediatePath = intermediatePath.getRelative(pathSegment);
-          if (intermediatePath.containsUplevelReferences()) {
-            String errorMessage =
-                String.format(
-                    "A TreeArtifact may not contain relative symlinks whose target paths traverse "
-                        + "outside of the TreeArtifact, found %s pointing to %s.",
-                    subpath, linkTarget);
-            throw new IOException(errorMessage);
-          }
-        }
-      } else if (dirent.getType() == Type.FILE) {
-        valuesBuilder.add(canonicalSubpathFragment);
-      } else {
-        // We shouldn't ever reach here.
-        Path subpath = dir.getRelative(dirent.getName());
-        throw new IllegalStateException("Could not determine type of file " + subpath);
+    for (Dirent dirent : parentDir.getRelative(subdir).readdir(Symlinks.NOFOLLOW)) {
+      PathFragment parentRelativePath = subdir.getChild(dirent.getName());
+      Dirent.Type type = dirent.getType();
+
+      if (type == Dirent.Type.UNKNOWN) {
+        throw new IOException(
+            "Could not determine type of file for " + parentRelativePath + " under " + parentDir);
+      }
+
+      if (type == Dirent.Type.SYMLINK) {
+        checkSymlink(subdir, parentDir.getRelative(parentRelativePath));
+      }
+
+      visitor.visit(parentRelativePath, type);
+
+      if (type == Dirent.Type.DIRECTORY) {
+        visitTree(parentDir, parentRelativePath, visitor);
       }
     }
   }
 
-  /**
-   * Recursively get all child files in a directory (excluding child directories themselves, but
-   * including all files in them).
-   *
-   * @throws IOException if there is any problem reading or validating outputs under the given tree
-   *     artifact.
-   */
-  public static Set<PathFragment> explodeDirectory(Path treeArtifactPath) throws IOException {
-    ImmutableSet.Builder<PathFragment> explodedDirectory = ImmutableSet.builder();
-    explodeDirectory(treeArtifactPath, PathFragment.EMPTY_FRAGMENT, explodedDirectory);
-    return explodedDirectory.build();
+  private static void checkSymlink(PathFragment subDir, Path path) throws IOException {
+    PathFragment linkTarget = path.readSymbolicLinkUnchecked();
+    if (linkTarget.isAbsolute()) {
+      // We tolerate absolute symlinks here. They will probably be dangling if any downstream
+      // consumer tries to read them, but let that be downstream's problem.
+      return;
+    }
+
+    // Visit each path segment of the link target to catch any path traversal outside of the
+    // TreeArtifact root directory. For example, for TreeArtifact a/b/c, it is possible to have a
+    // symlink, a/b/c/sym_link that points to ../outside_dir/../c/link_target. Although this symlink
+    // points to a file under the TreeArtifact, the link target traverses outside of the
+    // TreeArtifact into a/b/outside_dir.
+    PathFragment intermediatePath = subDir;
+    for (String pathSegment : linkTarget.getSegments()) {
+      intermediatePath = intermediatePath.getRelative(pathSegment);
+      if (intermediatePath.containsUplevelReferences()) {
+        String errorMessage =
+            String.format(
+                "A TreeArtifact may not contain relative symlinks whose target paths traverse "
+                    + "outside of the TreeArtifact, found %s pointing to %s.",
+                path, linkTarget);
+        throw new IOException(errorMessage);
+      }
+    }
   }
 
   private static final class BasicMultiBuilder implements MultiBuilder {