Skip to content

Commit d19e20b

Browse files
gregmagolanalexeagle
gregmagolan
authored and
alexeagle
committed
fix: don't symlink execroot node_modules when under bazel run
1 parent 33665eb commit d19e20b

File tree

8 files changed

+207
-146
lines changed

8 files changed

+207
-146
lines changed

‎.circleci/bazel.rc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,6 @@ test --flaky_test_attempts=2
2323
# Expose the SSH_AUTH_SOCK variable to integration tests that need to run git_repository repository rules
2424
# This is needed by the CircleCI git-over-ssh environment
2525
test --test_env=SSH_AUTH_SOCK
26+
27+
# Keep going after first failure
28+
build --keep_going

‎internal/linker/index.js‎

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,8 @@ function main(args, runfiles) {
289289
log_verbose('execroot:', execroot ? execroot : 'not found');
290290
const isExecroot = startCwd == execroot;
291291
log_verbose('isExecroot:', isExecroot.toString());
292+
const isBazelRun = !!process.env['BUILD_WORKSPACE_DIRECTORY'];
293+
log_verbose('isBazelRun:', isBazelRun.toString());
292294
if (!isExecroot && execroot) {
293295
process.chdir(execroot);
294296
log_verbose('changed directory to execroot', execroot);
@@ -327,32 +329,46 @@ function main(args, runfiles) {
327329
else {
328330
workspaceNodeModules = undefined;
329331
}
332+
let primaryNodeModules;
330333
if (packagePath) {
331-
yield mkdirp(packagePath);
332-
}
333-
const execrootNodeModules = path.posix.join(packagePath, `node_modules`);
334-
if (workspaceNodeModules) {
335-
yield symlinkWithUnlink(workspaceNodeModules, execrootNodeModules);
334+
const binNodeModules = path.posix.join(bin, packagePath, 'node_modules');
335+
yield mkdirp(path.dirname(binNodeModules));
336+
if (workspaceNodeModules) {
337+
yield symlinkWithUnlink(workspaceNodeModules, binNodeModules);
338+
primaryNodeModules = workspaceNodeModules;
339+
}
340+
else {
341+
yield mkdirp(binNodeModules);
342+
primaryNodeModules = binNodeModules;
343+
}
344+
if (!isBazelRun) {
345+
const execrootNodeModules = path.posix.join(packagePath, 'node_modules');
346+
yield mkdirp(path.dirname(execrootNodeModules));
347+
yield symlinkWithUnlink(primaryNodeModules, execrootNodeModules);
348+
}
336349
}
337350
else {
338-
yield mkdirp(execrootNodeModules);
339-
}
340-
if (packagePath) {
341-
const packagePathBin = path.posix.join(bin, packagePath);
342-
yield mkdirp(`${packagePathBin}`);
343-
yield symlinkWithUnlink(execrootNodeModules, `${packagePathBin}/node_modules`);
351+
const execrootNodeModules = 'node_modules';
352+
if (workspaceNodeModules) {
353+
yield symlinkWithUnlink(workspaceNodeModules, execrootNodeModules);
354+
primaryNodeModules = workspaceNodeModules;
355+
}
356+
else {
357+
yield mkdirp(execrootNodeModules);
358+
primaryNodeModules = execrootNodeModules;
359+
}
344360
}
345361
if (!isExecroot) {
346-
const runfilesPackagePath = path.posix.join(startCwd, packagePath);
347-
yield mkdirp(`${runfilesPackagePath}`);
348-
yield symlinkWithUnlink(!packagePath && workspaceNodeModules ? workspaceNodeModules : execrootNodeModules, `${runfilesPackagePath}/node_modules`);
362+
const runfilesNodeModules = path.posix.join(startCwd, packagePath, 'node_modules');
363+
yield mkdirp(path.dirname(runfilesNodeModules));
364+
yield symlinkWithUnlink(primaryNodeModules, runfilesNodeModules);
349365
}
350366
if (process.env['RUNFILES']) {
351367
const stat = yield gracefulLstat(process.env['RUNFILES']);
352368
if (stat && stat.isDirectory()) {
353-
const runfilesPackagePath = path.posix.join(process.env['RUNFILES'], workspace, packagePath);
354-
yield mkdirp(`${runfilesPackagePath}`);
355-
yield symlinkWithUnlink(!packagePath && workspaceNodeModules ? workspaceNodeModules : execrootNodeModules, `${runfilesPackagePath}/node_modules`);
369+
const runfilesNodeModules = path.posix.join(process.env['RUNFILES'], workspace, 'node_modules');
370+
yield mkdirp(path.dirname(runfilesNodeModules));
371+
yield symlinkWithUnlink(primaryNodeModules, runfilesNodeModules);
356372
}
357373
}
358374
}
@@ -393,7 +409,9 @@ function main(args, runfiles) {
393409
}
394410
function linkModules(package_path, m) {
395411
return __awaiter(this, void 0, void 0, function* () {
396-
const symlinkIn = package_path ? `${package_path}/node_modules` : 'node_modules';
412+
const symlinkIn = package_path ?
413+
path.posix.join(bin, package_path, 'node_modules') :
414+
'node_modules';
397415
if (path.dirname(m.name)) {
398416
yield mkdirp(`${symlinkIn}/${path.dirname(m.name)}`);
399417
}

‎internal/linker/link_node_modules.ts‎

Lines changed: 49 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,9 @@ export async function main(args: string[], runfiles: Runfiles) {
424424
const isExecroot = startCwd == execroot;
425425
log_verbose('isExecroot:', isExecroot.toString());
426426

427+
const isBazelRun = !!process.env['BUILD_WORKSPACE_DIRECTORY']
428+
log_verbose('isBazelRun:', isBazelRun.toString());
429+
427430
if (!isExecroot && execroot) {
428431
// If we're not in the execroot and we've found one then change to the execroot
429432
// directory to create the node_modules symlinks
@@ -481,55 +484,66 @@ export async function main(args: string[], runfiles: Runfiles) {
481484
workspaceNodeModules = undefined;
482485
}
483486

487+
let primaryNodeModules;
484488
if (packagePath) {
485-
// In some cases packagePath may not exist in sandbox if there are no source deps
486-
// and only generated file deps. we create it so that we that we can link to
487-
// packagePath/node_modules since packagePathBin/node_modules is a symlink to
488-
// packagePath/node_modules and is unguarded in launcher.sh as we allow symlinks to fall
489-
// through to from output tree to source tree to prevent resolving the same npm package to
490-
// multiple locations on disk
491-
await mkdirp(packagePath);
492-
}
493-
494-
// There are third party modules at this package path
495-
const execrootNodeModules = path.posix.join(packagePath, `node_modules`);
489+
const binNodeModules = path.posix.join(bin, packagePath, 'node_modules');
490+
await mkdirp(path.dirname(binNodeModules));
491+
492+
// Create bin/<package_path>/node_modules symlink
493+
// (or empty directory if there are no 3rd party deps to symlink to)
494+
if (workspaceNodeModules) {
495+
await symlinkWithUnlink(workspaceNodeModules, binNodeModules);
496+
primaryNodeModules = workspaceNodeModules;
497+
} else {
498+
await mkdirp(binNodeModules);
499+
primaryNodeModules = binNodeModules;
500+
}
496501

497-
if (workspaceNodeModules) {
498-
// Execroot symlink -> external workspace node_modules
499-
await symlinkWithUnlink(workspaceNodeModules, execrootNodeModules);
502+
if (!isBazelRun) {
503+
// Special case under bazel run where we don't want to create node_modules
504+
// in an execroot under a package path as this will end up in the user's
505+
// workspace via the package path folder symlink
506+
const execrootNodeModules = path.posix.join(packagePath, 'node_modules');
507+
await mkdirp(path.dirname(execrootNodeModules));
508+
await symlinkWithUnlink(primaryNodeModules, execrootNodeModules);
509+
}
500510
} else {
501-
// Create an execroot node_modules directory since there are no third party node_modules to symlink to
502-
await mkdirp(execrootNodeModules);
503-
}
511+
const execrootNodeModules = 'node_modules';
504512

505-
if (packagePath) {
506-
// Bin symlink -> execroot node_modules
507-
// NB: don't do this for the root of the bin tree since standard node_modules resolution
508-
// will fall back to the execroot node_modules naturally
509-
// See https://github.com/bazelbuild/rules_nodejs/issues/3054
510-
const packagePathBin = path.posix.join(bin, packagePath);
511-
await mkdirp(`${packagePathBin}`);
512-
await symlinkWithUnlink(execrootNodeModules, `${packagePathBin}/node_modules`);
513+
// Create execroot/node_modules symlink (or empty directory if there are
514+
// no 3rd party deps to symlink to)
515+
if (workspaceNodeModules) {
516+
await symlinkWithUnlink(workspaceNodeModules, execrootNodeModules);
517+
primaryNodeModules = workspaceNodeModules;
518+
} else {
519+
await mkdirp(execrootNodeModules);
520+
primaryNodeModules = execrootNodeModules;
521+
}
522+
523+
// NB: Don't create a bin/node_modules since standard node_modules
524+
// resolution will fall back to the execroot node_modules naturally. See
525+
// https://github.com/bazelbuild/rules_nodejs/issues/3054
513526
}
514527

515-
// Start CWD symlink -> execroot node_modules
528+
// If start cwd was in runfiles then create
529+
// start/cwd
516530
if (!isExecroot) {
517-
const runfilesPackagePath = path.posix.join(startCwd, packagePath);
518-
await mkdirp(`${runfilesPackagePath}`);
531+
const runfilesNodeModules = path.posix.join(startCwd, packagePath, 'node_modules');
532+
await mkdirp(path.dirname(runfilesNodeModules));
519533
// Don't link to the root execroot node_modules if there is a workspace node_modules.
520534
// Bazel will delete that symlink on rebuild in the ibazel run context.
521-
await symlinkWithUnlink(!packagePath && workspaceNodeModules ? workspaceNodeModules : execrootNodeModules, `${runfilesPackagePath}/node_modules`);
535+
await symlinkWithUnlink(primaryNodeModules, runfilesNodeModules);
522536
}
523537

524538
// RUNFILES symlink -> execroot node_modules
525539
if (process.env['RUNFILES']) {
526540
const stat = await gracefulLstat(process.env['RUNFILES']);
527541
if (stat && stat.isDirectory()) {
528-
const runfilesPackagePath = path.posix.join(process.env['RUNFILES'], workspace, packagePath);
529-
await mkdirp(`${runfilesPackagePath}`);
542+
const runfilesNodeModules = path.posix.join(process.env['RUNFILES'], workspace, 'node_modules');
543+
await mkdirp(path.dirname(runfilesNodeModules));
530544
// Don't link to the root execroot node_modules if there is a workspace node_modules.
531545
// Bazel will delete that symlink on rebuild in the ibazel run context.
532-
await symlinkWithUnlink(!packagePath && workspaceNodeModules ? workspaceNodeModules : execrootNodeModules, `${runfilesPackagePath}/node_modules`);
546+
await symlinkWithUnlink(primaryNodeModules, runfilesNodeModules);
533547
}
534548
}
535549
}
@@ -593,7 +607,9 @@ export async function main(args: string[], runfiles: Runfiles) {
593607
}
594608

595609
async function linkModules(package_path: string, m: LinkerTreeElement) {
596-
const symlinkIn = package_path ? `${package_path}/node_modules` : 'node_modules';
610+
const symlinkIn = package_path ?
611+
path.posix.join(bin, package_path, 'node_modules') :
612+
'node_modules';
597613

598614
// ensure the parent directory exist
599615
if (path.dirname(m.name)) {

‎internal/node/launcher.sh‎

Lines changed: 25 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -271,29 +271,24 @@ else
271271
RUNFILES_ROOT=
272272
fi
273273

274-
# Tell the node_patches_script that programs should not escape the execroot
275-
export BAZEL_PATCH_ROOTS="${EXECROOT}"
276-
# Set all bazel managed node_modules directories as guarded so no symlinks may
277-
# escape and no symlinks may enter.
278-
# We always guard against the root node_modules where 1st party deps go.
279-
# (e.g., /private/.../execroot/build_bazel_rules_nodejs/node_modules)
280-
export BAZEL_PATCH_ROOTS="${BAZEL_PATCH_ROOTS},${EXECROOT}/node_modules"
274+
# Tell the node_patches_script that programs should not escape the execroot or
275+
# root node_modules
276+
# For example,
277+
# .../execroot/build_bazel_rules_nodejs
278+
# .../execroot/build_bazel_rules_nodejs/node_modules
279+
export BAZEL_PATCH_ROOTS="${EXECROOT},${EXECROOT}/node_modules"
281280
if [[ "${RUNFILES_ROOT}" ]]; then
282-
# If in runfiles, guard the runfiles root itself
283-
export BAZEL_PATCH_ROOTS="${BAZEL_PATCH_ROOTS},${RUNFILES_ROOT}"
284-
# If in runfiles guard the node_modules location in runfiles as well
285-
# (e.g., /private/.../execroot/build_bazel_rules_nodejs/bazel-out/darwin-fastbuild/bin/internal/linker/test/multi_linker/test.sh.runfiles/build_bazel_rules_nodejs/node_modules)
286-
export BAZEL_PATCH_ROOTS="${BAZEL_PATCH_ROOTS},${RUNFILES_ROOT}/${BAZEL_WORKSPACE}/node_modules"
281+
# Guard the RUNFILES_ROOT & RUNFILES_ROOT/workspace/node_modules
282+
export BAZEL_PATCH_ROOTS="${BAZEL_PATCH_ROOTS},${RUNFILES_ROOT},${RUNFILES_ROOT}/${BAZEL_WORKSPACE}/node_modules"
287283
fi
288-
if [[ "${RUNFILES}" ]]; then
289-
# If in runfiles, guard the RUNFILES root itself
290-
export BAZEL_PATCH_ROOTS="${BAZEL_PATCH_ROOTS},${RUNFILES}"
291-
# If RUNFILES is set, guard the RUNFILES node_modules as well
292-
export BAZEL_PATCH_ROOTS="${BAZEL_PATCH_ROOTS},${RUNFILES}/${BAZEL_WORKSPACE}/node_modules"
284+
if [[ "${RUNFILES}" ]] && [[ "${RUNFILES}" != "${RUNFILES_ROOT}" ]]; then
285+
# Same as above but for RUNFILES is not equal to RUNFILES_ROOT
286+
export BAZEL_PATCH_ROOTS="${BAZEL_PATCH_ROOTS},${RUNFILES},${RUNFILES}/${BAZEL_WORKSPACE}/node_modules"
293287
fi
294288
if [[ -n "${BAZEL_NODE_MODULES_ROOTS:-}" ]]; then
295-
# BAZEL_NODE_MODULES_ROOTS is in the format "<path>:<workspace>,<path>:<workspace>"
296-
# (e.g., "internal/linker/test:npm_internal_linker_test,:npm")
289+
# BAZEL_NODE_MODULES_ROOTS is in the format "<path>:<workspace>,<path>:<workspace>".
290+
# For example,
291+
# "internal/linker/test:npm_internal_linker_test,:npm"
297292
if [[ -n "${VERBOSE_LOGS:-}" ]]; then
298293
echo "BAZEL_NODE_MODULES_ROOTS=${BAZEL_NODE_MODULES_ROOTS}" >&2
299294
fi
@@ -310,27 +305,20 @@ if [[ -n "${BAZEL_NODE_MODULES_ROOTS:-}" ]]; then
310305
root_path="${root_pair[0]}"
311306
root_workspace="${root_pair[1]:-}"
312307
if [[ "${root_path}" ]]; then
313-
# Guard non-root node_modules as well
314-
# (e.g., /private/.../execroot/build_bazel_rules_nodejs/internal/linker/test/node_modules)
315-
export BAZEL_PATCH_ROOTS="${BAZEL_PATCH_ROOTS},${EXECROOT}/${root_path}/node_modules"
308+
# Guard non-root execroot & bazel-out node_modules as well.
309+
# For example,
310+
# .../execroot/build_bazel_rules_nodejs/internal/linker/test/node_modules
311+
# .../execroot/build_bazel_rules_nodejs/bazel-out/*/bin/internal/linker/test/node_modules
312+
export BAZEL_PATCH_ROOTS="${BAZEL_PATCH_ROOTS},${EXECROOT}/${root_path}/node_modules,${EXECROOT}/bazel-out/*/bin/${root_path}/node_modules"
316313
if [[ "${RUNFILES_ROOT}" ]]; then
317-
# If in runfiles guard the node_modules location in runfiles as well
318-
# (e.g., /private/.../execroot/build_bazel_rules_nodejs/bazel-out/darwin-fastbuild/bin/internal/linker/test/multi_linker/test.sh.runfiles/build_bazel_rules_nodejs/internal/linker/test/node_modules)
314+
# If in runfiles guard the node_modules location in runfiles as well.
315+
# For example,
316+
# .../execroot/build_bazel_rules_nodejs/bazel-out/darwin-fastbuild/bin/internal/linker/test/multi_linker/test.sh.runfiles/build_bazel_rules_nodejs/internal/linker/test/node_modules
319317
export BAZEL_PATCH_ROOTS="${BAZEL_PATCH_ROOTS},${RUNFILES_ROOT}/${BAZEL_WORKSPACE}/${root_path}/node_modules"
320318
fi
321-
fi
322-
# TODO: the following guards on the external workspaces may not be necessary and could be removed in the future with care
323-
if [[ "${root_workspace}" ]] && [[ "${root_workspace}" != "${BAZEL_WORKSPACE}" ]]; then
324-
# Guard the external workspaces if they are not the user workspace
325-
# (e.g., /private/.../execroot/build_bazel_rules_nodejs/external/npm_internal_linker_test/node_modules)
326-
export BAZEL_PATCH_ROOTS="${BAZEL_PATCH_ROOTS},${EXECROOT}/external/${root_workspace}/node_modules"
327-
if [[ "${RUNFILES_ROOT}" ]]; then
328-
# If in runfiles guard the external workspace location in runfiles as well
329-
# (e.g., /private/.../execroot/build_bazel_rules_nodejs/bazel-out/darwin-fastbuild/bin/internal/linker/test/multi_linker/test.sh.runfiles/npm_internal_linker_test/node_modules)
330-
export BAZEL_PATCH_ROOTS="${BAZEL_PATCH_ROOTS},${RUNFILES_ROOT}/${root_workspace}/node_modules"
331-
# and include the legacy runfiles location incase legacy runfiles are enabled
332-
# (e.g., /private/.../bazel-out/darwin-fastbuild/bin/internal/linker/test/multi_linker/test.sh.runfiles/build_bazel_rules_nodejs/external/npm_internal_linker_test/node_modules)
333-
export BAZEL_PATCH_ROOTS="${BAZEL_PATCH_ROOTS},${RUNFILES_ROOT}/${BAZEL_WORKSPACE}/external/${root_workspace}/node_modules"
319+
if [[ "${RUNFILES}" ]] && [[ "${RUNFILES}" != "${RUNFILES_ROOT}" ]]; then
320+
# Same as above but for RUNFILES is not equal to RUNFILES_ROOT
321+
export BAZEL_PATCH_ROOTS="${BAZEL_PATCH_ROOTS},${RUNFILES}/${BAZEL_WORKSPACE}/${root_path}/node_modules"
334322
fi
335323
fi
336324
fi

‎internal/node/node_patches.js‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,18 @@ const patcher = (fs = fs__default['default'], roots) => {
486486
};
487487
exports.patcher = patcher;
488488
function isOutPath(root, str) {
489-
return !root || (!str.startsWith(root + path__default['default'].sep) && str !== root);
489+
if (!root)
490+
return true;
491+
let strParts = str.split(path__default['default'].sep);
492+
let rootParts = root.split(path__default['default'].sep);
493+
let i = 0;
494+
for (; i < rootParts.length && i < strParts.length; i++) {
495+
if (rootParts[i] === strParts[i] || rootParts[i] === '*') {
496+
continue;
497+
}
498+
break;
499+
}
500+
return i < rootParts.length;
490501
}
491502
exports.isOutPath = isOutPath;
492503
const escapeFunction = (roots) => {

0 commit comments

Comments
 (0)