From 1f3f9f4c4b2eded90518aacd1b0b80c1b0dfd1c5 Mon Sep 17 00:00:00 2001 From: lberki Date: Mon, 8 Feb 2021 04:15:53 -0800 Subject: [PATCH] Use the parent directory of the exec root as the input root on RBE. This allows --experimental_sibling_repository_layout to work with RBE (otherwise, we'd tell RBE that the action requires inputs like `../$REPO/$PATH`, which is outside of the allowable subtree) RELNOTES: None. PiperOrigin-RevId: 356234098 --- .../build/lib/exec/AbstractSpawnStrategy.java | 14 +- .../build/lib/exec/SpawnInputExpander.java | 60 +++++--- .../devtools/build/lib/exec/SpawnRunner.java | 14 +- .../build/lib/remote/RemoteCache.java | 27 ++-- .../build/lib/remote/RemoteSpawnCache.java | 7 +- .../build/lib/remote/RemoteSpawnRunner.java | 24 ++- .../sandbox/DarwinSandboxedSpawnRunner.java | 6 +- .../sandbox/DockerSandboxedSpawnRunner.java | 5 +- .../sandbox/LinuxSandboxedSpawnRunner.java | 6 +- .../ProcessWrapperSandboxedSpawnRunner.java | 6 +- .../sandbox/WindowsSandboxedSpawnRunner.java | 5 +- .../build/lib/worker/WorkerSpawnRunner.java | 5 +- .../lib/exec/SpawnInputExpanderTest.java | 54 ++++--- .../lib/exec/local/LocalSpawnRunnerTest.java | 2 +- .../build/lib/remote/GrpcCacheClientTest.java | 20 +-- .../build/lib/remote/RemoteCacheTests.java | 139 +++++++++--------- .../lib/remote/RemoteSpawnCacheTest.java | 5 +- .../lib/remote/RemoteSpawnRunnerTest.java | 2 +- ...SpawnRunnerWithGrpcRemoteExecutorTest.java | 9 +- .../util/FakeSpawnExecutionContext.java | 5 +- .../build/remote/worker/ExecutionServer.java | 6 +- 21 files changed, 264 insertions(+), 157 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java index a2af7191273077..079c54a540aeb7 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java @@ -172,7 +172,7 @@ public ImmutableList exec( spawnLogContext.logSpawn( spawn, actionExecutionContext.getMetadataProvider(), - context.getInputMapping(), + context.getInputMapping(PathFragment.EMPTY_FRAGMENT), context.getTimeout(), spawnResult); } catch (IOException e) { @@ -213,6 +213,7 @@ private final class SpawnExecutionContextImpl implements SpawnExecutionContext { // Memoize the input mapping so that prefetchInputs can reuse it instead of recomputing it. // TODO(ulfjack): Guard against client modification of this map. private SortedMap lazyInputMapping; + private PathFragment inputMappingBaseDirectory; SpawnExecutionContextImpl( Spawn spawn, @@ -235,7 +236,8 @@ public void prefetchInputs() throws IOException, InterruptedException { if (Spawns.shouldPrefetchInputsForLocalExecution(spawn)) { actionExecutionContext .getActionInputPrefetcher() - .prefetchFiles(getInputMapping().values(), getMetadataProvider()); + .prefetchFiles( + getInputMapping(PathFragment.EMPTY_FRAGMENT).values(), getMetadataProvider()); } } @@ -287,17 +289,21 @@ public FileOutErr getFileOutErr() { } @Override - public SortedMap getInputMapping() throws IOException { - if (lazyInputMapping == null) { + public SortedMap getInputMapping(PathFragment baseDirectory) + throws IOException { + if (lazyInputMapping == null || !inputMappingBaseDirectory.equals(baseDirectory)) { try (SilentCloseable c = Profiler.instance().profile("AbstractSpawnStrategy.getInputMapping")) { + inputMappingBaseDirectory = baseDirectory; lazyInputMapping = spawnInputExpander.getInputMapping( spawn, actionExecutionContext.getArtifactExpander(), + baseDirectory, actionExecutionContext.getMetadataProvider()); } } + return lazyInputMapping; } diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java index ad4fe00e5b98ac..c3c878368ef0a1 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java @@ -96,9 +96,10 @@ public SpawnInputExpander( private void addMapping( Map inputMappings, PathFragment targetLocation, - ActionInput input) { + ActionInput input, + PathFragment baseDirectory) { Preconditions.checkArgument(!targetLocation.isAbsolute(), targetLocation); - inputMappings.put(targetLocation, input); + inputMappings.put(baseDirectory.getRelative(targetLocation), input); } /** Adds runfiles inputs from runfilesSupplier to inputMappings. */ @@ -107,7 +108,8 @@ void addRunfilesToInputs( Map inputMap, RunfilesSupplier runfilesSupplier, MetadataProvider actionFileCache, - ArtifactExpander artifactExpander) + ArtifactExpander artifactExpander, + PathFragment baseDirectory) throws IOException { Map> rootsAndMappings = runfilesSupplier.getMappings(); @@ -129,7 +131,8 @@ void addRunfilesToInputs( addMapping( inputMap, location.getRelative(((TreeFileArtifact) input).getParentRelativePath()), - input); + input, + baseDirectory); } } else if (localArtifact.isFileset()) { ImmutableList filesetLinks; @@ -138,15 +141,15 @@ void addRunfilesToInputs( } catch (MissingExpansionException e) { throw new IllegalStateException(e); } - addFilesetManifest(location, localArtifact, filesetLinks, inputMap); + addFilesetManifest(location, localArtifact, filesetLinks, inputMap, baseDirectory); } else { if (strict) { failIfDirectory(actionFileCache, localArtifact); } - addMapping(inputMap, location, localArtifact); + addMapping(inputMap, location, localArtifact, baseDirectory); } } else { - addMapping(inputMap, location, VirtualActionInput.EMPTY_MARKER); + addMapping(inputMap, location, VirtualActionInput.EMPTY_MARKER, baseDirectory); } } } @@ -156,10 +159,12 @@ void addRunfilesToInputs( public Map addRunfilesToInputs( RunfilesSupplier runfilesSupplier, MetadataProvider actionFileCache, - ArtifactExpander artifactExpander) + ArtifactExpander artifactExpander, + PathFragment baseDirectory) throws IOException { Map inputMap = new HashMap<>(); - addRunfilesToInputs(inputMap, runfilesSupplier, actionFileCache, artifactExpander); + addRunfilesToInputs( + inputMap, runfilesSupplier, actionFileCache, artifactExpander, baseDirectory); return inputMap; } @@ -174,11 +179,16 @@ private static void failIfDirectory(MetadataProvider actionFileCache, ActionInpu @VisibleForTesting void addFilesetManifests( Map> filesetMappings, - Map inputMappings) + Map inputMappings, + PathFragment baseDirectory) throws IOException { for (Artifact fileset : filesetMappings.keySet()) { addFilesetManifest( - fileset.getExecPath(), fileset, filesetMappings.get(fileset), inputMappings); + fileset.getExecPath(), + fileset, + filesetMappings.get(fileset), + inputMappings, + baseDirectory); } } @@ -186,7 +196,8 @@ void addFilesetManifest( PathFragment location, Artifact filesetArtifact, ImmutableList filesetLinks, - Map inputMappings) + Map inputMappings, + PathFragment baseDirectory) throws IOException { Preconditions.checkState(filesetArtifact.isFileset(), filesetArtifact); FilesetManifest filesetManifest = @@ -198,16 +209,19 @@ void addFilesetManifest( value == null ? VirtualActionInput.EMPTY_MARKER : ActionInputHelper.fromPath(execRoot.getRelative(value).getPathString()); - addMapping(inputMappings, mapping.getKey(), artifact); + addMapping(inputMappings, mapping.getKey(), artifact, baseDirectory); } } private void addInputs( - Map inputMap, Spawn spawn, ArtifactExpander artifactExpander) { + Map inputMap, + Spawn spawn, + ArtifactExpander artifactExpander, + PathFragment baseDirectory) { List inputs = ActionInputHelper.expandArtifacts(spawn.getInputFiles(), artifactExpander); for (ActionInput input : inputs) { - addMapping(inputMap, input.getExecPath(), input); + addMapping(inputMap, input.getExecPath(), input, baseDirectory); } } @@ -223,14 +237,20 @@ private void addInputs( *

The returned map contains all runfiles, but not the {@code MANIFEST}. */ public SortedMap getInputMapping( - Spawn spawn, ArtifactExpander artifactExpander, MetadataProvider actionInputFileCache) + Spawn spawn, + ArtifactExpander artifactExpander, + PathFragment baseDirectory, + MetadataProvider actionInputFileCache) throws IOException { - TreeMap inputMap = new TreeMap<>(); - addInputs(inputMap, spawn, artifactExpander); + addInputs(inputMap, spawn, artifactExpander, baseDirectory); addRunfilesToInputs( - inputMap, spawn.getRunfilesSupplier(), actionInputFileCache, artifactExpander); - addFilesetManifests(spawn.getFilesetMappings(), inputMap); + inputMap, + spawn.getRunfilesSupplier(), + actionInputFileCache, + artifactExpander, + baseDirectory); + addFilesetManifests(spawn.getFilesetMappings(), inputMap, baseDirectory); return inputMap; } } diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java index 36c578e61859b7..7b79837baeabdf 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java @@ -198,7 +198,19 @@ default ArtifactPathResolver getPathResolver() { /** The files to which to write stdout and stderr. */ FileOutErr getFileOutErr(); - SortedMap getInputMapping() throws IOException; + /** + * Returns a sorted map from input paths to action inputs. + * + *

Resolves cases where a single input of the {@link Spawn} gives rise to multiple files in + * the input tree, for example, tree artifacts, runfiles trees and {@code Fileset} input + * manifests. + * + *

{@code baseDirectory} is prepended to every path in the input key. This is useful if the + * mapping is used in a context where the directory relative to which the keys are interpreted + * is not the same as the execroot. + */ + SortedMap getInputMapping(PathFragment baseDirectory) + throws IOException; /** Reports a progress update to the Spawn strategy. */ void report(ProgressStatus state, String name); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java index 6bfc730351d02b..1406218057ce47 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java @@ -314,7 +314,10 @@ public void download( FileOutErr origOutErr, OutputFilesLocker outputFilesLocker) throws ExecException, IOException, InterruptedException { - ActionResultMetadata metadata = parseActionResultMetadata(context, result, execRoot); + // The input root for RBE is the parent directory of the exec root so that paths to files in + // external repositories don't start with an uplevel reference + Path inputRoot = execRoot.getParentDirectory(); + ActionResultMetadata metadata = parseActionResultMetadata(context, result, inputRoot); List> downloads = Stream.concat( @@ -349,12 +352,12 @@ public void download( try { // Delete any (partially) downloaded output files. for (OutputFile file : result.getOutputFilesList()) { - toTmpDownloadPath(execRoot.getRelative(file.getPath())).delete(); + toTmpDownloadPath(inputRoot.getRelative(file.getPath())).delete(); } for (OutputDirectory directory : result.getOutputDirectoriesList()) { // Only delete the directories below the output directories because the output // directories will not be re-created - execRoot.getRelative(directory.getPath()).deleteTreesBelow(); + inputRoot.getRelative(directory.getPath()).deleteTreesBelow(); } if (tmpOutErr != null) { tmpOutErr.clearOut(); @@ -589,7 +592,11 @@ public InMemoryOutput downloadMinimal( ActionResultMetadata metadata; try (SilentCloseable c = Profiler.instance().profile("Remote.parseActionResultMetadata")) { - metadata = parseActionResultMetadata(context, result, execRoot); + // We tell RBE that the input root of the action is the parent directory of what is locally + // the execroot. This is so that paths of artifacts in external repositories don't start with + // an uplevel reference. + Path inputRoot = execRoot.getParentDirectory(); + metadata = parseActionResultMetadata(context, result, inputRoot); } if (!metadata.symlinks().isEmpty()) { @@ -720,14 +727,14 @@ private DirectoryMetadata parseDirectory( } private ActionResultMetadata parseActionResultMetadata( - RemoteActionExecutionContext context, ActionResult actionResult, Path execRoot) + RemoteActionExecutionContext context, ActionResult actionResult, Path inputRoot) throws IOException, InterruptedException { Preconditions.checkNotNull(actionResult, "actionResult"); Map> dirMetadataDownloads = Maps.newHashMapWithExpectedSize(actionResult.getOutputDirectoriesCount()); for (OutputDirectory dir : actionResult.getOutputDirectoriesList()) { dirMetadataDownloads.put( - execRoot.getRelative(dir.getPath()), + inputRoot.getRelative(dir.getPath()), Futures.transform( downloadBlob(context, dir.getTreeDigest()), (treeBytes) -> { @@ -758,9 +765,9 @@ private ActionResultMetadata parseActionResultMetadata( ImmutableMap.Builder files = ImmutableMap.builder(); for (OutputFile outputFile : actionResult.getOutputFilesList()) { files.put( - execRoot.getRelative(outputFile.getPath()), + inputRoot.getRelative(outputFile.getPath()), new FileMetadata( - execRoot.getRelative(outputFile.getPath()), + inputRoot.getRelative(outputFile.getPath()), outputFile.getDigest(), outputFile.getIsExecutable())); } @@ -772,9 +779,9 @@ private ActionResultMetadata parseActionResultMetadata( actionResult.getOutputDirectorySymlinksList()); for (OutputSymlink symlink : outputSymlinks) { symlinks.put( - execRoot.getRelative(symlink.getPath()), + inputRoot.getRelative(symlink.getPath()), new SymlinkMetadata( - execRoot.getRelative(symlink.getPath()), PathFragment.create(symlink.getTarget()))); + inputRoot.getRelative(symlink.getPath()), PathFragment.create(symlink.getTarget()))); } return new ActionResultMetadata(files.build(), symlinks.build(), directories.build()); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index 15273c27f84c65..488105b91e1a88 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -125,7 +125,8 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) Stopwatch totalTime = Stopwatch.createStarted(); - SortedMap inputMap = context.getInputMapping(); + SortedMap inputMap = + context.getInputMapping(PathFragment.create(execRoot.getBaseName())); MerkleTree merkleTree = MerkleTree.build(inputMap, context.getMetadataProvider(), execRoot, digestUtil); SpawnMetrics.Builder spawnMetrics = @@ -143,7 +144,7 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) spawn.getArguments(), spawn.getEnvironment(), platform, - /* workingDirectory= */ null); + execRoot.getBaseName()); RemoteOutputsMode remoteOutputsMode = options.remoteOutputsMode; Action action = RemoteSpawnRunner.buildAction( @@ -290,7 +291,7 @@ public void store(SpawnResult result) throws ExecException, InterruptedException actionKey, action, command, - execRoot, + execRoot.getParentDirectory(), files, context.getFileOutErr()); } catch (IOException e) { diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index c893c8fabbf949..92027b8410e5ff 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java @@ -213,7 +213,14 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) context.report(ProgressStatus.SCHEDULING, getName()); RemoteOutputsMode remoteOutputsMode = remoteOptions.remoteOutputsMode; - SortedMap inputMap = context.getInputMapping(); + // The "root directory" of the action from the point of view of RBE is the parent directory of + // the execroot locally. This is so that paths of artifacts in external repositories don't + // start with an uplevel reference... + SortedMap inputMap = + context.getInputMapping(PathFragment.create(execRoot.getBaseName())); + + // ...however, MerkleTree.build() uses its execRoot parameter to resolve artifacts based on + // ActionInput.getExecPath(), so it needs the execroot and not its parent directory. final MerkleTree merkleTree = MerkleTree.build(inputMap, context.getMetadataProvider(), execRoot, digestUtil); SpawnMetrics.Builder spawnMetrics = @@ -231,7 +238,7 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) spawn.getArguments(), spawn.getEnvironment(), platform, - /* workingDirectory= */ null); + execRoot.getBaseName()); Digest commandHash = digestUtil.compute(command); Action action = buildAction( @@ -719,12 +726,17 @@ static Command buildCommand( List arguments, ImmutableMap env, @Nullable Platform platform, - @Nullable String workingDirectory) { + @Nullable String workingDirectoryString) { Command.Builder command = Command.newBuilder(); ArrayList outputFiles = new ArrayList<>(); ArrayList outputDirectories = new ArrayList<>(); + PathFragment workingDirectoryPathFragment = + workingDirectoryString == null + ? PathFragment.EMPTY_FRAGMENT + : PathFragment.create(workingDirectoryString); for (ActionInput output : outputs) { - String pathString = output.getExecPathString(); + String pathString = + workingDirectoryPathFragment.getRelative(output.getExecPath()).getPathString(); if (output instanceof Artifact && ((Artifact) output).isTreeArtifact()) { outputDirectories.add(pathString); } else { @@ -746,8 +758,8 @@ static Command buildCommand( command.addEnvironmentVariablesBuilder().setName(var).setValue(env.get(var)); } - if (!Strings.isNullOrEmpty(workingDirectory)) { - command.setWorkingDirectory(workingDirectory); + if (!Strings.isNullOrEmpty(workingDirectoryString)) { + command.setWorkingDirectory(workingDirectoryString); } return command.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java index 4439713b42822c..c32638ec8657b8 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java @@ -35,6 +35,7 @@ import com.google.devtools.build.lib.util.OS; 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 java.io.BufferedWriter; import java.io.File; import java.io.IOException; @@ -230,7 +231,10 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( - context.getInputMapping(), spawn, context.getArtifactExpander(), execRoot); + context.getInputMapping(PathFragment.EMPTY_FRAGMENT), + spawn, + context.getArtifactExpander(), + execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); final Path sandboxConfigPath = sandboxPath.getRelative("sandbox.sb"); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java index 32d307a88c4157..13f09e5e116e54 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java @@ -221,7 +221,10 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( - context.getInputMapping(), spawn, context.getArtifactExpander(), execRoot); + context.getInputMapping(PathFragment.EMPTY_FRAGMENT), + spawn, + context.getArtifactExpander(), + execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); Duration timeout = context.getTimeout(); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java index b05045f9f502aa..41f4829f5d9a6d 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.util.OS; 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.Symlinks; import java.io.File; import java.io.IOException; @@ -176,7 +177,10 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( - context.getInputMapping(), spawn, context.getArtifactExpander(), execRoot); + context.getInputMapping(PathFragment.EMPTY_FRAGMENT), + spawn, + context.getArtifactExpander(), + execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); Duration timeout = context.getTimeout(); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java index 00991a57173e3c..dff0d99ef8b8ad 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java @@ -25,6 +25,7 @@ import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; import java.time.Duration; import javax.annotation.Nullable; @@ -109,7 +110,10 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( - context.getInputMapping(), spawn, context.getArtifactExpander(), execRoot); + context.getInputMapping(PathFragment.EMPTY_FRAGMENT), + spawn, + context.getArtifactExpander(), + execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); if (sandboxfsProcess != null) { diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java index 23a134ba715f2f..846090fd0b4d6b 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java @@ -70,7 +70,10 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs readablePaths = helpers.processInputFiles( - context.getInputMapping(), spawn, context.getArtifactExpander(), execRoot); + context.getInputMapping(PathFragment.EMPTY_FRAGMENT), + spawn, + context.getArtifactExpander(), + execRoot); readablePaths.materializeVirtualInputs(execRoot); 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 a810be5a2928f3..d144e476f362dd 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 @@ -180,7 +180,10 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) SandboxInputs inputFiles = helpers.processInputFiles( - context.getInputMapping(), spawn, context.getArtifactExpander(), execRoot); + context.getInputMapping(PathFragment.EMPTY_FRAGMENT), + spawn, + context.getArtifactExpander(), + execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); WorkerProtocolFormat protocolFormat = Spawns.getWorkerProtocolFormat(spawn); diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java index d3cfa3c89a5fdf..385a5363dd62da 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java @@ -76,7 +76,8 @@ public class SpawnInputExpanderTest { public void testEmptyRunfiles() throws Exception { RunfilesSupplier supplier = EmptyRunfilesSupplier.INSTANCE; FakeActionInputFileCache mockCache = new FakeActionInputFileCache(); - expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER); + expander.addRunfilesToInputs( + inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings).isEmpty(); } @@ -94,7 +95,8 @@ public void testRunfilesSingleFile() throws Exception { FileArtifactValue.createForNormalFile( FAKE_DIGEST, /*proxy=*/ null, 0L, /*isShareable=*/ true)); - expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER); + expander.addRunfilesToInputs( + inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings).hasSize(1); assertThat(inputMappings) .containsEntry(PathFragment.create("runfiles/workspace/dir/file"), artifact); @@ -128,7 +130,8 @@ public ImmutableList getFileset(Artifact artifact) { } }; - expander.addRunfilesToInputs(inputMappings, supplier, mockCache, filesetExpander); + expander.addRunfilesToInputs( + inputMappings, supplier, mockCache, filesetExpander, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings).hasSize(1); assertThat(inputMappings) .containsEntry( @@ -152,7 +155,11 @@ public void testRunfilesDirectoryStrict() { IOException.class, () -> expander.addRunfilesToInputs( - inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER)); + inputMappings, + supplier, + mockCache, + NO_ARTIFACT_EXPANDER, + PathFragment.EMPTY_FRAGMENT)); assertThat(expected).hasMessageThat().isEqualTo("Not a file: dir/file"); } @@ -168,7 +175,8 @@ public void testRunfilesDirectoryNonStrict() throws Exception { mockCache.put(artifact, FileArtifactValue.createForDirectoryWithMtime(-1)); expander = new SpawnInputExpander(execRoot, /*strict=*/ false); - expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER); + expander.addRunfilesToInputs( + inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings).hasSize(1); assertThat(inputMappings) .containsEntry(PathFragment.create("runfiles/workspace/dir/file"), artifact); @@ -197,7 +205,8 @@ public void testRunfilesTwoFiles() throws Exception { FileArtifactValue.createForNormalFile( FAKE_DIGEST, /*proxy=*/ null, 12L, /*isShareable=*/ true)); - expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER); + expander.addRunfilesToInputs( + inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings).hasSize(2); assertThat(inputMappings) .containsEntry(PathFragment.create("runfiles/workspace/dir/file"), artifact1); @@ -222,7 +231,8 @@ public void testRunfilesSymlink() throws Exception { FileArtifactValue.createForNormalFile( FAKE_DIGEST, /*proxy=*/ null, 1L, /*isShareable=*/ true)); - expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER); + expander.addRunfilesToInputs( + inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings).hasSize(1); assertThat(inputMappings) .containsEntry(PathFragment.create("runfiles/workspace/symlink"), artifact); @@ -245,7 +255,8 @@ public void testRunfilesRootSymlink() throws Exception { FileArtifactValue.createForNormalFile( FAKE_DIGEST, /*proxy=*/ null, 1L, /*isShareable=*/ true)); - expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER); + expander.addRunfilesToInputs( + inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings).hasSize(2); assertThat(inputMappings).containsEntry(PathFragment.create("runfiles/symlink"), artifact); // If there's no other entry, Runfiles adds an empty file in the workspace to make sure the @@ -276,7 +287,8 @@ public void testRunfilesWithTreeArtifacts() throws Exception { fakeCache.put(file1, FileArtifactValue.createForTesting(file1)); fakeCache.put(file2, FileArtifactValue.createForTesting(file2)); - expander.addRunfilesToInputs(inputMappings, supplier, fakeCache, artifactExpander); + expander.addRunfilesToInputs( + inputMappings, supplier, fakeCache, artifactExpander, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings).hasSize(2); assertThat(inputMappings) .containsEntry(PathFragment.create("runfiles/workspace/treeArtifact/file1"), file1); @@ -308,7 +320,8 @@ public void testRunfilesWithTreeArtifactsInSymlinks() throws Exception { fakeCache.put(file1, FileArtifactValue.createForTesting(file1)); fakeCache.put(file2, FileArtifactValue.createForTesting(file2)); - expander.addRunfilesToInputs(inputMappings, supplier, fakeCache, artifactExpander); + expander.addRunfilesToInputs( + inputMappings, supplier, fakeCache, artifactExpander, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings).hasSize(2); assertThat(inputMappings) .containsEntry(PathFragment.create("runfiles/workspace/symlink/file1"), file1); @@ -336,7 +349,8 @@ public void testTreeArtifactsInInputs() throws Exception { fakeCache.put(file2, FileArtifactValue.createForTesting(file2)); Spawn spawn = new SpawnBuilder("/bin/echo", "Hello World").withInput(treeArtifact).build(); - inputMappings = expander.getInputMapping(spawn, artifactExpander, fakeCache); + inputMappings = + expander.getInputMapping(spawn, artifactExpander, PathFragment.EMPTY_FRAGMENT, fakeCache); assertThat(inputMappings).hasSize(2); assertThat(inputMappings).containsEntry(PathFragment.create("out/treeArtifact/file1"), file1); assertThat(inputMappings).containsEntry(PathFragment.create("out/treeArtifact/file2"), file2); @@ -371,7 +385,7 @@ public void testEmptyManifest() throws Exception { Map> filesetMappings = ImmutableMap.of(createFileset("out"), ImmutableList.of()); - expander.addFilesetManifests(filesetMappings, inputMappings); + expander.addFilesetManifests(filesetMappings, inputMappings, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings).isEmpty(); } @@ -382,7 +396,7 @@ public void testManifestWithSingleFile() throws Exception { ImmutableMap.of( createFileset("out"), ImmutableList.of(filesetSymlink("foo/bar", "/dir/file"))); - expander.addFilesetManifests(filesetMappings, inputMappings); + expander.addFilesetManifests(filesetMappings, inputMappings, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings) .containsExactly( @@ -397,7 +411,7 @@ public void testManifestWithTwoFiles() throws Exception { ImmutableList.of( filesetSymlink("foo/bar", "/dir/file"), filesetSymlink("foo/baz", "/dir/file"))); - expander.addFilesetManifests(filesetMappings, inputMappings); + expander.addFilesetManifests(filesetMappings, inputMappings, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings) .containsExactly( @@ -410,7 +424,7 @@ public void testManifestWithDirectory() throws Exception { Map> filesetMappings = ImmutableMap.of(createFileset("out"), ImmutableList.of(filesetSymlink("foo/bar", "/some"))); - expander.addFilesetManifests(filesetMappings, inputMappings); + expander.addFilesetManifests(filesetMappings, inputMappings, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings) .containsExactly(PathFragment.create("out/foo/bar"), ActionInputHelper.fromPath("/some")); @@ -442,14 +456,17 @@ public void testManifestWithErrorOnRelativeSymlink() throws Exception { IOException e = assertThrows( IOException.class, - () -> expander.addFilesetManifests(simpleFilesetManifest(), inputMappings)); + () -> + expander.addFilesetManifests( + simpleFilesetManifest(), inputMappings, PathFragment.EMPTY_FRAGMENT)); assertThat(e).hasMessageThat().contains("runfiles target is not absolute: foo"); } @Test public void testManifestWithIgnoredRelativeSymlink() throws Exception { expander = new SpawnInputExpander(execRoot, /*strict=*/ true, IGNORE); - expander.addFilesetManifests(simpleFilesetManifest(), inputMappings); + expander.addFilesetManifests( + simpleFilesetManifest(), inputMappings, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings) .containsExactly( PathFragment.create("out/workspace/foo"), ActionInputHelper.fromPath("/root/bar")); @@ -458,7 +475,8 @@ public void testManifestWithIgnoredRelativeSymlink() throws Exception { @Test public void testManifestWithResolvedRelativeSymlink() throws Exception { expander = new SpawnInputExpander(execRoot, /*strict=*/ true, RESOLVE); - expander.addFilesetManifests(simpleFilesetManifest(), inputMappings); + expander.addFilesetManifests( + simpleFilesetManifest(), inputMappings, PathFragment.EMPTY_FRAGMENT); assertThat(inputMappings) .containsExactly( PathFragment.create("out/workspace/bar"), ActionInputHelper.fromPath("/root/bar"), diff --git a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java index e1214cf8a33239..e0324700b44e9f 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java @@ -259,7 +259,7 @@ public FileOutErr getFileOutErr() { } @Override - public SortedMap getInputMapping() { + public SortedMap getInputMapping(PathFragment baseDirectory) { return inputMapping; } diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java index 88586f56dee070..4a05a10775ae2c 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java @@ -146,7 +146,7 @@ public final void setUp() throws Exception { .start(); Chunker.setDefaultChunkSizeForTesting(1000); // Enough for everything to be one chunk. fs = new InMemoryFileSystem(new JavaClock(), DigestHashFunction.SHA256); - execRoot = fs.getPath("/exec/root"); + execRoot = fs.getPath("/execroot/main"); FileSystemUtils.createDirectoryAndParents(execRoot); fakeFileCache = new FakeActionInputFileCache(execRoot); @@ -200,7 +200,7 @@ private GrpcCacheClient newClient(RemoteOptions remoteOptions, Supplier throws IOException { AuthAndTLSOptions authTlsOptions = Options.getDefaults(AuthAndTLSOptions.class); authTlsOptions.useGoogleDefaultCredentials = true; - authTlsOptions.googleCredentials = "/exec/root/creds.json"; + authTlsOptions.googleCredentials = "/execroot/main/creds.json"; authTlsOptions.googleAuthScopes = ImmutableList.of("dummy.scope"); GenericJson json = new GenericJson(); @@ -383,9 +383,9 @@ public void testDownloadAllResults() throws Exception { new FakeImmutableCacheByteStreamImpl(fooDigest, "foo-contents", barDigest, "bar-contents")); ActionResult.Builder result = ActionResult.newBuilder(); - result.addOutputFilesBuilder().setPath("a/foo").setDigest(fooDigest); - result.addOutputFilesBuilder().setPath("b/empty").setDigest(emptyDigest); - result.addOutputFilesBuilder().setPath("a/bar").setDigest(barDigest).setIsExecutable(true); + result.addOutputFilesBuilder().setPath("main/a/foo").setDigest(fooDigest); + result.addOutputFilesBuilder().setPath("main/b/empty").setDigest(emptyDigest); + result.addOutputFilesBuilder().setPath("main/a/bar").setDigest(barDigest).setIsExecutable(true); remoteCache.download( context, result.build(), execRoot, null, /* outputFilesLocker= */ () -> {}); assertThat(DIGEST_UTIL.compute(execRoot.getRelative("a/foo"))).isEqualTo(fooDigest); @@ -421,8 +421,8 @@ public void testDownloadDirectory() throws Exception { quxDigest, "qux-contents"))); ActionResult.Builder result = ActionResult.newBuilder(); - result.addOutputFilesBuilder().setPath("a/foo").setDigest(fooDigest); - result.addOutputDirectoriesBuilder().setPath("a/bar").setTreeDigest(barTreeDigest); + result.addOutputFilesBuilder().setPath("main/a/foo").setDigest(fooDigest); + result.addOutputDirectoriesBuilder().setPath("main/a/bar").setTreeDigest(barTreeDigest); remoteCache.download( context, result.build(), execRoot, null, /* outputFilesLocker= */ () -> {}); @@ -444,7 +444,7 @@ public void testDownloadDirectoryEmpty() throws Exception { ImmutableMap.of(barTreeDigest, barTreeMessage.toByteString()))); ActionResult.Builder result = ActionResult.newBuilder(); - result.addOutputDirectoriesBuilder().setPath("a/bar").setTreeDigest(barTreeDigest); + result.addOutputDirectoriesBuilder().setPath("main/a/bar").setTreeDigest(barTreeDigest); remoteCache.download( context, result.build(), execRoot, null, /* outputFilesLocker= */ () -> {}); @@ -486,8 +486,8 @@ public void testDownloadDirectoryNested() throws Exception { quxDigest, "qux-contents"))); ActionResult.Builder result = ActionResult.newBuilder(); - result.addOutputFilesBuilder().setPath("a/foo").setDigest(fooDigest); - result.addOutputDirectoriesBuilder().setPath("a/bar").setTreeDigest(barTreeDigest); + result.addOutputFilesBuilder().setPath("main/a/foo").setDigest(fooDigest); + result.addOutputDirectoriesBuilder().setPath("main/a/bar").setTreeDigest(barTreeDigest); remoteCache.download( context, result.build(), execRoot, null, /* outputFilesLocker= */ () -> {}); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java index 7cb6b91bce0cbc..1a74c29470a027 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java @@ -118,7 +118,7 @@ public void setUp() throws Exception { "none", "none", Digest.getDefaultInstance().getHash(), null); context = RemoteActionExecutionContext.create(metadata); fs = new InMemoryFileSystem(new JavaClock(), DigestHashFunction.SHA256); - execRoot = fs.getPath("/execroot"); + execRoot = fs.getPath("/execroot/main"); execRoot.createDirectoryAndParents(); fakeFileCache = new FakeActionInputFileCache(execRoot); artifactRoot = ArtifactRoot.asDerivedRoot(execRoot, "outputs"); @@ -135,8 +135,8 @@ public void afterEverything() throws InterruptedException { @Test public void uploadAbsoluteFileSymlinkAsFile() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path link = fs.getPath("/execroot/link"); - Path target = fs.getPath("/execroot/target"); + Path link = fs.getPath("/execroot/main/link"); + Path target = fs.getPath("/execroot/main/target"); FileSystemUtils.writeContent(target, new byte[] {1, 2, 3, 4, 5}); link.createSymbolicLink(target); @@ -155,11 +155,11 @@ public void uploadAbsoluteFileSymlinkAsFile() throws Exception { @Test public void uploadAbsoluteDirectorySymlinkAsDirectory() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path dir = fs.getPath("/execroot/dir"); + Path dir = fs.getPath("/execroot/main/dir"); dir.createDirectory(); - Path foo = fs.getPath("/execroot/dir/foo"); + Path foo = fs.getPath("/execroot/main/dir/foo"); FileSystemUtils.writeContent(foo, new byte[] {1, 2, 3, 4, 5}); - Path link = fs.getPath("/execroot/link"); + Path link = fs.getPath("/execroot/main/link"); link.createSymbolicLink(dir); UploadManifest um = @@ -167,7 +167,7 @@ public void uploadAbsoluteDirectorySymlinkAsDirectory() throws Exception { digestUtil, result, execRoot, /*uploadSymlinks=*/ true, /*allowSymlinks=*/ true); um.addFiles(ImmutableList.of(link)); Digest digest = digestUtil.compute(foo); - assertThat(um.getDigestToFile()).containsExactly(digest, fs.getPath("/execroot/link/foo")); + assertThat(um.getDigestToFile()).containsExactly(digest, fs.getPath("/execroot/main/link/foo")); Tree tree = Tree.newBuilder() @@ -185,8 +185,8 @@ public void uploadAbsoluteDirectorySymlinkAsDirectory() throws Exception { @Test public void uploadRelativeFileSymlinkAsFile() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path link = fs.getPath("/execroot/link"); - Path target = fs.getPath("/execroot/target"); + Path link = fs.getPath("/execroot/main/link"); + Path target = fs.getPath("/execroot/main/target"); FileSystemUtils.writeContent(target, new byte[] {1, 2, 3, 4, 5}); link.createSymbolicLink(target.relativeTo(execRoot)); @@ -205,11 +205,11 @@ public void uploadRelativeFileSymlinkAsFile() throws Exception { @Test public void uploadRelativeDirectorySymlinkAsDirectory() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path dir = fs.getPath("/execroot/dir"); + Path dir = fs.getPath("/execroot/main/dir"); dir.createDirectory(); - Path foo = fs.getPath("/execroot/dir/foo"); + Path foo = fs.getPath("/execroot/main/dir/foo"); FileSystemUtils.writeContent(foo, new byte[] {1, 2, 3, 4, 5}); - Path link = fs.getPath("/execroot/link"); + Path link = fs.getPath("/execroot/main/link"); link.createSymbolicLink(dir.relativeTo(execRoot)); UploadManifest um = @@ -217,7 +217,7 @@ public void uploadRelativeDirectorySymlinkAsDirectory() throws Exception { digestUtil, result, execRoot, /*uploadSymlinks=*/ false, /*allowSymlinks=*/ true); um.addFiles(ImmutableList.of(link)); Digest digest = digestUtil.compute(foo); - assertThat(um.getDigestToFile()).containsExactly(digest, fs.getPath("/execroot/link/foo")); + assertThat(um.getDigestToFile()).containsExactly(digest, fs.getPath("/execroot/main/link/foo")); Tree tree = Tree.newBuilder() @@ -235,8 +235,8 @@ public void uploadRelativeDirectorySymlinkAsDirectory() throws Exception { @Test public void uploadRelativeFileSymlink() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path link = fs.getPath("/execroot/link"); - Path target = fs.getPath("/execroot/target"); + Path link = fs.getPath("/execroot/main/link"); + Path target = fs.getPath("/execroot/main/target"); FileSystemUtils.writeContent(target, new byte[] {1, 2, 3, 4, 5}); link.createSymbolicLink(target.relativeTo(execRoot)); @@ -254,11 +254,11 @@ public void uploadRelativeFileSymlink() throws Exception { @Test public void uploadRelativeDirectorySymlink() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path dir = fs.getPath("/execroot/dir"); + Path dir = fs.getPath("/execroot/main/dir"); dir.createDirectory(); - Path file = fs.getPath("/execroot/dir/foo"); + Path file = fs.getPath("/execroot/main/dir/foo"); FileSystemUtils.writeContent(file, new byte[] {1, 2, 3, 4, 5}); - Path link = fs.getPath("/execroot/link"); + Path link = fs.getPath("/execroot/main/link"); link.createSymbolicLink(dir.relativeTo(execRoot)); UploadManifest um = @@ -275,8 +275,8 @@ public void uploadRelativeDirectorySymlink() throws Exception { @Test public void uploadDanglingSymlinkError() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path link = fs.getPath("/execroot/link"); - Path target = fs.getPath("/execroot/target"); + Path link = fs.getPath("/execroot/main/link"); + Path target = fs.getPath("/execroot/main/target"); link.createSymbolicLink(target.relativeTo(execRoot)); UploadManifest um = @@ -284,15 +284,15 @@ public void uploadDanglingSymlinkError() throws Exception { digestUtil, result, execRoot, /*uploadSymlinks=*/ true, /*allowSymlinks=*/ true); IOException e = assertThrows(IOException.class, () -> um.addFiles(ImmutableList.of(link))); assertThat(e).hasMessageThat().contains("dangling"); - assertThat(e).hasMessageThat().contains("/execroot/link"); + assertThat(e).hasMessageThat().contains("/execroot/main/link"); assertThat(e).hasMessageThat().contains("target"); } @Test public void uploadSymlinksNoAllowError() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path link = fs.getPath("/execroot/link"); - Path target = fs.getPath("/execroot/target"); + Path link = fs.getPath("/execroot/main/link"); + Path target = fs.getPath("/execroot/main/target"); FileSystemUtils.writeContent(target, new byte[] {1, 2, 3, 4, 5}); link.createSymbolicLink(target.relativeTo(execRoot)); @@ -307,11 +307,11 @@ public void uploadSymlinksNoAllowError() throws Exception { @Test public void uploadAbsoluteFileSymlinkInDirectoryAsFile() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path dir = fs.getPath("/execroot/dir"); + Path dir = fs.getPath("/execroot/main/dir"); dir.createDirectory(); - Path target = fs.getPath("/execroot/target"); + Path target = fs.getPath("/execroot/main/target"); FileSystemUtils.writeContent(target, new byte[] {1, 2, 3, 4, 5}); - Path link = fs.getPath("/execroot/dir/link"); + Path link = fs.getPath("/execroot/main/dir/link"); link.createSymbolicLink(target); UploadManifest um = @@ -337,13 +337,13 @@ public void uploadAbsoluteFileSymlinkInDirectoryAsFile() throws Exception { @Test public void uploadAbsoluteDirectorySymlinkInDirectoryAsDirectory() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path dir = fs.getPath("/execroot/dir"); + Path dir = fs.getPath("/execroot/main/dir"); dir.createDirectory(); - Path bardir = fs.getPath("/execroot/bardir"); + Path bardir = fs.getPath("/execroot/main/bardir"); bardir.createDirectory(); - Path foo = fs.getPath("/execroot/bardir/foo"); + Path foo = fs.getPath("/execroot/main/bardir/foo"); FileSystemUtils.writeContent(foo, new byte[] {1, 2, 3, 4, 5}); - Path link = fs.getPath("/execroot/dir/link"); + Path link = fs.getPath("/execroot/main/dir/link"); link.createSymbolicLink(bardir); UploadManifest um = @@ -351,7 +351,8 @@ public void uploadAbsoluteDirectorySymlinkInDirectoryAsDirectory() throws Except digestUtil, result, execRoot, /*uploadSymlinks=*/ true, /*allowSymlinks=*/ true); um.addFiles(ImmutableList.of(dir)); Digest digest = digestUtil.compute(foo); - assertThat(um.getDigestToFile()).containsExactly(digest, fs.getPath("/execroot/dir/link/foo")); + assertThat(um.getDigestToFile()) + .containsExactly(digest, fs.getPath("/execroot/main/dir/link/foo")); Directory barDir = Directory.newBuilder() @@ -376,11 +377,11 @@ public void uploadAbsoluteDirectorySymlinkInDirectoryAsDirectory() throws Except @Test public void uploadRelativeFileSymlinkInDirectoryAsFile() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path dir = fs.getPath("/execroot/dir"); + Path dir = fs.getPath("/execroot/main/dir"); dir.createDirectory(); - Path target = fs.getPath("/execroot/target"); + Path target = fs.getPath("/execroot/main/target"); FileSystemUtils.writeContent(target, new byte[] {1, 2, 3, 4, 5}); - Path link = fs.getPath("/execroot/dir/link"); + Path link = fs.getPath("/execroot/main/dir/link"); link.createSymbolicLink(PathFragment.create("../target")); UploadManifest um = @@ -406,13 +407,13 @@ public void uploadRelativeFileSymlinkInDirectoryAsFile() throws Exception { @Test public void uploadRelativeDirectorySymlinkInDirectoryAsDirectory() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path dir = fs.getPath("/execroot/dir"); + Path dir = fs.getPath("/execroot/main/dir"); dir.createDirectory(); - Path bardir = fs.getPath("/execroot/bardir"); + Path bardir = fs.getPath("/execroot/main/bardir"); bardir.createDirectory(); - Path foo = fs.getPath("/execroot/bardir/foo"); + Path foo = fs.getPath("/execroot/main/bardir/foo"); FileSystemUtils.writeContent(foo, new byte[] {1, 2, 3, 4, 5}); - Path link = fs.getPath("/execroot/dir/link"); + Path link = fs.getPath("/execroot/main/dir/link"); link.createSymbolicLink(PathFragment.create("../bardir")); UploadManifest um = @@ -420,7 +421,8 @@ public void uploadRelativeDirectorySymlinkInDirectoryAsDirectory() throws Except digestUtil, result, execRoot, /*uploadSymlinks=*/ false, /*allowSymlinks=*/ true); um.addFiles(ImmutableList.of(dir)); Digest digest = digestUtil.compute(foo); - assertThat(um.getDigestToFile()).containsExactly(digest, fs.getPath("/execroot/dir/link/foo")); + assertThat(um.getDigestToFile()) + .containsExactly(digest, fs.getPath("/execroot/main/dir/link/foo")); Directory barDir = Directory.newBuilder() @@ -445,11 +447,11 @@ public void uploadRelativeDirectorySymlinkInDirectoryAsDirectory() throws Except @Test public void uploadRelativeFileSymlinkInDirectory() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path dir = fs.getPath("/execroot/dir"); + Path dir = fs.getPath("/execroot/main/dir"); dir.createDirectory(); - Path target = fs.getPath("/execroot/target"); + Path target = fs.getPath("/execroot/main/target"); FileSystemUtils.writeContent(target, new byte[] {1, 2, 3, 4, 5}); - Path link = fs.getPath("/execroot/dir/link"); + Path link = fs.getPath("/execroot/main/dir/link"); link.createSymbolicLink(PathFragment.create("../target")); UploadManifest um = @@ -474,13 +476,13 @@ public void uploadRelativeFileSymlinkInDirectory() throws Exception { @Test public void uploadRelativeDirectorySymlinkInDirectory() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path dir = fs.getPath("/execroot/dir"); + Path dir = fs.getPath("/execroot/main/dir"); dir.createDirectory(); - Path bardir = fs.getPath("/execroot/bardir"); + Path bardir = fs.getPath("/execroot/main/bardir"); bardir.createDirectory(); - Path foo = fs.getPath("/execroot/bardir/foo"); + Path foo = fs.getPath("/execroot/main/bardir/foo"); FileSystemUtils.writeContent(foo, new byte[] {1, 2, 3, 4, 5}); - Path link = fs.getPath("/execroot/dir/link"); + Path link = fs.getPath("/execroot/main/dir/link"); link.createSymbolicLink(PathFragment.create("../bardir")); UploadManifest um = @@ -523,11 +525,11 @@ public void uploadDanglingSymlinkInDirectoryError() throws Exception { @Test public void uploadSymlinkInDirectoryNoAllowError() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); - Path dir = fs.getPath("/execroot/dir"); + Path dir = fs.getPath("/execroot/main/dir"); dir.createDirectory(); - Path target = fs.getPath("/execroot/target"); + Path target = fs.getPath("/execroot/main/target"); FileSystemUtils.writeContent(target, new byte[] {1, 2, 3, 4, 5}); - Path link = fs.getPath("/execroot/dir/link"); + Path link = fs.getPath("/execroot/main/dir/link"); link.createSymbolicLink(target); UploadManifest um = @@ -543,7 +545,7 @@ public void uploadSymlinkInDirectoryNoAllowError() throws Exception { public void downloadRelativeFileSymlink() throws Exception { RemoteCache cache = newRemoteCache(); ActionResult.Builder result = ActionResult.newBuilder(); - result.addOutputFileSymlinksBuilder().setPath("a/b/link").setTarget("../../foo"); + result.addOutputFileSymlinksBuilder().setPath("main/a/b/link").setTarget("../../foo"); // Doesn't check for dangling links, hence download succeeds. cache.download(context, result.build(), execRoot, null, outputFilesLocker); Path path = execRoot.getRelative("a/b/link"); @@ -556,7 +558,7 @@ public void downloadRelativeFileSymlink() throws Exception { public void downloadRelativeDirectorySymlink() throws Exception { RemoteCache cache = newRemoteCache(); ActionResult.Builder result = ActionResult.newBuilder(); - result.addOutputDirectorySymlinksBuilder().setPath("a/b/link").setTarget("foo"); + result.addOutputDirectorySymlinksBuilder().setPath("main/a/b/link").setTarget("foo"); // Doesn't check for dangling links, hence download succeeds. cache.download(context, result.build(), execRoot, null, outputFilesLocker); Path path = execRoot.getRelative("a/b/link"); @@ -576,7 +578,7 @@ public void downloadRelativeSymlinkInDirectory() throws Exception { .build(); Digest treeDigest = cache.addContents(context, tree.toByteArray()); ActionResult.Builder result = ActionResult.newBuilder(); - result.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest); + result.addOutputDirectoriesBuilder().setPath("main/dir").setTreeDigest(treeDigest); // Doesn't check for dangling links, hence download succeeds. cache.download(context, result.build(), execRoot, null, outputFilesLocker); Path path = execRoot.getRelative("dir/link"); @@ -646,10 +648,11 @@ public void downloadFailureMaintainsDirectories() throws Exception { Digest otherFileDigest = cache.addContents(context, "otherfile"); ActionResult.Builder result = ActionResult.newBuilder(); - result.addOutputDirectoriesBuilder().setPath("outputdir").setTreeDigest(treeDigest); + result.addOutputDirectoriesBuilder().setPath("main/outputdir").setTreeDigest(treeDigest); + result.addOutputFiles( + OutputFile.newBuilder().setPath("main/outputdir/outputfile").setDigest(outputFileDigest)); result.addOutputFiles( - OutputFile.newBuilder().setPath("outputdir/outputfile").setDigest(outputFileDigest)); - result.addOutputFiles(OutputFile.newBuilder().setPath("otherfile").setDigest(otherFileDigest)); + OutputFile.newBuilder().setPath("main/otherfile").setDigest(otherFileDigest)); assertThrows( BulkTransferException.class, () -> cache.download(context, result.build(), execRoot, null, outputFilesLocker)); @@ -882,8 +885,8 @@ public void testDownloadClashes() throws Exception { ActionResult r = ActionResult.newBuilder() .setExitCode(0) - .addOutputFiles(OutputFile.newBuilder().setPath("outputs/foo.tmp").setDigest(d1)) - .addOutputFiles(OutputFile.newBuilder().setPath("outputs/foo").setDigest(d2)) + .addOutputFiles(OutputFile.newBuilder().setPath("main/outputs/foo.tmp").setDigest(d1)) + .addOutputFiles(OutputFile.newBuilder().setPath("main/outputs/foo").setDigest(d2)) .build(); Artifact a1 = ActionsTestUtil.createArtifact(artifactRoot, "foo.tmp"); @@ -913,8 +916,8 @@ public void testDownloadMinimalFiles() throws Exception { ActionResult r = ActionResult.newBuilder() .setExitCode(0) - .addOutputFiles(OutputFile.newBuilder().setPath("outputs/file1").setDigest(d1)) - .addOutputFiles(OutputFile.newBuilder().setPath("outputs/file2").setDigest(d2)) + .addOutputFiles(OutputFile.newBuilder().setPath("main/outputs/file1").setDigest(d1)) + .addOutputFiles(OutputFile.newBuilder().setPath("main/outputs/file2").setDigest(d2)) .build(); Artifact a1 = ActionsTestUtil.createArtifact(artifactRoot, "file1"); @@ -972,7 +975,7 @@ public void testDownloadMinimalDirectory() throws Exception { ActionResult.newBuilder() .setExitCode(0) .addOutputDirectories( - OutputDirectory.newBuilder().setPath("outputs/dir").setTreeDigest(dt)) + OutputDirectory.newBuilder().setPath("main/outputs/dir").setTreeDigest(dt)) .build(); SpecialArtifact dir = @@ -1131,8 +1134,8 @@ public void testDownloadMinimalWithInMemoryOutput() throws Exception { ActionResult r = ActionResult.newBuilder() .setExitCode(0) - .addOutputFiles(OutputFile.newBuilder().setPath("outputs/file1").setDigest(d1)) - .addOutputFiles(OutputFile.newBuilder().setPath("outputs/file2").setDigest(d2)) + .addOutputFiles(OutputFile.newBuilder().setPath("main/outputs/file1").setDigest(d1)) + .addOutputFiles(OutputFile.newBuilder().setPath("main/outputs/file2").setDigest(d2)) .build(); Artifact a1 = ActionsTestUtil.createArtifact(artifactRoot, "file1"); Artifact a2 = ActionsTestUtil.createArtifact(artifactRoot, "file2"); @@ -1274,8 +1277,8 @@ public void testDownloadDirectory() throws Exception { cas.put(barTreeDigest, barTreeMessage.toByteArray()); ActionResult.Builder result = ActionResult.newBuilder(); - result.addOutputFilesBuilder().setPath("a/foo").setDigest(fooDigest); - result.addOutputDirectoriesBuilder().setPath("a/bar").setTreeDigest(barTreeDigest); + result.addOutputFilesBuilder().setPath("main/a/foo").setDigest(fooDigest); + result.addOutputDirectoriesBuilder().setPath("main/a/bar").setTreeDigest(barTreeDigest); // act RemoteCache remoteCache = newRemoteCache(cas); @@ -1300,7 +1303,7 @@ public void testDownloadEmptyDirectory() throws Exception { map.put(barTreeDigest, barTreeMessage.toByteArray()); ActionResult.Builder result = ActionResult.newBuilder(); - result.addOutputDirectoriesBuilder().setPath("a/bar").setTreeDigest(barTreeDigest); + result.addOutputDirectoriesBuilder().setPath("main/a/bar").setTreeDigest(barTreeDigest); // act RemoteCache remoteCache = newRemoteCache(map); @@ -1344,8 +1347,8 @@ public void testDownloadNestedDirectory() throws Exception { map.put(quxDigest, "qux-contents".getBytes(UTF_8)); ActionResult.Builder result = ActionResult.newBuilder(); - result.addOutputFilesBuilder().setPath("a/foo").setDigest(fooDigest); - result.addOutputDirectoriesBuilder().setPath("a/bar").setTreeDigest(barTreeDigest); + result.addOutputFilesBuilder().setPath("main/a/foo").setDigest(fooDigest); + result.addOutputDirectoriesBuilder().setPath("main/a/bar").setTreeDigest(barTreeDigest); // act RemoteCache remoteCache = newRemoteCache(map); @@ -1396,7 +1399,7 @@ public void testDownloadDirectoryWithSameHash() throws Exception { map.put(treeDigest, tree.toByteArray()); ActionResult.Builder result = ActionResult.newBuilder(); - result.addOutputDirectoriesBuilder().setPath("a/").setTreeDigest(treeDigest); + result.addOutputDirectoriesBuilder().setPath("main/a/").setTreeDigest(treeDigest); // act RemoteCache remoteCache = newRemoteCache(map); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java index cff754e1686999..6962424956faa7 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java @@ -155,9 +155,10 @@ public FileOutErr getFileOutErr() { } @Override - public SortedMap getInputMapping() throws IOException { + public SortedMap getInputMapping(PathFragment baseDirectory) + throws IOException { return new SpawnInputExpander(execRoot, /*strict*/ false) - .getInputMapping(simpleSpawn, SIMPLE_ARTIFACT_EXPANDER, fakeFileCache); + .getInputMapping(simpleSpawn, SIMPLE_ARTIFACT_EXPANDER, baseDirectory, fakeFileCache); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java index 39a82f20ffaa84..9d81fe8650c52f 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java @@ -144,7 +144,7 @@ public class RemoteSpawnRunnerTest { // The action key of the Spawn returned by newSimpleSpawn(). private final String simpleActionId = - "eb45b20cc979d504f96b9efc9a08c48103c6f017afa09c0df5c70a5f92a98ea8"; + "b9a727771337fd8ce54821f4805e2d451c4739e92fec6f8ecdb18ff9d1983b27"; @Before public final void setUp() throws Exception { diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java index cde9ee666d6150..ebb1f37c4b4b87 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java @@ -181,7 +181,7 @@ public final void setUp() throws Exception { Chunker.setDefaultChunkSizeForTesting(1000); // Enough for everything to be one chunk. fs = new InMemoryFileSystem(new JavaClock(), DigestHashFunction.SHA256); - execRoot = fs.getPath("/exec/root"); + execRoot = fs.getPath("/execroot/main"); logDir = fs.getPath("/server-logs"); FileSystemUtils.createDirectoryAndParents(execRoot); fakeFileCache = new FakeActionInputFileCache(execRoot); @@ -207,7 +207,7 @@ public boolean isSymlink() { @Override public PathFragment getExecPath() { - return null; // unused here. + return PathFragment.create("foo"); } }, new ActionInput() { @@ -223,7 +223,7 @@ public boolean isSymlink() { @Override public PathFragment getExecPath() { - return null; // unused here. + return PathFragment.create("bar"); } }), ResourceSet.ZERO); @@ -321,7 +321,8 @@ public int maxConcurrency() { .setName("VARIABLE") .setValue("value") .build()) - .addAllOutputFiles(ImmutableList.of("bar", "foo")) + .addAllOutputFiles(ImmutableList.of("main/bar", "main/foo")) + .setWorkingDirectory("main") .build(); cmdDigest = DIGEST_UTIL.compute(command); channel.release(); diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java b/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java index f4610cfc6da08f..28d70a4dd6c7ab 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java +++ b/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java @@ -112,9 +112,10 @@ public FileOutErr getFileOutErr() { } @Override - public SortedMap getInputMapping() throws IOException { + public SortedMap getInputMapping(PathFragment baseDirectory) + throws IOException { return new SpawnInputExpander(execRoot, /*strict*/ false) - .getInputMapping(spawn, this::artifactExpander, metadataProvider); + .getInputMapping(spawn, this::artifactExpander, baseDirectory, metadataProvider); } @Override diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java index c3eaa4f1e49c90..2830e52b4d74a0 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java @@ -283,9 +283,13 @@ private ActionResult execute( outputs.add(file); } + Path workingDirectory = execRoot.getRelative(command.getWorkingDirectory()); + FileSystemUtils.createDirectoryAndParents(workingDirectory); + // TODO(ulfjack): This is basically a copy of LocalSpawnRunner. Ideally, we'd use that // implementation instead of copying it. - com.google.devtools.build.lib.shell.Command cmd = getCommand(command, execRoot.getPathString()); + com.google.devtools.build.lib.shell.Command cmd = + getCommand(command, workingDirectory.getPathString()); long startTime = System.currentTimeMillis(); CommandResult cmdResult = null;