Make execution root symlink builder handle PackageIdentifiers

Part of the rollforward for #1262.

--
MOS_MIGRATED_REVID=125562871

4 files changed, 87 insertions, 60 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
index a11cd65461..034651422b 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -18,7 +18,6 @@ import com.google.common.base.Joiner;
 import com.google.common.base.Stopwatch;
 import com.google.common.base.Throwables;
 import com.google.common.base.Verify;
-import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.actions.BuildFailedException;
 import com.google.devtools.build.lib.actions.TestExecException;
 import com.google.devtools.build.lib.analysis.AnalysisPhaseCompleteEvent;
@@ -73,11 +72,8 @@ import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.util.ExitCode;
 import com.google.devtools.build.lib.util.Preconditions;
-import com.google.devtools.build.lib.vfs.Path;
-import com.google.devtools.build.lib.vfs.PathFragment;
 
 import java.util.Collection;
-import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
@@ -204,7 +200,7 @@ public final class BuildTool {
       if (needsExecutionPhase(request.getBuildOptions())) {
         env.getSkyframeExecutor().injectTopLevelContext(request.getTopLevelArtifactContext());
         executionTool.executeBuild(request.getId(), analysisResult, result,
-            configurations, transformPackageRoots(analysisResult.getPackageRoots()));
+            configurations, analysisResult.getPackageRoots());
       }
 
       String delayedErrorMsg = analysisResult.getError();
@@ -299,15 +295,6 @@ public final class BuildTool {
     }
   }
 
