From cfe5994540f20901906ce0c39d6f57d3ad02d677 Mon Sep 17 00:00:00 2001 From: laszlocsomor Date: Fri, 16 Feb 2018 05:19:36 -0800 Subject: Automated rollback of commit 04757dba0174d22c0a695a7ed5fe511fd13df008. *** Reason for rollback *** There's already a --test_tmpdir flag, and Java tests don't pick up this new one. More info: https://github.com/bazelbuild/bazel/issues/4621#issuecomment-366217321 *** Original change description *** tmpdir,local-exec: implement --local_tmp_root Add new flag called `--local_tmp_root`, which (if specified) tells Bazel what temp directory should locally executed actions use. Fixes https://github.com/bazelbuild/bazel/issues/4621 Related to https://github.com/bazelbuild/bazel/issues/3215 RELNOTES[NEW]: The new "--local_tmp_root=" flag allows specifying the temp directory for locally executed actions. Change-Id: Ice69a5e63d0bf4d3b5c9ef4dbdd1ed1c5025f85e PiperOrigin-RevId: 185982705 --- .../lib/exec/apple/XCodeLocalEnvProvider.java | 21 +++++--------- .../build/lib/exec/local/LocalEnvProvider.java | 28 ++++--------------- .../lib/exec/local/LocalExecutionOptions.java | 17 ------------ .../build/lib/exec/local/LocalSpawnRunner.java | 12 ++------ .../lib/exec/local/PosixLocalEnvProvider.java | 32 ++++++---------------- .../lib/exec/local/WindowsLocalEnvProvider.java | 16 +++-------- .../lib/sandbox/AbstractSandboxSpawnRunner.java | 9 ------ .../com/google/devtools/build/lib/sandbox/BUILD | 1 - .../lib/sandbox/DarwinSandboxedSpawnRunner.java | 6 +--- .../lib/sandbox/LinuxSandboxedSpawnRunner.java | 6 +--- .../ProcessWrapperSandboxedSpawnRunner.java | 6 +--- 11 files changed, 31 insertions(+), 123 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib') 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 0cfa27e1d1..b0acfbf4b4 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 @@ -60,11 +60,7 @@ public final class XCodeLocalEnvProvider implements LocalEnvProvider { @Override public Map rewriteLocalEnv( - Map env, - Path execRoot, - String localTmpRoot, - String fallbackTmpDir, - String productName) + Map env, Path execRoot, String fallbackTmpDir, String productName) throws IOException { boolean containsXcodeVersion = env.containsKey(AppleConfiguration.XCODE_VERSION_ENV_NAME); boolean containsAppleSdkVersion = @@ -72,16 +68,13 @@ public final class XCodeLocalEnvProvider implements LocalEnvProvider { ImmutableMap.Builder newEnvBuilder = ImmutableMap.builder(); newEnvBuilder.putAll(Maps.filterKeys(env, k -> !k.equals("TMPDIR"))); - String p = localTmpRoot; + String p = clientEnv.get("TMPDIR"); if (Strings.isNullOrEmpty(p)) { - p = clientEnv.get("TMPDIR"); - if (Strings.isNullOrEmpty(p)) { - // Do not use `fallbackTmpDir`, use `/tmp` instead. This way if the user didn't export - // TMPDIR in their environment, Bazel will still set a TMPDIR that's Posixy enough and plays - // well with heavily path-length-limited scenarios, such as the socket creation scenario - // that motivated https://github.com/bazelbuild/bazel/issues/4376. - p = "/tmp"; - } + // Do not use `fallbackTmpDir`, use `/tmp` instead. This way if the user didn't export TMPDIR + // in their environment, Bazel will still set a TMPDIR that's Posixy enough and plays well + // with heavily path-length-limited scenarios, such as the socket creation scenario that + // motivated https://github.com/bazelbuild/bazel/issues/4376. + p = "/tmp"; } newEnvBuilder.put("TMPDIR", p); 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 7da1954891..40075652de 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 @@ -27,11 +27,7 @@ public interface LocalEnvProvider { new LocalEnvProvider() { @Override public Map rewriteLocalEnv( - Map env, - Path execRoot, - String localTmpRoot, - String fallbackTmpDir, - String productName) { + Map env, Path execRoot, String fallbackTmpDir, String productName) { return env; } }; @@ -41,25 +37,13 @@ public interface LocalEnvProvider { * * @param env the Spawn's environment to rewrite * @param execRoot the path where the Spawn is executed - * @param localTmpRoot an absolute path to a temp directory that the Spawn could use. Whether the - * particular implementation of {@link LocalEnvProvider} chooses to use this path, or {@code - * fallbackTmpDir}, or some other value, is up to the implementation. Typically the - * implementations will use {@code localTmpRoot}, or if empty then use the Bazel client's - * environment's TMPDIR/TMP/TEMP value (depending on the host OS), or if empty then use the - * {@code fallbackTmpDir} or some other value (typically "/tmp"). - * @param fallbackTmpDir an absolute path to a temp directory that the Spawn could use. Whether - * the particular implementation of {@link LocalEnvProvider} chooses to use this path, or - * {@code localTmpRoot}, or some other value, is up to the implementation. Typically the - * implementations will use {@code localTmpRoot}, or if empty then use the Bazel client's - * environment's TMPDIR/TMP/TEMP value (depending on the host OS), or if empty then use the - * {@code fallbackTmpDir} or some other value (typically "/tmp"). + * @param fallbackTmpDir an absolute path to a temp directory that the Spawn could use. The + * particular implementation of {@link LocalEnvProvider} may choose to use some other path, + * typically the "TMPDIR" environment variable in the Bazel client's environment, but if + * that's unavailable, the implementation may decide to use this {@code fallbackTmpDir}. * @param productName name of the Bazel binary, e.g. "bazel" */ Map rewriteLocalEnv( - Map env, - Path execRoot, - String localTmpRoot, - String fallbackTmpDir, - String productName) + Map env, Path execRoot, String fallbackTmpDir, String productName) throws IOException; } diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalExecutionOptions.java index dfcd5d06d8..08aaa53e33 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalExecutionOptions.java +++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalExecutionOptions.java @@ -60,21 +60,4 @@ public class LocalExecutionOptions extends OptionsBase { + "locally executed actions which don't use sandboxing" ) public boolean collectLocalExecutionStatistics; - - @Option( - name = "local_tmp_root", - defaultValue = "", - documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY, - effectTags = {OptionEffectTag.EXECUTION}, - help = - "Sets the TMPDIR environment variable's value for locally executed actions. If this flag's " - + "value is not empty, Bazel exports TMPDIR (on Linux/macOS) or TMP and TEMP (on " - + "Windows) with this value for locally executed actions. (This doesn't influence " - + "action caching, as TMPDIR/TMP/TEMP are not part of the action's cache key.) If this " - + "flag's value is empty, then Bazel picks up the user-defined TMPDIR (on Linux/macOS) " - + "or TMP/TEMP (on Windows) and exports that for local actions; and if that value is " - + "also empty, Bazel exports \"/tmp\" (on Linux/macOS) or a directory in the execroot " - + "(on Windows)." - ) - public String localTmpRoot; } 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 4e3a4412ea..de2c06fea0 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 @@ -278,11 +278,7 @@ public class LocalSpawnRunner implements SpawnRunner { new Command( cmdLine.toArray(new String[0]), localEnvProvider.rewriteLocalEnv( - spawn.getEnvironment(), - execRoot, - LocalSpawnRunner.this.localExecutionOptions.localTmpRoot, - commandTmpDir.getPathString(), - productName), + spawn.getEnvironment(), execRoot, commandTmpDir.getPathString(), productName), execRoot.getPathFile()); } else { stdOut = outErr.getOutputStream(); @@ -291,11 +287,7 @@ public class LocalSpawnRunner implements SpawnRunner { new Command( spawn.getArguments().toArray(new String[0]), localEnvProvider.rewriteLocalEnv( - spawn.getEnvironment(), - execRoot, - LocalSpawnRunner.this.localExecutionOptions.localTmpRoot, - commandTmpDir.getPathString(), - productName), + spawn.getEnvironment(), execRoot, commandTmpDir.getPathString(), productName), execRoot.getPathFile(), policy.getTimeout()); } diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/PosixLocalEnvProvider.java b/src/main/java/com/google/devtools/build/lib/exec/local/PosixLocalEnvProvider.java index 2769440043..1a30d3bb57 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/local/PosixLocalEnvProvider.java +++ b/src/main/java/com/google/devtools/build/lib/exec/local/PosixLocalEnvProvider.java @@ -36,35 +36,21 @@ public final class PosixLocalEnvProvider implements LocalEnvProvider { * Compute an environment map for local actions on Unix-like platforms (e.g. Linux, macOS). * *

