Change source artifact roots of external packages to $OUTPUT_BASE/ext…

…ernal. This is the first step for releasing the special 'external' package name for general use. This CL alone should not have any user-visible impacts.

PiperOrigin-RevId: 332451781

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

@@ -278,7 +278,7 @@ private static GeneratingActions assignOwnersAndMaybeFilterSharedActionsAndThrow
         outputFileNames != null
             ? Preconditions.checkNotNull(label, actionLookupKey)
                 .getPackageIdentifier()
-                .getSourceRoot()
+                .getPackagePath()
             : null;
     // Loop over the actions, looking at all outputs for conflicts.
     int actionIndex = 0;
@@ -309,9 +309,9 @@ private static GeneratingActions assignOwnersAndMaybeFilterSharedActionsAndThrow
         } else {
           // No: populate the output label map with this artifact if applicable: if this
           // artifact corresponds to a target that is an OutputFile with associated rule this label.
-          PathFragment rootRelativePath = output.getRootRelativePath();
-          if (packageDirectory != null && rootRelativePath.startsWith(packageDirectory)) {
-            PathFragment packageRelativePath = rootRelativePath.relativeTo(packageDirectory);
+          PathFragment packagePath = output.getPackagePath();
+          if (packageDirectory != null && packagePath.startsWith(packageDirectory)) {
+            PathFragment packageRelativePath = packagePath.relativeTo(packageDirectory);
             Label outputLabel = outputFileNames.get(packageRelativePath.getPathString());
             if (outputLabel != null) {
               artifactsByOutputLabel.put(outputLabel, artifact);

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

@@ -293,8 +293,9 @@ public ArchivedTreeArtifact getArchivedTreeArtifact(SpecialArtifact treeArtifact
   /** A Predicate that evaluates to true if the Artifact is not a middleman artifact. */
   public static final Predicate<Artifact> MIDDLEMAN_FILTER = input -> !input.isMiddlemanArtifact();
 
+  protected final ArtifactRoot root;
+
   private final int hashCode;
-  private final ArtifactRoot root;
   private final PathFragment execPath;
 
   private Artifact(ArtifactRoot root, PathFragment execPath) {
@@ -610,11 +611,24 @@ public final PathFragment getExecPath() {
   }
 
   /**
-   * Returns the relative path to this artifact relative to its root. (Useful when deriving output
-   * filenames from input files, etc.)
+   * Returns the relative path to this artifact relative to its root. It makes no guarantees as to
+   * the semantic meaning or the completeness of the returned path value. In other words, no
+   * assumptions should be made in terms of where the root portion of the path ends, and the
+   * returned value almost always needs to be used in conjunction with its root.
+   *
+   * <p>{#link Artifact#getPackagePath()} is more versatile for general use cases.
    */
   public abstract PathFragment getRootRelativePath();
 
+  /**
+   * Returns the fully-qualified package path to this artifact. By "fully-qualified", it means the
+   * returned path is prefixed with "external/<repository name>" if this artifact is in an external
+   * repository.
+   */
+  public PathFragment getPackagePath() {
+    return getRootRelativePath();
+  }
+
   /** Returns this.getExecPath().getPathString(). */
   @Override
   public final String getExecPathString() {
@@ -625,6 +639,10 @@ public final String getRootRelativePathString() {
     return getRootRelativePath().getPathString();
   }
 
+  public final String getPackagePathString() {
+    return getPackagePath().getPathString();
+  }
+
   @Override
   public boolean contentBasedPath() {
     return false;
@@ -734,7 +752,8 @@ public boolean isConstantMetadata() {
    * runfiles tree. For local targets, it returns the rootRelativePath.
    */
   public final PathFragment getRunfilesPath() {
-    PathFragment relativePath = getRootRelativePath();
+    PathFragment relativePath = getPackagePath();
+    // 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);
@@ -835,11 +854,11 @@ boolean ownersEqual(Artifact other) {
 
     @Override
     public PathFragment getRootRelativePath() {
-      // flag-less way of checking of the root is <execroot>/.., or sibling of __main__.
-      if (getExecPath().startsWith(LabelConstants.EXPERIMENTAL_EXTERNAL_PATH_PREFIX)) {
-        return LabelConstants.EXTERNAL_PATH_PREFIX.getRelative(getExecPath().subFragment(1));
-      }
+      return root.isExternalSourceRoot() ? getExecPath().subFragment(1) : getExecPath();
+    }
 
+    @Override
+    public PathFragment getPackagePath() {
       return getExecPath();
     }
 
@@ -1260,6 +1279,9 @@ public SourceArtifact deserialize(DeserializationContext context, CodedInputStre
   public static final Function<Artifact, String> ROOT_RELATIVE_PATH_STRING =
       artifact -> artifact.getRootRelativePath().getPathString();
 
+  public static final Function<Artifact, String> PACKAGE_PATH_STRING =
+      artifact -> artifact.getPackagePath().getPathString();
+
   /**
    * Converts a collection of artifacts into execution-time path strings, and
    * adds those to a given collection. Middleman artifacts are ignored by this
@@ -1310,6 +1332,24 @@ public static Iterable<String> toExecPaths(Iterable<Artifact> artifacts) {
         Iterables.filter(artifacts, MIDDLEMAN_FILTER), ActionInput::getExecPathString);
   }
 
+  /**
+   * Lazily converts artifacts into package path strings. Middleman artifacts are ignored by this
+   * method.
+   */
+  public static Iterable<String> toPackagePaths(NestedSet<Artifact> artifacts) {
+    return toPackagePaths(artifacts.toList());
+  }
+
+  /**
+   * Lazily converts artifacts into package path strings. Middleman artifacts are ignored by this
+   * method.
+   */
+  public static Iterable<String> toPackagePaths(Iterable<Artifact> artifacts) {
+    return Iterables.transform(
+        Iterables.filter(artifacts, MIDDLEMAN_FILTER),
+        artifact -> artifact.getPackagePath().getPathString());
+  }
+
   /**
    * Converts a collection of artifacts into execution-time path strings, and returns those as an
    * immutable list. Middleman artifacts are ignored by this method.

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

@@ -18,6 +18,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Interner;
 import com.google.common.collect.Interners;
+import com.google.devtools.build.lib.cmdline.LabelConstants;
 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.starlarkbuildapi.FileRootApi;
@@ -58,6 +59,20 @@ public static ArtifactRoot asSourceRoot(Root root) {
     return new ArtifactRoot(root, PathFragment.EMPTY_FRAGMENT, RootType.Source);
   }
 
+  /**
+   * Do not use except in tests and in {@link
+   * com.google.devtools.build.lib.skyframe.SkyframeExecutor}.
+   *
+   * <p>Returns the given path as the external source root. The path should end with {@link
+   * LabelConstants.EXTERNAL_REPOSITORY_LOCATION} since the external repository root is always
+   * $OUTPUT_BASE/external regardless of the layout of the exec root.
+   */
+  public static ArtifactRoot asExternalSourceRoot(Root root) {
+    Preconditions.checkArgument(
+        root.asPath().asFragment().endsWith(LabelConstants.EXTERNAL_REPOSITORY_LOCATION));
+    return new ArtifactRoot(root, PathFragment.EMPTY_FRAGMENT, RootType.ExternalSource);
+  }
+
   /**
    * Constructs an ArtifactRoot given the output prefixes. (eg, "bin"), and (eg, "testlogs")
    * relative to the execRoot.
@@ -116,7 +131,8 @@ static ArtifactRoot createForSerialization(
   enum RootType {
     Source,
     Output,
-    Middleman
+    Middleman,
+    ExternalSource
   }
 
   private final Root root;
@@ -156,7 +172,11 @@ public ImmutableList<String> getComponents() {
   }
 
   public boolean isSourceRoot() {
-    return rootType == RootType.Source;
+    return rootType == RootType.Source || isExternalSourceRoot();
+  }
+
+  public boolean isExternalSourceRoot() {
+    return rootType == RootType.ExternalSource;
   }
 
   boolean isMiddlemanRoot() {

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

@@ -159,7 +159,10 @@ public static PathFragment getManifestPathFromFilesetPath(PathFragment filesetDi
    * <p>For example "//pkg:target" -> "pkg/&lt;fragment&gt;/target.
    */
   public static PathFragment getUniqueDirectory(Label label, PathFragment fragment) {
-    return label.getPackageIdentifier().getSourceRoot().getRelative(fragment)
+    return label
+        .getPackageIdentifier()
+        .getPackagePath()
+        .getRelative(fragment)
         .getRelative(label.getName());
   }
 

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

@@ -63,7 +63,7 @@ public final class LocationExpander {
   private static final boolean EXACTLY_ONE = false;
   private static final boolean ALLOW_MULTIPLE = true;
 
-  private static final boolean USE_ROOT_PATHS = false;
+  private static final boolean USE_PACKAGE_PATHS = false;
   private static final boolean USE_EXEC_PATHS = true;
 
   private final RuleErrorConsumer ruleErrorConsumer;
@@ -95,7 +95,7 @@ private LocationExpander(
    * @param ruleContext BUILD rule
    * @param labelMap A mapping of labels to build artifacts.
    * @param execPaths If true, this expander will expand $(location)/$(locations) using
-   *     Artifact.getExecPath(); otherwise with Artifact.getRootRelativePath().
+   *     Artifact.getExecPath(); otherwise with Artifact.getPackagePath().
    * @param allowData If true, this expander will expand locations from the `data` attribute;
    *     otherwise it will not.
    */
@@ -115,9 +115,9 @@ private LocationExpander(
   }
 
   /**
-   * Creates an expander that expands $(location)/$(locations) using Artifact.getRootRelativePath().
+   * Creates an expander that expands $(location)/$(locations) using Artifact.getPackagePath().
    *
-   * <p>The expander expands $(rootpath)/$(rootpaths) using Artifact.getRootRelativePath(), and
+   * <p>The expander expands $(rootpath)/$(rootpaths) using Artifact.getPackagePath(), and
    * $(execpath)/$(execpaths) using Artifact.getExecPath().
    *
    * @param ruleContext BUILD rule
@@ -129,7 +129,7 @@ public static LocationExpander withRunfilesPaths(RuleContext ruleContext) {
   /**
    * Creates an expander that expands $(location)/$(locations) using Artifact.getExecPath().
    *
-   * <p>The expander expands $(rootpath)/$(rootpaths) using Artifact.getRootRelativePath(), and
+   * <p>The expander expands $(rootpath)/$(rootpaths) using Artifact.getPackagePath(), and
    * $(execpath)/$(execpaths) using Artifact.getExecPath().
    *
    * @param ruleContext BUILD rule
@@ -143,7 +143,7 @@ public static LocationExpander withExecPaths(
   /**
    * Creates an expander that expands $(location)/$(locations) using Artifact.getExecPath().
    *
-   * <p>The expander expands $(rootpath)/$(rootpaths) using Artifact.getRootRelativePath(), and
+   * <p>The expander expands $(rootpath)/$(rootpaths) using Artifact.getPackagePath(), and
    * $(execpath)/$(execpaths) using Artifact.getExecPath().
    *
    * @param ruleContext BUILD rule
@@ -301,18 +301,16 @@ private Collection<String> resolveLabel(Label unresolved) throws IllegalStateExc
     }
 
     /**
-     * Extracts list of all executables associated with given collection of label
-     * artifacts.
+     * Extracts list of all executables associated with given collection of label artifacts.
      *
      * @param artifacts to get the paths of
-     * @param takeExecPath if false, the root relative path will be taken
+     * @param takeExecPath if false, the package path will be taken
      * @return all associated executable paths
      */
     private Set<String> getPaths(Collection<Artifact> artifacts, boolean takeExecPath) {
       TreeSet<String> paths = Sets.newTreeSet();
       for (Artifact artifact : artifacts) {
-        PathFragment execPath =
-            takeExecPath ? artifact.getExecPath() : artifact.getRootRelativePath();
+        PathFragment execPath = takeExecPath ? artifact.getExecPath() : artifact.getPackagePath();
         if (execPath != null) {  // omit middlemen etc
           paths.add(execPath.getCallablePathString());
         }
@@ -334,8 +332,9 @@ static ImmutableMap<String, LocationFunction> allLocationFunctions(
     return new ImmutableMap.Builder<String, LocationFunction>()
         .put("location", new LocationFunction(root, locationMap, execPaths, EXACTLY_ONE))
         .put("locations", new LocationFunction(root, locationMap, execPaths, ALLOW_MULTIPLE))
-        .put("rootpath", new LocationFunction(root, locationMap, USE_ROOT_PATHS, EXACTLY_ONE))
-        .put("rootpaths", new LocationFunction(root, locationMap, USE_ROOT_PATHS, ALLOW_MULTIPLE))
+        .put("rootpath", new LocationFunction(root, locationMap, USE_PACKAGE_PATHS, EXACTLY_ONE))
+        .put(
+            "rootpaths", new LocationFunction(root, locationMap, USE_PACKAGE_PATHS, ALLOW_MULTIPLE))
         .put("execpath", new LocationFunction(root, locationMap, USE_EXEC_PATHS, EXACTLY_ONE))
         .put("execpaths", new LocationFunction(root, locationMap, USE_EXEC_PATHS, ALLOW_MULTIPLE))
         .build();

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

@@ -791,7 +791,7 @@ private Artifact getPackageRelativeArtifact(
 
   @Override
   public PathFragment getPackageDirectory() {
-    return getLabel().getPackageIdentifier().getSourceRoot();
+    return getLabel().getPackageIdentifier().getPackagePath();
   }
 
   /**

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

@@ -299,7 +299,7 @@ public NestedSet<String> getEmptyFilenames() {
     Set<PathFragment> manifestKeys =
         Streams.concat(
                 symlinks.toList().stream().map(SymlinkEntry::getPath),
-                getArtifacts().toList().stream().map(Artifact::getRootRelativePath))
+                getArtifacts().toList().stream().map(Artifact::getPackagePath))
             .collect(ImmutableSet.toImmutableSet());
     Iterable<PathFragment> emptyKeys = emptyFilesSupplier.getExtraPaths(manifestKeys);
     return NestedSetBuilder.<String>stableOrder()
@@ -385,7 +385,7 @@ public Map<PathFragment, Artifact> getRunfilesInputs(
     Map<PathFragment, Artifact> manifest = getSymlinksAsMap(checker);
     // Add artifacts (committed to inclusion on construction of runfiles).
     for (Artifact artifact : getArtifacts().toList()) {
-      checker.put(manifest, artifact.getRootRelativePath(), artifact);
+      checker.put(manifest, artifact.getPackagePath(), artifact);
     }
 
     manifest = filterListForObscuringSymlinks(eventHandler, location, manifest);
@@ -523,7 +523,7 @@ public Map<PathFragment, Artifact> asMapWithoutRootSymlinks() {
     // That is because the runfiles tree cannot contain the same artifact for different
     // configurations, because it only uses root-relative paths.
     for (Artifact artifact : artifacts.toList()) {
-      result.put(artifact.getRootRelativePath(), artifact);
+      result.put(artifact.getPackagePath(), artifact);
     }
     return result;
   }
@@ -1074,7 +1074,7 @@ public void fingerprint(Fingerprint fp) {
     }
 
     for (Artifact artifact : getArtifacts().toList()) {
-      fp.addPath(artifact.getRootRelativePath());
+      fp.addPath(artifact.getPackagePath());
       fp.addPath(artifact.getExecPath());
     }
 

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java

@@ -121,7 +121,7 @@ public Artifact declareFile(String filename, Object sibling) throws EvalExceptio
     if (Starlark.NONE.equals(sibling)) {
       fragment = ruleContext.getPackageDirectory().getRelative(PathFragment.create(filename));
     } else {
-      PathFragment original = ((Artifact) sibling).getRootRelativePath();
+      PathFragment original = ((Artifact) sibling).getPackagePath();
       fragment = original.replaceName(filename);
     }
 
@@ -141,7 +141,7 @@ public Artifact declareDirectory(String filename, Object sibling) throws EvalExc
     if (Starlark.NONE.equals(sibling)) {
       fragment = ruleContext.getPackageDirectory().getRelative(PathFragment.create(filename));
     } else {
-      PathFragment original = ((Artifact) sibling).getRootRelativePath();
+      PathFragment original = ((Artifact) sibling).getPackagePath();
       fragment = original.replaceName(filename);
     }
 
@@ -174,7 +174,7 @@ public Artifact declareSymlink(String filename, Object sibling) throws EvalExcep
     if (Starlark.NONE.equals(sibling)) {
       rootRelativePath = ruleContext.getPackageDirectory().getRelative(filename);
     } else {
-      PathFragment original = ((Artifact) sibling).getRootRelativePath();
+      PathFragment original = ((Artifact) sibling).getPackagePath();
       rootRelativePath = original.replaceName(filename);
     }
 
@@ -234,7 +234,7 @@ public void symlink(
     String progressMessage =
         (progressMessageUnchecked != Starlark.NONE)
             ? (String) progressMessageUnchecked
-            : "Creating symlink " + outputArtifact.getRootRelativePathString();
+            : "Creating symlink " + outputArtifact.getPackagePathString();
 
     SymlinkAction action;
     if (targetFile != Starlark.NONE) {

src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java

@@ -337,7 +337,7 @@ public Artifact createStubAction(
                   + "\nexport TEST_RUNTIME_CLASSPATH_FILE=${JAVA_RUNFILES}"
                   + File.separator
                   + workspacePrefix
-                  + testRuntimeClasspathArtifact.getRootRelativePathString()));
+                  + testRuntimeClasspathArtifact.getPackagePathString()));
     } else {
       arguments.add(
           new ComputedClasspathSubstitution(classpath, workspacePrefix, isRunfilesEnabled));
@@ -365,7 +365,7 @@ public Artifact createStubAction(
                 "export JACOCO_METADATA_JAR=${JAVA_RUNFILES}/"
                     + workspacePrefix
                     + "/"
-                    + runtimeClassPathArtifact.getRootRelativePathString()));
+                    + runtimeClassPathArtifact.getPackagePathString()));
       } else {
         // Remove the placeholder in the stub otherwise bazel coverage fails.
         arguments.add(Substitution.of(JavaSemantics.JACOCO_METADATA_PLACEHOLDER, ""));
@@ -436,7 +436,7 @@ private static Artifact createWindowsExeLauncher(
             .addJoinedValues(
                 "classpath",
                 ";",
-                Iterables.transform(classpath.toList(), Artifact.ROOT_RELATIVE_PATH_STRING))
+                Iterables.transform(classpath.toList(), Artifact.PACKAGE_PATH_STRING))
             // TODO(laszlocsomor): Change the Launcher to accept multiple jvm_flags entries. As of
             // 2019-02-13 the Launcher accepts just one jvm_flags entry, which contains all the
             // flags, joined by TAB characters. The Launcher splits up the string to get the
@@ -715,7 +715,7 @@ public List<String> getExtraArguments(RuleContext ruleContext, ImmutableList<Art
             && !ruleContext.attributes().isAttributeValueExplicitlySpecified("args")) {
           ImmutableList.Builder<String> builder = ImmutableList.builder();
           for (Artifact artifact : sources) {
-            PathFragment path = artifact.getRootRelativePath();
+            PathFragment path = artifact.getPackagePath();
             String className = JavaUtil.getJavaFullClassname(FileSystemUtils.removeExtension(path));
             if (className != null) {
               builder.add(className);