Create a clearer distinction between normal tree artifact children and action template expansion outputs.
Two different factory methods are used: createTreeOutput for normal tree artifact children and createTemplateExpansionOutput for template expansion outputs. Additional documentation and validation is added for action templates to ensure that implementations follow the contract.
After this, I plan to add a method on TreeFileArtifact to distinguish between the two so that various places can properly treat these two different tree file artifact types differently instead of always having to handle both possibilities.
A lot of tests needed to be cleaned up, but the only test that needed a major overhaul was TreeArtifactBuildTest since it was generally declaring the outputs before creating the action.
RELNOTES: None.
PiperOrigin-RevId: 312128057
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 28e4fef..9bb433f 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
@@ -23,7 +23,6 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Action;
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;
@@ -204,15 +203,9 @@
// artifact2 is a tree artifact generated by action template.
SpecialArtifact artifact2 = createDerivedTreeArtifactOnly("treeArtifact2");
TreeFileArtifact treeFileArtifact1 =
- createFakeTreeFileArtifact(
- artifact2,
- "child1",
- "hello1");
+ createFakeExpansionTreeFileArtifact(artifact2, "child1", "hello1");
TreeFileArtifact treeFileArtifact2 =
- createFakeTreeFileArtifact(
- artifact2,
- "child2",
- "hello2");
+ createFakeExpansionTreeFileArtifact(artifact2, "child2", "hello2");
actions.add(
ActionsTestUtil.createDummySpawnActionTemplate(artifact1, artifact2));
@@ -233,23 +226,17 @@
// artifact2 is a tree artifact generated by action template.
SpecialArtifact artifact2 = createDerivedTreeArtifactOnly("treeArtifact2");
- createFakeTreeFileArtifact(artifact2, "child1", "hello1");
- createFakeTreeFileArtifact(artifact2, "child2", "hello2");
+ createFakeExpansionTreeFileArtifact(artifact2, "child1", "hello1");
+ createFakeExpansionTreeFileArtifact(artifact2, "child2", "hello2");
actions.add(
ActionsTestUtil.createDummySpawnActionTemplate(artifact1, artifact2));
// artifact3 is a tree artifact generated by action template.
SpecialArtifact artifact3 = createDerivedTreeArtifactOnly("treeArtifact3");
TreeFileArtifact treeFileArtifact1 =
- createFakeTreeFileArtifact(
- artifact3,
- "child1",
- "hello1");
+ createFakeExpansionTreeFileArtifact(artifact3, "child1", "hello1");
TreeFileArtifact treeFileArtifact2 =
- createFakeTreeFileArtifact(
- artifact3,
- "child2",
- "hello2");
+ createFakeExpansionTreeFileArtifact(artifact3, "child2", "hello2");
actions.add(
ActionsTestUtil.createDummySpawnActionTemplate(artifact2, artifact3));
@@ -268,9 +255,9 @@
ActionMetadataHandler.fileArtifactValueFromArtifact(artifact1, null, null);
SpecialArtifact treeArtifact = createDerivedTreeArtifactOnly("tree");
treeArtifact.setGeneratingActionKey(dummyData);
- TreeFileArtifact treeFileArtifact = ActionInputHelper.treeFileArtifact(treeArtifact, "subpath");
+ TreeFileArtifact treeFileArtifact = TreeFileArtifact.createTreeOutput(treeArtifact, "subpath");
Path path = treeFileArtifact.getPath();
- FileSystemUtils.createDirectoryAndParents(path.getParentDirectory());
+ path.getParentDirectory().createDirectoryAndParents();
writeFile(path, "contents");
TreeArtifactValue treeArtifactValue =
TreeArtifactValue.create(
@@ -324,6 +311,7 @@
private SpecialArtifact createDerivedTreeArtifactWithAction(String path) {
SpecialArtifact treeArtifact = createDerivedTreeArtifactOnly(path);
actions.add(new DummyAction(NestedSetBuilder.emptySet(Order.STABLE_ORDER), treeArtifact));
+ treeArtifact.setGeneratingActionKey(ActionLookupData.create(ALL_OWNER, actions.size() - 1));
return treeArtifact;
}
@@ -336,10 +324,22 @@
private static TreeFileArtifact createFakeTreeFileArtifact(
SpecialArtifact treeArtifact, String parentRelativePath, String content) throws Exception {
TreeFileArtifact treeFileArtifact =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
- treeArtifact, parentRelativePath);
+ TreeFileArtifact.createTreeOutput(treeArtifact, parentRelativePath);
Path path = treeFileArtifact.getPath();
- FileSystemUtils.createDirectoryAndParents(path.getParentDirectory());
+ path.getParentDirectory().createDirectoryAndParents();
+ writeFile(path, content);
+ return treeFileArtifact;
+ }
+
+ private static TreeFileArtifact createFakeExpansionTreeFileArtifact(
+ SpecialArtifact treeArtifact, String parentRelativePath, String content) throws Exception {
+ TreeFileArtifact treeFileArtifact =
+ TreeFileArtifact.createTemplateExpansionOutput(
+ treeArtifact,
+ parentRelativePath,
+ ActionTemplateExpansionValue.key(ALL_OWNER, /*actionIndex=*/ 0));
+ Path path = treeFileArtifact.getPath();
+ path.getParentDirectory().createDirectoryAndParents();
writeFile(path, content);
return treeFileArtifact;
}
@@ -413,10 +413,10 @@
try {
if (output.isTreeArtifact()) {
- TreeFileArtifact treeFileArtifact1 = ActionInputHelper.treeFileArtifact(
- (SpecialArtifact) output, PathFragment.create("child1"));
- TreeFileArtifact treeFileArtifact2 = ActionInputHelper.treeFileArtifact(
- (SpecialArtifact) output, PathFragment.create("child2"));
+ TreeFileArtifact treeFileArtifact1 =
+ TreeFileArtifact.createTreeOutput((SpecialArtifact) output, "child1");
+ TreeFileArtifact treeFileArtifact2 =
+ TreeFileArtifact.createTreeOutput((SpecialArtifact) output, "child2");
TreeArtifactValue treeArtifactValue =
TreeArtifactValue.create(
ImmutableMap.of(