-  private ImmutableMap<PathFragment, Path> transformPackageRoots(
-      ImmutableMap<PackageIdentifier, Path> packageRoots) {
-    ImmutableMap.Builder<PathFragment, Path> builder = ImmutableMap.builder();
-    for (Map.Entry<PackageIdentifier, Path> entry : packageRoots.entrySet()) {
-      builder.put(entry.getKey().getPathFragment(), entry.getValue());
-    }
-    return builder.build();
-  }
-
   private void reportExceptionError(Exception e) {
     if (e.getMessage() != null) {
       getReporter().handle(Event.error(e.getMessage()));
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
index 76706bc538..0a8a0cbbb9 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
@@ -61,6 +61,7 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
 import com.google.devtools.build.lib.buildtool.buildevent.ExecutionPhaseCompleteEvent;
 import com.google.devtools.build.lib.buildtool.buildevent.ExecutionStartingEvent;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.events.EventKind;
@@ -335,7 +336,7 @@ public class ExecutionTool {
   void executeBuild(UUID buildId, AnalysisResult analysisResult,
       BuildResult buildResult,
       BuildConfigurationCollection configurations,
-      ImmutableMap<PathFragment, Path> packageRoots)
+      ImmutableMap<PackageIdentifier, Path> packageRoots)
       throws BuildFailedException, InterruptedException, TestExecException, AbruptExitException {
     Stopwatch timer = Stopwatch.createStarted();
     prepare(packageRoots);
@@ -495,7 +496,7 @@ public class ExecutionTool {
     }
   }
 
-  private void prepare(ImmutableMap<PathFragment, Path> packageRoots)
+  private void prepare(ImmutableMap<PackageIdentifier, Path> packageRoots)
       throws ExecutorInitException {
     // Prepare for build.
     Profiler.instance().markPhase(ProfilePhase.PREPARE);
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java b/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java
index ceeb77a726..5c8ff90565 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java
@@ -16,9 +16,13 @@ package com.google.devtools.build.lib.buildtool;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.concurrent.ThreadSafety;
+import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -37,13 +41,13 @@ class SymlinkForest {
   private static final Logger LOG = Logger.getLogger(SymlinkForest.class.getName());
   private static final boolean LOG_FINER = LOG.isLoggable(Level.FINER);
 
-  private final ImmutableMap<PathFragment, Path> packageRoots;
+  private final ImmutableMap<PackageIdentifier, Path> packageRoots;
   private final Path workspace;
   private final String productName;
   private final String[] prefixes;
 
   SymlinkForest(
-      ImmutableMap<PathFragment, Path> packageRoots, Path workspace, String productName) {
+      ImmutableMap<PackageIdentifier, Path> packageRoots, Path workspace, String productName) {
     this.packageRoots = packageRoots;
     this.workspace = workspace;
     this.productName = productName;
@@ -54,11 +58,14 @@ class SymlinkForest {
    * Returns the longest prefix from a given set of 'prefixes' that are
    * contained in 'path'. I.e the closest ancestor directory containing path.
    * Returns null if none found.
+   * @param path
+   * @param prefixes
    */
   @VisibleForTesting
-  static PathFragment longestPathPrefix(PathFragment path, Set<PathFragment> prefixes) {
-    for (int i = path.segmentCount(); i >= 0; i--) {
-      PathFragment prefix = path.subFragment(0, i);
+  static PackageIdentifier longestPathPrefix(
+      PackageIdentifier path, ImmutableSet<PackageIdentifier> prefixes) {
+    for (int i = path.getPackageFragment().segmentCount(); i >= 0; i--) {
+      PackageIdentifier prefix = createInRepo(path, path.getPackageFragment().subFragment(0, i));
       if (prefixes.contains(prefix)) {
         return prefix;
       }
@@ -76,8 +83,8 @@ class SymlinkForest {
     dirloop:
     for (Path p : dir.getDirectoryEntries()) {
       String name = p.getBaseName();
-      for (int i = 0; i < prefixes.length; i++) {
-        if (name.startsWith(prefixes[i])) {
+      for (String prefix : prefixes) {
+        if (name.startsWith(prefix)) {
           continue dirloop;
         }
       }
@@ -87,20 +94,20 @@ class SymlinkForest {
 
   void plantSymlinkForest() throws IOException {
     deleteTreesBelowNotPrefixed(workspace, prefixes);
-    Path emptyPackagePath = null;
 
     // Create a sorted map of all dirs (packages and their ancestors) to sets of their roots.
     // Packages come from exactly one root, but their shared ancestors may come from more.
     // The map is maintained sorted lexicographically, so parents are before their children.
-    Map<PathFragment, Set<Path>> dirRootsMap = Maps.newTreeMap();
-    for (Map.Entry<PathFragment, Path> entry : packageRoots.entrySet()) {
-      PathFragment pkgDir = entry.getKey();
+    Map<PackageIdentifier, Set<Path>> dirRootsMap = Maps.newTreeMap();
+    for (Map.Entry<PackageIdentifier, Path> entry : packageRoots.entrySet()) {
+      PackageIdentifier pkgId = entry.getKey();
       Path pkgRoot = entry.getValue();
-      if (pkgDir.segmentCount() == 0) {
-        emptyPackagePath = entry.getValue();
-      }
-      for (int i = 1; i <= pkgDir.segmentCount(); i++) {
-        PathFragment dir = pkgDir.subFragment(0, i);
+      for (int i = 1; i <= pkgId.getPackageFragment().segmentCount(); i++) {
+        if (pkgId.equals(Label.EXTERNAL_PACKAGE_IDENTIFIER)) {
+          // This isn't a "real" package, don't add it to the symlink tree.
+          continue;
+        }
+        PackageIdentifier dir = createInRepo(pkgId, pkgId.getPackageFragment().subFragment(0, i));
         Set<Path> roots = dirRootsMap.get(dir);
         if (roots == null) {
           roots = Sets.newHashSet();
@@ -110,59 +117,68 @@ class SymlinkForest {
       }
     }
     // Now add in roots for all non-pkg dirs that are in between two packages, and missed above.
-    for (Map.Entry<PathFragment, Set<Path>> entry : dirRootsMap.entrySet()) {
-      PathFragment dir = entry.getKey();
+    for (Map.Entry<PackageIdentifier, Set<Path>> entry : dirRootsMap.entrySet()) {
+      PackageIdentifier dir = entry.getKey();
       if (!packageRoots.containsKey(dir)) {
-        PathFragment pkgDir = longestPathPrefix(dir, packageRoots.keySet());
-        if (pkgDir != null) {
-          entry.getValue().add(packageRoots.get(pkgDir));
+        PackageIdentifier pkgId = longestPathPrefix(dir, packageRoots.keySet());
+        if (pkgId != null) {
+          entry.getValue().add(packageRoots.get(pkgId));
         }
       }
     }
     // Create output dirs for all dirs that have more than one root and need to be split.
-    for (Map.Entry<PathFragment, Set<Path>> entry : dirRootsMap.entrySet()) {
-      PathFragment dir = entry.getKey();
+    for (Map.Entry<PackageIdentifier, Set<Path>> entry : dirRootsMap.entrySet()) {
+      PackageIdentifier dir = entry.getKey();
+      if (!dir.getRepository().isMain()) {
+        FileSystemUtils.createDirectoryAndParents(
+            workspace.getRelative(dir.getRepository().getPathFragment()));
+      }
       if (entry.getValue().size() > 1) {
         if (LOG_FINER) {
-          LOG.finer("mkdir " + workspace.getRelative(dir));
+          LOG.finer("mkdir " + workspace.getRelative(dir.getPathFragment()));
         }
-        FileSystemUtils.createDirectoryAndParents(workspace.getRelative(dir));
+        FileSystemUtils.createDirectoryAndParents(workspace.getRelative(dir.getPathFragment()));
       }
     }
+
     // Make dir links for single rooted dirs.
-    for (Map.Entry<PathFragment, Set<Path>> entry : dirRootsMap.entrySet()) {
-      PathFragment dir = entry.getKey();
+    for (Map.Entry<PackageIdentifier, Set<Path>> entry : dirRootsMap.entrySet()) {
+      PackageIdentifier dir = entry.getKey();
       Set<Path> roots = entry.getValue();
       // Simple case of one root for this dir.
       if (roots.size() == 1) {
-        if (dir.segmentCount() > 1 && dirRootsMap.get(dir.getParentDirectory()).size() == 1) {
+        if (dir.getPackageFragment().segmentCount() > 1
+            && dirRootsMap.get(getParent(dir)).size() == 1) {
           continue;  // skip--an ancestor will link this one in from above
         }
         // This is the top-most dir that can be linked to a single root. Make it so.
         Path root = roots.iterator().next();  // lone root in set
         if (LOG_FINER) {
-          LOG.finer("ln -s " + root.getRelative(dir) + " " + workspace.getRelative(dir));
+          LOG.finer("ln -s " + root.getRelative(dir.getPathFragment()) + " "
+              + workspace.getRelative(dir.getPathFragment()));
         }
-        workspace.getRelative(dir).createSymbolicLink(root.getRelative(dir));
+        workspace.getRelative(dir.getPathFragment())
+            .createSymbolicLink(root.getRelative(dir.getPathFragment()));
       }
     }
     // Make links for dirs within packages, skip parent-only dirs.
-    for (Map.Entry<PathFragment, Set<Path>> entry : dirRootsMap.entrySet()) {
-      PathFragment dir = entry.getKey();
+    for (Map.Entry<PackageIdentifier, Set<Path>> entry : dirRootsMap.entrySet()) {
+      PackageIdentifier dir = entry.getKey();
       if (entry.getValue().size() > 1) {
         // If this dir is at or below a package dir, link in its contents.
-        PathFragment pkgDir = longestPathPrefix(dir, packageRoots.keySet());
-        if (pkgDir != null) {
-          Path root = packageRoots.get(pkgDir);
+        PackageIdentifier pkgId = longestPathPrefix(dir, packageRoots.keySet());
+        if (pkgId != null) {
+          Path root = packageRoots.get(pkgId);
           try {
-            Path absdir = root.getRelative(dir);
+            Path absdir = root.getRelative(dir.getPathFragment());
             if (absdir.isDirectory()) {
               if (LOG_FINER) {
-                LOG.finer("ln -s " + absdir + "/* " + workspace.getRelative(dir) + "/");
+                LOG.finer("ln -s " + absdir + "/* "
+                    + workspace.getRelative(dir.getPathFragment()) + "/");
               }
               for (Path target : absdir.getDirectoryEntries()) {
                 PathFragment p = target.relativeTo(root);
-                if (!dirRootsMap.containsKey(p)) {
+                if (!dirRootsMap.containsKey(createInRepo(pkgId, p))) {
                   //LOG.finest("ln -s " + target + " " + linkRoot.getRelative(p));
                   workspace.getRelative(p).createSymbolicLink(target);
                 }
@@ -178,17 +194,39 @@ class SymlinkForest {
       }
     }
 
-    if (emptyPackagePath != null) {
+    for (Map.Entry<PackageIdentifier, Path> entry : packageRoots.entrySet()) {
+      PackageIdentifier pkgId = entry.getKey();
+      if (!pkgId.getPackageFragment().equals(PathFragment.EMPTY_FRAGMENT)) {
+        continue;
+      }
+      Path execrootDirectory = workspace.getRelative(pkgId.getPathFragment());
+      // If there were no subpackages, this directory might not exist yet.
+      if (!execrootDirectory.exists()) {
+        FileSystemUtils.createDirectoryAndParents(execrootDirectory);
+      }
       // For the top-level directory, generate symlinks to everything in the directory instead of
       // the directory itself.
-      for (Path target : emptyPackagePath.getDirectoryEntries()) {
+      Path sourceDirectory = entry.getValue().getRelative(pkgId.getPathFragment());
+      for (Path target : sourceDirectory.getDirectoryEntries()) {
         String baseName = target.getBaseName();
+        Path execPath = execrootDirectory.getRelative(baseName);
         // Create any links that don't exist yet and don't start with bazel-.
-        if (!baseName.startsWith(productName + "-")
-            && !workspace.getRelative(baseName).exists()) {
-          workspace.getRelative(baseName).createSymbolicLink(target);
+        if (!baseName.startsWith(productName + "-") && !execPath.exists()) {
+          execPath.createSymbolicLink(target);
         }
       }
     }
   }
+
+  private static PackageIdentifier getParent(PackageIdentifier packageIdentifier) {
+    Preconditions.checkArgument(
+        packageIdentifier.getPackageFragment().getParentDirectory() != null);
+    return createInRepo(
+        packageIdentifier, packageIdentifier.getPackageFragment().getParentDirectory());
+  }
+
+  private static PackageIdentifier createInRepo(
+      PackageIdentifier repo, PathFragment packageFragment) {
+    return PackageIdentifier.create(repo.getRepository(), packageFragment);
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
index 74cd6fa8d7..6445f48b32 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
@@ -38,7 +38,6 @@ public final class PackageIdentifier implements Comparable<PackageIdentifier>, S
 
   private static final Interner<PackageIdentifier> INTERNER = Interners.newWeakInterner();
 
-
   public static PackageIdentifier create(String repository, PathFragment pkgName)
       throws LabelSyntaxException {
     return create(RepositoryName.create(repository), pkgName);
@@ -51,11 +50,13 @@ public final class PackageIdentifier implements Comparable<PackageIdentifier>, S
   public static final String DEFAULT_REPOSITORY = "";
   public static final RepositoryName DEFAULT_REPOSITORY_NAME;
   public static final RepositoryName MAIN_REPOSITORY_NAME;
+  public static final PackageIdentifier EMPTY_PACKAGE_ID;
 
   static {
     try {
       DEFAULT_REPOSITORY_NAME = RepositoryName.create(DEFAULT_REPOSITORY);
       MAIN_REPOSITORY_NAME = RepositoryName.create("@");
+      EMPTY_PACKAGE_ID = createInMainRepo(PathFragment.EMPTY_FRAGMENT);
     } catch (LabelSyntaxException e) {
       throw new IllegalStateException(e);
     }