Subdivide derived root types so that Artifact class methods can alway…

…s handle derived artifact paths properly regardless of the --experimental_sibling_repository_layout value.

PiperOrigin-RevId: 353665168

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

@@ -650,11 +650,6 @@ public PathFragment getPathForLocationExpansion() {
    * that this is available on every Artifact type, including source artifacts. As a matter of fact,
    * one of its most common use cases is to construct a derived artifact's output path out of a
    * sibling source artifact's by replacing the basename in its output-dir-relative path.
-   *
-   * <p>This is just a wrapper over {@link Artifact#getPathForLocationExpansion} at the moment.
-   * However, since it will be kept in sync with the output directory layout, which is planned for
-   * an overhaul, it must be preferred over {@link Artifact#getPathForLocationExpansion} whenever
-   * possible.
    */
   public PathFragment getOutputDirRelativePath(boolean siblingRepositoryLayout) {
     return getRootRelativePath();
@@ -665,10 +660,12 @@ public PathFragment getOutputDirRelativePath(boolean siblingRepositoryLayout) {
    * path always starts with a corresponding package name, if exists.
    */
   public PathFragment getRepositoryRelativePath() {
-    PathFragment fullPath = getPathForLocationExpansion();
-    return fullPath.startsWith(LabelConstants.EXTERNAL_PATH_PREFIX)
-        ? fullPath.subFragment(2)
-        : fullPath;
+    PathFragment relativePath = getRootRelativePath();
+    // External artifacts under legacy roots are still prefixed with "external/<repo name>".
+    if (root.isLegacy() && relativePath.startsWith(LabelConstants.EXTERNAL_PATH_PREFIX)) {
+      relativePath = relativePath.subFragment(2);
+    }
+    return relativePath;
   }
 
   /** Returns this.getExecPath().getPathString(). */
@@ -794,13 +791,30 @@ public boolean isConstantMetadata() {
    * runfiles tree. For local targets, it returns the rootRelativePath.
    */
   public final PathFragment getRunfilesPath() {
-    PathFragment relativePath = getOutputDirRelativePath(false);
-    // We can't use root.isExternalSource() here since it needs to handle derived artifacts too.
-    if (relativePath.startsWith(LabelConstants.EXTERNAL_PATH_PREFIX)) {
-      // Turn external/repo/foo into ../repo/foo.
-      relativePath = relativePath.relativeTo(LabelConstants.EXTERNAL_PATH_PREFIX);
-      relativePath = LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX.getRelative(relativePath);
+    PathFragment relativePath = getRootRelativePath();
+    // Runfile paths for external artifacts should be prefixed with "../<repo name>".
+    if (root.isLegacy()) {
+      // Root-relative paths of external artifacts under legacy roots are already prefixed with
+      // "external/<repo name>". Just replace "external" with "..".
+      if (relativePath.startsWith(LabelConstants.EXTERNAL_PATH_PREFIX)) {
+        relativePath = relativePath.relativeTo(LabelConstants.EXTERNAL_PATH_PREFIX);
+        relativePath = LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX.getRelative(relativePath);
+      }
+    } else {
+      if (root.isExternal()) {
+        // Both external source artifacts and external derived artifacts have their repo name as
+        // their 2nd level directory name in their exec paths.
+        // i.e. external/<repo name>/... and bazel-out/<repo name>/...
+        // This is a pure coincidence, and the below line needs to be updated if any of the
+        // directory structures change.
+        String repoName = getExecPath().getSegment(1);
+        relativePath =
+            LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX
+                .getRelative(repoName)
+                .getRelative(relativePath);
+      }
     }
+    // We can't use root.isExternalSource() here since it needs to handle derived artifacts too.
     return relativePath;
   }
 
@@ -896,7 +910,7 @@ boolean ownersEqual(Artifact other) {
 
     @Override
     public PathFragment getRootRelativePath() {
-      return root.isExternalSourceRoot() ? getExecPath().subFragment(2) : getExecPath();
+      return root.isExternal() ? getExecPath().subFragment(2) : getExecPath();
     }
 
     @Override
@@ -1123,8 +1137,10 @@ private static ArtifactRoot createRootForArchivedArtifact(
         PathFragment customDerivedTreeRoot) {
       return ArtifactRoot.asDerivedRoot(
           getExecRoot(treeArtifactRoot),
-          // e.g. bazel-out/{customDerivedTreeRoot}/k8-fastbuild/bin
           false,
+          false,
+          false,
+          // e.g. bazel-out/{customDerivedTreeRoot}/k8-fastbuild/bin
           getExecPathWithinCustomDerivedRoot(
               derivedPathPrefix, customDerivedTreeRoot, treeArtifactRoot.getExecPath()));
     }

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

@@ -84,15 +84,20 @@ public static ArtifactRoot asExternalSourceRoot(Root root) {
    * <p>Be careful with this method - all derived roots must be within the derived artifacts tree,
    * defined in ArtifactFactory (see {@link ArtifactFactory#isDerivedArtifact(PathFragment)}).
    */
-  public static ArtifactRoot asDerivedRoot(Path execRoot, boolean isMiddleman, String... prefixes) {
+  public static ArtifactRoot asDerivedRoot(
+      Path execRoot,
+      boolean isMiddleman,
+      boolean isExternal,
+      boolean siblingRepositoryLayout,
+      String... prefixes) {
     PathFragment execPath = PathFragment.EMPTY_FRAGMENT;
     for (String prefix : prefixes) {
       // Tests can have empty segments here, be gentle to them.
       if (!prefix.isEmpty()) {
         execPath = execPath.getChild(prefix);
       }
     }
-    return asDerivedRoot(execRoot, isMiddleman, execPath);
+    return asDerivedRoot(execRoot, isMiddleman, isExternal, siblingRepositoryLayout, execPath);
   }
 
   /**
@@ -102,36 +107,67 @@ public static ArtifactRoot asDerivedRoot(Path execRoot, boolean isMiddleman, Str
    * defined in ArtifactFactory (see {@link ArtifactFactory#isDerivedArtifact(PathFragment)}).
    */
   public static ArtifactRoot asDerivedRoot(
-      Path execRoot, boolean isMiddleman, PathFragment execPath) {
+      Path execRoot,
+      boolean isMiddleman,
+      boolean isExternal,
+      boolean siblingRepositoryLayout,
+      PathFragment execPath) {
     // Make sure that we are not creating a derived artifact under the execRoot.
     Preconditions.checkArgument(!execPath.isEmpty(), "empty execPath");
     Preconditions.checkArgument(
         !execPath.getSegments().contains(".."),
         "execPath: %s contains parent directory reference (..)",
         execPath);
     Path root = execRoot.getRelative(execPath);
-    return INTERNER.intern(
-        new ArtifactRoot(
-            Root.fromPath(root), execPath, isMiddleman ? RootType.Middleman : RootType.Output));
+    RootType type;
+    if (siblingRepositoryLayout) {
+      type =
+          isMiddleman
+              ? (isExternal ? RootType.ExternalMiddleman : RootType.Middleman)
+              : (isExternal ? RootType.ExternalOutput : RootType.Output);
+    } else {
+      Preconditions.checkArgument(
+          !isExternal,
+          "external derived roots unavailable without --experimental_sibling_repository_layout");
+      type = isMiddleman ? RootType.LegacyMiddleman : RootType.LegacyOutput;
+    }
+    return INTERNER.intern(new ArtifactRoot(Root.fromPath(root), execPath, type));
   }
 
   @AutoCodec.VisibleForSerialization
   @AutoCodec.Instantiator
   static ArtifactRoot createForSerialization(
       Root rootForSerialization, PathFragment execPath, RootType rootType) {
-    if (rootType != RootType.Output) {
+    if (!isOutputRootType(rootType)) {
       return INTERNER.intern(new ArtifactRoot(rootForSerialization, execPath, rootType));
     }
     return asDerivedRoot(
-        rootForSerialization.asPath(), false, execPath.getSegments().toArray(new String[0]));
+        rootForSerialization.asPath(),
+        false,
+        rootType == RootType.ExternalOutput,
+        rootType != RootType.LegacyOutput,
+        execPath.getSegments().toArray(new String[0]));
   }
 
   @AutoCodec.VisibleForSerialization
   enum RootType {
     Source,
     Output,
     Middleman,
-    ExternalSource
+    // Root types for external repository artifacts. Note that ExternalOutput and ExternalMiddleman
+    // are activated only if --experimental_sibling_repository_layout is true, which will become
+    // the default value soon. The ExternalSource type is already in effect by default.
+    ExternalSource,
+    ExternalOutput,
+    ExternalMiddleman,
+    // Legacy root types for derived artifacts. Even though they're called legacy, they are still
+    // the default, but soon to be deprecated when --experimental_sibling_repository_layout is set
+    // true by default. In terms of the actual root paths, there are no differences between these
+    // and Output and Middleman. Their sole purpose is to embed the
+    // --experimental_sibling_repository_layout flag value information in Artifacts without
+    // additional storage cost.
+    LegacyOutput,
+    LegacyMiddleman
   }
 
   private final Root root;
@@ -171,15 +207,29 @@ public ImmutableList<String> getComponents() {
   }
 
   public boolean isSourceRoot() {
-    return rootType == RootType.Source || isExternalSourceRoot();
+    return rootType == RootType.Source || rootType == RootType.ExternalSource;
   }
 
-  public boolean isExternalSourceRoot() {
-    return rootType == RootType.ExternalSource;
+  private static boolean isOutputRootType(RootType rootType) {
+    return rootType == RootType.Output
+        || rootType == RootType.ExternalOutput
+        || rootType == RootType.LegacyOutput;
   }
 
   boolean isMiddlemanRoot() {
-    return rootType == RootType.Middleman;
+    return rootType == RootType.Middleman
+        || rootType == RootType.ExternalMiddleman
+        || rootType == RootType.LegacyMiddleman;
+  }
+
+  public boolean isExternal() {
+    return rootType == RootType.ExternalSource
+        || rootType == RootType.ExternalOutput
+        || rootType == RootType.ExternalMiddleman;
+  }
+
+  public boolean isLegacy() {
+    return rootType == RootType.LegacyOutput || rootType == RootType.LegacyMiddleman;
   }
 
   @Override
@@ -200,7 +250,7 @@ public int hashCode() {
    */
   @SuppressWarnings("unused") // Used by @AutoCodec.
   Root getRootForSerialization() {
-    if (rootType != RootType.Output) {
+    if (!isOutputRootType(rootType)) {
       return root;
     }
     // Find fragment of root that does not include execPath and return just that root. It is likely

src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java

@@ -206,7 +206,7 @@ public Path getEmbeddedBinariesRoot() {
    */
   public ArtifactRoot getBuildDataDirectory(String workspaceName) {
     return ArtifactRoot.asDerivedRoot(
-        getExecRoot(workspaceName), false, getRelativeOutputPath(productName));
+        getExecRoot(workspaceName), false, false, false, getRelativeOutputPath(productName));
   }
 
   /**

src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java

@@ -111,7 +111,13 @@ public ArtifactRoot getRoot(
       Path execRoot = directories.getExecRoot(mainRepositoryName.strippedName());
       // e.g., [[execroot/repo1]/bazel-out/config/bin]
       return ArtifactRoot.asDerivedRoot(
-          execRoot, middleman, directories.getRelativeOutputPath(), outputDirName, nameFragment);
+          execRoot,
+          middleman,
+          false,
+          false,
+          directories.getRelativeOutputPath(),
+          outputDirName,
+          nameFragment);
     }
   }
 
@@ -216,6 +222,8 @@ private ArtifactRoot buildDerivedRoot(
     return ArtifactRoot.asDerivedRoot(
         execRoot,
         isMiddleman,
+        !repository.isMain() && !repository.isDefault(),
+        true,
         directories.getRelativeOutputPath(),
         repository.strippedName(),
         outputDirName,

src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/actions/NinjaGraphArtifactsHelper.java

@@ -77,7 +77,7 @@ class NinjaGraphArtifactsHelper {
             .getExecRoot(ruleContext.getWorkspaceName());
     this.derivedOutputRoot =
         ArtifactRoot.asDerivedRoot(
-            execRoot, false, outputRootPath.getSegments().toArray(new String[0]));
+            execRoot, false, false, false, outputRootPath.getSegments().toArray(new String[0]));
     this.sourceRoot = ruleContext.getRule().getPackage().getSourceRoot().get();
   }
 

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

@@ -365,7 +365,11 @@ public void testDeletedConstantMetadataOutputCausesReexecution() throws Exceptio
     SpecialArtifact output =
         new Artifact.SpecialArtifact(
             ArtifactRoot.asDerivedRoot(
-                new InMemoryFileSystem(DigestHashFunction.SHA256).getPath("/output"), false, "bin"),
+                new InMemoryFileSystem(DigestHashFunction.SHA256).getPath("/output"),
+                false,
+                false,
+                false,
+                "bin"),
             PathFragment.create("bin/dummy"),
             ActionsTestUtil.NULL_ARTIFACT_OWNER,
             SpecialArtifactType.CONSTANT_METADATA);

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

@@ -75,7 +75,7 @@ public final void createFiles() throws Exception  {
     clientRoot = Root.fromPath(scratch.dir("/client/workspace"));
     clientRoRoot = Root.fromPath(scratch.dir("/client/RO/workspace"));
     alienRoot = Root.fromPath(scratch.dir("/client/workspace"));
-    outRoot = ArtifactRoot.asDerivedRoot(execRoot, false, "out-root", "x", "bin");
+    outRoot = ArtifactRoot.asDerivedRoot(execRoot, false, false, false, "out-root", "x", "bin");
 
     fooPath = PathFragment.create("foo");
     fooPackage = PackageIdentifier.createInMainRepo(fooPath);