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 4be5e977c4fa17..3848b4291a0953 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 @@ -124,7 +124,7 @@ public List exec( spawnLogContext.logSpawn( spawn, actionExecutionContext.getMetadataProvider(), - context.getInputMapping(), + context.getInputMapping(true), context.getTimeout(), spawnResult); } catch (IOException e) { @@ -184,10 +184,9 @@ public int getId() { @Override public void prefetchInputs() throws IOException { if (Spawns.shouldPrefetchInputsForLocalExecution(spawn)) { - // TODO(philwo): Benchmark whether using an ExecutionService to do multiple operations in - // parallel speeds up prefetching of inputs. - // TODO(philwo): Do we have to expand middleman artifacts here? - actionExecutionContext.getActionInputPrefetcher().prefetchFiles(getInputMapping().values()); + actionExecutionContext + .getActionInputPrefetcher() + .prefetchFiles(getInputMapping(true).values()); } } @@ -232,13 +231,15 @@ public FileOutErr getFileOutErr() { } @Override - public SortedMap getInputMapping() throws IOException { + public SortedMap getInputMapping( + boolean expandTreeArtifactsInRunfiles) throws IOException { if (lazyInputMapping == null) { lazyInputMapping = spawnInputExpander.getInputMapping( spawn, actionExecutionContext.getArtifactExpander(), - actionExecutionContext.getMetadataProvider()); + actionExecutionContext.getMetadataProvider(), + expandTreeArtifactsInRunfiles); } 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 0e965b32dd3b77..a2a875cf90b801 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 @@ -106,7 +106,8 @@ void addRunfilesToInputs( Map inputMap, RunfilesSupplier runfilesSupplier, MetadataProvider actionFileCache, - ArtifactExpander artifactExpander) + ArtifactExpander artifactExpander, + boolean expandTreeArtifactsInRunfiles) throws IOException { Map> rootsAndMappings = runfilesSupplier.getMappings(); @@ -120,7 +121,7 @@ void addRunfilesToInputs( Artifact localArtifact = mapping.getValue(); if (localArtifact != null) { Preconditions.checkState(!localArtifact.isMiddlemanArtifact()); - if (localArtifact.isTreeArtifact()) { + if (expandTreeArtifactsInRunfiles && localArtifact.isTreeArtifact()) { List expandedInputs = ActionInputHelper.expandArtifacts( Collections.singletonList(localArtifact), artifactExpander); @@ -200,8 +201,8 @@ private void addInputs( /** * Convert the inputs and runfiles of the given spawn to a map from exec-root relative paths to - * {@link ActionInput}s. The returned map does not contain tree artifacts as they are expanded - * to file artifacts. + * {@link ActionInput}s. The returned map does not contain tree artifacts as they are expanded to + * file artifacts. * *

The returned map never contains {@code null} values; it uses {@link #EMPTY_FILE} for empty * files, which is an instance of {@link @@ -210,12 +211,19 @@ 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, + MetadataProvider actionInputFileCache, + boolean expandTreeArtifactsInRunfiles) throws IOException { TreeMap inputMap = new TreeMap<>(); addInputs(inputMap, spawn, artifactExpander); addRunfilesToInputs( - inputMap, spawn.getRunfilesSupplier(), actionInputFileCache, artifactExpander); + inputMap, + spawn.getRunfilesSupplier(), + actionInputFileCache, + artifactExpander, + expandTreeArtifactsInRunfiles); addFilesetManifests(spawn.getFilesetMappings(), inputMap); 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 2fcfb05817c1ff..7a3c6c2b7f35c4 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 @@ -193,7 +193,8 @@ default ArtifactPathResolver getPathResolver() { /** The files to which to write stdout and stderr. */ FileOutErr getFileOutErr(); - SortedMap getInputMapping() throws IOException; + SortedMap getInputMapping(boolean expandTreeArtifactsInRunfiles) + 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/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index 619a67b2942c73..410ca80e67fc37 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 @@ -97,7 +97,7 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) // Temporary hack: the TreeNodeRepository should be created and maintained upstream! TreeNodeRepository repository = new TreeNodeRepository(execRoot, context.getMetadataProvider(), digestUtil); - SortedMap inputMap = context.getInputMapping(); + SortedMap inputMap = context.getInputMapping(true); TreeNode inputRoot = repository.buildFromActionInputs(inputMap); repository.computeMerkleDigests(inputRoot); Command command = 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 18eb6bb8eaf290..d5bb71671dfeaa 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 @@ -143,7 +143,7 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) // Temporary hack: the TreeNodeRepository should be created and maintained upstream! MetadataProvider inputFileCache = context.getMetadataProvider(); TreeNodeRepository repository = new TreeNodeRepository(execRoot, inputFileCache, digestUtil); - SortedMap inputMap = context.getInputMapping(); + SortedMap inputMap = context.getInputMapping(true); TreeNode inputRoot = repository.buildFromActionInputs(inputMap); repository.computeMerkleDigests(inputRoot); maybeWriteParamFilesLocally(spawn); 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 6da13d34520269..e1b44433d29278 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 @@ -259,7 +259,12 @@ protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) allowNetwork || Spawns.requiresNetwork(spawn, getSandboxOptions().defaultSandboxAllowNetwork); - Map inputs = SandboxHelpers.processInputFiles(spawn, context, execRoot); + Map inputs = + SandboxHelpers.processInputFiles( + spawn, + context, + execRoot, + getSandboxOptions().symlinkedSandboxExpandsTreeArtifactsInRunfilesTree); SandboxedSpawn sandbox; if (sandboxfsProcess != null) { 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 9b2f468066fa03..17dcd038228ed5 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 @@ -263,7 +263,11 @@ protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) sandboxExecRoot, cmdLine.build(), cmdEnv.getClientEnv(), - SandboxHelpers.processInputFiles(spawn, context, execRoot), + SandboxHelpers.processInputFiles( + spawn, + context, + execRoot, + getSandboxOptions().symlinkedSandboxExpandsTreeArtifactsInRunfilesTree), outputs, ImmutableSet.of()); 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 5e1339d255cc31..6d6255260fd0bf 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 @@ -190,7 +190,11 @@ spawn, getSandboxOptions().defaultSandboxAllowNetwork))) sandboxPath, commandLineBuilder.build(), environment, - SandboxHelpers.processInputFiles(spawn, context, execRoot), + SandboxHelpers.processInputFiles( + spawn, + context, + execRoot, + getSandboxOptions().symlinkedSandboxExpandsTreeArtifactsInRunfilesTree), outputs, ImmutableSet.of()); } else { @@ -200,7 +204,11 @@ spawn, getSandboxOptions().defaultSandboxAllowNetwork))) sandboxExecRoot, commandLineBuilder.build(), environment, - SandboxHelpers.processInputFiles(spawn, context, execRoot), + SandboxHelpers.processInputFiles( + spawn, + context, + execRoot, + getSandboxOptions().symlinkedSandboxExpandsTreeArtifactsInRunfilesTree), outputs, writableDirs); } 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 af5444fd90f8d0..be864c03e36a20 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 @@ -101,7 +101,11 @@ protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) sandboxExecRoot, commandLineBuilder.build(), environment, - SandboxHelpers.processInputFiles(spawn, context, execRoot), + SandboxHelpers.processInputFiles( + spawn, + context, + execRoot, + getSandboxOptions().symlinkedSandboxExpandsTreeArtifactsInRunfilesTree), SandboxHelpers.getOutputFiles(spawn), getWritableDirs(sandboxExecRoot, environment)); 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 ee6a959398c88d..289e2308642074 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 @@ -48,9 +48,16 @@ public final class SandboxHelpers { * @throws IOException If any files could not be written. */ public static Map processInputFiles( - Spawn spawn, SpawnExecutionContext context, Path execRoot) throws IOException { + Spawn spawn, + SpawnExecutionContext context, + Path execRoot, + boolean expandTreeArtifactsInRunfiles) + throws IOException { return processInputFiles( - context.getInputMapping(), spawn, context.getArtifactExpander(), execRoot); + context.getInputMapping(expandTreeArtifactsInRunfiles), + spawn, + context.getArtifactExpander(), + execRoot); } /** diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java index b22095dbfc59ff..d228a36a1faa01 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java @@ -24,6 +24,7 @@ import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; +import com.google.devtools.common.options.OptionMetadataTag; import com.google.devtools.common.options.OptionsBase; import com.google.devtools.common.options.OptionsParsingException; import java.io.IOException; @@ -281,4 +282,18 @@ public ImmutableSet getInaccessiblePaths(FileSystem fs) { effectTags = {OptionEffectTag.UNKNOWN}, help = "Allow network access by default for actions.") public boolean defaultSandboxAllowNetwork; + + @Option( + name = "incompatible_symlinked_sandbox_expands_tree_artifacts_in_runfiles_tree", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY, + effectTags = {OptionEffectTag.EXECUTION}, + metadataTags = { + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES, + OptionMetadataTag.INCOMPATIBLE_CHANGE + }, + help = + "If enabled, the sandbox will expand tree artifacts in runfiles, thus the files that " + + "are contained in the tree artifact will be symlinked as individual files.") + public boolean symlinkedSandboxExpandsTreeArtifactsInRunfilesTree; } diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java index 2dce384801391f..46bb0dc554547c 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.runtime.Command; import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.runtime.commands.CleanCommand.CleanStartingEvent; +import com.google.devtools.build.lib.sandbox.SandboxOptions; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.common.options.OptionsBase; @@ -145,7 +146,10 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB workerPool, extraFlags, env.getReporter(), - createFallbackRunner(env)); + createFallbackRunner(env), + env.getOptions() + .getOptions(SandboxOptions.class) + .symlinkedSandboxExpandsTreeArtifactsInRunfilesTree); builder.addActionContext(new WorkerSpawnStrategy(env.getExecRoot(), spawnRunner)); builder.addStrategyByContext(SpawnActionContext.class, "standalone"); 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 958b38f68adc97..e3af3f41a33fd4 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 @@ -76,18 +76,21 @@ final class WorkerSpawnRunner implements SpawnRunner { private final Multimap extraFlags; private final EventHandler reporter; private final SpawnRunner fallbackRunner; + private final boolean sandboxUsesExpandedTreeArtifactsInRunfiles; public WorkerSpawnRunner( Path execRoot, WorkerPool workers, Multimap extraFlags, EventHandler reporter, - SpawnRunner fallbackRunner) { + SpawnRunner fallbackRunner, + boolean sandboxUsesExpandedTreeArtifactsInRunfiles) { this.execRoot = execRoot; this.workers = Preconditions.checkNotNull(workers); this.extraFlags = extraFlags; this.reporter = reporter; this.fallbackRunner = fallbackRunner; + this.sandboxUsesExpandedTreeArtifactsInRunfiles = sandboxUsesExpandedTreeArtifactsInRunfiles; } @Override @@ -135,7 +138,9 @@ private SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) HashCode workerFilesCombinedHash = WorkerFilesHash.getCombinedHash(workerFiles); - Map inputFiles = SandboxHelpers.processInputFiles(spawn, context, execRoot); + Map inputFiles = + SandboxHelpers.processInputFiles( + spawn, context, execRoot, sandboxUsesExpandedTreeArtifactsInRunfiles); Set outputFiles = SandboxHelpers.getOutputFiles(spawn); WorkerKey key = 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 201da354368651..99ed136cb78733 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 @@ -88,7 +88,7 @@ private void scratchFile(String file, String... lines) throws Exception { 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, true); assertThat(inputMappings).isEmpty(); } @@ -103,7 +103,7 @@ public void testRunfilesSingleFile() throws Exception { FakeActionInputFileCache mockCache = new FakeActionInputFileCache(); mockCache.put(artifact, FileArtifactValue.createNormalFile(FAKE_DIGEST, 0)); - expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER); + expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, true); assertThat(inputMappings).hasSize(1); assertThat(inputMappings) .containsEntry(PathFragment.create("runfiles/workspace/dir/file"), artifact); @@ -121,7 +121,7 @@ public void testRunfilesDirectoryStrict() { mockCache.put(artifact, FileArtifactValue.createDirectory(-1)); try { - expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER); + expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, true); fail(); } catch (IOException expected) { assertThat(expected).hasMessageThat().isEqualTo("Not a file: dir/file"); @@ -140,7 +140,7 @@ public void testRunfilesDirectoryNonStrict() throws Exception { mockCache.put(artifact, FileArtifactValue.createDirectory(-1)); expander = new SpawnInputExpander(execRoot, /*strict=*/ false); - expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER); + expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, true); assertThat(inputMappings).hasSize(1); assertThat(inputMappings) .containsEntry(PathFragment.create("runfiles/workspace/dir/file"), artifact); @@ -163,7 +163,7 @@ public void testRunfilesTwoFiles() throws Exception { mockCache.put(artifact1, FileArtifactValue.createNormalFile(FAKE_DIGEST, 1)); mockCache.put(artifact2, FileArtifactValue.createNormalFile(FAKE_DIGEST, 2)); - expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER); + expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, true); assertThat(inputMappings).hasSize(2); assertThat(inputMappings) .containsEntry(PathFragment.create("runfiles/workspace/dir/file"), artifact1); @@ -185,7 +185,7 @@ public void testRunfilesSymlink() throws Exception { FakeActionInputFileCache mockCache = new FakeActionInputFileCache(); mockCache.put(artifact, FileArtifactValue.createNormalFile(FAKE_DIGEST, 1)); - expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER); + expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, true); assertThat(inputMappings).hasSize(1); assertThat(inputMappings) .containsEntry(PathFragment.create("runfiles/workspace/symlink"), artifact); @@ -205,7 +205,7 @@ public void testRunfilesRootSymlink() throws Exception { FakeActionInputFileCache mockCache = new FakeActionInputFileCache(); mockCache.put(artifact, FileArtifactValue.createNormalFile(FAKE_DIGEST, 1)); - expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER); + expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, true); 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 @@ -236,7 +236,7 @@ public void testRunfilesWithTreeArtifacts() throws Exception { fakeCache.put(file1, FileArtifactValue.create(file1)); fakeCache.put(file2, FileArtifactValue.create(file2)); - expander.addRunfilesToInputs(inputMappings, supplier, fakeCache, artifactExpander); + expander.addRunfilesToInputs(inputMappings, supplier, fakeCache, artifactExpander, true); assertThat(inputMappings).hasSize(2); assertThat(inputMappings) .containsEntry(PathFragment.create("runfiles/workspace/treeArtifact/file1"), file1); @@ -268,7 +268,7 @@ public void testRunfilesWithTreeArtifactsInSymlinks() throws Exception { fakeCache.put(file1, FileArtifactValue.create(file1)); fakeCache.put(file2, FileArtifactValue.create(file2)); - expander.addRunfilesToInputs(inputMappings, supplier, fakeCache, artifactExpander); + expander.addRunfilesToInputs(inputMappings, supplier, fakeCache, artifactExpander, true); assertThat(inputMappings).hasSize(2); assertThat(inputMappings) .containsEntry(PathFragment.create("runfiles/workspace/symlink/file1"), file1); @@ -296,7 +296,7 @@ public void testTreeArtifactsInInputs() throws Exception { fakeCache.put(file2, FileArtifactValue.create(file2)); Spawn spawn = new SpawnBuilder("/bin/echo", "Hello World").withInput(treeArtifact).build(); - inputMappings = expander.getInputMapping(spawn, artifactExpander, fakeCache); + inputMappings = expander.getInputMapping(spawn, artifactExpander, fakeCache, true); assertThat(inputMappings).hasSize(2); assertThat(inputMappings).containsEntry(PathFragment.create("treeArtifact/file1"), file1); assertThat(inputMappings).containsEntry(PathFragment.create("treeArtifact/file2"), file2); 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 64b8b3e04db825..bc9fa265555988 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 @@ -242,7 +242,8 @@ public FileOutErr getFileOutErr() { } @Override - public SortedMap getInputMapping() { + public SortedMap getInputMapping( + boolean expandTreeArtifactsInRunfiles) { return inputMapping; } diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java index f87e78b42b83f6..8b392e23ce6c9c 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java @@ -181,9 +181,10 @@ public FileOutErr getFileOutErr() { } @Override - public SortedMap getInputMapping() throws IOException { + public SortedMap getInputMapping( + boolean expandTreeArtifactsInRunfiles) throws IOException { return new SpawnInputExpander(execRoot, /*strict*/ false) - .getInputMapping(simpleSpawn, SIMPLE_ARTIFACT_EXPANDER, fakeFileCache); + .getInputMapping(simpleSpawn, SIMPLE_ARTIFACT_EXPANDER, fakeFileCache, true); } @Override 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 b3387bd2d3dd91..7aba2488799435 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 @@ -144,9 +144,10 @@ public FileOutErr getFileOutErr() { } @Override - public SortedMap getInputMapping() throws IOException { + public SortedMap getInputMapping( + boolean expandTreeArtifactsInRunfiles) throws IOException { return new SpawnInputExpander(execRoot, /*strict*/ false) - .getInputMapping(simpleSpawn, SIMPLE_ARTIFACT_EXPANDER, fakeFileCache); + .getInputMapping(simpleSpawn, SIMPLE_ARTIFACT_EXPANDER, fakeFileCache, true); } @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 105a881deac785..8d8123622677f9 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 @@ -1094,9 +1094,10 @@ public FileOutErr getFileOutErr() { } @Override - public SortedMap getInputMapping() throws IOException { + public SortedMap getInputMapping( + boolean expandTreeArtifactsInRunfiles) throws IOException { return new SpawnInputExpander(execRoot, /*strict*/ false) - .getInputMapping(spawn, artifactExpander, fakeFileCache); + .getInputMapping(spawn, artifactExpander, fakeFileCache, true); } @Override diff --git a/src/test/shell/bazel/bazel_sandboxing_test.sh b/src/test/shell/bazel/bazel_sandboxing_test.sh index bd7a6d6db14182..0daee01d296c27 100755 --- a/src/test/shell/bazel/bazel_sandboxing_test.sh +++ b/src/test/shell/bazel/bazel_sandboxing_test.sh @@ -608,7 +608,6 @@ EOF } function test_sandbox_mount_customized_path () { - if ! [ "${PLATFORM-}" = "linux" -a \ "$(cat /dev/null /etc/*release | grep 'DISTRIB_CODENAME=' | sed 's/^.*=//')" = "trusty" ]; then echo "Skipping test: the toolchain used in this test is only supported on trusty." @@ -704,6 +703,64 @@ EOF rm -rf ${target_root}/x86_64-unknown-linux-gnu } +function test_experimental_symlinked_sandbox_uses_expanded_tree_artifacts_in_runfiles_tree() { + touch WORKSPACE + + cat > def.bzl <<'EOF' +def _mkdata_impl(ctx): + out = ctx.actions.declare_directory(ctx.label.name + ".d") + script = "mkdir -p {out}; touch {out}/file; ln -s file {out}/link".format(out = out.path) + ctx.actions.run_shell( + outputs = [out], + command = script, + ) + runfiles = ctx.runfiles(files = [out]) + return [DefaultInfo( + files = depset([out]), + runfiles = runfiles, + )] + +mkdata = rule( + _mkdata_impl, +) +EOF + + cat > mkdata_test.sh <<'EOF' +#!/bin/bash + +set -euo pipefail + +test_dir="$1" +cd "$test_dir" +ls -l | cut -f1,9 -d' ' >&2 + +if [ ! -f file -o -L file ]; then + echo "'file' is not a regular file" >&2 + exit 1 +fi +EOF + chmod +x mkdata_test.sh + + cat > BUILD <<'EOF' +load("//:def.bzl", "mkdata") + +mkdata(name = "mkdata") + +sh_test( + name = "mkdata_test", + srcs = ["mkdata_test.sh"], + args = ["$(location :mkdata)"], + data = [":mkdata"], +) +EOF + + bazel test --incompatible_symlinked_sandbox_expands_tree_artifacts_in_runfiles_tree \ + --test_output=streamed :mkdata_test &>$TEST_log && fail "expected test to fail" || true + + bazel test --noincompatible_symlinked_sandbox_expands_tree_artifacts_in_runfiles_tree \ + --test_output=streamed :mkdata_test &>$TEST_log || fail "expected test to pass" +} + # The test shouldn't fail if the environment doesn't support running it. check_supported_platform || exit 0 check_sandbox_allowed || exit 0