Ignore external/ directory in users' source tree when creating execro…

…ot symlink tree. Fixes bazelbuild#3857 (comment) RELNOTES: None PiperOrigin-RevId: 257786258
uber-common · Jul 12, 2019 · c6ca6c2 · c6ca6c2
1 parent 2204bd2
commit c6ca6c2
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 3 deletions.
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
@@ -110,8 +110,10 @@ private void plantSymlinkForestWithFullMainRepository(Path mainRepoRoot) throws
     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 @@ void plantSymlinkForest() throws IOException {
         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);
         }

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
@@ -309,6 +309,65 @@ public void testPlantSymlinkForestForMainRepo() throws Exception {
     assertLinksTo(linkRoot, outputBase, LabelConstants.EXTERNAL_PATH_PREFIX + "/X");
   }
 
+  @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");