From 14086a847670f88cbc2da0da39f73113e3c87356 Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Mon, 25 Jan 2021 14:27:55 -0800 Subject: [PATCH] fix: linker fix for invalid symlink creation path in createSymlinkAndPreserveContents --- internal/linker/index.js | 23 ++++++++------ internal/linker/link_node_modules.ts | 47 ++++++++++++---------------- 2 files changed, 33 insertions(+), 37 deletions(-) diff --git a/internal/linker/index.js b/internal/linker/index.js index eef8710225..1d32eaf3c5 100644 --- a/internal/linker/index.js +++ b/internal/linker/index.js @@ -99,9 +99,6 @@ function deleteDirectory(p) { function symlink(target, p) { return __awaiter(this, void 0, void 0, function* () { log_verbose(`symlink( ${p} -> ${target} )`); - if (!(yield exists(target))) { - return false; - } try { yield fs.promises.symlink(target, p, 'junction'); return true; @@ -455,7 +452,7 @@ function main(args, runfiles) { yield mkdirp(path.dirname(m.name)); if (m.link) { const [root, modulePath] = m.link; - let target = ''; + let target; switch (root) { case 'execroot': if (isExecroot) { @@ -487,17 +484,23 @@ function main(args, runfiles) { } } } - catch (_a) { - target = ''; + catch (err) { + target = undefined; + log_verbose(`runfiles resolve failed for module '${m.name}': ${err.message}`); } break; } - const stats = yield gracefulLstat(m.name); - if (stats !== null && (yield isLeftoverDirectoryFromLinker(stats, m.name))) { - yield createSymlinkAndPreserveContents(stats, m.name, target); + if (target) { + const stats = yield gracefulLstat(m.name); + if (stats !== null && (yield isLeftoverDirectoryFromLinker(stats, m.name))) { + yield createSymlinkAndPreserveContents(stats, m.name, target); + } + else { + yield symlink(target, m.name); + } } else { - yield symlink(target, m.name); + log_verbose(`no symlink target found for module ${m.name}`); } } if (m.children) { diff --git a/internal/linker/link_node_modules.ts b/internal/linker/link_node_modules.ts index 72f0dcd645..be2dff48d6 100644 --- a/internal/linker/link_node_modules.ts +++ b/internal/linker/link_node_modules.ts @@ -108,18 +108,6 @@ async function deleteDirectory(p: string) { async function symlink(target: string, p: string): Promise { log_verbose(`symlink( ${p} -> ${target} )`); - // 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 (!await exists(target)) { - // 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. - return false; - } - // Use junction on Windows since symlinks require elevated permissions. // We only link to directories so junctions work for us. try { @@ -714,7 +702,7 @@ export async function main(args: string[], runfiles: Runfiles) { if (m.link) { const [root, modulePath] = m.link; - let target: string = ''; + let target: string|undefined; switch (root) { case 'execroot': if (isExecroot) { @@ -749,25 +737,30 @@ export async function main(args: string[], runfiles: Runfiles) { throw e; } } - } catch { - target = ''; + } catch (err) { + target = undefined; + log_verbose(`runfiles resolve failed for module '${m.name}': ${err.message}`); } break; } - 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)) { - await createSymlinkAndPreserveContents(stats, m.name, target); + 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)) { + await createSymlinkAndPreserveContents(stats, m.name, target); + } else { + await symlink(target, m.name); + } } else { - await symlink(target, m.name); + log_verbose(`no symlink target found for module ${m.name}`); } }