sandbox: Extract the common methods that get the input files from a Spawn into a shared helper class.

--
MOS_MIGRATED_REVID=132546638

4 files changed, 222 insertions, 184 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
index 51c4d64ccf..4c8f0c3897 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
@@ -71,6 +71,7 @@ public class DarwinSandboxedStrategy extends SandboxStrategy {
   private final boolean verboseFailures;
   private final String productName;
   private final ImmutableList<Path> confPaths;
+  private final SpawnHelpers spawnHelpers;
 
   private final UUID uuid = UUID.randomUUID();
   private final AtomicInteger execCounter = new AtomicInteger();
@@ -82,7 +83,8 @@ public class DarwinSandboxedStrategy extends SandboxStrategy {
       ExecutorService backgroundWorkers,
       boolean verboseFailures,
       String productName,
-      ImmutableList<Path> confPaths) {
+      ImmutableList<Path> confPaths,
+      SpawnHelpers spawnHelpers) {
     super(blazeDirs, verboseFailures, buildRequest.getOptions(SandboxOptions.class));
     this.buildRequest = buildRequest;
     this.clientEnv = ImmutableMap.copyOf(clientEnv);
@@ -93,6 +95,7 @@ public class DarwinSandboxedStrategy extends SandboxStrategy {
     this.verboseFailures = verboseFailures;
     this.productName = productName;
     this.confPaths = confPaths;
+    this.spawnHelpers = spawnHelpers;
   }
 
   public static DarwinSandboxedStrategy create(
@@ -121,7 +124,8 @@ public class DarwinSandboxedStrategy extends SandboxStrategy {
         backgroundWorkers,
         verboseFailures,
         productName,
-        writablePaths.build());
+        writablePaths.build(),
+        new SpawnHelpers(blazeDirs.getExecRoot()));
   }
 
   /**
@@ -250,16 +254,17 @@ public class DarwinSandboxedStrategy extends SandboxStrategy {
     return inaccessiblePaths.build();
   }
 
-  private Map<PathFragment, Path> getMounts(Spawn spawn, ActionExecutionContext executionContext)
+  @Override
+  public Map<PathFragment, Path> getMounts(Spawn spawn, ActionExecutionContext executionContext)
       throws ExecException {
     try {
       Map<PathFragment, Path> mounts = new HashMap<>();
-      mountInputs(mounts, spawn, executionContext);
+      spawnHelpers.mountInputs(mounts, spawn, executionContext);
 
       Map<PathFragment, Path> unfinalized = new HashMap<>();
-      mountRunfilesFromManifests(unfinalized, spawn);
-      mountRunfilesFromSuppliers(unfinalized, spawn);
-      mountFilesFromFilesetManifests(unfinalized, spawn, executionContext);
+      spawnHelpers.mountRunfilesFromManifests(unfinalized, spawn);
+      spawnHelpers.mountRunfilesFromSuppliers(unfinalized, spawn);
+      spawnHelpers.mountFilesFromFilesetManifests(unfinalized, spawn, executionContext);
       mounts.putAll(finalizeLinks(unfinalized));
 
       return mounts;
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
index d23f864faf..e16c6dc9e4 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.sandbox;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
-import com.google.devtools.build.lib.actions.EnvironmentalExecException;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.ExecutionStrategy;
 import com.google.devtools.build.lib.actions.Executor;
@@ -31,8 +30,6 @@ import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.ExecutorService;
@@ -146,17 +143,4 @@ public class LinuxSandboxedStrategy extends SandboxStrategy {
     }
   }
 
-  private Map<PathFragment, Path> getMounts(Spawn spawn, ActionExecutionContext executionContext)
-      throws ExecException {
-    try {
-      Map<PathFragment, Path> mounts = new HashMap<>();
-      mountRunfilesFromManifests(mounts, spawn);
-      mountRunfilesFromSuppliers(mounts, spawn);
-      mountFilesFromFilesetManifests(mounts, spawn, executionContext);
-      mountInputs(mounts, spawn, executionContext);
-      return mounts;
-    } catch (IllegalArgumentException | IOException e) {
-      throw new EnvironmentalExecException("Could not prepare mounts for sandbox execution", e);
-    }
-  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java
index 645e669a23..7a12e9b600 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java
@@ -16,43 +16,31 @@ package com.google.devtools.build.lib.sandbox;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSet.Builder;
-import com.google.common.io.Files;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
-import com.google.devtools.build.lib.actions.ActionInput;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
-import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.EnvironmentalExecException;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.SpawnActionContext;
-import com.google.devtools.build.lib.analysis.AnalysisUtils;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
-import com.google.devtools.build.lib.rules.cpp.CppCompileAction;
-import com.google.devtools.build.lib.rules.fileset.FilesetActionContext;
-import com.google.devtools.build.lib.util.Preconditions;
-import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
-import java.io.File;
 import java.io.IOException;
-import java.nio.charset.StandardCharsets;
-import java.util.List;
 import java.util.Map;
 
 /** Abstract common ancestor for sandbox strategies implementing the common parts. */
 abstract class SandboxStrategy implements SpawnActionContext {
 
   private final BlazeDirectories blazeDirs;
-  private final Path execRoot;
   private final boolean verboseFailures;
   private final SandboxOptions sandboxOptions;
+  private final SpawnHelpers spawnHelpers;
 
   public SandboxStrategy(
       BlazeDirectories blazeDirs, boolean verboseFailures, SandboxOptions sandboxOptions) {
     this.blazeDirs = blazeDirs;
-    this.execRoot = blazeDirs.getExecRoot();
     this.verboseFailures = verboseFailures;
     this.sandboxOptions = sandboxOptions;
+    this.spawnHelpers = new SpawnHelpers(blazeDirs.getExecRoot());
   }
 
   /** Gets the list of directories that the spawn will assume to be writable. */
@@ -73,153 +61,6 @@ abstract class SandboxStrategy implements SpawnActionContext {
     return inaccessiblePaths.build();
   }
 
-  /** Mount all runfiles that the spawn needs as specified in its runfiles manifests. */
-  protected void mountRunfilesFromManifests(Map<PathFragment, Path> mounts, Spawn spawn)
-      throws IOException, ExecException {
-    for (Map.Entry<PathFragment, Artifact> manifest : spawn.getRunfilesManifests().entrySet()) {
-      String manifestFilePath = manifest.getValue().getPath().getPathString();
-      Preconditions.checkState(!manifest.getKey().isAbsolute());
-      PathFragment targetDirectory = manifest.getKey();
-
-      parseManifestFile(
-          blazeDirs.getFileSystem(),
-          mounts,
-          targetDirectory,
-          new File(manifestFilePath),
-          false,
-          "");
-    }
-  }
-
-  /** Mount all files that the spawn needs as specified in its fileset manifests. */
-  protected void mountFilesFromFilesetManifests(
-      Map<PathFragment, Path> mounts, Spawn spawn, ActionExecutionContext executionContext)
-      throws IOException, ExecException {
-    final FilesetActionContext filesetContext =
-        executionContext.getExecutor().getContext(FilesetActionContext.class);
-    for (Artifact fileset : spawn.getFilesetManifests()) {
-      File manifestFile =
-          new File(
-              execRoot.getPathString(),
-              AnalysisUtils.getManifestPathFromFilesetPath(fileset.getExecPath()).getPathString());
-      PathFragment targetDirectory = fileset.getExecPath();
-
-      parseManifestFile(
-          blazeDirs.getFileSystem(),
-          mounts,
-          targetDirectory,
-          manifestFile,
-          true,
-          filesetContext.getWorkspaceName());
-    }
-  }
-
-  /** A parser for the MANIFEST files used by Filesets and runfiles. */
-  static void parseManifestFile(
-      FileSystem fs,
-      Map<PathFragment, Path> mounts,
-      PathFragment targetDirectory,
-      File manifestFile,
-      boolean isFilesetManifest,
-      String workspaceName)
-      throws IOException, ExecException {
-    int lineNum = 0;
-    for (String line : Files.readLines(manifestFile, StandardCharsets.UTF_8)) {
-      if (isFilesetManifest && (++lineNum % 2 == 0)) {
-        continue;
-      }
-      if (line.isEmpty()) {
-        continue;
-      }
-
-      String[] fields = line.trim().split(" ");
-
-      // The "target" field is always a relative path that is to be interpreted in this way:
-      // (1) If this is a fileset manifest and our workspace name is not empty, the first segment
-      // of each "target" path must be the workspace name, which is then stripped before further
-      // processing.
-      // (2) The "target" path is then appended to the "targetDirectory", which is a path relative
-      // to the execRoot. Together, this results in the full path in the execRoot in which place a
-      // symlink referring to "source" has to be created (see below).
-      PathFragment targetPath;
-      if (isFilesetManifest) {
-        PathFragment targetPathFragment = new PathFragment(fields[0]);
-        if (!workspaceName.isEmpty()) {
-          if (!targetPathFragment.getSegment(0).equals(workspaceName)) {
-            throw new EnvironmentalExecException(
-                "Fileset manifest line must start with workspace name");
-          }
-          targetPathFragment = targetPathFragment.subFragment(1, targetPathFragment.segmentCount());
-        }
-        targetPath = targetDirectory.getRelative(targetPathFragment);
-      } else {
-        targetPath = targetDirectory.getRelative(fields[0]);
-      }
-
-      // The "source" field, if it exists, is always an absolute path and may point to any file in
-      // the filesystem (it is not limited to files in the workspace or execroot).
-      Path source;
-      switch (fields.length) {
-        case 1:
-          source = fs.getPath("/dev/null");
-          break;
-        case 2:
-          source = fs.getPath(fields[1]);
-          break;
-        default:
-          throw new IllegalStateException("'" + line + "' splits into more than 2 parts");
-      }
-
-      mounts.put(targetPath, source);
-    }
-  }
-
-  /** Mount all runfiles that the spawn needs as specified via its runfiles suppliers. */
-  protected void mountRunfilesFromSuppliers(Map<PathFragment, Path> mounts, Spawn spawn)
-      throws IOException {
-    Map<PathFragment, Map<PathFragment, Artifact>> rootsAndMappings =
-        spawn.getRunfilesSupplier().getMappings();
-    for (Map.Entry<PathFragment, Map<PathFragment, Artifact>> rootAndMappings :
-        rootsAndMappings.entrySet()) {
-      PathFragment root = rootAndMappings.getKey();
-      if (root.isAbsolute()) {
-        root = root.relativeTo(execRoot.asFragment());
-      }
-      for (Map.Entry<PathFragment, Artifact> mapping : rootAndMappings.getValue().entrySet()) {
-        Artifact sourceArtifact = mapping.getValue();
-        PathFragment source =
-            (sourceArtifact != null) ? sourceArtifact.getExecPath() : new PathFragment("/dev/null");
-
-        Preconditions.checkArgument(!mapping.getKey().isAbsolute());
-        PathFragment target = root.getRelative(mapping.getKey());
-        mounts.put(target, execRoot.getRelative(source));
-      }
-    }
-  }
-
-  /** Mount all inputs of the spawn. */
-  protected void mountInputs(
-      Map<PathFragment, Path> mounts, Spawn spawn, ActionExecutionContext actionExecutionContext) {
-    List<ActionInput> inputs =
-        ActionInputHelper.expandArtifacts(
-            spawn.getInputFiles(), actionExecutionContext.getArtifactExpander());
-
-    if (spawn.getResourceOwner() instanceof CppCompileAction) {
-      CppCompileAction action = (CppCompileAction) spawn.getResourceOwner();
-      if (action.shouldScanIncludes()) {
-        inputs.addAll(action.getAdditionalInputs());
-      }
-    }
-
-    for (ActionInput input : inputs) {
-      if (input.getExecPathString().contains("internal/_middlemen/")) {
-        continue;
-      }
-      PathFragment mount = new PathFragment(input.getExecPathString());
-      mounts.put(mount, execRoot.getRelative(mount));
-    }
-  }
-
   @Override
   public boolean willExecuteRemotely(boolean remotable) {
     return false;
@@ -234,4 +75,14 @@ abstract class SandboxStrategy implements SpawnActionContext {
   public boolean shouldPropagateExecException() {
     return verboseFailures && sandboxOptions.sandboxDebug;
   }
+
+  public Map<PathFragment, Path> getMounts(Spawn spawn, ActionExecutionContext executionContext)
+      throws ExecException {
+    try {
+      return spawnHelpers.getMounts(spawn, executionContext);
+    } catch (IOException e) {
+      throw new EnvironmentalExecException("Could not prepare mounts for sandbox execution", e);
+    }
+  }
+
 }
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java
new file mode 100644
index 0000000000..34ddd9e782
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java
@@ -0,0 +1,198 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.sandbox;
+
+import com.google.common.io.Files;
+import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.ActionInputHelper;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Spawn;
+import com.google.devtools.build.lib.analysis.AnalysisUtils;
+import com.google.devtools.build.lib.rules.cpp.CppCompileAction;
+import com.google.devtools.build.lib.rules.fileset.FilesetActionContext;
+import com.google.devtools.build.lib.util.Preconditions;
+import com.google.devtools.build.lib.vfs.FileSystem;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/** Contains common helper methods that extract information from {@link Spawn} objects. */
+public final class SpawnHelpers {
+
+  private final Path execRoot;
+
+  public SpawnHelpers(Path execRoot) {
+    this.execRoot = execRoot;
+  }
+
+  /**
+   * 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.
+   */
+  public Map<PathFragment, Path> getMounts(Spawn spawn, ActionExecutionContext executionContext)
+      throws IOException {
+    Map<PathFragment, Path> mounts = new HashMap<>();
+    mountRunfilesFromManifests(mounts, spawn);
+    mountRunfilesFromSuppliers(mounts, spawn);
+    mountFilesFromFilesetManifests(mounts, spawn, executionContext);
+    mountInputs(mounts, spawn, executionContext);
+    return mounts;
+  }
+
+  /** Mount all runfiles that the spawn needs as specified in its runfiles manifests. */
+  void mountRunfilesFromManifests(Map<PathFragment, Path> mounts, Spawn spawn) throws IOException {
+    for (Map.Entry<PathFragment, Artifact> manifest : spawn.getRunfilesManifests().entrySet()) {
+      String manifestFilePath = manifest.getValue().getPath().getPathString();
+      Preconditions.checkState(!manifest.getKey().isAbsolute());
+      PathFragment targetDirectory = manifest.getKey();
+
+      parseManifestFile(
+          execRoot.getFileSystem(), mounts, targetDirectory, new File(manifestFilePath), false, "");
+    }
+  }
+
+  /** Mount all files that the spawn needs as specified in its fileset manifests. */
+  void mountFilesFromFilesetManifests(
+      Map<PathFragment, Path> mounts, Spawn spawn, ActionExecutionContext executionContext)
+      throws IOException {
+    final FilesetActionContext filesetContext =
+        executionContext.getExecutor().getContext(FilesetActionContext.class);
+    for (Artifact fileset : spawn.getFilesetManifests()) {
+      File manifestFile =
+          new File(
+              execRoot.getPathString(),
+              AnalysisUtils.getManifestPathFromFilesetPath(fileset.getExecPath()).getPathString());
+      PathFragment targetDirectory = fileset.getExecPath();
+
+      parseManifestFile(
+          execRoot.getFileSystem(),
+          mounts,
+          targetDirectory,
+          manifestFile,
+          true,
+          filesetContext.getWorkspaceName());
+    }
+  }
+
+  /** A parser for the MANIFEST files used by Filesets and runfiles. */
+  static void parseManifestFile(
+      FileSystem fs,
+      Map<PathFragment, Path> mounts,
+      PathFragment targetDirectory,
+      File manifestFile,
+      boolean isFilesetManifest,
+      String workspaceName)
+      throws IOException {
+    int lineNum = 0;
+    for (String line : Files.readLines(manifestFile, StandardCharsets.UTF_8)) {
+      if (isFilesetManifest && (++lineNum % 2 == 0)) {
+        continue;
+      }
+      if (line.isEmpty()) {
+        continue;
+      }
+
+      String[] fields = line.trim().split(" ");
+
+      // The "target" field is always a relative path that is to be interpreted in this way:
+      // (1) If this is a fileset manifest and our workspace name is not empty, the first segment
+      // of each "target" path must be the workspace name, which is then stripped before further
+      // processing.
+      // (2) The "target" path is then appended to the "targetDirectory", which is a path relative
+      // to the execRoot. Together, this results in the full path in the execRoot in which place a
+      // symlink referring to "source" has to be created (see below).
+      PathFragment targetPath;
+      if (isFilesetManifest) {
+        PathFragment targetPathFragment = new PathFragment(fields[0]);
+        if (!workspaceName.isEmpty()) {
+          Preconditions.checkState(
+              targetPathFragment.getSegment(0).equals(workspaceName),
+              "Fileset manifest line must start with workspace name");
+          targetPathFragment = targetPathFragment.subFragment(1, targetPathFragment.segmentCount());
+        }
+        targetPath = targetDirectory.getRelative(targetPathFragment);
+      } else {
+        targetPath = targetDirectory.getRelative(fields[0]);
+      }
+
+      // The "source" field, if it exists, is always an absolute path and may point to any file in
+      // the filesystem (it is not limited to files in the workspace or execroot).
+      Path source;
+      switch (fields.length) {
+        case 1:
+          source = fs.getPath("/dev/null");
+          break;
+        case 2:
+          source = fs.getPath(fields[1]);
+          break;
+        default:
+          throw new IllegalStateException("'" + line + "' splits into more than 2 parts");
+      }
+
+      mounts.put(targetPath, source);
+    }
+  }
+
+  /** Mount all runfiles that the spawn needs as specified via its runfiles suppliers. */
+  void mountRunfilesFromSuppliers(Map<PathFragment, Path> mounts, Spawn spawn) throws IOException {
+    Map<PathFragment, Map<PathFragment, Artifact>> rootsAndMappings =
+        spawn.getRunfilesSupplier().getMappings();
+    for (Map.Entry<PathFragment, Map<PathFragment, Artifact>> rootAndMappings :
+        rootsAndMappings.entrySet()) {
+      PathFragment root = rootAndMappings.getKey();
+      if (root.isAbsolute()) {
+        root = root.relativeTo(execRoot.asFragment());
+      }
+      for (Map.Entry<PathFragment, Artifact> mapping : rootAndMappings.getValue().entrySet()) {
+        Artifact sourceArtifact = mapping.getValue();
+        PathFragment source =
+            (sourceArtifact != null) ? sourceArtifact.getExecPath() : new PathFragment("/dev/null");
+
+        Preconditions.checkArgument(!mapping.getKey().isAbsolute());
+        PathFragment target = root.getRelative(mapping.getKey());
+        mounts.put(target, execRoot.getRelative(source));
+      }
+    }
+  }
+
+  /** Mount all inputs of the spawn. */
+  void mountInputs(
+      Map<PathFragment, Path> mounts, Spawn spawn, ActionExecutionContext actionExecutionContext) {
+    List<ActionInput> inputs =
+        ActionInputHelper.expandArtifacts(
+            spawn.getInputFiles(), actionExecutionContext.getArtifactExpander());
+
+    if (spawn.getResourceOwner() instanceof CppCompileAction) {
+      CppCompileAction action = (CppCompileAction) spawn.getResourceOwner();
+      if (action.shouldScanIncludes()) {
+        inputs.addAll(action.getAdditionalInputs());
+      }
+    }
+
+    for (ActionInput input : inputs) {
+      if (input.getExecPathString().contains("internal/_middlemen/")) {
+        continue;
+      }
+      PathFragment mount = new PathFragment(input.getExecPathString());
+      mounts.put(mount, execRoot.getRelative(mount));
+    }
+  }
+}