commit c6ca6c2ccd9e11a87f837341ae05d2ba037975b1
author pcloudy <pcloudy@google.com> Fri Jul 12 05:29:41 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Fri Jul 12 05:30:39 2019 -0700
tree d142c5256c0ae9370af19030b4cc9c42cb1cf58e
parent 2204bd2d329140f1d7aa6ce2552dd45340ab7d10

Ignore external/ directory in users' source tree when creating execroot symlink tree.

Fixes https://github.com/bazelbuild/bazel/issues/3857#issuecomment-510538152

RELNOTES: None
PiperOrigin-RevId: 257786258

src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java

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 aca8cfe..0f04ceb 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
@@ -110,8 +110,10 @@
     for (Path target : mainRepoRoot.getDirectoryEntries()) {
       String baseName = target.getBaseName();
       Path execPath = execroot.getRelative(baseName);
-      // Create any links that don't start with bazel-.
-      if (!baseName.startsWith(prefix)) {
+      // Create any links that don't start with bazel-, and ignore external/ directory if
+      // user has it in the source tree because it conflicts with external repository location.
+      if (!baseName.startsWith(prefix)
+          && !baseName.equals(LabelConstants.EXTERNAL_PATH_PREFIX.getBaseName())) {
         execPath.createSymbolicLink(target);
       }
     }
@@ -284,7 +286,13 @@
         if (pkgId.getPackageFragment().equals(PathFragment.EMPTY_FRAGMENT)) {
           shouldLinkAllTopLevelItems = true;
         } else {
-          Path execrootLink = execroot.getRelative(pkgId.getPackageFragment().getSegment(0));
+          String baseName = pkgId.getPackageFragment().getSegment(0);
+          // ignore external/ directory if user has it in the source tree
+          // because it conflicts with external repository location.
+          if (baseName.equals(LabelConstants.EXTERNAL_PATH_PREFIX.getBaseName())) {
+            continue;
+          }
+          Path execrootLink = execroot.getRelative(baseName);
           Path sourcePath = entry.getValue().getRelative(pkgId.getSourceRoot().getSegment(0));
           mainRepoLinks.putIfAbsent(execrootLink, sourcePath);
         }

src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java
index 17124c6..4be5fa1 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java
@@ -310,6 +310,65 @@
   }
 
   @Test
+  public void testTestExternalDirInMainRepoIsIgnored1() throws Exception {
+    // Test external/ is ignored even when packages like "//external/foo" is specified.
+    Root outputBase = Root.fromPath(fileSystem.getPath("/ob"));
+    Root mainRepo = Root.fromPath(fileSystem.getPath("/my_repo"));
+    Path linkRoot = outputBase.getRelative("execroot/ws_name");
+
+    linkRoot.createDirectoryAndParents();
+    mainRepo.asPath().createDirectoryAndParents();
+
+    ImmutableMap<PackageIdentifier, Root> packageRootMap =
+        ImmutableMap.<PackageIdentifier, Root>builder()
+            .put(createMainPkg(mainRepo, "dir1/pkg/foo"), mainRepo)
+            .put(createMainPkg(mainRepo, "dir2/pkg"), mainRepo)
+            .put(createMainPkg(mainRepo, "dir3"), mainRepo)
+            // external/ should not be linked even we have "//external/foo" package
+            .put(createMainPkg(mainRepo, "external/foo"), mainRepo)
+            .put(createExternalPkg(outputBase, "X", "dir_x/pkg"), outputBase)
+            .build();
+
+    new SymlinkForest(packageRootMap, linkRoot, TestConstants.PRODUCT_NAME).plantSymlinkForest();
+
+    assertLinksTo(linkRoot, mainRepo, "dir1");
+    assertLinksTo(linkRoot, mainRepo, "dir2");
+    assertLinksTo(linkRoot, mainRepo, "dir3");
+    assertLinksTo(linkRoot, outputBase, LabelConstants.EXTERNAL_PATH_PREFIX + "/X");
+    assertThat(outputBase.getRelative("external/foo").exists()).isFalse();
+  }
+
+  @Test
+  public void testTestExternalDirInMainRepoIsIgnored2() throws Exception {
+    // Test external/ is ignored when root package "//:" is specified.
+    Root outputBase = Root.fromPath(fileSystem.getPath("/ob"));
+    Root mainRepo = Root.fromPath(fileSystem.getPath("/my_repo"));
+    Path linkRoot = outputBase.getRelative("execroot/ws_name");
+
+    linkRoot.createDirectoryAndParents();
+    mainRepo.asPath().createDirectoryAndParents();
+    mainRepo.getRelative("dir3").createDirectoryAndParents();
+    mainRepo.getRelative("external/foo").createDirectoryAndParents();
+
+    ImmutableMap<PackageIdentifier, Root> packageRootMap =
+        ImmutableMap.<PackageIdentifier, Root>builder()
+            .put(createMainPkg(mainRepo, "dir1/pkg/foo"), mainRepo)
+            .put(createMainPkg(mainRepo, "dir2/pkg"), mainRepo)
+            // Empty package will cause every top-level files to be linked, except external/
+            .put(createMainPkg(mainRepo, ""), mainRepo)
+            .put(createExternalPkg(outputBase, "X", "dir_x/pkg"), outputBase)
+            .build();
+
+    new SymlinkForest(packageRootMap, linkRoot, TestConstants.PRODUCT_NAME).plantSymlinkForest();
+
+    assertLinksTo(linkRoot, mainRepo, "dir1");
+    assertLinksTo(linkRoot, mainRepo, "dir2");
+    assertLinksTo(linkRoot, mainRepo, "dir3");
+    assertLinksTo(linkRoot, outputBase, LabelConstants.EXTERNAL_PATH_PREFIX + "/X");
+    assertThat(outputBase.getRelative("external/foo").exists()).isFalse();
+  }
+
+  @Test
   public void testExternalPackage() throws Exception {
     Path linkRoot = fileSystem.getPath("/linkRoot");
     linkRoot.createDirectoryAndParents();