Stop injecting WorkspaceStatusAction into the Skyframe graph as a precomputed value. Instead, manually check if the value has changed, and if it has, invalidate its consuming WorkspaceStatusValue node, forcing its re-evaluation, where it will pick up the new value.
This seems more awkward than the original code, but it is more correct in spirit: injecting a precomputed value which can change even while the source state does not is a smell. Long-term, the key for the WorkspaceStatusValue should incorporate a hash of the action, and that hash should be in the configuration, just as other configuration flags are. That isn't possible right now just because we don't have configuration trimming, and we drop all nodes on configuration changes, so putting workspace status options into the configuration would lose change pruning whenever we changed workspace status options.
If/when those problems are fixed, we can extend this change to have WorkspaceStatusFunction continue to request the action out-of-band, but keyed by the hash. Then we can stop invalidating stale nodes.
See also https://github.com/bazelbuild/bazel/issues/3785.
PiperOrigin-RevId: 169947071
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index faee80d..5935aa6 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -712,15 +712,14 @@
}
/**
- * Returns the ConfiguredTarget for the specified label, using the
- * given build configuration. If the label corresponds to a target with a top-level configuration
- * transition, that transition is applied to the given config in the returned ConfiguredTarget.
+ * Returns the ConfiguredTarget for the specified label, using the given build configuration. If
+ * the label corresponds to a target with a top-level configuration transition, that transition is
+ * applied to the given config in the returned ConfiguredTarget.
*
- * <p>If the evaluation of the SkyKey corresponding to the configured target fails, this
- * method may return null. In that case, use a debugger to inspect the {@link ErrorInfo}
- * for the evaluation, which is produced by the
- * {@link MemoizingEvaluator#getExistingValueForTesting} call in
- * {@link SkyframeExecutor#getConfiguredTargetForTesting}. See also b/26382502.
+ * <p>If the evaluation of the SkyKey corresponding to the configured target fails, this method
+ * may return null. In that case, use a debugger to inspect the {@link ErrorInfo} for the
+ * evaluation, which is produced by the {@link MemoizingEvaluator#getExistingValue} call in {@link
+ * SkyframeExecutor#getConfiguredTargetForTesting}. See also b/26382502.
*/
protected ConfiguredTarget getConfiguredTarget(Label label, BuildConfiguration config) {
return view.getConfiguredTargetForTesting(reporter, BlazeTestUtils.convertLabel(label), config);
@@ -768,7 +767,7 @@
invalidatePackages();
// Need to re-initialize the workspace status.
- getSkyframeExecutor().injectWorkspaceStatusData("test");
+ getSkyframeExecutor().maybeInvalidateWorkspaceStatusValue("test");
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
index 5e2dba7..673fcfb 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
@@ -40,7 +40,6 @@
import com.google.devtools.build.lib.events.NullEventHandler;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.FileStatus;
-import com.google.devtools.build.lib.vfs.FileSystem.HashFunction;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -461,7 +460,7 @@
}
private void setGeneratingActions() {
- if (evaluator.getExistingValueForTesting(OWNER_KEY) == null) {
+ if (evaluator.getExistingValue(OWNER_KEY) == null) {
differencer.inject(
ImmutableMap.of(
OWNER_KEY,
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
index b243828..22033a3 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -227,7 +227,7 @@
return new Builder() {
private void setGeneratingActions() {
- if (evaluator.getExistingValueForTesting(OWNER_KEY) == null) {
+ if (evaluator.getExistingValue(OWNER_KEY) == null) {
differencer.inject(
ImmutableMap.of(
OWNER_KEY, new ActionLookupValue(ImmutableList.copyOf(actions), false)));
@@ -253,8 +253,8 @@
skyframeActionExecutor.prepareForExecution(
reporter,
executor,
- keepGoing, /*explain=*/
- false,
+ keepGoing,
+ /*explain=*/ false,
new ActionCacheChecker(actionCache, null, ALWAYS_EXECUTE_FILTER, null),
null);
skyframeActionExecutor.setActionExecutionProgressReportingObjects(
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
index b1d8b08..6b53a00 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
@@ -224,7 +224,7 @@
}
private void setGeneratingActions() {
- if (evaluator.getExistingValueForTesting(OWNER_KEY) == null) {
+ if (evaluator.getExistingValue(OWNER_KEY) == null) {
differencer.inject(
ImmutableMap.of(
OWNER_KEY,
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/util/SkyframeExecutorTestUtils.java b/src/test/java/com/google/devtools/build/lib/skyframe/util/SkyframeExecutorTestUtils.java
index 21ba998..30b0076 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/util/SkyframeExecutorTestUtils.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/util/SkyframeExecutorTestUtils.java
@@ -50,7 +50,7 @@
*/
@Nullable
public static SkyValue getExistingValue(SkyframeExecutor skyframeExecutor, SkyKey key) {
- return skyframeExecutor.getEvaluatorForTesting().getExistingValueForTesting(key);
+ return skyframeExecutor.getEvaluatorForTesting().getExistingValue(key);
}
/**
