Turn ProcessWrapperUtil helper functions into a ProcessWrapper class. This will make it easier to carry the process-wrapper configuration around. In particular, I want to add the ability to pass arbitrary extra flags to the tool, and carrying them around explicitly and separately from the process-wrapper path adds a lot of noise everywhere. This also cleans up the way we gain access to the process-wrapper binary and the checks to see if it is supported by homogenizing all logic into a single place and removing OS-specific checks from a variety of places. Part of https://github.com/bazelbuild/bazel/issues/10245. RELNOTES: None. PiperOrigin-RevId: 310908570
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index 3894ccd..0bcdb36 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
@@ -59,6 +59,7 @@ import com.google.devtools.build.lib.runtime.BlazeRuntime; import com.google.devtools.build.lib.runtime.Command; import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor; import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutorFactory; import com.google.devtools.build.lib.runtime.ServerBuilder; @@ -231,6 +232,8 @@ resolvedFileReplacingWorkspace = Optional.<RootedPath>absent(); outputVerificationRules = ImmutableSet.<String>of(); + skylarkRepositoryFunction.setProcessWrapper(ProcessWrapper.fromCommandEnvironment(env)); + RepositoryOptions repoOptions = env.getOptions().getOptions(RepositoryOptions.class); if (repoOptions != null) { repositoryCache.setHardlink(repoOptions.useHardlinks);
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java index bb0a94d..c82863a 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
@@ -48,7 +48,7 @@ import com.google.devtools.build.lib.rules.repository.RepositoryFunction; import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException; import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper; -import com.google.devtools.build.lib.runtime.ProcessWrapperUtil; +import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor; import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor.ExecutionResult; import com.google.devtools.build.lib.skylarkbuildapi.repository.SkylarkRepositoryContextApi; @@ -60,7 +60,6 @@ import com.google.devtools.build.lib.syntax.Starlark; import com.google.devtools.build.lib.syntax.StarlarkSemantics; import com.google.devtools.build.lib.syntax.StarlarkThread; -import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.OsUtils; import com.google.devtools.build.lib.util.StringUtilities; import com.google.devtools.build.lib.util.io.OutErr; @@ -106,13 +105,13 @@ private final Rule rule; private final PathPackageLocator packageLocator; private final Path outputDirectory; - private final Path embeddedBinariesRoot; private final StructImpl attrObject; private final SkylarkOS osObject; private final ImmutableSet<PathFragment> blacklistedPatterns; private final Environment env; private final DownloadManager downloadManager; private final double timeoutScaling; + @Nullable private final ProcessWrapper processWrapper; private final Map<String, String> markerData; private final StarlarkSemantics starlarkSemantics; private final RepositoryRemoteExecutor remoteExecutor; @@ -129,8 +128,8 @@ Environment environment, Map<String, String> env, DownloadManager downloadManager, - Path embeddedBinariesRoot, double timeoutScaling, + @Nullable ProcessWrapper processWrapper, Map<String, String> markerData, StarlarkSemantics starlarkSemantics, @Nullable RepositoryRemoteExecutor remoteExecutor) @@ -138,12 +137,12 @@ this.rule = rule; this.packageLocator = packageLocator; this.outputDirectory = outputDirectory; - this.embeddedBinariesRoot = embeddedBinariesRoot; this.blacklistedPatterns = blacklistedPatterns; this.env = environment; this.osObject = new SkylarkOS(env); this.downloadManager = downloadManager; this.timeoutScaling = timeoutScaling; + this.processWrapper = processWrapper; this.markerData = markerData; WorkspaceAttributeMapper attrs = WorkspaceAttributeMapper.of(rule); ImmutableMap.Builder<String, Object> attrBuilder = new ImmutableMap.Builder<>(); @@ -561,14 +560,12 @@ createDirectory(outputDirectory); long timeoutMillis = Math.round(timeout.longValue() * 1000 * timeoutScaling); - if (OS.getCurrent() != OS.WINDOWS && embeddedBinariesRoot != null) { - Path processWrapper = ProcessWrapperUtil.getProcessWrapper(embeddedBinariesRoot); - if (processWrapper.exists()) { - args = - ProcessWrapperUtil.commandLineBuilder(processWrapper.getPathString(), args) - .setTimeout(Duration.ofMillis(timeoutMillis)) - .build(); - } + if (processWrapper != null) { + args = + processWrapper + .commandLineBuilder(args) + .setTimeout(Duration.ofMillis(timeoutMillis)) + .build(); } Path workingDirectoryPath = outputDirectory;
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java index 78fc21a..889f47e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
@@ -35,6 +35,7 @@ import com.google.devtools.build.lib.rules.repository.RepositoryFunction; import com.google.devtools.build.lib.rules.repository.ResolvedHashesValue; import com.google.devtools.build.lib.rules.repository.WorkspaceFileHelper; +import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor; import com.google.devtools.build.lib.skyframe.BlacklistedPackagePrefixesValue; import com.google.devtools.build.lib.skyframe.PrecomputedValue; @@ -60,6 +61,7 @@ private final DownloadManager downloadManager; private double timeoutScaling = 1.0; + @Nullable private ProcessWrapper processWrapper = null; @Nullable private RepositoryRemoteExecutor repositoryRemoteExecutor; public SkylarkRepositoryFunction(DownloadManager downloadManager) { @@ -70,6 +72,10 @@ this.timeoutScaling = timeoutScaling; } + public void setProcessWrapper(@Nullable ProcessWrapper processWrapper) { + this.processWrapper = processWrapper; + } + static String describeSemantics(StarlarkSemantics semantics) { // Here we use the hash code provided by AutoValue. This is unique, as long // as the number of bits in the StarlarkSemantics is small enough. We will have to @@ -167,8 +173,8 @@ env, clientEnvironment, downloadManager, - directories.getEmbeddedBinariesRoot(), timeoutScaling, + processWrapper, markerData, starlarkSemantics, repositoryRemoteExecutor);
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 ef9886b..c8c26d6 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
@@ -42,7 +42,7 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; -import com.google.devtools.build.lib.runtime.ProcessWrapperUtil; +import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.server.FailureDetails; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.server.FailureDetails.Spawn.Code; @@ -51,8 +51,6 @@ import com.google.devtools.build.lib.shell.SubprocessBuilder; import com.google.devtools.build.lib.shell.TerminationStatus; import com.google.devtools.build.lib.util.NetUtil; -import com.google.devtools.build.lib.util.OS; -import com.google.devtools.build.lib.util.OsUtils; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.Path; import com.google.errorprone.annotations.FormatMethod; @@ -87,65 +85,31 @@ private final LocalExecutionOptions localExecutionOptions; - private final boolean useProcessWrapper; - private final Path processWrapper; + @Nullable private final ProcessWrapper processWrapper; private final LocalEnvProvider localEnvProvider; private final BinTools binTools; private final RunfilesTreeUpdater runfilesTreeUpdater; - // TODO(b/62588075): Move this logic to ProcessWrapperUtil? - private static Path getProcessWrapper(BinTools binTools, OS localOs) { - // We expect binTools to be null only under testing. - return binTools == null - ? null - : binTools.getEmbeddedPath("process-wrapper" + OsUtils.executableExtension(localOs)); - } - - private static boolean processWrapperExists(BinTools binTools) { - Path wrapper = getProcessWrapper(binTools, OS.getCurrent()); - return wrapper != null && wrapper.exists(); - } - public LocalSpawnRunner( Path execRoot, LocalExecutionOptions localExecutionOptions, ResourceManager resourceManager, - boolean useProcessWrapper, - OS localOs, LocalEnvProvider localEnvProvider, BinTools binTools, + ProcessWrapper processWrapper, RunfilesTreeUpdater runfilesTreeUpdater) { this.execRoot = execRoot; - this.processWrapper = getProcessWrapper(binTools, localOs); + this.processWrapper = processWrapper; this.localExecutionOptions = Preconditions.checkNotNull(localExecutionOptions); this.hostName = NetUtil.getCachedShortHostName(); this.resourceManager = resourceManager; - this.useProcessWrapper = useProcessWrapper; this.localEnvProvider = localEnvProvider; this.binTools = binTools; this.runfilesTreeUpdater = runfilesTreeUpdater; } - public LocalSpawnRunner( - Path execRoot, - LocalExecutionOptions localExecutionOptions, - ResourceManager resourceManager, - LocalEnvProvider localEnvProvider, - BinTools binTools, - RunfilesTreeUpdater runfilesTreeUpdater) { - this( - execRoot, - localExecutionOptions, - resourceManager, - OS.getCurrent() != OS.WINDOWS && processWrapperExists(binTools), - OS.getCurrent(), - localEnvProvider, - binTools, - runfilesTreeUpdater); - } - @Override public String getName() { return "local"; @@ -357,13 +321,13 @@ subprocessBuilder.setStderr(outErr.getErrorPath().getPathFile()); subprocessBuilder.setEnv(environment); ImmutableList<String> args; - if (useProcessWrapper) { + if (processWrapper != null) { // 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. - ProcessWrapperUtil.CommandLineBuilder commandLineBuilder = - ProcessWrapperUtil.commandLineBuilder( - processWrapper.getPathString(), spawn.getArguments()) + ProcessWrapper.CommandLineBuilder commandLineBuilder = + processWrapper + .commandLineBuilder(spawn.getArguments()) .setTimeout(context.getTimeout()) .setKillDelay(Duration.ofSeconds(localExecutionOptions.localSigkillGraceSeconds)); if (localExecutionOptions.collectLocalExecutionStatistics) { @@ -422,7 +386,7 @@ Duration wallTime = Duration.ofMillis(System.currentTimeMillis() - startTime); boolean wasTimeout = terminationStatus.timedOut() - || (useProcessWrapper && wasTimeout(context.getTimeout(), wallTime)); + || (processWrapper != null && wasTimeout(context.getTimeout(), wallTime)); int exitCode = wasTimeout ? SpawnResult.POSIX_TIMEOUT_EXIT_CODE : terminationStatus.getRawExitCode(); Status status =
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/ProcessWrapperUtil.java b/src/main/java/com/google/devtools/build/lib/runtime/ProcessWrapper.java similarity index 69% rename from src/main/java/com/google/devtools/build/lib/runtime/ProcessWrapperUtil.java rename to src/main/java/com/google/devtools/build/lib/runtime/ProcessWrapper.java index 23b44f0..d85e645 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/ProcessWrapperUtil.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/ProcessWrapper.java
@@ -14,43 +14,49 @@ package com.google.devtools.build.lib.runtime; +import com.google.common.annotations.VisibleForTesting; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.OsUtils; import com.google.devtools.build.lib.vfs.Path; import java.time.Duration; import java.util.ArrayList; import java.util.List; +import javax.annotation.Nullable; -/** - * Utility functions for the process wrapper embedded tool, which should work on most platforms and - * gives at least some isolation between running actions. - */ -public final class ProcessWrapperUtil { - private static final String PROCESS_WRAPPER = "process-wrapper" + OsUtils.executableExtension(); +/** Tracks process-wrapper configuration and allows building command lines that rely on it. */ +public final class ProcessWrapper { - /** Returns whether using the process wrapper is supported in the {@link CommandEnvironment}. */ - public static boolean isSupported(CommandEnvironment cmdEnv) { - // We can only use this runner, if the process-wrapper exists in the embedded tools. - // This might not always be the case, e.g. while bootstrapping. - return getProcessWrapper(cmdEnv) != null; - } + /** Name of the process-wrapper binary, without any path components. */ + private static final String BIN_BASENAME = "process-wrapper" + OsUtils.executableExtension(); - /** Returns the {@link Path} of the process wrapper binary, or null if it doesn't exist. */ - public static Path getProcessWrapper(CommandEnvironment cmdEnv) { - return cmdEnv.getBlazeWorkspace().getBinTools().getEmbeddedPath(PROCESS_WRAPPER); + /** Path to the process-wrapper binary to use. */ + private final Path binPath; + + /** Creates a new process-wrapper instance from explicit values. */ + @VisibleForTesting + public ProcessWrapper(Path binPath) { + this.binPath = binPath; } /** - * Return the {@link Path} of the process wrapper binary, given the path of the embedded binaries - * root. + * Constructs a new process-wrapper instance based on the context of an invocation. + * + * @param cmdEnv command environment for this invocation + * @return a process-wrapper handler, or null if this is not supported in the current system */ - public static Path getProcessWrapper(Path embeddedBinariesRoot) { - return embeddedBinariesRoot.getRelative(PROCESS_WRAPPER); + @Nullable + public static ProcessWrapper fromCommandEnvironment(CommandEnvironment cmdEnv) { + Path path = cmdEnv.getBlazeWorkspace().getBinTools().getEmbeddedPath(BIN_BASENAME); + if (OS.isPosixCompatible() && path != null && path.exists()) { + return new ProcessWrapper(path); + } else { + return null; + } } - /** Returns a new {@link CommandLineBuilder} for the process wrapper tool. */ - public static CommandLineBuilder commandLineBuilder( - String processWrapperPath, List<String> commandArguments) { - return new CommandLineBuilder(processWrapperPath, commandArguments); + /** Returns a new {@link CommandLineBuilder} for the process-wrapper tool. */ + public CommandLineBuilder commandLineBuilder(List<String> commandArguments) { + return new CommandLineBuilder(binPath.getPathString(), commandArguments); } /**
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 1d93957..1901a11 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
@@ -27,7 +27,7 @@ import com.google.devtools.build.lib.exec.TreeDeleter; import com.google.devtools.build.lib.exec.local.LocalEnvProvider; import com.google.devtools.build.lib.runtime.CommandEnvironment; -import com.google.devtools.build.lib.runtime.ProcessWrapperUtil; +import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs; import com.google.devtools.build.lib.shell.Command; @@ -70,7 +70,7 @@ if (OS.getCurrent() != OS.DARWIN) { return false; } - if (!ProcessWrapperUtil.isSupported(cmdEnv)) { + if (ProcessWrapper.fromCommandEnvironment(cmdEnv) == null) { return false; } if (isSupported == null) { @@ -102,7 +102,7 @@ private final SandboxHelpers helpers; private final Path execRoot; private final boolean allowNetwork; - private final Path processWrapper; + private final ProcessWrapper processWrapper; private final Path sandboxBase; private final Duration timeoutKillDelay; @Nullable private final SandboxfsProcess sandboxfsProcess; @@ -143,7 +143,7 @@ this.execRoot = cmdEnv.getExecRoot(); this.allowNetwork = helpers.shouldAllowNetwork(cmdEnv.getOptions()); this.alwaysWritableDirs = getAlwaysWritableDirs(cmdEnv.getRuntime().getFileSystem()); - this.processWrapper = ProcessWrapperUtil.getProcessWrapper(cmdEnv); + this.processWrapper = ProcessWrapper.fromCommandEnvironment(cmdEnv); this.localEnvProvider = LocalEnvProvider.forCurrentOs(cmdEnv.getClientEnv()); this.sandboxBase = sandboxBase; this.timeoutKillDelay = timeoutKillDelay; @@ -246,9 +246,8 @@ final Path sandboxConfigPath = sandboxPath.getRelative("sandbox.sb"); Duration timeout = context.getTimeout(); - ProcessWrapperUtil.CommandLineBuilder processWrapperCommandLineBuilder = - ProcessWrapperUtil.commandLineBuilder(processWrapper.getPathString(), spawn.getArguments()) - .setTimeout(timeout); + ProcessWrapper.CommandLineBuilder processWrapperCommandLineBuilder = + processWrapper.commandLineBuilder(spawn.getArguments()).setTimeout(timeout); processWrapperCommandLineBuilder.setKillDelay(timeoutKillDelay);
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DockerCommandLineBuilder.java b/src/main/java/com/google/devtools/build/lib/sandbox/DockerCommandLineBuilder.java index 9f90209..62ad449 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DockerCommandLineBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DockerCommandLineBuilder.java
@@ -17,7 +17,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.runtime.ProcessWrapperUtil; +import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.time.Duration; @@ -26,7 +26,7 @@ import java.util.UUID; final class DockerCommandLineBuilder { - private Path processWrapper; + private ProcessWrapper processWrapper; private Path dockerClient; private String imageName; private List<String> commandArguments; @@ -42,7 +42,7 @@ private boolean privileged; private List<Map.Entry<String, String>> additionalMounts; - public DockerCommandLineBuilder setProcessWrapper(Path processWrapper) { + public DockerCommandLineBuilder setProcessWrapper(ProcessWrapper processWrapper) { this.processWrapper = processWrapper; return this; } @@ -174,9 +174,8 @@ dockerCmdLine.add(imageName); dockerCmdLine.addAll(commandArguments); - ProcessWrapperUtil.CommandLineBuilder processWrapperCmdLine = - ProcessWrapperUtil.commandLineBuilder( - this.processWrapper.getPathString(), dockerCmdLine.build()); + ProcessWrapper.CommandLineBuilder processWrapperCmdLine = + processWrapper.commandLineBuilder(dockerCmdLine.build()); if (timeout != null) { processWrapperCmdLine.setTimeout(timeout); }
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java index ccbd38b..6a5e5fd 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java
@@ -34,7 +34,7 @@ import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.runtime.CommandCompleteEvent; import com.google.devtools.build.lib.runtime.CommandEnvironment; -import com.google.devtools.build.lib.runtime.ProcessWrapperUtil; +import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs; import com.google.devtools.build.lib.shell.Command; @@ -75,15 +75,14 @@ public static boolean isSupported(CommandEnvironment cmdEnv, Path dockerClient) { boolean verbose = cmdEnv.getOptions().getOptions(SandboxOptions.class).dockerVerbose; - if (!ProcessWrapperUtil.isSupported(cmdEnv)) { + if (ProcessWrapper.fromCommandEnvironment(cmdEnv) == null) { if (verbose) { cmdEnv .getReporter() .handle( Event.error( - "Docker sandboxing is disabled, because ProcessWrapperUtil.isSupported " - + "returned false. This should never happen - is your Bazel binary " - + "corrupted?")); + "Docker sandboxing is disabled because ProcessWrapper is not supported. " + + "This should never happen - is your Bazel binary corrupted?")); } return false; } @@ -141,7 +140,7 @@ private final Path execRoot; private final boolean allowNetwork; private final Path dockerClient; - private final Path processWrapper; + private final ProcessWrapper processWrapper; private final Path sandboxBase; private final String defaultImage; private final LocalEnvProvider localEnvProvider; @@ -181,7 +180,7 @@ this.execRoot = cmdEnv.getExecRoot(); this.allowNetwork = helpers.shouldAllowNetwork(cmdEnv.getOptions()); this.dockerClient = dockerClient; - this.processWrapper = ProcessWrapperUtil.getProcessWrapper(cmdEnv); + this.processWrapper = ProcessWrapper.fromCommandEnvironment(cmdEnv); this.sandboxBase = sandboxBase; this.defaultImage = defaultImage; this.localEnvProvider = LocalEnvProvider.forCurrentOs(cmdEnv.getClientEnv());
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 65f6748..6ae9010 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
@@ -21,7 +21,7 @@ import com.google.devtools.build.lib.exec.TreeDeleter; import com.google.devtools.build.lib.exec.local.LocalEnvProvider; import com.google.devtools.build.lib.runtime.CommandEnvironment; -import com.google.devtools.build.lib.runtime.ProcessWrapperUtil; +import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs; import com.google.devtools.build.lib.util.OS; @@ -34,11 +34,11 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { public static boolean isSupported(CommandEnvironment cmdEnv) { - return OS.isPosixCompatible() && ProcessWrapperUtil.isSupported(cmdEnv); + return OS.isPosixCompatible() && ProcessWrapper.fromCommandEnvironment(cmdEnv) != null; } private final SandboxHelpers helpers; - private final Path processWrapper; + private final ProcessWrapper processWrapper; private final Path execRoot; private final Path sandboxBase; private final LocalEnvProvider localEnvProvider; @@ -68,7 +68,7 @@ TreeDeleter treeDeleter) { super(cmdEnv); this.helpers = helpers; - this.processWrapper = ProcessWrapperUtil.getProcessWrapper(cmdEnv); + this.processWrapper = ProcessWrapper.fromCommandEnvironment(cmdEnv); this.execRoot = cmdEnv.getExecRoot(); this.localEnvProvider = LocalEnvProvider.forCurrentOs(cmdEnv.getClientEnv()); this.sandboxBase = sandboxBase; @@ -100,9 +100,8 @@ localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), binTools, "/tmp"); Duration timeout = context.getTimeout(); - ProcessWrapperUtil.CommandLineBuilder commandLineBuilder = - ProcessWrapperUtil.commandLineBuilder(processWrapper.getPathString(), spawn.getArguments()) - .setTimeout(timeout); + ProcessWrapper.CommandLineBuilder commandLineBuilder = + processWrapper.commandLineBuilder(spawn.getArguments()).setTimeout(timeout); commandLineBuilder.setKillDelay(timeoutKillDelay);
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 c230124..c539429 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
@@ -40,6 +40,7 @@ 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.runtime.ProcessWrapper; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.server.FailureDetails.Sandbox; import com.google.devtools.build.lib.util.AbruptExitException; @@ -435,6 +436,7 @@ env.getLocalResourceManager(), LocalEnvProvider.forCurrentOs(env.getClientEnv()), env.getBlazeWorkspace().getBinTools(), + ProcessWrapper.fromCommandEnvironment(env), // TODO(buchgr): Replace singleton by a command-scoped RunfilesTreeUpdater RunfilesTreeUpdater.INSTANCE); }
diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java index d0601eb..23fb189 100644 --- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java +++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java
@@ -36,6 +36,7 @@ import com.google.devtools.build.lib.rules.test.ExclusiveTestStrategy; import com.google.devtools.build.lib.runtime.BlazeModule; import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.server.FailureDetails.LocalExecution; import com.google.devtools.build.lib.server.FailureDetails.LocalExecution.Code; @@ -108,6 +109,7 @@ env.getLocalResourceManager(), LocalEnvProvider.forCurrentOs(env.getClientEnv()), env.getBlazeWorkspace().getBinTools(), + ProcessWrapper.fromCommandEnvironment(env), // TODO(buchgr): Replace singleton by a command-scoped RunfilesTreeUpdater RunfilesTreeUpdater.INSTANCE);
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java index 02d5d96..89b8a3f 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java
@@ -31,6 +31,7 @@ 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.runtime.ProcessWrapper; import com.google.devtools.build.lib.runtime.commands.CleanCommand.CleanStartingEvent; import com.google.devtools.build.lib.sandbox.SandboxHelpers; import com.google.devtools.build.lib.sandbox.SandboxOptions; @@ -171,6 +172,7 @@ env.getLocalResourceManager(), localEnvProvider, env.getBlazeWorkspace().getBinTools(), + ProcessWrapper.fromCommandEnvironment(env), // TODO(buchgr): Replace singleton by a command-scoped RunfilesTreeUpdater RunfilesTreeUpdater.INSTANCE); }
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java index d75521e..416be8b 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java
@@ -162,8 +162,8 @@ environment, ImmutableMap.of("FOO", "BAR"), downloader, - null, 1.0, + /*processWrapper=*/ null, new HashMap<>(), starlarkSemantics, repoRemoteExecutor);
diff --git a/src/test/java/com/google/devtools/build/lib/exec/local/BUILD b/src/test/java/com/google/devtools/build/lib/exec/local/BUILD index fa14ab8..c6b9e86 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/local/BUILD +++ b/src/test/java/com/google/devtools/build/lib/exec/local/BUILD
@@ -21,6 +21,7 @@ "//src/test/shell/integration:spend_cpu_time", ], deps = [ + "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/exec:bin_tools",
diff --git a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java index 6bde2b5..5a0158e 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
@@ -47,6 +47,7 @@ import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.exec.util.SpawnBuilder; +import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.shell.JavaSubprocessFactory; import com.google.devtools.build.lib.shell.Subprocess; import com.google.devtools.build.lib.shell.SubprocessBuilder; @@ -58,7 +59,6 @@ import com.google.devtools.build.lib.unix.UnixFileSystem; import com.google.devtools.build.lib.util.NetUtil; import com.google.devtools.build.lib.util.OS; -import com.google.devtools.build.lib.util.OsUtils; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; @@ -99,30 +99,21 @@ /** Unit tests for {@link LocalSpawnRunner}. */ @RunWith(JUnit4.class) public class LocalSpawnRunnerTest { - private static final boolean USE_WRAPPER = true; - private static final boolean NO_WRAPPER = false; private static class TestedLocalSpawnRunner extends LocalSpawnRunner { public TestedLocalSpawnRunner( Path execRoot, - Path embeddedBin, LocalExecutionOptions localExecutionOptions, ResourceManager resourceManager, - boolean useProcessWrapper, - OS localOs, + ProcessWrapper processWrapper, LocalEnvProvider localEnvProvider) { super( execRoot, localExecutionOptions, resourceManager, - useProcessWrapper, - localOs, localEnvProvider, - useProcessWrapper - ? BinTools.forEmbeddedBin( - embeddedBin, - ImmutableList.of("process-wrapper" + OsUtils.executableExtension(localOs))) - : null, + /*binTools=*/ null, + processWrapper, Mockito.mock(RunfilesTreeUpdater.class)); } @@ -321,6 +312,10 @@ return new InMemoryFileSystem(); } + private static ProcessWrapper makeProcessWrapper(FileSystem fs) { + return new ProcessWrapper(fs.getPath("/process-wrapper")); + } + /** * Enables real execution by default. * @@ -351,11 +346,9 @@ LocalSpawnRunner runner = new TestedLocalSpawnRunner( fs.getPath("/execroot"), - fs.getPath("/embedded_bin"), options, resourceManager, - USE_WRAPPER, - OS.LINUX, + makeProcessWrapper(fs), LocalSpawnRunnerTest::keepLocalEnvUnchanged); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); @@ -372,11 +365,7 @@ assertThat(captor.getValue().getArgv()) .containsExactlyElementsIn( ImmutableList.of( - "/embedded_bin/process-wrapper", - "--timeout=123", - "--kill_delay=456", - "/bin/echo", - "Hi!")); + "/process-wrapper", "--timeout=123", "--kill_delay=456", "/bin/echo", "Hi!")); assertThat(captor.getValue().getEnv()).containsExactly("VARIABLE", "value"); assertThat(captor.getValue().getTimeoutMillis()).isEqualTo(0); assertThat(captor.getValue().getStdout()).isEqualTo(StreamAction.REDIRECT); @@ -408,11 +397,9 @@ LocalSpawnRunner runner = new TestedLocalSpawnRunner( execRoot, - fs.getPath("/embedded_bin"), options, resourceManager, - USE_WRAPPER, - OS.LINUX, + makeProcessWrapper(fs), LocalSpawnRunnerTest::keepLocalEnvUnchanged); ParamFileActionInput paramFileActionInput = new ParamFileActionInput( @@ -462,11 +449,9 @@ LocalSpawnRunner runner = new TestedLocalSpawnRunner( fs.getPath("/execroot"), - fs.getPath("/embedded_bin"), options, resourceManager, - NO_WRAPPER, - OS.LINUX, + /*processWrapper=*/ null, LocalSpawnRunnerTest::keepLocalEnvUnchanged); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); @@ -507,11 +492,9 @@ LocalSpawnRunner runner = new TestedLocalSpawnRunner( fs.getPath("/execroot"), - fs.getPath("/embedded_bin"), options, resourceManager, - USE_WRAPPER, - OS.LINUX, + makeProcessWrapper(fs), LocalSpawnRunnerTest::keepLocalEnvUnchanged); assertThat(fs.getPath("/execroot").createDirectory()).isTrue(); @@ -528,11 +511,7 @@ .containsExactlyElementsIn( ImmutableList.of( // process-wrapper timeout grace_time stdout stderr - "/embedded_bin/process-wrapper", - "--timeout=0", - "--kill_delay=15", - "/bin/echo", - "Hi!")); + "/process-wrapper", "--timeout=0", "--kill_delay=15", "/bin/echo", "Hi!")); assertThat(captor.getValue().getEnv()).containsExactly("VARIABLE", "value"); assertThat(captor.getValue().getStdout()).isEqualTo(StreamAction.REDIRECT); assertThat(captor.getValue().getStdoutFile()).isEqualTo(new File("/out/stdout")); @@ -555,11 +534,9 @@ LocalSpawnRunner runner = new TestedLocalSpawnRunner( fs.getPath("/execroot"), - fs.getPath("/embedded_bin"), options, resourceManager, - USE_WRAPPER, - OS.LINUX, + makeProcessWrapper(fs), LocalSpawnRunnerTest::keepLocalEnvUnchanged); assertThat(fs.getPath("/out").createDirectory()).isTrue(); @@ -591,11 +568,9 @@ LocalSpawnRunner runner = new TestedLocalSpawnRunner( fs.getPath("/execroot"), - fs.getPath("/embedded_bin"), options, resourceManager, - USE_WRAPPER, - OS.LINUX, + makeProcessWrapper(fs), LocalSpawnRunnerTest::keepLocalEnvUnchanged); assertThat(fs.getPath("/execroot").createDirectory()).isTrue(); @@ -642,11 +617,9 @@ LocalSpawnRunner runner = new TestedLocalSpawnRunner( fs.getPath("/execroot"), - fs.getPath("/embedded_bin"), options, resourceManager, - USE_WRAPPER, - OS.LINUX, + makeProcessWrapper(fs), LocalSpawnRunnerTest::keepLocalEnvUnchanged); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); @@ -671,10 +644,9 @@ tempDir, Options.getDefaults(LocalExecutionOptions.class), resourceManager, - /*useProcessWrapper=*/ false, - OS.LINUX, LocalEnvProvider.forCurrentOs(ImmutableMap.of()), /*binTools=*/ null, + /*processWrapper=*/ null, Mockito.mock(RunfilesTreeUpdater.class)); FileOutErr fileOutErr = new FileOutErr(tempDir.getRelative("stdout"), tempDir.getRelative("stderr")); @@ -755,11 +727,9 @@ LocalSpawnRunner runner = new TestedLocalSpawnRunner( fs.getPath("/execroot"), - fs.getPath("/embedded_bin"), options, resourceManager, - USE_WRAPPER, - OS.LINUX, + makeProcessWrapper(fs), LocalSpawnRunnerTest::keepLocalEnvUnchanged); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); @@ -782,11 +752,9 @@ LocalSpawnRunner runner = new TestedLocalSpawnRunner( fs.getPath("/execroot"), - fs.getPath("/embedded_bin"), options, resourceManager, - USE_WRAPPER, - OS.LINUX, + makeProcessWrapper(fs), LocalSpawnRunnerTest::keepLocalEnvUnchanged); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); @@ -813,11 +781,9 @@ LocalSpawnRunner runner = new TestedLocalSpawnRunner( fs.getPath("/execroot"), - fs.getPath("/embedded_bin"), options, resourceManager, - USE_WRAPPER, - OS.LINUX, + makeProcessWrapper(fs), localEnvProvider); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); @@ -833,51 +799,6 @@ matches("^/execroot/tmp[0-9a-fA-F]+_[0-9a-fA-F]+/work$")); } - @Test - public void useCorrectExtensionOnWindows() throws Exception { - // TODO(#3536): Make this test work on Windows. - // The Command API implicitly absolutizes the path, and we get weird paths on Windows: - // T:\execroot\execroot\_bin\process-wrapper.exe - assumeTrue(OS.getCurrent() != OS.WINDOWS); - - FileSystem fs = setupEnvironmentForFakeExecution(); - - SubprocessFactory factory = mock(SubprocessFactory.class); - ArgumentCaptor<SubprocessBuilder> captor = ArgumentCaptor.forClass(SubprocessBuilder.class); - when(factory.create(captor.capture())).thenReturn(new FinishedSubprocess(0)); - SubprocessBuilder.setDefaultSubprocessFactory(factory); - - LocalExecutionOptions options = Options.getDefaults(LocalExecutionOptions.class); - options.localSigkillGraceSeconds = 654; - LocalSpawnRunner runner = - new TestedLocalSpawnRunner( - fs.getPath("/execroot"), - fs.getPath("/embedded_bin"), - options, - resourceManager, - USE_WRAPPER, - OS.WINDOWS, - LocalSpawnRunnerTest::keepLocalEnvUnchanged); - - FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); - SpawnExecutionContextForTesting policy = new SpawnExecutionContextForTesting(fileOutErr); - policy.timeoutMillis = 321 * 1000L; - assertThat(fs.getPath("/execroot").createDirectory()).isTrue(); - SpawnResult result = runner.execAsync(SIMPLE_SPAWN, policy).get(); - verify(factory).create(any(SubprocessBuilder.class)); - assertThat(result.status()).isEqualTo(SpawnResult.Status.SUCCESS); - - assertThat(captor.getValue().getArgv()) - .containsExactlyElementsIn( - ImmutableList.of( - // process-wrapper timeout grace_time stdout stderr - "/embedded_bin/process-wrapper.exe", - "--timeout=321", - "--kill_delay=654", - "/bin/echo", - "Hi!")); - } - /** * Copies the {@code process-wrapper} tool into the path under the temporary execRoot where the * {@link LocalSpawnRunner} expects to find it. @@ -978,7 +899,8 @@ Path embeddedBinaries = getTemporaryEmbeddedBin(fs); BinTools binTools = BinTools.forEmbeddedBin(embeddedBinaries, ImmutableList.of("process-wrapper")); - copyProcessWrapperIntoExecRoot(binTools.getEmbeddedPath("process-wrapper")); + Path processWrapperPath = binTools.getEmbeddedPath("process-wrapper"); + copyProcessWrapperIntoExecRoot(processWrapperPath); Path cpuTimeSpenderPath = copyCpuTimeSpenderIntoExecRoot(execRoot); LocalSpawnRunner runner = @@ -986,10 +908,9 @@ execRoot, options, resourceManager, - USE_WRAPPER, - OS.LINUX, LocalSpawnRunnerTest::keepLocalEnvUnchanged, binTools, + new ProcessWrapper(processWrapperPath), Mockito.mock(RunfilesTreeUpdater.class)); Spawn spawn = @@ -1042,7 +963,8 @@ Path embeddedBinaries = getTemporaryEmbeddedBin(fs); BinTools binTools = BinTools.forEmbeddedBin(embeddedBinaries, ImmutableList.of("process-wrapper")); - copyProcessWrapperIntoExecRoot(binTools.getEmbeddedPath("process-wrapper")); + Path processWrapperPath = binTools.getEmbeddedPath("process-wrapper"); + copyProcessWrapperIntoExecRoot(processWrapperPath); Path cpuTimeSpenderPath = copyCpuTimeSpenderIntoExecRoot(execRoot); LocalSpawnRunner runner = @@ -1050,10 +972,9 @@ execRoot, options, resourceManager, - USE_WRAPPER, - OS.LINUX, LocalSpawnRunnerTest::keepLocalEnvUnchanged, binTools, + new ProcessWrapper(processWrapperPath), Mockito.mock(RunfilesTreeUpdater.class)); Spawn spawn = @@ -1102,11 +1023,9 @@ LocalSpawnRunner runner = new TestedLocalSpawnRunner( fs.getPath("/execroot"), - fs.getPath("/embedded_bin"), Options.getDefaults(LocalExecutionOptions.class), resourceManager, - NO_WRAPPER, - OS.LINUX, + /*processWrapper=*/ null, LocalSpawnRunnerTest::keepLocalEnvUnchanged); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr"));
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/ProcessWrapperUtilTest.java b/src/test/java/com/google/devtools/build/lib/runtime/ProcessWrapperTest.java similarity index 72% rename from src/test/java/com/google/devtools/build/lib/runtime/ProcessWrapperUtilTest.java rename to src/test/java/com/google/devtools/build/lib/runtime/ProcessWrapperTest.java index ff1fdd5..63b0746 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/ProcessWrapperUtilTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/ProcessWrapperTest.java
@@ -20,6 +20,7 @@ import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; +import java.io.IOException; import java.time.Duration; import java.util.List; import org.junit.Before; @@ -27,35 +28,41 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -/** Unit tests for {@link ProcessWrapperUtil}. */ +/** Unit tests for {@link ProcessWrapper}. */ @RunWith(JUnit4.class) -public final class ProcessWrapperUtilTest { +public final class ProcessWrapperTest { + private FileSystem testFS; @Before - public final void createFileSystem() { + public void setUp() { testFS = new InMemoryFileSystem(); } - @Test - public void testProcessWrapperCommandLineBuilder_BuildsWithoutOptionalArguments() { - String processWrapperPath = "process-wrapper"; + private ProcessWrapper getProcessWrapper(String path) throws IOException { + Path processWrapperPath = testFS.getPath(path); + processWrapperPath.getParentDirectory().createDirectoryAndParents(); + processWrapperPath.getOutputStream().close(); + return new ProcessWrapper(processWrapperPath); + } + @Test + public void testProcessWrapperCommandLineBuilder_BuildsWithoutOptionalArguments() + throws IOException { ImmutableList<String> commandArguments = ImmutableList.of("echo", "hello, world"); ImmutableList<String> expectedCommandLine = - ImmutableList.<String>builder().add(processWrapperPath).addAll(commandArguments).build(); + ImmutableList.<String>builder().add("/some/bin/path").addAll(commandArguments).build(); - List<String> commandLine = - ProcessWrapperUtil.commandLineBuilder(processWrapperPath, commandArguments).build(); + ProcessWrapper processWrapper = getProcessWrapper("/some/bin/path"); + List<String> commandLine = processWrapper.commandLineBuilder(commandArguments).build(); assertThat(commandLine).containsExactlyElementsIn(expectedCommandLine).inOrder(); } @Test - public void testProcessWrapperCommandLineBuilder_BuildsWithOptionalArguments() { - String processWrapperPath = "process-wrapper"; - + public void testProcessWrapperCommandLineBuilder_BuildsWithOptionalArguments() + throws IOException { ImmutableList<String> commandArguments = ImmutableList.of("echo", "hello, world"); Duration timeout = Duration.ofSeconds(10); @@ -66,7 +73,7 @@ ImmutableList<String> expectedCommandLine = ImmutableList.<String>builder() - .add(processWrapperPath) + .add("/path/to/process-wrapper") .add("--timeout=" + timeout.getSeconds()) .add("--kill_delay=" + killDelay.getSeconds()) .add("--stdout=" + stdoutPath) @@ -75,8 +82,10 @@ .addAll(commandArguments) .build(); + ProcessWrapper processWrapper = getProcessWrapper("/path/to/process-wrapper"); List<String> commandLine = - ProcessWrapperUtil.commandLineBuilder(processWrapperPath, commandArguments) + processWrapper + .commandLineBuilder(commandArguments) .setTimeout(timeout) .setKillDelay(killDelay) .setStdoutPath(stdoutPath)
diff --git a/src/test/java/com/google/devtools/build/lib/shell/CommandUsingProcessWrapperTest.java b/src/test/java/com/google/devtools/build/lib/shell/CommandUsingProcessWrapperTest.java index a7c32c1..e6e87e8 100644 --- a/src/test/java/com/google/devtools/build/lib/shell/CommandUsingProcessWrapperTest.java +++ b/src/test/java/com/google/devtools/build/lib/shell/CommandUsingProcessWrapperTest.java
@@ -17,7 +17,7 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.runtime.ProcessWrapperUtil; +import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.testutil.BlazeTestUtils; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.testutil.TestUtils; @@ -43,8 +43,11 @@ testFS = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked()); } - private String getProcessWrapperPath() { - return BlazeTestUtils.runfilesDir() + "/" + TestConstants.PROCESS_WRAPPER_PATH; + private ProcessWrapper getProcessWrapper() { + return new ProcessWrapper( + testFS + .getPath(BlazeTestUtils.runfilesDir()) + .getRelative(TestConstants.PROCESS_WRAPPER_PATH)); } private String getCpuTimeSpenderPath() { @@ -66,8 +69,7 @@ public void testProcessWrappedCommand_Echo() throws Exception { ImmutableList<String> commandArguments = ImmutableList.of("echo", "even drones can fly away"); - List<String> fullCommandLine = - ProcessWrapperUtil.commandLineBuilder(getProcessWrapperPath(), commandArguments).build(); + List<String> fullCommandLine = getProcessWrapper().commandLineBuilder(commandArguments).build(); Command command = new Command(fullCommandLine.toArray(new String[0])); CommandResult commandResult = command.execute(); @@ -88,7 +90,8 @@ Path statisticsFilePath = outputDir.getRelative("stats.out"); List<String> fullCommandLine = - ProcessWrapperUtil.commandLineBuilder(getProcessWrapperPath(), commandArguments) + getProcessWrapper() + .commandLineBuilder(commandArguments) .setStatisticsPath(statisticsFilePath) .build();
diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java index ff66eb2..32154dc 100644 --- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java +++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
@@ -150,6 +150,7 @@ resourceManager, (env, binTools1, fallbackTmpDir) -> ImmutableMap.copyOf(env), binTools, + /*processWrapper=*/ null, Mockito.mock(RunfilesTreeUpdater.class))); this.executor = new TestExecutorBuilder(fileSystem, directories, binTools)