Make the execution root match the runfiles tree structure for external repositories

One interesting side effect of how this is implemented is that for external
repositories, bin/ and genfiles/ are combined.  External repo output is under
bazel-out/local-fastbuild/repo_name for each repo.

Fixes #1262.

RELNOTES[INC]: Previously, an external repository would be symlinked into the
execution root at execroot/local_repo/external/remote_repo. This changes it to
be at execroot/remote_repo. This may break genrules/Skylark actions that
hardcode execution root paths. If this causes breakages for you, ensure that
genrules are using $(location :target) to access files and Skylark rules are
using http://bazel.io/docs/skylark/lib/File.html's path, dirname, etc.
functions.

--
MOS_MIGRATED_REVID=125095799

22 files changed, 353 insertions, 225 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
index b3b86823da..4c4a5e1546 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
@@ -46,12 +46,13 @@ import javax.annotation.Nullable;
 /**
  * An Artifact represents a file used by the build system, whether it's a source
  * file or a derived (output) file. Not all Artifacts have a corresponding
- * FileTarget object in the <code>build.packages</code> API: for example,
+ * FileTarget object in the <code>build.lib.packages</code> API: for example,
  * low-level intermediaries internal to a given rule, such as a Java class files
  * or C++ object files. However all FileTargets have a corresponding Artifact.
  *
- * <p>In any given call to Builder#buildArtifacts(), no two Artifacts in the
- * action graph may refer to the same path.
+ * <p>In any given call to
+ * {@link com.google.devtools.build.lib.skyframe.SkyframeExecutor#buildArtifacts}, no two Artifacts
+ * in the action graph may refer to the same path.
  *
  * <p>Artifacts generally fall into two classifications, source and derived, but
  * there exist a few other cases that are fuzzy and difficult to classify. The
@@ -154,6 +155,64 @@ public class Artifact
   private final PathFragment rootRelativePath;
   private final ArtifactOwner owner;
 
+  private static boolean pathEndsWith(PathFragment path, PathFragment suffix) {
+    if (suffix.isNormalized()) {
+      return path.endsWith(suffix);
+    }
+
+    for (int suffixIndex = suffix.segmentCount() - 1, pathIndex = path.segmentCount() - 1;
+         suffixIndex >= 0; suffixIndex--) {
+      if (suffix.getSegment(suffixIndex).equals("..")) {
+        if (suffixIndex > 0) {
+          // The path foo/bar/../baz matches the suffix foo/baz.
+          suffixIndex--;
+          continue;
+        } else {
+          // The path repo/foo matches the suffix ../repo/foo.
+          return true;
+        }
+      }
+      if (pathIndex < 0 || !path.getSegment(pathIndex).equals(suffix.getSegment(suffixIndex))) {
+        return false;
+      }
+      pathIndex--;
+    }
+
+    return true;
+  }
+
+  // Like startsWith, but allows prefix to match the path.getParentDirectory() instead of just
+  // path (so that bazel-out/config/repo/foo "starts with" bazel-out/config/bin).
+  private static boolean pathMostlyStartsWith(Path path, Path prefix) {
+    return path.startsWith(prefix) || path.startsWith(prefix.getParentDirectory());
+  }
+
+  private static PathFragment getRelativePath(PathFragment path, PathFragment ancestorDirectory) {
+    if (path.startsWith(ancestorDirectory)) {
+      // Local path.
+      return path.relativeTo(ancestorDirectory);
+    }
+
+    // External repository.
+    int ancestorLength = ancestorDirectory.segmentCount();
+
+    PathFragment builder = PathFragment.EMPTY_FRAGMENT;
+    int diffIndex = -1;
+    for (int i = 0; i < ancestorLength; i++) {
+      if (diffIndex == -1 && i < path.segmentCount()
+          && ancestorDirectory.getSegment(i).equals(path.getSegment(i))) {
+        continue;
+      }
+      diffIndex = i;
+      builder = builder.getRelative("..");
+    }
+    int tailIndex = diffIndex == -1 ? ancestorLength : diffIndex;
+    for (int i = tailIndex; i < path.segmentCount(); i++) {
+      builder = builder.getRelative(path.getSegment(i));
+    }
+    return builder;
+  }
+
   /**
    * Constructs an artifact for the specified path, root and execPath. The root must be an ancestor
    * of path, and execPath must be a non-absolute tail of path. Outside of testing, this method
@@ -166,18 +225,18 @@ public class Artifact
    * </pre>
    *
    * <p>In a derived Artifact, the execPath will overlap with part of the root, which in turn will
-   * be below of the execRoot.
+   * be below the execRoot.
    * <pre>
    *  [path] == [/root][pathTail] == [/execRoot][execPath] == [/execRoot][rootPrefix][pathTail]
    * <pre>
    */
   @VisibleForTesting
   public Artifact(Path path, Root root, PathFragment execPath, ArtifactOwner owner) {
-    if (root == null || !path.startsWith(root.getPath())) {
+    if (root == null || !pathMostlyStartsWith(path, root.getPath())) {
       throw new IllegalArgumentException(root + ": illegal root for " + path
           + " (execPath: " + execPath + ")");
     }
-    if (execPath == null || execPath.isAbsolute() || !path.asFragment().endsWith(execPath)) {
+    if (execPath == null || execPath.isAbsolute() || !pathEndsWith(path.asFragment(), execPath)) {
       throw new IllegalArgumentException(execPath + ": illegal execPath for " + path
           + " (root: " + root + ")");
     }
@@ -188,12 +247,14 @@ public class Artifact
     // These two lines establish the invariant that
     // execPath == rootRelativePath <=> execPath.equals(rootRelativePath)
     // This is important for isSourceArtifact.
-    PathFragment rootRel = path.relativeTo(root.getPath());
-    if (!execPath.endsWith(rootRel)) {
+    PathFragment rootRel = getRelativePath(path.asFragment(), root.getPath().asFragment());
+    if (!pathEndsWith(execPath, rootRel)) {
       throw new IllegalArgumentException(execPath + ": illegal execPath doesn't end with "
           + rootRel + " at " + path + " with root " + root);
     }
-    this.rootRelativePath = rootRel.equals(execPath) ? execPath : rootRel;
+
+    this.rootRelativePath = root.getPath().getRelative(rootRel).asFragment().normalize().equals(
+        root.getPath().getRelative(execPath).asFragment().normalize()) ? execPath : rootRel;
     this.owner = Preconditions.checkNotNull(owner, path);
   }
 
@@ -498,16 +559,10 @@ public class Artifact
   /**
    * For targets in external repositories, this returns the path the artifact live at in the
    * runfiles tree. For local targets, it returns the rootRelativePath.
+   * TODO(kchodorow): remove.
    */
   public final PathFragment getRunfilesPath() {
-    PathFragment relativePath = rootRelativePath;
-    if (relativePath.segmentCount() > 1
-        && relativePath.getSegment(0).equals(Label.EXTERNAL_PATH_PREFIX)) {
-      // Turn external/repo/foo into ../repo/foo.
-      relativePath = relativePath.relativeTo(Label.EXTERNAL_PATH_PREFIX);
-      relativePath = new PathFragment("..").getRelative(relativePath);
-    }
-    return relativePath;
+    return getRootRelativePath();
   }
 
   @SkylarkCallable(
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
index 2642687713..48960c5cf6 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
@@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSortedSet;
 import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
@@ -200,7 +201,8 @@ public class ArtifactFactory implements ArtifactResolver, ArtifactSerializer, Ar
 
   private void validatePath(PathFragment rootRelativePath, Root root) {
     Preconditions.checkArgument(!rootRelativePath.isAbsolute(), rootRelativePath);
-    Preconditions.checkArgument(rootRelativePath.isNormalized(), rootRelativePath);
+    Preconditions.checkArgument(
+        root.getWorkspaceDirectory().getRelative(rootRelativePath).normalize().isNormalized());
     Preconditions.checkArgument(root.getPath().startsWith(execRoot), "%s %s", root, execRoot);
     Preconditions.checkArgument(!root.getPath().equals(execRoot), "%s %s", root, execRoot);
     // TODO(bazel-team): this should only accept roots from derivedRoots.
@@ -314,7 +316,9 @@ public class ArtifactFactory implements ArtifactResolver, ArtifactSerializer, Ar
     PathFragment execPath =
         baseExecPath == null ? relativePath : baseExecPath.getRelative(relativePath);
     execPath = execPath.normalize();
-    if (execPath.containsUplevelReferences()) {
+    if (execPath.containsUplevelReferences()
+        && (execPath.segmentCount() >= 2
+        && !execPath.startsWith(new PathFragment(Label.EXTERNAL_PATH_PREFIX)))) {
       // Source exec paths cannot escape the source root.
       return null;
     }
@@ -375,8 +379,9 @@ public class ArtifactFactory implements ArtifactResolver, ArtifactSerializer, Ar
 
     for (PathFragment execPath : execPaths) {
       PathFragment execPathNormalized = execPath.normalize();
-      if (execPathNormalized.containsUplevelReferences()) {
-        // Source exec paths cannot escape the source root.
+      if (execPathNormalized.subFragment(1, execPathNormalized.segmentCount())
+          .containsUplevelReferences()) {
+        // Source exec paths cannot escape the execution root.
         result.put(execPath, null);
         continue;
       }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Root.java b/src/main/java/com/google/devtools/build/lib/actions/Root.java
index b09dfa35c8..f92e4bda3c 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Root.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Root.java
@@ -122,6 +122,10 @@ public final class Root implements Comparable<Root>, Serializable {
     return execPath;
   }
 
+  public PathFragment getWorkspaceDirectory() {
+    return new PathFragment(isSourceRoot() ? path.getBaseName() : execRoot.getBaseName());
+  }
+
   @SkylarkCallable(name = "path", structField = true,
       doc = "Returns the relative path from the exec root to the actual root.")
   public String getExecPathString() {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
index d1f1434a39..273506f284 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
@@ -428,7 +428,7 @@ public final class Runfiles {
     Map<PathFragment, Artifact> manifest = getSymlinksAsMap(checker);
     // Add unconditional artifacts (committed to inclusion on construction of runfiles).
     for (Artifact artifact : getUnconditionalArtifactsWithoutMiddlemen()) {
-      checker.put(manifest, artifact.getRootRelativePath(), artifact);
+      checker.put(manifest, artifact.getRootRelativePath().normalize(), artifact);
     }
 
     // Add conditional artifacts (only included if they appear in a pruning manifest).
@@ -485,7 +485,7 @@ public final class Runfiles {
     // workspace.
     private boolean sawWorkspaceName;
 
-    public ManifestBuilder(
+    ManifestBuilder(
         PathFragment workspaceName, boolean legacyExternalRunfiles) {
       this.manifest = new HashMap<>();
       this.workspaceName = workspaceName;
@@ -496,20 +496,18 @@ public final class Runfiles {
     /**
      * Adds a map under the workspaceName.
      */
-    public void addUnderWorkspace(
+    void addUnderWorkspace(
         Map<PathFragment, Artifact> inputManifest, ConflictChecker checker) {
       for (Map.Entry<PathFragment, Artifact> entry : inputManifest.entrySet()) {
         PathFragment path = entry.getKey();
         if (isUnderWorkspace(path)) {
           sawWorkspaceName = true;
-          checker.put(manifest, workspaceName.getRelative(path), entry.getValue());
-        } else {
-          if (legacyExternalRunfiles) {
-            checker.put(manifest, workspaceName.getRelative(path), entry.getValue());
-          }
-          // Always add the non-legacy .runfiles/repo/whatever path.
-          checker.put(manifest, getExternalPath(path), entry.getValue());
+        } else if (legacyExternalRunfiles) {
+          // Turn ../repo/foo info wsname/external/repo/foo.
+          checker.put(manifest, workspaceName.getRelative(Label.EXTERNAL_PACKAGE_NAME)
+              .getRelative(workspaceName).getRelative(path).normalize(), entry.getValue());
         }
+        checker.put(manifest, workspaceName.getRelative(path).normalize(), entry.getValue());
       }
     }
 
@@ -536,17 +534,14 @@ public final class Runfiles {
       return manifest;
     }
 
-    private PathFragment getExternalPath(PathFragment path) {
-      return checkForWorkspace(path.relativeTo(Label.EXTERNAL_PACKAGE_NAME));
-    }
-
     private PathFragment checkForWorkspace(PathFragment path) {
-      sawWorkspaceName = sawWorkspaceName || path.getSegment(0).equals(workspaceName);
+      sawWorkspaceName = sawWorkspaceName
+          || path.getSegment(0).equals(workspaceName.getPathString());
       return path;
     }
 
     private static boolean isUnderWorkspace(PathFragment path) {
-      return !path.startsWith(Label.EXTERNAL_PACKAGE_NAME);
+      return !path.startsWith(new PathFragment(Label.EXTERNAL_PATH_PREFIX));
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java
index e906c11e17..816865149b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java
@@ -205,7 +205,7 @@ public class GenRule implements RuleConfiguredTargetFactory {
                 }
                 PathFragment relPath =
                     ruleContext.getRule().getLabel().getPackageIdentifier().getPathFragment();
-                return dir.getRelative(relPath).getPathString();
+                return dir.getRelative(relPath).normalize().getPathString();
               }
             } else {
               return super.lookupMakeVariable(name);
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 a07fb9de73..c809e64436 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;
@@ -121,6 +122,7 @@ import java.util.logging.Logger;
  * @see BuildView
  */
 public class ExecutionTool {
+
   private static class StrategyConverter {
     private Table<Class<? extends ActionContext>, String, ActionContext> classMap =
         HashBasedTable.create();
@@ -333,9 +335,9 @@ public class ExecutionTool {
    * creation
    */
   void executeBuild(UUID buildId, AnalysisResult analysisResult,
-      BuildResult buildResult,
-      BuildConfigurationCollection configurations,
-      ImmutableMap<PathFragment, Path> packageRoots)
+                    BuildResult buildResult,
+                    BuildConfigurationCollection configurations,
+                    Map<PackageIdentifier, Path> packageRoots)
       throws BuildFailedException, InterruptedException, TestExecException, AbruptExitException {
     Stopwatch timer = Stopwatch.createStarted();
     prepare(packageRoots);
@@ -495,8 +497,7 @@ public class ExecutionTool {
     }
   }
 
-  private void prepare(ImmutableMap<PathFragment, Path> packageRoots)
-      throws ExecutorInitException {
+  private void prepare(Map<PackageIdentifier, Path> packageRoots) throws ExecutorInitException {
     // Prepare for build.
     Profiler.instance().markPhase(ProfilePhase.PREPARE);
 
@@ -515,12 +516,12 @@ public class ExecutionTool {
     }
   }
 
-  private void plantSymlinkForest(ImmutableMap<PathFragment, Path> packageRoots)
+  private void plantSymlinkForest(Map<PackageIdentifier, Path> packageRoots)
       throws ExecutorInitException {
     try {
-      FileSystemUtils.deleteTreesBelowNotPrefixed(getExecRoot(),
-          new String[] { ".", "_", runtime.getProductName() + "-"});
-      FileSystemUtils.plantLinkForest(packageRoots, getExecRoot(), runtime.getProductName());
+      SymlinkForest forest = new SymlinkForest(
+          packageRoots, getExecRoot(), runtime.getProductName());
+      forest.plantLinkForest();
     } catch (IOException e) {
       throw new ExecutorInitException("Source forest creation failed", e);
     }
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
new file mode 100644
index 0000000000..4cf27ebf38
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java
@@ -0,0 +1,206 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.buildtool;
+
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
+
+import java.io.IOException;
+import java.util.Map;
+import java.util.Set;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+/**
+ * Creates a symlink forest based on a package path map.
+ */
+class SymlinkForest {
+
+  private static final Logger LOG = Logger.getLogger(SymlinkForest.class.getName());
+  private static final boolean LOG_FINER = LOG.isLoggable(Level.FINER);
+
+  private final Map<PackageIdentifier, Path> packageRoots;
+  private final Path workspace;
+  private final String productName;
+
+  SymlinkForest(
+      Map<PackageIdentifier, Path> packageRoots, Path workspace, String productName) {
+    this.packageRoots = packageRoots;
+    this.workspace = workspace;
+    this.productName = productName;
+  }
+
+  /**
+   * Takes a map of directory fragments to root paths, and creates a symlink
+   * forest under an existing linkRoot to the corresponding source dirs or
+   * files. Symlink are made at the highest dir possible, linking files directly
+   * only when needed with nested packages.
+   */
+  void plantLinkForest() throws IOException {
+    deleteExisting();
+
+    // 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<PackageIdentifier, Set<Path>> dirRootsMap = Maps.newTreeMap();
+    for (Map.Entry<PackageIdentifier, Path> entry : packageRoots.entrySet()) {
+      PackageIdentifier packageIdentifier = entry.getKey();
+      PathFragment pkgDir = packageIdentifier.getPackageFragment();
+      Path pkgRoot = entry.getValue();
+      for (int i = 0; i <= pkgDir.segmentCount(); i++) {
+        PackageIdentifier dir = PackageIdentifier.create(
+            packageIdentifier.getRepository(), pkgDir.subFragment(0, i));
+        Set<Path> roots = dirRootsMap.get(dir);
+        if (roots == null) {
+          roots = Sets.newHashSet();
+          dirRootsMap.put(dir, roots);
+        }
+        roots.add(pkgRoot);
+      }
+    }
+    // Now add in roots for all non-pkg dirs that are in between two packages, and missed above.
+    for (Map.Entry<PackageIdentifier, Set<Path>> entry : dirRootsMap.entrySet()) {
+      PackageIdentifier packageIdentifier = entry.getKey();
+      if (!packageRoots.containsKey(packageIdentifier)) {
+        PackageIdentifier pkgDir = longestPathPrefix(packageIdentifier, packageRoots.keySet());
+        if (pkgDir != null) {
+          entry.getValue().add(packageRoots.get(pkgDir));
+        }
+      }
+    }
+    // Create output dirs for all dirs that have more than one root and need to be split.
+    for (Map.Entry<PackageIdentifier, Set<Path>> entry : dirRootsMap.entrySet()) {
+      PathFragment dir = entry.getKey().getPathFragment();
+      if (entry.getValue().size() > 1) {
+        if (LOG_FINER) {
+          LOG.finer("mkdir " + workspace.getRelative(dir));
+        }
+        FileSystemUtils.createDirectoryAndParents(workspace.getRelative(dir));
+      }
+    }
+    // Make dir links for single rooted dirs.
+    for (Map.Entry<PackageIdentifier, Set<Path>> entry : dirRootsMap.entrySet()) {
+      PackageIdentifier pkgId = entry.getKey();
+      Path linkRoot = workspace.getRelative(pkgId.getRepository().getPathFragment());
+      PathFragment dir = entry.getKey().getPackageFragment();
+      Set<Path> roots = entry.getValue();
+      // Simple case of one root for this dir.
+      if (roots.size() == 1) {
+        // Special case: the main repository is not deleted (because it contains symlinks to
+        // bazel-out et al) so don't attempt to symlink it in.
+        if (pkgId.equals(PackageIdentifier.EMPTY_PACKAGE_IDENTIFIER)) {
+          symlinkEmptyPackage(roots.iterator().next());
+          continue;
+        }
+        if (dir.segmentCount() > 0) {
+          PackageIdentifier parent = PackageIdentifier.create(
+              pkgId.getRepository(), dir.getParentDirectory());
+          if (dir.segmentCount() > 0 && dirRootsMap.get(parent).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) + " " + linkRoot.getRelative(dir));
+        }
+        linkRoot.getRelative(dir).createSymbolicLink(root.getRelative(dir));
+      }
+    }
+    // Make links for dirs within packages, skip parent-only dirs.
+    for (Map.Entry<PackageIdentifier, Set<Path>> entry : dirRootsMap.entrySet()) {
+      Path linkRoot = workspace.getRelative(entry.getKey().getRepository().getPathFragment());
+      PathFragment dir = entry.getKey().getPackageFragment();
+      if (entry.getValue().size() > 1) {
+        // If this dir is at or below a package dir, link in its contents.
+        PackageIdentifier pkgDir = longestPathPrefix(entry.getKey(), packageRoots.keySet());
+        if (pkgDir != null) {
+          Path root = packageRoots.get(pkgDir);
+          try {
+            Path absdir = root.getRelative(dir);
+            if (absdir.isDirectory()) {
+              if (LOG_FINER) {
+                LOG.finer("ln -s " + absdir + "/* " + linkRoot.getRelative(dir) + "/");
+              }
+              for (Path target : absdir.getDirectoryEntries()) {
+                PackageIdentifier dirent = PackageIdentifier.create(
+                    pkgDir.getRepository(), target.relativeTo(root));
+                if (!dirRootsMap.containsKey(dirent)) {
+                  linkRoot.getRelative(dirent.getPackageFragment()).createSymbolicLink(target);
+                }
+              }
+            } else {
+              LOG.fine("Symlink planting skipping dir '" + absdir + "'");
+            }
+          } catch (IOException e) {
+            e.printStackTrace();
+          }
+          // Otherwise its just an otherwise empty common parent dir.
+        }
+      }
+    }
+  }
+
+  private void deleteExisting() throws IOException {
+    FileSystemUtils.createDirectoryAndParents(workspace);
+    FileSystemUtils.deleteTreesBelowNotPrefixed(workspace,
+        new String[] { ".", "_", productName + "-"});
+    for (Map.Entry<PackageIdentifier, Path> entry : packageRoots.entrySet()) {
+      RepositoryName repo = entry.getKey().getRepository();
+      Path repoPath = workspace.getRelative(repo.getPathFragment());
+      if (!repo.isMain() && repoPath.exists()) {
+        FileSystemUtils.deleteTree(repoPath);
+      }
+    }
+  }
+
+  /**
+   * For the top-level directory, generate symlinks to everything in the directory instead of the
+   * directory itself.
+   */
+  private void symlinkEmptyPackage(Path emptyPackagePath) throws IOException {
+    for (Path target : emptyPackagePath.getDirectoryEntries()) {
+      String baseName = target.getBaseName();
+      // 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);
+      }
+    }
+  }
+
+  /**
+   * 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.
+   */
+  static PackageIdentifier longestPathPrefix(
+      PackageIdentifier packageIdentifier, Set<PackageIdentifier> prefixes) {
+    PathFragment pkg = packageIdentifier.getPackageFragment();
+    for (int i = pkg.segmentCount(); i >= 0; i--) {
+      PackageIdentifier prefix = PackageIdentifier.create(
+          packageIdentifier.getRepository(), pkg.subFragment(0, i));
+      if (prefixes.contains(prefix)) {
+        return prefix;
+      }
+    }
+    return null;
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
index 73c7e5ecdb..75b7b2955f 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
@@ -65,7 +65,7 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkPrin
   public static final PackageIdentifier EXTERNAL_PACKAGE_IDENTIFIER =
       PackageIdentifier.createInMainRepo(EXTERNAL_PACKAGE_NAME);
 
-  public static final String EXTERNAL_PATH_PREFIX = "external";
+  public static final String EXTERNAL_PATH_PREFIX = "..";
 
   private static final Interner<Label> LABEL_INTERNER = Interners.newWeakInterner();
 
@@ -310,7 +310,7 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkPrin
       doc = "Returns the execution root for the workspace of this label, relative to the execroot. "
       + "For instance:<br>"
       + "<pre class=language-python>Label(\"@repo//pkg/foo:abc\").workspace_root =="
-      + " \"external/repo\"</pre>")
+      + " \"../repo\"</pre>")
   public String getWorkspaceRoot() {
     return packageIdentifier.getRepository().getPathFragment().toString();
   }
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..4f0652a981 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
@@ -48,14 +48,17 @@ public final class PackageIdentifier implements Comparable<PackageIdentifier>, S
     return INTERNER.intern(new PackageIdentifier(repository, pkgName));
   }
 
-  public static final String DEFAULT_REPOSITORY = "";
-  public static final RepositoryName DEFAULT_REPOSITORY_NAME;
+  static final String DEFAULT_REPOSITORY = "";
   public static final RepositoryName MAIN_REPOSITORY_NAME;
+  public static final RepositoryName DEFAULT_REPOSITORY_NAME;
+  public static final PackageIdentifier EMPTY_PACKAGE_IDENTIFIER;
 
   static {
     try {
       DEFAULT_REPOSITORY_NAME = RepositoryName.create(DEFAULT_REPOSITORY);
       MAIN_REPOSITORY_NAME = RepositoryName.create("@");
+      EMPTY_PACKAGE_IDENTIFIER =
+          PackageIdentifier.create(MAIN_REPOSITORY_NAME, PathFragment.EMPTY_FRAGMENT);
     } catch (LabelSyntaxException e) {
       throw new IllegalStateException(e);
     }
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java
index 6b89a6ee7a..eab5320371 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java
@@ -205,10 +205,10 @@ public final class RepositoryName implements Serializable {
    * Returns the runfiles path for this repository (relative to the x.runfiles/main-repo/
    * directory). If we don't know the name of this repo (i.e., it is in the main repository),
    * return an empty path fragment.
+   * TODO(kchodorow): remove this.
    */
   public PathFragment getRunfilesPath() {
-    return isDefault() || isMain()
-        ? PathFragment.EMPTY_FRAGMENT : new PathFragment("..").getRelative(strippedName());
+    return getPathFragment();
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index 44d9e308a3..ae0915e3cd 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -294,7 +294,7 @@ public class Package {
     this.filename = builder.getFilename();
     this.packageDirectory = filename.getParentDirectory();
 
-    this.sourceRoot = getSourceRoot(filename, packageIdentifier.getPathFragment());
+    this.sourceRoot = getSourceRoot(filename, packageIdentifier.getPackageFragment());
     if ((sourceRoot == null
         || !sourceRoot.getRelative(packageIdentifier.getPathFragment()).equals(packageDirectory))
         && !filename.getBaseName().equals("WORKSPACE")) {
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java
index f00606daf4..687ed98fc0 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java
@@ -18,6 +18,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
@@ -110,8 +111,11 @@ public class PathPackageLocator implements Serializable {
       // $OUTPUT_BASE/external, which is created by the appropriate RepositoryDirectoryValue. This
       // is true for the invocation in GlobCache, but not for the locator.getBuildFileForPackage()
       // invocation in Parser#include().
-      Path buildFile = outputBase.getRelative(
-          packageIdentifier.getPathFragment()).getRelative("BUILD");
+      Path buildFile = outputBase
+          .getRelative(Label.EXTERNAL_PACKAGE_NAME)
+          .getRelative(packageIdentifier.getRepository().strippedName())
+          .getRelative(packageIdentifier.getPackageFragment())
+          .getRelative("BUILD");
       FileStatus stat = cache.get().statNullable(buildFile, Symlinks.FOLLOW);
       if (stat != null && stat.isFile()) {
         return buildFile;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java
index c067d366aa..130f7a87ba 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java
@@ -432,11 +432,11 @@ public final class JackCompilationHelper {
    */
   private Artifact postprocessPartialJackAndAddResources(
       Artifact partialJackLibrary, Artifact resources) {
-    Artifact result = ruleContext.getUniqueDirectoryArtifact(
-        JACK_DIRECTORY,
-        partialJackLibrary.getRootRelativePath().relativeTo(
-            ruleContext.getUniqueDirectory(PARTIAL_JACK_DIRECTORY)),
-        ruleContext.getBinOrGenfilesDirectory());
+    PathFragment partialPath = new PathFragment(
+        partialJackLibrary.getRootRelativePath().getPathString()
+            .replace(PARTIAL_JACK_DIRECTORY, JACK_DIRECTORY));
+    Artifact result = ruleContext.getDerivedArtifact(
+        partialPath, ruleContext.getBinOrGenfilesDirectory());
     CustomCommandLine.Builder builder =
         CustomCommandLine.builder()
             // Have jack double-check its behavior and crash rather than producing invalid output
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index 8ccd6e1dea..fb1ca8c96d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -397,9 +397,12 @@ public final class CcCommon {
         continue;
       }
       PathFragment includesPath = packageFragment.getRelative(includesAttr).normalize();
-      if (!includesPath.isNormalized()) {
+      // It's okay for the includes path to start with ../workspace-name for external repos.
+      if ((packageIdentifier.getRepository().isMain() && !includesPath.isNormalized())
+          || (!packageIdentifier.getRepository().isMain()
+              && !includesPath.startsWith(packageIdentifier.getRepository().getPathFragment()))) {
         ruleContext.attributeError("includes",
-            "Path references a path above the execution root.");
+            includesAttr + " references a path above the execution root (" + includesPath + ").");
       }
       if (includesPath.segmentCount() == 0) {
         ruleContext.attributeError(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 5d273fe816..bb2149e004 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -947,7 +947,12 @@ public class CppCompileAction extends AbstractAction
           // are, it's probably due to a non-hermetic #include, & we should stop
           // the build with an error.
           if (execPath.startsWith(execRoot)) {
-            execPathFragment = execPath.relativeTo(execRoot); // funky but tolerable path
+            // funky but tolerable path
+            execPathFragment = execPath.relativeTo(execRoot);
+          } else if (execPath.startsWith(execRoot.getParentDirectory())) {
+            // External repository.
+            execPathFragment = new PathFragment("..")
+                .getRelative(execPath.relativeTo(execRoot.getParentDirectory()));
           } else {
             problems.add(execPathFragment.getPathString());
             continue;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
index d0da0c420a..9cf27c994d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
@@ -468,7 +468,8 @@ public final class JavaCompilationHelper extends BaseJavaCompilationHelper {
     String basename = FileSystemUtils.removeExtension(outputJar.getExecPath().getBaseName());
     return getConfiguration().getBinDirectory().getExecPath()
         .getRelative(ruleContext.getUniqueDirectory("_javac"))
-        .getRelative(basename + suffix);
+        .getRelative(basename + suffix)
+        .normalize();
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java
index ae43116519..05e479d980 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java
@@ -437,7 +437,7 @@ public final class PyCommon {
     }
 
     PathFragment workspaceName = new PathFragment(ruleContext.getRule().getWorkspaceName());
-    return workspaceName.getRelative(mainArtifact.getRunfilesPath()).getPathString();
+    return workspaceName.getRelative(mainArtifact.getRunfilesPath()).normalize().getPathString();
   }
 
   public Artifact getExecutable() {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
index 043bd0f0df..7f28eb52f2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
@@ -262,7 +262,7 @@ public abstract class RepositoryFunction {
 
   /**
    * Uses a remote repository name to fetch the corresponding Rule describing how to get it.
-   * 
+   *
    * This should be the unique entry point for resolving a remote repository function.
    */
   @Nullable
@@ -352,9 +352,7 @@ public abstract class RepositoryFunction {
   }
 
   public static Path getExternalRepositoryDirectory(BlazeDirectories directories) {
-    return directories
-        .getOutputBase()
-        .getRelative(Label.EXTERNAL_PATH_PREFIX);
+    return directories.getOutputBase().getRelative(Label.EXTERNAL_PACKAGE_NAME);
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
index 792cc5fa13..9a35fba289 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
@@ -138,7 +138,7 @@ public class ExternalFilesHelper {
       return FileType.EXTERNAL_MUTABLE;
     }
     if (rootedPath.asPath().startsWith(outputBase)) {
-      Path externalRepoDir = outputBase.getRelative(Label.EXTERNAL_PATH_PREFIX);
+      Path externalRepoDir = outputBase.getRelative(Label.EXTERNAL_PACKAGE_NAME);
       if (rootedPath.asPath().startsWith(externalRepoDir)) {
         anyNonOutputExternalFilesSeen = true;
         return FileType.EXTERNAL_REPO;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java
index 604ef07b0c..f8aea5a1b5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java
@@ -74,7 +74,7 @@ class SkyframePackageLoaderWithValueEnvironment implements PackageProviderForCon
   @Override
   public void addDependency(Package pkg, String fileName) throws LabelSyntaxException, IOException {
     RootedPath fileRootedPath = RootedPath.toRootedPath(pkg.getSourceRoot(),
-        pkg.getPackageIdentifier().getPathFragment().getRelative(fileName));
+        pkg.getPackageIdentifier().getPackageFragment().getRelative(fileName));
     FileValue result = (FileValue) env.getValue(FileValue.key(fileRootedPath));
     if (result != null && !result.exists()) {
       throw new IOException();
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java
index 0facfefde1..aaac85007c 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java
@@ -16,9 +16,6 @@ package com.google.devtools.build.lib.vfs;
 import static java.nio.charset.StandardCharsets.ISO_8859_1;
 
 import com.google.common.base.Predicate;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
 import com.google.common.io.ByteSink;
 import com.google.common.io.ByteSource;
 import com.google.common.io.ByteStreams;
@@ -35,10 +32,6 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 /**
  * Helper functions that implement often-used complex operations on file
@@ -47,9 +40,6 @@ import java.util.logging.Logger;
 @ConditionallyThreadSafe // ThreadSafe except for deleteTree.
 public class FileSystemUtils {
 
-  static final Logger LOG = Logger.getLogger(FileSystemUtils.class.getName());
-  static final boolean LOG_FINER = LOG.isLoggable(Level.FINER);
-
   private FileSystemUtils() {}
 
   /****************************************************************************
@@ -137,21 +127,6 @@ public class FileSystemUtils {
   }
 
   /**
-   * 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.
-   */
-  static PathFragment longestPathPrefix(PathFragment path, Set<PathFragment> prefixes) {
-    for (int i = path.segmentCount(); i >= 0; i--) {
-      PathFragment prefix = path.subFragment(0, i);
-      if (prefixes.contains(prefix)) {
-        return prefix;
-      }
-    }
-    return null;
-  }
-
-  /**
    * Removes the shortest suffix beginning with '.' from the basename of the
    * filename string. If the basename contains no '.', the filename is returned
    * unchanged.
@@ -676,120 +651,6 @@ public class FileSystemUtils {
     return true;
   }
 
-  /**
-   * Takes a map of directory fragments to root paths, and creates a symlink
-   * forest under an existing linkRoot to the corresponding source dirs or
-   * files. Symlink are made at the highest dir possible, linking files directly
-   * only when needed with nested packages.
-   */
-  public static void plantLinkForest(ImmutableMap<PathFragment, Path> packageRootMap,
-      Path linkRoot, String productName)
-      throws IOException {
-    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 : packageRootMap.entrySet()) {
-      PathFragment pkgDir = 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);
-        Set<Path> roots = dirRootsMap.get(dir);
-        if (roots == null) {
-          roots = Sets.newHashSet();
-          dirRootsMap.put(dir, roots);
-        }
-        roots.add(pkgRoot);
-      }
-    }
-    // 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();
-      if (!packageRootMap.containsKey(dir)) {
-        PathFragment pkgDir = longestPathPrefix(dir, packageRootMap.keySet());
-        if (pkgDir != null) {
-          entry.getValue().add(packageRootMap.get(pkgDir));
-        }
-      }
-    }
-    // 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();
-      if (entry.getValue().size() > 1) {
-        if (LOG_FINER) {
-          LOG.finer("mkdir " + linkRoot.getRelative(dir));
-        }
-        createDirectoryAndParents(linkRoot.getRelative(dir));
-      }
-    }
-    // Make dir links for single rooted dirs.
-    for (Map.Entry<PathFragment, Set<Path>> entry : dirRootsMap.entrySet()) {
-      PathFragment 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) {
-          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) + " " + linkRoot.getRelative(dir));
-        }
-        linkRoot.getRelative(dir).createSymbolicLink(root.getRelative(dir));
-      }
-    }
-    // Make links for dirs within packages, skip parent-only dirs.
-    for (Map.Entry<PathFragment, Set<Path>> entry : dirRootsMap.entrySet()) {
-      PathFragment 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, packageRootMap.keySet());
-        if (pkgDir != null) {
-          Path root = packageRootMap.get(pkgDir);
-          try {
-            Path absdir = root.getRelative(dir);
-            if (absdir.isDirectory()) {
-              if (LOG_FINER) {
-                LOG.finer("ln -s " + absdir + "/* " + linkRoot.getRelative(dir) + "/");
-              }
-              for (Path target : absdir.getDirectoryEntries()) {
-                PathFragment p = target.relativeTo(root);
-                if (!dirRootsMap.containsKey(p)) {
-                  //LOG.finest("ln -s " + target + " " + linkRoot.getRelative(p));
-                  linkRoot.getRelative(p).createSymbolicLink(target);
-                }
-              }
-            } else {
-              LOG.fine("Symlink planting skipping dir '" + absdir + "'");
-            }
-          } catch (IOException e) {
-            e.printStackTrace();
-          }
-          // Otherwise its just an otherwise empty common parent dir.
-        }
-      }
-    }
-
-    if (emptyPackagePath != null) {
-      // For the top-level directory, generate symlinks to everything in the directory instead of
-      // the directory itself.
-      for (Path target : emptyPackagePath.getDirectoryEntries()) {
-        String baseName = target.getBaseName();
-        // Create any links that don't exist yet and don't start with bazel-.
-        if (!baseName.startsWith(productName + "-")
-            && !linkRoot.getRelative(baseName).exists()) {
-          linkRoot.getRelative(baseName).createSymbolicLink(target);
-        }
-      }
-    }
-  }
-
   /****************************************************************************
    * Whole-file I/O utilities for characters and bytes. These convenience
    * methods are not efficient and should not be used for large amounts of data!