remote: add support for directory inputs in runfiles

Add support for tree artifacts (ctx.action.declare_directory(...)) in
runfiles. Before this change we would throw away the information
about the files inside a tree artifact before executing an action.
That's fine for local execution where the sandbox just copies/symlinks
a directory and doesn't care much what's inside. However, in remote
execution we actually need to upload each individual file and so
we need to be aware of all individual files not just directories.

This change makes it so that this information is made available to a
SpawnRunner via the SpawnInputExpander.

RELNOTES: None
PiperOrigin-RevId: 207091668

1 files changed, 32 insertions, 19 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
index 467391af18..484eda71c3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
@@ -122,8 +122,8 @@ class ArtifactFunction implements SkyFunction {
               actionLookupValue,
               actionIndex);
       if (isAggregatingValue(action)) {
-        return createAggregatingValue(artifact, action,
-            actionValue.getArtifactValue(artifact), env);
+        return createAggregatingValue(
+            artifact, action, actionValue.getArtifactValue(artifact), env);
       }
     }
     return createSimpleFileArtifactValue(artifact, actionValue);
@@ -243,8 +243,8 @@ class ArtifactFunction implements SkyFunction {
   private static MissingInputFileException makeMissingInputFileException(
       Artifact artifact, boolean mandatory, Exception failure, EventHandler reporter) {
     String extraMsg = (failure == null) ? "" : (":" + failure.getMessage());
-    MissingInputFileException ex = new MissingInputFileException(
-        constructErrorMessage(artifact) + extraMsg, null);
+    MissingInputFileException ex =
+        new MissingInputFileException(constructErrorMessage(artifact) + extraMsg, null);
     if (mandatory) {
       reporter.handle(Event.error(ex.getLocation(), ex.getMessage()));
     }
@@ -262,10 +262,10 @@ class ArtifactFunction implements SkyFunction {
     // Middleman artifacts have no corresponding files, so their ArtifactValues should have already
     // been constructed during execution of the action.
     Preconditions.checkState(!artifact.isMiddlemanArtifact(), artifact);
-    FileValue data = Preconditions.checkNotNull(actionValue.getData(artifact),
-        "%s %s", artifact, actionValue);
-    Preconditions.checkNotNull(data.getDigest(),
-          "Digest should already have been calculated for %s (%s)", artifact, data);
+    FileValue data =
+        Preconditions.checkNotNull(actionValue.getData(artifact), "%s %s", artifact, actionValue);
+    Preconditions.checkNotNull(
+        data.getDigest(), "Digest should already have been calculated for %s (%s)", artifact, data);
     // Directories are special-cased because their mtimes are used, so should have been constructed
     // during execution of the action (in ActionMetadataHandler#maybeStoreAdditionalData).
     Preconditions.checkState(data.isFile(), "Unexpected not file %s (%s)", artifact, data);
@@ -279,8 +279,10 @@ class ArtifactFunction implements SkyFunction {
       FileArtifactValue value,
       SkyFunction.Environment env)
       throws InterruptedException {
-    // This artifact aggregates other artifacts. Keep track of them so callers can find them.
-    ImmutableList.Builder<Pair<Artifact, FileArtifactValue>> inputs = ImmutableList.builder();
+    ImmutableList.Builder<Pair<Artifact, FileArtifactValue>> fileInputsBuilder =
+        ImmutableList.builder();
+    ImmutableList.Builder<Pair<Artifact, TreeArtifactValue>> directoryInputsBuilder =
+        ImmutableList.builder();
     for (Map.Entry<SkyKey, SkyValue> entry : env.getValues(action.getInputs()).entrySet()) {
       Artifact input = ArtifactSkyKey.artifact(entry.getKey());
       SkyValue inputValue = entry.getValue();
@@ -288,21 +290,32 @@ class ArtifactFunction implements SkyFunction {
         return null;
       }
       if (inputValue instanceof FileArtifactValue) {
-        inputs.add(Pair.of(input, (FileArtifactValue) inputValue));
+        fileInputsBuilder.add(Pair.of(input, (FileArtifactValue) inputValue));
       } else if (inputValue instanceof TreeArtifactValue) {
-        inputs.add(Pair.of(input, ((TreeArtifactValue) inputValue).getSelfData()));
+        directoryInputsBuilder.add(Pair.of(input, (TreeArtifactValue) inputValue));
       } else {
         // We do not recurse in aggregating middleman artifacts.
-        Preconditions.checkState(!(inputValue instanceof AggregatingArtifactValue),
-            "%s %s %s", artifact, action, inputValue);
+        Preconditions.checkState(
+            !(inputValue instanceof AggregatingArtifactValue),
+            "%s %s %s",
+            artifact,
+            action,
+            inputValue);
       }
     }
+
+    ImmutableList<Pair<Artifact, FileArtifactValue>> fileInputs =
+        ImmutableList.sortedCopyOf(
+            Comparator.comparing(pair -> pair.getFirst().getExecPathString()),
+            fileInputsBuilder.build());
+    ImmutableList<Pair<Artifact, TreeArtifactValue>> directoryInputs =
+        ImmutableList.sortedCopyOf(
+            Comparator.comparing(pair -> pair.getFirst().getExecPathString()),
+            directoryInputsBuilder.build());
+
     return (action.getActionType() == MiddlemanType.AGGREGATING_MIDDLEMAN)
-        ? new AggregatingArtifactValue(
-            ImmutableList.sortedCopyOf(
-                Comparator.comparing(pair -> pair.first.getExecPathString()), inputs.build()),
-            value)
-        : new RunfilesArtifactValue(inputs.build(), value);
+        ? new AggregatingArtifactValue(fileInputs, directoryInputs, value)
+        : new RunfilesArtifactValue(fileInputs, directoryInputs, value);
   }
 
   /**