diff options
Diffstat (limited to 'src/main/java/com')
11 files changed, 256 insertions, 127 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/exec/apple/XCodeLocalEnvProvider.java b/src/main/java/com/google/devtools/build/lib/exec/apple/XCodeLocalEnvProvider.java index 57f1082978..e979958a17 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/apple/XCodeLocalEnvProvider.java +++ b/src/main/java/com/google/devtools/build/lib/exec/apple/XCodeLocalEnvProvider.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.exec.apple; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.exec.local.LocalEnvProvider; import com.google.devtools.build.lib.rules.apple.AppleConfiguration; @@ -48,16 +49,19 @@ public final class XCodeLocalEnvProvider implements LocalEnvProvider { @Override public Map<String, String> rewriteLocalEnv( - Map<String, String> env, Path execRoot, String productName) throws IOException { + Map<String, String> env, Path execRoot, Path tmpDir, String productName) throws IOException { boolean containsXcodeVersion = env.containsKey(AppleConfiguration.XCODE_VERSION_ENV_NAME); boolean containsAppleSdkVersion = env.containsKey(AppleConfiguration.APPLE_SDK_VERSION_ENV_NAME); + + ImmutableMap.Builder<String, String> newEnvBuilder = ImmutableMap.builder(); + newEnvBuilder.putAll(Maps.filterKeys(env, k -> !k.equals("TMPDIR"))); + newEnvBuilder.put("TMPDIR", tmpDir.getPathString()); + if (!containsXcodeVersion && !containsAppleSdkVersion) { - return env; + return newEnvBuilder.build(); } - ImmutableMap.Builder<String, String> newEnvBuilder = ImmutableMap.builder(); - newEnvBuilder.putAll(env); // Empty developer dir indicates to use the system default. // TODO(bazel-team): Bazel's view of the xcode version and developer dir should be explicitly // set for build hermeticity. @@ -78,6 +82,7 @@ public final class XCodeLocalEnvProvider implements LocalEnvProvider { "SDKROOT", getSdkRoot(execRoot, developerDir, iosSdkVersion, appleSdkPlatform, productName)); } + return newEnvBuilder.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalEnvProvider.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalEnvProvider.java index 52610761c5..e0bda380c3 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalEnvProvider.java +++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalEnvProvider.java @@ -13,6 +13,8 @@ // limitations under the License. package com.google.devtools.build.lib.exec.local; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; import java.util.Map; @@ -22,15 +24,46 @@ import java.util.Map; * probably should not exist, but is currently necessary for our local MacOS support. */ public interface LocalEnvProvider { - public static final LocalEnvProvider UNMODIFIED = new LocalEnvProvider() { - @Override - public Map<String, String> rewriteLocalEnv( - Map<String, String> env, Path execRoot, String productName) throws IOException { - return env; - } - }; + + public static final LocalEnvProvider UNMODIFIED = + new LocalEnvProvider() { + @Override + public Map<String, String> rewriteLocalEnv( + Map<String, String> env, Path execRoot, Path tmpDir, String productName) + throws IOException { + return env; + } + }; + + public static final LocalEnvProvider ADD_TEMP_POSIX = + new LocalEnvProvider() { + @Override + public Map<String, String> rewriteLocalEnv( + Map<String, String> env, Path execRoot, Path tmpDir, String productName) + throws IOException { + ImmutableMap.Builder<String, String> result = ImmutableMap.builder(); + result.putAll(Maps.filterKeys(env, k -> !k.equals("TMPDIR"))); + result.put("TMPDIR", tmpDir.getPathString()); + return result.build(); + } + }; + + public static final LocalEnvProvider ADD_TEMP_WINDOWS = + new LocalEnvProvider() { + @Override + public Map<String, String> rewriteLocalEnv( + Map<String, String> env, Path execRoot, Path tmpDir, String productName) + throws IOException { + ImmutableMap.Builder<String, String> result = ImmutableMap.builder(); + result.putAll(Maps.filterKeys(env, k -> !k.equals("TMP") && !k.equals("TEMP"))); + String tmpPath = tmpDir.getPathString().replace('/', '\\'); + result.put("TMP", tmpPath); + result.put("TEMP", tmpPath); + return result.build(); + } + }; /** Rewrites the environment if necessary. */ - Map<String, String> rewriteLocalEnv(Map<String, String> env, Path execRoot, String productName) - throws IOException; + Map<String, String> rewriteLocalEnv( + Map<String, String> env, Path execRoot, Path tmpDir, String productName) throws IOException; } diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java index 09b69604f3..6aadb75dba 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.exec.local; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.logging.Level.INFO; import static java.util.logging.Level.SEVERE; +import static java.util.logging.Level.WARNING; import com.google.common.base.Joiner; import com.google.common.io.ByteStreams; @@ -37,6 +38,7 @@ import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.OsUtils; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.util.io.FileOutErr; +import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; import java.io.OutputStream; @@ -127,6 +129,23 @@ public final class LocalSpawnRunner implements SpawnRunner { } } + private static Path createActionTemp(Path execRoot) throws IOException { + // Use this executor thread's ID as the directory name's suffix. + // The ID is safe for the following reasons: + // - being a thread ID, it's guaranteed to be unique among other action executor threads + // - this thread will only execute one action at a time, so there's no risk of concurrently + // running actions using the same temp directory. + // The next action that this thread executes can reuse the temp directory name. The only caveat + // is, if {@link #start} fails to delete directory after the action is done, the next + // action will see stale files in its temp directory. + String idStr = Long.toHexString(Thread.currentThread().getId()); + Path result = execRoot.getRelative("tmp" + idStr); + if (!result.exists() && !result.createDirectory()) { + throw new IOException(String.format("Could not create temp directory '%s'", result)); + } + return result; + } + private final class SubprocessHandler { private final Spawn spawn; private final SpawnExecutionPolicy policy; @@ -234,76 +253,103 @@ public final class LocalSpawnRunner implements SpawnRunner { stepLog(INFO, "running locally"); setState(State.LOCAL_ACTION_RUNNING); - Command cmd; - OutputStream stdOut = ByteStreams.nullOutputStream(); - OutputStream stdErr = ByteStreams.nullOutputStream(); - if (useProcessWrapper) { - // If the process wrapper is enabled, we use its timeout feature, which first interrupts the - // subprocess and only kills it after a grace period so that the subprocess can output a - // stack trace, test log or similar, which is incredibly helpful for debugging. The process - // wrapper also supports output file redirection, so we don't need to stream the output - // through this process. - List<String> cmdLine = new ArrayList<>(); - cmdLine.add(processWrapper); - cmdLine.add("--timeout=" + policy.getTimeout().getSeconds()); - cmdLine.add("--kill_delay=" + localExecutionOptions.localSigkillGraceSeconds); - cmdLine.add("--stdout=" + getPathOrDevNull(outErr.getOutputPath())); - cmdLine.add("--stderr=" + getPathOrDevNull(outErr.getErrorPath())); - cmdLine.addAll(spawn.getArguments()); - cmd = new Command( - cmdLine.toArray(new String[0]), - localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName), - execRoot.getPathFile()); - } else { - stdOut = outErr.getOutputStream(); - stdErr = outErr.getErrorStream(); - cmd = new Command( - spawn.getArguments().toArray(new String[0]), - localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName), - execRoot.getPathFile(), - policy.getTimeout()); - } - - long startTime = System.currentTimeMillis(); - CommandResult result; + Path tmpDir = createActionTemp(execRoot); try { - result = cmd.execute(stdOut, stdErr); - if (Thread.currentThread().isInterrupted()) { - throw new InterruptedException(); + Command cmd; + OutputStream stdOut = ByteStreams.nullOutputStream(); + OutputStream stdErr = ByteStreams.nullOutputStream(); + if (useProcessWrapper) { + // If the process wrapper is enabled, we use its timeout feature, which first interrupts + // the subprocess and only kills it after a grace period so that the subprocess can output + // a stack trace, test log or similar, which is incredibly helpful for debugging. The + // process wrapper also supports output file redirection, so we don't need to stream the + // output through this process. + List<String> cmdLine = new ArrayList<>(); + cmdLine.add(processWrapper); + cmdLine.add("--timeout=" + policy.getTimeout().getSeconds()); + cmdLine.add("--kill_delay=" + localExecutionOptions.localSigkillGraceSeconds); + cmdLine.add("--stdout=" + getPathOrDevNull(outErr.getOutputPath())); + cmdLine.add("--stderr=" + getPathOrDevNull(outErr.getErrorPath())); + cmdLine.addAll(spawn.getArguments()); + cmd = + new Command( + cmdLine.toArray(new String[0]), + localEnvProvider.rewriteLocalEnv( + spawn.getEnvironment(), execRoot, tmpDir, productName), + execRoot.getPathFile()); + } else { + stdOut = outErr.getOutputStream(); + stdErr = outErr.getErrorStream(); + cmd = + new Command( + spawn.getArguments().toArray(new String[0]), + localEnvProvider.rewriteLocalEnv( + spawn.getEnvironment(), execRoot, tmpDir, productName), + execRoot.getPathFile(), + policy.getTimeout()); } - } catch (AbnormalTerminationException e) { - if (Thread.currentThread().isInterrupted()) { - throw new InterruptedException(); + + long startTime = System.currentTimeMillis(); + CommandResult result = null; + try { + result = cmd.execute(stdOut, stdErr); + if (Thread.currentThread().isInterrupted()) { + throw new InterruptedException(); + } + } catch (AbnormalTerminationException e) { + if (Thread.currentThread().isInterrupted()) { + throw new InterruptedException(); + } + result = e.getResult(); + } catch (CommandException e) { + // At the time this comment was written, this must be a ExecFailedException encapsulating + // an IOException from the underlying Subprocess.Factory. + String msg = e.getMessage() == null ? e.getClass().getName() : e.getMessage(); + setState(State.PERMANENT_ERROR); + outErr + .getErrorStream() + .write(("Action failed to execute: " + msg + "\n").getBytes(UTF_8)); + outErr.getErrorStream().flush(); + return new SpawnResult.Builder() + .setStatus(Status.EXECUTION_FAILED) + .setExitCode(LOCAL_EXEC_ERROR) + .setExecutorHostname(hostName) + .build(); } - result = e.getResult(); - } catch (CommandException e) { - // At the time this comment was written, this must be a ExecFailedException encapsulating an - // IOException from the underlying Subprocess.Factory. - String msg = e.getMessage() == null ? e.getClass().getName() : e.getMessage(); - setState(State.PERMANENT_ERROR); - outErr.getErrorStream().write(("Action failed to execute: " + msg + "\n").getBytes(UTF_8)); - outErr.getErrorStream().flush(); + setState(State.SUCCESS); + long wallTimeMillis = System.currentTimeMillis() - startTime; + boolean wasTimeout = + result.getTerminationStatus().timedout() + || (useProcessWrapper && wasTimeout(policy.getTimeout(), wallTimeMillis)); + Status status = wasTimeout ? Status.TIMEOUT : Status.SUCCESS; + int exitCode = + status == Status.TIMEOUT + ? POSIX_TIMEOUT_EXIT_CODE + : result.getTerminationStatus().getRawExitCode(); return new SpawnResult.Builder() - .setStatus(Status.EXECUTION_FAILED) - .setExitCode(LOCAL_EXEC_ERROR) + .setStatus(status) + .setExitCode(exitCode) .setExecutorHostname(hostName) + .setWallTimeMillis(wallTimeMillis) .build(); + } finally { + // Delete the temp directory tree, so the next action that this thread executes will get a + // fresh, empty temp directory. + // File deletion tends to be slow on Windows, so deleting this tree may take several + // seconds. Delete it after having measured the wallTimeMillis. + try { + FileSystemUtils.deleteTree(tmpDir); + } catch (IOException ignored) { + // We can't handle this exception in any meaningful way, nor should we, but let's log it. + stepLog( + WARNING, + String.format( + "failed to delete temp directory '%s'; this might indicate that the action " + + "created subprocesses that didn't terminate and hold files open in that " + + "directory", + tmpDir)); + } } - setState(State.SUCCESS); - - long wallTimeMillis = System.currentTimeMillis() - startTime; - boolean wasTimeout = result.getTerminationStatus().timedout() - || (useProcessWrapper && wasTimeout(policy.getTimeout(), wallTimeMillis)); - Status status = wasTimeout ? Status.TIMEOUT : Status.SUCCESS; - int exitCode = status == Status.TIMEOUT - ? POSIX_TIMEOUT_EXIT_CODE - : result.getTerminationStatus().getRawExitCode(); - return new SpawnResult.Builder() - .setStatus(status) - .setExitCode(exitCode) - .setExecutorHostname(hostName) - .setWallTimeMillis(wallTimeMillis) - .build(); } private String getPathOrDevNull(Path path) { 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 7eac6bb6be..b538fed1c6 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 @@ -77,6 +77,9 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { } } + // TODO(laszlocsomor): refactor this class to make `actuallyExec`'s contract clearer: the caller + // of `actuallyExec` should not depend on `actuallyExec` calling `runSpawn` because it's easy to + // forget to do so in `actuallyExec`'s implementations. protected abstract SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy) throws ExecException, InterruptedException, IOException; @@ -85,14 +88,15 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { SandboxedSpawn sandbox, SpawnExecutionPolicy policy, Path execRoot, + Path tmpDir, Duration timeout) - throws ExecException, IOException, InterruptedException { + throws ExecException, IOException, InterruptedException { try { sandbox.createFileSystem(); OutErr outErr = policy.getFileOutErr(); policy.prefetchInputs(); - SpawnResult result = run(sandbox, outErr, timeout); + SpawnResult result = run(sandbox, outErr, timeout, tmpDir); policy.lockOutputFiles(); try { @@ -126,7 +130,8 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { } } - private final SpawnResult run(SandboxedSpawn sandbox, OutErr outErr, Duration timeout) + private final SpawnResult run( + SandboxedSpawn sandbox, OutErr outErr, Duration timeout, Path tmpDir) throws IOException, InterruptedException { Command cmd = new Command( sandbox.getArguments().toArray(new String[0]), @@ -136,6 +141,9 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { long startTime = System.currentTimeMillis(); CommandResult result; try { + if (!tmpDir.exists() && !tmpDir.createDirectory()) { + throw new IOException(String.format("Could not create temp directory '%s'", tmpDir)); + } result = cmd.execute(outErr.getOutputStream(), outErr.getErrorStream()); if (Thread.currentThread().isInterrupted()) { throw new InterruptedException(); @@ -190,8 +198,8 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { * * @throws IOException because we might resolve symlinks, which throws {@link IOException}. */ - protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env) - throws IOException { + protected ImmutableSet<Path> getWritableDirs( + Path sandboxExecRoot, Map<String, String> env, Path tmpDir) throws IOException { // We have to make the TEST_TMPDIR directory writable if it is specified. ImmutableSet.Builder<Path> writablePaths = ImmutableSet.builder(); writablePaths.add(sandboxExecRoot); @@ -208,6 +216,8 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { } } + writablePaths.add(tmpDir); + FileSystem fileSystem = sandboxExecRoot.getFileSystem(); for (String writablePath : sandboxOptions.sandboxWritablePath) { Path path = fileSystem.getPath(writablePath); 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 6227ac4915..cc3150db9f 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 @@ -128,15 +128,17 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { HashSet<Path> writableDirs = new HashSet<>(); addPathToSetIfExists(fs, writableDirs, "/dev"); - addPathToSetIfExists(fs, writableDirs, System.getenv("TMPDIR")); addPathToSetIfExists(fs, writableDirs, "/tmp"); addPathToSetIfExists(fs, writableDirs, "/private/tmp"); addPathToSetIfExists(fs, writableDirs, "/private/var/tmp"); - // On macOS, in addition to what is specified in $TMPDIR, two other temporary directories may be - // written to by processes. We have to get their location by calling "getconf". + // On macOS, processes may write to not only $TMPDIR but also to two other temporary + // directories. We have to get their location by calling "getconf". addPathToSetIfExists(fs, writableDirs, getConfStr("DARWIN_USER_TEMP_DIR")); addPathToSetIfExists(fs, writableDirs, getConfStr("DARWIN_USER_CACHE_DIR")); + // We don't add any value for $TMPDIR here, instead we compute its value later in + // {@link #actuallyExec} and add it as a writable directory in + // {@link AbstractSandboxSpawnRunner#getWritableDirs}. // ~/Library/Cache and ~/Library/Logs need to be writable (cf. issue #2231). Path homeDir = fs.getPath(System.getProperty("user.home")); @@ -173,11 +175,16 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { Path sandboxPath = getSandboxRoot(); Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName()); + // 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"); + Map<String, String> spawnEnvironment = - localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName); + localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir, productName); final HashSet<Path> writableDirs = new HashSet<>(alwaysWritableDirs); - ImmutableSet<Path> extraWritableDirs = getWritableDirs(sandboxExecRoot, spawnEnvironment); + ImmutableSet<Path> extraWritableDirs = + getWritableDirs(sandboxExecRoot, spawnEnvironment, tmpDir); writableDirs.addAll(extraWritableDirs); ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn); @@ -187,7 +194,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { List<String> arguments = computeCommandLine(spawn, timeout, sandboxConfigPath, timeoutGraceSeconds); Map<String, String> environment = - localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName); + localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir, productName); boolean allowNetworkForThisSpawn = allowNetwork || SandboxHelpers.shouldAllowNetwork(spawn); SandboxedSpawn sandbox = new SymlinkedSandboxedSpawn( @@ -205,7 +212,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { sandboxConfigPath, writableDirs, getInaccessiblePaths(), allowNetworkForThisSpawn); } }; - return runSpawn(spawn, sandbox, policy, execRoot, timeout); + return runSpawn(spawn, sandbox, policy, execRoot, tmpDir, timeout); } private List<String> computeCommandLine( 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 fb8a21e884..4246735919 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 @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.analysis.BlazeDirectories; +import com.google.devtools.build.lib.exec.local.LocalEnvProvider; import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.shell.Command; import com.google.devtools.build.lib.shell.CommandException; @@ -86,22 +87,27 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { private final Path linuxSandbox; private final Path inaccessibleHelperFile; private final Path inaccessibleHelperDir; + private final LocalEnvProvider localEnvProvider; private final int timeoutGraceSeconds; + private final String productName; LinuxSandboxedSpawnRunner( CommandEnvironment cmdEnv, Path sandboxBase, + String productName, Path inaccessibleHelperFile, Path inaccessibleHelperDir, int timeoutGraceSeconds) { super(cmdEnv, sandboxBase); this.blazeDirs = cmdEnv.getDirectories(); this.execRoot = cmdEnv.getExecRoot(); + this.productName = productName; this.allowNetwork = SandboxHelpers.shouldAllowNetwork(cmdEnv.getOptions()); this.linuxSandbox = getLinuxSandbox(cmdEnv); this.inaccessibleHelperFile = inaccessibleHelperFile; this.inaccessibleHelperDir = inaccessibleHelperDir; this.timeoutGraceSeconds = timeoutGraceSeconds; + this.localEnvProvider = LocalEnvProvider.ADD_TEMP_POSIX; } @Override @@ -111,7 +117,11 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { Path sandboxPath = getSandboxRoot(); Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName()); - Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, spawn.getEnvironment()); + // 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"); + + Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, spawn.getEnvironment(), tmpDir); ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn); Duration timeout = policy.getTimeout(); List<String> arguments = @@ -124,16 +134,19 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { getReadOnlyBindMounts(blazeDirs, sandboxExecRoot), allowNetwork || SandboxHelpers.shouldAllowNetwork(spawn), spawn.getExecutionInfo().containsKey("requires-fakeroot")); - - SandboxedSpawn sandbox = new SymlinkedSandboxedSpawn( - sandboxPath, - sandboxExecRoot, - arguments, - spawn.getEnvironment(), - SandboxHelpers.getInputFiles(spawn, policy, execRoot), - outputs, - writableDirs); - return runSpawn(spawn, sandbox, policy, execRoot, timeout); + Map<String, String> environment = + localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir, productName); + + SandboxedSpawn sandbox = + new SymlinkedSandboxedSpawn( + sandboxPath, + sandboxExecRoot, + arguments, + environment, + SandboxHelpers.getInputFiles(spawn, policy, execRoot), + outputs, + writableDirs); + return runSpawn(spawn, sandbox, policy, execRoot, tmpDir, timeout); } private List<String> computeCommandLine( @@ -214,10 +227,10 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { } @Override - protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env) - throws IOException { + protected ImmutableSet<Path> getWritableDirs( + Path sandboxExecRoot, Map<String, String> env, Path tmpDir) throws IOException { ImmutableSet.Builder<Path> writableDirs = ImmutableSet.builder(); - writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, env)); + writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, env, tmpDir)); FileSystem fs = sandboxExecRoot.getFileSystem(); writableDirs.add(fs.getPath("/dev/shm").resolveSymbolicLinks()); 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 2f86fa92d3..7a844317fb 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 @@ -40,10 +40,8 @@ public final class LinuxSandboxedStrategy extends AbstractSpawnStrategy { } static LinuxSandboxedSpawnRunner create( - CommandEnvironment cmdEnv, - Path sandboxBase, - int timeoutGraceSeconds) - throws IOException { + CommandEnvironment cmdEnv, Path sandboxBase, String productName, int timeoutGraceSeconds) + throws IOException { Path inaccessibleHelperFile = sandboxBase.getRelative("inaccessibleHelperFile"); FileSystemUtils.touchFile(inaccessibleHelperFile); inaccessibleHelperFile.setReadable(false); @@ -59,6 +57,7 @@ public final class LinuxSandboxedStrategy extends AbstractSpawnStrategy { return new LinuxSandboxedSpawnRunner( cmdEnv, sandboxBase, + productName, inaccessibleHelperFile, inaccessibleHelperDir, timeoutGraceSeconds); 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 1ec4a999a8..de5fd8c95d 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 @@ -50,9 +50,10 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne this.productName = productName; this.timeoutGraceSeconds = timeoutGraceSeconds; this.processWrapper = ProcessWrapperRunner.getProcessWrapper(cmdEnv); - this.localEnvProvider = OS.getCurrent() == OS.DARWIN - ? new XCodeLocalEnvProvider() - : LocalEnvProvider.UNMODIFIED; + this.localEnvProvider = + OS.getCurrent() == OS.DARWIN + ? new XCodeLocalEnvProvider() + : LocalEnvProvider.ADD_TEMP_POSIX; } @Override @@ -62,22 +63,28 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne Path sandboxPath = getSandboxRoot(); Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName()); + // 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"); + Duration timeout = policy.getTimeout(); List<String> arguments = ProcessWrapperRunner.getCommandLine( processWrapper, spawn.getArguments(), timeout, timeoutGraceSeconds); Map<String, String> environment = - localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName); + localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir, productName); + + SandboxedSpawn sandbox = + new SymlinkedSandboxedSpawn( + sandboxPath, + sandboxExecRoot, + arguments, + environment, + SandboxHelpers.getInputFiles(spawn, policy, execRoot), + SandboxHelpers.getOutputFiles(spawn), + getWritableDirs(sandboxExecRoot, spawn.getEnvironment(), tmpDir)); - SandboxedSpawn sandbox = new SymlinkedSandboxedSpawn( - sandboxPath, - sandboxExecRoot, - arguments, - environment, - SandboxHelpers.getInputFiles(spawn, policy, execRoot), - SandboxHelpers.getOutputFiles(spawn), - getWritableDirs(sandboxExecRoot, spawn.getEnvironment())); - return runSpawn(spawn, sandbox, policy, execRoot, timeout); + return runSpawn(spawn, sandbox, policy, execRoot, tmpDir, timeout); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java index f92bceef64..53173e4bea 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java @@ -66,7 +66,8 @@ final class SandboxActionContextProvider extends ActionContextProvider { if (LinuxSandboxedSpawnRunner.isSupported(cmdEnv)) { SpawnRunner spawnRunner = withFallback( - cmdEnv, LinuxSandboxedStrategy.create(cmdEnv, sandboxBase, timeoutGraceSeconds)); + cmdEnv, + LinuxSandboxedStrategy.create(cmdEnv, sandboxBase, productName, timeoutGraceSeconds)); contexts.add(new LinuxSandboxedStrategy(spawnRunner)); } @@ -90,9 +91,10 @@ final class SandboxActionContextProvider extends ActionContextProvider { private static SpawnRunner createFallbackRunner(CommandEnvironment env) { LocalExecutionOptions localExecutionOptions = env.getOptions().getOptions(LocalExecutionOptions.class); - LocalEnvProvider localEnvProvider = OS.getCurrent() == OS.DARWIN - ? new XCodeLocalEnvProvider() - : LocalEnvProvider.UNMODIFIED; + LocalEnvProvider localEnvProvider = + OS.getCurrent() == OS.DARWIN + ? new XCodeLocalEnvProvider() + : LocalEnvProvider.ADD_TEMP_POSIX; return new LocalSpawnRunner( env.getExecRoot(), diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextProvider.java index 040f706592..15afb0c4da 100644 --- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextProvider.java @@ -100,9 +100,12 @@ public class StandaloneActionContextProvider extends ActionContextProvider { private static SpawnRunner createLocalRunner(CommandEnvironment env) { LocalExecutionOptions localExecutionOptions = env.getOptions().getOptions(LocalExecutionOptions.class); - LocalEnvProvider localEnvProvider = OS.getCurrent() == OS.DARWIN - ? new XCodeLocalEnvProvider() - : LocalEnvProvider.UNMODIFIED; + LocalEnvProvider localEnvProvider = + OS.getCurrent() == OS.DARWIN + ? new XCodeLocalEnvProvider() + : (OS.getCurrent() == OS.WINDOWS + ? LocalEnvProvider.ADD_TEMP_WINDOWS + : LocalEnvProvider.ADD_TEMP_POSIX); return new LocalSpawnRunner( env.getExecRoot(), diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java index 50f6e2dd8f..e6e9382fc6 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java @@ -55,7 +55,11 @@ final class WorkerActionContextProvider extends ActionContextProvider { LocalExecutionOptions localExecutionOptions = env.getOptions().getOptions(LocalExecutionOptions.class); LocalEnvProvider localEnvProvider = - OS.getCurrent() == OS.DARWIN ? new XCodeLocalEnvProvider() : LocalEnvProvider.UNMODIFIED; + OS.getCurrent() == OS.DARWIN + ? new XCodeLocalEnvProvider() + : (OS.getCurrent() == OS.WINDOWS + ? LocalEnvProvider.ADD_TEMP_WINDOWS + : LocalEnvProvider.ADD_TEMP_POSIX); return new LocalSpawnRunner( env.getExecRoot(), localExecutionOptions, |