Returns a map with the same keys and values as {@code env}. Overrides the value of TMPDIR - * (or adds it if not present in {@code env}) by: - * - *

    - *
  • the value of {@code localTmpRoot}, or if that's empty or null, then - *
  • the value of {@code clientEnv.get("TMPDIR")}, or if that's empty or null, then - *
  • the "/tmp" (NOT by {@code fallbackTmpDir}. - *
- * - *

This method ignores {@code fallbackTmpDir}. + * (or adds it if not present in {@code env}) by the value of {@code clientEnv.get("TMPDIR")}, or + * if that's empty or null, then by "/tmp". */ @Override public Map rewriteLocalEnv( - Map env, - Path execRoot, - String localTmpRoot, - String fallbackTmpDir, - String productName) { + Map env, Path execRoot, String fallbackTmpDir, String productName) { ImmutableMap.Builder result = ImmutableMap.builder(); result.putAll(Maps.filterKeys(env, k -> !k.equals("TMPDIR"))); - String p = localTmpRoot; + String p = clientEnv.get("TMPDIR"); if (Strings.isNullOrEmpty(p)) { - p = clientEnv.get("TMPDIR"); - if (Strings.isNullOrEmpty(p)) { - // Do not use `fallbackTmpDir`, use `/tmp` instead. This way if the user didn't export - // TMPDIR in their environment, Bazel will still set a TMPDIR that's Posixy enough and plays - // well with heavily path-length-limited scenarios, such as the socket creation scenario - // that motivated https://github.com/bazelbuild/bazel/issues/4376. - p = "/tmp"; - } + // Do not use `fallbackTmpDir`, use `/tmp` instead. This way if the user didn't export TMPDIR + // in their environment, Bazel will still set a TMPDIR that's Posixy enough and plays well + // with heavily path-length-limited scenarios, such as the socket creation scenario that + // motivated https://github.com/bazelbuild/bazel/issues/4376. + p = "/tmp"; } result.put("TMPDIR", p); return result.build(); diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/WindowsLocalEnvProvider.java b/src/main/java/com/google/devtools/build/lib/exec/local/WindowsLocalEnvProvider.java index e99a8d6f6b..91e218beac 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/local/WindowsLocalEnvProvider.java +++ b/src/main/java/com/google/devtools/build/lib/exec/local/WindowsLocalEnvProvider.java @@ -39,7 +39,6 @@ public final class WindowsLocalEnvProvider implements LocalEnvProvider { * TEMP (or adds them if not present in {@code env}) by the same value, which is: * *

    - *
  • the value of {@code localTmpRoot}, or if that's empty or null, then *
  • the value of {@code clientEnv.get("TMP")}, or if that's empty or null, then *
  • the value of {@code clientEnv.get("TEMP")}, or if that's empty or null, then *
  • the value of {@code fallbackTmpDir}. @@ -49,21 +48,14 @@ public final class WindowsLocalEnvProvider implements LocalEnvProvider { */ @Override public Map rewriteLocalEnv( - Map env, - Path execRoot, - String localTmpRoot, - String fallbackTmpDir, - String productName) { + Map env, Path execRoot, String fallbackTmpDir, String productName) { ImmutableMap.Builder result = ImmutableMap.builder(); result.putAll(Maps.filterKeys(env, k -> !k.equals("TMP") && !k.equals("TEMP"))); - String p = localTmpRoot; + String p = clientEnv.get("TMP"); if (Strings.isNullOrEmpty(p)) { - p = clientEnv.get("TMP"); + p = clientEnv.get("TEMP"); if (Strings.isNullOrEmpty(p)) { - p = clientEnv.get("TEMP"); - if (Strings.isNullOrEmpty(p)) { - p = fallbackTmpDir; - } + p = fallbackTmpDir; } } p = p.replace('/', '\\'); 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 9c4f787076..6985392380 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 @@ -28,7 +28,6 @@ import com.google.devtools.build.lib.actions.SpawnResult.Status; import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.exec.SpawnRunner; -import com.google.devtools.build.lib.exec.local.LocalExecutionOptions; import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.shell.AbnormalTerminationException; import com.google.devtools.build.lib.shell.Command; @@ -43,7 +42,6 @@ import java.io.IOException; import java.time.Duration; import java.util.Map; import java.util.Optional; -import javax.annotation.Nullable; /** Abstract common ancestor for sandbox spawn runners implementing the common parts. */ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { @@ -55,14 +53,12 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { private final Path sandboxBase; private final SandboxOptions sandboxOptions; - private final String localTmpRoot; private final boolean verboseFailures; private final ImmutableSet inaccessiblePaths; public AbstractSandboxSpawnRunner(CommandEnvironment cmdEnv, Path sandboxBase) { this.sandboxBase = sandboxBase; this.sandboxOptions = cmdEnv.getOptions().getOptions(SandboxOptions.class); - this.localTmpRoot = cmdEnv.getOptions().getOptions(LocalExecutionOptions.class).localTmpRoot; this.verboseFailures = cmdEnv.getOptions().getOptions(ExecutionOptions.class).verboseFailures; this.inaccessiblePaths = sandboxOptions.getInaccessiblePaths(cmdEnv.getRuntime().getFileSystem()); @@ -297,10 +293,5 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { return sandboxOptions; } - @Nullable - protected final String getLocalTmpRoot() { - return localTmpRoot; - } - protected abstract String getName(); } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD index 243d0b5553..9d849a8fc1 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD +++ b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD @@ -31,6 +31,5 @@ java_library( "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/common/options", "//third_party:guava", - "//third_party:jsr305", ], ) 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 0f68c29eae..ebd75bf48b 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 @@ -224,11 +224,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { Map environment = localEnvProvider.rewriteLocalEnv( - spawn.getEnvironment(), - execRoot, - getLocalTmpRoot(), - tmpDir.getPathString(), - productName); + spawn.getEnvironment(), execRoot, tmpDir.getPathString(), productName); final HashSet writableDirs = new HashSet<>(alwaysWritableDirs); ImmutableSet extraWritableDirs = getWritableDirs(sandboxExecRoot, environment); 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 386dedf531..ad677f17ae 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 @@ -183,11 +183,7 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { Map environment = localEnvProvider.rewriteLocalEnv( - spawn.getEnvironment(), - execRoot, - getLocalTmpRoot(), - tmpDir.getPathString(), - productName); + spawn.getEnvironment(), execRoot, tmpDir.getPathString(), productName); Set writableDirs = getWritableDirs(sandboxExecRoot, environment); ImmutableSet outputs = SandboxHelpers.getOutputFiles(spawn); 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 ca6279fd25..18b33c2de4 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 @@ -107,11 +107,7 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne Map environment = localEnvProvider.rewriteLocalEnv( - spawn.getEnvironment(), - execRoot, - getLocalTmpRoot(), - tmpDir.getPathString(), - productName); + spawn.getEnvironment(), execRoot, tmpDir.getPathString(), productName); Duration timeout = policy.getTimeout(); ProcessWrapperUtil.CommandLineBuilder commandLineBuilder = -- cgit v1.2.3