Don't symlink into the execroot if possible in SymlinkAction: instead, symlink directly to the target artifact. Also offer the option to not provide the package roots to create the execroot: we would like to avoid the execroot if possible.

PiperOrigin-RevId: 170515263

3 files changed, 27 insertions, 31 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 07688e6474..61dd9f199c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -35,7 +35,7 @@ import com.google.devtools.build.lib.actions.ActionLookupValue;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ArtifactFactory;
 import com.google.devtools.build.lib.actions.ArtifactOwner;
-import com.google.devtools.build.lib.actions.Root;
+import com.google.devtools.build.lib.actions.PackageRoots;
 import com.google.devtools.build.lib.analysis.DependencyResolver.InconsistentAspectOrderException;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
@@ -50,7 +50,6 @@ import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory;
 import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory.CoverageReportActionsWrapper;
 import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProvider;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
@@ -95,7 +94,6 @@ import com.google.devtools.build.lib.util.OrderedSetMultimap;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.util.RegexFilter;
-import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.WalkableGraph;
@@ -107,7 +105,6 @@ import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsParsingException;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
@@ -348,7 +345,7 @@ public class BuildView {
     private final ImmutableSet<ConfiguredTarget> exclusiveTests;
     @Nullable private final TopLevelArtifactContext topLevelContext;
     private final ImmutableList<AspectValue> aspects;
-    private final ImmutableMap<PackageIdentifier, Path> packageRoots;
+    private final PackageRoots packageRoots;
     private final String workspaceName;
 
     private AnalysisResult(
@@ -362,7 +359,7 @@ public class BuildView {
         Collection<ConfiguredTarget> parallelTests,
         Collection<ConfiguredTarget> exclusiveTests,
         TopLevelArtifactContext topLevelContext,
-        ImmutableMap<PackageIdentifier, Path> packageRoots,
+        PackageRoots packageRoots,
         String workspaceName) {
       this.targetsToBuild = ImmutableSet.copyOf(targetsToBuild);
       this.aspects = ImmutableList.copyOf(aspects);
@@ -385,11 +382,8 @@ public class BuildView {
       return targetsToBuild;
     }
 
-    /**
-     * The map from package names to the package root where each package was found; this is used to
-     * set up the symlink tree.
-     */
-    public ImmutableMap<PackageIdentifier, Path> getPackageRoots() {
+    /** @see PackageRoots */
+    public PackageRoots getPackageRoots() {
       return packageRoots;
     }
 
@@ -1023,19 +1017,8 @@ public class BuildView {
    * paths with unknown roots to artifacts.
    */
   @VisibleForTesting // for BuildViewTestCase
-  public void setArtifactRoots(ImmutableMap<PackageIdentifier, Path> packageRoots) {
-    Map<Path, Root> rootMap = new HashMap<>();
-    Map<PackageIdentifier, Root> realPackageRoots = new HashMap<>();
-    for (Map.Entry<PackageIdentifier, Path> entry : packageRoots.entrySet()) {
-      Root root = rootMap.get(entry.getValue());
-      if (root == null) {
-        root = Root.asSourceRoot(entry.getValue(), entry.getKey().getRepository().isMain());
-        rootMap.put(entry.getValue(), root);
-      }
-      realPackageRoots.put(entry.getKey(), root);
-    }
-    // Source Artifact roots:
-    getArtifactFactory().setPackageRoots(realPackageRoots);
+  public void setArtifactRoots(PackageRoots packageRoots) {
+    getArtifactFactory().setPackageRoots(packageRoots.getPackageRootLookup());
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/ExecutableSymlinkAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/ExecutableSymlinkAction.java
index 4070e5a73a..4cdf2f1e12 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/ExecutableSymlinkAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/ExecutableSymlinkAction.java
@@ -35,7 +35,7 @@ public final class ExecutableSymlinkAction extends SymlinkAction {
   @Override
   public void execute(ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException {
-    Path inputPath = actionExecutionContext.getExecRoot().getRelative(getInputPath());
+    Path inputPath = getPrimaryInput().getPath();
     try {
       // Validate that input path is a file with the executable bit is set.
       if (!inputPath.isFile()) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java
index b79f152fb7..df3b56f829 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java
@@ -26,6 +26,7 @@ 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.io.IOException;
+import javax.annotation.Nullable;
 
 /**
  * Action to create a symbolic link.
@@ -34,7 +35,9 @@ public class SymlinkAction extends AbstractAction {
 
   private static final String GUID = "349675b5-437c-4da8-891a-7fb98fba6ab5";
 
-  private final PathFragment inputPath;
+  /** Null when {@link #getPrimaryInput} is the target of the symlink. */
+  @Nullable private final PathFragment inputPath;
+
   private final Artifact output;
   private final String progressMessage;
 
@@ -50,7 +53,10 @@ public class SymlinkAction extends AbstractAction {
       String progressMessage) {
     // These actions typically have only one input and one output, which
     // become the sole and primary in their respective lists.
-    this(owner, input.getExecPath(), input, output, progressMessage);
+    super(owner, ImmutableList.of(input), ImmutableList.of(output));
+    this.inputPath = null;
+    this.output = Preconditions.checkNotNull(output);
+    this.progressMessage = progressMessage;
   }
 
   /**
@@ -93,7 +99,7 @@ public class SymlinkAction extends AbstractAction {
   }
 
   public PathFragment getInputPath() {
-    return inputPath;
+    return inputPath == null ? getPrimaryInput().getExecPath() : inputPath;
   }
 
   public Path getOutputPath() {
@@ -103,9 +109,14 @@ public class SymlinkAction extends AbstractAction {
   @Override
   public void execute(ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException {
+    Path srcPath;
+    if (inputPath == null) {
+      srcPath = getPrimaryInput().getPath();
+    } else {
+      srcPath = actionExecutionContext.getExecRoot().getRelative(inputPath);
+    }
     try {
-      getOutputPath()
-          .createSymbolicLink(actionExecutionContext.getExecRoot().getRelative(inputPath));
+      getOutputPath().createSymbolicLink(srcPath);
     } catch (IOException e) {
       throw new ActionExecutionException("failed to create symbolic link '"
           + Iterables.getOnlyElement(getOutputs()).prettyPrint()
@@ -120,7 +131,9 @@ public class SymlinkAction extends AbstractAction {
     f.addString(GUID);
     // We don't normally need to add inputs to the key. In this case, however, the inputPath can be
     // different from the actual input artifact.
-    f.addPath(inputPath);
+    if (inputPath != null) {
+      f.addPath(inputPath);
+    }
     return f.hexDigestAndReset();
   }