From 546341687417bbe03201ce4fdae5e03ebe391eb9 Mon Sep 17 00:00:00 2001
From: Greg Magolan <greg.magolan@robinhood.com>
Date: Mon, 25 Jan 2021 18:57:06 -0800
Subject: [PATCH] chore: cleanup

---
 internal/linker/index.js             | 17 ++++++++---
 internal/linker/link_node_modules.ts | 42 ++++++++++++++++++++--------
 2 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/internal/linker/index.js b/internal/linker/index.js
index 1d32eaf3c5..b9346636b3 100644
--- a/internal/linker/index.js
+++ b/internal/linker/index.js
@@ -490,9 +490,10 @@ function main(args, runfiles) {
                             }
                             break;
                     }
-                    if (target) {
-                        const stats = yield gracefulLstat(m.name);
-                        if (stats !== null && (yield isLeftoverDirectoryFromLinker(stats, m.name))) {
+                    const stats = yield gracefulLstat(m.name);
+                    const isLeftOver = (stats !== null && (yield isLeftoverDirectoryFromLinker(stats, m.name)));
+                    if (target && (yield exists(target))) {
+                        if (stats !== null && isLeftOver) {
                             yield createSymlinkAndPreserveContents(stats, m.name, target);
                         }
                         else {
@@ -500,7 +501,15 @@ function main(args, runfiles) {
                         }
                     }
                     else {
-                        log_verbose(`no symlink target found for module ${m.name}`);
+                        if (!target) {
+                            log_verbose(`no symlink target found for module ${m.name}`);
+                        }
+                        else {
+                            log_verbose(`potential target ${target} does not exists for module ${m.name}`);
+                        }
+                        if (isLeftOver) {
+                            yield unlink(m.name);
+                        }
                     }
                 }
                 if (m.children) {
diff --git a/internal/linker/link_node_modules.ts b/internal/linker/link_node_modules.ts
index be2dff48d6..3cd59bfc19 100644
--- a/internal/linker/link_node_modules.ts
+++ b/internal/linker/link_node_modules.ts
@@ -744,23 +744,41 @@ export async function main(args: string[], runfiles: Runfiles) {
           break;
       }
 
-      if (target) {
-        const stats = await gracefulLstat(m.name);
-        // In environments where runfiles are not symlinked (e.g. Windows), existing linked
-        // modules are preserved. This could cause issues when a link is created at higher level
-        // as a conflicting directory is already on disk. e.g. consider in a previous run, we
-        // linked the modules `my-pkg/overlay`. Later on, in another run, we have a module mapping
-        // for `my-pkg` itself. The linker cannot create `my-pkg` because the directory `my-pkg`
-        // already exists. To ensure that the desired link is generated, we create the new desired
-        // link and move all previous nested links from the old module into the new link. Read more
-        // about this in the description of `createSymlinkAndPreserveContents`.
-        if (stats !== null && await isLeftoverDirectoryFromLinker(stats, m.name)) {
+      // In environments where runfiles are not symlinked (e.g. Windows), existing linked
+      // modules are preserved. This could cause issues when a link is created at higher level
+      // as a conflicting directory is already on disk. e.g. consider in a previous run, we
+      // linked the modules `my-pkg/overlay`. Later on, in another run, we have a module mapping
+      // for `my-pkg` itself. The linker cannot create `my-pkg` because the directory `my-pkg`
+      // already exists. To ensure that the desired link is generated, we create the new desired
+      // link and move all previous nested links from the old module into the new link. Read more
+      // about this in the description of `createSymlinkAndPreserveContents`.
+      const stats = await gracefulLstat(m.name);
+      const isLeftOver = (stats !== null && await isLeftoverDirectoryFromLinker(stats, m.name));
+
+      // Check if the target exists before creating the symlink.
+      // This is an extra filesystem access on top of the symlink but
+      // it is necessary for the time being.
+      if (target && await exists(target)) {
+        if (stats !== null && isLeftOver) {
           await createSymlinkAndPreserveContents(stats, m.name, target);
         } else {
           await symlink(target, m.name);
         }
       } else {
-        log_verbose(`no symlink target found for module ${m.name}`);
+        if (!target) {
+          log_verbose(`no symlink target found for module ${m.name}`);
+        } else {
+          // This can happen if a module mapping is propogated from a dependency
+          // but the target that generated the mapping in not in the deps. We don't
+          // want to create symlinks to non-existant targets as this will
+          // break any nested symlinks that may be created under the module name
+          // after this.
+          log_verbose(`potential target ${target} does not exists for module ${m.name}`);
+        }
+        if (isLeftOver) {
+          // Remove left over directory if it exists
+          await unlink(m.name);
+        }
       }
     }