diff options
18 files changed, 256 insertions, 151 deletions
diff --git a/scripts/bootstrap/compile.sh b/scripts/bootstrap/compile.sh index 3fcffba31e..6ca0e85b38 100755 --- a/scripts/bootstrap/compile.sh +++ b/scripts/bootstrap/compile.sh @@ -233,42 +233,6 @@ cp $1 $2/MANIFEST EOF chmod 0755 ${ARCHIVE_DIR}/_embedded_binaries/build-runfiles${EXE_EXT} -log "Creating process-wrapper..." -cat <<'EOF' >${ARCHIVE_DIR}/_embedded_binaries/process-wrapper${EXE_EXT} -#!/bin/sh -# Dummy process wrapper, does not support timeout -shift 2 -stdout="$1" -stderr="$2" -shift 2 - -if [ "$stdout" = "-" ] -then - if [ "$stderr" = "-" ] - then - "$@" - exit $? - else - "$@" 2>"$stderr" - exit $? - fi -else - if [ "$stderr" = "-" ] - then - "$@" >"$stdout" - exit $? - else - "$@" 2>"$stderr" >"$stdout" - exit $? - fi -fi - - -"$@" -exit $? -EOF -chmod 0755 ${ARCHIVE_DIR}/_embedded_binaries/process-wrapper${EXE_EXT} - function build_jni() { local -r output_dir=$1 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 14fe8b1fdc..565631e24f 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 @@ -33,7 +33,7 @@ public class LocalExecutionOptions extends OptionsBase { "Time to wait between terminating a local process due to timeout and forcefully " + "shutting it down." ) - public double localSigkillGraceSeconds; + public int localSigkillGraceSeconds; @Option( name = "allowed_local_actions_regex", 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 5cd191be3b..e77c656a6b 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 @@ -81,6 +81,10 @@ public final class LocalSpawnRunner implements SpawnRunner { private final String productName; private final LocalEnvProvider localEnvProvider; + private static Path getProcessWrapper(Path execRoot, OS localOs) { + return execRoot.getRelative("_bin/process-wrapper" + OsUtils.executableExtension(localOs)); + } + public LocalSpawnRunner( Logger logger, AtomicInteger execCount, @@ -95,10 +99,7 @@ public final class LocalSpawnRunner implements SpawnRunner { this.logger = logger; this.execRoot = execRoot; this.actionInputPrefetcher = Preconditions.checkNotNull(actionInputPrefetcher); - this.processWrapper = - execRoot - .getRelative("_bin/process-wrapper" + OsUtils.executableExtension(localOs)) - .getPathString(); + this.processWrapper = getProcessWrapper(execRoot, localOs).getPathString(); this.localExecutionOptions = Preconditions.checkNotNull(localExecutionOptions); this.hostName = NetUtil.findShortHostName(); this.execCount = execCount; @@ -122,7 +123,7 @@ public final class LocalSpawnRunner implements SpawnRunner { actionInputPrefetcher, localExecutionOptions, resourceManager, - /*useProcessWrapper=*/OS.getCurrent() != OS.WINDOWS, + OS.getCurrent() != OS.WINDOWS && getProcessWrapper(execRoot, OS.getCurrent()).exists(), OS.getCurrent(), productName, localEnvProvider); @@ -256,10 +257,10 @@ public final class LocalSpawnRunner implements SpawnRunner { if (useProcessWrapper) { List<String> cmdLine = new ArrayList<>(); cmdLine.add(processWrapper); - cmdLine.add(Float.toString(timeoutSeconds)); - cmdLine.add(Double.toString(localExecutionOptions.localSigkillGraceSeconds)); - cmdLine.add(getPathOrDevNull(outErr.getOutputPath())); - cmdLine.add(getPathOrDevNull(outErr.getErrorPath())); + cmdLine.add("--timeout=" + timeoutSeconds); + 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]), diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java index 6f29578dd8..6a6dbbc66a 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java @@ -255,10 +255,6 @@ public class RunCommand implements BlazeCommand { Preconditions.checkNotNull( processWrapperPath, PROCESS_WRAPPER + " not found in embedded tools"); cmdLine.add(env.getExecRoot().getRelative(processWrapperPath).getPathString()); - cmdLine.add("-1"); - cmdLine.add("15"); - cmdLine.add("-"); - cmdLine.add("-"); } List<String> prettyCmdLine = new ArrayList<>(); // Insert the command prefix specified by the "--run_under=<command-prefix>" option diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java index e96c417980..9d8e5acfe9 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.shell.Command; import com.google.devtools.build.lib.shell.CommandException; import com.google.devtools.build.lib.shell.CommandResult; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; @@ -48,6 +49,10 @@ import java.util.concurrent.atomic.AtomicReference; ) public class DarwinSandboxedStrategy extends SandboxStrategy { + public static boolean isSupported(CommandEnvironment cmdEnv) { + return OS.getCurrent() == OS.DARWIN && DarwinSandboxRunner.isSupported(cmdEnv); + } + private final Path execRoot; private final boolean sandboxDebug; private final boolean verboseFailures; 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 ec4a5e5d9b..1d6e5ca4fd 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 @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.buildtool.BuildRequest; import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; @@ -47,7 +48,7 @@ import java.util.concurrent.atomic.AtomicReference; public class LinuxSandboxedStrategy extends SandboxStrategy { public static boolean isSupported(CommandEnvironment cmdEnv) { - return LinuxSandboxRunner.isSupported(cmdEnv); + return OS.getCurrent() == OS.LINUX && LinuxSandboxRunner.isSupported(cmdEnv); } private final SandboxOptions sandboxOptions; diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperRunner.java index 71ceebac73..e6a3061da5 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperRunner.java @@ -66,10 +66,8 @@ final class ProcessWrapperRunner extends SandboxRunner { CommandEnvironment cmdEnv, List<String> spawnArguments, int timeout) { List<String> commandLineArgs = new ArrayList<>(5 + spawnArguments.size()); commandLineArgs.add(getProcessWrapper(cmdEnv).getPathString()); - commandLineArgs.add(Integer.toString(timeout)); - commandLineArgs.add("5"); /* kill delay: give some time to print stacktraces and whatnot. */ - commandLineArgs.add("-"); /* stdout. */ - commandLineArgs.add("-"); /* stderr. */ + commandLineArgs.add("--timeout=" + timeout); + commandLineArgs.add("--kill_delay=5"); /* give some time to print stacktraces and whatnot. */ commandLineArgs.addAll(spawnArguments); return commandLineArgs; } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java index 977916cb54..0ccdd989e3 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java @@ -43,7 +43,7 @@ import java.util.concurrent.atomic.AtomicReference; public class ProcessWrapperSandboxedStrategy extends SandboxStrategy { public static boolean isSupported(CommandEnvironment cmdEnv) { - return ProcessWrapperRunner.isSupported(cmdEnv); + return OS.isPosixCompatible() && ProcessWrapperRunner.isSupported(cmdEnv); } private final SandboxOptions sandboxOptions; diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextConsumer.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextConsumer.java index 1bbeacf863..653c9dafa7 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextConsumer.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextConsumer.java @@ -21,7 +21,6 @@ import com.google.devtools.build.lib.actions.ActionContext; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.exec.ActionContextConsumer; import com.google.devtools.build.lib.runtime.CommandEnvironment; -import com.google.devtools.build.lib.util.OS; /** * {@link ActionContextConsumer} that requests the action contexts necessary for sandboxed @@ -37,9 +36,9 @@ final class SandboxActionContextConsumer implements ActionContextConsumer { ImmutableMultimap.builder(); ImmutableMap.Builder<String, String> spawnContexts = ImmutableMap.builder(); - if ((OS.getCurrent() == OS.LINUX && LinuxSandboxedStrategy.isSupported(cmdEnv)) - || (OS.getCurrent() == OS.DARWIN && DarwinSandboxRunner.isSupported(cmdEnv)) - || (OS.isPosixCompatible() && ProcessWrapperSandboxedStrategy.isSupported(cmdEnv))) { + if (LinuxSandboxedStrategy.isSupported(cmdEnv) + || DarwinSandboxedStrategy.isSupported(cmdEnv) + || ProcessWrapperSandboxedStrategy.isSupported(cmdEnv)) { // This makes the "sandboxed" strategy available via --spawn_strategy=sandboxed, // but it is not necessarily the default. contexts.put(SpawnActionContext.class, "sandboxed"); 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 adcc21462f..d9829a2cb7 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 @@ -20,7 +20,6 @@ import com.google.devtools.build.lib.buildtool.BuildRequest; import com.google.devtools.build.lib.exec.ActionContextProvider; import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.runtime.CommandEnvironment; -import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; @@ -41,29 +40,25 @@ final class SandboxActionContextProvider extends ActionContextProvider { boolean verboseFailures = buildRequest.getOptions(ExecutionOptions.class).verboseFailures; String productName = cmdEnv.getRuntime().getProductName(); - // The ProcessWrapperSandboxedStrategy works on all POSIX-compatible operating systems. - if (OS.isPosixCompatible()) { + // This works on most platforms, but isn't the best choice, so we put it first and let later + // platform-specific sandboxing strategies become the default. + if (ProcessWrapperSandboxedStrategy.isSupported(cmdEnv)) { contexts.add( new ProcessWrapperSandboxedStrategy( cmdEnv, buildRequest, sandboxBase, verboseFailures, productName)); } - switch (OS.getCurrent()) { - case LINUX: - if (LinuxSandboxedStrategy.isSupported(cmdEnv)) { - contexts.add( - LinuxSandboxedStrategy.create(cmdEnv, buildRequest, sandboxBase, verboseFailures)); - } - break; - case DARWIN: - if (DarwinSandboxRunner.isSupported(cmdEnv)) { - contexts.add( - DarwinSandboxedStrategy.create( - cmdEnv, buildRequest, sandboxBase, verboseFailures, productName)); - } - break; - default: - // No additional platform-specific sandboxing available. + // This is the preferred sandboxing strategy on Linux. + if (LinuxSandboxedStrategy.isSupported(cmdEnv)) { + contexts.add( + LinuxSandboxedStrategy.create(cmdEnv, buildRequest, sandboxBase, verboseFailures)); + } + + // This is the preferred sandboxing strategy on macOS. + if (DarwinSandboxedStrategy.isSupported(cmdEnv)) { + contexts.add( + DarwinSandboxedStrategy.create( + cmdEnv, buildRequest, sandboxBase, verboseFailures, productName)); } return new SandboxActionContextProvider(contexts.build()); diff --git a/src/main/tools/BUILD b/src/main/tools/BUILD index 4aa0408d12..dc788a4c54 100644 --- a/src/main/tools/BUILD +++ b/src/main/tools/BUILD @@ -23,6 +23,8 @@ cc_binary( "process-wrapper.h", "process-wrapper-legacy.cc", "process-wrapper-legacy.h", + "process-wrapper-options.cc", + "process-wrapper-options.h", ], }), linkopts = ["-lm"], diff --git a/src/main/tools/process-wrapper-legacy.cc b/src/main/tools/process-wrapper-legacy.cc index 2c6e6d224a..ac7b3ec5be 100644 --- a/src/main/tools/process-wrapper-legacy.cc +++ b/src/main/tools/process-wrapper-legacy.cc @@ -25,6 +25,7 @@ #include "src/main/tools/logging.h" #include "src/main/tools/process-tools.h" +#include "src/main/tools/process-wrapper-options.h" #include "src/main/tools/process-wrapper.h" pid_t LegacyProcessWrapper::child_pid = 0; diff --git a/src/main/tools/process-wrapper-options.cc b/src/main/tools/process-wrapper-options.cc new file mode 100644 index 0000000000..cd03628b45 --- /dev/null +++ b/src/main/tools/process-wrapper-options.cc @@ -0,0 +1,131 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "src/main/tools/process-wrapper-options.h" + +#include <getopt.h> +#include <stdarg.h> +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <memory> +#include <string> +#include <vector> + +#include "src/main/tools/logging.h" + +struct Options opt; + +// Print out a usage error. argc and argv are the argument counter and vector, +// fmt is a format, string for the error message to print. +static void Usage(char *program_name, const char *fmt, ...) { + va_list ap; + va_start(ap, fmt); + vfprintf(stderr, fmt, ap); + va_end(ap); + + fprintf(stderr, "\nUsage: %s -- command arg1 @args\n", program_name); + fprintf( + stderr, + "\nPossible arguments:\n" + " -t/--timeout <timeout> timeout after which the child process will be " + "terminated with SIGTERM\n" + " -k/--kill_delay <timeout> in case timeout occurs, how long to wait " + "before killing the child with SIGKILL\n" + " -o/--stdout <file> redirect stdout to a file\n" + " -e/--stderr <file> redirect stderr to a file\n" + " -d/--debug if set, debug info will be printed\n" + " -- command to run inside sandbox, followed by arguments\n"); + exit(EXIT_FAILURE); +} + +static void ValidateIsAbsolutePath(char *path, char *program_name, char flag) { + if (path[0] != '/') { + Usage(program_name, "The -%c option must be used with absolute paths only.", + flag); + } +} + +// Parses command line flags from an argv array and puts the results into the +// global `opt` struct. +static void ParseCommandLine(const std::vector<char *> &args) { + static struct option long_options[] = { + {"timeout", required_argument, 0, 't'}, + {"kill_delay", required_argument, 0, 'k'}, + {"stdout", required_argument, 0, 'o'}, + {"stderr", required_argument, 0, 'e'}, + {"debug", no_argument, 0, 'd'}, + {0, 0, 0, 0}}; + extern char *optarg; + extern int optind, optopt; + int c; + + while ((c = getopt_long(args.size(), args.data(), "+:t:k:o:e:d", long_options, + nullptr)) != -1) { + switch (c) { + case 't': + if (sscanf(optarg, "%lf", &opt.timeout_secs) != 1) { + Usage(args.front(), "Invalid timeout (-t) value: %s", optarg); + } + break; + case 'k': + if (sscanf(optarg, "%lf", &opt.kill_delay_secs) != 1) { + Usage(args.front(), "Invalid kill delay (-k) value: %s", optarg); + } + break; + case 'o': + if (opt.stdout_path.empty()) { + opt.stdout_path.assign(optarg); + } else { + Usage(args.front(), + "Cannot redirect stdout (-o) to more than one destination."); + } + break; + case 'e': + if (opt.stderr_path.empty()) { + opt.stderr_path.assign(optarg); + } else { + Usage(args.front(), + "Cannot redirect stderr (-e) to more than one destination."); + } + break; + case 'd': + opt.debug = true; + break; + case '?': + Usage(args.front(), "Unrecognized argument: -%c (%d)", optopt, optind); + break; + case ':': + Usage(args.front(), "Flag -%c requires an argument", optopt); + break; + } + } + + if (optind < static_cast<int>(args.size())) { + opt.args.assign(args.begin() + optind, args.end()); + } +} + +void ParseOptions(int argc, char *argv[]) { + std::vector<char *> args(argv, argv + argc); + + ParseCommandLine(args); + + if (opt.args.empty()) { + Usage(args.front(), "No command specified."); + } + + // argv[] passed to execve() must be a null-terminated array. + opt.args.push_back(nullptr); +} diff --git a/src/main/tools/process-wrapper-options.h b/src/main/tools/process-wrapper-options.h new file mode 100644 index 0000000000..cb173027e2 --- /dev/null +++ b/src/main/tools/process-wrapper-options.h @@ -0,0 +1,42 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef SRC_MAIN_TOOLS_PROCESS_WRAPPER_OPTIONS_H_ +#define SRC_MAIN_TOOLS_PROCESS_WRAPPER_OPTIONS_H_ + +#include <string> +#include <vector> + +// Options parsing result. +struct Options { + // How long to wait before killing the child (-t) + double timeout_secs; + // How long to wait before sending SIGKILL in case of timeout (-k) + double kill_delay_secs; + // Where to redirect stdout (-o) + std::string stdout_path; + // Where to redirect stderr (-e) + std::string stderr_path; + // Whether to print debugging messages (-d) + bool debug; + // Command to run (--) + std::vector<char *> args; +}; + +extern struct Options opt; + +// Handles parsing all command line flags and populates the global opt struct. +void ParseOptions(int argc, char *argv[]); + +#endif diff --git a/src/main/tools/process-wrapper.cc b/src/main/tools/process-wrapper.cc index 09c1a88038..25765db833 100644 --- a/src/main/tools/process-wrapper.cc +++ b/src/main/tools/process-wrapper.cc @@ -41,44 +41,10 @@ #include "src/main/tools/logging.h" #include "src/main/tools/process-tools.h" #include "src/main/tools/process-wrapper-legacy.h" - -struct Options opt; - -// Print out a usage error and exit with EXIT_FAILURE. -static void Usage(char *program_name) { - fprintf(stderr, - "Usage: %s <timeout-secs> <kill-delay-secs> <stdout-redirect> " - "<stderr-redirect> <command> [args] ...\n", - program_name); - exit(EXIT_FAILURE); -} - -// Parse the command line flags and return the result in an Options structure -// passed as argument. -static void ParseCommandLine(std::vector<char *> args) { - if (args.size() < 5) { - Usage(args.front()); - } - - int optind = 1; - - if (sscanf(args[optind++], "%lf", &opt.timeout_secs) != 1) { - DIE("timeout_secs is not a real number.\n"); - } - if (sscanf(args[optind++], "%lf", &opt.kill_delay_secs) != 1) { - DIE("kill_delay_secs is not a real number.\n"); - } - opt.stdout_path.assign(args[optind++]); - opt.stderr_path.assign(args[optind++]); - opt.args.assign(args.begin() + optind, args.end()); - - // argv[] passed to execve() must be a null-terminated array. - opt.args.push_back(nullptr); -} +#include "src/main/tools/process-wrapper-options.h" int main(int argc, char *argv[]) { - std::vector<char *> args(argv, argv + argc); - ParseCommandLine(args); + ParseOptions(argc, argv); SwitchToEuid(); SwitchToEgid(); diff --git a/src/main/tools/process-wrapper.h b/src/main/tools/process-wrapper.h index 061eeace0e..fdd3948cf7 100644 --- a/src/main/tools/process-wrapper.h +++ b/src/main/tools/process-wrapper.h @@ -15,18 +15,4 @@ #ifndef SRC_MAIN_TOOLS_PROCESS_WRAPPER_H_ #define SRC_MAIN_TOOLS_PROCESS_WRAPPER_H_ -#include <string> -#include <vector> - -// Options parsing result. -struct Options { - double timeout_secs; - double kill_delay_secs; - std::string stdout_path; - std::string stderr_path; - std::vector<char *> args; -}; - -extern struct Options opt; - #endif 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 cb4bad6db6..7a23ab3d92 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 @@ -231,10 +231,15 @@ public class LocalSpawnRunnerTest { assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.findShortHostName()); assertThat(captor.getValue().getArgv()) - .isEqualTo(ImmutableList.of( - // process-wrapper timeout grace_time stdout stderr - "/execroot/_bin/process-wrapper", "123.0", "456.0", "/out/stdout", "/out/stderr", - "/bin/echo", "Hi!")); + .containsExactlyElementsIn( + ImmutableList.of( + "/execroot/_bin/process-wrapper", + "--timeout=123", + "--kill_delay=456", + "--stdout=/out/stdout", + "--stderr=/out/stderr", + "/bin/echo", + "Hi!")); assertThat(captor.getValue().getEnv()).containsExactly("VARIABLE", "value"); assertThat(captor.getValue().getTimeoutMillis()).isEqualTo(-1); @@ -266,7 +271,7 @@ public class LocalSpawnRunnerTest { assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.findShortHostName()); assertThat(captor.getValue().getArgv()) - .isEqualTo(ImmutableList.of("/bin/echo", "Hi!")); + .containsExactlyElementsIn(ImmutableList.of("/bin/echo", "Hi!")); assertThat(captor.getValue().getEnv()).containsExactly("VARIABLE", "value"); // Without the process wrapper, we use the Command API to enforce the timeout. assertThat(captor.getValue().getTimeoutMillis()).isEqualTo(timeoutMillis); @@ -295,10 +300,16 @@ public class LocalSpawnRunnerTest { assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.findShortHostName()); assertThat(captor.getValue().getArgv()) - .isEqualTo(ImmutableList.of( - // process-wrapper timeout grace_time stdout stderr - "/execroot/_bin/process-wrapper", "0.0", "15.0", "/out/stdout", "/out/stderr", - "/bin/echo", "Hi!")); + .containsExactlyElementsIn( + ImmutableList.of( + // process-wrapper timeout grace_time stdout stderr + "/execroot/_bin/process-wrapper", + "--timeout=0", + "--kill_delay=15", + "--stdout=/out/stdout", + "--stderr=/out/stderr", + "/bin/echo", + "Hi!")); assertThat(captor.getValue().getEnv()).containsExactly("VARIABLE", "value"); assertThat(calledLockOutputFiles).isTrue(); @@ -500,9 +511,15 @@ public class LocalSpawnRunnerTest { assertThat(result.status()).isEqualTo(SpawnResult.Status.SUCCESS); assertThat(captor.getValue().getArgv()) - .isEqualTo(ImmutableList.of( - // process-wrapper timeout grace_time stdout stderr - "/execroot/_bin/process-wrapper.exe", "321.0", "654.0", "/out/stdout", "/out/stderr", - "/bin/echo", "Hi!")); + .containsExactlyElementsIn( + ImmutableList.of( + // process-wrapper timeout grace_time stdout stderr + "/execroot/_bin/process-wrapper.exe", + "--timeout=321", + "--kill_delay=654", + "--stdout=/out/stdout", + "--stderr=/out/stderr", + "/bin/echo", + "Hi!")); } } diff --git a/src/test/shell/bazel/process-wrapper_test.sh b/src/test/shell/bazel/process-wrapper_test.sh index 09c6a088e4..63972acf28 100755 --- a/src/test/shell/bazel/process-wrapper_test.sh +++ b/src/test/shell/bazel/process-wrapper_test.sh @@ -41,37 +41,38 @@ function assert_output() { } function test_basic_functionality() { - $process_wrapper -1 0 $OUT $ERR /bin/echo hi there &> $TEST_log || fail + $process_wrapper --stdout=$OUT --stderr=$ERR /bin/echo hi there &> $TEST_log || fail assert_output "hi there" "" } function test_to_stderr() { - $process_wrapper -1 0 $OUT $ERR /bin/bash -c "/bin/echo hi there >&2" &> $TEST_log || fail + $process_wrapper --stdout=$OUT --stderr=$ERR /bin/bash -c "/bin/echo hi there >&2" &> $TEST_log || fail assert_output "" "hi there" } function test_exit_code() { local code=0 - $process_wrapper -1 0 $OUT $ERR /bin/bash -c "exit 71" &> $TEST_log || code=$? + $process_wrapper --stdout=$OUT --stderr=$ERR /bin/bash -c "exit 71" &> $TEST_log || code=$? assert_equals 71 "$code" } function test_signal_death() { local code=0 - $process_wrapper -1 0 $OUT $ERR /bin/bash -c 'kill -ABRT $$' &> $TEST_log || code=$? + $process_wrapper --stdout=$OUT --stderr=$ERR /bin/bash -c 'kill -ABRT $$' &> $TEST_log || code=$? assert_equals 134 "$code" # SIGNAL_BASE + SIGABRT = 128 + 6 } function test_signal_catcher() { local code=0 - $process_wrapper 1 2 $OUT $ERR /bin/bash -c \ + $process_wrapper --timeout=1 --kill_delay=2 --stdout=$OUT --stderr=$ERR /bin/bash -c \ 'trap "echo later; exit 0" SIGINT SIGTERM SIGALRM; sleep 10' &> $TEST_log || code=$? assert_equals 142 "$code" # SIGNAL_BASE + SIGALRM = 128 + 14 assert_stdout "later" } function test_basic_timeout() { - $process_wrapper 1 2 $OUT $ERR /bin/bash -c "echo before; sleep 10; echo after" &> $TEST_log && fail + $process_wrapper --timeout=1 --kill_delay=2 --stdout=$OUT --stderr=$ERR /bin/bash -c \ + "echo before; sleep 10; echo after" &> $TEST_log && fail assert_stdout "before" } @@ -81,7 +82,7 @@ function test_basic_timeout() { # grace period, thus printing "beforeafter". function test_timeout_grace() { local code=0 - $process_wrapper 1 10 $OUT $ERR /bin/bash -c \ + $process_wrapper --timeout=1 --kill_delay=10 --stdout=$OUT --stderr=$ERR /bin/bash -c \ 'trap "echo -n "before"; sleep 1; echo "after"; exit 0" SIGINT SIGTERM SIGALRM; sleep 10' \ &> $TEST_log || code=$? assert_equals 142 "$code" # SIGNAL_BASE + SIGALRM = 128 + 14 @@ -94,7 +95,7 @@ function test_timeout_grace() { # trap takes longer than the grace period, thus only printing "before". function test_timeout_kill() { local code=0 - $process_wrapper 1 2 $OUT $ERR /bin/bash -c \ + $process_wrapper --timeout=1 --kill_delay=2 --stdout=$OUT --stderr=$ERR /bin/bash -c \ 'trap "echo before; sleep 10; echo after; exit 0" SIGINT SIGTERM SIGALRM; sleep 10' \ &> $TEST_log || code=$? assert_equals 142 "$code" # SIGNAL_BASE + SIGALRM = 128 + 14 @@ -103,7 +104,7 @@ function test_timeout_kill() { function test_execvp_error_message() { local code=0 - $process_wrapper -1 0 $OUT $ERR /bin/notexisting &> $TEST_log || code=$? + $process_wrapper --stdout=$OUT --stderr=$ERR /bin/notexisting &> $TEST_log || code=$? assert_equals 1 "$code" assert_contains "\"execvp(/bin/notexisting, ...)\": No such file or directory" "$ERR" } |