Skip to content

Commit

Permalink
fix(builtin): fix runfiles linking issue on Windows
Browse files Browse the repository at this point in the history
This is a partial fix for linker issue on Windows when all build actions are cached bazel-contrib#1781. This fixes incorrectly symlinks but it does not actually address the underlying issue of programs (tests) running outside of the sandbox and directly out of the execroot on Windows (either out of execroot sources or execroot bazel-out).

If all build actions are cached then the linker only runs for the test that is executed. If there are no third-part npm deps then it makes a node_modules folder in the runfiles for that test and puts its symlinks in there. The problem is that when the test runs, the .js spec is in execroot bazel-out and it doesn't see the node_modules created in the runfiles folder. Since no build actions have run that may have created the symlinks needed for the test in the execroot node_modules, the symlinks that it needs do not exist.
  • Loading branch information
gregmagolan committed Apr 6, 2020
1 parent 9d91154 commit fc67b22
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 34 deletions.
22 changes: 4 additions & 18 deletions internal/linker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,28 +455,14 @@ Include as much of the build output as you can without disclosing anything confi
case 'execroot':
if (runfiles.execroot) {
target = path.posix.join(workspaceAbs, modulePath);
break;
}
else {
// If under runfiles, convert from execroot path to runfiles path.
// First strip the bin portion if it exists:
let runfilesPath = modulePath;
if (runfilesPath.startsWith(`${bin}/`)) {
runfilesPath = runfilesPath.slice(bin.length + 1);
}
else if (runfilesPath === bin) {
runfilesPath = '';
}
// Next replace `external/` with `../` if it exists:
if (runfilesPath.startsWith(externalPrefix)) {
runfilesPath = `../${runfilesPath.slice(externalPrefix.length)}`;
}
target = path.posix.join(workspaceAbs, runfilesPath);
}
break;
// If under runfiles, the fall through to 'runfiles' case
case 'runfiles':
// Transform execroot path to the runfiles manifest path so that
// it can be resolved with runfiles.resolve()
// it can be resolved with runfiles.resolve().
let runfilesPath = modulePath;
// First strip the bin portion if it exists
if (runfilesPath.startsWith(`${bin}/`)) {
runfilesPath = runfilesPath.slice(bin.length + 1);
}
Expand Down
20 changes: 4 additions & 16 deletions internal/linker/link_node_modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -520,26 +520,14 @@ export async function main(args: string[], runfiles: Runfiles) {
case 'execroot':
if (runfiles.execroot) {
target = path.posix.join(workspaceAbs, modulePath);
} else {
// If under runfiles, convert from execroot path to runfiles path.
// First strip the bin portion if it exists:
let runfilesPath = modulePath;
if (runfilesPath.startsWith(`${bin}/`)) {
runfilesPath = runfilesPath.slice(bin.length + 1);
} else if (runfilesPath === bin) {
runfilesPath = '';
}
// Next replace `external/` with `../` if it exists:
if (runfilesPath.startsWith(externalPrefix)) {
runfilesPath = `../${runfilesPath.slice(externalPrefix.length)}`;
}
target = path.posix.join(workspaceAbs, runfilesPath);
break;
}
break;
// If under runfiles, the fall through to 'runfiles' case
case 'runfiles':
// Transform execroot path to the runfiles manifest path so that
// it can be resolved with runfiles.resolve()
// it can be resolved with runfiles.resolve().
let runfilesPath = modulePath;
// First strip the bin portion if it exists
if (runfilesPath.startsWith(`${bin}/`)) {
runfilesPath = runfilesPath.slice(bin.length + 1);
}
Expand Down

0 comments on commit fc67b22

Please sign in to comment.