diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java index ea5f758f408121..a27c3165354975 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java @@ -103,9 +103,9 @@ public PathFragment getExecPath() { } /** - * Expands middleman artifacts in a sequence of {@link ActionInput}s. + * Expands middleman and tree artifacts in a sequence of {@link ActionInput}s. * - *

Non-middleman artifacts are returned untouched. + *

Non-middleman, non-tree artifacts are returned untouched. */ public static List expandArtifacts( NestedSet inputs, ArtifactExpander artifactExpander) { diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java index 780e263ea3ba27..6f107c5f5c0161 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java @@ -774,6 +774,10 @@ public boolean isChildOfDeclaredDirectory() { return false; } + public boolean isEmptyDirectoryMarker() { + return false; + } + /** * Returns whether the artifact represents a Fileset. * @@ -1292,6 +1296,10 @@ public ArchivedTreeArtifact deserialize( * */ public static final class TreeFileArtifact extends DerivedArtifact { + + private static final PathFragment EMPTY_DIRECTORY_MARKER = PathFragment.createAlreadyNormalized( + "BAZEL_TREE_FILE_ARTIFACT_EMPTY_DIRECTORY_MARKER"); + private final SpecialArtifact parent; private final PathFragment parentRelativePath; @@ -1329,6 +1337,10 @@ public static TreeFileArtifact createTreeOutput( return createTreeOutput(parent, PathFragment.create(parentRelativePath)); } + public static TreeFileArtifact createEmptyDirectoryMarker(SpecialArtifact parent) { + return createTreeOutput(parent, EMPTY_DIRECTORY_MARKER); + } + /** * Creates a {@link TreeFileArtifact} representing the output of an action generated dynamically * by an {@link ActionTemplate} during the execution phase. @@ -1392,6 +1404,11 @@ public boolean isChildOfDeclaredDirectory() { return !isActionTemplateExpansionKey(getArtifactOwner()); } + @Override + public boolean isEmptyDirectoryMarker() { + return parentRelativePath == EMPTY_DIRECTORY_MARKER; + } + private static boolean isActionTemplateExpansionKey(ActionLookupKey key) { return SkyFunctions.ACTION_TEMPLATE_EXPANSION.equals(key.functionName()); } @@ -1513,14 +1530,17 @@ public static String joinExecPaths(String delimiter, Iterable artifact } /** - * Renders a collection of artifacts as root-relative paths and joins - * them into a single string. Middleman artifacts are ignored by this method. + * Renders a collection of artifacts as root-relative paths and joins them into a single string. + * Middleman artifacts are ignored by this method. */ public static String joinRootRelativePaths(String delimiter, Iterable artifacts) { return Joiner.on(delimiter).join(toRootRelativePaths(artifacts)); } - /** Adds a collection of artifacts to a given collection, with middleman actions expanded once. */ + /** + * Adds a collection of artifacts to a given collection, with middleman actions and tree artifacts + * expanded once. + */ static void addExpandedArtifacts( Iterable artifacts, Collection output, @@ -1529,9 +1549,8 @@ static void addExpandedArtifacts( } /** - * Converts a collection of artifacts into the outputs computed by - * outputFormatter and adds them to a given collection. Middleman artifacts - * are expanded once. + * Converts a collection of artifacts into the outputs computed by outputFormatter and adds them + * to a given collection. Middleman artifacts and tree artifacts are expanded once. */ private static void addExpandedArtifacts(Iterable artifacts, Collection output, @@ -1556,6 +1575,10 @@ private static void expandArtifact(Artifact middleman, for (Artifact artifact : artifacts) { output.add(outputFormatter.apply(artifact)); } + if (middleman.isTreeArtifact() && artifacts.isEmpty()) { + output.add(outputFormatter.apply(TreeFileArtifact.createEmptyDirectoryMarker( + (SpecialArtifact) middleman))); + } } /** diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java index 8da6e8881e809b..ede9b2769ddbce 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java @@ -19,6 +19,7 @@ import com.google.common.hash.HashCode; import com.google.devtools.build.lib.actions.ActionContext; import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.MetadataProvider; @@ -101,6 +102,9 @@ public void logSpawn( if (input instanceof VirtualActionInput.EmptyActionInput) { continue; } + if (input instanceof Artifact && ((Artifact) input).isEmptyDirectoryMarker()) { + continue; + } Path inputPath = execRoot.getRelative(input.getExecPathString()); if (inputPath.isDirectory()) { listDirectoryContents(inputPath, builder::addInputs, metadataProvider); diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java index 3f58e71dd23755..885ff46774c44c 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java @@ -586,7 +586,6 @@ private static Spawn createCoveragePostProcessingSpawn( .addTransitive(action.getInputs()) .addAll(expandedCoverageDir) .add(action.getCollectCoverageScript()) - .add(action.getCoverageDirectoryTreeArtifact()) .add(action.getCoverageManifest()) .addTransitive(action.getLcovMergerFilesToRun().build()) .build(), diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java index 378c05b8b2737b..d47bbdb18923e4 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java @@ -27,6 +27,7 @@ import com.google.common.util.concurrent.MoreExecutors; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputPrefetcher; +import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; @@ -132,7 +133,7 @@ private Single downloadOneInput( SandboxHelpers.atomicallyWriteVirtualInput( virtualActionInput, outputPath, ".remote"); } - } else { + } else if (!(input instanceof Artifact) || !((Artifact) input).isEmptyDirectoryMarker()) { FileArtifactValue metadata = metadataProvider.getMetadata(input); if (metadata != null && metadata.isRemote()) { Path path = execRoot.getRelative(input.getExecPath()); 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 67d02013fb2520..626b84ec868491 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 @@ -21,6 +21,7 @@ import com.google.common.base.Stopwatch; import com.google.common.base.Throwables; import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.ForbiddenActionInputException; @@ -203,6 +204,9 @@ private void checkForConcurrentModifications() if (input instanceof VirtualActionInput) { continue; } + if (input instanceof Artifact && ((Artifact) input).isEmptyDirectoryMarker()) { + continue; + } FileArtifactValue metadata = context.getMetadataProvider().getMetadata(input); Path path = execRoot.getRelative(input.getExecPath()); if (metadata.wasModifiedSinceDigest(path)) { 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 07a11923f0f5d8..c5abc55966955c 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 @@ -30,6 +30,7 @@ import com.google.common.base.Throwables; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ForbiddenActionInputException; @@ -543,6 +544,9 @@ private Map getInputCtimes(SortedMap inpu if (input instanceof VirtualActionInput) { continue; } + if (input instanceof Artifact && ((Artifact) input).isEmptyDirectoryMarker()) { + continue; + } Path path = execRoot.getRelative(input.getExecPathString()); try { ctimes.put(path, path.stat().getLastChangeTime()); diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/BUILD b/src/main/java/com/google/devtools/build/lib/remote/merkletree/BUILD index 46283f29a44b54..b7831a8136a7f6 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/BUILD @@ -20,6 +20,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/remote/util", + "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//third_party:guava", diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java index a29304b1ba40cd..175e4077569685 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java @@ -17,6 +17,7 @@ import com.google.common.base.Preconditions; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputHelper; +import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; @@ -133,6 +134,8 @@ private static int buildFromActionInputs( FileNode.createExecutable( path.getBaseName(), virtualActionInput.getBytes(), d)); return childAdded ? 1 : 0; + } else if (input instanceof Artifact && ((Artifact) input).isEmptyDirectoryMarker()) { + return 0; } FileArtifactValue metadata = diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java index f519402d77bf23..0aaf9fe92f953b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java +++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java @@ -252,7 +252,8 @@ private static MerkleTree build(DirectoryTree tree, DigestUtil digestUtil) { for (DirectoryTree.DirectoryNode dir : dirs) { PathFragment subDirname = dirname.getRelative(dir.getPathSegment()); MerkleTree subMerkleTree = - Preconditions.checkNotNull(m.remove(subDirname), "subMerkleTree was null"); + Preconditions.checkNotNull(m.remove(subDirname), "subMerkleTree at '%s' was null", + subDirname); subDirs.put(dir.getPathSegment(), subMerkleTree); } MerkleTree mt = buildMerkleTree(new TreeSet<>(files), subDirs, digestUtil); 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 d1f5ec24143e56..630a4551485e7f 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 @@ -233,8 +233,6 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT), - spawn, - context.getArtifactExpander(), execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); 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 f56eafd8cf9573..1a46186e76c991 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 @@ -223,8 +223,6 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT), - spawn, - context.getArtifactExpander(), execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); 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 ffb7a5f3b7fd02..de0195db89f3e8 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 @@ -20,6 +20,7 @@ import com.google.common.collect.Maps; import com.google.common.io.ByteStreams; import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionRequirements; import com.google.devtools.build.lib.actions.FileArtifactValue; @@ -184,8 +185,6 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT), - spawn, - context.getArtifactExpander(), execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); @@ -389,6 +388,9 @@ private void checkForConcurrentModifications(SpawnExecutionContext context) if (input instanceof VirtualActionInput) { continue; } + if (input instanceof Artifact && ((Artifact) input).isEmptyDirectoryMarker()) { + continue; + } FileArtifactValue metadata = context.getMetadataProvider().getMetadata(input); Path path = execRoot.getRelative(input.getExecPath()); 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 79b625ea896c0c..6b6835eff2b549 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 @@ -112,8 +112,6 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT), - spawn, - context.getArtifactExpander(), execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); 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 ea9f071b89ba3c..68178749a91418 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 @@ -27,7 +27,6 @@ import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.cache.VirtualActionInput.EmptyActionInput; @@ -42,10 +41,8 @@ import com.google.devtools.common.options.OptionsParsingResult; import java.io.IOException; import java.io.OutputStream; -import java.util.ArrayList; import java.util.HashSet; import java.util.LinkedHashSet; -import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -487,27 +484,8 @@ private static void writeVirtualInputTo(VirtualActionInput input, Path target) */ public SandboxInputs processInputFiles( Map inputMap, - Spawn spawn, - ArtifactExpander artifactExpander, Path execRoot) throws IOException { - // SpawnInputExpander#getInputMapping uses ArtifactExpander#expandArtifacts to expand - // middlemen and tree artifacts, which expands empty tree artifacts to no entry. However, - // actions that accept TreeArtifacts as inputs generally expect that the empty directory is - // created. So we add those explicitly here. - // TODO(ulfjack): Move this code to SpawnInputExpander. - for (ActionInput input : spawn.getInputFiles().toList()) { - if (input instanceof Artifact && ((Artifact) input).isTreeArtifact()) { - List containedArtifacts = new ArrayList<>(); - artifactExpander.expand((Artifact) input, containedArtifacts); - // Attempting to mount a non-empty directory results in ERR_DIRECTORY_NOT_EMPTY, so we - // only mount empty TreeArtifacts as directories. - if (containedArtifacts.isEmpty()) { - inputMap.put(input.getExecPath(), input); - } - } - } - Map inputFiles = new TreeMap<>(); Set virtualInputs = new HashSet<>(); Map inputSymlinks = new TreeMap<>(); @@ -516,6 +494,10 @@ public SandboxInputs processInputFiles( PathFragment pathFragment = e.getKey(); ActionInput actionInput = e.getValue(); + if (actionInput instanceof Artifact && ((Artifact) actionInput).isEmptyDirectoryMarker()) { + continue; + } + // TODO(b/150963503): Make delayVirtualInputMaterialization the default and remove the // alternate code path. if (delayVirtualInputMaterialization) { 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 f602fab1adbc8a..3be3a5496bedcd 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 @@ -72,8 +72,6 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs readablePaths = helpers.processInputFiles( 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 ec16d6a1ddb14d..146a7d1ce14c37 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 @@ -202,8 +202,6 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) inputFiles = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT), - spawn, - context.getArtifactExpander(), execRoot); } SandboxOutputs outputs = helpers.getOutputs(spawn); diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java index 61fa76665192c1..9083a1e36f57e6 100644 --- a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java +++ b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java @@ -24,14 +24,11 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionInput; -import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput; import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; -import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.exec.BinTools; -import com.google.devtools.build.lib.exec.util.SpawnBuilder; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs; import com.google.devtools.build.lib.testutil.Scratch; @@ -66,9 +63,6 @@ @RunWith(JUnit4.class) public class SandboxHelpersTest { - private static final ArtifactExpander EMPTY_EXPANDER = (ignored1, ignored2) -> {}; - private static final Spawn SPAWN = new SpawnBuilder().build(); - private final Scratch scratch = new Scratch(); private Path execRoot; @Nullable private ExecutorService executorToCleanup; @@ -99,7 +93,7 @@ public void processInputFiles_materializesParamFile() throws Exception { UTF_8); SandboxInputs inputs = - sandboxHelpers.processInputFiles(inputMap(paramFile), SPAWN, EMPTY_EXPANDER, execRoot); + sandboxHelpers.processInputFiles(inputMap(paramFile), execRoot); assertThat(inputs.getFiles()) .containsExactly(PathFragment.create("paramFile"), execRoot.getChild("paramFile")); @@ -119,7 +113,7 @@ public void processInputFiles_materializesBinToolsFile() throws Exception { PathFragment.create("_bin/say_hello")); SandboxInputs inputs = - sandboxHelpers.processInputFiles(inputMap(tool), SPAWN, EMPTY_EXPANDER, execRoot); + sandboxHelpers.processInputFiles(inputMap(tool), execRoot); assertThat(inputs.getFiles()) .containsExactly( @@ -143,7 +137,7 @@ public void processInputFiles_delayVirtualInputMaterialization_doesNotStoreVirtu UTF_8); SandboxInputs inputs = - sandboxHelpers.processInputFiles(inputMap(paramFile), SPAWN, EMPTY_EXPANDER, execRoot); + sandboxHelpers.processInputFiles(inputMap(paramFile), execRoot); assertThat(inputs.getFiles()).isEmpty(); assertThat(inputs.getSymlinks()).isEmpty(); @@ -165,7 +159,7 @@ public void sandboxInputMaterializeVirtualInputs_delayMaterialization_writesCorr scratch.file("tool", "tool_code"), PathFragment.create("tools/tool")); SandboxInputs inputs = sandboxHelpers.processInputFiles( - inputMap(paramFile, tool), SPAWN, EMPTY_EXPANDER, execRoot); + inputMap(paramFile, tool), execRoot); inputs.materializeVirtualInputs(scratch.dir("/sandbox")); @@ -213,13 +207,13 @@ protected void setExecutable(PathFragment path, boolean executable) throws IOExc () -> { try { sandboxHelpers.processInputFiles( - inputMap(input), SPAWN, EMPTY_EXPANDER, customExecRoot); + inputMap(input), customExecRoot); finishProcessingSemaphore.release(); } catch (IOException e) { throw new IllegalArgumentException(e); } }); - sandboxHelpers.processInputFiles(inputMap(input), SPAWN, EMPTY_EXPANDER, customExecRoot); + sandboxHelpers.processInputFiles(inputMap(input), customExecRoot); finishProcessingSemaphore.release(); future.get(); diff --git a/src/test/shell/bazel/bazel_sandboxing_test.sh b/src/test/shell/bazel/bazel_sandboxing_test.sh index 3e421533d977df..41541fb2e626b0 100755 --- a/src/test/shell/bazel/bazel_sandboxing_test.sh +++ b/src/test/shell/bazel/bazel_sandboxing_test.sh @@ -794,7 +794,7 @@ EOF } # regression test for https://github.com/bazelbuild/bazel/issues/6262 -function test_create_tree_artifact_inputs() { +function test_create_tree_artifact_outputs() { create_workspace_with_default_repos WORKSPACE cat > def.bzl <<'EOF' @@ -818,6 +818,38 @@ EOF bazel build --test_output=streamed :a &>$TEST_log || fail "expected build to succeed" } +function test_create_tree_artifact_inputs() { + create_workspace_with_default_repos WORKSPACE + + mkdir -p pkg + + cat > pkg/def.bzl <<'EOF' +def _r(ctx): + empty_d = ctx.actions.declare_directory("%s/empty_dir" % ctx.label.name) + ctx.actions.run_shell( + outputs = [empty_d], + command = "mkdir -p %s" % empty_d.path, + ) + f = ctx.actions.declare_file("%s/file" % ctx.label.name) + ctx.actions.run_shell( + inputs = [empty_d], + outputs = [f], + command = "touch %s && cd %s && pwd" % (f.path, empty_d.path), + ) + return [DefaultInfo(files = depset([f]))] + +r = rule(implementation = _r) +EOF + +cat > pkg/BUILD <<'EOF' +load(":def.bzl", "r") + +r(name = "a") +EOF + + bazel build //pkg:a &>$TEST_log || fail "expected build to succeed" +} + # The test shouldn't fail if the environment doesn't support running it. check_sandbox_allowed || exit 0 diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index 4c64bc30e2af2a..d08646c4067e0b 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -3071,6 +3071,47 @@ end_of_record" assert_equals "$expected_result" "$(cat bazel-testlogs/java/factorial/fact-test/coverage.dat)" } +function test_create_tree_artifact_inputs() { + touch WORKSPACE + mkdir -p pkg + + cat > pkg/def.bzl <<'EOF' +def _r(ctx): + empty_d = ctx.actions.declare_directory("%s/empty_dir" % ctx.label.name) + ctx.actions.run_shell( + outputs = [empty_d], + command = "mkdir -p %s" % empty_d.path, + ) + f = ctx.actions.declare_file("%s/file" % ctx.label.name) + ctx.actions.run_shell( + inputs = [empty_d], + outputs = [f], + command = "touch %s && cd %s && pwd" % (f.path, empty_d.path), + ) + return [DefaultInfo(files = depset([f]))] + +r = rule(implementation = _r) +EOF + +cat > pkg/BUILD <<'EOF' +load(":def.bzl", "r") + +r(name = "a") +EOF + + bazel build \ + --spawn_strategy=remote \ + --remote_executor=grpc://localhost:${worker_port} \ + //pkg:a &>$TEST_log || fail "expected build to succeed" + + bazel clean --expunge + bazel build \ + --spawn_strategy=remote \ + --remote_executor=grpc://localhost:${worker_port} \ + --experimental_remote_merkle_tree_cache \ + //pkg:a &>$TEST_log || fail "expected build to succeed with Merkle tree cache" +} + # Runs coverage with `cc_test` and RE then checks the coverage file is returned. # Older versions of gcov are not supported with bazel coverage and so will be skipped. # See the above `test_java_rbe_coverage_produces_report` for more information.