Simplify asDerivedRoot's signature by replacing boolean parameters wi…

…th a RootType object.

PiperOrigin-RevId: 354214100

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

@@ -29,6 +29,7 @@
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Streams;
 import com.google.devtools.build.lib.actions.ArtifactResolver.ArtifactResolverSupplier;
+import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelConstants;
 import com.google.devtools.build.lib.collect.nestedset.Depset;
@@ -1137,10 +1138,8 @@ private static ArtifactRoot createRootForArchivedArtifact(
         PathFragment customDerivedTreeRoot) {
       return ArtifactRoot.asDerivedRoot(
           getExecRoot(treeArtifactRoot),
-          false,
-          false,
-          false,
           // e.g. bazel-out/{customDerivedTreeRoot}/k8-fastbuild/bin
+          RootType.Output,
           getExecPathWithinCustomDerivedRoot(
               derivedPathPrefix, customDerivedTreeRoot, treeArtifactRoot.getExecPath()));
     }

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

@@ -56,7 +56,8 @@ public final class ArtifactRoot implements Comparable<ArtifactRoot>, Serializabl
    * <p>Returns the given path as a source root. The path may not be {@code null}.
    */
   public static ArtifactRoot asSourceRoot(Root root) {
-    return INTERNER.intern(new ArtifactRoot(root, PathFragment.EMPTY_FRAGMENT, RootType.Source));
+    return INTERNER.intern(
+        new ArtifactRoot(root, PathFragment.EMPTY_FRAGMENT, RootType.MainSource));
   }
 
   /**
@@ -84,20 +85,15 @@ 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,
-      boolean isExternal,
-      boolean siblingRepositoryLayout,
-      String... prefixes) {
+  public static ArtifactRoot asDerivedRoot(Path execRoot, RootType rootType, 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, isExternal, siblingRepositoryLayout, execPath);
+    return asDerivedRoot(execRoot, rootType, execPath);
   }
 
   /**
@@ -107,31 +103,19 @@ public static ArtifactRoot asDerivedRoot(
    * defined in ArtifactFactory (see {@link ArtifactFactory#isDerivedArtifact(PathFragment)}).
    */
   public static ArtifactRoot asDerivedRoot(
-      Path execRoot,
-      boolean isMiddleman,
-      boolean isExternal,
-      boolean siblingRepositoryLayout,
-      PathFragment execPath) {
+      Path execRoot, RootType rootType, 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);
+    Preconditions.checkArgument(
+        isOutputRootType(rootType) || isMiddlemanRootType(rootType),
+        "%s is not a derived root type",
+        rootType);
     Path root = execRoot.getRelative(execPath);
-    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));
+    return INTERNER.intern(new ArtifactRoot(Root.fromPath(root), execPath, rootType));
   }
 
   @AutoCodec.VisibleForSerialization
@@ -142,32 +126,25 @@ static ArtifactRoot createForSerialization(
       return INTERNER.intern(new ArtifactRoot(rootForSerialization, execPath, rootType));
     }
     return asDerivedRoot(
-        rootForSerialization.asPath(),
-        false,
-        rootType == RootType.ExternalOutput,
-        rootType != RootType.LegacyOutput,
-        execPath.getSegments().toArray(new String[0]));
+        rootForSerialization.asPath(), rootType, execPath.getSegments().toArray(new String[0]));
   }
 
