Skip to content

Commit

Permalink
Make the Linux sandbox work with ActionInputs with absolute "exec pat…
Browse files Browse the repository at this point in the history
…hs".

Progress towards #3236.

RELNOTES: None.
PiperOrigin-RevId: 493542957
Change-Id: Iff396e77d7624bdb033b198068aa137397495db0
  • Loading branch information
lberki authored and Copybara-Service committed Dec 7, 2022
1 parent 0e8de27 commit 70691f2
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,11 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context

SandboxInputs inputs =
helpers.processInputFiles(
context.getInputMapping(PathFragment.EMPTY_FRAGMENT), execRoot, execRoot, null);
context.getInputMapping(PathFragment.EMPTY_FRAGMENT),
execRoot,
execRoot,
ImmutableList.of(),
null);
SandboxOutputs outputs = helpers.getOutputs(spawn);

final Path sandboxConfigPath = sandboxPath.getRelative("sandbox.sb");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,11 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context

SandboxInputs inputs =
helpers.processInputFiles(
context.getInputMapping(PathFragment.EMPTY_FRAGMENT), execRoot, execRoot, null);
context.getInputMapping(PathFragment.EMPTY_FRAGMENT),
execRoot,
execRoot,
ImmutableList.of(),
null);
SandboxOutputs outputs = helpers.getOutputs(spawn);

Duration timeout = context.getTimeout();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.Symlinks;
import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -128,6 +129,7 @@ private static boolean computeIsSupported(CommandEnvironment cmdEnv, Path linuxS
private final boolean sandboxfsMapSymlinkTargets;
private final TreeDeleter treeDeleter;
private final Reporter reporter;
private final ImmutableList<Root> packageRoots;

/**
* Creates a sandboxed spawn runner that uses the {@code linux-sandbox} tool.
Expand Down Expand Up @@ -168,6 +170,7 @@ private static boolean computeIsSupported(CommandEnvironment cmdEnv, Path linuxS
this.localEnvProvider = new PosixLocalEnvProvider(cmdEnv.getClientEnv());
this.treeDeleter = treeDeleter;
this.reporter = cmdEnv.getReporter();
this.packageRoots = cmdEnv.getPackageLocator().getPathEntries();
}

@Override
Expand Down Expand Up @@ -219,7 +222,11 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context

SandboxInputs inputs =
helpers.processInputFiles(
context.getInputMapping(PathFragment.EMPTY_FRAGMENT), execRoot, execRoot, null);
context.getInputMapping(PathFragment.EMPTY_FRAGMENT),
execRoot,
execRoot,
packageRoots,
null);
SandboxOutputs outputs = helpers.getOutputs(spawn);

Duration timeout = context.getTimeout();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

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

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.ForbiddenActionInputException;
Expand Down Expand Up @@ -111,7 +112,11 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context

SandboxInputs inputs =
helpers.processInputFiles(
context.getInputMapping(PathFragment.EMPTY_FRAGMENT), execRoot, execRoot, null);
context.getInputMapping(PathFragment.EMPTY_FRAGMENT),
execRoot,
execRoot,
ImmutableList.of(),
null);
SandboxOutputs outputs = helpers.getOutputs(spawn);

if (sandboxfsProcess != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

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.common.collect.Iterables;
Expand Down Expand Up @@ -461,16 +462,50 @@ private static byte[] writeVirtualInputTo(VirtualActionInput input, Path target)
return digest;
}

/**
* Returns the appropriate {@link RootedPath} for a Fileset symlink.
*
* <p>Filesets are weird because sometimes exec paths of the {@link ActionInput}s in them are not
* relative, as exec paths should be, but absolute and point to under one of the package roots. In
* order to handle this, if we find such an absolute exec path, we iterate over the package path
* entries to turn it into a {@link RootedPath}.
*
* <p>The inputs to this function should be symlinks that are contained within Filesets; in
* particular, this is different from "unresolved symlinks" in that Fileset contents are regular
* files (but implemented by symlinks in the output tree) whose contents matter and unresolved
* symlinks are symlinks for which the important content is the result of {@code readlink()}
*/
private static RootedPath processFilesetSymlink(
PathFragment symlink, ImmutableList<Root> packageRoots) {
for (Root packageRoot : packageRoots) {
if (packageRoot.contains(symlink)) {
return RootedPath.toRootedPath(packageRoot, packageRoot.relativize(symlink));
}
}

throw new IllegalStateException(
String.format(
"absolute action input path '%s' not found under package roots",
symlink.getPathString()));
}

/**
* Returns the inputs of a Spawn as a map of PathFragments relative to an execRoot to paths in the
* host filesystem where the input files can be found.
*
* @param inputMap the map of action inputs and where they should be visible in the action
* @param execRootPath the exec root from the point of view of the Bazel server
* @param withinSandboxExecRootPath the exec root from within the sandbox (different from {@code
* execRootPath} because the sandbox does magic with fiile system namespaces)
* @param packageRoots the package path entries during this build
* @param sandboxSourceRoots the directory where source roots are mapped within the sandbox
* @throws IOException if processing symlinks fails
*/
public SandboxInputs processInputFiles(
Map<PathFragment, ActionInput> inputMap,
Path execRootPath,
Path withinSandboxExecRootPath,
ImmutableList<Root> packageRoots,
Path sandboxSourceRoots)
throws IOException {
Root withinSandboxExecRoot = Root.fromPath(withinSandboxExecRootPath);
Expand Down Expand Up @@ -503,7 +538,14 @@ public SandboxInputs processInputFiles(
if (actionInput instanceof EmptyActionInput) {
inputPath = null;
} else {
inputPath = RootedPath.toRootedPath(execRoot, actionInput.getExecPath());
PathFragment execPath = actionInput.getExecPath();
if (execPath.isAbsolute()) {
// This happens for ActionInputs that are part of Filesets (see the Javadoc on
// processFilesetSymlink())
inputPath = processFilesetSymlink(actionInput.getExecPath(), packageRoots);
} else {
inputPath = RootedPath.toRootedPath(execRoot, actionInput.getExecPath());
}
}

inputFiles.put(pathFragment, inputPath);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.google.devtools.build.lib.sandbox;

import com.google.common.base.Joiner;
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.ActionInput;
Expand Down Expand Up @@ -71,7 +72,11 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context

SandboxInputs readablePaths =
helpers.processInputFiles(
context.getInputMapping(PathFragment.EMPTY_FRAGMENT), execRoot, execRoot, null);
context.getInputMapping(PathFragment.EMPTY_FRAGMENT),
execRoot,
execRoot,
ImmutableList.of(),
null);

ImmutableSet.Builder<Path> writablePaths = ImmutableSet.builder();
writablePaths.addAll(getWritableDirs(execRoot, environment));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Stopwatch;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.hash.HashCode;
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
Expand Down Expand Up @@ -187,7 +188,11 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
Profiler.instance().profile(ProfilerTask.WORKER_SETUP, "Setting up inputs")) {
inputFiles =
helpers.processInputFiles(
context.getInputMapping(PathFragment.EMPTY_FRAGMENT), execRoot, execRoot, null);
context.getInputMapping(PathFragment.EMPTY_FRAGMENT),
execRoot,
execRoot,
ImmutableList.of(),
null);
}
SandboxOutputs outputs = helpers.getOutputs(spawn);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ public void processInputFiles_materializesParamFile() throws Exception {
UTF_8);

