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 db3a0e94a47705..2567ad78dae858 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,6 +61,7 @@ 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.XattrProvider; import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest; import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse; @@ -246,6 +247,7 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) private WorkRequest createWorkRequest( Spawn spawn, SpawnExecutionContext context, + SandboxInputs inputFiles, List flagfiles, Map virtualInputDigests, InputMetadataProvider inputFileCache, @@ -253,7 +255,7 @@ private WorkRequest createWorkRequest( throws IOException, InterruptedException { WorkRequest.Builder requestBuilder = WorkRequest.newBuilder(); for (String flagfile : flagfiles) { - expandArgument(execRoot, flagfile, requestBuilder); + expandArgument(inputFiles, flagfile, requestBuilder); } List inputs = @@ -298,27 +300,34 @@ private WorkRequest createWorkRequest( *

Also check that the argument is not an external repository label, because they start with * `@` and are not flagfile locations. * - * @param execRoot the current execroot of the build (relative paths will be assumed to be - * relative to this directory). + * @param inputs the inputs to locate flag files in. * @param arg the argument to expand. * @param requestBuilder the WorkRequest to whose arguments the expanded arguments will be added. * @throws java.io.IOException if one of the files containing options cannot be read. */ - static void expandArgument(Path execRoot, String arg, WorkRequest.Builder requestBuilder) + static void expandArgument(SandboxInputs inputs, String arg, WorkRequest.Builder requestBuilder) throws IOException, InterruptedException { if (arg.startsWith("@") && !arg.startsWith("@@") && !isExternalRepositoryLabel(arg)) { if (Thread.interrupted()) { throw new InterruptedException(); } String argValue = arg.substring(1); - Path path = execRoot.getRelative(argValue); + RootedPath 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, UTF_8)) { - expandArgument(execRoot, line, requestBuilder); + for (String line : FileSystemUtils.readLines(path.asPath(), UTF_8)) { + expandArgument(inputs, line, requestBuilder); } } catch (IOException e) { throw new IOException( - String.format("Failed to read @-argument '%s' from file '%s'.", argValue, path), e); + String.format( + "Failed to read @-argument '%s' from file '%s'.", + argValue, path.asPath().getPathString()), + e); } } else { requestBuilder.addArguments(arg); @@ -418,7 +427,8 @@ WorkResponse execInWorker( workerOwner = new WorkerOwner(handle.getWorker()); workerOwner.getWorker().setReporter(workerOptions.workerVerbose ? reporter : null); request = - createWorkRequest(spawn, context, flagFiles, virtualInputDigests, inputFileCache, key); + createWorkRequest( + spawn, context, inputFiles, flagFiles, virtualInputDigests, inputFileCache, key); // We acquired a worker and resources -- mark that as queuing time. spawnMetrics.setQueueTimeInMs((int) queueStopwatch.elapsed().toMillis()); 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 10118028244fd8..59bf28335db05a 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 @@ -61,6 +61,9 @@ import com.google.devtools.build.lib.vfs.FileSystem; 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.SyscallCache; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.build.lib.worker.WorkerPoolImpl.WorkerPoolConfig; @@ -435,7 +438,16 @@ public void testExpandArgument_expandsArgumentsRecursively() WorkRequest.Builder requestBuilder = WorkRequest.newBuilder(); FileSystemUtils.writeIsoLatin1(fs.getPath("/file"), "arg1\n@file2\nmulti arg\n"); FileSystemUtils.writeIsoLatin1(fs.getPath("/file2"), "arg2\narg3"); - WorkerSpawnRunner.expandArgument(fs.getPath("/"), "@file", requestBuilder); + SandboxInputs inputs = + new SandboxInputs( + ImmutableMap.of( + PathFragment.create("file"), + asRootedPath("/file"), + PathFragment.create("file2"), + asRootedPath("/file2")), + ImmutableMap.of(), + ImmutableMap.of(), ImmutableMap.of()); + WorkerSpawnRunner.expandArgument(inputs, "@file", requestBuilder); assertThat(requestBuilder.getArgumentsList()) .containsExactly("arg1", "arg2", "arg3", "multi arg", ""); } @@ -445,18 +457,28 @@ public void testExpandArgument_expandsOnlyProperArguments() throws IOException, InterruptedException { WorkRequest.Builder requestBuilder = WorkRequest.newBuilder(); FileSystemUtils.writeIsoLatin1(fs.getPath("/file"), "arg1\n@@nonfile\n@foo//bar\narg2"); - WorkerSpawnRunner.expandArgument(fs.getPath("/"), "@file", requestBuilder); + SandboxInputs inputs = + new SandboxInputs( + ImmutableMap.of(PathFragment.create("file"), asRootedPath("/file")), ImmutableMap.of(), + ImmutableMap.of(), ImmutableMap.of()); + WorkerSpawnRunner.expandArgument(inputs, "@file", requestBuilder); assertThat(requestBuilder.getArgumentsList()) .containsExactly("arg1", "@@nonfile", "@foo//bar", "arg2"); } @Test - public void testExpandArgument_failsOnMissingFile() throws IOException { + 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(), + ImmutableMap.of()); IOException e = assertThrows( IOException.class, - () -> WorkerSpawnRunner.expandArgument(fs.getPath("/dir/"), "@file", requestBuilder)); + () -> WorkerSpawnRunner.expandArgument(inputs, "@file", requestBuilder)); assertThat(e).hasMessageThat().contains("file"); assertThat(e).hasMessageThat().contains("/dir/file"); } @@ -564,6 +586,24 @@ private WorkerSpawnRunner createWorkerSpawnRunner(WorkerOptions workerOptions) { new JavaClock()); } + @Test + public void testExpandArgument_failsOnUndeclaredInput() { + WorkRequest.Builder requestBuilder = WorkRequest.newBuilder(); + SandboxInputs inputs = + new SandboxInputs( + ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()); + IOException e = + assertThrows( + IOException.class, + () -> WorkerSpawnRunner.expandArgument(inputs, "@file", requestBuilder)); + assertThat(e).hasMessageThat().contains("file"); + 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 e369fd9c7cdbe0..d20f8626626c84 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 @@ -17,8 +17,12 @@ import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +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; @@ -47,9 +51,14 @@ public void expandArgumentsPreservesEmptyLines() throws Exception { flags.forEach(pw::println); } - Path execRoot = fs.getPath(folder.getRoot().getAbsolutePath()); + RootedPath path = + RootedPath.toRootedPath(Root.absoluteRoot(fs), fs.getPath(flagfile.getAbsolutePath())); WorkRequest.Builder requestBuilder = WorkRequest.newBuilder(); - WorkerSpawnRunner.expandArgument(execRoot, "@flagfile.txt", requestBuilder); + SandboxInputs inputs = + new SandboxInputs( + ImmutableMap.of(PathFragment.create("flagfile.txt"), path), ImmutableMap.of(), + ImmutableMap.of(), ImmutableMap.of()); + WorkerSpawnRunner.expandArgument(inputs, "@flagfile.txt", requestBuilder); assertThat(requestBuilder.getArgumentsList()).containsExactlyElementsIn(flags); }