From 13510adb265e19e1818d19f8fc5034a83f082727 Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Thu, 2 Apr 2020 15:48:11 -0700 Subject: [PATCH] fix(builtin): under runfiles linker should link node_modules folder at 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`. --- internal/linker/index.js | 15 ++++++++++---- internal/linker/link_node_modules.ts | 20 +++++++++++-------- .../linker/test/link_node_modules.spec.ts | 2 +- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/internal/linker/index.js b/internal/linker/index.js index 95107354a0..2d1aadad82 100644 --- a/internal/linker/index.js +++ b/internal/linker/index.js @@ -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) { @@ -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; } }); } @@ -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); diff --git a/internal/linker/link_node_modules.ts b/internal/linker/link_node_modules.ts index 6fee528644..06b546a83a 100644 --- a/internal/linker/link_node_modules.ts +++ b/internal/linker/link_node_modules.ts @@ -95,6 +95,11 @@ async function symlink(target: string, p: string): Promise { * @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) { @@ -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; } } @@ -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); diff --git a/internal/linker/test/link_node_modules.spec.ts b/internal/linker/test/link_node_modules.spec.ts index c756298f08..d6a93909c8 100644 --- a/internal/linker/test/link_node_modules.spec.ts +++ b/internal/linker/test/link_node_modules.spec.ts @@ -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'); }); }); \ No newline at end of file