When rewinding actions that produce tree artifacts, invalidate the cached directory creation.
Since the action is to be rewound, it will completely re-execute including `Action#prepare`, which deletes its outputs including tree directories. Invalidate these directories in the `knownDirectories` cache so that they are created again.
This change only affects builds which use rewinding without an in-memory action filesystem.
This will soon be integration tested as part of b/229395817 - in fact, that's how I encountered this bug.
PiperOrigin-RevId: 455250588
Change-Id: I5ca188c95d4528d7fbbfcd108b0b2192f2321d8d
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputDirectoryHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputDirectoryHelper.java
index 53f9b59..8d2a745 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputDirectoryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputDirectoryHelper.java
@@ -60,9 +60,11 @@
}
/**
- * Create output directories for an ActionFS. The action-local filesystem starts empty, so we
- * expect the output directory creation to always succeed. There can be no interference from state
- * left behind by prior builds or other actions intra-build.
+ * Creates output directories for an {@link
+ * com.google.devtools.build.lib.vfs.OutputService.ActionFileSystemType#IN_MEMORY_FILE_SYSTEM}.
+ * The action-local filesystem starts empty, so we expect the output directory creation to always
+ * succeed. There can be no interference from state left behind by prior builds or other actions
+ * intra-build.
*/
void createActionFsOutputDirectories(
ImmutableSet<Artifact> actionOutputs, ArtifactPathResolver artifactPathResolver)
@@ -87,6 +89,25 @@
}
/**
+ * Called to invalidate the cached creation of tree artifact directories when an action is going
+ * to be rewound.
+ *
+ * <p>We use {@link #knownDirectories} to only create an output directory once per build. With
+ * rewinding, actions that output tree artifacts need to recreate the directories because they are
+ * deleted as part of the {@link com.google.devtools.build.lib.actions.Action#prepare} step.
+ *
+ * <p>Note that this does not need to be called if using {@link
+ * com.google.devtools.build.lib.vfs.OutputService.ActionFileSystemType#IN_MEMORY_FILE_SYSTEM}.
+ */
+ void invalidateTreeArtifactDirectoryCreation(ImmutableSet<Artifact> actionOutputs) {
+ for (Artifact output : actionOutputs) {
+ if (output.isTreeArtifact()) {
+ knownDirectories.remove(output.getPath().asFragment());
+ }
+ }
+ }
+
+ /**
* Creates output directories, with ancestors, for all action outputs.
*
* <p>For regular file outputs, creates the parent directories, for tree outputs creates the tree
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index 5cd3034..5b0610b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -390,6 +390,9 @@
actionExecutionState.obsolete(actionLookupData, buildActionMap, ownerlessArtifactWrapper);
}
completedAndResetActions.add(ownerlessArtifactWrapper);
+ if (!actionFileSystemType().inMemoryFileSystem()) {
+ outputDirectoryHelper.invalidateTreeArtifactDirectoryCreation(action.getOutputs());
+ }
}
@Nullable
@@ -412,6 +415,9 @@
if (!lostDiscoveredInputs.isEmpty()) {
lostDiscoveredInputsMap.put(ownerlessArtifactWrapper, lostDiscoveredInputs);
}
+ if (!actionFileSystemType().inMemoryFileSystem()) {
+ outputDirectoryHelper.invalidateTreeArtifactDirectoryCreation(action.getOutputs());
+ }
}
void noteActionEvaluationStarted(ActionLookupData actionLookupData, Action action) {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputDirectoryHelperTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputDirectoryHelperTest.java
index 9d21806..a679135 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputDirectoryHelperTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputDirectoryHelperTest.java
@@ -199,6 +199,25 @@
assertThat(e).hasCauseThat().isSameInstanceAs(injectedException);
}
+ @Test
+ public void invalidateTreeArtifactDirectoryCreation_onlyInvalidatesTreeArtifactDirs()
+ throws Exception {
+ ActionOutputDirectoryHelper outputDirectoryHelper = createActionOutputDirectoryHelper();
+ Artifact regularOutput = createOutput("example/regular/file.txt");
+ SpecialArtifact treeOutput = createTreeOutput("example/tree/tree_dir");
+ ImmutableSet<Artifact> outputs = ImmutableSet.of(regularOutput, treeOutput);
+
+ outputDirectoryHelper.createOutputDirectories(outputs);
+ regularOutput.getPath().getParentDirectory().deleteTree();
+ treeOutput.getPath().deleteTree();
+ outputDirectoryHelper.invalidateTreeArtifactDirectoryCreation(outputs);
+ outputDirectoryHelper.createOutputDirectories(outputs);
+
+ // Only tree artifact directories are recreated.
+ assertThat(regularOutput.getPath().getParentDirectory().exists()).isFalse();
+ assertThat(treeOutput.getPath().exists()).isTrue();
+ }
+
private FileSystem createFileSystemInjectingException(
PathFragment failingPath, IOException injectedException) {
return new DelegateFileSystem(execRoot.getFileSystem()) {
