Skip to content

Commit d18a1ff

Browse files
gregmagolanAlex Eagle
authored and
Alex Eagle
committed
fix: linker fix for invalid symlink creation path in createSymlinkAndPreserveContents
1 parent 1011b22 commit d18a1ff

File tree

2 files changed

+33
-37
lines changed

2 files changed

+33
-37
lines changed

internal/linker/index.js

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,6 @@ function deleteDirectory(p) {
9999
function symlink(target, p) {
100100
return __awaiter(this, void 0, void 0, function* () {
101101
log_verbose(`symlink( ${p} -> ${target} )`);
102-
if (!(yield exists(target))) {
103-
return false;
104-
}
105102
try {
106103
yield fs.promises.symlink(target, p, 'junction');
107104
return true;
@@ -455,7 +452,7 @@ function main(args, runfiles) {
455452
yield mkdirp(path.dirname(m.name));
456453
if (m.link) {
457454
const [root, modulePath] = m.link;
458-
let target = '<package linking failed>';
455+
let target;
459456
switch (root) {
460457
case 'execroot':
461458
if (isExecroot) {
@@ -487,17 +484,23 @@ function main(args, runfiles) {
487484
}
488485
}
489486
}
490-
catch (_a) {
491-
target = '<runfiles resolution failed>';
487+
catch (err) {
488+
target = undefined;
489+
log_verbose(`runfiles resolve failed for module '${m.name}': ${err.message}`);
492490
}
493491
break;
494492
}
495-
const stats = yield gracefulLstat(m.name);
496-
if (stats !== null && (yield isLeftoverDirectoryFromLinker(stats, m.name))) {
497-
yield createSymlinkAndPreserveContents(stats, m.name, target);
493+
if (target) {
494+
const stats = yield gracefulLstat(m.name);
495+
if (stats !== null && (yield isLeftoverDirectoryFromLinker(stats, m.name))) {
496+
yield createSymlinkAndPreserveContents(stats, m.name, target);
497+
}
498+
else {
499+
yield symlink(target, m.name);
500+
}
498501
}
499502
else {
500-
yield symlink(target, m.name);
503+
log_verbose(`no symlink target found for module ${m.name}`);
501504
}
502505
}
503506
if (m.children) {

internal/linker/link_node_modules.ts

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -108,18 +108,6 @@ async function deleteDirectory(p: string) {
108108
async function symlink(target: string, p: string): Promise<boolean> {
109109
log_verbose(`symlink( ${p} -> ${target} )`);
110110

111-
// Check if the target exists before creating the symlink.
112-
// This is an extra filesystem access on top of the symlink but
113-
// it is necessary for the time being.
114-
if (!await exists(target)) {
115-
// This can happen if a module mapping is propogated from a dependency
116-
// but the target that generated the mapping in not in the deps. We don't
117-
// want to create symlinks to non-existant targets as this will
118-
// break any nested symlinks that may be created under the module name
119-
// after this.
120-
return false;
121-
}
122-
123111
// Use junction on Windows since symlinks require elevated permissions.
124112
// We only link to directories so junctions work for us.
125113
try {
@@ -714,7 +702,7 @@ export async function main(args: string[], runfiles: Runfiles) {
714702

715703
if (m.link) {
716704
const [root, modulePath] = m.link;
717-
let target: string = '<package linking failed>';
705+
let target: string|undefined;
718706
switch (root) {
719707
case 'execroot':
720708
if (isExecroot) {
@@ -749,25 +737,30 @@ export async function main(args: string[], runfiles: Runfiles) {
749737
throw e;
750738
}
751739
}
752-
} catch {
753-
target = '<runfiles resolution failed>';
740+
} catch (err) {
741+
target = undefined;
742+
log_verbose(`runfiles resolve failed for module '${m.name}': ${err.message}`);
754743
}
755744
break;
756745
}
757746

758-
const stats = await gracefulLstat(m.name);
759-
// In environments where runfiles are not symlinked (e.g. Windows), existing linked
760-
// modules are preserved. This could cause issues when a link is created at higher level
761-
// as a conflicting directory is already on disk. e.g. consider in a previous run, we
762-
// linked the modules `my-pkg/overlay`. Later on, in another run, we have a module mapping
763-
// for `my-pkg` itself. The linker cannot create `my-pkg` because the directory `my-pkg`
764-
// already exists. To ensure that the desired link is generated, we create the new desired
765-
// link and move all previous nested links from the old module into the new link. Read more
766-
// about this in the description of `createSymlinkAndPreserveContents`.
767-
if (stats !== null && await isLeftoverDirectoryFromLinker(stats, m.name)) {
768-
await createSymlinkAndPreserveContents(stats, m.name, target);
747+
if (target) {
748+
const stats = await gracefulLstat(m.name);
749+
// In environments where runfiles are not symlinked (e.g. Windows), existing linked
750+
// modules are preserved. This could cause issues when a link is created at higher level
751+
// as a conflicting directory is already on disk. e.g. consider in a previous run, we
752+
// linked the modules `my-pkg/overlay`. Later on, in another run, we have a module mapping
753+
// for `my-pkg` itself. The linker cannot create `my-pkg` because the directory `my-pkg`
754+
// already exists. To ensure that the desired link is generated, we create the new desired
755+
// link and move all previous nested links from the old module into the new link. Read more
756+
// about this in the description of `createSymlinkAndPreserveContents`.
757+
if (stats !== null && await isLeftoverDirectoryFromLinker(stats, m.name)) {
758+
await createSymlinkAndPreserveContents(stats, m.name, target);
759+
} else {
760+
await symlink(target, m.name);
761+
}
769762
} else {
770-
await symlink(target, m.name);
763+
log_verbose(`no symlink target found for module ${m.name}`);
771764
}
772765
}
773766

0 commit comments

Comments
 (0)