diff options
27 files changed, 280 insertions, 267 deletions
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 b10a62f6ec..c9575d0679 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 @@ -48,7 +48,6 @@ import java.time.Duration; import java.util.EnumMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -249,7 +248,7 @@ public class LocalSpawnRunner implements SpawnRunner { setState(State.LOCAL_ACTION_RUNNING); Path tmpDir = createActionTemp(execRoot); - Optional<String> statisticsPath = Optional.empty(); + Path statisticsPath = null; try { Command cmd; OutputStream stdOut; @@ -274,8 +273,8 @@ public class LocalSpawnRunner implements SpawnRunner { .setTimeout(policy.getTimeout()) .setKillDelay(Duration.ofSeconds(localExecutionOptions.localSigkillGraceSeconds)); if (localExecutionOptions.collectLocalExecutionStatistics) { - statisticsPath = Optional.of(tmpDir.getRelative("stats.out").getPathString()); - commandLineBuilder.setStatisticsPath(statisticsPath.get()); + statisticsPath = tmpDir.getRelative("stats.out"); + commandLineBuilder.setStatisticsPath(statisticsPath); } List<String> cmdLine = commandLineBuilder.build(); cmd = @@ -343,19 +342,19 @@ public class LocalSpawnRunner implements SpawnRunner { .setExitCode(exitCode) .setExecutorHostname(hostName) .setWallTime(wallTime); - if (statisticsPath.isPresent()) { - Optional<ExecutionStatistics.ResourceUsage> resourceUsage = - ExecutionStatistics.getResourceUsage(statisticsPath.get()); - if (resourceUsage.isPresent()) { - spawnResultBuilder.setUserTime(resourceUsage.get().getUserExecutionTime()); - spawnResultBuilder.setSystemTime(resourceUsage.get().getSystemExecutionTime()); - spawnResultBuilder.setNumBlockOutputOperations( - resourceUsage.get().getBlockOutputOperations()); - spawnResultBuilder.setNumBlockInputOperations( - resourceUsage.get().getBlockInputOperations()); - spawnResultBuilder.setNumInvoluntaryContextSwitches( - resourceUsage.get().getInvoluntaryContextSwitches()); - } + if (statisticsPath != null) { + ExecutionStatistics.getResourceUsage(statisticsPath) + .ifPresent( + resourceUsage -> { + spawnResultBuilder.setUserTime(resourceUsage.getUserExecutionTime()); + spawnResultBuilder.setSystemTime(resourceUsage.getSystemExecutionTime()); + spawnResultBuilder.setNumBlockOutputOperations( + resourceUsage.getBlockOutputOperations()); + spawnResultBuilder.setNumBlockInputOperations( + resourceUsage.getBlockInputOperations()); + spawnResultBuilder.setNumInvoluntaryContextSwitches( + resourceUsage.getInvoluntaryContextSwitches()); + }); } return spawnResultBuilder.build(); } finally { @@ -378,8 +377,8 @@ public class LocalSpawnRunner implements SpawnRunner { } } - private String getPathOrDevNull(Path path) { - return path == null ? "/dev/null" : path.getPathString(); + private Path getPathOrDevNull(Path path) { + return path == null ? execRoot.getRelative("/dev/null") : path; } private boolean wasTimeout(Duration timeout, Duration wallTime) { diff --git a/src/main/java/com/google/devtools/build/lib/profiler/BUILD b/src/main/java/com/google/devtools/build/lib/profiler/BUILD index e0a8dfb7b6..28e7c7b417 100644 --- a/src/main/java/com/google/devtools/build/lib/profiler/BUILD +++ b/src/main/java/com/google/devtools/build/lib/profiler/BUILD @@ -18,7 +18,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/clock", "//src/main/java/com/google/devtools/build/lib/collect", "//src/main/java/com/google/devtools/build/lib/concurrent", - "//src/main/java/com/google/devtools/build/lib/shell", "//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/runtime/ProcessWrapperUtil.java b/src/main/java/com/google/devtools/build/lib/runtime/ProcessWrapperUtil.java index 17ee702fdd..b83025969d 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/ProcessWrapperUtil.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/ProcessWrapperUtil.java @@ -20,7 +20,6 @@ import com.google.devtools.build.lib.vfs.PathFragment; import java.time.Duration; import java.util.ArrayList; import java.util.List; -import java.util.Optional; /** * Utility functions for the process wrapper embedded tool, which should work on most platforms and @@ -55,11 +54,11 @@ public final class ProcessWrapperUtil { public static class CommandLineBuilder { private final String processWrapperPath; private final List<String> commandArguments; - private Optional<String> stdoutPath = Optional.empty(); - private Optional<String> stderrPath = Optional.empty(); - private Optional<Duration> timeout = Optional.empty(); - private Optional<Duration> killDelay = Optional.empty(); - private Optional<String> statisticsPath = Optional.empty(); + private Path stdoutPath; + private Path stderrPath; + private Duration timeout; + private Duration killDelay; + private Path statisticsPath; private CommandLineBuilder(String processWrapperPath, List<String> commandArguments) { this.processWrapperPath = processWrapperPath; @@ -67,20 +66,20 @@ public final class ProcessWrapperUtil { } /** Sets the path to use for redirecting stdout, if any. */ - public CommandLineBuilder setStdoutPath(String stdoutPath) { - this.stdoutPath = Optional.of(stdoutPath); + public CommandLineBuilder setStdoutPath(Path stdoutPath) { + this.stdoutPath = stdoutPath; return this; } /** Sets the path to use for redirecting stderr, if any. */ - public CommandLineBuilder setStderrPath(String stderrPath) { - this.stderrPath = Optional.of(stderrPath); + public CommandLineBuilder setStderrPath(Path stderrPath) { + this.stderrPath = stderrPath; return this; } /** Sets the timeout for the command run using the process-wrapper tool. */ public CommandLineBuilder setTimeout(Duration timeout) { - this.timeout = Optional.of(timeout); + this.timeout = timeout; return this; } @@ -89,13 +88,13 @@ public final class ProcessWrapperUtil { * timeout. */ public CommandLineBuilder setKillDelay(Duration killDelay) { - this.killDelay = Optional.of(killDelay); + this.killDelay = killDelay; return this; } /** Sets the path for writing execution statistics (e.g. resource usage). */ - public CommandLineBuilder setStatisticsPath(String statisticsPath) { - this.statisticsPath = Optional.of(statisticsPath); + public CommandLineBuilder setStatisticsPath(Path statisticsPath) { + this.statisticsPath = statisticsPath; return this; } @@ -104,20 +103,20 @@ public final class ProcessWrapperUtil { List<String> fullCommandLine = new ArrayList<>(); fullCommandLine.add(processWrapperPath); - if (timeout.isPresent()) { - fullCommandLine.add("--timeout=" + timeout.get().getSeconds()); + if (timeout != null) { + fullCommandLine.add("--timeout=" + timeout.getSeconds()); } - if (killDelay.isPresent()) { - fullCommandLine.add("--kill_delay=" + killDelay.get().getSeconds()); + if (killDelay != null) { + fullCommandLine.add("--kill_delay=" + killDelay.getSeconds()); } - if (stdoutPath.isPresent()) { - fullCommandLine.add("--stdout=" + stdoutPath.get()); + if (stdoutPath != null) { + fullCommandLine.add("--stdout=" + stdoutPath); } - if (stderrPath.isPresent()) { - fullCommandLine.add("--stderr=" + stderrPath.get()); + if (stderrPath != null) { + fullCommandLine.add("--stderr=" + stderrPath); } - if (statisticsPath.isPresent()) { - fullCommandLine.add("--stats=" + statisticsPath.get()); + if (statisticsPath != null) { + fullCommandLine.add("--stats=" + statisticsPath); } fullCommandLine.addAll(commandArguments); 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 6f7046061a..17db773bf8 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 @@ -212,8 +212,8 @@ public class RunCommand implements BlazeCommand { // on that platform. Also we skip it when writing the command-line to a file instead // of executing it directly. if (OS.getCurrent() != OS.WINDOWS && runOptions.scriptPath == null) { - Preconditions.checkState(ProcessWrapperUtil.isSupported(env), - "process-wraper not found in embedded tools"); + Preconditions.checkState( + ProcessWrapperUtil.isSupported(env), "process-wrapper not found in embedded tools"); cmdLine.add(ProcessWrapperUtil.getProcessWrapper(env).getPathString()); } List<String> prettyCmdLine = new ArrayList<>(); 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 29c46b2e8d..33b21209da 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 @@ -41,7 +41,6 @@ import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; import java.time.Duration; import java.util.Map; -import java.util.Optional; /** Abstract common ancestor for sandbox spawn runners implementing the common parts. */ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { @@ -91,7 +90,7 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { Path execRoot, Path tmpDir, Duration timeout, - Optional<String> statisticsPath) + Path statisticsPath) throws IOException, InterruptedException { try { sandbox.createFileSystem(); @@ -123,7 +122,7 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { Duration timeout, Path execRoot, Path tmpDir, - Optional<String> statisticsPath) + Path statisticsPath) throws IOException, InterruptedException { Command cmd = new Command( sandbox.getArguments().toArray(new String[0]), @@ -192,19 +191,19 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { .setWallTime(wallTime) .setFailureMessage(status != Status.SUCCESS || exitCode != 0 ? failureMessage : ""); - if (statisticsPath.isPresent()) { - Optional<ExecutionStatistics.ResourceUsage> resourceUsage = - ExecutionStatistics.getResourceUsage(statisticsPath.get()); - if (resourceUsage.isPresent()) { - spawnResultBuilder.setUserTime(resourceUsage.get().getUserExecutionTime()); - spawnResultBuilder.setSystemTime(resourceUsage.get().getSystemExecutionTime()); - spawnResultBuilder.setNumBlockOutputOperations( - resourceUsage.get().getBlockOutputOperations()); - spawnResultBuilder.setNumBlockInputOperations( - resourceUsage.get().getBlockInputOperations()); - spawnResultBuilder.setNumInvoluntaryContextSwitches( - resourceUsage.get().getInvoluntaryContextSwitches()); - } + if (statisticsPath != null) { + ExecutionStatistics.getResourceUsage(statisticsPath) + .ifPresent( + resourceUsage -> { + spawnResultBuilder.setUserTime(resourceUsage.getUserExecutionTime()); + spawnResultBuilder.setSystemTime(resourceUsage.getSystemExecutionTime()); + spawnResultBuilder.setNumBlockOutputOperations( + resourceUsage.getBlockOutputOperations()); + spawnResultBuilder.setNumBlockInputOperations( + resourceUsage.getBlockInputOperations()); + spawnResultBuilder.setNumInvoluntaryContextSwitches( + resourceUsage.getInvoluntaryContextSwitches()); + }); } return spawnResultBuilder.build(); 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 daaf92a5e7..d18cd10d65 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD +++ b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD @@ -23,6 +23,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/exec/apple", "//src/main/java/com/google/devtools/build/lib/exec/local", "//src/main/java/com/google/devtools/build/lib/exec/local:options", 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 109f5e4443..1060034ccc 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 @@ -48,7 +48,6 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import javax.annotation.Nullable; @@ -97,7 +96,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { private final Path execRoot; private final boolean allowNetwork; private final Path processWrapper; - private final Optional<Duration> timeoutKillDelay; + private final Duration timeoutKillDelay; private final @Nullable SandboxfsProcess sandboxfsProcess; /** @@ -114,15 +113,14 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { * * @param cmdEnv the command environment to use * @param sandboxBase path to the sandbox base directory - * @param timeoutKillDelay an optional, additional grace period before killing timing out - * commands. If not present, then no grace period is used and commands are killed instantly. + * @param timeoutKillDelay additional grace period before killing timing out commands * @param sandboxfsProcess instance of the sandboxfs process to use; may be null for none, in * which case the runner uses a symlinked sandbox */ DarwinSandboxedSpawnRunner( CommandEnvironment cmdEnv, Path sandboxBase, - Optional<Duration> timeoutKillDelay, + Duration timeoutKillDelay, @Nullable SandboxfsProcess sandboxfsProcess) throws IOException { super(cmdEnv, sandboxBase); @@ -224,16 +222,14 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { ProcessWrapperUtil.commandLineBuilder(processWrapper.getPathString(), spawn.getArguments()) .setTimeout(timeout); - if (timeoutKillDelay.isPresent()) { - processWrapperCommandLineBuilder.setKillDelay(timeoutKillDelay.get()); - } + processWrapperCommandLineBuilder.setKillDelay(timeoutKillDelay); - final Optional<String> statisticsPath; + final Path statisticsPath; if (getSandboxOptions().collectLocalSandboxExecutionStatistics) { - statisticsPath = Optional.of(sandboxPath.getRelative("stats.out").getPathString()); - processWrapperCommandLineBuilder.setStatisticsPath(statisticsPath.get()); + statisticsPath = sandboxPath.getRelative("stats.out"); + processWrapperCommandLineBuilder.setStatisticsPath(statisticsPath); } else { - statisticsPath = Optional.empty(); + statisticsPath = null; } ImmutableList<String> commandLine = @@ -300,7 +296,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { Set<Path> writableDirs, Set<Path> inaccessiblePaths, boolean allowNetwork, - Optional<String> statisticsPath) + Path statisticsPath) throws IOException { try (PrintWriter out = new PrintWriter( @@ -325,8 +321,8 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { for (Path path : writableDirs) { out.println(" (subpath \"" + path.getPathString() + "\")"); } - if (statisticsPath.isPresent()) { - out.println(" (literal \"" + statisticsPath.get() + "\")"); + if (statisticsPath != null) { + out.println(" (literal \"" + statisticsPath.getPathString() + "\")"); } out.println(")"); diff --git a/src/main/java/com/google/devtools/build/lib/runtime/LinuxSandboxUtil.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxUtil.java index 97d6427400..ca5dd15607 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/LinuxSandboxUtil.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxUtil.java @@ -12,18 +12,20 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.devtools.build.lib.runtime; +package com.google.devtools.build.lib.sandbox; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.util.OsUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.time.Duration; -import java.util.Collection; import java.util.List; import java.util.Map; -import java.util.Optional; +import java.util.Set; /** Utility functions for the {@code linux-sandbox} embedded tool. */ public final class LinuxSandboxUtil { @@ -44,7 +46,7 @@ public final class LinuxSandboxUtil { /** Returns a new command line builder for the {@code linux-sandbox} tool. */ public static CommandLineBuilder commandLineBuilder( - String linuxSandboxPath, Collection<String> commandArguments) { + Path linuxSandboxPath, List<String> commandArguments) { return new CommandLineBuilder() .setLinuxSandboxPath(linuxSandboxPath) .setCommandArguments(commandArguments); @@ -55,43 +57,42 @@ public final class LinuxSandboxUtil { * linux-sandbox} tool. */ public static class CommandLineBuilder { - // TODO(b/62588075): Reconsider using Path/PathFragment instead of Strings for file paths. - private Optional<String> linuxSandboxPath = Optional.empty(); - private Optional<String> workingDirectory = Optional.empty(); - private Optional<Duration> timeout = Optional.empty(); - private Optional<Duration> killDelay = Optional.empty(); - private Optional<String> stdoutPath = Optional.empty(); - private Optional<String> stderrPath = Optional.empty(); - private Optional<Iterable<Path>> writableFilesAndDirectories = Optional.empty(); - private Optional<Iterable<Path>> tmpfsDirectories = Optional.empty(); - private Optional<Map<Path, Path>> bindMounts = Optional.empty(); - private Optional<String> statisticsPath = Optional.empty(); + private Path linuxSandboxPath; + private Path workingDirectory; + private Duration timeout; + private Duration killDelay; + private Path stdoutPath; + private Path stderrPath; + private Set<Path> writableFilesAndDirectories = ImmutableSet.of(); + private Set<Path> tmpfsDirectories = ImmutableSet.of(); + private Map<Path, Path> bindMounts = ImmutableMap.of(); + private Path statisticsPath; private boolean useFakeHostname = false; private boolean createNetworkNamespace = false; private boolean useFakeRoot = false; private boolean useFakeUsername = false; private boolean useDebugMode = false; - private Optional<Collection<String>> commandArguments = Optional.empty(); + private List<String> commandArguments = ImmutableList.of(); private CommandLineBuilder() { // Prevent external construction via "new". } /** Sets the path of the {@code linux-sandbox} tool. */ - public CommandLineBuilder setLinuxSandboxPath(String linuxSandboxPath) { - this.linuxSandboxPath = Optional.of(linuxSandboxPath); + public CommandLineBuilder setLinuxSandboxPath(Path linuxSandboxPath) { + this.linuxSandboxPath = linuxSandboxPath; return this; } /** Sets the working directory to use, if any. */ - public CommandLineBuilder setWorkingDirectory(String workingDirectory) { - this.workingDirectory = Optional.of(workingDirectory); + public CommandLineBuilder setWorkingDirectory(Path workingDirectory) { + this.workingDirectory = workingDirectory; return this; } /** Sets the timeout for the command run using the {@code linux-sandbox} tool. */ public CommandLineBuilder setTimeout(Duration timeout) { - this.timeout = Optional.of(timeout); + this.timeout = timeout; return this; } @@ -100,32 +101,32 @@ public final class LinuxSandboxUtil { * timeout. */ public CommandLineBuilder setKillDelay(Duration killDelay) { - this.killDelay = Optional.of(killDelay); + this.killDelay = killDelay; return this; } /** Sets the path to use for redirecting stdout, if any. */ - public CommandLineBuilder setStdoutPath(String stdoutPath) { - this.stdoutPath = Optional.of(stdoutPath); + public CommandLineBuilder setStdoutPath(Path stdoutPath) { + this.stdoutPath = stdoutPath; return this; } /** Sets the path to use for redirecting stderr, if any. */ - public CommandLineBuilder setStderrPath(String stderrPath) { - this.stderrPath = Optional.of(stderrPath); + public CommandLineBuilder setStderrPath(Path stderrPath) { + this.stderrPath = stderrPath; return this; } /** Sets the files or directories to make writable for the sandboxed process, if any. */ public CommandLineBuilder setWritableFilesAndDirectories( - Iterable<Path> writableFilesAndDirectories) { - this.writableFilesAndDirectories = Optional.of(writableFilesAndDirectories); + Set<Path> writableFilesAndDirectories) { + this.writableFilesAndDirectories = writableFilesAndDirectories; return this; } /** Sets the directories where to mount an empty tmpfs, if any. */ - public CommandLineBuilder setTmpfsDirectories(Iterable<Path> tmpfsDirectories) { - this.tmpfsDirectories = Optional.of(tmpfsDirectories); + public CommandLineBuilder setTmpfsDirectories(Set<Path> tmpfsDirectories) { + this.tmpfsDirectories = tmpfsDirectories; return this; } @@ -134,13 +135,13 @@ public final class LinuxSandboxUtil { * if any. */ public CommandLineBuilder setBindMounts(Map<Path, Path> bindMounts) { - this.bindMounts = Optional.of(bindMounts); + this.bindMounts = bindMounts; return this; } /** Sets the path for writing execution statistics (e.g. resource usage). */ - public CommandLineBuilder setStatisticsPath(String statisticsPath) { - this.statisticsPath = Optional.of(statisticsPath); + public CommandLineBuilder setStatisticsPath(Path statisticsPath) { + this.statisticsPath = statisticsPath; return this; } @@ -175,61 +176,55 @@ public final class LinuxSandboxUtil { } /** Sets the command (and its arguments) to run using the {@code linux-sandbox} tool. */ - public CommandLineBuilder setCommandArguments(Collection<String> commandArguments) { - this.commandArguments = Optional.of(commandArguments); + public CommandLineBuilder setCommandArguments(List<String> commandArguments) { + this.commandArguments = commandArguments; return this; } /** * Builds the command line to invoke a specific command using the {@code linux-sandbox} tool. */ - public List<String> build() { - Preconditions.checkState(this.linuxSandboxPath.isPresent(), "linuxSandboxPath is required"); - Preconditions.checkState(this.commandArguments.isPresent(), "commandArguments are required"); + public ImmutableList<String> build() { + Preconditions.checkNotNull(this.linuxSandboxPath, "linuxSandboxPath is required"); + Preconditions.checkState(!this.commandArguments.isEmpty(), "commandArguments are required"); Preconditions.checkState( !(this.useFakeUsername && this.useFakeRoot), "useFakeUsername and useFakeRoot are exclusive"); - ImmutableList.Builder<String> commandLineBuilder = ImmutableList.<String>builder(); + ImmutableList.Builder<String> commandLineBuilder = ImmutableList.builder(); - commandLineBuilder.add(linuxSandboxPath.get()); - if (workingDirectory.isPresent()) { - commandLineBuilder.add("-W", workingDirectory.get()); + commandLineBuilder.add(linuxSandboxPath.getPathString()); + if (workingDirectory != null) { + commandLineBuilder.add("-W", workingDirectory.getPathString()); } - if (timeout.isPresent()) { - commandLineBuilder.add("-T", Long.toString(timeout.get().getSeconds())); + if (timeout != null) { + commandLineBuilder.add("-T", Long.toString(timeout.getSeconds())); } - if (killDelay.isPresent()) { - commandLineBuilder.add("-t", Long.toString(killDelay.get().getSeconds())); + if (killDelay != null) { + commandLineBuilder.add("-t", Long.toString(killDelay.getSeconds())); } - if (stdoutPath.isPresent()) { - commandLineBuilder.add("-l", stdoutPath.get()); + if (stdoutPath != null) { + commandLineBuilder.add("-l", stdoutPath.getPathString()); } - if (stderrPath.isPresent()) { - commandLineBuilder.add("-L", stderrPath.get()); + if (stderrPath != null) { + commandLineBuilder.add("-L", stderrPath.getPathString()); } - if (writableFilesAndDirectories.isPresent()) { - for (Path writablePath : writableFilesAndDirectories.get()) { - commandLineBuilder.add("-w", writablePath.getPathString()); - } + for (Path writablePath : writableFilesAndDirectories) { + commandLineBuilder.add("-w", writablePath.getPathString()); } - if (tmpfsDirectories.isPresent()) { - for (Path tmpfsPath : tmpfsDirectories.get()) { - commandLineBuilder.add("-e", tmpfsPath.getPathString()); - } + for (Path tmpfsPath : tmpfsDirectories) { + commandLineBuilder.add("-e", tmpfsPath.getPathString()); } - if (bindMounts.isPresent()) { - for (Path bindMountTarget : bindMounts.get().keySet()) { - Path bindMountSource = bindMounts.get().get(bindMountTarget); - commandLineBuilder.add("-M", bindMountSource.getPathString()); - // The file is mounted in a custom location inside the sandbox. - if (!bindMountSource.equals(bindMountTarget)) { - commandLineBuilder.add("-m", bindMountTarget.getPathString()); - } + for (Path bindMountTarget : bindMounts.keySet()) { + Path bindMountSource = bindMounts.get(bindMountTarget); + commandLineBuilder.add("-M", bindMountSource.getPathString()); + // The file is mounted in a custom location inside the sandbox. + if (!bindMountSource.equals(bindMountTarget)) { + commandLineBuilder.add("-m", bindMountTarget.getPathString()); } } - if (statisticsPath.isPresent()) { - commandLineBuilder.add("-S", statisticsPath.get()); + if (statisticsPath != null) { + commandLineBuilder.add("-S", statisticsPath.getPathString()); } if (useFakeHostname) { commandLineBuilder.add("-H"); @@ -247,18 +242,9 @@ public final class LinuxSandboxUtil { commandLineBuilder.add("-D"); } commandLineBuilder.add("--"); - commandLineBuilder.addAll(commandArguments.get()); + commandLineBuilder.addAll(commandArguments); return commandLineBuilder.build(); } - - /** - * Builds the command line to invoke a specific command using the {@code linux-sandbox} tool. - * - * @return the command line as an array of strings - */ - public String[] buildAsArray() { - return build().toArray(new String[0]); - } } } 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 7db25c861d..e4c5ba6524 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 @@ -29,7 +29,6 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.exec.local.LocalEnvProvider; import com.google.devtools.build.lib.exec.local.PosixLocalEnvProvider; import com.google.devtools.build.lib.runtime.CommandEnvironment; -import com.google.devtools.build.lib.runtime.LinuxSandboxUtil; import com.google.devtools.build.lib.shell.Command; import com.google.devtools.build.lib.shell.CommandException; import com.google.devtools.build.lib.util.OS; @@ -40,10 +39,7 @@ import com.google.devtools.build.lib.vfs.Symlinks; import java.io.File; import java.io.IOException; import java.time.Duration; -import java.util.List; import java.util.Map; -import java.util.Optional; -import java.util.Set; import java.util.SortedMap; /** Spawn runner that uses linux sandboxing APIs to execute a local subprocess. */ @@ -57,10 +53,9 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { return false; } - List<String> linuxSandboxArgv = + ImmutableList<String> linuxSandboxArgv = LinuxSandboxUtil.commandLineBuilder( - LinuxSandboxUtil.getLinuxSandbox(cmdEnv).getPathString(), - ImmutableList.of("/bin/true")) + LinuxSandboxUtil.getLinuxSandbox(cmdEnv), ImmutableList.of("/bin/true")) .build(); ImmutableMap<String, String> env = ImmutableMap.of(); Path execRoot = cmdEnv.getExecRoot(); @@ -84,7 +79,7 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { private final Path inaccessibleHelperFile; private final Path inaccessibleHelperDir; private final LocalEnvProvider localEnvProvider; - private final Optional<Duration> timeoutKillDelay; + private final Duration timeoutKillDelay; /** * Creates a sandboxed spawn runner that uses the {@code linux-sandbox} tool. @@ -93,15 +88,14 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { * @param sandboxBase path to the sandbox base directory * @param inaccessibleHelperFile path to a file that is (already) inaccessible * @param inaccessibleHelperDir path to a directory that is (already) inaccessible - * @param timeoutKillDelay an optional, additional grace period before killing timing out - * commands. If not present, then no grace period is used and commands are killed instantly. + * @param timeoutKillDelay an additional grace period before killing timing out commands */ LinuxSandboxedSpawnRunner( CommandEnvironment cmdEnv, Path sandboxBase, Path inaccessibleHelperFile, Path inaccessibleHelperDir, - Optional<Duration> timeoutKillDelay) { + Duration timeoutKillDelay) { super(cmdEnv, sandboxBase); this.fileSystem = cmdEnv.getRuntime().getFileSystem(); this.blazeDirs = cmdEnv.getDirectories(); @@ -128,12 +122,12 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { Map<String, String> environment = localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir.getPathString()); - Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, environment); + ImmutableSet<Path> writableDirs = getWritableDirs(sandboxExecRoot, environment); ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn); Duration timeout = policy.getTimeout(); LinuxSandboxUtil.CommandLineBuilder commandLineBuilder = - LinuxSandboxUtil.commandLineBuilder(linuxSandbox.getPathString(), spawn.getArguments()) + LinuxSandboxUtil.commandLineBuilder(linuxSandbox, spawn.getArguments()) .setWritableFilesAndDirectories(writableDirs) .setTmpfsDirectories(getTmpfsPaths()) .setBindMounts(getReadOnlyBindMounts(blazeDirs, sandboxExecRoot)) @@ -144,19 +138,17 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { if (!timeout.isZero()) { commandLineBuilder.setTimeout(timeout); } - if (timeoutKillDelay.isPresent()) { - commandLineBuilder.setKillDelay(timeoutKillDelay.get()); - } + commandLineBuilder.setKillDelay(timeoutKillDelay); if (spawn.getExecutionInfo().containsKey(ExecutionRequirements.REQUIRES_FAKEROOT)) { commandLineBuilder.setUseFakeRoot(true); } else if (getSandboxOptions().sandboxFakeUsername) { commandLineBuilder.setUseFakeUsername(true); } - Optional<String> statisticsPath = Optional.empty(); + Path statisticsPath = null; if (getSandboxOptions().collectLocalSandboxExecutionStatistics) { - statisticsPath = Optional.of(sandboxPath.getRelative("stats.out").getPathString()); - commandLineBuilder.setStatisticsPath(statisticsPath.get()); + statisticsPath = sandboxPath.getRelative("stats.out"); + commandLineBuilder.setStatisticsPath(statisticsPath); } SandboxedSpawn sandbox = 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 32997347b8..bbebefa81d 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 @@ -23,7 +23,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; import java.time.Duration; -import java.util.Optional; /** Strategy that uses sandboxing to execute a process. */ // TODO(ulfjack): This class only exists for this annotation. Find a better way to handle this! @@ -46,14 +45,10 @@ public final class LinuxSandboxedStrategy extends AbstractSpawnStrategy { * * @param cmdEnv the command environment to use * @param sandboxBase path to the sandbox base directory - * @param timeoutKillDelay an optional, additional grace period before killing timing out - * commands. If not present, then no grace period is used and commands are killed instantly. + * @param timeoutKillDelay additional grace period before killing timing out commands */ static LinuxSandboxedSpawnRunner create( - CommandEnvironment cmdEnv, - Path sandboxBase, - Optional<Duration> timeoutKillDelay) - throws IOException { + CommandEnvironment cmdEnv, Path sandboxBase, Duration timeoutKillDelay) throws IOException { Path inaccessibleHelperFile = sandboxBase.getRelative("inaccessibleHelperFile"); FileSystemUtils.touchFile(inaccessibleHelperFile); inaccessibleHelperFile.setReadable(false); 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 a61b6e150f..fdc4abfdd7 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 @@ -27,7 +27,6 @@ import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; import java.time.Duration; import java.util.Map; -import java.util.Optional; /** Strategy that uses sandboxing to execute a process. */ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { @@ -39,7 +38,7 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne private final Path execRoot; private final Path processWrapper; private final LocalEnvProvider localEnvProvider; - private final Optional<Duration> timeoutKillDelay; + private final Duration timeoutKillDelay; /** * Creates a sandboxed spawn runner that uses the {@code process-wrapper} tool. @@ -47,14 +46,10 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne * @param cmdEnv the command environment to use * @param sandboxBase path to the sandbox base directory * @param productName the product name to use - * @param timeoutKillDelay an optional, additional grace period before killing timing out - * commands. If not present, then no grace period is used and commands are killed instantly. + * @param timeoutKillDelay additional grace period before killing timing out commands */ ProcessWrapperSandboxedSpawnRunner( - CommandEnvironment cmdEnv, - Path sandboxBase, - String productName, - Optional<Duration> timeoutKillDelay) { + CommandEnvironment cmdEnv, Path sandboxBase, String productName, Duration timeoutKillDelay) { super(cmdEnv, sandboxBase); this.execRoot = cmdEnv.getExecRoot(); this.timeoutKillDelay = timeoutKillDelay; @@ -84,14 +79,12 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne ProcessWrapperUtil.commandLineBuilder(processWrapper.getPathString(), spawn.getArguments()) .setTimeout(timeout); - if (timeoutKillDelay.isPresent()) { - commandLineBuilder.setKillDelay(timeoutKillDelay.get()); - } + commandLineBuilder.setKillDelay(timeoutKillDelay); - Optional<String> statisticsPath = Optional.empty(); + Path statisticsPath = null; if (getSandboxOptions().collectLocalSandboxExecutionStatistics) { - statisticsPath = Optional.of(sandboxPath.getRelative("stats.out").getPathString()); - commandLineBuilder.setStatisticsPath(statisticsPath.get()); + statisticsPath = sandboxPath.getRelative("stats.out"); + commandLineBuilder.setStatisticsPath(statisticsPath); } SandboxedSpawn sandbox = 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 31da786227..a7f7a5bce3 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 @@ -34,7 +34,6 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.common.options.OptionsProvider; import java.io.IOException; import java.time.Duration; -import java.util.Optional; import javax.annotation.Nullable; /** @@ -53,12 +52,9 @@ final class SandboxActionContextProvider extends ActionContextProvider { ImmutableList.Builder<ActionContext> contexts = ImmutableList.builder(); OptionsProvider options = cmdEnv.getOptions(); - int timeoutKillDelaySeconds = - options.getOptions(LocalExecutionOptions.class).localSigkillGraceSeconds; - Optional<Duration> timeoutKillDelay = Optional.empty(); - if (timeoutKillDelaySeconds >= 0) { - timeoutKillDelay = Optional.of(Duration.ofSeconds(timeoutKillDelaySeconds)); - } + Duration timeoutKillDelay = + Duration.ofSeconds( + options.getOptions(LocalExecutionOptions.class).localSigkillGraceSeconds); // 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. 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 d0f6c75da3..3065393a96 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 @@ -104,7 +104,6 @@ public final class SandboxModule extends BlazeModule { ActionContextProvider provider; try { sandboxBase.createDirectoryAndParents(); - if (options.useSandboxfs) { Path mountPoint = sandboxBase.getRelative("sandboxfs"); mountPoint.createDirectory(); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java index 9657d04203..f4d41e38a3 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java @@ -192,7 +192,7 @@ public class SymlinkedSandboxedSpawn implements SandboxedSpawn { private static void createDirectoryAndParentsWithCache(Set<Path> cache, Path dir) throws IOException { if (cache.add(dir)) { - FileSystemUtils.createDirectoryAndParents(dir); + dir.createDirectoryAndParents(); } } } diff --git a/src/main/java/com/google/devtools/build/lib/shell/BUILD b/src/main/java/com/google/devtools/build/lib/shell/BUILD index 8b8d9301a5..91fa895856 100644 --- a/src/main/java/com/google/devtools/build/lib/shell/BUILD +++ b/src/main/java/com/google/devtools/build/lib/shell/BUILD @@ -14,6 +14,7 @@ java_library( name = "shell", srcs = glob(["*.java"]), deps = [ + "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/protobuf:execution_statistics_java_proto", "//third_party:auto_value", "//third_party:guava", diff --git a/src/main/java/com/google/devtools/build/lib/shell/ExecutionStatistics.java b/src/main/java/com/google/devtools/build/lib/shell/ExecutionStatistics.java index e74ff50f27..bda5feafa3 100644 --- a/src/main/java/com/google/devtools/build/lib/shell/ExecutionStatistics.java +++ b/src/main/java/com/google/devtools/build/lib/shell/ExecutionStatistics.java @@ -14,8 +14,8 @@ package com.google.devtools.build.lib.shell; +import com.google.devtools.build.lib.vfs.Path; import java.io.BufferedInputStream; -import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.time.Duration; @@ -29,16 +29,17 @@ public final class ExecutionStatistics { * @param executionStatisticsProtoPath path to a materialized ExecutionStatistics proto * @return a {@link ResourceUsage} object containing execution statistics, if available */ - public static Optional<ResourceUsage> getResourceUsage(String executionStatisticsProtoPath) + public static Optional<ResourceUsage> getResourceUsage(Path executionStatisticsProtoPath) throws IOException { - InputStream protoInputStream = - new BufferedInputStream(new FileInputStream(executionStatisticsProtoPath)); - com.google.devtools.build.lib.shell.Protos.ExecutionStatistics executionStatisticsProto = - com.google.devtools.build.lib.shell.Protos.ExecutionStatistics.parseFrom(protoInputStream); - if (executionStatisticsProto.hasResourceUsage()) { - return Optional.of(new ResourceUsage(executionStatisticsProto.getResourceUsage())); - } else { - return Optional.empty(); + try (InputStream protoInputStream = + new BufferedInputStream(executionStatisticsProtoPath.getInputStream())) { + Protos.ExecutionStatistics executionStatisticsProto = + Protos.ExecutionStatistics.parseFrom(protoInputStream); + if (executionStatisticsProto.hasResourceUsage()) { + return Optional.of(new ResourceUsage(executionStatisticsProto.getResourceUsage())); + } else { + return Optional.empty(); + } } } @@ -47,11 +48,10 @@ public final class ExecutionStatistics { * call. */ public static class ResourceUsage { - private final com.google.devtools.build.lib.shell.Protos.ResourceUsage resourceUsageProto; + private final Protos.ResourceUsage resourceUsageProto; /** Provides resource usage statistics via a ResourceUsage proto object. */ - public ResourceUsage( - com.google.devtools.build.lib.shell.Protos.ResourceUsage resourceUsageProto) { + public ResourceUsage(Protos.ResourceUsage resourceUsageProto) { this.resourceUsageProto = resourceUsageProto; } diff --git a/src/main/java/com/google/devtools/build/lib/vfs/BUILD b/src/main/java/com/google/devtools/build/lib/vfs/BUILD index 18ecb8114d..348a476752 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/BUILD +++ b/src/main/java/com/google/devtools/build/lib/vfs/BUILD @@ -51,7 +51,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/clock", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/profiler", - "//src/main/java/com/google/devtools/build/lib/shell", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", "//src/main/java/com/google/devtools/common/options", "//third_party:guava", diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index 1e80aee5df..a92cc32a39 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -1195,6 +1195,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib:io", "//src/main/java/com/google/devtools/build/lib:packages", "//src/main/java/com/google/devtools/build/lib:runtime", + "//src/main/java/com/google/devtools/build/lib:unix", "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader", @@ -1203,6 +1204,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/buildeventstream/transports", "//src/main/java/com/google/devtools/build/lib/clock", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", + "//src/main/java/com/google/devtools/build/lib/sandbox", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", "//src/main/java/com/google/devtools/common/options", diff --git a/src/test/java/com/google/devtools/build/lib/runtime/ProcessWrapperUtilTest.java b/src/test/java/com/google/devtools/build/lib/runtime/ProcessWrapperUtilTest.java index 7dc1bff177..ff1fdd50d5 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/ProcessWrapperUtilTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/ProcessWrapperUtilTest.java @@ -17,8 +17,12 @@ package com.google.devtools.build.lib.runtime; import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.ImmutableList; +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.time.Duration; import java.util.List; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -26,7 +30,12 @@ import org.junit.runners.JUnit4; /** Unit tests for {@link ProcessWrapperUtil}. */ @RunWith(JUnit4.class) public final class ProcessWrapperUtilTest { + private FileSystem testFS; + @Before + public final void createFileSystem() { + testFS = new InMemoryFileSystem(); + } @Test public void testProcessWrapperCommandLineBuilder_BuildsWithoutOptionalArguments() { @@ -51,9 +60,9 @@ public final class ProcessWrapperUtilTest { Duration timeout = Duration.ofSeconds(10); Duration killDelay = Duration.ofSeconds(2); - String stdoutPath = "stdout.txt"; - String stderrPath = "stderr.txt"; - String statisticsPath = "stats.out"; + Path stdoutPath = testFS.getPath("/stdout.txt"); + Path stderrPath = testFS.getPath("/stderr.txt"); + Path statisticsPath = testFS.getPath("/stats.out"); ImmutableList<String> expectedCommandLine = ImmutableList.<String>builder() diff --git a/src/test/java/com/google/devtools/build/lib/runtime/LinuxSandboxUtilTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxUtilTest.java index 7267551580..8a2b4b3a75 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/LinuxSandboxUtilTest.java +++ b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxUtilTest.java @@ -12,18 +12,20 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.devtools.build.lib.runtime; +package com.google.devtools.build.lib.sandbox; import static com.google.common.truth.Truth.assertThat; import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; 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.time.Duration; import java.util.List; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -31,9 +33,16 @@ import org.junit.runners.JUnit4; /** Unit tests for {@link LinuxSandboxUtil}. */ @RunWith(JUnit4.class) public final class LinuxSandboxUtilTest { + private FileSystem testFS; + + @Before + public final void createFileSystem() { + testFS = new InMemoryFileSystem(); + } + @Test public void testLinuxSandboxCommandLineBuilder_fakeRootAndFakeUsernameAreExclusive() { - String linuxSandboxPath = "linux-sandbox"; + Path linuxSandboxPath = testFS.getPath("/linux-sandbox"); ImmutableList<String> commandArguments = ImmutableList.of("echo", "hello, flo"); Exception e = @@ -49,13 +58,13 @@ public final class LinuxSandboxUtilTest { @Test public void testLinuxSandboxCommandLineBuilder_BuildsWithoutOptionalArguments() { - String linuxSandboxPath = "linux-sandbox"; + Path linuxSandboxPath = testFS.getPath("/linux-sandbox"); ImmutableList<String> commandArguments = ImmutableList.of("echo", "hello, max"); ImmutableList<String> expectedCommandLine = ImmutableList.<String>builder() - .add(linuxSandboxPath) + .add(linuxSandboxPath.getPathString()) .add("--") .addAll(commandArguments) .build(); @@ -68,17 +77,17 @@ public final class LinuxSandboxUtilTest { @Test public void testLinuxSandboxCommandLineBuilder_BuildsWithOptionalArguments() { - String linuxSandboxPath = "linux-sandbox"; + Path linuxSandboxPath = testFS.getPath("/linux-sandbox"); ImmutableList<String> commandArguments = ImmutableList.of("echo", "hello, tom"); Duration timeout = Duration.ofSeconds(10); Duration killDelay = Duration.ofSeconds(2); - String statisticsPath = "stats.out"; + Path statisticsPath = testFS.getPath("/stats.out"); - String workingDirectory = "/all-work-and-no-play"; - String stdoutPath = "stdout.txt"; - String stderrPath = "stderr.txt"; + Path workingDirectory = testFS.getPath("/all-work-and-no-play"); + Path stdoutPath = testFS.getPath("/stdout.txt"); + Path stderrPath = testFS.getPath("/stderr.txt"); // These two flags are exclusive. boolean useFakeUsername = true; @@ -106,9 +115,9 @@ public final class LinuxSandboxUtilTest { Path tmpfsDir1 = sandboxDir.getRelative("tmpfs1"); Path tmpfsDir2 = sandboxDir.getRelative("tmpfs2"); - ImmutableList<Path> writableFilesAndDirectories = ImmutableList.of(writableDir1, writableDir2); + ImmutableSet<Path> writableFilesAndDirectories = ImmutableSet.of(writableDir1, writableDir2); - ImmutableList<Path> tmpfsDirectories = ImmutableList.of(tmpfsDir1, tmpfsDir2); + ImmutableSet<Path> tmpfsDirectories = ImmutableSet.of(tmpfsDir1, tmpfsDir2); ImmutableSortedMap<Path, Path> bindMounts = ImmutableSortedMap.<Path, Path>naturalOrder() @@ -119,12 +128,12 @@ public final class LinuxSandboxUtilTest { ImmutableList<String> expectedCommandLine = ImmutableList.<String>builder() - .add(linuxSandboxPath) - .add("-W", workingDirectory) + .add(linuxSandboxPath.getPathString()) + .add("-W", workingDirectory.getPathString()) .add("-T", Long.toString(timeout.getSeconds())) .add("-t", Long.toString(killDelay.getSeconds())) - .add("-l", stdoutPath) - .add("-L", stderrPath) + .add("-l", stdoutPath.getPathString()) + .add("-L", stderrPath.getPathString()) .add("-w", writableDir1.getPathString()) .add("-w", writableDir2.getPathString()) .add("-e", tmpfsDir1.getPathString()) @@ -134,7 +143,7 @@ public final class LinuxSandboxUtilTest { .add("-m", bindMountTarget1.getPathString()) .add("-M", bindMountSource2.getPathString()) .add("-m", bindMountTarget2.getPathString()) - .add("-S", statisticsPath) + .add("-S", statisticsPath.getPathString()) .add("-H") .add("-N") .add("-U") diff --git a/src/test/java/com/google/devtools/build/lib/shell/BUILD b/src/test/java/com/google/devtools/build/lib/shell/BUILD index 6bbce901f1..d173fd4ec6 100644 --- a/src/test/java/com/google/devtools/build/lib/shell/BUILD +++ b/src/test/java/com/google/devtools/build/lib/shell/BUILD @@ -33,9 +33,11 @@ java_library( "//src/main/java/com/google/devtools/build/lib:bazel-main", "//src/main/java/com/google/devtools/build/lib:os_util", "//src/main/java/com/google/devtools/build/lib:runtime", + "//src/main/java/com/google/devtools/build/lib:unix", "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib/collect", "//src/main/java/com/google/devtools/build/lib/shell", + "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/protobuf:execution_statistics_java_proto", "//src/test/java/com/google/devtools/build/lib:foundations_testutil", "//src/test/java/com/google/devtools/build/lib:test_runner", @@ -107,9 +109,12 @@ java_test( "//src/main/java/com/google/devtools/build/lib:bazel-main", "//src/main/java/com/google/devtools/build/lib:os_util", "//src/main/java/com/google/devtools/build/lib:runtime", + "//src/main/java/com/google/devtools/build/lib:unix", "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib/collect", + "//src/main/java/com/google/devtools/build/lib/sandbox", "//src/main/java/com/google/devtools/build/lib/shell", + "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/protobuf:execution_statistics_java_proto", "//src/test/java/com/google/devtools/build/lib:foundations_testutil", "//src/test/java/com/google/devtools/build/lib:test_runner", diff --git a/src/test/java/com/google/devtools/build/lib/shell/CommandUsingLinuxSandboxTest.java b/src/test/java/com/google/devtools/build/lib/shell/CommandUsingLinuxSandboxTest.java index 67a0981a7b..2aee18cb31 100644 --- a/src/test/java/com/google/devtools/build/lib/shell/CommandUsingLinuxSandboxTest.java +++ b/src/test/java/com/google/devtools/build/lib/shell/CommandUsingLinuxSandboxTest.java @@ -18,15 +18,18 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assume.assumeTrue; import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.runtime.LinuxSandboxUtil; +import com.google.devtools.build.lib.sandbox.LinuxSandboxUtil; import com.google.devtools.build.lib.testutil.BlazeTestUtils; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.testutil.TestUtils; +import com.google.devtools.build.lib.unix.UnixFileSystem; import com.google.devtools.build.lib.util.OS; -import java.io.File; +import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; import java.time.Duration; import java.util.List; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -34,12 +37,21 @@ import org.junit.runners.JUnit4; /** Unit tests for {@link Command}s that are run using the {@code linux-sandbox}. */ @RunWith(JUnit4.class) public final class CommandUsingLinuxSandboxTest { - private String getLinuxSandboxPath() { - return BlazeTestUtils.runfilesDir() + "/" + TestConstants.LINUX_SANDBOX_PATH; + private FileSystem testFS; + private Path runfilesDir; + + @Before + public final void createFileSystem() throws Exception { + testFS = new UnixFileSystem(); + runfilesDir = testFS.getPath(BlazeTestUtils.runfilesDir()); + } + + private Path getLinuxSandboxPath() { + return runfilesDir.getRelative(TestConstants.LINUX_SANDBOX_PATH); } - private String getCpuTimeSpenderPath() { - return BlazeTestUtils.runfilesDir() + "/" + TestConstants.CPU_TIME_SPENDER_PATH; + private Path getCpuTimeSpenderPath() { + return runfilesDir.getRelative(TestConstants.CPU_TIME_SPENDER_PATH); } @Test @@ -76,12 +88,12 @@ public final class CommandUsingLinuxSandboxTest { throws IOException, CommandException { ImmutableList<String> commandArguments = ImmutableList.of( - getCpuTimeSpenderPath(), + getCpuTimeSpenderPath().getPathString(), Long.toString(userTimeToSpend.getSeconds()), Long.toString(systemTimeToSpend.getSeconds())); - File outputDir = TestUtils.makeTempDir(); - String statisticsFilePath = outputDir.getAbsolutePath() + "/" + "stats.out"; + Path outputDir = testFS.getPath(TestUtils.makeTempDir().getCanonicalPath()); + Path statisticsFilePath = outputDir.getRelative("stats.out"); List<String> fullCommandLine = LinuxSandboxUtil.commandLineBuilder(getLinuxSandboxPath(), commandArguments) 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 40dd37598c..b6643914be 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 @@ -21,10 +21,13 @@ import com.google.devtools.build.lib.runtime.ProcessWrapperUtil; import com.google.devtools.build.lib.testutil.BlazeTestUtils; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.testutil.TestUtils; -import java.io.File; +import com.google.devtools.build.lib.unix.UnixFileSystem; +import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; import java.time.Duration; import java.util.List; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -32,6 +35,13 @@ import org.junit.runners.JUnit4; /** Unit tests for {@link Command}s that are wrapped using the {@code process-wrapper}. */ @RunWith(JUnit4.class) public final class CommandUsingProcessWrapperTest { + private FileSystem testFS; + + @Before + public final void createFileSystem() throws Exception { + testFS = new UnixFileSystem(); + } + private String getProcessWrapperPath() { return BlazeTestUtils.runfilesDir() + "/" + TestConstants.PROCESS_WRAPPER_PATH; } @@ -73,8 +83,8 @@ public final class CommandUsingProcessWrapperTest { Long.toString(userTimeToSpend.getSeconds()), Long.toString(systemTimeToSpend.getSeconds())); - File outputDir = TestUtils.makeTempDir(); - String statisticsFilePath = outputDir.getAbsolutePath() + "/" + "stats.out"; + Path outputDir = testFS.getPath(TestUtils.makeTempDir().getCanonicalPath()); + Path statisticsFilePath = outputDir.getRelative("stats.out"); List<String> fullCommandLine = ProcessWrapperUtil.commandLineBuilder(getProcessWrapperPath(), commandArguments) diff --git a/src/test/java/com/google/devtools/build/lib/shell/ExecutionStatisticsTest.java b/src/test/java/com/google/devtools/build/lib/shell/ExecutionStatisticsTest.java index 81ea266c00..acc37d2904 100644 --- a/src/test/java/com/google/devtools/build/lib/shell/ExecutionStatisticsTest.java +++ b/src/test/java/com/google/devtools/build/lib/shell/ExecutionStatisticsTest.java @@ -17,11 +17,14 @@ package com.google.devtools.build.lib.shell; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; +import com.google.devtools.build.lib.testutil.TestUtils; +import com.google.devtools.build.lib.unix.UnixFileSystem; +import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.Path; import java.io.BufferedOutputStream; -import java.io.File; -import java.io.FileOutputStream; import java.time.Duration; import java.util.Optional; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -29,24 +32,31 @@ import org.junit.runners.JUnit4; /** Tests for {@link ExecutionStatistics}. */ @RunWith(JUnit4.class) public final class ExecutionStatisticsTest { - private String createExecutionStatisticsProtoFile( + private FileSystem testFS; + private Path workingDir; + + @Before + public final void createFileSystem() throws Exception { + testFS = new UnixFileSystem(); + workingDir = testFS.getPath(TestUtils.makeTempDir().getCanonicalPath()); + } + + private Path createExecutionStatisticsProtoFile( com.google.devtools.build.lib.shell.Protos.ExecutionStatistics executionStatisticsProto) throws Exception { - byte[] protoBytes = executionStatisticsProto.toByteArray(); - File encodedProtoFile = File.createTempFile("encoded_action_execution_proto", ""); - String protoFilename = encodedProtoFile.getPath(); + Path encodedProtoFile = workingDir.getRelative("encoded_action_execution_proto"); try (BufferedOutputStream bufferedOutputStream = - new BufferedOutputStream(new FileOutputStream(encodedProtoFile))) { - bufferedOutputStream.write(protoBytes); + new BufferedOutputStream(encodedProtoFile.getOutputStream())) { + executionStatisticsProto.writeTo(bufferedOutputStream); } - return protoFilename; + return encodedProtoFile; } @Test public void testNoResourceUsage_whenNoResourceUsageProto() throws Exception { com.google.devtools.build.lib.shell.Protos.ExecutionStatistics executionStatisticsProto = com.google.devtools.build.lib.shell.Protos.ExecutionStatistics.getDefaultInstance(); - String protoFilename = createExecutionStatisticsProtoFile(executionStatisticsProto); + Path protoFilename = createExecutionStatisticsProtoFile(executionStatisticsProto); Optional<ExecutionStatistics.ResourceUsage> resourceUsage = ExecutionStatistics.getResourceUsage(protoFilename); @@ -98,7 +108,7 @@ public final class ExecutionStatisticsTest { com.google.devtools.build.lib.shell.Protos.ExecutionStatistics.newBuilder() .setResourceUsage(resourceUsageProto) .build(); - String protoFilename = createExecutionStatisticsProtoFile(executionStatisticsProto); + Path protoFilename = createExecutionStatisticsProtoFile(executionStatisticsProto); Optional<ExecutionStatistics.ResourceUsage> maybeResourceUsage = ExecutionStatistics.getResourceUsage(protoFilename); diff --git a/src/test/java/com/google/devtools/build/lib/shell/ExecutionStatisticsTestUtil.java b/src/test/java/com/google/devtools/build/lib/shell/ExecutionStatisticsTestUtil.java index e2aafd5252..221cac995c 100644 --- a/src/test/java/com/google/devtools/build/lib/shell/ExecutionStatisticsTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/shell/ExecutionStatisticsTestUtil.java @@ -17,6 +17,7 @@ package com.google.devtools.build.lib.shell; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; +import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; import java.time.Duration; import java.util.List; @@ -40,7 +41,7 @@ public class ExecutionStatisticsTestUtil { Duration userTimeToSpend, Duration systemTimeToSpend, List<String> fullCommandLine, - String statisticsFilePath) + Path statisticsFilePath) throws CommandException, IOException { Duration userTimeLowerBound = userTimeToSpend; Duration userTimeUpperBound = userTimeToSpend.plusSeconds(2); diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD index 7e364f70a4..0c126c5b30 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD @@ -22,6 +22,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/remote/blobstore", "//src/main/java/com/google/devtools/build/lib/remote/util", + "//src/main/java/com/google/devtools/build/lib/sandbox", "//src/main/java/com/google/devtools/build/lib/shell", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/common/options", diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java index f6788493e4..5bf8cbbad9 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java @@ -32,7 +32,7 @@ import com.google.devtools.build.lib.remote.blobstore.OnDiskBlobStore; import com.google.devtools.build.lib.remote.blobstore.SimpleBlobStore; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; -import com.google.devtools.build.lib.runtime.LinuxSandboxUtil; +import com.google.devtools.build.lib.sandbox.LinuxSandboxUtil; import com.google.devtools.build.lib.shell.Command; import com.google.devtools.build.lib.shell.CommandException; import com.google.devtools.build.lib.shell.CommandResult; @@ -203,7 +203,7 @@ public final class RemoteWorker { .getMulticastConfig() .setEnabled(false); HazelcastInstance instance = Hazelcast.newHazelcastInstance(config); - return new ConcurrentMapBlobStore(instance.<String, byte[]>getMap("cache")); + return new ConcurrentMapBlobStore(instance.getMap("cache")); } public static void main(String[] args) throws Exception { @@ -315,10 +315,10 @@ public final class RemoteWorker { CommandResult cmdResult = null; Command cmd = new Command( - LinuxSandboxUtil.commandLineBuilder( - sandboxPath.getPathString(), ImmutableList.of("true")) - .buildAsArray(), - ImmutableMap.<String, String>of(), + LinuxSandboxUtil.commandLineBuilder(sandboxPath, ImmutableList.of("true")) + .build() + .toArray(new String[0]), + ImmutableMap.of(), sandboxPath.getParentDirectory().getPathFile()); try { cmdResult = cmd.execute(); |