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/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=*/
