Skip to content

Commit

Permalink
This change makes it possible to use the Linux sandbox when either th…
Browse files Browse the repository at this point in the history
…e execroot, some package path entries, or both are under /tmp.

This is achieved by a reshuffling of the sandbox directory layout in the following way:

* The exec root base is `/tmp/bazel-working-directory` (cwd is under it)
* Source roots are mapped under `/tmp/bazel-source-roots/$NUMBER`
* The "real" exec root is mapped to `/tmp/bazel-execroot`.
*
All this is achieved with subtle manipulation of bind mounts:

1. The real exec root (bazel info execution_root) under `$SANDBOX/_tmp/bazel-execroot`
2. The sandbox exec root (the symlink tree the sandbox creates) under `$SANDBOX/_tmp/bazel-working-directory`
3. Each source root under `$SANDBOX/_tmp/bazel-source-roots/$NUMBER`
4. `$SANDBOX/_tmp` under `/tmp`

This makes the directories in (1), (2) and (3) available as `/tmp/$NAME` even if they were originally under `/tmp` (which gets clobbered in step (4))

The functionality is gated under `--incompatible_sandbox_hermetic_tmp` since it requires `/tmp` to be in a known state, which only that flag can guarantee.

Notably, putting these three directories under `/` does not work, because the non-hermetic sandbox uses the real file system and the root directory is not writable. We could conceivably get around that by bind mounting every first child of the "real" root directory in the sandbox root directory and using a writable directory as the sandbox root, but why bother if this one works.

Progress towards #3236 (the flag still needs to be flipped)

RELNOTES: None.
PiperOrigin-RevId: 494650851
Change-Id: I0b3d1baf748a357a5bb4dee799cf807c2d75ef15
  • Loading branch information
lberki authored and copybara-github committed Dec 12, 2022
1 parent 0a2c4ed commit 8e32f44
Show file tree
Hide file tree
Showing 12 changed files with 338 additions and 110 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -310,17 +310,21 @@ private boolean wasTimeout(Duration timeout, Duration wallTime) {
/**
* Gets the list of directories that the spawn will assume to be writable.
*
* @param sandboxExecRoot the exec root of the sandbox from the point of view of the Bazel process
* @param withinSandboxExecRoot the exec root from the point of view of the sandboxed processes
* @param env the environment of the sandboxed processes
* @throws IOException because we might resolve symlinks, which throws {@link IOException}.
*/
protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env)
protected ImmutableSet<Path> getWritableDirs(
Path sandboxExecRoot, Path withinSandboxExecRoot, Map<String, String> env)
throws IOException {
// We have to make the TEST_TMPDIR directory writable if it is specified.
ImmutableSet.Builder<Path> writablePaths = ImmutableSet.builder();

// On Windows, sandboxExecRoot is actually the main execroot. We will specify
// exactly which output path is writable.
if (OS.getCurrent() != OS.WINDOWS) {
writablePaths.add(sandboxExecRoot);
writablePaths.add(withinSandboxExecRoot);
}

String testTmpdir = env.get("TEST_TMPDIR");
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/sandbox/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//third_party:auto_value",
"//third_party:flogger",
"//third_party:guava",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,8 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), binTools, "/tmp");

final HashSet<Path> writableDirs = new HashSet<>(alwaysWritableDirs);
ImmutableSet<Path> extraWritableDirs = getWritableDirs(sandboxExecRoot, environment);
ImmutableSet<Path> extraWritableDirs =
getWritableDirs(sandboxExecRoot, sandboxExecRoot, environment);
writableDirs.addAll(extraWritableDirs);

SandboxInputs inputs =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@

package com.google.devtools.build.lib.sandbox;

import com.google.auto.value.AutoValue;
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.actions.ExecutionRequirements;
import com.google.devtools.build.lib.vfs.Path;
Expand All @@ -32,6 +32,20 @@
* linux-sandbox} tool.
*/
public class LinuxSandboxCommandLineBuilder {
/** A bind mount that needs to be present when the sandboxed command runs. */
@AutoValue
public abstract static class BindMount {
public static BindMount of(Path mountPoint, Path source) {
return new AutoValue_LinuxSandboxCommandLineBuilder_BindMount(mountPoint, source);
}

/** "target" in mount(2) */
public abstract Path getMountPoint();

/** "source" in mount(2) */
public abstract Path getContent();
}

private final Path linuxSandboxPath;
private final List<String> commandArguments;
private Path hermeticSandboxPath;
Expand All @@ -43,7 +57,7 @@ public class LinuxSandboxCommandLineBuilder {
private Path stderrPath;
private Set<Path> writableFilesAndDirectories = ImmutableSet.of();
private ImmutableSet<PathFragment> tmpfsDirectories = ImmutableSet.of();
private Map<Path, Path> bindMounts = ImmutableMap.of();
private List<BindMount> bindMounts = ImmutableList.of();
private Path statisticsPath;
private boolean useFakeHostname = false;
private boolean createNetworkNamespace = false;
Expand Down Expand Up @@ -140,7 +154,7 @@ public LinuxSandboxCommandLineBuilder setTmpfsDirectories(
* if any.
*/
@CanIgnoreReturnValue
public LinuxSandboxCommandLineBuilder setBindMounts(Map<Path, Path> bindMounts) {
public LinuxSandboxCommandLineBuilder setBindMounts(List<BindMount> bindMounts) {
this.bindMounts = bindMounts;
return this;
}
Expand Down Expand Up @@ -248,12 +262,11 @@ public ImmutableList<String> build() {
for (PathFragment tmpfsPath : tmpfsDirectories) {
commandLineBuilder.add("-e", tmpfsPath.getPathString());
}
for (Path bindMountTarget : bindMounts.keySet()) {
Path bindMountSource = bindMounts.get(bindMountTarget);
commandLineBuilder.add("-M", bindMountSource.getPathString());
for (BindMount bindMount : bindMounts) {
commandLineBuilder.add("-M", bindMount.getContent().getPathString());
// The file is mounted in a custom location inside the sandbox.
if (!bindMountSource.equals(bindMountTarget)) {
commandLineBuilder.add("-m", bindMountTarget.getPathString());
if (!bindMount.getContent().equals(bindMount.getMountPoint())) {
commandLineBuilder.add("-m", bindMount.getMountPoint().getPathString());
}
}
if (statisticsPath != null) {
Expand Down
Loading

0 comments on commit 8e32f44

Please sign in to comment.