diff options
author | 2017-09-25 21:48:27 +0200 | |
---|---|---|
committer | 2017-09-26 12:30:42 +0200 | |
commit | c8dce99f7d4a5a6fdd129cdaab7c3434f4f5141a (patch) | |
tree | eb22ba90f3dfedeff575381f4af3b46153da42bb /src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java | |
parent | aa12ec3dc70797c3c514f9f08a976c6ad79ffd38 (diff) |
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
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java | 21 |
1 files changed, 16 insertions, 5 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java index d665632beb..5bed654a01 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java @@ -13,31 +13,42 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; -import com.google.common.base.Supplier; import com.google.devtools.build.lib.analysis.WorkspaceStatusAction; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import java.util.function.Supplier; /** Creates the workspace status artifacts and action. */ public class WorkspaceStatusFunction implements SkyFunction { + interface WorkspaceStatusActionFactory { + WorkspaceStatusAction create(String workspaceName); + } + private final Supplier<Boolean> removeActionAfterEvaluation; + private final WorkspaceStatusActionFactory workspaceStatusActionFactory; - WorkspaceStatusFunction(Supplier<Boolean> removeActionAfterEvaluation) { + WorkspaceStatusFunction( + Supplier<Boolean> removeActionAfterEvaluation, + WorkspaceStatusActionFactory workspaceStatusActionFactory) { this.removeActionAfterEvaluation = Preconditions.checkNotNull(removeActionAfterEvaluation); + this.workspaceStatusActionFactory = workspaceStatusActionFactory; } @Override public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException { Preconditions.checkState( WorkspaceStatusValue.SKY_KEY.equals(skyKey), WorkspaceStatusValue.SKY_KEY); - - WorkspaceStatusAction action = PrecomputedValue.WORKSPACE_STATUS_KEY.get(env); - if (action == null) { + WorkspaceNameValue workspaceNameValue = + (WorkspaceNameValue) env.getValue(WorkspaceNameValue.key()); + if (env.valuesMissing()) { return null; } + WorkspaceStatusAction action = + workspaceStatusActionFactory.create(workspaceNameValue.getName()); + return new WorkspaceStatusValue( action.getStableStatus(), action.getVolatileStatus(), |