Make Artifact#equals take the owner into account for derived artifacts.
Derived artifacts' owners are important because they are used to determine the artifact's generating action. Source artifacts' owners are not used in this way, so I left them alone.
This allows us to get rid of most uses of ArtifactSkyKey. We may be able to delete it entirely in a follow-up.
PiperOrigin-RevId: 199836436
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 2b137ed..8a22813 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
@@ -31,6 +31,7 @@
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.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.BasicActionLookupValue;
import com.google.devtools.build.lib.actions.FileArtifactValue;
@@ -150,10 +151,7 @@
actions.add(action);
file(input2.getPath(), "contents");
file(input1.getPath(), "source contents");
- evaluate(
- Iterables.toArray(
- ArtifactSkyKey.mandatoryKeys(ImmutableSet.of(input2, input1, input2, tree)),
- SkyKey.class));
+ evaluate(Iterables.toArray(ImmutableSet.of(input2, input1, input2, tree), SkyKey.class));
SkyValue value = evaluateArtifactValue(output);
assertThat(((AggregatingArtifactValue) value).getInputs())
.containsExactly(
@@ -190,12 +188,14 @@
@Test
public void testActionTreeArtifactOutput() throws Throwable {
SpecialArtifact artifact = createDerivedTreeArtifactWithAction("treeArtifact");
- TreeFileArtifact treeFileArtifact1 = createFakeTreeFileArtifact(artifact, "child1", "hello1");
- TreeFileArtifact treeFileArtifact2 = createFakeTreeFileArtifact(artifact, "child2", "hello2");
+ TreeFileArtifact treeFileArtifact1 =
+ createFakeTreeFileArtifact(artifact, ALL_OWNER, "child1", "hello1");
+ TreeFileArtifact treeFileArtifact2 =
+ createFakeTreeFileArtifact(artifact, ALL_OWNER, "child2", "hello2");
TreeArtifactValue value = (TreeArtifactValue) evaluateArtifactValue(artifact);
- assertThat(value.getChildValues().get(treeFileArtifact1)).isNotNull();
- assertThat(value.getChildValues().get(treeFileArtifact2)).isNotNull();
+ assertThat(value.getChildValues()).containsKey(treeFileArtifact1);
+ assertThat(value.getChildValues()).containsKey(treeFileArtifact2);
assertThat(value.getChildValues().get(treeFileArtifact1).getDigest()).isNotNull();
assertThat(value.getChildValues().get(treeFileArtifact2).getDigest()).isNotNull();
}
@@ -209,15 +209,27 @@
// artifact2 is a tree artifact generated by action template.
SpecialArtifact artifact2 = createDerivedTreeArtifactOnly("treeArtifact2");
- TreeFileArtifact treeFileArtifact1 = createFakeTreeFileArtifact(artifact2, "child1", "hello1");
- TreeFileArtifact treeFileArtifact2 = createFakeTreeFileArtifact(artifact2, "child2", "hello2");
+ TreeFileArtifact treeFileArtifact1 =
+ createFakeTreeFileArtifact(
+ artifact2,
+ ActionTemplateExpansionValue.ActionTemplateExpansionKey.of(
+ (ActionLookupValue.ActionLookupKey) artifact2.getArtifactOwner(), 1),
+ "child1",
+ "hello1");
+ TreeFileArtifact treeFileArtifact2 =
+ createFakeTreeFileArtifact(
+ artifact2,
+ ActionTemplateExpansionValue.ActionTemplateExpansionKey.of(
+ (ActionLookupValue.ActionLookupKey) artifact2.getArtifactOwner(), 1),
+ "child2",
+ "hello2");
actions.add(
ActionsTestUtil.createDummySpawnActionTemplate(artifact1, artifact2));
TreeArtifactValue value = (TreeArtifactValue) evaluateArtifactValue(artifact2);
- assertThat(value.getChildValues().get(treeFileArtifact1)).isNotNull();
- assertThat(value.getChildValues().get(treeFileArtifact2)).isNotNull();
+ assertThat(value.getChildValues()).containsKey(treeFileArtifact1);
+ assertThat(value.getChildValues()).containsKey(treeFileArtifact2);
assertThat(value.getChildValues().get(treeFileArtifact1).getDigest()).isNotNull();
assertThat(value.getChildValues().get(treeFileArtifact2).getDigest()).isNotNull();
}
@@ -238,14 +250,26 @@
// artifact3 is a tree artifact generated by action template.
SpecialArtifact artifact3 = createDerivedTreeArtifactOnly("treeArtifact3");
- TreeFileArtifact treeFileArtifact1 = createFakeTreeFileArtifact(artifact3, "child1", "hello1");
- TreeFileArtifact treeFileArtifact2 = createFakeTreeFileArtifact(artifact3, "child2", "hello2");
+ TreeFileArtifact treeFileArtifact1 =
+ createFakeTreeFileArtifact(
+ artifact3,
+ ActionTemplateExpansionValue.ActionTemplateExpansionKey.of(
+ (ActionLookupValue.ActionLookupKey) artifact2.getArtifactOwner(), 2),
+ "child1",
+ "hello1");
+ TreeFileArtifact treeFileArtifact2 =
+ createFakeTreeFileArtifact(
+ artifact3,
+ ActionTemplateExpansionValue.ActionTemplateExpansionKey.of(
+ (ActionLookupValue.ActionLookupKey) artifact2.getArtifactOwner(), 2),
+ "child2",
+ "hello2");
actions.add(
ActionsTestUtil.createDummySpawnActionTemplate(artifact2, artifact3));
TreeArtifactValue value = (TreeArtifactValue) evaluateArtifactValue(artifact3);
- assertThat(value.getChildValues().get(treeFileArtifact1)).isNotNull();
- assertThat(value.getChildValues().get(treeFileArtifact2)).isNotNull();
+ assertThat(value.getChildValues()).containsKey(treeFileArtifact1);
+ assertThat(value.getChildValues()).containsKey(treeFileArtifact2);
assertThat(value.getChildValues().get(treeFileArtifact1).getDigest()).isNotNull();
assertThat(value.getChildValues().get(treeFileArtifact2).getDigest()).isNotNull();
}
@@ -256,7 +280,10 @@
}
private Artifact createSourceArtifact(String path) {
- return new Artifact(PathFragment.create(path), ArtifactRoot.asSourceRoot(Root.fromPath(root)));
+ return new Artifact.SourceArtifact(
+ ArtifactRoot.asSourceRoot(Root.fromPath(root)),
+ PathFragment.create(path),
+ ArtifactOwner.NullArtifactOwner.INSTANCE);
}
private Artifact createDerivedArtifact(String path) {
@@ -293,8 +320,23 @@
private TreeFileArtifact createFakeTreeFileArtifact(
SpecialArtifact treeArtifact, String parentRelativePath, String content) throws Exception {
- TreeFileArtifact treeFileArtifact = ActionInputHelper.treeFileArtifact(
- treeArtifact, PathFragment.create(parentRelativePath));
+ return createFakeTreeFileArtifact(
+ treeArtifact,
+ ActionTemplateExpansionValue.ActionTemplateExpansionKey.of(
+ (ActionLookupValue.ActionLookupKey) treeArtifact.getArtifactOwner(), 0),
+ parentRelativePath,
+ content);
+ }
+
+ private TreeFileArtifact createFakeTreeFileArtifact(
+ SpecialArtifact treeArtifact,
+ ArtifactOwner artifactOwner,
+ String parentRelativePath,
+ String content)
+ throws Exception {
+ TreeFileArtifact treeFileArtifact =
+ ActionInputHelper.treeFileArtifact(
+ treeArtifact, PathFragment.create(parentRelativePath), artifactOwner);
Path path = treeFileArtifact.getPath();
FileSystemUtils.createDirectoryAndParents(path.getParentDirectory());
writeFile(path, content);
