diff options
author | 2018-04-16 06:40:19 -0700 | |
---|---|---|
committer | 2018-04-16 06:41:42 -0700 | |
commit | d3de5cc7e25e56fba666e1f39a9ebf3c76fdd69c (patch) | |
tree | 02716ad2664f1823976642c4344af7a3702d07a1 /src/main/java/com/google/devtools/build/lib/sandbox | |
parent | 8caa745545a1757fe93c6684d5ab98a47fa2718c (diff) |
Roll forward of https://github.com/bazelbuild/bazel/commit/656a0bab1e025ff3c27d595284a4bf1c5a8d8028 with test (unknown commit) and fix.
Big round of sandbox fixes / performance improvements.
- The number of stat() syscalls in the SymlinkedSandboxedSpawn was way too high. Do less, feel better.
- When using --experimental_sandbox_base, ensure that symlinks in the path are resolved. Before this, you had to check whether on your system /dev/shm is a symlink to /run/shm and then use that instead. Now it no longer matters, as symlinks are resolved.
- Remove an unnecessary directory creation from each sandboxed invocation. Turns out that the "tmpdir" that we created was no longer used after some changes to Bazel's TMPDIR handling.
- Use simpler sandbox paths, by using the unique ID for each Spawn provided by SpawnExecutionPolicy instead of a randomly generated temp folder name. This also saves a round-trip from our VFS to NIO and back. Clean up the sandbox base before each build to ensure that the unique IDs are actually unique. ;)
- Use Java 8's Process#isAlive to check whether a process is alive instead of trying to get the exitcode and catching an exception.
Closes #4913.
PiperOrigin-RevId: 193031017
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/sandbox')
9 files changed, 93 insertions, 151 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java index 33b21209da..9fee6544dd 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java @@ -50,13 +50,11 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { private static final String SANDBOX_DEBUG_SUGGESTION = "\n\nUse --sandbox_debug to see verbose messages from the sandbox"; - private final Path sandboxBase; private final SandboxOptions sandboxOptions; private final boolean verboseFailures; private final ImmutableSet<Path> inaccessiblePaths; - public AbstractSandboxSpawnRunner(CommandEnvironment cmdEnv, Path sandboxBase) { - this.sandboxBase = sandboxBase; + public AbstractSandboxSpawnRunner(CommandEnvironment cmdEnv) { this.sandboxOptions = cmdEnv.getOptions().getOptions(SandboxOptions.class); this.verboseFailures = cmdEnv.getOptions().getOptions(ExecutionOptions.class).verboseFailures; this.inaccessiblePaths = @@ -88,7 +86,6 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { SandboxedSpawn sandbox, SpawnExecutionPolicy policy, Path execRoot, - Path tmpDir, Duration timeout, Path statisticsPath) throws IOException, InterruptedException { @@ -97,8 +94,7 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { OutErr outErr = policy.getFileOutErr(); policy.prefetchInputs(); - SpawnResult result = - run(originalSpawn, sandbox, outErr, timeout, execRoot, tmpDir, statisticsPath); + SpawnResult result = run(originalSpawn, sandbox, outErr, timeout, execRoot, statisticsPath); policy.lockOutputFiles(); try { @@ -121,7 +117,6 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { OutErr outErr, Duration timeout, Path execRoot, - Path tmpDir, Path statisticsPath) throws IOException, InterruptedException { Command cmd = new Command( @@ -145,9 +140,6 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { long startTime = System.currentTimeMillis(); CommandResult commandResult; try { - if (!tmpDir.exists() && !tmpDir.createDirectory()) { - throw new IOException(String.format("Could not create temp directory '%s'", tmpDir)); - } commandResult = cmd.execute(outErr.getOutputStream(), outErr.getErrorStream()); if (Thread.currentThread().isInterrupted()) { throw new InterruptedException(); @@ -214,17 +206,6 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { } /** - * Returns a temporary directory that should be used as the sandbox directory for a single action. - */ - protected Path getSandboxRoot() throws IOException { - return sandboxBase.getRelative( - java.nio.file.Files.createTempDirectory( - java.nio.file.Paths.get(sandboxBase.getPathString()), "") - .getFileName() - .toString()); - } - - /** * Gets the list of directories that the spawn will assume to be writable. * * @throws IOException because we might resolve symlinks, which throws {@link IOException}. 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 index 1060034ccc..d20eec70b1 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java @@ -21,7 +21,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.io.ByteStreams; -import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionStrategy; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; @@ -96,6 +95,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { private final Path execRoot; private final boolean allowNetwork; private final Path processWrapper; + private final Path sandboxBase; private final Duration timeoutKillDelay; private final @Nullable SandboxfsProcess sandboxfsProcess; @@ -123,13 +123,14 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { Duration timeoutKillDelay, @Nullable SandboxfsProcess sandboxfsProcess) throws IOException { - super(cmdEnv, sandboxBase); + super(cmdEnv); this.execRoot = cmdEnv.getExecRoot(); this.allowNetwork = SandboxHelpers.shouldAllowNetwork(cmdEnv.getOptions()); this.alwaysWritableDirs = getAlwaysWritableDirs(cmdEnv.getRuntime().getFileSystem()); this.processWrapper = ProcessWrapperUtil.getProcessWrapper(cmdEnv); this.localEnvProvider = new XcodeLocalEnvProvider(cmdEnv.getRuntime().getProductName(), cmdEnv.getClientEnv()); + this.sandboxBase = sandboxBase; this.timeoutKillDelay = timeoutKillDelay; this.sandboxfsProcess = sandboxfsProcess; } @@ -193,21 +194,19 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { @Override protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy) - throws ExecException, IOException, InterruptedException { + throws IOException, InterruptedException { // Each invocation of "exec" gets its own sandbox. - Path sandboxPath = getSandboxRoot(); - Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName()); + Path sandboxPath = sandboxBase.getRelative(Integer.toString(policy.getId())); + sandboxPath.createDirectory(); - // Each sandboxed action runs in its own directory so we don't need to make the temp directory's - // name unique (like we have to with standalone execution strategy). - // - // Note that, for sandboxfs-based executions, this temp directory lives outside of the sandboxfs - // instance. This is perfectly fine (because sandbox-exec controls accesses to this directory) - // and is actually desirable for performance reasons. - Path tmpDir = sandboxPath.getRelative("tmp"); + // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like + // the normal execroot does. + Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName()); + sandboxExecRoot.getParentDirectory().createDirectory(); + sandboxExecRoot.createDirectory(); Map<String, String> environment = - localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir.getPathString()); + localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, "/tmp"); final HashSet<Path> writableDirs = new HashSet<>(alwaysWritableDirs); ImmutableSet<Path> extraWritableDirs = getWritableDirs(sandboxExecRoot, environment); @@ -288,7 +287,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { } }; } - return runSpawn(spawn, sandbox, policy, execRoot, tmpDir, timeout, statisticsPath); + return runSpawn(spawn, sandbox, policy, execRoot, timeout, statisticsPath); } private void writeConfig( 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 fc8a3c3fad..a4c05ea5a7 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 @@ -33,6 +33,6 @@ final class DarwinSandboxedStrategy extends AbstractSpawnStrategy { @Override public String toString() { - return "sandboxed"; + return "darwin-sandbox"; } } 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 index 5e0a8d8acf..f2aedf8615 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java @@ -77,6 +77,7 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { private final Path execRoot; private final boolean allowNetwork; private final Path linuxSandbox; + private final Path sandboxBase; private final Path inaccessibleHelperFile; private final Path inaccessibleHelperDir; private final LocalEnvProvider localEnvProvider; @@ -101,12 +102,13 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { Path inaccessibleHelperDir, Duration timeoutKillDelay, @Nullable SandboxfsProcess sandboxfsProcess) { - super(cmdEnv, sandboxBase); + super(cmdEnv); this.fileSystem = cmdEnv.getRuntime().getFileSystem(); this.blazeDirs = cmdEnv.getDirectories(); this.execRoot = cmdEnv.getExecRoot(); this.allowNetwork = SandboxHelpers.shouldAllowNetwork(cmdEnv.getOptions()); this.linuxSandbox = LinuxSandboxUtil.getLinuxSandbox(cmdEnv); + this.sandboxBase = sandboxBase; this.inaccessibleHelperFile = inaccessibleHelperFile; this.inaccessibleHelperDir = inaccessibleHelperDir; this.timeoutKillDelay = timeoutKillDelay; @@ -117,21 +119,18 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { @Override protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy) throws IOException, ExecException, InterruptedException { - // Each invocation of "exec" gets its own sandbox. - Path sandboxPath = getSandboxRoot(); - Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName()); - sandboxExecRoot.createDirectoryAndParents(); + // Each invocation of "exec" gets its own sandbox base, execroot and temporary directory. + Path sandboxPath = sandboxBase.getRelative(Integer.toString(policy.getId())); + sandboxPath.createDirectory(); - // Each sandboxed action runs in its own directory so we don't need to make the temp directory's - // name unique (like we have to with standalone execution strategy). - // - // Note that, for sandboxfs-based executions, this temp directory lives outside of the sandboxfs - // instance. This is perfectly fine (because linux-sandbox controls accesses to this directory) - // and is actually desirable for performance reasons. - Path tmpDir = sandboxPath.getRelative("tmp"); + // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like + // the normal execroot does. + Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName()); + sandboxExecRoot.getParentDirectory().createDirectory(); + sandboxExecRoot.createDirectory(); Map<String, String> environment = - localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir.getPathString()); + localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, "/tmp"); ImmutableSet<Path> writableDirs = getWritableDirs(sandboxExecRoot, environment); ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn); @@ -144,12 +143,13 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { .setBindMounts(getReadOnlyBindMounts(blazeDirs, sandboxExecRoot)) .setUseFakeHostname(getSandboxOptions().sandboxFakeHostname) .setCreateNetworkNamespace(!(allowNetwork || Spawns.requiresNetwork(spawn))) - .setUseDebugMode(getSandboxOptions().sandboxDebug); + .setUseDebugMode(getSandboxOptions().sandboxDebug) + .setKillDelay(timeoutKillDelay); if (!timeout.isZero()) { commandLineBuilder.setTimeout(timeout); } - commandLineBuilder.setKillDelay(timeoutKillDelay); + if (spawn.getExecutionInfo().containsKey(ExecutionRequirements.REQUIRES_FAKEROOT)) { commandLineBuilder.setUseFakeRoot(true); } else if (getSandboxOptions().sandboxFakeUsername) { @@ -185,7 +185,7 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { writableDirs); } - return runSpawn(spawn, sandbox, policy, execRoot, tmpDir, timeout, statisticsPath); + return runSpawn(spawn, sandbox, policy, execRoot, timeout, statisticsPath); } @Override 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 7aac514eb6..154ac7960a 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 @@ -38,7 +38,7 @@ public final class LinuxSandboxedStrategy extends AbstractSpawnStrategy { @Override public String toString() { - return "sandboxed"; + return "linux-sandbox"; } /** 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 index fdc4abfdd7..e79b2abcd7 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java @@ -35,8 +35,9 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne return OS.isPosixCompatible() && ProcessWrapperUtil.isSupported(cmdEnv); } - private final Path execRoot; private final Path processWrapper; + private final Path execRoot; + private final Path sandboxBase; private final LocalEnvProvider localEnvProvider; private final Duration timeoutKillDelay; @@ -50,29 +51,32 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne */ ProcessWrapperSandboxedSpawnRunner( CommandEnvironment cmdEnv, Path sandboxBase, String productName, Duration timeoutKillDelay) { - super(cmdEnv, sandboxBase); - this.execRoot = cmdEnv.getExecRoot(); - this.timeoutKillDelay = timeoutKillDelay; + super(cmdEnv); this.processWrapper = ProcessWrapperUtil.getProcessWrapper(cmdEnv); + this.execRoot = cmdEnv.getExecRoot(); this.localEnvProvider = OS.getCurrent() == OS.DARWIN ? new XcodeLocalEnvProvider(productName, cmdEnv.getClientEnv()) : new PosixLocalEnvProvider(cmdEnv.getClientEnv()); + this.sandboxBase = sandboxBase; + this.timeoutKillDelay = timeoutKillDelay; } @Override protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy) throws ExecException, IOException, InterruptedException { // Each invocation of "exec" gets its own sandbox. - Path sandboxPath = getSandboxRoot(); - Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName()); + Path sandboxPath = sandboxBase.getRelative(Integer.toString(policy.getId())); + sandboxPath.createDirectory(); - // Each sandboxed action runs in its own execroot, so we don't need to make the temp directory's - // name unique (like we have to with standalone execution strategy). - Path tmpDir = sandboxExecRoot.getRelative("tmp"); + // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like + // the normal execroot does. + Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName()); + sandboxExecRoot.getParentDirectory().createDirectory(); + sandboxExecRoot.createDirectory(); Map<String, String> environment = - localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir.getPathString()); + localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, "/tmp"); Duration timeout = policy.getTimeout(); ProcessWrapperUtil.CommandLineBuilder commandLineBuilder = @@ -97,7 +101,7 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne SandboxHelpers.getOutputFiles(spawn), getWritableDirs(sandboxExecRoot, environment)); - return runSpawn(spawn, sandbox, policy, execRoot, tmpDir, timeout, statisticsPath); + return runSpawn(spawn, sandbox, policy, execRoot, timeout, statisticsPath); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java index 83c279a726..096f564ad8 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java @@ -33,6 +33,6 @@ final class ProcessWrapperSandboxedStrategy extends AbstractSpawnStrategy { @Override public String toString() { - return "sandboxed"; + return "processwrapper-sandbox"; } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java index 3065393a96..a8fb190e48 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java @@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkState; import com.google.common.collect.ImmutableList; import com.google.common.eventbus.Subscribe; +import com.google.devtools.build.lib.actions.ExecutorInitException; import com.google.devtools.build.lib.buildtool.BuildRequest; import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent; import com.google.devtools.build.lib.buildtool.buildevent.BuildInterruptedEvent; @@ -28,8 +29,6 @@ import com.google.devtools.build.lib.exec.ExecutorBuilder; import com.google.devtools.build.lib.runtime.BlazeModule; import com.google.devtools.build.lib.runtime.Command; import com.google.devtools.build.lib.runtime.CommandEnvironment; -import com.google.devtools.build.lib.util.AbruptExitException; -import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -67,14 +66,16 @@ public final class SandboxModule extends BlazeModule { } /** Computes the path to the sandbox base tree for the given running command. */ - private static Path computeSandboxBase(SandboxOptions options, CommandEnvironment env) { + private static Path computeSandboxBase(SandboxOptions options, CommandEnvironment env) + throws IOException { if (options.sandboxBase.isEmpty()) { return env.getOutputBase().getRelative("sandbox"); } else { String dirName = String.format("%s-sandbox.%s", env.getRuntime().getProductName(), Fingerprint.md5Digest(env.getOutputBase().toString())); FileSystem fileSystem = env.getRuntime().getFileSystem(); - return fileSystem.getPath(options.sandboxBase).getRelative(dirName); + Path resolvedSandboxBase = fileSystem.getPath(options.sandboxBase).resolveSymbolicLinks(); + return resolvedSandboxBase.getRelative(dirName); } } @@ -91,18 +92,30 @@ public final class SandboxModule extends BlazeModule { } @Override - public void executorInit( - CommandEnvironment cmdEnv, BuildRequest request, ExecutorBuilder builder) { + public void executorInit(CommandEnvironment cmdEnv, BuildRequest request, ExecutorBuilder builder) + throws ExecutorInitException { checkNotNull(env, "env not initialized; was beforeCommand called?"); SandboxOptions options = env.getOptions().getOptions(SandboxOptions.class); checkNotNull(options, "We were told to initialize the executor but the SandboxOptions are " + "not present; were they registered for all build commands?"); - sandboxBase = computeSandboxBase(options, env); + try { + sandboxBase = computeSandboxBase(options, env); + } catch (IOException e) { + throw new ExecutorInitException( + "--experimental_sandbox_base points to an invalid directory", e); + } ActionContextProvider provider; try { + // Ensure that each build starts with a clean sandbox base directory. Otherwise using the `id` + // that is provided by SpawnExecutionPolicy#getId to compute a base directory for a sandbox + // might result in an already existing directory. + if (sandboxBase.exists()) { + FileSystemUtils.deleteTree(sandboxBase); + } + sandboxBase.createDirectoryAndParents(); if (options.useSandboxfs) { Path mountPoint = sandboxBase.getRelative("sandboxfs"); @@ -117,11 +130,7 @@ public final class SandboxModule extends BlazeModule { provider = SandboxActionContextProvider.create(cmdEnv, sandboxBase, null); } } catch (IOException e) { - env.getBlazeModuleEnvironment().exit( - new AbruptExitException( - "Failed to initialize sandbox: " + e, - ExitCode.LOCAL_ENVIRONMENTAL_ERROR)); - return; + throw new ExecutorInitException("Failed to initialize sandbox", e); } builder.addActionContextProvider(provider); builder.addActionContextConsumer(new SandboxActionContextConsumer(cmdEnv)); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java index f4d41e38a3..d2806db1a1 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java @@ -15,14 +15,13 @@ package com.google.devtools.build.lib.sandbox; import com.google.common.base.Preconditions; -import com.google.devtools.build.lib.vfs.FileStatus; +import com.google.common.collect.Iterables; import com.google.devtools.build.lib.vfs.FileSystemUtils; 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 java.io.IOException; import java.util.Collection; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -75,36 +74,8 @@ public class SymlinkedSandboxedSpawn implements SandboxedSpawn { @Override public void createFileSystem() throws IOException { - Set<Path> createdDirs = new HashSet<>(); - cleanFileSystem(inputs.keySet()); - createDirectoryAndParentsWithCache(createdDirs, sandboxExecRoot); - createParentDirectoriesForInputs(createdDirs, inputs.keySet()); + createDirectories(); createInputs(inputs); - createWritableDirectories(createdDirs, writableDirs); - createDirectoriesForOutputs(createdDirs, outputs); - } - - private void cleanFileSystem(Set<PathFragment> allowedFiles) throws IOException { - if (sandboxExecRoot.exists(Symlinks.NOFOLLOW)) { - deleteExceptAllowedFiles(sandboxExecRoot, allowedFiles); - } - } - - private void deleteExceptAllowedFiles(Path root, Set<PathFragment> allowedFiles) - throws IOException { - for (Path p : root.getDirectoryEntries()) { - FileStatus stat = p.stat(Symlinks.NOFOLLOW); - if (!stat.isDirectory()) { - if (!allowedFiles.contains(p.relativeTo(sandboxExecRoot))) { - p.delete(); - } - } else { - deleteExceptAllowedFiles(p, allowedFiles); - if (p.readdir(Symlinks.NOFOLLOW).isEmpty()) { - p.delete(); - } - } - } } /** @@ -119,29 +90,32 @@ public class SymlinkedSandboxedSpawn implements SandboxedSpawn { * directories, too, because we'll get an IOException with EEXIST if inputs happen to be nested * once we start creating the symlinks for all inputs. */ - private void createParentDirectoriesForInputs(Set<Path> createdDirs, Set<PathFragment> inputs) - throws IOException { - for (PathFragment inputPath : inputs) { - Path dir = sandboxExecRoot.getRelative(inputPath).getParentDirectory(); - Preconditions.checkArgument( - dir.startsWith(sandboxExecRoot), "Bad relative path: '%s'", inputPath); - createDirectoryAndParentsWithCache(createdDirs, dir); + private void createDirectories() throws IOException { + LinkedHashSet<Path> dirsToCreate = new LinkedHashSet<>(); + + for (PathFragment path : Iterables.concat(inputs.keySet(), outputs)) { + Preconditions.checkArgument(!path.isAbsolute()); + Preconditions.checkArgument(!path.containsUplevelReferences()); + for (int i = 0; i < path.segmentCount(); i++) { + dirsToCreate.add(sandboxExecRoot.getRelative(path.subFragment(0, i))); + } + } + + for (Path path : dirsToCreate) { + path.createDirectory(); + } + + for (Path dir : writableDirs) { + if (dir.startsWith(sandboxExecRoot)) { + dir.createDirectoryAndParents(); + } } } - private void createInputs(Map<PathFragment, Path> inputs) throws IOException { + protected void createInputs(Map<PathFragment, Path> inputs) throws IOException { // All input files are relative to the execroot. for (Entry<PathFragment, Path> entry : inputs.entrySet()) { Path key = sandboxExecRoot.getRelative(entry.getKey()); - FileStatus keyStat = key.statNullable(Symlinks.NOFOLLOW); - if (keyStat != null) { - if (keyStat.isSymbolicLink() - && entry.getValue() != null - && key.readSymbolicLink().equals(entry.getValue().asFragment())) { - continue; - } - key.delete(); - } // A null value means that we're supposed to create an empty file as the input. if (entry.getValue() != null) { key.createSymbolicLink(entry.getValue()); @@ -151,24 +125,6 @@ public class SymlinkedSandboxedSpawn implements SandboxedSpawn { } } - private void createWritableDirectories(Set<Path> createdDirs, Set<Path> writableDirs) - throws IOException { - for (Path writablePath : writableDirs) { - if (writablePath.startsWith(sandboxExecRoot)) { - createDirectoryAndParentsWithCache(createdDirs, writablePath); - } - } - } - - /** Prepare the output directories in the sandbox. */ - private void createDirectoriesForOutputs(Set<Path> createdDirs, Collection<PathFragment> outputs) - throws IOException { - for (PathFragment output : outputs) { - createDirectoryAndParentsWithCache( - createdDirs, sandboxExecRoot.getRelative(output.getParentDirectory())); - } - } - @Override public void copyOutputs(Path execRoot) throws IOException { SandboxedSpawn.moveOutputs(outputs, sandboxExecRoot, execRoot); @@ -188,11 +144,4 @@ public class SymlinkedSandboxedSpawn implements SandboxedSpawn { // on here. } } - - private static void createDirectoryAndParentsWithCache(Set<Path> cache, Path dir) - throws IOException { - if (cache.add(dir)) { - dir.createDirectoryAndParents(); - } - } } |