Skip to content

Commit

Permalink
fix(builtin): under runfiles linker should link node_modules folder a…
Browse files Browse the repository at this point in the history
…t root of runfiles tree

Current it is link node_modules folder in the cwd which is `runfiles/wksp/node_modules` but this location means that script in external repositories such as `runfiles/external_wksp/path/to/script.js` will not resolve packages. This PR changes the linker to link to `runfiles/node_modules`.
  • Loading branch information
gregmagolan committed Apr 3, 2020
1 parent ee7bc69 commit 13510ad
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 13 deletions.
15 changes: 11 additions & 4 deletions internal/linker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ Include as much of the build output as you can without disclosing anything confi
*/
function resolveRoot(root, runfiles) {
return __awaiter(this, void 0, void 0, function* () {
if (!runfiles.execroot) {
// Under runfiles, the repository should be layed out in the parent directory
// since bazel sets our working directory to the repository where the build is happening
process.chdir('..');
}
// create a node_modules directory if no root
// this will be the case if only first-party modules are installed
if (!root) {
Expand All @@ -135,7 +140,7 @@ Include as much of the build output as you can without disclosing anything confi
else {
// Under runfiles, the repository should be layed out in the parent directory
// since bazel sets our working directory to the repository where the build is happening
return path.join('..', root);
return root;
}
});
}
Expand Down Expand Up @@ -425,12 +430,14 @@ Include as much of the build output as you can without disclosing anything confi
const [modulesManifest] = args;
let { bin, root, modules, workspace } = JSON.parse(fs.readFileSync(modulesManifest));
modules = modules || {};
log_verbose(`module manifest '${modulesManifest}': workspace ${workspace}, bin ${bin}, root ${root} with first-party packages\n`, modules);
log_verbose('manifest file', modulesManifest);
log_verbose('manifest contents', JSON.stringify({ workspace, bin, root, modules }, null, 2));
// Bazel starts actions with pwd=execroot/my_wksp
const workspaceDir = path.resolve('.');
// resolveRoot will change the cwd when under runfiles
const rootDir = yield resolveRoot(root, runfiles);
log_verbose('resolved root', root, 'to', rootDir);
log_verbose('cwd', process.cwd());
// Bazel starts actions with pwd=execroot/my_wksp
const workspaceDir = path.resolve('.');
// Create the $pwd/node_modules directory that node will resolve from
yield symlink(rootDir, 'node_modules');
process.chdir(rootDir);
Expand Down
20 changes: 12 additions & 8 deletions internal/linker/link_node_modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ async function symlink(target: string, p: string): Promise<boolean> {
* @param root a string like 'npm/node_modules'
*/
async function resolveRoot(root: string|undefined, runfiles: Runfiles) {
if (!runfiles.execroot) {
// Under runfiles, the repository should be layed out in the parent directory
// since bazel sets our working directory to the repository where the build is happening
process.chdir('..');
}
// create a node_modules directory if no root
// this will be the case if only first-party modules are installed
if (!root) {
Expand All @@ -118,7 +123,7 @@ async function resolveRoot(root: string|undefined, runfiles: Runfiles) {
} else {
// Under runfiles, the repository should be layed out in the parent directory
// since bazel sets our working directory to the repository where the build is happening
return path.join('..', root);
return root;
}
}

Expand Down Expand Up @@ -485,18 +490,17 @@ export async function main(args: string[], runfiles: Runfiles) {
const [modulesManifest] = args;
let {bin, root, modules, workspace} = JSON.parse(fs.readFileSync(modulesManifest));
modules = modules || {};
log_verbose(
`module manifest '${modulesManifest}': workspace ${workspace}, bin ${bin}, root ${
root} with first-party packages\n`,
modules);
log_verbose('manifest file', modulesManifest);
log_verbose('manifest contents', JSON.stringify({workspace, bin, root, modules}, null, 2));

// Bazel starts actions with pwd=execroot/my_wksp
const workspaceDir = path.resolve('.');

// resolveRoot will change the cwd when under runfiles
const rootDir = await resolveRoot(root, runfiles);
log_verbose('resolved root', root, 'to', rootDir);
log_verbose('cwd', process.cwd());

// Bazel starts actions with pwd=execroot/my_wksp
const workspaceDir = path.resolve('.');

// Create the $pwd/node_modules directory that node will resolve from
await symlink(rootDir, 'node_modules');
process.chdir(rootDir);
Expand Down
2 changes: 1 addition & 1 deletion internal/linker/test/link_node_modules.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,6 @@ describe('link_node_modules', () => {

// The linker expects to run as its own process, so it changes the wd
process.chdir(path.join(process.env['TEST_TMPDIR']!, workspace));
expect(fs.readdirSync(path.join('node_modules', 'some-package'))).toContain('index.js');
expect(fs.readdirSync(path.join('..', 'node_modules', 'some-package'))).toContain('index.js');
});
});

0 comments on commit 13510ad

Please sign in to comment.