Permit omitting top-level action template expansion outputs.
If the outputs are omitted, just return OMITTED_FILE_MARKER from ArtifactFunction instead of attempting to create a TreeArtifactValue, which crashes.
RELNOTES: None.
PiperOrigin-RevId: 312159642
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 9bb433f..76d381b 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
@@ -17,6 +17,7 @@
import static com.google.devtools.build.lib.actions.FileArtifactValue.createForTesting;
import static org.junit.Assert.assertThrows;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -25,6 +26,7 @@
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType;
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.ActionLookupValue;
+import com.google.devtools.build.lib.actions.ActionTemplate;
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;
@@ -38,6 +40,7 @@
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.analysis.actions.SpawnActionTemplate;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.events.NullEventHandler;
@@ -60,7 +63,9 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Map;
+import java.util.Set;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -73,9 +78,11 @@
@RunWith(JUnit4.class)
public class ArtifactFunctionTest extends ArtifactFunctionTestCase {
+ private final Set<Artifact> omittedOutputs = new HashSet<>();
+
@Before
- public final void setUp() throws Exception {
- delegateActionExecutionFunction = new SimpleActionExecutionFunction();
+ public final void setUp() {
+ delegateActionExecutionFunction = new SimpleActionExecutionFunction(omittedOutputs);
}
private void assertFileArtifactValueMatches(boolean expectDigest) throws Throwable {
@@ -202,13 +209,13 @@
// artifact2 is a tree artifact generated by action template.
SpecialArtifact artifact2 = createDerivedTreeArtifactOnly("treeArtifact2");
+ SpawnActionTemplate actionTemplate =
+ ActionsTestUtil.createDummySpawnActionTemplate(artifact1, artifact2);
+ actions.add(actionTemplate);
TreeFileArtifact treeFileArtifact1 =
- createFakeExpansionTreeFileArtifact(artifact2, "child1", "hello1");
+ createFakeExpansionTreeFileArtifact(actionTemplate, "child1", "hello1");
TreeFileArtifact treeFileArtifact2 =
- createFakeExpansionTreeFileArtifact(artifact2, "child2", "hello2");
-
- actions.add(
- ActionsTestUtil.createDummySpawnActionTemplate(artifact1, artifact2));
+ createFakeExpansionTreeFileArtifact(actionTemplate, "child2", "hello2");
TreeArtifactValue value = (TreeArtifactValue) evaluateArtifactValue(artifact2);
assertThat(value.getChildValues()).containsKey(treeFileArtifact1);
@@ -226,19 +233,21 @@
// artifact2 is a tree artifact generated by action template.
SpecialArtifact artifact2 = createDerivedTreeArtifactOnly("treeArtifact2");
- createFakeExpansionTreeFileArtifact(artifact2, "child1", "hello1");
- createFakeExpansionTreeFileArtifact(artifact2, "child2", "hello2");
- actions.add(
- ActionsTestUtil.createDummySpawnActionTemplate(artifact1, artifact2));
+ SpawnActionTemplate template2 =
+ ActionsTestUtil.createDummySpawnActionTemplate(artifact1, artifact2);
+ actions.add(template2);
+ createFakeExpansionTreeFileArtifact(template2, "child1", "hello1");
+ createFakeExpansionTreeFileArtifact(template2, "child2", "hello2");
// artifact3 is a tree artifact generated by action template.
SpecialArtifact artifact3 = createDerivedTreeArtifactOnly("treeArtifact3");
+ SpawnActionTemplate template3 =
+ ActionsTestUtil.createDummySpawnActionTemplate(artifact2, artifact3);
+ actions.add(template3);
TreeFileArtifact treeFileArtifact1 =
- createFakeExpansionTreeFileArtifact(artifact3, "child1", "hello1");
+ createFakeExpansionTreeFileArtifact(template3, "child1", "hello1");
TreeFileArtifact treeFileArtifact2 =
- createFakeExpansionTreeFileArtifact(artifact3, "child2", "hello2");
- actions.add(
- ActionsTestUtil.createDummySpawnActionTemplate(artifact2, artifact3));
+ createFakeExpansionTreeFileArtifact(template3, "child2", "hello2");
TreeArtifactValue value = (TreeArtifactValue) evaluateArtifactValue(artifact3);
assertThat(value.getChildValues()).containsKey(treeFileArtifact1);
@@ -248,6 +257,60 @@
}
@Test
+ public void actionTemplateExpansionOutputsOmitted() throws Throwable {
+ // artifact1 is a tree artifact generated by normal action.
+ SpecialArtifact artifact1 = createDerivedTreeArtifactWithAction("treeArtifact1");
+ createFakeTreeFileArtifact(artifact1, "child1", "hello1");
+ createFakeTreeFileArtifact(artifact1, "child2", "hello2");
+
+ // artifact2 is a tree artifact generated by action template.
+ SpecialArtifact artifact2 = createDerivedTreeArtifactOnly("treeArtifact2");
+ SpawnActionTemplate actionTemplate =
+ ActionsTestUtil.createDummySpawnActionTemplate(artifact1, artifact2);
+ actions.add(actionTemplate);
+ TreeFileArtifact treeFileArtifact1 =
+ createFakeExpansionTreeFileArtifact(actionTemplate, "child1", "hello1");
+ TreeFileArtifact treeFileArtifact2 =
+ createFakeExpansionTreeFileArtifact(actionTemplate, "child2", "hello2");
+
+ omittedOutputs.add(treeFileArtifact1);
+ omittedOutputs.add(treeFileArtifact2);
+
+ SkyValue value = evaluateArtifactValue(artifact2);
+ assertThat(value).isEqualTo(FileArtifactValue.OMITTED_FILE_MARKER);
+ }
+
+ @Test
+ public void cannotOmitSomeButNotAllActionTemplateExpansionOutputs() throws Throwable {
+ // artifact1 is a tree artifact generated by normal action.
+ SpecialArtifact artifact1 = createDerivedTreeArtifactWithAction("treeArtifact1");
+ createFakeTreeFileArtifact(artifact1, "child1", "hello1");
+ createFakeTreeFileArtifact(artifact1, "child2", "hello2");
+
+ // artifact2 is a tree artifact generated by action template.
+ SpecialArtifact artifact2 = createDerivedTreeArtifactOnly("treeArtifact2");
+ SpawnActionTemplate actionTemplate =
+ ActionsTestUtil.createDummySpawnActionTemplate(artifact1, artifact2);
+ actions.add(actionTemplate);
+ TreeFileArtifact treeFileArtifact1 =
+ createFakeExpansionTreeFileArtifact(actionTemplate, "child1", "hello1");
+ TreeFileArtifact treeFileArtifact2 =
+ createFakeExpansionTreeFileArtifact(actionTemplate, "child2", "hello2");
+
+ omittedOutputs.add(treeFileArtifact1);
+
+ Exception e = assertThrows(RuntimeException.class, () -> evaluateArtifactValue(artifact2));
+ assertThat(e).hasCauseThat().isInstanceOf(IllegalStateException.class);
+ assertThat(e)
+ .hasCauseThat()
+ .hasMessageThat()
+ .matches(
+ "Action template expansion has some but not all outputs omitted, present outputs: .*"
+ + treeFileArtifact2.getParentRelativePath()
+ + ".*");
+ }
+
+ @Test
public void actionExecutionValueSerialization() throws Exception {
ActionLookupData dummyData = ActionLookupData.create(ALL_OWNER, 0);
Artifact.DerivedArtifact artifact1 = createDerivedArtifact("one");
@@ -331,13 +394,16 @@
return treeFileArtifact;
}
- private static TreeFileArtifact createFakeExpansionTreeFileArtifact(
- SpecialArtifact treeArtifact, String parentRelativePath, String content) throws Exception {
+ private TreeFileArtifact createFakeExpansionTreeFileArtifact(
+ ActionTemplate<?> actionTemplate, String parentRelativePath, String content)
+ throws Exception {
+ int actionIndex = Iterables.indexOf(actions, actionTemplate::equals);
+ Preconditions.checkState(actionIndex >= 0, "%s not registered", actionTemplate);
TreeFileArtifact treeFileArtifact =
TreeFileArtifact.createTemplateExpansionOutput(
- treeArtifact,
+ actionTemplate.getOutputTreeArtifact(),
parentRelativePath,
- ActionTemplateExpansionValue.key(ALL_OWNER, /*actionIndex=*/ 0));
+ ActionTemplateExpansionValue.key(ALL_OWNER, actionIndex));
Path path = treeFileArtifact.getPath();
path.getParentDirectory().createDirectoryAndParents();
writeFile(path, content);
@@ -399,8 +465,17 @@
return driver.evaluate(Arrays.asList(keys), evaluationContext);
}
- /** Value Builder for actions that just stats and stores the output file (which must exist). */
- private static class SimpleActionExecutionFunction implements SkyFunction {
+ /**
+ * Value builder for actions that just stats and stores the output file (which must either be
+ * orphaned or exist).
+ */
+ private static final class SimpleActionExecutionFunction implements SkyFunction {
+ private final Set<Artifact> omittedOutputs;
+
+ SimpleActionExecutionFunction(Set<Artifact> omittedOutputs) {
+ this.omittedOutputs = omittedOutputs;
+ }
+
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
Map<Artifact, FileArtifactValue> artifactData = new HashMap<>();
@@ -412,7 +487,10 @@
Artifact output = Iterables.getOnlyElement(action.getOutputs());
try {
- if (output.isTreeArtifact()) {
+ if (omittedOutputs.contains(output)) {
+ Preconditions.checkState(!output.isTreeArtifact(), "Cannot omit %s", output);
+ artifactData.put(output, FileArtifactValue.OMITTED_FILE_MARKER);
+ } else if (output.isTreeArtifact()) {
TreeFileArtifact treeFileArtifact1 =
TreeFileArtifact.createTreeOutput((SpecialArtifact) output, "child1");
TreeFileArtifact treeFileArtifact2 =