Fail gracefully on conflicting actions generated by an aspect. These can come from Skylark, so we shouldn't crash. As a safety measure, subclasses of ActionLookupValue are now responsible for detecting action conflicts themselves.
PiperOrigin-RevId: 187095271
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
index 8ed14d1..510b0b1 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
@@ -20,7 +20,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
-import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.skyframe.SkyKey;
@@ -39,34 +38,14 @@
protected final List<ActionAnalysisMetadata> actions;
private final ImmutableMap<Artifact, Integer> generatingActionIndex;
- private static Actions.GeneratingActions filterSharedActionsAndThrowRuntimeIfConflict(
- ActionKeyContext actionKeyContext, List<ActionAnalysisMetadata> actions) {
- try {
- return Actions.filterSharedActionsAndThrowActionConflict(actionKeyContext, actions);
- } catch (ActionConflictException e) {
- // Programming bug.
- throw new IllegalStateException(e);
- }
+ protected ActionLookupValue(
+ ActionAnalysisMetadata action,
+ boolean removeActionAfterEvaluation) {
+ this(Actions.GeneratingActions.fromSingleAction(action), removeActionAfterEvaluation);
}
@VisibleForTesting
public ActionLookupValue(
- ActionKeyContext actionKeyContext,
- List<ActionAnalysisMetadata> actions,
- boolean removeActionsAfterEvaluation) {
- this(
- filterSharedActionsAndThrowRuntimeIfConflict(actionKeyContext, actions),
- removeActionsAfterEvaluation);
- }
-
- protected ActionLookupValue(
- ActionKeyContext actionKeyContext,
- ActionAnalysisMetadata action,
- boolean removeActionAfterEvaluation) {
- this(actionKeyContext, ImmutableList.of(action), removeActionAfterEvaluation);
- }
-
- protected ActionLookupValue(
Actions.GeneratingActions generatingActions, boolean removeActionsAfterEvaluation) {
if (removeActionsAfterEvaluation) {
this.actions = new ArrayList<>(generatingActions.getActions());
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Actions.java b/src/main/java/com/google/devtools/build/lib/actions/Actions.java
index 9b49d5c..4336db0 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Actions.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Actions.java
@@ -14,8 +14,8 @@
package com.google.devtools.build.lib.actions;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.escape.Escaper;
@@ -33,6 +33,7 @@
import java.util.Map;
import java.util.SortedMap;
import java.util.TreeMap;
+import java.util.function.Function;
import javax.annotation.Nullable;
/**
@@ -110,7 +111,7 @@
* output
*/
public static GeneratingActions filterSharedActionsAndThrowActionConflict(
- ActionKeyContext actionKeyContext, List<ActionAnalysisMetadata> actions)
+ ActionKeyContext actionKeyContext, List<? extends ActionAnalysisMetadata> actions)
throws ActionConflictException {
return Actions.maybeFilterSharedActionsAndThrowIfConflict(
actionKeyContext, actions, /*allowSharedAction=*/ true);
@@ -118,7 +119,7 @@
private static GeneratingActions maybeFilterSharedActionsAndThrowIfConflict(
ActionKeyContext actionKeyContext,
- List<ActionAnalysisMetadata> actions,
+ List<? extends ActionAnalysisMetadata> actions,
boolean allowSharedAction)
throws ActionConflictException {
Map<Artifact, Integer> generatingActions = new HashMap<>();
@@ -309,24 +310,35 @@
}
/** Container class for actions and the artifacts they generate. */
- @VisibleForTesting
public static class GeneratingActions {
- private final List<ActionAnalysisMetadata> actions;
+ public static final GeneratingActions EMPTY =
+ new GeneratingActions(ImmutableList.of(), ImmutableMap.of());
+
+ private final List<? extends ActionAnalysisMetadata> actions;
private final ImmutableMap<Artifact, Integer> generatingActionIndex;
- @VisibleForTesting
- public GeneratingActions(
- List<ActionAnalysisMetadata> actions,
+ private GeneratingActions(
+ List<? extends ActionAnalysisMetadata> actions,
ImmutableMap<Artifact, Integer> generatingActionIndex) {
this.actions = actions;
this.generatingActionIndex = generatingActionIndex;
}
+ public static GeneratingActions fromSingleAction(ActionAnalysisMetadata action) {
+ return new GeneratingActions(
+ ImmutableList.of(action),
+ ImmutableMap.copyOf(
+ action
+ .getOutputs()
+ .stream()
+ .collect(ImmutableMap.toImmutableMap(Function.identity(), (a) -> 0))));
+ }
+
public ImmutableMap<Artifact, Integer> getGeneratingActionIndex() {
return generatingActionIndex;
}
- public List<ActionAnalysisMetadata> getActions() {
+ public List<? extends ActionAnalysisMetadata> getActions() {
return actions;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java
index e3081aa..d49d533 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java
@@ -73,14 +73,14 @@
return null;
}
Iterable<TreeFileArtifact> inputTreeFileArtifacts = treeArtifactValue.getChildren();
- Iterable<Action> expandedActions;
+ GeneratingActions generatingActions;
try {
// Expand the action template using the list of expanded input TreeFileArtifacts.
- expandedActions = ImmutableList.<Action>copyOf(
- actionTemplate.generateActionForInputArtifacts(inputTreeFileArtifacts, key));
// TODO(rduan): Add a check to verify the inputs of expanded actions are subsets of inputs
// of the ActionTemplate.
- checkActionAndArtifactConflicts(expandedActions);
+ generatingActions =
+ checkActionAndArtifactConflicts(
+ actionTemplate.generateActionForInputArtifacts(inputTreeFileArtifacts, key));
} catch (ActionConflictException e) {
e.reportTo(env.getListener());
throw new ActionTemplateExpansionFunctionException(e);
@@ -92,8 +92,7 @@
throw new ActionTemplateExpansionFunctionException(e);
}
- return new ActionTemplateExpansionValue(
- actionKeyContext, expandedActions, removeActionsAfterEvaluation.get());
+ return new ActionTemplateExpansionValue(generatingActions, removeActionsAfterEvaluation.get());
}
/** Exception thrown by {@link ActionTemplateExpansionFunction}. */
@@ -111,7 +110,7 @@
}
}
- private void checkActionAndArtifactConflicts(Iterable<Action> actions)
+ private GeneratingActions checkActionAndArtifactConflicts(Iterable<? extends Action> actions)
throws ActionConflictException, ArtifactPrefixConflictException {
GeneratingActions generatingActions =
Actions.findAndThrowActionConflict(actionKeyContext, ImmutableList.copyOf(actions));
@@ -123,6 +122,7 @@
if (!artifactPrefixConflictMap.isEmpty()) {
throw artifactPrefixConflictMap.values().iterator().next();
}
+ return generatingActions;
}
@Nullable
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java
index 8ba11ee..0536e82 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java
@@ -14,11 +14,9 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.base.MoreObjects;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.Interner;
-import com.google.devtools.build.lib.actions.Action;
-import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue;
+import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
@@ -30,10 +28,8 @@
*/
public final class ActionTemplateExpansionValue extends ActionLookupValue {
ActionTemplateExpansionValue(
- ActionKeyContext actionKeyContext,
- Iterable<Action> expandedActions,
- boolean removeActionsAfterEvaluation) {
- super(actionKeyContext, ImmutableList.copyOf(expandedActions), removeActionsAfterEvaluation);
+ GeneratingActions generatingActions, boolean removeActionsAfterEvaluation) {
+ super(generatingActions, removeActionsAfterEvaluation);
}
static ActionTemplateExpansionKey key(ActionLookupKey actionLookupKey, int actionIndex) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index 9d3f49c..0f3b752 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -19,7 +19,9 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.actions.ActionKeyContext;
+import com.google.devtools.build.lib.actions.Actions;
+import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.AliasProvider;
import com.google.devtools.build.lib.analysis.AspectResolver;
import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment;
@@ -274,7 +276,6 @@
if (baseConfiguredTargetValue.getConfiguredTarget().getProvider(AliasProvider.class) != null) {
return createAliasAspect(
env,
- view.getActionKeyContext(),
associatedConfiguredTargetAndTarget.getTarget(),
aspect,
key,
@@ -394,7 +395,6 @@
return createAspect(
env,
- view.getActionKeyContext(),
key,
aspectPath,
aspect,
@@ -478,7 +478,6 @@
private SkyValue createAliasAspect(
Environment env,
- ActionKeyContext actionKeyContext,
Target originalTarget,
Aspect aspect,
AspectKey originalKey,
@@ -513,8 +512,7 @@
originalTarget.getLabel(),
originalTarget.getLocation(),
ConfiguredAspect.forAlias(real.getConfiguredAspect()),
- actionKeyContext,
- ImmutableList.of(),
+ GeneratingActions.EMPTY,
transitivePackagesForPackageRootResolution,
removeActionsAfterEvaluation.get());
}
@@ -522,7 +520,6 @@
@Nullable
private AspectValue createAspect(
Environment env,
- ActionKeyContext actionKeyContext,
AspectKey key,
ImmutableList<Aspect> aspectPath,
Aspect aspect,
@@ -587,14 +584,23 @@
analysisEnvironment.disable(associatedTarget.getTarget());
Preconditions.checkNotNull(configuredAspect);
+ GeneratingActions generatingActions;
+ // Check for conflicting actions within this aspect (indicates a bug in the implementation).
+ try {
+ generatingActions =
+ Actions.filterSharedActionsAndThrowActionConflict(
+ analysisEnvironment.getActionKeyContext(),
+ analysisEnvironment.getRegisteredActions());
+ } catch (ActionConflictException e) {
+ throw new AspectFunctionException(e);
+ }
return new AspectValue(
key,
aspect,
associatedTarget.getTarget().getLabel(),
associatedTarget.getTarget().getLocation(),
configuredAspect,
- actionKeyContext,
- ImmutableList.copyOf(analysisEnvironment.getRegisteredActions()),
+ generatingActions,
transitivePackagesForPackageRootResolution == null
? null
: transitivePackagesForPackageRootResolution.build(),
@@ -661,7 +667,7 @@
super(e, Transience.PERSISTENT);
}
- public AspectFunctionException(InconsistentAspectOrderException cause) {
+ public AspectFunctionException(ActionConflictException cause) {
super(cause, Transience.PERSISTENT);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index 0186315..73ce248 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -19,9 +19,8 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Interner;
-import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
-import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue;
+import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
import com.google.devtools.build.lib.analysis.ConfiguredAspect;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.cmdline.Label;
@@ -39,7 +38,6 @@
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.syntax.SkylarkImport;
import com.google.devtools.build.skyframe.SkyFunctionName;
-import java.util.List;
import javax.annotation.Nullable;
/**
@@ -444,11 +442,10 @@
Label label,
Location location,
ConfiguredAspect configuredAspect,
- ActionKeyContext actionKeyContext,
- List<ActionAnalysisMetadata> actions,
+ GeneratingActions actions,
NestedSet<Package> transitivePackagesForPackageRootResolution,
boolean removeActionsAfterEvaluation) {
- super(actionKeyContext, actions, removeActionsAfterEvaluation);
+ super(actions, removeActionsAfterEvaluation);
this.label = Preconditions.checkNotNull(label, actions);
this.aspect = Preconditions.checkNotNull(aspect, label);
this.location = Preconditions.checkNotNull(location, label);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
index e8230a6..0794901 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
@@ -18,9 +18,13 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionKeyContext;
+import com.google.devtools.build.lib.actions.Actions;
+import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactFactory;
import com.google.devtools.build.lib.actions.ArtifactRoot;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
+import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoCollection;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoContext;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoKey;
@@ -85,9 +89,7 @@
: factory.getDerivedArtifact(rootRelativePath, root, keyAndConfig);
}
};
-
- return new BuildInfoCollectionValue(
- actionKeyContext,
+ BuildInfoCollection collection =
buildInfoFactories
.get(keyAndConfig.getInfoKey())
.create(
@@ -96,8 +98,17 @@
.getConfiguration(),
infoArtifactValue.getStableArtifact(),
infoArtifactValue.getVolatileArtifact(),
- repositoryName),
- removeActionsAfterEvaluation.get());
+ repositoryName);
+ GeneratingActions generatingActions;
+ try {
+ generatingActions =
+ Actions.filterSharedActionsAndThrowActionConflict(
+ actionKeyContext, collection.getActions());
+ } catch (ActionConflictException e) {
+ throw new IllegalStateException("Action conflicts not expected in build info: " + skyKey, e);
+ }
+ return new BuildInfoCollectionValue(
+ collection, generatingActions, removeActionsAfterEvaluation.get());
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java
index b96e94b..7d3fe84 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java
@@ -15,8 +15,8 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.Interner;
-import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue;
+import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoCollection;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -35,10 +35,10 @@
private final BuildInfoCollection collection;
BuildInfoCollectionValue(
- ActionKeyContext actionKeyContext,
BuildInfoCollection collection,
+ GeneratingActions generatingActions,
boolean removeActionsAfterEvaluation) {
- super(actionKeyContext, collection.getActions(), removeActionsAfterEvaluation);
+ super(generatingActions, removeActionsAfterEvaluation);
this.collection = collection;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
index 3fbf2d9..b1e11dc 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
@@ -20,7 +20,10 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionKeyContext;
+import com.google.devtools.build.lib.actions.Actions;
+import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
@@ -57,7 +60,14 @@
outputs.addAll(action.getOutputs());
}
- return new CoverageReportValue(actionKeyContext, actions, removeActionsAfterEvaluation.get());
+ GeneratingActions generatingActions;
+ try {
+ generatingActions =
+ Actions.filterSharedActionsAndThrowActionConflict(actionKeyContext, actions);
+ } catch (ActionConflictException e) {
+ throw new IllegalStateException("Action conflicts not expected in coverage: " + skyKey, e);
+ }
+ return new CoverageReportValue(generatingActions, removeActionsAfterEvaluation.get());
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java
index aa35490..edb9226 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java
@@ -14,10 +14,8 @@
package com.google.devtools.build.lib.skyframe;
-import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
-import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue;
+import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.skyframe.SkyFunctionName;
@@ -30,11 +28,8 @@
// There should only ever be one CoverageReportValue value in the graph.
public static final CoverageReportKey COVERAGE_REPORT_KEY = CoverageReportKey.INSTANCE;
- CoverageReportValue(
- ActionKeyContext actionKeyContext,
- ImmutableList<ActionAnalysisMetadata> coverageReportActions,
- boolean removeActionsAfterEvaluation) {
- super(actionKeyContext, coverageReportActions, removeActionsAfterEvaluation);
+ CoverageReportValue(GeneratingActions generatingActions, boolean removeActionsAfterEvaluation) {
+ super(generatingActions, removeActionsAfterEvaluation);
}
@AutoCodec(strategy = AutoCodec.Strategy.SINGLETON)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 0b65ba7..38ad081 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -453,8 +453,7 @@
actionKeyContext, artifactFactory, buildInfoFactories, removeActionsAfterEvaluation));
map.put(
SkyFunctions.BUILD_INFO,
- new WorkspaceStatusFunction(
- actionKeyContext, removeActionsAfterEvaluation, this::makeWorkspaceStatusAction));
+ new WorkspaceStatusFunction(removeActionsAfterEvaluation, this::makeWorkspaceStatusAction));
map.put(
SkyFunctions.COVERAGE_REPORT,
new CoverageReportFunction(actionKeyContext, removeActionsAfterEvaluation));
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 b231bae..68e6550 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
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.base.Preconditions;
-import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
@@ -27,15 +26,12 @@
WorkspaceStatusAction create(String workspaceName);
}
- private final ActionKeyContext actionKeyContext;
private final Supplier<Boolean> removeActionAfterEvaluation;
private final WorkspaceStatusActionFactory workspaceStatusActionFactory;
WorkspaceStatusFunction(
- ActionKeyContext actionKeyContext,
Supplier<Boolean> removeActionAfterEvaluation,
WorkspaceStatusActionFactory workspaceStatusActionFactory) {
- this.actionKeyContext = actionKeyContext;
this.removeActionAfterEvaluation = Preconditions.checkNotNull(removeActionAfterEvaluation);
this.workspaceStatusActionFactory = workspaceStatusActionFactory;
}
@@ -54,7 +50,6 @@
workspaceStatusActionFactory.create(workspaceNameValue.getName());
return new WorkspaceStatusValue(
- actionKeyContext,
action.getStableStatus(),
action.getVolatileStatus(),
action,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
index a628fb6..02030e4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
@@ -36,12 +35,11 @@
public static final BuildInfoKey BUILD_INFO_KEY = BuildInfoKey.INSTANCE;
WorkspaceStatusValue(
- ActionKeyContext actionKeyContext,
Artifact stableArtifact,
Artifact volatileArtifact,
WorkspaceStatusAction action,
boolean removeActionAfterEvaluation) {
- super(actionKeyContext, action, removeActionAfterEvaluation);
+ super(action, removeActionAfterEvaluation);
this.stableArtifact = stableArtifact;
this.volatileArtifact = volatileArtifact;
}
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionLookupValueTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionLookupValueTest.java
index e2ddd4f..f6c5961 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ActionLookupValueTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ActionLookupValueTest.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -47,7 +48,7 @@
Artifact artifact = mock(Artifact.class);
when(action.getOutputs()).thenReturn(ImmutableSet.of(artifact));
when(action.canRemoveAfterExecution()).thenReturn(true);
- ActionLookupValue underTest = new ActionLookupValue(actionKeyContext, action, false);
+ ActionLookupValue underTest = new ActionLookupValue(action, false);
assertThat(underTest.getGeneratingActionIndex(artifact)).isEqualTo(0);
assertThat(underTest.getAction(0)).isSameAs(action);
underTest.actionEvaluated(0, action);
@@ -55,7 +56,7 @@
}
@Test
- public void testActionNotPresentAfterEvaluation() {
+ public void testActionNotPresentAfterEvaluation() throws ActionConflictException {
Path execRoot = fs.getPath("/execroot");
Path outputRootPath = execRoot.getRelative("blaze-out");
ArtifactRoot root = ArtifactRoot.asDerivedRoot(execRoot, outputRootPath);
@@ -69,7 +70,9 @@
when(persistentAction.canRemoveAfterExecution()).thenReturn(false);
ActionLookupValue underTest =
new ActionLookupValue(
- actionKeyContext, ImmutableList.of(normalAction, persistentAction), true);
+ Actions.filterSharedActionsAndThrowActionConflict(
+ actionKeyContext, ImmutableList.of(normalAction, persistentAction)),
+ true);
assertThat(underTest.getGeneratingActionIndex(normalArtifact)).isEqualTo(0);
assertThat(underTest.getAction(0)).isSameAs(normalAction);
assertThat(underTest.getGeneratingActionIndex(persistentOutput)).isEqualTo(1);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
index dba65eb..ba1a3c2 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
@@ -196,9 +196,7 @@
ConfiguredTargetKey.of(Label.parseAbsoluteUnchecked("//foo:foo"), null);
private List<Action> evaluate(SpawnActionTemplate spawnActionTemplate) throws Exception {
- ConfiguredTargetValue ctValue =
- createConfiguredTargetValue(
- spawnActionTemplate.getOutputTreeArtifact(), spawnActionTemplate);
+ ConfiguredTargetValue ctValue = createConfiguredTargetValue(spawnActionTemplate);
differencer.inject(CTKEY, ctValue);
ActionTemplateExpansionKey templateKey = ActionTemplateExpansionValue.key(CTKEY, 0);
@@ -220,11 +218,10 @@
}
private static ConfiguredTargetValue createConfiguredTargetValue(
- Artifact artifact, ActionTemplate<?> actionTemplate) {
+ ActionTemplate<?> actionTemplate) {
return new ConfiguredTargetValue(
Mockito.mock(ConfiguredTarget.class),
- new Actions.GeneratingActions(
- ImmutableList.of(actionTemplate), ImmutableMap.of(artifact, 0)),
+ Actions.GeneratingActions.fromSingleAction(actionTemplate),
NestedSetBuilder.<Package>stableOrder().build(),
/*removeActionsAfterEvaluation=*/ false);
}
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 0a8c460..8a4f3af 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
@@ -22,17 +22,18 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Action;
-import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType;
import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.ActionLookupValue;
+import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.MissingInputFileException;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.actions.util.TestAction.DummyAction;
import com.google.devtools.build.lib.events.NullEventHandler;
@@ -318,18 +319,20 @@
return result.get(key);
}
- private void setGeneratingActions() throws InterruptedException {
+ private void setGeneratingActions() throws InterruptedException, ActionConflictException {
if (evaluator.getExistingValue(ALL_OWNER) == null) {
differencer.inject(
ImmutableMap.of(
ALL_OWNER,
new ActionLookupValue(
- actionKeyContext, ImmutableList.<ActionAnalysisMetadata>copyOf(actions), false)));
+ Actions.filterSharedActionsAndThrowActionConflict(
+ actionKeyContext, ImmutableList.copyOf(actions)),
+ false)));
}
}
private <E extends SkyValue> EvaluationResult<E> evaluate(SkyKey... keys)
- throws InterruptedException {
+ throws InterruptedException, ActionConflictException {
setGeneratingActions();
return driver.evaluate(
Arrays.asList(keys),
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 278f1d0..d2aca56 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
@@ -38,10 +38,12 @@
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.ActionResult;
+import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.BuildFailedException;
import com.google.devtools.build.lib.actions.Executor;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.actions.ResourceManager;
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.actions.TestExecException;
@@ -233,12 +235,15 @@
PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
return new Builder() {
- private void setGeneratingActions() {
+ private void setGeneratingActions() throws ActionConflictException {
if (evaluator.getExistingValue(ACTION_LOOKUP_KEY) == null) {
differencer.inject(
ImmutableMap.of(
ACTION_LOOKUP_KEY,
- new ActionLookupValue(actionKeyContext, ImmutableList.copyOf(actions), false)));
+ new ActionLookupValue(
+ Actions.filterSharedActionsAndThrowActionConflict(
+ actionKeyContext, ImmutableList.copyOf(actions)),
+ false)));
}
}
@@ -274,7 +279,11 @@
keys.add(ArtifactSkyKey.key(artifact, true));
}
- setGeneratingActions();
+ try {
+ setGeneratingActions();
+ } catch (ActionConflictException e) {
+ throw new IllegalStateException(e);
+ }
EvaluationResult<SkyValue> result = driver.evaluate(keys, keepGoing, threadCount, reporter);
if (result.hasError()) {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunctionTest.java
index 4f8b3bc..0194819 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunctionTest.java
@@ -22,7 +22,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.testing.EqualsTester;
-import com.google.devtools.build.lib.actions.Actions;
+import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
import com.google.devtools.build.lib.cmdline.Label;
@@ -48,7 +48,7 @@
ConfiguredTarget configuredTarget) {
return new ConfiguredTargetValue(
configuredTarget,
- new Actions.GeneratingActions(ImmutableList.of(), ImmutableMap.of()),
+ GeneratingActions.EMPTY,
NestedSetBuilder.emptySet(Order.STABLE_ORDER),
/*removeActionsAfterEvaluation=*/ false);
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
index 411fef6..b15782c 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
@@ -34,12 +34,14 @@
import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionResult;
+import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.BuildFailedException;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.actions.cache.MetadataHandler;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.actions.util.TestAction;
@@ -1232,7 +1234,12 @@
@Override
public SkyValue compute(SkyKey skyKey, Environment env) {
- return new ActionTemplateExpansionValue(actionKeyContext, actions, false);
+ try {
+ return new ActionTemplateExpansionValue(
+ Actions.filterSharedActionsAndThrowActionConflict(actionKeyContext, actions), false);
+ } catch (ActionConflictException e) {
+ throw new IllegalStateException(e);
+ }
}
@Override
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 aaadc22..7a804e4 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
@@ -24,16 +24,17 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Action;
-import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.ActionLookupValue;
+import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.MissingInputFileException;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.actions.cache.DigestUtils;
import com.google.devtools.build.lib.actions.cache.Metadata;
import com.google.devtools.build.lib.actions.util.TestAction.DummyAction;
@@ -222,18 +223,20 @@
return result.get(key);
}
- private void setGeneratingActions() throws InterruptedException {
+ private void setGeneratingActions() throws InterruptedException, ActionConflictException {
if (evaluator.getExistingValue(ALL_OWNER) == null) {
differencer.inject(
ImmutableMap.of(
ALL_OWNER,
new ActionLookupValue(
- actionKeyContext, ImmutableList.<ActionAnalysisMetadata>copyOf(actions), false)));
+ Actions.filterSharedActionsAndThrowActionConflict(
+ actionKeyContext, ImmutableList.copyOf(actions)),
+ false)));
}
}
private <E extends SkyValue> EvaluationResult<E> evaluate(SkyKey... keys)
- throws InterruptedException {
+ throws InterruptedException, ActionConflictException {
setGeneratingActions();
return driver.evaluate(
Arrays.asList(keys), /*keepGoing=*/