-  @AutoCodec.VisibleForSerialization
-  enum RootType {
-    Source,
+  /**
+   * ArtifactRoot types. Callers of asDerivedRoot methods need to specify which type of derived root
+   * artifact they want to create, which is why this enum is public.
+   */
+  public enum RootType {
+    MainSource,
+    ExternalSource,
     Output,
     Middleman,
-    // 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
+    // Sibling root types are in effect when --experimental_sibling_repository_layout is activated.
+    // These will eventually replace the above Output and Middleman types when the flag becomes
+    // the default option and then removed.
+    SiblingMainOutput,
+    SiblingMainMiddleman,
+    SiblingExternalOutput,
+    SiblingExternalMiddleman,
   }
 
   private final Root root;
@@ -207,29 +184,37 @@ public ImmutableList<String> getComponents() {
   }
 
   public boolean isSourceRoot() {
-    return rootType == RootType.Source || rootType == RootType.ExternalSource;
+    return rootType == RootType.MainSource || rootType == RootType.ExternalSource;
   }
 
   private static boolean isOutputRootType(RootType rootType) {
-    return rootType == RootType.Output
-        || rootType == RootType.ExternalOutput
-        || rootType == RootType.LegacyOutput;
+    return rootType == RootType.SiblingMainOutput
+        || rootType == RootType.SiblingExternalOutput
+        || rootType == RootType.Output;
+  }
+
+  private static boolean isMiddlemanRootType(RootType rootType) {
+    return rootType == RootType.SiblingMainMiddleman
+        || rootType == RootType.SiblingExternalMiddleman
+        || rootType == RootType.Middleman;
   }
 
   boolean isMiddlemanRoot() {
-    return rootType == RootType.Middleman
-        || rootType == RootType.ExternalMiddleman
-        || rootType == RootType.LegacyMiddleman;
+    return isMiddlemanRootType(rootType);
   }
 
   public boolean isExternal() {
     return rootType == RootType.ExternalSource
-        || rootType == RootType.ExternalOutput
-        || rootType == RootType.ExternalMiddleman;
+        || rootType == RootType.SiblingExternalOutput
+        || rootType == RootType.SiblingExternalMiddleman;
   }
 
+  /**
+   * Returns true if the ArtifactRoot is a legacy derived root type, i.e. a derived root type
+   * created without the --experimental_sibling_repository_layout flag set.
+   */
   public boolean isLegacy() {
-    return rootType == RootType.LegacyOutput || rootType == RootType.LegacyMiddleman;
+    return rootType == RootType.Output || rootType == RootType.Middleman;
   }
 
   @Override

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

@@ -18,6 +18,7 @@
 import com.google.common.base.Ascii;
 import com.google.common.hash.HashCode;
 import com.google.devtools.build.lib.actions.ArtifactRoot;
+import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.util.StringCanonicalizer;
@@ -206,7 +207,7 @@ public Path getEmbeddedBinariesRoot() {
    */
   public ArtifactRoot getBuildDataDirectory(String workspaceName) {
     return ArtifactRoot.asDerivedRoot(
-        getExecRoot(workspaceName), false, false, false, getRelativeOutputPath(productName));
+        getExecRoot(workspaceName), RootType.Output, getRelativeOutputPath(productName));
   }
 
   /**

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

@@ -18,6 +18,7 @@
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSortedMap;
 import com.google.devtools.build.lib.actions.ArtifactRoot;
+import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.server.FailureDetails.BuildConfiguration.Code;
@@ -112,9 +113,7 @@ public ArtifactRoot getRoot(
       // e.g., [[execroot/repo1]/bazel-out/config/bin]
       return ArtifactRoot.asDerivedRoot(
           execRoot,
-          middleman,
-          false,
-          false,
+          middleman ? RootType.Middleman : RootType.Output,
           directories.getRelativeOutputPath(),
           outputDirName,
           nameFragment);
@@ -219,11 +218,15 @@ private ArtifactRoot buildDerivedRoot(
     // TODO(jungjw): Ideally, we would like to do execroot_base/repoName/bazel-out/config/bin
     // instead. However, it requires individually symlinking the top-level elements of external
     // repositories, which is blocked by a Windows symlink issue #8704.
+    RootType rootType;
+    if (repository.isMain() || repository.isDefault()) {
+      rootType = isMiddleman ? RootType.SiblingMainMiddleman : RootType.SiblingMainOutput;
+    } else {
+      rootType = isMiddleman ? RootType.SiblingExternalMiddleman : RootType.SiblingExternalOutput;
+    }
     return ArtifactRoot.asDerivedRoot(
         execRoot,
-        isMiddleman,
-        !repository.isMain() && !repository.isDefault(),
-        true,
+        rootType,
         directories.getRelativeOutputPath(),
         repository.strippedName(),
         outputDirName,

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

@@ -21,6 +21,7 @@
 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.analysis.RuleContext;
 import com.google.devtools.build.lib.bazel.rules.ninja.file.GenericParsingException;
 import com.google.devtools.build.lib.vfs.Path;
@@ -77,7 +78,7 @@ class NinjaGraphArtifactsHelper {
             .getExecRoot(ruleContext.getWorkspaceName());
     this.derivedOutputRoot =
         ArtifactRoot.asDerivedRoot(
-            execRoot, false, false, false, outputRootPath.getSegments().toArray(new String[0]));
+            execRoot, RootType.Output, outputRootPath.getSegments().toArray(new String[0]));
     this.sourceRoot = ruleContext.getRule().getPackage().getSourceRoot().get();
   }
 

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

@@ -23,6 +23,7 @@
 import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
 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.ArtifactRoot.RootType;
 import com.google.devtools.build.lib.actions.cache.ActionCache;
 import com.google.devtools.build.lib.actions.cache.CompactPersistentActionCache;
 import com.google.devtools.build.lib.actions.cache.MetadataHandler;
@@ -366,9 +367,7 @@ public void testDeletedConstantMetadataOutputCausesReexecution() throws Exceptio
         new Artifact.SpecialArtifact(
             ArtifactRoot.asDerivedRoot(
                 new InMemoryFileSystem(DigestHashFunction.SHA256).getPath("/output"),
-                false,
-                false,
-                false,
+                RootType.Output,
                 "bin"),
             PathFragment.create("bin/dummy"),
             ActionsTestUtil.NULL_ARTIFACT_OWNER,

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

@@ -22,6 +22,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
+import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
 import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
@@ -75,7 +76,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, false, false, "out-root", "x", "bin");
+    outRoot = ArtifactRoot.asDerivedRoot(execRoot, RootType.Output, "out-root", "x", "bin");
 
     fooPath = PathFragment.create("foo");
     fooPackage = PackageIdentifier.createInMainRepo(fooPath);