Skip to content

Commit

Permalink
Replace Path with RootedPath in SandboxHelpers and maintain a set of …
Browse files Browse the repository at this point in the history
…roots in SandboxHelpers.

This will make it possible to replace the root of inputs easily.

Work towards #3236.

RELNOTES: None.
PiperOrigin-RevId: 492432489
Change-Id: I896cd02001338779a1a9a7073131a86ea189ab51
  • Loading branch information
lberki authored and Copybara-Service committed Dec 2, 2022
1 parent e2549df commit a556969
Show file tree
Hide file tree
Showing 17 changed files with 217 additions and 130 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.RootedPath;
import java.io.IOException;
import java.util.LinkedHashSet;
import java.util.Set;
Expand Down Expand Up @@ -133,9 +134,9 @@ void createInputs(Iterable<PathFragment> inputsToCreate, SandboxInputs inputs)
for (PathFragment fragment : inputsToCreate) {
Path key = sandboxExecRoot.getRelative(fragment);
if (inputs.getFiles().containsKey(fragment)) {
Path fileDest = inputs.getFiles().get(fragment);
RootedPath fileDest = inputs.getFiles().get(fragment);
if (fileDest != null) {
copyFile(fileDest, key);
copyFile(fileDest.asPath(), key);
} else {
FileSystemUtils.createEmptyFile(key);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context

SandboxInputs inputs =
helpers.processInputFiles(
context.getInputMapping(PathFragment.EMPTY_FRAGMENT),
execRoot);
context.getInputMapping(PathFragment.EMPTY_FRAGMENT), execRoot, execRoot, 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,8 +222,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context

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

Duration timeout = context.getTimeout();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
ImmutableSet<Path> writableDirs = getWritableDirs(sandboxExecRoot, environment);

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

Duration timeout = context.getTimeout();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context

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

if (sandboxfsProcess != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import com.google.devtools.build.lib.vfs.FileSystemUtils.MoveResult;
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.RootedPath;
import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.devtools.common.options.OptionsParsingResult;
import java.io.IOException;
Expand Down Expand Up @@ -236,9 +238,9 @@ private static void cleanRecursively(
*/
static Optional<PathFragment> getExpectedSymlinkDestination(
PathFragment fragment, SandboxInputs inputs) {
Path file = inputs.getFiles().get(fragment);
RootedPath file = inputs.getFiles().get(fragment);
if (file != null) {
return Optional.of(file.asFragment());
return Optional.of(file.asPath().asFragment());
}
return Optional.ofNullable(inputs.getSymlinks().get(fragment));
}
Expand Down Expand Up @@ -347,27 +349,30 @@ public static final class SandboxInputs {
private static final AtomicInteger tempFileUniquifierForVirtualInputWrites =
new AtomicInteger();

private final Map<PathFragment, Path> files;
private final Map<PathFragment, RootedPath> files;
private final Map<VirtualActionInput, byte[]> virtualInputs;
private final Map<PathFragment, PathFragment> symlinks;
private final Map<Root, Path> sourceRootBindMounts;

private static final SandboxInputs EMPTY_INPUTS =
new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of());
new SandboxInputs(
ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of());

public SandboxInputs(
Map<PathFragment, Path> files,
Map<PathFragment, RootedPath> files,
Map<VirtualActionInput, byte[]> virtualInputs,
Map<PathFragment, PathFragment> symlinks) {
Map<PathFragment, PathFragment> symlinks,
Map<Root, Path> sourceRootBindMounts) {
this.files = files;
this.virtualInputs = virtualInputs;
this.symlinks = symlinks;
this.sourceRootBindMounts = sourceRootBindMounts;
}

public static SandboxInputs getEmptyInputs() {
return EMPTY_INPUTS;
}

public Map<PathFragment, Path> getFiles() {
public Map<PathFragment, RootedPath> getFiles() {
return files;
}

Expand Down Expand Up @@ -420,10 +425,16 @@ public ImmutableMap<VirtualActionInput, byte[]> getVirtualInputDigests() {
* included.
*/
public SandboxInputs limitedCopy(Set<PathFragment> allowed) {
Map<PathFragment, RootedPath> limitedFiles = Maps.filterKeys(files, allowed::contains);
Map<PathFragment, PathFragment> limitedSymlinks =
Maps.filterKeys(symlinks, allowed::contains);
Set<Root> usedRoots =
new HashSet<>(Maps.transformValues(limitedFiles, RootedPath::getRoot).values());
Map<Root, Path> limitedSourceRoots =
Maps.filterKeys(sourceRootBindMounts, usedRoots::contains);

return new SandboxInputs(
Maps.filterKeys(files, allowed::contains),
ImmutableMap.of(),
Maps.filterKeys(symlinks, allowed::contains));
limitedFiles, ImmutableMap.of(), limitedSymlinks, limitedSourceRoots);
}

@Override
Expand Down Expand Up @@ -456,35 +467,55 @@ private static byte[] writeVirtualInputTo(VirtualActionInput input, Path target)
*
* @throws IOException if processing symlinks fails
*/
public SandboxInputs processInputFiles(Map<PathFragment, ActionInput> inputMap, Path execRoot)
public SandboxInputs processInputFiles(
Map<PathFragment, ActionInput> inputMap,
Path execRootPath,
Path withinSandboxExecRootPath,
Path sandboxSourceRoots)
throws IOException {
Map<PathFragment, Path> inputFiles = new TreeMap<>();
Root withinSandboxExecRoot = Root.fromPath(withinSandboxExecRootPath);
Root execRoot =
withinSandboxExecRootPath.equals(execRootPath)
? withinSandboxExecRoot
: Root.fromPath(execRootPath);

Map<PathFragment, RootedPath> inputFiles = new TreeMap<>();
Map<PathFragment, PathFragment> inputSymlinks = new TreeMap<>();
Map<VirtualActionInput, byte[]> virtualInputs = new HashMap<>();
Map<Root, Root> sourceRootToSandboxSourceRoot = new TreeMap<>();

for (Map.Entry<PathFragment, ActionInput> e : inputMap.entrySet()) {
PathFragment pathFragment = e.getKey();
ActionInput actionInput = e.getValue();

if (actionInput instanceof VirtualActionInput) {
byte[] digest =
SandboxInputs.materializeVirtualInput(
(VirtualActionInput) actionInput, execRoot, /* isExecRootSandboxed=*/ true);
(VirtualActionInput) actionInput, execRootPath, /* isExecRootSandboxed= */ true);
virtualInputs.put((VirtualActionInput) actionInput, digest);
}

if (actionInput.isSymlink()) {
Path inputPath = execRoot.getRelative(actionInput.getExecPath());
inputSymlinks.put(pathFragment, inputPath.readSymbolicLink());
} else {
Path inputPath =
actionInput instanceof EmptyActionInput
? null
: execRoot.getRelative(actionInput.getExecPath());
RootedPath inputPath;

if (actionInput instanceof EmptyActionInput) {
inputPath = null;
} else {
inputPath = RootedPath.toRootedPath(execRoot, actionInput.getExecPath());
}

inputFiles.put(pathFragment, inputPath);
}
}
return new SandboxInputs(inputFiles, virtualInputs, inputSymlinks);

Map<Root, Path> sandboxRootToSourceRoot = new TreeMap<>();
for (Map.Entry<Root, Root> entry : sourceRootToSandboxSourceRoot.entrySet()) {
sandboxRootToSourceRoot.put(entry.getValue(), entry.getKey().asPath());
}

return new SandboxInputs(inputFiles, virtualInputs, inputSymlinks, sandboxRootToSourceRoot);
}

/** The file and directory outputs of a sandboxed spawn. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.RootedPath;
import java.io.IOException;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -320,19 +321,19 @@ private static void createSandbox(
// created once we encounter the first symlink in the list of inputs.
Map<PathFragment, PathFragment> symlinks = null;

for (Map.Entry<PathFragment, Path> entry : inputs.getFiles().entrySet()) {
for (Map.Entry<PathFragment, RootedPath> entry : inputs.getFiles().entrySet()) {
if (entry.getValue() == null) {
if (emptyFile == null) {
Path emptyFilePath = scratchDir.getRelative("empty");
FileSystemUtils.createEmptyFile(emptyFilePath);
emptyFile = emptyFilePath.asFragment();
}
} else {
if (sandboxfsMapSymlinkTargets && entry.getValue().isSymbolicLink()) {
if (sandboxfsMapSymlinkTargets && entry.getValue().asPath().isSymbolicLink()) {
if (symlinks == null) {
symlinks = new HashMap<>();
}
computeSymlinkMappings(entry.getKey(), entry.getValue(), symlinks);
computeSymlinkMappings(entry.getKey(), entry.getValue().asPath(), symlinks);
}
}
}
Expand All @@ -348,13 +349,13 @@ private static void createSandbox(
(mapper) -> {
mapper.map(rootFragment, scratchDir.asFragment(), true);

for (Map.Entry<PathFragment, Path> entry : inputs.getFiles().entrySet()) {
for (Map.Entry<PathFragment, RootedPath> entry : inputs.getFiles().entrySet()) {
PathFragment target;
if (entry.getValue() == null) {
checkNotNull(finalEmptyFile, "Must have been initialized above by matching logic");
target = finalEmptyFile;
} else {
target = entry.getValue().asFragment();
target = entry.getValue().asPath().asFragment();
}
mapper.map(rootFragment.getRelative(entry.getKey()), target, false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.devtools.build.lib.shell.SubprocessBuilder.StreamAction;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.io.ByteArrayOutputStream;
import java.io.File;
Expand Down Expand Up @@ -106,7 +107,7 @@ public static class CommandLineBuilder {
private Path stdoutPath;
private Path stderrPath;
private Set<Path> writableFilesAndDirectories = ImmutableSet.of();
private Map<PathFragment, Path> readableFilesAndDirectories = new TreeMap<>();
private Map<PathFragment, RootedPath> readableFilesAndDirectories = new TreeMap<>();
private Set<Path> inaccessiblePaths = ImmutableSet.of();
private boolean useDebugMode = false;
private List<String> commandArguments = ImmutableList.of();
Expand Down Expand Up @@ -165,7 +166,7 @@ public CommandLineBuilder setWritableFilesAndDirectories(
/** Sets the files or directories to make readable for the sandboxed process, if any. */
@CanIgnoreReturnValue
public CommandLineBuilder setReadableFilesAndDirectories(
Map<PathFragment, Path> readableFilesAndDirectories) {
Map<PathFragment, RootedPath> readableFilesAndDirectories) {
this.readableFilesAndDirectories = readableFilesAndDirectories;
return this;
}
Expand Down Expand Up @@ -212,8 +213,8 @@ public ImmutableList<String> build() {
for (Path writablePath : writableFilesAndDirectories) {
commandLineBuilder.add("-w", writablePath.getPathString());
}
for (Path readablePath : readableFilesAndDirectories.values()) {
commandLineBuilder.add("-r", readablePath.getPathString());
for (RootedPath readablePath : readableFilesAndDirectories.values()) {
commandLineBuilder.add("-r", readablePath.asPath().getPathString());
}
for (Path writablePath : inaccessiblePaths) {
commandLineBuilder.add("-b", writablePath.getPathString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context

SandboxInputs readablePaths =
helpers.processInputFiles(
context.getInputMapping(PathFragment.EMPTY_FRAGMENT),
execRoot);
context.getInputMapping(PathFragment.EMPTY_FRAGMENT), execRoot, execRoot, 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 @@ -21,6 +21,7 @@
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.RootedPath;
import java.io.IOException;
import java.util.LinkedHashSet;
import java.util.List;
Expand Down Expand Up @@ -80,9 +81,9 @@ static void createInputs(Iterable<PathFragment> inputsToCreate, SandboxInputs in
for (PathFragment fragment : inputsToCreate) {
Path key = dir.getRelative(fragment);
if (inputs.getFiles().containsKey(fragment)) {
Path fileDest = inputs.getFiles().get(fragment);
RootedPath fileDest = inputs.getFiles().get(fragment);
if (fileDest != null) {
key.createSymbolicLink(fileDest);
key.createSymbolicLink(fileDest.asPath());
} else {
FileSystemUtils.createEmptyFile(key);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ 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);
context.getInputMapping(PathFragment.EMPTY_FRAGMENT), execRoot, execRoot, null);
}
SandboxOutputs outputs = helpers.getOutputs(spawn);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
import java.io.IOException;
Expand Down Expand Up @@ -371,17 +372,18 @@ private static SandboxInputs createSandboxInputs(

private static SandboxInputs createSandboxInputs(
ImmutableList<String> files, ImmutableMap<String, String> symlinks) {
Map<PathFragment, Path> filesMap = Maps.newHashMapWithExpectedSize(files.size());
Map<PathFragment, RootedPath> filesMap = Maps.newHashMapWithExpectedSize(files.size());
for (String file : files) {
filesMap.put(PathFragment.create(file), null);
}
return new SandboxInputs(
filesMap,
/*virtualInputs=*/ ImmutableMap.of(),
/* virtualInputs= */ ImmutableMap.of(),
symlinks.entrySet().stream()
.collect(
toImmutableMap(
e -> PathFragment.create(e.getKey()), e -> PathFragment.create(e.getValue()))));
e -> PathFragment.create(e.getKey()), e -> PathFragment.create(e.getValue()))),
ImmutableMap.of());
}

/** Return a list of all entries under the provided directory recursively. */
Expand Down

0 comments on commit a556969

Please sign in to comment.