commit c8dce99f7d4a5a6fdd129cdaab7c3434f4f5141a
author janakr <janakr@google.com> Mon Sep 25 21:48:27 2017 +0200
committer Vladimir Moskva <vladmos@google.com> Tue Sep 26 12:30:42 2017 +0200
tree eb22ba90f3dfedeff575381f4af3b46153da42bb
parent aa12ec3dc70797c3c514f9f08a976c6ad79ffd38

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

src/main/java/com/google/devtools/build/lib/analysis/BuildView.java

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 9abeb1a..cd641b1 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -326,7 +326,7 @@
 
   @VisibleForTesting
   WorkspaceStatusAction getLastWorkspaceBuildInfoActionForTesting() {
-    return skyframeExecutor.getLastWorkspaceStatusActionForTesting();
+    return skyframeExecutor.getLastWorkspaceStatusAction();
   }
 
   @Override
@@ -593,7 +593,7 @@
               target.getFirst(), target.getSecond(), aspectConfigurations.get(target)));
     }
 
-    skyframeExecutor.injectWorkspaceStatusData(loadingResult.getWorkspaceName());
+    skyframeExecutor.maybeInvalidateWorkspaceStatusValue(loadingResult.getWorkspaceName());
     SkyframeAnalysisResult skyframeAnalysisResult;
     try {
       skyframeAnalysisResult =