diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java index 3b2930118f43ca..bfc280c42d8b73 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java @@ -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; @@ -163,9 +162,9 @@ void createInputs(Iterable 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); } 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 d72f9fbd648c13..9eb3915e27cc18 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 @@ -373,7 +373,7 @@ protected ImmutableSet 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"); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java index 69ca623aed23f2..7c2d84a5814ef8 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java @@ -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; @@ -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; @@ -246,9 +242,9 @@ private static void cleanRecursively( */ static Optional 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)); } @@ -384,31 +380,27 @@ public static void mountAdditionalPaths( /** Wrapper class for the inputs of a sandbox. */ public static final class SandboxInputs { - private final Map files; + private final Map files; private final Map virtualInputs; private final Map symlinks; - private final ImmutableSet 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 files, + Map files, Map virtualInputs, - Map symlinks, - Set sourceRoots) { + Map symlinks) { this.files = files; this.virtualInputs = virtualInputs; this.symlinks = symlinks; - this.sourceRoots = ImmutableSet.copyOf(sourceRoots); } public static SandboxInputs getEmptyInputs() { return EMPTY_INPUTS; } - public Map getFiles() { + public Map getFiles() { return files; } @@ -416,10 +408,6 @@ public Map getSymlinks() { return symlinks; } - public ImmutableSet getSourceRoots() { - return sourceRoots; - } - public ImmutableMap getVirtualInputDigests() { return ImmutableMap.copyOf(virtualInputs); } @@ -429,20 +417,15 @@ public ImmutableMap getVirtualInputDigests() { * included. */ public SandboxInputs limitedCopy(Set 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; } } @@ -456,10 +439,9 @@ public String toString() { */ public SandboxInputs processInputFiles(Map inputMap, Path execRoot) throws IOException, InterruptedException { - Map inputFiles = new TreeMap<>(); + Map inputFiles = new TreeMap<>(); Map inputSymlinks = new TreeMap<>(); Map virtualInputs = new HashMap<>(); - ImmutableSet.Builder sourceRoots = ImmutableSet.builder(); for (Map.Entry e : inputMap.entrySet()) { if (Thread.interrupted()) { @@ -470,22 +452,20 @@ public SandboxInputs processInputFiles(Map 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. */ diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxUtil.java b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxUtil.java index 707f541e98f7e8..c582ecb3df3da5 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxUtil.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxUtil.java @@ -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; @@ -107,7 +106,7 @@ public static class CommandLineBuilder { private Path stdoutPath; private Path stderrPath; private Set writableFilesAndDirectories = ImmutableSet.of(); - private Map readableFilesAndDirectories = new TreeMap<>(); + private Map readableFilesAndDirectories = new TreeMap<>(); private Set inaccessiblePaths = ImmutableSet.of(); private boolean useDebugMode = false; private List commandArguments = ImmutableList.of(); @@ -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 readableFilesAndDirectories) { + Map readableFilesAndDirectories) { this.readableFilesAndDirectories = readableFilesAndDirectories; return this; } @@ -213,8 +212,8 @@ public ImmutableList 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()); diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java index d8802d06f7b66b..f45f4f47eb26c6 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java @@ -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; @@ -87,9 +86,9 @@ static void createInputs(Iterable 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); } diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java index 8ffb94168b5c86..265e2092a5a569 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java @@ -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; @@ -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 { diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawnTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawnTest.java index c68453d8036d89..700189556cbf9d 100644 --- a/src/test/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawnTest.java +++ b/src/test/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawnTest.java @@ -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; @@ -374,18 +373,17 @@ private static SandboxInputs createSandboxInputs( private static SandboxInputs createSandboxInputs( ImmutableList files, ImmutableMap symlinks) { - Map filesMap = Maps.newHashMapWithExpectedSize(files.size()); + Map 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. */ diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java index 6d9af8bdddc256..5c8b4044bb81cb 100644 --- a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java +++ b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java @@ -43,8 +43,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.Root; -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; @@ -70,14 +68,12 @@ public class SandboxHelpersTest { private final Scratch scratch = new Scratch(); - private Path execRootPath; - private Root execRoot; + private Path execRoot; @Nullable private ExecutorService executorToCleanup; @Before public void createExecRoot() throws IOException { - execRootPath = scratch.dir("/execRoot"); - execRoot = Root.fromPath(execRootPath); + execRoot = scratch.dir("/execRoot"); } @After @@ -90,10 +86,6 @@ public void shutdownExecutor() throws InterruptedException { executorToCleanup.awaitTermination(TestUtils.WAIT_TIMEOUT_SECONDS, SECONDS); } - private RootedPath execRootedPath(String execPath) { - return RootedPath.toRootedPath(execRoot, PathFragment.create(execPath)); - } - @Test public void processInputFiles_materializesParamFile() throws Exception { SandboxHelpers sandboxHelpers = new SandboxHelpers(); @@ -104,15 +96,15 @@ public void processInputFiles_materializesParamFile() throws Exception { ParameterFileType.UNQUOTED, UTF_8); - SandboxInputs inputs = sandboxHelpers.processInputFiles(inputMap(paramFile), execRootPath); + SandboxInputs inputs = sandboxHelpers.processInputFiles(inputMap(paramFile), execRoot); assertThat(inputs.getFiles()) - .containsExactly(PathFragment.create("paramFile"), execRootedPath("paramFile")); + .containsExactly(PathFragment.create("paramFile"), execRoot.getChild("paramFile")); assertThat(inputs.getSymlinks()).isEmpty(); - assertThat(FileSystemUtils.readLines(execRootPath.getChild("paramFile"), UTF_8)) + assertThat(FileSystemUtils.readLines(execRoot.getChild("paramFile"), UTF_8)) .containsExactly("-a", "-b") .inOrder(); - assertThat(execRootPath.getChild("paramFile").isExecutable()).isTrue(); + assertThat(execRoot.getChild("paramFile").isExecutable()).isTrue(); } @Test @@ -123,15 +115,16 @@ public void processInputFiles_materializesBinToolsFile() throws Exception { scratch.file("tool", "#!/bin/bash", "echo hello"), PathFragment.create("_bin/say_hello")); - SandboxInputs inputs = sandboxHelpers.processInputFiles(inputMap(tool), execRootPath); + SandboxInputs inputs = sandboxHelpers.processInputFiles(inputMap(tool), execRoot); assertThat(inputs.getFiles()) - .containsExactly(PathFragment.create("_bin/say_hello"), execRootedPath("_bin/say_hello")); + .containsExactly( + PathFragment.create("_bin/say_hello"), execRoot.getRelative("_bin/say_hello")); assertThat(inputs.getSymlinks()).isEmpty(); - assertThat(FileSystemUtils.readLines(execRootPath.getRelative("_bin/say_hello"), UTF_8)) + assertThat(FileSystemUtils.readLines(execRoot.getRelative("_bin/say_hello"), UTF_8)) .containsExactly("#!/bin/bash", "echo hello") .inOrder(); - assertThat(execRootPath.getRelative("_bin/say_hello").isExecutable()).isTrue(); + assertThat(execRoot.getRelative("_bin/say_hello").isExecutable()).isTrue(); } /** @@ -237,10 +230,8 @@ public void atomicallyWriteVirtualInput_writesArbitraryVirtualInput() throws Exc @Test public void cleanExisting_updatesDirs() throws IOException, InterruptedException { - RootedPath inputTxt = - RootedPath.toRootedPath( - Root.fromPath(scratch.getFileSystem().getPath("/")), PathFragment.create("hello.txt")); - Path rootDir = execRootPath.getParentDirectory(); + Path inputTxt = scratch.getFileSystem().getPath(PathFragment.create("/hello.txt")); + Path rootDir = execRoot.getParentDirectory(); PathFragment input1 = PathFragment.create("existing/directory/with/input1.txt"); PathFragment input2 = PathFragment.create("partial/directory/input2.txt"); PathFragment input3 = PathFragment.create("new/directory/input3.txt"); @@ -248,8 +239,7 @@ public void cleanExisting_updatesDirs() throws IOException, InterruptedException new SandboxInputs( ImmutableMap.of(input1, inputTxt, input2, inputTxt, input3, inputTxt), ImmutableMap.of(), - ImmutableMap.of(), - ImmutableSet.of()); + ImmutableMap.of()); Set inputsToCreate = new LinkedHashSet<>(); LinkedHashSet dirsToCreate = new LinkedHashSet<>(); SandboxHelpers.populateInputsAndDirsToCreate( @@ -269,17 +259,17 @@ public void cleanExisting_updatesDirs() throws IOException, InterruptedException assertThat(inputsToCreate).containsExactly(input1, input2, input3); // inputdir1 exists fully - execRootPath.getRelative(inputDir1).createDirectoryAndParents(); + execRoot.getRelative(inputDir1).createDirectoryAndParents(); // inputdir2 exists partially, should be kept nonetheless. - execRootPath + execRoot .getRelative(inputDir2) .getParentDirectory() .getRelative("doomedSubdir") .createDirectoryAndParents(); // inputDir3 just doesn't exist // outputDir only exists partially - execRootPath.getRelative(outputDir).getParentDirectory().createDirectoryAndParents(); - execRootPath.getRelative("justSomeDir/thatIsDoomed").createDirectoryAndParents(); + execRoot.getRelative(outputDir).getParentDirectory().createDirectoryAndParents(); + execRoot.getRelative("justSomeDir/thatIsDoomed").createDirectoryAndParents(); // `thiswillbeafile/output` simulates a directory that was in the stashed dir but whose same // path is used later for a regular file. scratch.dir("/execRoot/thiswillbeafile/output"); @@ -290,23 +280,22 @@ public void cleanExisting_updatesDirs() throws IOException, InterruptedException new SandboxInputs( ImmutableMap.of(input1, inputTxt, input2, inputTxt, input3, inputTxt, input4, inputTxt), ImmutableMap.of(), - ImmutableMap.of(), - ImmutableSet.of()); - SandboxHelpers.cleanExisting(rootDir, inputs2, inputsToCreate, dirsToCreate, execRootPath); + ImmutableMap.of()); + SandboxHelpers.cleanExisting(rootDir, inputs2, inputsToCreate, dirsToCreate, execRoot); assertThat(dirsToCreate).containsExactly(inputDir2, inputDir3, outputDir); - assertThat(execRootPath.getRelative("existing/directory/with").exists()).isTrue(); - assertThat(execRootPath.getRelative("partial").exists()).isTrue(); - assertThat(execRootPath.getRelative("partial/doomedSubdir").exists()).isFalse(); - assertThat(execRootPath.getRelative("partial/directory").exists()).isFalse(); - assertThat(execRootPath.getRelative("justSomeDir/thatIsDoomed").exists()).isFalse(); - assertThat(execRootPath.getRelative("out").exists()).isTrue(); - assertThat(execRootPath.getRelative("out/dir").exists()).isFalse(); + assertThat(execRoot.getRelative("existing/directory/with").exists()).isTrue(); + assertThat(execRoot.getRelative("partial").exists()).isTrue(); + assertThat(execRoot.getRelative("partial/doomedSubdir").exists()).isFalse(); + assertThat(execRoot.getRelative("partial/directory").exists()).isFalse(); + assertThat(execRoot.getRelative("justSomeDir/thatIsDoomed").exists()).isFalse(); + assertThat(execRoot.getRelative("out").exists()).isTrue(); + assertThat(execRoot.getRelative("out/dir").exists()).isFalse(); } @Test public void populateInputsAndDirsToCreate_createsMappedDirectories() { ArtifactRoot outputRoot = - ArtifactRoot.asDerivedRoot(execRootPath, ArtifactRoot.RootType.Output, "outputs"); + ArtifactRoot.asDerivedRoot(execRoot, ArtifactRoot.RootType.Output, "outputs"); ActionInput outputFile = ActionsTestUtil.createArtifact(outputRoot, "bin/config/dir/file"); ActionInput outputDir = ActionsTestUtil.createTreeArtifactWithGeneratingAction( @@ -346,13 +335,13 @@ public void moveOutputs_mappedPathMovedToUnmappedPath() throws Exception { .setPathMapper(pathMapper) .build(); var sandboxHelpers = new SandboxHelpers(); - Path sandboxBase = execRootPath.getRelative("sandbox"); + Path sandboxBase = execRoot.getRelative("sandbox"); PathFragment mappedOutputPath = PathFragment.create("bin/output"); sandboxBase.getRelative(mappedOutputPath).getParentDirectory().createDirectoryAndParents(); FileSystemUtils.writeLinesAs( sandboxBase.getRelative(mappedOutputPath), UTF_8, "hello", "pathmapper"); - Path realBase = execRootPath.getRelative("real"); + Path realBase = execRoot.getRelative("real"); SandboxHelpers.moveOutputs(sandboxHelpers.getOutputs(spawn), sandboxBase, realBase); assertThat( diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawnTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawnTest.java index c6674d61431116..9c26026eeafe61 100644 --- a/src/test/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawnTest.java +++ b/src/test/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawnTest.java @@ -26,8 +26,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.Root; -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; @@ -62,9 +60,8 @@ public final void setupTestDirs() throws IOException { @Test public void createFileSystem() throws Exception { - RootedPath helloTxt = - RootedPath.toRootedPath(Root.fromPath(workspaceDir), PathFragment.create("hello.txt")); - FileSystemUtils.createEmptyFile(helloTxt.asPath()); + Path helloTxt = workspaceDir.getRelative("hello.txt"); + FileSystemUtils.createEmptyFile(helloTxt); SymlinkedSandboxedSpawn symlinkedExecRoot = new SymlinkedSandboxedSpawn( @@ -75,8 +72,7 @@ public void createFileSystem() throws Exception { new SandboxInputs( ImmutableMap.of(PathFragment.create("such/input.txt"), helloTxt), ImmutableMap.of(), - ImmutableMap.of(), - ImmutableSet.of()), + ImmutableMap.of()), SandboxOutputs.create( ImmutableSet.of(PathFragment.create("very/output.txt")), ImmutableSet.of()), ImmutableSet.of(execRoot.getRelative("wow/writable")), @@ -89,8 +85,7 @@ public void createFileSystem() throws Exception { symlinkedExecRoot.createFileSystem(); assertThat(execRoot.getRelative("such/input.txt").isSymbolicLink()).isTrue(); - assertThat(execRoot.getRelative("such/input.txt").resolveSymbolicLinks()) - .isEqualTo(helloTxt.asPath()); + assertThat(execRoot.getRelative("such/input.txt").resolveSymbolicLinks()).isEqualTo(helloTxt); assertThat(execRoot.getRelative("very").isDirectory()).isTrue(); assertThat(execRoot.getRelative("wow/writable").isDirectory()).isTrue(); } @@ -107,8 +102,7 @@ public void copyOutputs() throws Exception { execRoot, ImmutableList.of("/bin/true"), ImmutableMap.of(), - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableSet.of()), + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create( ImmutableSet.of(outputFile.relativeTo(execRoot)), ImmutableSet.of()), ImmutableSet.of(), diff --git a/src/test/java/com/google/devtools/build/lib/worker/SandboxHelper.java b/src/test/java/com/google/devtools/build/lib/worker/SandboxHelper.java index 3a4f18f5fc38a2..788614b4e47256 100644 --- a/src/test/java/com/google/devtools/build/lib/worker/SandboxHelper.java +++ b/src/test/java/com/google/devtools/build/lib/worker/SandboxHelper.java @@ -24,8 +24,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.Root; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.io.IOException; import java.util.ArrayList; @@ -40,7 +38,7 @@ class SandboxHelper { /** Map from workdir-relative input path to optional real file path. */ - private final Map inputs = new HashMap<>(); + private final Map inputs = new HashMap<>(); private final Map virtualInputs = new HashMap<>(); private final Map symlinks = new HashMap<>(); @@ -49,15 +47,12 @@ class SandboxHelper { private final List outputDirs = new ArrayList<>(); /** The global execRoot. */ - final Path execRootPath; - - final Root execRoot; + final Path execRoot; /** The worker process's sandbox root. */ final Path workDir; public SandboxHelper(Path execRoot, Path workDir) { - this.execRootPath = execRoot; - this.execRoot = Root.fromPath(execRoot); + this.execRoot = execRoot; this.workDir = workDir; } @@ -69,9 +64,7 @@ public SandboxHelper(Path execRoot, Path workDir) { public SandboxHelper addInputFile(String relativePath, String workspacePath) { inputs.put( PathFragment.create(relativePath), - workspacePath != null - ? RootedPath.toRootedPath(execRoot, PathFragment.create(workspacePath)) - : null); + workspacePath != null ? execRoot.getRelative(workspacePath) : null); return this; } @@ -84,7 +77,7 @@ public SandboxHelper addInputFile(String relativePath, String workspacePath) { public SandboxHelper addAndCreateInputFile( String relativePath, String workspacePath, String contents) throws IOException { addInputFile(relativePath, workspacePath); - Path absPath = execRootPath.getRelative(workspacePath); + Path absPath = execRoot.getRelative(workspacePath); absPath.getParentDirectory().createDirectoryAndParents(); FileSystemUtils.writeContentAsLatin1(absPath, contents); return this; @@ -95,7 +88,7 @@ public SandboxHelper addAndCreateInputFile( public SandboxHelper addAndCreateVirtualInput(String relativePath, String contents) { VirtualActionInput input = ActionsTestUtil.createVirtualActionInput(relativePath, contents); byte[] digest = - execRootPath + execRoot .getRelative(relativePath) .getFileSystem() .getDigestFunction() @@ -134,7 +127,7 @@ public SandboxHelper addOutputDir(String relativePath) { */ @CanIgnoreReturnValue public SandboxHelper addWorkerFile(String relativePath) { - Path absPath = execRootPath.getRelative(relativePath); + Path absPath = execRoot.getRelative(relativePath); workerFiles.put(PathFragment.create(relativePath), absPath); return this; } @@ -147,7 +140,7 @@ public SandboxHelper addWorkerFile(String relativePath) { public SandboxHelper addAndCreateWorkerFile(String relativePath, String contents) throws IOException { addWorkerFile(relativePath); - Path absPath = execRootPath.getRelative(relativePath); + Path absPath = execRoot.getRelative(relativePath); absPath.getParentDirectory().createDirectoryAndParents(); FileSystemUtils.writeContentAsLatin1(absPath, contents); return this; @@ -172,7 +165,7 @@ public SandboxHelper createExecRootFile(String relativePath, String contents) th @CanIgnoreReturnValue public SandboxHelper createWorkspaceDirFile(String workspaceDirPath, String contents) throws IOException { - Path absPath = execRootPath.getRelative(workspaceDirPath); + Path absPath = execRoot.getRelative(workspaceDirPath); absPath.getParentDirectory().createDirectoryAndParents(); FileSystemUtils.writeContentAsLatin1(absPath, contents); return this; @@ -191,7 +184,7 @@ public SandboxHelper createSymlink(String relativePath, String relativeDestinati } public SandboxInputs getSandboxInputs() { - return new SandboxInputs(inputs, virtualInputs, symlinks, ImmutableSet.of()); + return new SandboxInputs(inputs, virtualInputs, symlinks); } public SandboxOutputs getSandboxOutputs() { diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java index fe4032b0f84d06..9a70b84886bf40 100644 --- a/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java @@ -63,8 +63,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.Root; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest; import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse; @@ -125,8 +123,7 @@ public void testExecInWorker_happyPath() throws ExecException, InterruptedExcept spawn, key, context, - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableSet.of()), + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()), ImmutableList.of(), inputFileCache, @@ -218,8 +215,7 @@ public void testExecInWorker_finishesAsyncOnInterrupt() spawn, key, context, - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableSet.of()), + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()), ImmutableList.of(), inputFileCache, @@ -262,8 +258,7 @@ public void testExecInWorker_sendsCancelMessageOnInterrupt() spawn, key, context, - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableSet.of()), + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()), ImmutableList.of(), inputFileCache, @@ -305,8 +300,7 @@ public void testExecInWorker_unsandboxedDiesOnInterrupt() spawn, key, context, - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableSet.of()), + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()), ImmutableList.of(), inputFileCache, @@ -339,8 +333,7 @@ public void testExecInWorker_noMultiplexWithDynamic() spawn, key, context, - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableSet.of()), + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()), ImmutableList.of(), inputFileCache, @@ -377,8 +370,7 @@ private void assertRecordedResponsethrowsException(String recordedResponse, Stri spawn, key, context, - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableSet.of()), + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()), ImmutableList.of(), inputFileCache, @@ -424,12 +416,11 @@ public void testExpandArgument_expandsArgumentsRecursively() new SandboxInputs( ImmutableMap.of( PathFragment.create("file"), - asRootedPath("/file"), + fs.getPath("/file"), PathFragment.create("file2"), - asRootedPath("/file2")), + fs.getPath("/file2")), ImmutableMap.of(), - ImmutableMap.of(), - ImmutableSet.of()); + ImmutableMap.of()); WorkerSpawnRunner.expandArgument(inputs, "@file", requestBuilder); assertThat(requestBuilder.getArgumentsList()) .containsExactly("arg1", "arg2", "arg3", "multi arg", ""); @@ -442,10 +433,9 @@ public void testExpandArgument_expandsOnlyProperArguments() FileSystemUtils.writeIsoLatin1(fs.getPath("/file"), "arg1\n@@nonfile\n@foo//bar\narg2"); SandboxInputs inputs = new SandboxInputs( - ImmutableMap.of(PathFragment.create("file"), asRootedPath("/file")), - ImmutableMap.of(), + ImmutableMap.of(PathFragment.create("file"), fs.getPath("/file")), ImmutableMap.of(), - ImmutableSet.of()); + ImmutableMap.of()); WorkerSpawnRunner.expandArgument(inputs, "@file", requestBuilder); assertThat(requestBuilder.getArgumentsList()) .containsExactly("arg1", "@@nonfile", "@foo//bar", "arg2"); @@ -456,10 +446,9 @@ public void testExpandArgument_failsOnMissingFile() { WorkRequest.Builder requestBuilder = WorkRequest.newBuilder(); SandboxInputs inputs = new SandboxInputs( - ImmutableMap.of(PathFragment.create("file"), asRootedPath("/dir/file")), - ImmutableMap.of(), + ImmutableMap.of(PathFragment.create("file"), fs.getPath("/dir/file")), ImmutableMap.of(), - ImmutableSet.of()); + ImmutableMap.of()); IOException e = assertThrows( IOException.class, @@ -573,8 +562,7 @@ private WorkerSpawnRunner createWorkerSpawnRunner(WorkerOptions workerOptions) { public void testExpandArgument_failsOnUndeclaredInput() { WorkRequest.Builder requestBuilder = WorkRequest.newBuilder(); SandboxInputs inputs = - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableSet.of()); + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()); IOException e = assertThrows( IOException.class, @@ -583,10 +571,6 @@ public void testExpandArgument_failsOnUndeclaredInput() { assertThat(e).hasMessageThat().contains("declared input"); } - private RootedPath asRootedPath(String path) { - return RootedPath.toRootedPath(Root.absoluteRoot(fs), fs.getPath(path)); - } - private static String logMarker(String text) { return "---8<---8<--- " + text + " ---8<---8<---\n"; } diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategyTest.java index 2fb55d528be326..6c8ff50f2e2729 100644 --- a/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategyTest.java +++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategyTest.java @@ -18,12 +18,10 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; 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.RootedPath; import com.google.devtools.build.lib.vfs.util.FileSystems; import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest; import java.io.File; @@ -52,13 +50,13 @@ public void expandArgumentsPreservesEmptyLines() throws Exception { flags.forEach(pw::println); } - RootedPath path = - RootedPath.toRootedPath(Root.absoluteRoot(fs), fs.getPath(flagfile.getAbsolutePath())); + Path path = fs.getPath(flagfile.getAbsolutePath()); WorkRequest.Builder requestBuilder = WorkRequest.newBuilder(); SandboxInputs inputs = new SandboxInputs( - ImmutableMap.of(PathFragment.create("flagfile.txt"), path), ImmutableMap.of(), - ImmutableMap.of(), ImmutableSet.of()); + ImmutableMap.of(PathFragment.create("flagfile.txt"), path), + ImmutableMap.of(), + ImmutableMap.of()); WorkerSpawnRunner.expandArgument(inputs, "@flagfile.txt", requestBuilder); assertThat(requestBuilder.getArgumentsList()).containsExactlyElementsIn(flags);