[8.8.0] Expand tree artifact inputs from metadata in the compact execution log (#30830)
An empty tree artifact whose generating action template expands into no
actions is never materialized on disk when a disk or remote cache is in
use: nothing creates the directory and the `RemoteActionFileSystem` has
no entries for it either. Logging a spawn that has such a tree artifact
as an input then fails the action with
ERROR: Linking _nativedeps/6a5ccd5.so failed: IOException while logging
spawn: .../bin/_objs/biome_proto.upb_minitable/iome (No such file or
directory)
since `expandDirectory` readdirs the directory. This happens while
logging *inputs*, which isn't covered by the try/catch around output
logging added in #30717.
Expand tree artifacts from their `TreeArtifactValue` instead. Their
contents are already known, so this avoids the traversal entirely and
also keeps the log complete for trees whose files aren't materialized
locally. Source directories and filesets keep using the filesystem
traversal.
Fixes
https://github.com/bazelbuild/bazel/issues/22920#issuecomment-5345473352
### Description
### Motivation
### Build API Changes
No
### Checklist
- [x] I have added tests for the new use cases (if any).
- [ ] I have updated the documentation (if applicable).
### Release Notes
RELNOTES: Fixed a build failure with `--execution_log_compact_file` when
a C++ action template has no inputs.
Closes #30816.
PiperOrigin-RevId: 968391836
Change-Id: Id05a603a4e1987eef2c50d7722a8ee5955530e83
(cherry picked from commit df9ab4fa0aa2362d007e586a92bcce477dccaf4e)
8.8.0 adaptation: `InputMetadataProvider` has no `getTreeMetadata` on
this branch, so the pick adds one. It is keyed on the exec path rather
than the `ActionInput` so that the existing
`ActionInputMap#getTreeMetadata(PathFragment)` implements it as is, and
it defaults to `null`, which keeps the filesystem traversal for
providers that can't answer (`StaticInputMetadataProvider`,
`SingleBuildFileCache`). The providers that a spawn is actually logged
with forward to their delegates: `ActionInputMetadataProvider`,
`DelegatingPairInputMetadataProvider` and
`ActionExecutionContext.OverriddenRunfilesPathInputMetadataProvider`.
`internalToUnicode` doesn't exist on this branch either, so paths are
used as is, matching the surrounding code.
Closes #30818
Co-authored-by: Ian (Hee) Cha <heec@google.com>
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
index e3eb373..8125018 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
@@ -34,6 +34,7 @@
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.SyscallCache;
@@ -157,6 +158,12 @@
@Nullable
@Override
+ public TreeArtifactValue getTreeMetadata(PathFragment execPath) {
+ return wrapped.getTreeMetadata(execPath);
+ }
+
+ @Nullable
+ @Override
public ActionInput getInput(String execPath) {
return wrapped.getInput(execPath);
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputMap.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputMap.java
index 04b54a8..ed24da6 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionInputMap.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputMap.java
@@ -299,6 +299,7 @@
* artifact exists.
*/
@Nullable
+ @Override
public TreeArtifactValue getTreeMetadata(PathFragment execPath) {
int index = getIndex(execPath.getPathString());
if (index < 0) {
diff --git a/src/main/java/com/google/devtools/build/lib/actions/DelegatingPairInputMetadataProvider.java b/src/main/java/com/google/devtools/build/lib/actions/DelegatingPairInputMetadataProvider.java
index 6e59cf9..3da30a3 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/DelegatingPairInputMetadataProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/DelegatingPairInputMetadataProvider.java
@@ -15,6 +15,8 @@
package com.google.devtools.build.lib.actions;
import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
+import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.util.LinkedHashSet;
import javax.annotation.Nullable;
@@ -41,6 +43,13 @@
@Override
@Nullable
+ public TreeArtifactValue getTreeMetadata(PathFragment execPath) {
+ TreeArtifactValue result = primary.getTreeMetadata(execPath);
+ return result != null ? result : secondary.getTreeMetadata(execPath);
+ }
+
+ @Override
+ @Nullable
public RunfilesArtifactValue getRunfilesMetadata(ActionInput input) {
RunfilesArtifactValue result = primary.getRunfilesMetadata(input);
return result != null ? result : secondary.getRunfilesMetadata(input);
diff --git a/src/main/java/com/google/devtools/build/lib/actions/InputMetadataProvider.java b/src/main/java/com/google/devtools/build/lib/actions/InputMetadataProvider.java
index 9097296..28402db 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/InputMetadataProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/InputMetadataProvider.java
@@ -16,7 +16,9 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
import com.google.devtools.build.lib.vfs.FileSystem;
+import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import javax.annotation.Nullable;
@@ -43,6 +45,15 @@
FileArtifactValue getInputMetadata(ActionInput input) throws IOException;
/**
+ * Returns the {@link TreeArtifactValue} for the given exec path, or null if it does not point at a
+ * tree artifact known to this provider.
+ */
+ @Nullable
+ default TreeArtifactValue getTreeMetadata(PathFragment execPath) {
+ return null;
+ }
+
+ /**
* Returns the {@link RunfilesArtifactValue} for the given {@link ActionInput}, which must be a
* runfiles middleman artifact.
*
diff --git a/src/main/java/com/google/devtools/build/lib/exec/BUILD b/src/main/java/com/google/devtools/build/lib/exec/BUILD
index b3f0f1c..467ef5c 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD
@@ -288,6 +288,7 @@
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/remote/options",
"//src/main/java/com/google/devtools/build/lib/remote/util:digest_utils",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/util/io",
"//src/main/java/com/google/devtools/build/lib/util/io:io-proto",
"//src/main/java/com/google/devtools/build/lib/vfs",
diff --git a/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java
index c49db49..2f17580 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java
@@ -47,6 +47,7 @@
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
+import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
import com.google.devtools.build.lib.util.io.AsynchronousMessageOutputStream;
import com.google.devtools.build.lib.util.io.MessageOutputStream;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
@@ -552,7 +553,7 @@
.setDirectory(
ExecLogEntry.Directory.newBuilder()
.setPath(input.getExecPathString())
- .addAllFiles(expandDirectory(root, inputMetadataProvider))));
+ .addAllFiles(expandDirectory(input, root, inputMetadataProvider))));
}
/**
@@ -624,10 +625,52 @@
/**
* Expands a directory.
*
+ * @param input the input representing the directory
* @param root the path to the directory
* @return the list of files transitively contained in the directory
*/
private List<ExecLogEntry.File> expandDirectory(
+ ActionInput input, Path root, InputMetadataProvider inputMetadataProvider)
+ throws IOException, InterruptedException {
+ if (input instanceof Artifact artifact && artifact.isTreeArtifact()) {
+ TreeArtifactValue treeMetadata =
+ inputMetadataProvider.getTreeMetadata(artifact.getExecPath());
+ if (treeMetadata != null) {
+ // Using the metadata over a filesystem traversal is not just an optimization: an empty tree
+ // artifact may not be materialized on disk.
+ return expandTreeArtifact(treeMetadata, root, inputMetadataProvider);
+ }
+ }
+ return expandDirectoryFromFileSystem(root, inputMetadataProvider);
+ }
+
+ /** Expands a tree artifact into its contents as recorded in its metadata. */
+ private List<ExecLogEntry.File> expandTreeArtifact(
+ TreeArtifactValue treeMetadata, Path root, InputMetadataProvider inputMetadataProvider)
+ throws IOException {
+ var files = new ArrayList<ExecLogEntry.File>(treeMetadata.getChildren().size());
+ for (var child : treeMetadata.getChildren()) {
+ PathFragment parentRelativePath = child.getParentRelativePath();
+ Digest digest =
+ computeDigest(
+ child,
+ root.getRelative(parentRelativePath),
+ inputMetadataProvider,
+ xattrProvider,
+ digestHashFunction,
+ /* includeHashFunctionName= */ false);
+ files.add(
+ ExecLogEntry.File.newBuilder()
+ .setPath(parentRelativePath.getPathString())
+ .setDigest(digest)
+ .build());
+ }
+ files.sort(EXEC_LOG_ENTRY_FILE_COMPARATOR);
+ return files;
+ }
+
+ /** Expands a directory by traversing it on the filesystem. */
+ private List<ExecLogEntry.File> expandDirectoryFromFileSystem(
Path root, InputMetadataProvider inputMetadataProvider)
throws IOException, InterruptedException {
ArrayList<ExecLogEntry.File> files = new ArrayList<>();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMetadataProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMetadataProvider.java
index 68bab8c..8cd7834 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMetadataProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMetadataProvider.java
@@ -101,6 +101,12 @@
@Nullable
@Override
+ public TreeArtifactValue getTreeMetadata(PathFragment execPath) {
+ return inputArtifactData.getTreeMetadata(execPath);
+ }
+
+ @Nullable
+ @Override
public RunfilesArtifactValue getRunfilesMetadata(ActionInput input) {
return inputArtifactData.getRunfilesMetadata(input);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index c626e5b..36d6fc4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -541,6 +541,7 @@
name = "action_input_metadata_provider",
srcs = ["ActionInputMetadataProvider.java"],
deps = [
+ ":tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
diff --git a/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java b/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java
index ce2881f..25de7c0 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java
@@ -21,9 +21,11 @@
import com.github.luben.zstd.ZstdInputStream;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSortedMap;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.BuildConfigurationEvent;
import com.google.devtools.build.lib.actions.RunfilesTree;
import com.google.devtools.build.lib.actions.Spawn;
@@ -35,8 +37,10 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.exec.Protos.File;
import com.google.devtools.build.lib.exec.Protos.SpawnExec;
+import com.google.devtools.build.lib.exec.util.FakeActionInputFileCache;
import com.google.devtools.build.lib.exec.util.SpawnBuilder;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
+import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.Path;
@@ -241,6 +245,34 @@
}
@Test
+ public void testUnmaterializedEmptyTreeInput() throws Exception {
+ SpecialArtifact treeInput =
+ ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputDir, "tree");
+
+ // Deliberately don't create the directory: an empty tree artifact may not be materialized on
+ // disk, so its (empty) contents are only known to in-memory metadata.
+ assertThat(treeInput.getPath().exists()).isFalse();
+
+ TreeArtifactValue treeMetadata = TreeArtifactValue.newBuilder(treeInput).build();
+ FakeActionInputFileCache inputMetadataProvider = new FakeActionInputFileCache();
+ inputMetadataProvider.put(treeInput, treeMetadata.getMetadata());
+ inputMetadataProvider.putTreeArtifact(treeInput, treeMetadata);
+
+ SpawnLogContext context = createSpawnLogContext();
+
+ context.logSpawn(
+ defaultSpawnBuilder().withInputs(treeInput).build(),
+ inputMetadataProvider,
+ // SpawnInputExpander keeps empty tree artifacts in the input map.
+ ImmutableSortedMap.of(treeInput.getExecPath(), treeInput),
+ fs,
+ defaultTimeout(),
+ defaultSpawnResult());
+
+ closeAndAssertLog(context, defaultSpawnExecBuilder().build());
+ }
+
+ @Test
public void testUnreadableOutputs(@TestParameter OutputsMode outputsMode) throws Exception {
Artifact readableFile = ActionsTestUtil.createArtifact(outputDir, "readable");
Artifact unreadableFile = ActionsTestUtil.createArtifact(outputDir, "unreadable");
diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/BUILD b/src/test/java/com/google/devtools/build/lib/exec/util/BUILD
index 0d87bec..b94b81e 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/util/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/exec/util/BUILD
@@ -50,8 +50,10 @@
"//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_registry",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_resolver",
"//src/main/java/com/google/devtools/build/lib/exec:symlink_tree_strategy",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//src/main/java/com/google/devtools/build/lib/vfs",
+ "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/common/options",
"//src/main/java/net/starlark/java/syntax",
"//src/test/java/com/google/devtools/build/lib/testutil:TestConstants",
diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/FakeActionInputFileCache.java b/src/test/java/com/google/devtools/build/lib/exec/util/FakeActionInputFileCache.java
index 29ecf12..744bfd5 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/util/FakeActionInputFileCache.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/util/FakeActionInputFileCache.java
@@ -19,6 +19,8 @@
import com.google.devtools.build.lib.actions.InputMetadataProvider;
import com.google.devtools.build.lib.actions.RunfilesArtifactValue;
import com.google.devtools.build.lib.actions.RunfilesTree;
+import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
+import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
@@ -31,6 +33,7 @@
private static final byte[] EMPTY_DIGEST = new byte[0];
private final Map<ActionInput, FileArtifactValue> inputs = new HashMap<>();
+ private final Map<PathFragment, TreeArtifactValue> treeArtifacts = new HashMap<>();
private final Map<ActionInput, RunfilesArtifactValue> runfilesInputs = new HashMap<>();
private final List<RunfilesTree> runfilesTrees = new ArrayList<>();
@@ -40,6 +43,10 @@
inputs.put(artifact, metadata);
}
+ public void putTreeArtifact(ActionInput artifact, TreeArtifactValue metadata) {
+ treeArtifacts.put(artifact.getExecPath(), metadata);
+ }
+
public void putRunfilesTree(ActionInput middleman, RunfilesTree runfilesTree) {
RunfilesArtifactValue runfilesArtifactValue =
new RunfilesArtifactValue(
@@ -61,6 +68,12 @@
@Override
@Nullable
+ public TreeArtifactValue getTreeMetadata(PathFragment execPath) {
+ return treeArtifacts.get(execPath);
+ }
+
+ @Override
+ @Nullable
public RunfilesArtifactValue getRunfilesMetadata(ActionInput input) {
return runfilesInputs.get(input);
}
