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..bc3d15c7b73a4b 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,12 +103,19 @@ 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. + *

The constructed list never contains middleman artifacts. If {@code keepEmptyTreeArtifacts} + * is true, a tree artifact will be included in the constructed list when it expands into zero + * file artifacts. Otherwise, only the file artifacts the tree artifact expands into will be + * included. + * + *

Non-middleman, non-tree artifacts are returned untouched. */ public static List expandArtifacts( - NestedSet inputs, ArtifactExpander artifactExpander) { + NestedSet inputs, + ArtifactExpander artifactExpander, + boolean keepEmptyTreeArtifacts) { List result = new ArrayList<>(); List containedArtifacts = new ArrayList<>(); for (ActionInput input : inputs.toList()) { @@ -118,7 +125,8 @@ public static List expandArtifacts( } containedArtifacts.add((Artifact) input); } - Artifact.addExpandedArtifacts(containedArtifacts, result, artifactExpander); + Artifact.addExpandedArtifacts( + containedArtifacts, result, artifactExpander, keepEmptyTreeArtifacts); return result; } 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..206019dc275a18 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 @@ -1520,42 +1520,63 @@ public static String joinRootRelativePaths(String delimiter, Iterable 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. + * + *

The constructed list never contains middleman artifacts. If {@code keepEmptyTreeArtifacts} + * is true, a tree artifact will be included in the constructed list when it expands into zero + * file artifacts. Otherwise, only the file artifacts the tree artifact expands into will be + * included. + */ static void addExpandedArtifacts( Iterable artifacts, Collection output, - ArtifactExpander artifactExpander) { - addExpandedArtifacts(artifacts, output, Functions.identity(), artifactExpander); + ArtifactExpander artifactExpander, + boolean keepEmptyTreeArtifacts) { + addExpandedArtifacts( + artifacts, output, Functions.identity(), artifactExpander, keepEmptyTreeArtifacts); } /** - * 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. + * + *

The constructed list never contains middleman artifacts. If {@code keepEmptyTreeArtifacts} + * is true, a tree artifact will be included in the constructed list when it expands into zero + * file artifacts. Otherwise, only the file artifacts the tree artifact expands into will be + * included. */ - private static void addExpandedArtifacts(Iterable artifacts, - Collection output, - Function outputFormatter, - ArtifactExpander artifactExpander) { + private static void addExpandedArtifacts( + Iterable artifacts, + Collection output, + Function outputFormatter, + ArtifactExpander artifactExpander, + boolean keepEmptyTreeArtifacts) { for (Artifact artifact : artifacts) { if (artifact.isMiddlemanArtifact() || artifact.isTreeArtifact()) { - expandArtifact(artifact, output, outputFormatter, artifactExpander); + expandArtifact(artifact, output, outputFormatter, artifactExpander, keepEmptyTreeArtifacts); } else { output.add(outputFormatter.apply(artifact)); } } } - private static void expandArtifact(Artifact middleman, + private static void expandArtifact( + Artifact middleman, Collection output, Function outputFormatter, - ArtifactExpander artifactExpander) { + ArtifactExpander artifactExpander, + boolean keepEmptyTreeArtifacts) { Preconditions.checkArgument(middleman.isMiddlemanArtifact() || middleman.isTreeArtifact()); List artifacts = new ArrayList<>(); artifactExpander.expand(middleman, artifacts); for (Artifact artifact : artifacts) { output.add(outputFormatter.apply(artifact)); } + if (keepEmptyTreeArtifacts && middleman.isTreeArtifact() && artifacts.isEmpty()) { + output.add(outputFormatter.apply(middleman)); + } } /** 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 44cb0210b92584..944892e7012473 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 @@ -130,7 +130,9 @@ void addRunfilesToInputs( if (localArtifact.isTreeArtifact()) { List expandedInputs = ActionInputHelper.expandArtifacts( - NestedSetBuilder.create(Order.STABLE_ORDER, localArtifact), artifactExpander); + NestedSetBuilder.create(Order.STABLE_ORDER, localArtifact), + artifactExpander, + /* keepEmptyTreeArtifacts= */ false); for (ActionInput input : expandedInputs) { addMapping( inputMap, @@ -222,7 +224,12 @@ private static void addInputs( NestedSet inputFiles, ArtifactExpander artifactExpander, PathFragment baseDirectory) { - List inputs = ActionInputHelper.expandArtifacts(inputFiles, artifactExpander); + // Actions that accept TreeArtifacts as inputs generally expect the directory corresponding + // to the artifact to be created, even if it is empty. We explicitly keep empty TreeArtifacts + // here to signal consumers that they should create the directory. + List inputs = + ActionInputHelper.expandArtifacts( + inputFiles, artifactExpander, /* keepEmptyTreeArtifacts= */ true); for (ActionInput input : inputs) { addMapping(inputMap, input.getExecPath(), input, baseDirectory); } @@ -230,8 +237,10 @@ private static 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 non-empty tree artifacts as they are + * expanded to file artifacts. Tree artifacts that would expand to the empty set under the + * provided {@link ArtifactExpander} are left untouched so that their corresponding empty + * directories can be created. * *

The returned map never contains {@code null} values. * 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/merkletree/BUILD b/src/main/java/com/google/devtools/build/lib/remote/merkletree/BUILD index 5a1a76569c4f9d..bca6b54363c084 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/util:string", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", 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..c641f80accfe0c 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,12 +17,14 @@ 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.DerivedArtifact; 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; import com.google.devtools.build.lib.remote.merkletree.DirectoryTree.DirectoryNode; import com.google.devtools.build.lib.remote.merkletree.DirectoryTree.FileNode; import com.google.devtools.build.lib.remote.util.DigestUtil; +import com.google.devtools.build.lib.skyframe.TreeArtifactValue; import com.google.devtools.build.lib.vfs.Dirent; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -33,6 +35,7 @@ import java.util.Map; import java.util.SortedMap; import java.util.TreeMap; +import javax.annotation.Nullable; /** Builder for directory trees. */ class DirectoryTreeBuilder { @@ -94,6 +97,7 @@ private static int buildFromPaths( Map tree) throws IOException { return build( + null, inputs, tree, (input, path, currDir) -> { @@ -122,6 +126,7 @@ private static int buildFromActionInputs( Map tree) throws IOException { return build( + metadataProvider, inputs, tree, (input, path, currDir) -> { @@ -177,6 +182,7 @@ private static int buildFromActionInputs( } private static int build( + @Nullable MetadataProvider metadataProvider, SortedMap inputs, Map tree, FileNodeVisitor fileNodeVisitor) @@ -192,6 +198,32 @@ private static int build( // Path relative to the exec root PathFragment path = e.getKey(); T input = e.getValue(); + + if (input instanceof DerivedArtifact && ((DerivedArtifact) input).isTreeArtifact()) { + DerivedArtifact artifact = (DerivedArtifact) input; + // MetadataProvider is provided by all callers for which T is a superclass of + // DerivedArtifact. + Preconditions.checkNotNull(metadataProvider); + FileArtifactValue metadata = + Preconditions.checkNotNull( + metadataProvider.getMetadata(artifact), + "missing metadata for '%s'", + artifact.getExecPathString()); + Preconditions.checkState( + metadata.equals(TreeArtifactValue.empty().getMetadata()), + "Encountered non-empty TreeArtifact '%s' with metadata '%s', which should have" + + " been expanded by SpawnInputExpander. This is a bug.", + path, + metadata); + // Create an empty directory and its parent directories but don't visit the TreeArtifact + // input itself: A TreeArtifact's metadata has type REGULAR_FILE, not DIRECTORY, and would + // thus lead to an empty file being created in the buildFromActionInputs visitor. + DirectoryNode emptyDir = new DirectoryNode(path.getBaseName()); + tree.put(path, emptyDir); + createParentDirectoriesIfNotExist(path, emptyDir, tree); + continue; + } + if (dirname == null || !path.getParentDirectory().equals(dirname)) { dirname = path.getParentDirectory(); dir = tree.get(dirname); 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 08ce6d6cc7d8d5..87ff072a8f612a 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 @@ -254,7 +254,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..b9fdbf3c69f69a 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 @@ -184,8 +184,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/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..0bf2dd42cf8379 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<>(); 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/WorkerFilesHash.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerFilesHash.java index e3afa41371c43c..85a4986e7a9d46 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerFilesHash.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerFilesHash.java @@ -64,7 +64,8 @@ public static SortedMap getWorkerFilesWithDigests( TreeMap workerFilesMap = new TreeMap<>(); List tools = - ActionInputHelper.expandArtifacts(spawn.getToolFiles(), artifactExpander); + ActionInputHelper.expandArtifacts( + spawn.getToolFiles(), artifactExpander, /* keepEmptyTreeArtifacts= */ false); for (ActionInput tool : tools) { @Nullable FileArtifactValue metadata = actionInputFileCache.getMetadata(tool); if (metadata == null) { 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..a7e001578990ad 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); @@ -259,7 +257,10 @@ private WorkRequest createWorkRequest( } List inputs = - ActionInputHelper.expandArtifacts(spawn.getInputFiles(), context.getArtifactExpander()); + ActionInputHelper.expandArtifacts( + spawn.getInputFiles(), + context.getArtifactExpander(), + /* keepEmptyTreeArtifacts= */ false); for (ActionInput input : inputs) { byte[] digestBytes = inputFileCache.getMetadata(input).getDigest(); 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..f8ed2c393fab2f 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; @@ -98,8 +92,7 @@ public void processInputFiles_materializesParamFile() throws Exception { ParameterFileType.UNQUOTED, UTF_8); - SandboxInputs inputs = - sandboxHelpers.processInputFiles(inputMap(paramFile), SPAWN, EMPTY_EXPANDER, execRoot); + SandboxInputs inputs = sandboxHelpers.processInputFiles(inputMap(paramFile), execRoot); assertThat(inputs.getFiles()) .containsExactly(PathFragment.create("paramFile"), execRoot.getChild("paramFile")); @@ -118,8 +111,7 @@ public void processInputFiles_materializesBinToolsFile() throws Exception { scratch.file("tool", "#!/bin/bash", "echo hello"), PathFragment.create("_bin/say_hello")); - SandboxInputs inputs = - sandboxHelpers.processInputFiles(inputMap(tool), SPAWN, EMPTY_EXPANDER, execRoot); + SandboxInputs inputs = sandboxHelpers.processInputFiles(inputMap(tool), execRoot); assertThat(inputs.getFiles()) .containsExactly( @@ -142,8 +134,7 @@ public void processInputFiles_delayVirtualInputMaterialization_doesNotStoreVirtu ParameterFileType.UNQUOTED, UTF_8); - SandboxInputs inputs = - sandboxHelpers.processInputFiles(inputMap(paramFile), SPAWN, EMPTY_EXPANDER, execRoot); + SandboxInputs inputs = sandboxHelpers.processInputFiles(inputMap(paramFile), execRoot); assertThat(inputs.getFiles()).isEmpty(); assertThat(inputs.getSymlinks()).isEmpty(); @@ -163,9 +154,7 @@ public void sandboxInputMaterializeVirtualInputs_delayMaterialization_writesCorr BinTools.PathActionInput tool = new BinTools.PathActionInput( scratch.file("tool", "tool_code"), PathFragment.create("tools/tool")); - SandboxInputs inputs = - sandboxHelpers.processInputFiles( - inputMap(paramFile, tool), SPAWN, EMPTY_EXPANDER, execRoot); + SandboxInputs inputs = sandboxHelpers.processInputFiles(inputMap(paramFile, tool), execRoot); inputs.materializeVirtualInputs(scratch.dir("/sandbox")); @@ -212,14 +201,13 @@ protected void setExecutable(PathFragment path, boolean executable) throws IOExc executorToCleanup.submit( () -> { try { - sandboxHelpers.processInputFiles( - inputMap(input), SPAWN, EMPTY_EXPANDER, customExecRoot); + sandboxHelpers.processInputFiles(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..a11e71249c97a9 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,40 @@ EOF bazel build --test_output=streamed :a &>$TEST_log || fail "expected build to succeed" } +function test_empty_tree_artifact_as_inputs() { + # Test that when an empty tree artifact is the input, an empty directory is + # created in the sandbox for action to read. + 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 b9ccff357fa831..d669cb32fd1512 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -1006,63 +1006,6 @@ EOF || fail "Failed to run //a:starlark_output_dir_test with remote execution" } -function generate_empty_treeartifact_build() { - mkdir -p a - cat > a/BUILD <<'EOF' -load(":output_dir.bzl", "gen_output_dir") -gen_output_dir( - name = "output-dir", - outdir = "dir", -) -EOF - cat > a/output_dir.bzl <<'EOF' -def _gen_output_dir_impl(ctx): - output_dir = ctx.actions.declare_directory(ctx.attr.outdir) - ctx.actions.run_shell( - outputs = [output_dir], - inputs = [], - command = "", - arguments = [output_dir.path], - ) - return [ - DefaultInfo(files = depset(direct = [output_dir])), - ] - -gen_output_dir = rule( - implementation = _gen_output_dir_impl, - attrs = { - "outdir": attr.string(mandatory = True), - }, -) -EOF -} - -function test_empty_treeartifact_works_with_remote_execution() { - # Test that empty tree artifact works with remote execution - generate_empty_treeartifact_build - - bazel build \ - --remote_executor=grpc://localhost:${worker_port} \ - //a:output-dir >& $TEST_log || fail "Failed to build" -} - -function test_empty_treeartifact_works_with_remote_cache() { - # Test that empty tree artifact works with remote cache - generate_empty_treeartifact_build - - bazel build \ - --remote_cache=grpc://localhost:${worker_port} \ - //a:output-dir >& $TEST_log || fail "Failed to build" - - bazel clean - - bazel build \ - --remote_cache=grpc://localhost:${worker_port} \ - //a:output-dir >& $TEST_log || fail "Failed to build" - - expect_log "remote cache hit" -} - function test_downloads_minimal() { # Test that genrule outputs are not downloaded when using # --remote_download_minimal @@ -3076,6 +3019,77 @@ end_of_record" assert_equals "$expected_result" "$(cat bazel-testlogs/java/factorial/fact-test/coverage.dat)" } +function generate_empty_tree_artifact_as_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 +} + +function test_empty_tree_artifact_as_inputs() { + # Test that when an empty tree artifact is the input, an empty directory is + # created in the remote executor for action to read. + generate_empty_tree_artifact_as_inputs + + 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" + + bazel clean --expunge + bazel build \ + --spawn_strategy=remote \ + --remote_executor=grpc://localhost:${worker_port} \ + --experimental_sibling_repository_layout \ + //pkg:a &>$TEST_log || fail "expected build to succeed with sibling repository layout" +} + +function test_empty_tree_artifact_as_inputs_remote_cache() { + # Test that when empty tree artifact works for remote cache. + generate_empty_tree_artifact_as_inputs + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + //pkg:a &>$TEST_log || fail "expected build to succeed" + + bazel clean + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + //pkg:a &>$TEST_log || fail "expected build to succeed" + + expect_log "remote cache hit" +} + # 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.