Remove ActionMetadataHandler.transformAfterInputDiscovery().
It doesn't change anything in ActionMetadataHandler now that getInputMetadata()
does not throw on unknown inputs anymore.
RELNOTES: None.
PiperOrigin-RevId: 523978022
Change-Id: Id4837071a778c26f39cda8c703d8c0a1a3d01ad4
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index d9f7cf7..4106f10 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -748,7 +748,6 @@
tsgm.get(),
pathResolver,
skyframeActionExecutor.getExecRoot().asFragment(),
- PathFragment.create(directories.getRelativeOutputPath()),
expandedFilesets);
// We only need to check the action cache if we haven't done it on a previous run.
@@ -808,15 +807,12 @@
"Input discovery returned null but no more deps were requested by %s",
action);
}
- switch (addDiscoveredInputs(state, env, action)) {
- case VALUES_MISSING:
- return null;
- case NO_DISCOVERED_DATA:
- break;
- case DISCOVERED_DATA:
- metadataHandler = metadataHandler.transformAfterInputDiscovery(new OutputStore());
- metadataHandler.prepareForActionExecution();
+
+ addDiscoveredInputs(state, env, action);
+ if (env.valuesMissing()) {
+ return null;
}
+
// When discover inputs completes, post an event with the duration values.
env.getListener()
.post(
@@ -866,17 +862,9 @@
state.getExpandedFilesets();
if (action.discoversInputs()) {
state.discoveredInputs = action.getInputs();
- switch (addDiscoveredInputs(state, env, action)) {
- case VALUES_MISSING:
- return;
- case NO_DISCOVERED_DATA:
- break;
- case DISCOVERED_DATA:
- // We are in the interesting case of an action that discovered its inputs during
- // execution, and found some new ones, but the new ones were already present in the
- // graph. We must therefore cache the metadata for those new ones.
- metadataHandler =
- metadataHandler.transformAfterInputDiscovery(metadataHandler.getOutputStore());
+ addDiscoveredInputs(state, env, action);
+ if (env.valuesMissing()) {
+ return;
}
}
checkState(!env.valuesMissing(), action);
@@ -895,13 +883,7 @@
}
}
- private enum DiscoveredState {
- VALUES_MISSING,
- NO_DISCOVERED_DATA,
- DISCOVERED_DATA
- }
-
- private DiscoveredState addDiscoveredInputs(
+ private void addDiscoveredInputs(
InputDiscoveryState state, Environment env, Action actionForError)
throws InterruptedException, ActionExecutionException {
// TODO(janakr): This code's assumptions are wrong in the face of Starlark actions with unused
@@ -919,7 +901,7 @@
}
if (unknownDiscoveredInputs.isEmpty()) {
- return DiscoveredState.NO_DISCOVERED_DATA;
+ return;
}
SkyframeLookupResult nonMandatoryDiscovered =
@@ -980,7 +962,6 @@
"unknown metadata for " + input.getExecPathString() + ": " + retrievedMetadata);
}
}
- return env.valuesMissing() ? DiscoveredState.VALUES_MISSING : DiscoveredState.DISCOVERED_DATA;
}
@Nullable
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
index 43d4f99..d439ebf 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -85,11 +85,6 @@
/**
* Creates a new metadata handler.
- *
- * <p>If the handler is for use during input discovery, calling {@link #getMetadata} with an
- * artifact which is neither in {@code inputArtifactData} nor {@code outputs} is tolerated and
- * will return {@code null}. To subsequently transform the handler for regular action execution
- * (where such a call is not permitted), use {@link #transformAfterInputDiscovery}.
*/
static ActionMetadataHandler create(
ActionInputMap inputArtifactData,
@@ -100,7 +95,6 @@
TimestampGranularityMonitor tsgm,
ArtifactPathResolver artifactPathResolver,
PathFragment execRoot,
- PathFragment derivedPathPrefix,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets) {
return new ActionMetadataHandler(
inputArtifactData,
@@ -111,7 +105,6 @@
tsgm,
artifactPathResolver,
execRoot,
- derivedPathPrefix,
createFilesetMapping(expandedFilesets, execRoot),
new OutputStore());
}
@@ -128,7 +121,6 @@
private final TimestampGranularityMonitor tsgm;
private final ArtifactPathResolver artifactPathResolver;
private final PathFragment execRoot;
- private final PathFragment derivedPathPrefix;
private final AtomicBoolean executionMode = new AtomicBoolean(false);
private final OutputStore store;
@@ -142,7 +134,6 @@
TimestampGranularityMonitor tsgm,
ArtifactPathResolver artifactPathResolver,
PathFragment execRoot,
- PathFragment derivedPathPrefix,
ImmutableMap<PathFragment, FileArtifactValue> filesetMapping,
OutputStore store) {
this.inputArtifactData = checkNotNull(inputArtifactData);
@@ -153,38 +144,11 @@
this.tsgm = checkNotNull(tsgm);
this.artifactPathResolver = checkNotNull(artifactPathResolver);
this.execRoot = checkNotNull(execRoot);
- this.derivedPathPrefix = checkNotNull(derivedPathPrefix);
this.filesetMapping = checkNotNull(filesetMapping);
this.store = checkNotNull(store);
}
/**
- * Returns a new handler mostly identical to this one, except uses the given {@code store} and
- * does not permit {@link #getMetadata} to be called with an artifact which is neither in inputs
- * nor outputs.
- *
- * <p>The returned handler will be in the mode for action cache checking. To prepare it for action
- * execution, call {@link #prepareForActionExecution}.
- *
- * <p>This method is designed to be called after input discovery when a fresh handler is needed
- * but all of the parameters in {@link #create} would be the same as the original handler.
- */
- ActionMetadataHandler transformAfterInputDiscovery(OutputStore store) {
- return new ActionMetadataHandler(
- inputArtifactData,
- archivedTreeArtifactsEnabled,
- outputPermissions,
- outputs,
- xattrProvider,
- tsgm,
- artifactPathResolver,
- execRoot,
- derivedPathPrefix,
- filesetMapping,
- store);
- }
-
- /**
* If {@code value} represents an existing file, returns it as is, otherwise throws {@link
* FileNotFoundException}.
*/
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
index 7b8be7e..1b574cb 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
@@ -103,7 +103,6 @@
tsgm,
ArtifactPathResolver.IDENTITY,
execRoot.asFragment(),
- derivedPathPrefix,
/* expandedFilesets= */ ImmutableMap.of());
}
@@ -420,7 +419,6 @@
tsgm,
ArtifactPathResolver.IDENTITY,
execRoot.asFragment(),
- derivedPathPrefix,
expandedFilesets);
// Only the regular FileArtifactValue should have its metadata stored.
@@ -530,7 +528,6 @@
tsgm,
ArtifactPathResolver.IDENTITY,
execRoot.asFragment(),
- derivedPathPrefix,
/* expandedFilesets= */ ImmutableMap.of());
handler.prepareForActionExecution();
@@ -582,37 +579,6 @@
}
@Test
- public void transformAfterInputDiscovery() throws Exception {
- Artifact known =
- ActionsTestUtil.createArtifactWithRootRelativePath(
- outputRoot, PathFragment.create("known"));
- Artifact unknown =
- ActionsTestUtil.createArtifactWithRootRelativePath(
- outputRoot, PathFragment.create("unknown"));
- ActionMetadataHandler handler =
- createHandler(
- new ActionInputMap(0), /* forInputDiscovery= *//* outputs= */ ImmutableSet.of(known));
-
- // Unknown artifact returns null during input discovery.
- assertThat(handler.getInputMetadata(unknown)).isNull();
-
- OutputStore newStore = new OutputStore();
- FileArtifactValue knownMetadata =
- RemoteFileArtifactValue.create(new byte[] {1, 2, 3}, 5, 1, /* expireAtEpochMilli= */ -1);
- newStore.putArtifactData(known, knownMetadata);
- ActionMetadataHandler newHandler = handler.transformAfterInputDiscovery(newStore);
- assertThat(newHandler.getOutputStore()).isNotEqualTo(handler.getOutputStore());
- assertThat(newHandler.getOutputStore()).isEqualTo(newStore);
-
- assertThat(newHandler.getOutputMetadata(known)).isEqualTo(knownMetadata);
- // Unknown artifact throws outside of input discovery.
- // assertThrows(IllegalStateException.class, () -> newHandler.getInputMetadata(unknown));
- // We can transform it again.
- assertThat(newHandler.transformAfterInputDiscovery(new OutputStore())).isNotNull();
- assertThat(chmodCalls).isEmpty();
- }
-
- @Test
public void getTreeArtifactChildren_noData_returnsEmptySet() {
SpecialArtifact treeArtifact =
ActionsTestUtil.createTreeArtifactWithGeneratingAction(
