Fix output base under /tmp.

Resolved symlink action outputs and param files are both fixed.
bazelbuild · Dec 19, 2023 · d1ab02e · d1ab02e
1 parent 103830f
commit d1ab02e
Show file tree

Hide file tree

Showing 11 changed files with 78 additions and 6 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
@@ -214,6 +214,12 @@ public static FileArtifactValue createForSourceArtifact(
         xattrProvider);
   }
 
+  public static FileArtifactValue createForResolvedSymlink(
+      PathFragment realPath, FileArtifactValue metadata, @Nullable byte[] digest) {
+    return new ResolvedSymlinkFileArtifactValue(
+        realPath, digest, metadata.getContentsProxy(), metadata.getSize());
+  }
+
   public static FileArtifactValue createFromInjectedDigest(
       FileArtifactValue metadata, @Nullable byte[] digest) {
     return createForNormalFile(digest, metadata.getContentsProxy(), metadata.getSize());
@@ -439,7 +445,22 @@ public String toString() {
     }
   }
 
-  private static final class RegularFileArtifactValue extends FileArtifactValue {
+  private static final class ResolvedSymlinkFileArtifactValue extends RegularFileArtifactValue {
+    private final PathFragment realPath;
+
+    private ResolvedSymlinkFileArtifactValue(PathFragment realPath, @Nullable byte[] digest,
+        @Nullable FileContentsProxy proxy, long size) {
+      super(digest, proxy, size);
+      this.realPath = realPath;
+    }
+
+    @Override
+    public Optional<PathFragment> getMaterializationExecPath() {
+      return Optional.of(realPath);
+    }
+  }
+
+  private static class RegularFileArtifactValue extends FileArtifactValue {
     private final byte[] digest;
     @Nullable private final FileContentsProxy proxy;
     private final long size;

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD
@@ -17,6 +17,7 @@ java_library(
     deps = [
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
+        "//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
         "//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/vfs",

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
@@ -228,6 +228,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
     SandboxInputs inputs =
         helpers.processInputFiles(
             context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true),
+            context.getInputMetadataProvider(),
             execRoot,
             execRoot,
             packageRoots,

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java
@@ -226,6 +226,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
     SandboxInputs inputs =
         helpers.processInputFiles(
             context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true),
+            context.getInputMetadataProvider(),
             execRoot,
             execRoot,
             packageRoots,

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
@@ -281,6 +281,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
     SandboxInputs inputs =
         helpers.processInputFiles(
             context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true),
+            context.getInputMetadataProvider(),
             execRoot,
             withinSandboxExecRoot,
             packageRoots,

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java
@@ -100,6 +100,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
     SandboxInputs inputs =
         helpers.processInputFiles(
             context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true),
+            context.getInputMetadataProvider(),
             execRoot,
             execRoot,
             packageRoots,

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java
@@ -29,6 +29,8 @@
 import com.google.common.flogger.GoogleLogger;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.FileArtifactValue;
+import com.google.devtools.build.lib.actions.InputMetadataProvider;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.UserExecException;
 import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
@@ -444,6 +446,25 @@ private static RootedPath processFilesetSymlink(
             symlink.getPathString()));
   }
 
+  private static RootedPath processResolvedSymlink(
+      Root absoluteRoot,
+      PathFragment symlinkTarget,
+      Root execRootWithinSandbox,
+      PathFragment execRootFragment,
+      ImmutableList<Root> packageRoots) {
+    for (Root packageRoot : packageRoots) {
+      if (packageRoot.contains(symlinkTarget)) {
+        return RootedPath.toRootedPath(packageRoot, packageRoot.relativize(symlinkTarget));
+      }
+    }
+
+    if (symlinkTarget.startsWith(execRootFragment)) {
+      return RootedPath.toRootedPath(execRootWithinSandbox, symlinkTarget.relativeTo(execRootFragment));
+    }
+
+    return RootedPath.toRootedPath(absoluteRoot, symlinkTarget);
+  }
+
   /**
    * Returns the inputs of a Spawn as a map of PathFragments relative to an execRoot to paths in the
    * host filesystem where the input files can be found.
@@ -458,6 +479,7 @@ private static RootedPath processFilesetSymlink(
    */
   public SandboxInputs processInputFiles(
       Map<PathFragment, ActionInput> inputMap,
+      InputMetadataProvider inputMetadataProvider,
       Path execRootPath,
       Path withinSandboxExecRootPath,
       ImmutableList<Root> packageRoots,
@@ -468,6 +490,7 @@ public SandboxInputs processInputFiles(
         withinSandboxExecRootPath.equals(execRootPath)
             ? withinSandboxExecRoot
             : Root.fromPath(execRootPath);
+    Root absoluteRoot = Root.absoluteRoot(execRootPath.getFileSystem());
 
     Map<PathFragment, RootedPath> inputFiles = new TreeMap<>();
     Map<PathFragment, PathFragment> inputSymlinks = new TreeMap<>();
@@ -503,9 +526,11 @@ public SandboxInputs processInputFiles(
 
         if (actionInput instanceof EmptyActionInput) {
           inputPath = null;
+        } else if (actionInput instanceof VirtualActionInput) {
+          inputPath = RootedPath.toRootedPath(withinSandboxExecRoot, actionInput.getExecPath());
         } else if (actionInput instanceof Artifact) {
           Artifact inputArtifact = (Artifact) actionInput;
-          if (inputArtifact.isSourceArtifact() && sandboxSourceRoots != null) {
+          if (inputArtifact.isSourceArtifact() && !inputArtifact.isDirectory() && sandboxSourceRoots != null) {
             Root sourceRoot = inputArtifact.getRoot().getRoot();
             if (!sourceRootToSandboxSourceRoot.containsKey(sourceRoot)) {
               int next = sourceRootToSandboxSourceRoot.size();
@@ -518,6 +543,18 @@ public SandboxInputs processInputFiles(
                 RootedPath.toRootedPath(
                     sourceRootToSandboxSourceRoot.get(sourceRoot),
                     inputArtifact.getRootRelativePath());
+          } else if (!inputArtifact.isSourceArtifact() && sandboxSourceRoots != null) {
+            FileArtifactValue metadata = inputMetadataProvider.getInputMetadata(actionInput);
+            if (!metadata.isRemote() && metadata.getMaterializationExecPath().isPresent()) {
+              inputPath = processResolvedSymlink(
+                  absoluteRoot,
+                  metadata.getMaterializationExecPath().get(),
+                  execRoot,
+                  execRootPath.asFragment(),
+                  packageRoots);
+            } else {
+              inputPath = RootedPath.toRootedPath(withinSandboxExecRoot, inputArtifact.getExecPath());
+            }
           } else {
             inputPath = RootedPath.toRootedPath(withinSandboxExecRoot, inputArtifact.getExecPath());
           }

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java
@@ -76,6 +76,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
     SandboxInputs readablePaths =
         helpers.processInputFiles(
             context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true),
+            context.getInputMetadataProvider(),
             execRoot,
             execRoot,
             packageRoots,

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java
@@ -536,6 +536,8 @@ private FileArtifactValue constructFileArtifactValue(
         // possible to hit the digest cache - we probably already computed the digest for the
         // target during previous action execution.
         path = statAndValue.realPath();
+        injectedDigest = DigestUtils.manuallyComputeDigest(path, value.getSize());
+        return FileArtifactValue.createForResolvedSymlink(path.asFragment(), value, injectedDigest);
       }
 
       injectedDigest = DigestUtils.manuallyComputeDigest(path, value.getSize());

diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java
@@ -189,6 +189,7 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
             helpers.processInputFiles(
                 context.getInputMapping(
                     PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true),
+                context.getInputMetadataProvider(),
                 execRoot,
                 execRoot,
                 packageRoots,

diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java
@@ -32,6 +32,7 @@
 import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
 import com.google.devtools.build.lib.exec.BinTools;
+import com.google.devtools.build.lib.exec.util.FakeActionInputFileCache;
 import com.google.devtools.build.lib.exec.util.SpawnBuilder;
 import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs;
 import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs;
@@ -106,7 +107,9 @@ public void processInputFiles_materializesParamFile() throws Exception {
 
     SandboxInputs inputs =
         sandboxHelpers.processInputFiles(
-            inputMap(paramFile), execRootPath, execRootPath, ImmutableList.of(), null);
+            inputMap(paramFile),
+            new FakeActionInputFileCache(),
+            execRootPath, execRootPath, ImmutableList.of(), null);
 
     assertThat(inputs.getFiles())
         .containsExactly(PathFragment.create("paramFile"), execRootedPath("paramFile"));
@@ -127,7 +130,9 @@ public void processInputFiles_materializesBinToolsFile() throws Exception {
 
     SandboxInputs inputs =
         sandboxHelpers.processInputFiles(
-            inputMap(tool), execRootPath, execRootPath, ImmutableList.of(), null);
+            inputMap(tool),
+            new FakeActionInputFileCache(),
+            execRootPath, execRootPath, ImmutableList.of(), null);
 
     assertThat(inputs.getFiles())
         .containsExactly(PathFragment.create("_bin/say_hello"), execRootedPath("_bin/say_hello"));
@@ -173,15 +178,15 @@ protected void setExecutable(PathFragment path, boolean executable) throws IOExc
               try {
                 var unused =
                     sandboxHelpers.processInputFiles(
-                        inputMap(input), customExecRoot, customExecRoot, ImmutableList.of(), null);
+                        inputMap(input), new FakeActionInputFileCache(), customExecRoot, customExecRoot, ImmutableList.of(), null);
                 finishProcessingSemaphore.release();
               } catch (IOException | InterruptedException e) {
                 throw new IllegalArgumentException(e);
               }
             });
     var unused =
         sandboxHelpers.processInputFiles(
-            inputMap(input), customExecRoot, customExecRoot, ImmutableList.of(), null);
+            inputMap(input), new FakeActionInputFileCache(), customExecRoot, customExecRoot, ImmutableList.of(), null);
     finishProcessingSemaphore.release();
     future.get();