Replace Artifact constructors with factory methods and simplify des…

…erialization code.

Private constructors now take `Object` for `owner`, allowing deserialization to pass the generating action key directly instead of calling `setGeneratingActionKey`.

PiperOrigin-RevId: 399206504

src/main/java/com/google/devtools/build/lib/actions/Artifact.java

@@ -388,22 +388,32 @@ public static class DerivedArtifact extends Artifact {
      */
     private final boolean contentBasedPath;
 
-    /** Standard constructor for derived artifacts. */
-    public DerivedArtifact(ArtifactRoot root, PathFragment execPath, ActionLookupKey owner) {
-      this(root, execPath, owner, /*contentBasedPath=*/ false);
+    /** Standard factory method for derived artifacts. */
+    public static DerivedArtifact create(
+        ArtifactRoot root, PathFragment execPath, ActionLookupKey owner) {
+      return create(root, execPath, owner, /*contentBasedPath=*/ false);
     }
 
     /**
-     * Same as {@link #DerivedArtifact(ArtifactRoot, PathFragment, ActionLookupKey)} but includes
-     * tge option to use a content-based path for this artifact (see {@link
+     * Same as {@link #create(ArtifactRoot, PathFragment, ActionLookupKey)} but includes the option
+     * to use a content-based path for this artifact (see {@link
      * com.google.devtools.build.lib.analysis.config.BuildConfiguration#useContentBasedOutputPaths}).
      */
-    public DerivedArtifact(
+    public static DerivedArtifact create(
         ArtifactRoot root, PathFragment execPath, ActionLookupKey owner, boolean contentBasedPath) {
+      return new DerivedArtifact(root, execPath, owner, contentBasedPath);
+    }
+
+    private DerivedArtifact(ArtifactRoot root, PathFragment execPath, Object owner) {
+      this(root, execPath, owner, /*contentBasedPath=*/ false);
+    }
+
+    private DerivedArtifact(
+        ArtifactRoot root, PathFragment execPath, Object owner, boolean contentBasedPath) {
       super(root, execPath);
       Preconditions.checkState(
           !root.getExecPath().isEmpty(), "Derived root has no exec path: %s, %s", root, execPath);
-      this.owner = owner;
+      this.owner = Preconditions.checkNotNull(owner);
       this.contentBasedPath = contentBasedPath;
     }
 
@@ -503,45 +513,40 @@ public void serialize(
         SerializationContext context, DerivedArtifact obj, CodedOutputStream codedOut)
         throws SerializationException, IOException {
       context.serialize(obj.getRoot(), codedOut);
-      context.serialize(obj.getGeneratingActionKey(), codedOut);
       context.serialize(obj.getRootRelativePath(), codedOut);
+      context.serialize(obj.getGeneratingActionKey(), codedOut);
     }
 
     @Override
     public DerivedArtifact deserialize(DeserializationContext context, CodedInputStream codedIn)
         throws SerializationException, IOException {
       ArtifactRoot root = context.deserialize(codedIn);
+      PathFragment rootRelativePath = context.deserialize(codedIn);
       ActionLookupData generatingActionKey = context.deserialize(codedIn);
       DerivedArtifact artifact =
           new DerivedArtifact(
               root,
-              validateAndGetRootExecPath(root, generatingActionKey, context, codedIn),
-              generatingActionKey.getActionLookupKey());
-      artifact.setGeneratingActionKey(generatingActionKey);
+              getExecPathForDeserialization(root, rootRelativePath, generatingActionKey),
+              generatingActionKey);
       return context.getDependency(ArtifactSerializationContext.class).intern(artifact);
     }
+  }
 
-    static PathFragment validateAndGetRootExecPath(
-        ArtifactRoot root,
-        ActionLookupData generatingActionKey,
-        DeserializationContext context,
-        CodedInputStream codedIn)
-        throws IOException, SerializationException {
-      PathFragment rootRelativePath = context.deserialize(codedIn);
-      if (rootRelativePath == null
-          || rootRelativePath.isAbsolute() != root.getRoot().isAbsolute()) {
-        throw new IllegalArgumentException(
-            rootRelativePath
-                + ": illegal rootRelativePath for "
-                + root
-                + " (generatingActionKey: "
-                + generatingActionKey
-                + ")");
-      }
-      Preconditions.checkState(
-          !root.isSourceRoot(), "Root not derived: %s %s", root, rootRelativePath);
-      return root.getExecPath().getRelative(rootRelativePath);
-    }
+  private static PathFragment getExecPathForDeserialization(
+      ArtifactRoot root, PathFragment rootRelativePath, ActionLookupData generatingActionKey) {
+    Preconditions.checkArgument(
+        !root.isSourceRoot(),
+        "Root not derived: %s (rootRelativePath=%s, generatingActionKey=%s)",
+        root,
+        rootRelativePath,
+        generatingActionKey);
+    Preconditions.checkArgument(
+        root.getRoot().isAbsolute() == rootRelativePath.isAbsolute(),
+        "Illegal root relative path: %s (root=%s, generatingActionKey=%s)",
+        rootRelativePath,
+        root,
+        generatingActionKey);
+    return root.getExecPath().getRelative(rootRelativePath);
   }
 
   public final Path getPath() {
@@ -1002,8 +1007,13 @@ public static final class SpecialArtifact extends DerivedArtifact {
     private final SpecialArtifactType type;
 
     @VisibleForTesting
-    public SpecialArtifact(
+    public static SpecialArtifact create(
         ArtifactRoot root, PathFragment execPath, ActionLookupKey owner, SpecialArtifactType type) {
+      return new SpecialArtifact(root, execPath, owner, type);
+    }
+
+    private SpecialArtifact(
+        ArtifactRoot root, PathFragment execPath, Object owner, SpecialArtifactType type) {
       super(root, execPath, owner);
       this.type = type;
     }
@@ -1058,25 +1068,24 @@ public void serialize(
         SerializationContext context, SpecialArtifact obj, CodedOutputStream codedOut)
         throws SerializationException, IOException {
       context.serialize(obj.getRoot(), codedOut);
+      context.serialize(obj.getRootRelativePath(), codedOut);
       context.serialize(obj.getGeneratingActionKey(), codedOut);
       context.serialize(obj.type, codedOut);
-      context.serialize(obj.getRootRelativePath(), codedOut);
     }
 
     @Override
     public SpecialArtifact deserialize(DeserializationContext context, CodedInputStream codedIn)
         throws SerializationException, IOException {
       ArtifactRoot root = context.deserialize(codedIn);
+      PathFragment rootRelativePath = context.deserialize(codedIn);
       ActionLookupData generatingActionKey = context.deserialize(codedIn);
       SpecialArtifactType type = context.deserialize(codedIn);
       SpecialArtifact artifact =
           new SpecialArtifact(
               root,
-              DerivedArtifactCodec.validateAndGetRootExecPath(
-                  root, generatingActionKey, context, codedIn),
-              generatingActionKey.getActionLookupKey(),
+              getExecPathForDeserialization(root, rootRelativePath, generatingActionKey),
+              generatingActionKey,
               type);
-      artifact.setGeneratingActionKey(generatingActionKey);
       return (SpecialArtifact)
           context.getDependency(ArtifactSerializationContext.class).intern(artifact);
     }
@@ -1307,7 +1316,7 @@ public static TreeFileArtifact createTemplateExpansionOutput(
     }
 
     private TreeFileArtifact(
-        SpecialArtifact parent, PathFragment parentRelativePath, ActionLookupKey owner) {
+        SpecialArtifact parent, PathFragment parentRelativePath, Object owner) {
       super(parent.getRoot(), parent.getExecPath().getRelative(parentRelativePath), owner);
       Preconditions.checkArgument(
           parent.isTreeArtifact(),
@@ -1328,11 +1337,7 @@ static TreeFileArtifact createInternal(
         SpecialArtifact parent,
         PathFragment parentRelativePath,
         ActionLookupData generatingActionKey) {
-      TreeFileArtifact result =
-          new TreeFileArtifact(
-              parent, parentRelativePath, generatingActionKey.getActionLookupKey());
-      result.setGeneratingActionKey(generatingActionKey);
-      return result;
+      return new TreeFileArtifact(parent, parentRelativePath, generatingActionKey);
     }
 
     @Override
@@ -1575,7 +1580,7 @@ public void repr(Printer printer) {
    * A utility class that compares {@link Artifact}s without taking their owners into account.
    * Should only be used for detecting action conflicts and merging shared action data.
    */
-  public static class OwnerlessArtifactWrapper {
+  public static final class OwnerlessArtifactWrapper {
     private final Artifact artifact;
 
     public OwnerlessArtifactWrapper(Artifact artifact) {

src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java

@@ -15,7 +15,9 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
+import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
 import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
+import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
 import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
 import com.google.devtools.build.lib.cmdline.LabelConstants;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
@@ -349,9 +351,9 @@ private static Artifact createArtifact(
     if (type == null) {
       return root.isSourceRoot()
           ? new Artifact.SourceArtifact(root, execPath, owner)
-          : new Artifact.DerivedArtifact(root, execPath, (ActionLookupKey) owner, contentBasedPath);
+          : DerivedArtifact.create(root, execPath, (ActionLookupKey) owner, contentBasedPath);
     } else {
-      return new Artifact.SpecialArtifact(root, execPath, (ActionLookupKey) owner, type);
+      return SpecialArtifact.create(root, execPath, (ActionLookupKey) owner, type);
     }
   }
 

src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java

@@ -508,7 +508,7 @@ private static BuildEventStreamProtos.TestResult.ExecutionInfo extractExecutionI
   private static Artifact.DerivedArtifact createArtifactOutput(
       TestRunnerAction action, PathFragment outputPath) {
     Artifact.DerivedArtifact testLog = (Artifact.DerivedArtifact) action.getTestLog();
-    return new DerivedArtifact(testLog.getRoot(), outputPath, testLog.getArtifactOwner());
+    return DerivedArtifact.create(testLog.getRoot(), outputPath, testLog.getArtifactOwner());
   }
 
   /**

src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java

@@ -425,7 +425,7 @@ public synchronized NestedSet<Artifact> getInputs() {
   @Test
   public void testDeletedConstantMetadataOutputCausesReexecution() throws Exception {
     SpecialArtifact output =
-        new Artifact.SpecialArtifact(
+        SpecialArtifact.create(
             artifactRoot,
             PathFragment.create("bin/dummy"),
             NULL_ARTIFACT_OWNER,

src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java

@@ -228,7 +228,7 @@ public void testCodec() throws Exception {
     ArtifactRoot anotherRoot =
         ArtifactRoot.asDerivedRoot(scratch.getFileSystem().getPath("/"), RootType.Output, "src");
     DerivedArtifact anotherArtifact =
-        new DerivedArtifact(
+        DerivedArtifact.create(
             anotherRoot,
             anotherRoot.getExecPath().getRelative("src/c"),
             ActionsTestUtil.NULL_ARTIFACT_OWNER);
@@ -359,10 +359,10 @@ public SkyFunctionName functionName() {
           }
         };
     DerivedArtifact derived1 =
-        new DerivedArtifact(root, PathFragment.create("newRoot/shared"), firstOwner);
+        DerivedArtifact.create(root, PathFragment.create("newRoot/shared"), firstOwner);
     derived1.setGeneratingActionKey(ActionLookupData.create(firstOwner, 0));
     DerivedArtifact derived2 =
-        new DerivedArtifact(root, PathFragment.create("newRoot/shared"), secondOwner);
+        DerivedArtifact.create(root, PathFragment.create("newRoot/shared"), secondOwner);
     derived2.setGeneratingActionKey(ActionLookupData.create(secondOwner, 0));
     ArtifactRoot sourceRoot = ArtifactRoot.asSourceRoot(Root.fromPath(root.getRoot().asPath()));
     Artifact source1 = new SourceArtifact(sourceRoot, PathFragment.create("shared"), firstOwner);
@@ -401,7 +401,7 @@ public void canDeclareContentBasedOutput() {
     Path execRoot = scratch.getFileSystem().getPath("/");
     ArtifactRoot root = ArtifactRoot.asDerivedRoot(execRoot, RootType.Output, "newRoot");
     assertThat(
-            new DerivedArtifact(
+            DerivedArtifact.create(
                     root,
                     PathFragment.create("newRoot/my.output"),
                     ActionsTestUtil.NULL_ARTIFACT_OWNER,
@@ -577,7 +577,7 @@ private static SpecialArtifact createTreeArtifact(ArtifactRoot root, String rela
   private static SpecialArtifact createTreeArtifact(
       ArtifactRoot root, String relativePath, ActionLookupData actionLookupData) {
     SpecialArtifact treeArtifact =
-        new SpecialArtifact(
+        SpecialArtifact.create(
             root,
             root.getExecPath().getRelative(relativePath),
             actionLookupData.getActionLookupKey(),

src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java

@@ -249,13 +249,13 @@ public static Artifact createArtifactWithRootRelativePath(
   public static Artifact createArtifactWithExecPath(ArtifactRoot root, PathFragment execPath) {
     return root.isSourceRoot()
         ? new Artifact.SourceArtifact(root, execPath, ArtifactOwner.NULL_OWNER)
-        : new Artifact.DerivedArtifact(root, execPath, NULL_ARTIFACT_OWNER);
+        : DerivedArtifact.create(root, execPath, NULL_ARTIFACT_OWNER);
   }
 
   public static SpecialArtifact createTreeArtifactWithGeneratingAction(
       ArtifactRoot root, PathFragment execPath) {
     SpecialArtifact treeArtifact =
-        new SpecialArtifact(root, execPath, NULL_ARTIFACT_OWNER, SpecialArtifactType.TREE);
+        SpecialArtifact.create(root, execPath, NULL_ARTIFACT_OWNER, SpecialArtifactType.TREE);
     treeArtifact.setGeneratingActionKey(NULL_ACTION_LOOKUP_DATA);
     return treeArtifact;
   }

src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java

@@ -23,6 +23,7 @@
 import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.actions.ActionLookupKey;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
 import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
@@ -180,10 +181,8 @@ public void testPutDerivedArtifactWithDifferentOwnerDoesNotConflict() throws Exc
 
     SimpleActionLookupKey owner1 = new SimpleActionLookupKey("//owner1");
     SimpleActionLookupKey owner2 = new SimpleActionLookupKey("//owner2");
-    Artifact artifact1 =
-        new Artifact.DerivedArtifact(root, root.getExecPath().getRelative(path), owner1);
-    Artifact artifact2 =
-        new Artifact.DerivedArtifact(root, root.getExecPath().getRelative(path), owner2);
+    Artifact artifact1 = DerivedArtifact.create(root, root.getExecPath().getRelative(path), owner1);
+    Artifact artifact2 = DerivedArtifact.create(root, root.getExecPath().getRelative(path), owner2);
 
     Map<PathFragment, Artifact> map = new LinkedHashMap<>();
 
@@ -205,10 +204,9 @@ public void testPutDerivedArtifactWithDifferentPathConflicts() throws Exception
 
     SimpleActionLookupKey owner1 = new SimpleActionLookupKey("//owner1");
     SimpleActionLookupKey owner2 = new SimpleActionLookupKey("//owner2");
-    Artifact artifact1 =
-        new Artifact.DerivedArtifact(root, root.getExecPath().getRelative(path), owner1);
+    Artifact artifact1 = DerivedArtifact.create(root, root.getExecPath().getRelative(path), owner1);
     Artifact artifact2 =
-        new Artifact.DerivedArtifact(root, root.getExecPath().getRelative(path2), owner2);
+        DerivedArtifact.create(root, root.getExecPath().getRelative(path2), owner2);
 
     Map<PathFragment, Artifact> map = new LinkedHashMap<>();
 

src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplateTest.java

@@ -379,7 +379,7 @@ private SpecialArtifact createOutputTreeArtifact() {
   private SpecialArtifact createTreeArtifact(String rootRelativePath) {
     PathFragment relpath = PathFragment.create(rootRelativePath);
     SpecialArtifact result =
-        new SpecialArtifact(
+        SpecialArtifact.create(
             root,
             root.getExecPath().getRelative(relpath),
             ActionsTestUtil.NULL_ARTIFACT_OWNER,

src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLineTest.java

@@ -205,7 +205,7 @@ private SpecialArtifact createTreeArtifact(String relativePath) {
   }
 
   private SpecialArtifact createSpecialArtifact(String relativePath, SpecialArtifactType type) {
-    return new SpecialArtifact(
+    return SpecialArtifact.create(
         derivedRoot,
         derivedRoot.getExecPath().getRelative(relativePath),
         ActionsTestUtil.NULL_ARTIFACT_OWNER,

src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java

@@ -385,7 +385,7 @@ private SpecialArtifact createSpecialArtifact(String relPath, SpecialArtifactTyp
     Path outputPath = outputDir.getRelative(relPath);
     outputPath.createDirectoryAndParents();
     ArtifactRoot derivedRoot = ArtifactRoot.asDerivedRoot(execRoot, RootType.Output, outputSegment);
-    return new SpecialArtifact(
+    return SpecialArtifact.create(
         derivedRoot,
         derivedRoot.getExecPath().getRelative(derivedRoot.getRoot().relativize(outputPath)),
         ActionsTestUtil.NULL_ARTIFACT_OWNER,
@@ -455,7 +455,7 @@ private ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> simpleFilese
   }
 
   private SpecialArtifact createFileset(String execPath) {
-    return new SpecialArtifact(
+    return SpecialArtifact.create(
         rootDir,
         PathFragment.create(execPath),
         ActionsTestUtil.NULL_ARTIFACT_OWNER,

src/test/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscoveryTest.java

@@ -81,7 +81,7 @@ private void checkHeaderInclusion(
   }
 
   private SpecialArtifact treeArtifact(Path path) {
-    return new SpecialArtifact(
+    return SpecialArtifact.create(
         artifactRoot,
         artifactRoot.getExecPath().getRelative(artifactRoot.getRoot().relativize(path)),
         ActionsTestUtil.NULL_ARTIFACT_OWNER,

src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java

@@ -22,6 +22,7 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
 import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
 import com.google.devtools.build.lib.actions.ResourceSet;
@@ -430,7 +431,7 @@ private Artifact artifact(String ownerLabel, String path) {
   /** Creates a dummy artifact with the given path, that actually resides in /out/<path>. */
   private Artifact derivedArtifact(String path) {
     Artifact.DerivedArtifact derivedArtifact =
-        new Artifact.DerivedArtifact(
+        DerivedArtifact.create(
             derivedRoot,
             derivedRoot.getExecPath().getRelative(path),
             ActionsTestUtil.NULL_ARTIFACT_OWNER);

src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionValueTransformSharedTreeArtifactsTest.java

@@ -234,12 +234,13 @@ private TreeArtifactValue createTreeArtifactValue(
   }
 
   private DerivedArtifact createFileArtifact(String relativePath, ActionLookupKey owner) {
-    return new DerivedArtifact(derivedRoot, DERIVED_PATH_PREFIX.getRelative(relativePath), owner);
+    return DerivedArtifact.create(
+        derivedRoot, DERIVED_PATH_PREFIX.getRelative(relativePath), owner);
   }
 
   private SpecialArtifact createTreeArtifact(String relativePath, ActionLookupKey owner) {
     SpecialArtifact treeArtifact =
-        new SpecialArtifact(
+        SpecialArtifact.create(
             derivedRoot,
             DERIVED_PATH_PREFIX.getRelative(relativePath),
             owner,