[7.1.0] Fixes for Bazel's own integration tests fail locally on Linux (…

…#20822)

Includes 3 commits.

c48392c,
bc1d9d3,
b0db044

Progress towards #20753

---------

Co-authored-by: Googler <lberki@google.com>

src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java

@@ -21,7 +21,10 @@
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
+import java.util.TreeSet;
 
 /** Helper utility to create ActionInput instances. */
 public final class ActionInputHelper {
@@ -122,14 +125,24 @@ public static List<ActionInput> expandArtifacts(
       ArtifactExpander artifactExpander,
       boolean keepEmptyTreeArtifacts) {
     List<ActionInput> result = new ArrayList<>();
+    Set<Artifact> emptyTreeArtifacts = new TreeSet<>();
+    Set<Artifact> treeFileArtifactParents = new HashSet<>();
     for (ActionInput input : inputs.toList()) {
       if (input instanceof Artifact) {
-        Artifact.addExpandedArtifact(
-            (Artifact) input, result, artifactExpander, keepEmptyTreeArtifacts);
+        Artifact inputArtifact = (Artifact) input;
+        Artifact.addExpandedArtifact(inputArtifact, result, artifactExpander, emptyTreeArtifacts);
+        if (inputArtifact.isChildOfDeclaredDirectory()) {
+          treeFileArtifactParents.add(inputArtifact.getParent());
+        }
       } else {
         result.add(input);
       }
     }
+
+    if (keepEmptyTreeArtifacts) {
+      emptyTreeArtifacts.removeAll(treeFileArtifactParents);
+      result.addAll(emptyTreeArtifacts);
+    }
     return result;
   }
 

src/main/java/com/google/devtools/build/lib/actions/Artifact.java

@@ -59,6 +59,7 @@
 import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.function.UnaryOperator;
 import javax.annotation.Nullable;
 import net.starlark.java.eval.EvalException;
@@ -1544,21 +1545,20 @@ public static String joinRootRelativePaths(String delimiter, Iterable<Artifact>
   /**
    * Adds an artifact to a collection, expanding it once if it's a middleman or tree artifact.
    *
-   * <p>A middleman artifact is never added to the collection. If {@code keepEmptyTreeArtifacts} is
-   * true, a tree artifact will be added to the collection when it expands into zero file artifacts.
-   * Otherwise, only the file artifacts the tree artifact expands into will be added.
+   * <p>The middleman or tree artifact is never added to the output collection. If a tree artifact
+   * expands into zero file artifacts, it is added to emptyTreeArtifacts.
    */
   static void addExpandedArtifact(
       Artifact artifact,
       Collection<? super Artifact> output,
       ArtifactExpander artifactExpander,
-      boolean keepEmptyTreeArtifacts) {
+      Set<Artifact> emptyTreeArtifacts) {
     if (artifact.isMiddlemanArtifact() || artifact.isTreeArtifact()) {
       List<Artifact> expandedArtifacts = new ArrayList<>();
       artifactExpander.expand(artifact, expandedArtifacts);
       output.addAll(expandedArtifacts);
-      if (keepEmptyTreeArtifacts && artifact.isTreeArtifact() && expandedArtifacts.isEmpty()) {
-        output.add(artifact);
+      if (artifact.isTreeArtifact() && expandedArtifacts.isEmpty()) {
+        emptyTreeArtifacts.add(artifact);
       }
     } else {
       output.add(artifact);

src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java

@@ -740,6 +740,12 @@ private TestAttemptResult runTestAttempt(
               .getOutputMetadataStore()
               .getTreeArtifactChildren(
                   (SpecialArtifact) testAction.getCoverageDirectoryTreeArtifact());
+      ImmutableSet<ActionInput> coverageSpawnMetadata =
+          ImmutableSet.<ActionInput>builder()
+              .addAll(expandedCoverageDir)
+              .add(testAction.getCoverageDirectoryTreeArtifact())
+              .build();
+
       Spawn coveragePostProcessingSpawn =
           createCoveragePostProcessingSpawn(
               actionExecutionContext,
@@ -759,7 +765,7 @@ private TestAttemptResult runTestAttempt(
       ActionExecutionContext coverageActionExecutionContext =
           actionExecutionContext
               .withFileOutErr(coverageOutErr)
-              .withOutputsAsInputs(expandedCoverageDir);
+              .withOutputsAsInputs(coverageSpawnMetadata);
 
       writeOutFile(coverageOutErr.getErrorPath(), coverageOutErr.getOutputPath());
       appendCoverageLog(coverageOutErr, fileOutErr);

src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java

@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.sandbox;
 
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.NetworkNamespace.NETNS_WITH_LOOPBACK;
 import static com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.NetworkNamespace.NO_NETNS;
 
@@ -61,6 +62,8 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.SortedMap;
+import java.util.Set;
+import java.util.TreeSet;
 import java.util.concurrent.atomic.AtomicBoolean;
 import javax.annotation.Nullable;
 
@@ -390,7 +393,7 @@ public String getName() {
   protected ImmutableSet<Path> getWritableDirs(
       Path sandboxExecRoot, Path withinSandboxExecRoot, Map<String, String> env)
       throws IOException {
-    ImmutableSet.Builder<Path> writableDirs = ImmutableSet.builder();
+    Set<Path> writableDirs = new TreeSet<>();
     writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, withinSandboxExecRoot, env));
     if (getSandboxOptions().memoryLimitMb > 0) {
       CgroupsInfo cgroupsInfo = CgroupsInfo.getInstance();
@@ -400,7 +403,23 @@ protected ImmutableSet<Path> getWritableDirs(
     writableDirs.add(fs.getPath("/dev/shm").resolveSymbolicLinks());
     writableDirs.add(fs.getPath("/tmp"));
 
-    return writableDirs.build();
+    if (sandboxExecRoot.equals(withinSandboxExecRoot)) {
+      return ImmutableSet.copyOf(writableDirs);
+    }
+
+    // If a writable directory is under the sandbox exec root, transform it so that its path will
+    // be the one that it will be available at after processing the bind mounts (this is how the
+    // sandbox interprets the corresponding arguments)
+    //
+    // Notably, this is usually the case for $TEST_TMPDIR because its default value is under the
+    // execroot.
+    return writableDirs.stream()
+        .map(
+            d ->
+                d.startsWith(sandboxExecRoot)
+                    ? withinSandboxExecRoot.getRelative(d.relativeTo(sandboxExecRoot))
+                    : d)
+        .collect(toImmutableSet());
   }
 
   private ImmutableList<BindMount> getBindMounts(

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

@@ -1246,8 +1246,7 @@ private <S extends ActionInputMapSink, R> R accumulateInputs(
             topLevelFilesets,
             input,
             value,
-            env,
-            skyframeActionExecutor.requiresTreeMetadataWhenTreeFileIsInput());
+            env);
       }
     }
 

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

@@ -30,6 +30,7 @@
 import com.google.devtools.build.lib.actions.FileArtifactValue;
 import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
 import com.google.devtools.build.lib.analysis.actions.SymlinkAction;
+import com.google.devtools.build.lib.skyframe.TreeArtifactValue.ArchivedRepresentation;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import com.google.devtools.build.skyframe.SkyValue;
 import java.util.Map;
@@ -48,8 +49,7 @@ static void addToMap(
       Map<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets,
       Artifact key,
       SkyValue value,
-      Environment env,
-      boolean requiresTreeMetadataWhenTreeFileIsInput)
+      Environment env)
       throws InterruptedException {
     addToMap(
         inputMap,
@@ -60,8 +60,7 @@ static void addToMap(
         key,
         value,
         env,
-        MetadataConsumerForMetrics.NO_OP,
-        requiresTreeMetadataWhenTreeFileIsInput);
+        MetadataConsumerForMetrics.NO_OP);
   }
 
   /**
@@ -77,8 +76,7 @@ static void addToMap(
       Artifact key,
       SkyValue value,
       Environment env,
-      MetadataConsumerForMetrics consumer,
-      boolean requiresTreeMetadataWhenTreeFileIsInput)
+      MetadataConsumerForMetrics consumer)
       throws InterruptedException {
     if (value instanceof RunfilesArtifactValue) {
       // Note: we don't expand the .runfiles/MANIFEST file into the inputs. The reason for that
@@ -125,15 +123,17 @@ static void addToMap(
           /*depOwner=*/ key);
       consumer.accumulate(treeArtifactValue);
     } else if (value instanceof ActionExecutionValue) {
-      if (requiresTreeMetadataWhenTreeFileIsInput && key.isChildOfDeclaredDirectory()) {
-        // Actions resulting from the expansion of an ActionTemplate consume only one of the files
-        // in a tree artifact. However, the input prefetcher requires access to the tree metadata
-        // to determine the prefetch location of a tree artifact materialized as a symlink
-        // (cf. TreeArtifactValue#getMaterializationExecPath()).
+      // 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 (cf. TreeArtifactValue#getMaterializationExecPath()).
+      if (key.isChildOfDeclaredDirectory()) {
         SpecialArtifact treeArtifact = key.getParent();
         TreeArtifactValue treeArtifactValue =
             ((ActionExecutionValue) value).getTreeArtifactValue(treeArtifact);
         inputMap.putTreeArtifact(treeArtifact, treeArtifactValue, /* depOwner= */ treeArtifact);
+        addArchivedTreeArtifactMaybe(
+            treeArtifact, treeArtifactValue, archivedTreeArtifacts, inputMap, key);
         consumer.accumulate(treeArtifactValue);
       }
       FileArtifactValue metadata = ((ActionExecutionValue) value).getExistingFileArtifactValue(key);
@@ -221,20 +221,27 @@ private static void expandTreeArtifactAndPopulateArtifactData(
       return;
     }
 
-    inputMap.putTreeArtifact((SpecialArtifact) treeArtifact, value, depOwner);
     expandedArtifacts.put(treeArtifact, value.getChildren());
+    inputMap.putTreeArtifact((SpecialArtifact) treeArtifact, value, depOwner);
+    addArchivedTreeArtifactMaybe(
+        (SpecialArtifact) treeArtifact, value, archivedTreeArtifacts, inputMap, depOwner);
+  }
+
+  private static void addArchivedTreeArtifactMaybe(
+      SpecialArtifact treeArtifact,
+      TreeArtifactValue value,
+      Map<SpecialArtifact, ArchivedTreeArtifact> archivedTreeArtifacts,
+      ActionInputMapSink inputMap,
+      Artifact depOwner) {
+    if (!value.getArchivedRepresentation().isPresent()) {
+      return;
+    }
 
-    value
-        .getArchivedRepresentation()
-        .ifPresent(
-            archivedRepresentation -> {
-              inputMap.put(
-                  archivedRepresentation.archivedTreeFileArtifact(),
-                  archivedRepresentation.archivedFileValue(),
-                  depOwner);
-              archivedTreeArtifacts.put(
-                  (SpecialArtifact) treeArtifact,
-                  archivedRepresentation.archivedTreeFileArtifact());
-            });
+    ArchivedRepresentation archivedRepresentation = value.getArchivedRepresentation().get();
+    inputMap.put(
+        archivedRepresentation.archivedTreeFileArtifact(),
+        archivedRepresentation.archivedFileValue(),
+        depOwner);
+    archivedTreeArtifacts.put(treeArtifact, archivedRepresentation.archivedTreeFileArtifact());
   }
 }

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

@@ -205,8 +205,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
                 input,
                 artifactValue,
                 env,
-                currentConsumer,
-                skyframeActionExecutor.requiresTreeMetadataWhenTreeFileIsInput());
+                currentConsumer);
             if (!allArtifactsAreImportant && importantArtifactSet.contains(input)) {
               // Calling #addToMap a second time with `input` and `artifactValue` will perform no-op
               // updates to the secondary collections passed in (eg. expandedArtifacts,
@@ -220,8 +219,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
                   topLevelFilesets,
                   input,
                   artifactValue,
-                  env,
-                  skyframeActionExecutor.requiresTreeMetadataWhenTreeFileIsInput());
+                  env);
             }
           }
         }

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

@@ -350,10 +350,6 @@ boolean useArchivedTreeArtifacts(ActionAnalysisMetadata action) {
         .test(action.getMnemonic());
   }
 
-  boolean requiresTreeMetadataWhenTreeFileIsInput() {
-    return actionInputPrefetcher.requiresTreeMetadataWhenTreeFileIsInput();
-  }
-
   boolean publishTargetSummaries() {
     return options.getOptions(BuildEventProtocolOptions.class).publishTargetSummary;
   }