SandboxInputs inputs =
sandboxHelpers.processInputFiles(inputMap(paramFile), execRootPath, execRootPath, null);
sandboxHelpers.processInputFiles(
inputMap(paramFile), execRootPath, execRootPath, ImmutableList.of(), null);

assertThat(inputs.getFiles())
.containsExactly(PathFragment.create("paramFile"), execRootedPath("paramFile"));
Expand All @@ -121,7 +122,8 @@ public void processInputFiles_materializesBinToolsFile() throws Exception {
PathFragment.create("_bin/say_hello"));

SandboxInputs inputs =
sandboxHelpers.processInputFiles(inputMap(tool), execRootPath, execRootPath, null);
sandboxHelpers.processInputFiles(
inputMap(tool), execRootPath, execRootPath, ImmutableList.of(), null);

assertThat(inputs.getFiles())
.containsExactly(PathFragment.create("_bin/say_hello"), execRootedPath("_bin/say_hello"));
Expand Down Expand Up @@ -167,14 +169,15 @@ protected void setExecutable(PathFragment path, boolean executable) throws IOExc
try {
var unused =
sandboxHelpers.processInputFiles(
inputMap(input), customExecRoot, customExecRoot, null);
inputMap(input), customExecRoot, customExecRoot, ImmutableList.of(), null);
finishProcessingSemaphore.release();
} catch (IOException e) {
throw new IllegalArgumentException(e);
}
});
var unused =
sandboxHelpers.processInputFiles(inputMap(input), customExecRoot, customExecRoot, null);
sandboxHelpers.processInputFiles(
inputMap(input), customExecRoot, customExecRoot, ImmutableList.of(), null);
finishProcessingSemaphore.release();
future.get();

Expand Down

0 comments on commit 70691f2

Please sign in to comment.