Skip to content

Commit

Permalink
Re-revert the switch to RootedPath
Browse files Browse the repository at this point in the history
  • Loading branch information
@fmeum
fmeum committed May 14, 2024
1 parent 9c0c738 commit 9ebdcc2
Show file tree
Hide file tree
Showing 12 changed files with 100 additions and 164 deletions.
5 changes: 2 additions & 3 deletions ...main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
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 @@ -163,9 +162,9 @@ void createInputs(Iterable<PathFragment> inputsToCreate, SandboxInputs inputs)
}
Path key = sandboxExecRoot.getRelative(fragment);
if (inputs.getFiles().containsKey(fragment)) {
RootedPath fileDest = inputs.getFiles().get(fragment);
Path fileDest = inputs.getFiles().get(fragment);
if (fileDest != null) {
copyFile(fileDest.asPath(), key);
copyFile(fileDest, key);
} else {
FileSystemUtils.createEmptyFile(key);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ protected ImmutableSet<Path> getWritableDirs(
// 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(execRoot);
}

String testTmpdir = env.get("TEST_TMPDIR");
Expand Down
54 changes: 17 additions & 37 deletions src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,10 @@

import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import com.google.common.collect.Collections2;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.Artifact;
Expand All @@ -46,8 +44,6 @@
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 @@ -246,9 +242,9 @@ private static void cleanRecursively(
*/
static Optional<PathFragment> getExpectedSymlinkDestination(
PathFragment fragment, SandboxInputs inputs) {
RootedPath file = inputs.getFiles().get(fragment);
Path file = inputs.getFiles().get(fragment);
if (file != null) {
return Optional.of(file.asPath().asFragment());
return Optional.of(file.asFragment());
}
return Optional.ofNullable(inputs.getSymlinks().get(fragment));
}
Expand Down Expand Up @@ -384,42 +380,34 @@ public static void mountAdditionalPaths(

/** Wrapper class for the inputs of a sandbox. */
public static final class SandboxInputs {
private final Map<PathFragment, RootedPath> files;
private final Map<PathFragment, Path> files;
private final Map<VirtualActionInput, byte[]> virtualInputs;
private final Map<PathFragment, PathFragment> symlinks;
private final ImmutableSet<Root> sourceRoots;

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

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

public static SandboxInputs getEmptyInputs() {
return EMPTY_INPUTS;
}

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

public Map<PathFragment, PathFragment> getSymlinks() {
return symlinks;
}

public ImmutableSet<Root> getSourceRoots() {
return sourceRoots;
}

public ImmutableMap<VirtualActionInput, byte[]> getVirtualInputDigests() {
return ImmutableMap.copyOf(virtualInputs);
}
Expand All @@ -429,20 +417,15 @@ public ImmutableMap<VirtualActionInput, byte[]> getVirtualInputDigests() {
* included.
*/
public SandboxInputs limitedCopy(Set<PathFragment> allowed) {
var limitedFiles = Maps.filterKeys(files, allowed::contains);
var limitedFilesRoots =
new HashSet<>(Collections2.transform(limitedFiles.values(), RootedPath::getRoot));
return new SandboxInputs(
limitedFiles,
Maps.filterKeys(files, allowed::contains),
ImmutableMap.of(),
Maps.filterKeys(symlinks, allowed::contains),
Sets.intersection(sourceRoots, limitedFilesRoots));
Maps.filterKeys(symlinks, allowed::contains));
}

@Override
public String toString() {
return "Files: %s\nVirtualInputs: %s\nSymlinks: %s\nSourceRoots: %s"
.formatted(files, virtualInputs, symlinks, sourceRoots);
return "Files: " + files + "\nVirtualInputs: " + virtualInputs + "\nSymlinks: " + symlinks;
}
}

Expand All @@ -456,10 +439,9 @@ public String toString() {
*/
public SandboxInputs processInputFiles(Map<PathFragment, ActionInput> inputMap, Path execRoot)
throws IOException, InterruptedException {
Map<PathFragment, RootedPath> inputFiles = new TreeMap<>();
Map<PathFragment, Path> inputFiles = new TreeMap<>();
Map<PathFragment, PathFragment> inputSymlinks = new TreeMap<>();
Map<VirtualActionInput, byte[]> virtualInputs = new HashMap<>();
ImmutableSet.Builder<Root> sourceRoots = ImmutableSet.builder();

for (Map.Entry<PathFragment, ActionInput> e : inputMap.entrySet()) {
if (Thread.interrupted()) {
Expand All @@ -470,22 +452,20 @@ public SandboxInputs processInputFiles(Map<PathFragment, ActionInput> inputMap,
if (actionInput instanceof VirtualActionInput input) {
byte[] digest = input.atomicallyWriteRelativeTo(execRoot);
virtualInputs.put(input, digest);
} else if (actionInput instanceof Artifact artifact && artifact.isSourceArtifact()) {
sourceRoots.add(artifact.getRoot().getRoot());
}

if (actionInput.isSymlink()) {
Path inputPath = execRoot.getRelative(actionInput.getExecPath());
inputSymlinks.put(pathFragment, inputPath.readSymbolicLink());
} else if (actionInput instanceof EmptyActionInput) {
inputFiles.put(pathFragment, null);
} else {
inputFiles.put(
pathFragment,
RootedPath.toRootedPath(Root.fromPath(execRoot), actionInput.getExecPath()));
Path inputPath =
actionInput instanceof EmptyActionInput
? null
: execRoot.getRelative(actionInput.getExecPath());
inputFiles.put(pathFragment, inputPath);
}
}
return new SandboxInputs(inputFiles, virtualInputs, inputSymlinks, sourceRoots.build());
return new SandboxInputs(inputFiles, virtualInputs, inputSymlinks);
}

/** The file and directory outputs of a sandboxed spawn. */
Expand Down
9 changes: 4 additions & 5 deletions src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
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 @@ -107,7 +106,7 @@ public static class CommandLineBuilder {
private Path stdoutPath;
private Path stderrPath;
private Set<Path> writableFilesAndDirectories = ImmutableSet.of();
private Map<PathFragment, RootedPath> readableFilesAndDirectories = new TreeMap<>();
private Map<PathFragment, Path> readableFilesAndDirectories = new TreeMap<>();
private Set<Path> inaccessiblePaths = ImmutableSet.of();
private boolean useDebugMode = false;
private List<String> commandArguments = ImmutableList.of();
Expand Down Expand Up @@ -166,7 +165,7 @@ public CommandLineBuilder setWritableFilesAndDirectories(
/** Sets the files or directories to make readable for the sandboxed process, if any. */
@CanIgnoreReturnValue
public CommandLineBuilder setReadableFilesAndDirectories(
Map<PathFragment, RootedPath> readableFilesAndDirectories) {
Map<PathFragment, Path> readableFilesAndDirectories) {
this.readableFilesAndDirectories = readableFilesAndDirectories;
return this;
}
Expand Down Expand Up @@ -213,8 +212,8 @@ public ImmutableList<String> build() {
for (Path writablePath : writableFilesAndDirectories) {
commandLineBuilder.add("-w", writablePath.getPathString());
}
for (RootedPath readablePath : readableFilesAndDirectories.values()) {
commandLineBuilder.add("-r", readablePath.asPath().getPathString());
for (Path readablePath : readableFilesAndDirectories.values()) {
commandLineBuilder.add("-r", readablePath.getPathString());
}
for (Path writablePath : inaccessiblePaths) {
commandLineBuilder.add("-b", writablePath.getPathString());
Expand Down
5 changes: 2 additions & 3 deletions src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
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 @@ -87,9 +86,9 @@ static void createInputs(Iterable<PathFragment> inputsToCreate, SandboxInputs in
}
Path key = dir.getRelative(fragment);
if (inputs.getFiles().containsKey(fragment)) {
RootedPath fileDest = inputs.getFiles().get(fragment);
Path fileDest = inputs.getFiles().get(fragment);
if (fileDest != null) {
key.createSymbolicLink(fileDest.asPath());
key.createSymbolicLink(fileDest);
} else {
FileSystemUtils.createEmptyFile(key);
}
Expand Down
11 changes: 7 additions & 4 deletions src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@
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.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.XattrProvider;
import com.google.devtools.build.lib.vfs.XattrProvider;
import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest;
import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse;
import com.google.protobuf.ByteString;
Expand Down Expand Up @@ -308,21 +312,20 @@ static void expandArgument(SandboxInputs inputs, String arg, WorkRequest.Builder
throw new InterruptedException();
}
String argValue = arg.substring(1);
RootedPath path = inputs.getFiles().get(PathFragment.create(argValue));
Path path = inputs.getFiles().get(PathFragment.create(argValue));
if (path == null) {
throw new IOException(
String.format(
"Failed to read @-argument '%s': file is not a declared input", argValue));
}
try {
for (String line : FileSystemUtils.readLines(path.asPath(), UTF_8)) {
for (String line : FileSystemUtils.readLines(path, UTF_8)) {
expandArgument(inputs, line, requestBuilder);
}
} catch (IOException e) {
throw new IOException(
String.format(
"Failed to read @-argument '%s' from file '%s'.",
argValue, path.asPath().getPathString()),
"Failed to read @-argument '%s' from file '%s'.", argValue, path.getPathString()),
e);
}
} else {
Expand Down
8 changes: 3 additions & 5 deletions .../java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawnTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
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 @@ -374,18 +373,17 @@ private static SandboxInputs createSandboxInputs(

private static SandboxInputs createSandboxInputs(
ImmutableList<String> files, ImmutableMap<String, String> symlinks) {
Map<PathFragment, RootedPath> filesMap = Maps.newHashMapWithExpectedSize(files.size());
Map<PathFragment, Path> 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()))),
ImmutableSet.of());
e -> PathFragment.create(e.getKey()), e -> PathFragment.create(e.getValue()))));
}

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

0 comments on commit 9ebdcc2

Please sign in to comment.