From 7e980890fbe93de42cbffd5662b0961fb59f5d04 Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Thu, 17 Oct 2019 13:44:19 -0700 Subject: [PATCH] fix(builtin): allow more than 2 segments in linker module names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The linking code is made more robust to handle more than two segments in the module names. There is no technical reason to not allow this although it does diverge from the npm package standard of either foo or @scope/foo names. Module names are sorted and grouped to ensure that nested modules such as ‘c’ are symlinked first before ‘c/c’, ‘c/c/c/c’, etc and that groups of nested modules are symlinked in order while maintaining the parallelized symlink code for module names that are not nested. Gaps in the nested paths are filled with mkdir as needed and common paths between groups are mkdir'd before handing off to the parallelized symlink code. --- internal/linker/index.js | 225 +++++++++++++----- internal/linker/link_node_modules.ts | 192 +++++++++++---- .../linker/test/link_node_modules.spec.ts | 138 +++++++++-- packages/rollup/test/integration/BUILD.bazel | 6 +- .../rollup/test/integration/far/a/BUILD.bazel | 9 + .../test/integration/far/a/b/c/BUILD.bazel | 9 + .../test/integration/far/a/b/c/index.ts | 1 + .../rollup/test/integration/far/a/index.ts | 1 + packages/rollup/test/integration/foo.js | 9 +- .../rollup/test/integration/foo/BUILD.bazel | 7 +- .../rollup/test/integration/foo_a/BUILD.bazel | 10 + .../rollup/test/integration/foo_a/index.ts | 4 + .../test/integration/foo_aaa/BUILD.bazel | 10 + .../rollup/test/integration/foo_aaa/index.ts | 4 + .../rollup/test/integration/fum/BUILD.bazel | 3 - .../rollup/test/integration/golden.amd.js_ | 13 +- .../rollup/test/integration/golden.cjs.js_ | 13 +- .../rollup/test/integration/golden.esm.js_ | 13 +- .../rollup/test/integration/golden.iife.js_ | 13 +- .../rollup/test/integration/golden.system.js_ | 13 +- .../rollup/test/integration/golden.umd.js_ | 13 +- .../test/integration/golden.umd.sha256_ | 2 +- 22 files changed, 559 insertions(+), 149 deletions(-) create mode 100644 packages/rollup/test/integration/far/a/BUILD.bazel create mode 100644 packages/rollup/test/integration/far/a/b/c/BUILD.bazel create mode 100644 packages/rollup/test/integration/far/a/b/c/index.ts create mode 100644 packages/rollup/test/integration/far/a/index.ts create mode 100644 packages/rollup/test/integration/foo_a/BUILD.bazel create mode 100644 packages/rollup/test/integration/foo_a/index.ts create mode 100644 packages/rollup/test/integration/foo_aaa/BUILD.bazel create mode 100644 packages/rollup/test/integration/foo_aaa/index.ts diff --git a/internal/linker/index.js b/internal/linker/index.js index 02bbfc82cf..3aa580d984 100644 --- a/internal/linker/index.js +++ b/internal/linker/index.js @@ -30,6 +30,9 @@ if (VERBOSE_LOGS) console.error('[link_node_modules.js]', ...m); } + function log_error(...m) { + console.error('[link_node_modules.js]', ...m); + } function panic(m) { throw new Error(`Internal error! Please run again with --define=VERBOSE_LOG=1 @@ -45,18 +48,41 @@ Include as much of the build output as you can without disclosing anything confi * if they do not exist. */ function mkdirp(p) { - if (!fs.existsSync(p)) { - mkdirp(path.dirname(p)); - fs.mkdirSync(p); - } + return __awaiter(this, void 0, void 0, function* () { + if (p && !(yield exists(p))) { + yield mkdirp(path.dirname(p)); + log_verbose(`mkdir( ${p} )`); + try { + yield fs.promises.mkdir(p); + } + catch (e) { + if (e.code !== 'EEXIST') { + // can happen if path being created exists via a symlink + throw e; + } + } + } + }); } function symlink(target, path) { return __awaiter(this, void 0, void 0, function* () { log_verbose(`symlink( ${path} -> ${target} )`); + // Check if the target exists before creating the symlink. + // This is an extra filesystem access on top of the symlink but + // it is necessary for the time being. + if (!(yield exists(target))) { + // This can happen if a module mapping is propogated from a dependency + // but the targat that generated the mapping in not in the deps. We don't + // want to create symlinks to non-existant targets as this will + // break any nested symlinks that may be created under the module name + // after this. + return false; + } // Use junction on Windows since symlinks require elevated permissions. // We only link to directories so junctions work for us. try { yield fs.promises.symlink(target, path, 'junction'); + return true; } catch (e) { if (e.code !== 'EEXIST') { @@ -65,15 +91,16 @@ Include as much of the build output as you can without disclosing anything confi // We assume here that the path is already linked to the correct target. // Could add some logic that asserts it here, but we want to avoid an extra // filesystem access so we should only do it under some kind of strict mode. - } - if (VERBOSE_LOGS) { - // Be verbose about creating a bad symlink - // Maybe this should fail in production as well, but again we want to avoid - // any unneeded file I/O - if (!fs.existsSync(path)) { - log_verbose('ERROR\n***\nLooks like we created a bad symlink:' + - `\n pwd ${process.cwd()}\n target ${target}\n path ${path}\n***`); + if (VERBOSE_LOGS) { + // Be verbose about creating a bad symlink + // Maybe this should fail in production as well, but again we want to avoid + // any unneeded file I/O + if (!(yield exists(path))) { + log_verbose('ERROR\n***\nLooks like we created a bad symlink:' + + `\n pwd ${process.cwd()}\n target ${target}\n path ${path}\n***`); + } } + return false; } }); } @@ -83,30 +110,32 @@ Include as much of the build output as you can without disclosing anything confi * @param root a string like 'npm/node_modules' */ function resolveRoot(root, runfiles) { - // create a node_modules directory if no root - // this will be the case if only first-party modules are installed - if (!root) { - if (!fs.existsSync('node_modules')) { - log_verbose('no third-party packages; mkdir node_modules in ', process.cwd()); - fs.mkdirSync('node_modules'); + return __awaiter(this, void 0, void 0, function* () { + // create a node_modules directory if no root + // this will be the case if only first-party modules are installed + if (!root) { + if (!(yield exists('node_modules'))) { + log_verbose('no third-party packages; mkdir node_modules in ', process.cwd()); + yield fs.promises.mkdir('node_modules'); + } + return 'node_modules'; } - return 'node_modules'; - } - // If we got a runfilesManifest map, look through it for a resolution - // This will happen if we are running a binary that had some npm packages - // "statically linked" into its runfiles - const fromManifest = runfiles.lookupDirectory(root); - if (fromManifest) - return fromManifest; - // Account for Bazel --legacy_external_runfiles - // which look like 'my_wksp/external/npm/node_modules' - if (fs.existsSync(path.join('external', root))) { - log_verbose('Found legacy_external_runfiles, switching root to', path.join('external', root)); - return path.join('external', root); - } - // 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); + // If we got a runfilesManifest map, look through it for a resolution + // This will happen if we are running a binary that had some npm packages + // "statically linked" into its runfiles + const fromManifest = runfiles.lookupDirectory(root); + if (fromManifest) + return fromManifest; + // Account for Bazel --legacy_external_runfiles + // which look like 'my_wksp/external/npm/node_modules' + if (yield exists(path.join('external', root))) { + log_verbose('found legacy_external_runfiles, switching root to', path.join('external', root)); + return path.join('external', root); + } + // 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); + }); } class Runfiles { constructor(env) { @@ -217,16 +246,71 @@ Include as much of the build output as you can without disclosing anything confi } }); } + function groupAndReduceModules(modules) { + // Group nested modules names as these need to be symlinked in order. + // For example, given a list of module keys such as: + // ['a', '@foo/c/c/c/c', 'b/b', 'b', '@foo/c', '@foo/c/c'] + // this reducer should output the groups list: + // [ [ '@foo/c', '@foo/c/c', '@foo/c/c/c/c' ], [ 'a' ], [ 'b', 'b/b' ] ] + const grouped = Object.keys(modules).sort().reduce((grouped, module, index, array) => { + if (index > 0 && module.startsWith(`${array[index - 1]}/`)) { + grouped[grouped.length - 1].push(module); + } + else { + grouped.push([module]); + } + return grouped; + }, []); + // Reduce links such as `@foo/b/c => /path/to/a/b/c` to their + // lowest common denominator `@foo => /path/to/a` & then remove + // duplicates. + return grouped.map(group => { + return group + .map(name => { + let [kind, modulePath] = modules[name]; + for (;;) { + const bn = path.basename(name); + const bmp = path.basename(modulePath); + if (bn == bmp && bn !== name && bmp !== modulePath) { + // strip off the last segment as it is common + name = path.dirname(name); + modulePath = path.dirname(modulePath); + log_verbose(`module mapping ( ${name}/${bn} => ${modulePath}/${bmp} ) reduced to ( ${name} => ${modulePath} )`); + } + else { + break; + } + } + return { name, root: kind, modulePath }; + }) + .reduce((result, current) => { + if (result.length > 0) { + const last = result[result.length - 1]; + if (current.name === last.name && current.modulePath === last.modulePath) { + // duplicate mapping after reduction + if (current.root !== last.root) { + throw new Error(`conflicting module mappings for '${last.name}' => '${last.modulePath}' of kind '${last.root}' and '${current.root}'`); + } + return result; + } + } + result.push(current); + return result; + }, []); + }); + } + exports.groupAndReduceModules = groupAndReduceModules; function main(args, runfiles) { return __awaiter(this, void 0, void 0, function* () { if (!args || args.length < 1) - throw new Error('link_node_modules.js requires one argument: modulesManifest path'); + throw new Error('requires one argument: modulesManifest path'); const [modulesManifest] = args; let { bin, root, modules, workspace } = JSON.parse(fs.readFileSync(modulesManifest)); modules = modules || {}; log_verbose(`module manifest: workspace ${workspace}, bin ${bin}, root ${root} with first-party packages\n`, modules); - const rootDir = resolveRoot(root, 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('.'); // Convert from runfiles path @@ -250,37 +334,40 @@ Include as much of the build output as you can without disclosing anything confi const workspaceAbs = path.resolve(workspaceDir); // Now add symlinks to each of our first-party packages so they appear under the node_modules tree const links = []; - const linkModule = (name, root, modulePath) => __awaiter(this, void 0, void 0, function* () { - let target = ''; - switch (root) { - case 'bin': - // FIXME(#1196) - target = path.join(workspaceAbs, bin, toWorkspaceDir(modulePath)); - break; - case 'src': - target = path.join(workspaceAbs, toWorkspaceDir(modulePath)); - break; - case 'runfiles': - target = runfiles.resolve(modulePath) || ''; - break; - } - yield symlink(target, name); - }); - for (const m of Object.keys(modules)) { - const segments = m.split('/'); - if (segments.length > 2) { - throw new Error(`module ${m} has more than 2 segments which is not a valid node module name`); - } - if (segments.length == 2) { - // ensure the scope exists - mkdirp(segments[0]); - } - const [kind, modulePath] = modules[m]; - links.push(linkModule(m, kind, modulePath)); + function linkModules(modules) { + return __awaiter(this, void 0, void 0, function* () { + for (const m of modules) { + let target = ''; + switch (m.root) { + case 'bin': + // FIXME(#1196) + target = path.join(workspaceAbs, bin, toWorkspaceDir(m.modulePath)); + break; + case 'src': + target = path.join(workspaceAbs, toWorkspaceDir(m.modulePath)); + break; + case 'runfiles': + target = runfiles.resolve(m.modulePath) || ''; + break; + } + // ensure the subdirectories exist + yield mkdirp(path.dirname(m.name)); + yield symlink(target, m.name); + } + }); + } + const groupedMappings = groupAndReduceModules(modules); + log_verbose(`grouped mappings ${JSON.stringify(groupedMappings)}`); + for (const mappings of groupedMappings) { + // ensure that common directories between groups exists + // to prevent race conditions between parallelized linkModules + yield mkdirp(path.dirname(mappings[0].name)); + // call linkModules for each group + links.push(linkModules(mappings)); } let code = 0; yield Promise.all(links).catch(e => { - console.error(e); + log_error(e); code = 1; }); return code; @@ -290,7 +377,13 @@ Include as much of the build output as you can without disclosing anything confi exports.runfiles = new Runfiles(process.env); if (require.main === module) { (() => __awaiter(this, void 0, void 0, function* () { - process.exitCode = yield main(process.argv.slice(2), exports.runfiles); + try { + process.exitCode = yield main(process.argv.slice(2), exports.runfiles); + } + catch (e) { + log_error(e); + process.exitCode = 1; + } }))(); } }); diff --git a/internal/linker/link_node_modules.ts b/internal/linker/link_node_modules.ts index 7914bd75d4..0f961120aa 100644 --- a/internal/linker/link_node_modules.ts +++ b/internal/linker/link_node_modules.ts @@ -13,6 +13,10 @@ function log_verbose(...m: string[]) { if (VERBOSE_LOGS) console.error('[link_node_modules.js]', ...m); } +function log_error(...m: string[]) { + console.error('[link_node_modules.js]', ...m); +} + function panic(m: string) { throw new Error(`Internal error! Please run again with --define=VERBOSE_LOG=1 @@ -28,19 +32,41 @@ Include as much of the build output as you can without disclosing anything confi * Create a new directory and any necessary subdirectories * if they do not exist. */ -function mkdirp(p: string) { - if (!fs.existsSync(p)) { - mkdirp(path.dirname(p)); - fs.mkdirSync(p); +async function mkdirp(p: string) { + if (p && !await exists(p)) { + await mkdirp(path.dirname(p)); + log_verbose(`mkdir( ${p} )`); + try { + await fs.promises.mkdir(p); + } catch (e) { + if (e.code !== 'EEXIST') { + // can happen if path being created exists via a symlink + throw e; + } + } } } -async function symlink(target: string, path: string) { +async function symlink(target: string, path: string): Promise { log_verbose(`symlink( ${path} -> ${target} )`); + + // Check if the target exists before creating the symlink. + // This is an extra filesystem access on top of the symlink but + // it is necessary for the time being. + if (!await exists(target)) { + // This can happen if a module mapping is propogated from a dependency + // but the targat that generated the mapping in not in the deps. We don't + // want to create symlinks to non-existant targets as this will + // break any nested symlinks that may be created under the module name + // after this. + return false; + } + // Use junction on Windows since symlinks require elevated permissions. // We only link to directories so junctions work for us. try { await fs.promises.symlink(target, path, 'junction'); + return true; } catch (e) { if (e.code !== 'EEXIST') { throw e; @@ -48,17 +74,18 @@ async function symlink(target: string, path: string) { // We assume here that the path is already linked to the correct target. // Could add some logic that asserts it here, but we want to avoid an extra // filesystem access so we should only do it under some kind of strict mode. - } - if (VERBOSE_LOGS) { - // Be verbose about creating a bad symlink - // Maybe this should fail in production as well, but again we want to avoid - // any unneeded file I/O - if (!fs.existsSync(path)) { - log_verbose( - 'ERROR\n***\nLooks like we created a bad symlink:' + - `\n pwd ${process.cwd()}\n target ${target}\n path ${path}\n***`); + if (VERBOSE_LOGS) { + // Be verbose about creating a bad symlink + // Maybe this should fail in production as well, but again we want to avoid + // any unneeded file I/O + if (!await exists(path)) { + log_verbose( + 'ERROR\n***\nLooks like we created a bad symlink:' + + `\n pwd ${process.cwd()}\n target ${target}\n path ${path}\n***`); + } } + return false; } } @@ -67,13 +94,13 @@ async function symlink(target: string, path: string) { * where node_modules was installed * @param root a string like 'npm/node_modules' */ -function resolveRoot(root: string|undefined, runfiles: Runfiles) { +async function resolveRoot(root: string|undefined, runfiles: Runfiles) { // create a node_modules directory if no root // this will be the case if only first-party modules are installed if (!root) { - if (!fs.existsSync('node_modules')) { + if (!await exists('node_modules')) { log_verbose('no third-party packages; mkdir node_modules in ', process.cwd()); - fs.mkdirSync('node_modules'); + await fs.promises.mkdir('node_modules'); } return 'node_modules'; } @@ -86,8 +113,8 @@ function resolveRoot(root: string|undefined, runfiles: Runfiles) { // Account for Bazel --legacy_external_runfiles // which look like 'my_wksp/external/npm/node_modules' - if (fs.existsSync(path.join('external', root))) { - log_verbose('Found legacy_external_runfiles, switching root to', path.join('external', root)); + if (await exists(path.join('external', root))) { + log_verbose('found legacy_external_runfiles, switching root to', path.join('external', root)); return path.join('external', root); } @@ -225,6 +252,63 @@ async function exists(p: string) { } } +export function groupAndReduceModules(modules: {[name: string]: any[]}): + {name: string, root: LinkerRoot, modulePath: string}[][] { + // Group nested modules names as these need to be symlinked in order. + // For example, given a list of module keys such as: + // ['a', '@foo/c/c/c/c', 'b/b', 'b', '@foo/c', '@foo/c/c'] + // this reducer should output the groups list: + // [ [ '@foo/c', '@foo/c/c', '@foo/c/c/c/c' ], [ 'a' ], [ 'b', 'b/b' ] ] + const grouped = Object.keys(modules).sort().reduce( + (grouped: string[][], module: string, index: number, array: string[]): string[][] => { + if (index > 0 && module.startsWith(`${array[index - 1]}/`)) { + grouped[grouped.length - 1].push(module); + } else { + grouped.push([module]); + } + return grouped; + }, []); + + // Reduce links such as `@foo/b/c => /path/to/a/b/c` to their + // lowest common denominator `@foo => /path/to/a` & then remove + // duplicates. + return grouped.map(group => { + return group + .map(name => { + let [kind, modulePath] = modules[name]; + for (;;) { + const bn = path.basename(name); + const bmp = path.basename(modulePath); + if (bn == bmp && bn !== name && bmp !== modulePath) { + // strip off the last segment as it is common + name = path.dirname(name); + modulePath = path.dirname(modulePath); + log_verbose(`module mapping ( ${name}/${bn} => ${modulePath}/${bmp} ) reduced to ( ${ + name} => ${modulePath} )`); + } else { + break; + } + } + return {name, root: kind, modulePath}; + }) + .reduce((result: {name: string, root: LinkerRoot, modulePath: string}[], current) => { + if (result.length > 0) { + const last = result[result.length - 1]; + if (current.name === last.name && current.modulePath === last.modulePath) { + // duplicate mapping after reduction + if (current.root !== last.root) { + throw new Error(`conflicting module mappings for '${last.name}' => '${ + last.modulePath}' of kind '${last.root}' and '${current.root}'`); + } + return result; + } + } + result.push(current); + return result; + }, []); + }); +} + // See link_node_modules.bzl where these three strings // are used to indicate which root the linker should target // for each package: @@ -234,8 +318,7 @@ async function exists(p: string) { type LinkerRoot = 'bin'|'src'|'runfiles'; export async function main(args: string[], runfiles: Runfiles) { - if (!args || args.length < 1) - throw new Error('link_node_modules.js requires one argument: modulesManifest path'); + if (!args || args.length < 1) throw new Error('requires one argument: modulesManifest path'); const [modulesManifest] = args; let {bin, root, modules, workspace} = JSON.parse(fs.readFileSync(modulesManifest)); @@ -245,8 +328,9 @@ export async function main(args: string[], runfiles: Runfiles) { root} with first-party packages\n`, modules); - const rootDir = resolveRoot(root, 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('.'); @@ -276,41 +360,42 @@ export async function main(args: string[], runfiles: Runfiles) { // Now add symlinks to each of our first-party packages so they appear under the node_modules tree const links = []; - const linkModule = - async (name: string, root: LinkerRoot, modulePath: string) => { - let target: string = ''; - switch (root) { - case 'bin': - // FIXME(#1196) - target = path.join(workspaceAbs, bin, toWorkspaceDir(modulePath)); - break; - case 'src': - target = path.join(workspaceAbs, toWorkspaceDir(modulePath)); - break; - case 'runfiles': - target = runfiles.resolve(modulePath) || ''; - break; - } + async function linkModules(modules: {name: string, root: LinkerRoot, modulePath: string}[]) { + for (const m of modules) { + let target: string = ''; + switch (m.root) { + case 'bin': + // FIXME(#1196) + target = path.join(workspaceAbs, bin, toWorkspaceDir(m.modulePath)); + break; + case 'src': + target = path.join(workspaceAbs, toWorkspaceDir(m.modulePath)); + break; + case 'runfiles': + target = runfiles.resolve(m.modulePath) || ''; + break; + } - await symlink(target, name); + // ensure the subdirectories exist + await mkdirp(path.dirname(m.name)); + await symlink(target, m.name); + } } - for (const m of Object.keys(modules)) { - const segments = m.split('/'); - if (segments.length > 2) { - throw new Error(`module ${m} has more than 2 segments which is not a valid node module name`); - } - if (segments.length == 2) { - // ensure the scope exists - mkdirp(segments[0]); - } - const [kind, modulePath] = modules[m]; - links.push(linkModule(m, kind, modulePath)); + const groupedMappings = groupAndReduceModules(modules); + log_verbose(`grouped mappings ${JSON.stringify(groupedMappings)}`); + for (const mappings of groupedMappings) { + // ensure that common directories between groups exists + // to prevent race conditions between parallelized linkModules + await mkdirp(path.dirname(mappings[0].name)); + + // call linkModules for each group + links.push(linkModules(mappings)); } let code = 0; await Promise.all(links).catch(e => { - console.error(e); + log_error(e); code = 1; }); @@ -321,6 +406,11 @@ export const runfiles = new Runfiles(process.env); if (require.main === module) { (async () => { - process.exitCode = await main(process.argv.slice(2), runfiles); + try { + process.exitCode = await main(process.argv.slice(2), runfiles); + } catch (e) { + log_error(e); + process.exitCode = 1; + } })(); } diff --git a/internal/linker/test/link_node_modules.spec.ts b/internal/linker/test/link_node_modules.spec.ts index 849eb0ad4f..7fce3aefe1 100644 --- a/internal/linker/test/link_node_modules.spec.ts +++ b/internal/linker/test/link_node_modules.spec.ts @@ -32,6 +32,48 @@ describe('link_node_modules', () => { mkdirp(workspace); }); + it('should group and reduce modules appropriate', () => { + const modules = { + 'a': ['src', `${workspace}/path/to/lib_a`], + '@foo/c/c/c/c': ['bin', `${workspace}/path/to/foo_cccc`], + 'b/b': ['src', 'other_wksp/path/to/lib_bb'], + 'b': ['src', 'other_wksp/path/to/lib_b'], + '@foo/c': ['bin', `${workspace}/path/to/foo_c`], + '@foo/c/c': ['bin', `${workspace}/path/to/foo_cc`], + '@foo/d/bar/fum/far': ['bin', `${workspace}/path/to/foo_d/bar/fum/far`], + '@foo/d/bar': ['bin', `${workspace}/path/to/foo_d/bar`], + // don't include `@foo/d` as the group & reduce function should derive that + // from the lowest common denominator of the module name & module path + }; + + const result = linker.groupAndReduceModules(modules); + + const expected = [ + [ + // `@foo/c`, `@foo/c/c` and `@foo/c/c/c/c` should + // be grouped but not reduced as their arrangement + // on disk does not match the modules names + {'name': '@foo/c', 'root': 'bin', 'modulePath': `${workspace}/path/to/foo_c`}, + {'name': '@foo/c/c', 'root': 'bin', 'modulePath': `${workspace}/path/to/foo_cc`}, + {'name': '@foo/c/c/c/c', 'root': 'bin', 'modulePath': `${workspace}/path/to/foo_cccc`} + ], + [ + // `@foo/d/bar/fum/far` and `@foo/d/bar` should + // be grouped and reduced to `@foo/d` + {'name': '@foo/d', 'root': 'bin', 'modulePath': `${workspace}/path/to/foo_d`}, + ], + [{'name': 'a', 'root': 'src', 'modulePath': `${workspace}/path/to/lib_a`}], + [ + // `b`, `b/b` should be grouped but not reduced as + // their arrangement on disk does not match the modules names + {'name': 'b', 'root': 'src', 'modulePath': 'other_wksp/path/to/lib_b'}, + {'name': 'b/b', 'root': 'src', 'modulePath': 'other_wksp/path/to/lib_bb'} + ] + ]; + + expect(JSON.stringify(result)).toEqual(JSON.stringify(expected)); + }) + it('should report when modules manifest absent', async () => { try { await (linker as any).main(); @@ -56,22 +98,49 @@ describe('link_node_modules', () => { // Create a package in the user workspace mkdirp('path/to/lib_a'); - fs.writeFileSync('path/to/lib_a/index.js', 'exports = {}', 'utf-8'); + fs.writeFileSync('path/to/lib_a/index.js', '/*a*/exports = {}', 'utf-8'); - // Create a package in a different workspace + // Create a nested package in a different workspace mkdirp('external/other_wksp/path/to/lib_b'); - fs.writeFileSync('external/other_wksp/path/to/lib_b/index.js', 'exports = {}', 'utf-8'); - - // Create a package in bazel-bin - mkdirp(`${BIN_DIR}/path/to/lib_c`); - fs.writeFileSync(`${BIN_DIR}/path/to/lib_c/index.js`, 'exports = {}', 'utf-8'); + fs.writeFileSync('external/other_wksp/path/to/lib_b/index.js', '/*b*/exports = {}', 'utf-8'); + mkdirp('external/other_wksp/path/to/lib_bb'); + fs.writeFileSync('external/other_wksp/path/to/lib_bb/index.js', '/*b/b*/exports = {}', 'utf-8'); + + // Create a nested package in bazel-bin where module names don't match directory structure + mkdirp(`${BIN_DIR}/path/to/foo_c`); + fs.writeFileSync(`${BIN_DIR}/path/to/foo_c/index.js`, '/*@foo/c*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/foo_cc`); + fs.writeFileSync(`${BIN_DIR}/path/to/foo_cc/index.js`, '/*@foo/c/c*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/foo_cccc`); + fs.writeFileSync( + `${BIN_DIR}/path/to/foo_cccc/index.js`, '/*@foo/c/c/c/c*/exports = {}', 'utf-8'); + + // Create a nested package in bazel-bin where module names match directory structure + mkdirp(`${BIN_DIR}/path/to/foo_d`); + fs.writeFileSync(`${BIN_DIR}/path/to/foo_d/index.js`, '/*@foo/d*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/foo_d/bar`); + fs.writeFileSync( + `${BIN_DIR}/path/to/foo_d/bar/index.js`, '/*@foo/d/bar*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/foo_d/bar/fum/far`); + fs.writeFileSync( + `${BIN_DIR}/path/to/foo_d/bar/fum/far/index.js`, '/*@foo/d/bar/fum/far*/exports = {}', + 'utf-8'); writeManifest({ 'bin': BIN_DIR, + // intentionally out of order so that linker has to sort + // and create nested modules in the correct order 'modules': { 'a': ['src', `${workspace}/path/to/lib_a`], + '@foo/c/c/c/c': ['bin', `${workspace}/path/to/foo_cccc`], + 'b/b': ['src', 'other_wksp/path/to/lib_bb'], 'b': ['src', 'other_wksp/path/to/lib_b'], - 'c': ['bin', `${workspace}/path/to/lib_c`], + '@foo/c': ['bin', `${workspace}/path/to/foo_c`], + '@foo/c/c': ['bin', `${workspace}/path/to/foo_cc`], + '@foo/d/bar/fum/far': ['bin', `${workspace}/path/to/foo_d/bar/fum/far`], + '@foo/d/bar': ['bin', `${workspace}/path/to/foo_d/bar`], + // don't include `@foo/d` as the linker should derive that symlink + // from the lowest common denominator of the module name & module path }, 'workspace': workspace, }); @@ -81,12 +150,53 @@ describe('link_node_modules', () => { // The linker expects to run as its own process, so it changes the wd process.chdir(path.join()); - expect(fs.readdirSync(path.join(process.env['TEST_TMPDIR']!, workspace, 'node_modules', 'a'))) - .toContain('index.js'); - expect(fs.readdirSync(path.join(process.env['TEST_TMPDIR']!, workspace, 'node_modules', 'b'))) - .toContain('index.js'); - expect(fs.readdirSync(path.join(process.env['TEST_TMPDIR']!, workspace, 'node_modules', 'c'))) - .toContain('index.js'); + expect(fs.readFileSync( + path.join(process.env['TEST_TMPDIR']!, workspace, 'node_modules', 'a', 'index.js'), + 'utf-8')) + .toEqual('/*a*/exports = {}'); + expect(fs.readFileSync( + path.join(process.env['TEST_TMPDIR']!, workspace, 'node_modules', 'b', 'index.js'), + 'utf-8')) + .toEqual('/*b*/exports = {}'); + expect( + fs.readFileSync( + path.join(process.env['TEST_TMPDIR']!, workspace, 'node_modules', 'b', 'b', 'index.js'), + 'utf-8')) + .toEqual('/*b/b*/exports = {}'); + expect(fs.readFileSync( + path.join( + process.env['TEST_TMPDIR']!, workspace, 'node_modules', '@foo', 'c', 'index.js'), + 'utf-8')) + .toEqual('/*@foo/c*/exports = {}'); + expect(fs.readFileSync( + path.join( + process.env['TEST_TMPDIR']!, workspace, 'node_modules', '@foo', 'c', 'c', + 'index.js'), + 'utf-8')) + .toEqual('/*@foo/c/c*/exports = {}'); + expect(fs.readFileSync( + path.join( + process.env['TEST_TMPDIR']!, workspace, 'node_modules', '@foo', 'c', 'c', 'c', + 'c', 'index.js'), + 'utf-8')) + .toEqual('/*@foo/c/c/c/c*/exports = {}'); + expect(fs.readFileSync( + path.join( + process.env['TEST_TMPDIR']!, workspace, 'node_modules', '@foo', 'd', 'index.js'), + 'utf-8')) + .toEqual('/*@foo/d*/exports = {}'); + expect(fs.readFileSync( + path.join( + process.env['TEST_TMPDIR']!, workspace, 'node_modules', '@foo', 'd', 'bar', + 'index.js'), + 'utf-8')) + .toEqual('/*@foo/d/bar*/exports = {}'); + expect(fs.readFileSync( + path.join( + process.env['TEST_TMPDIR']!, workspace, 'node_modules', '@foo', 'd', 'bar', + 'fum', 'far', 'index.js'), + 'utf-8')) + .toEqual('/*@foo/d/bar/fum/far*/exports = {}'); }); it('should handle third-party packages in runfiles', async () => { diff --git a/packages/rollup/test/integration/BUILD.bazel b/packages/rollup/test/integration/BUILD.bazel index e417b4b5b3..2e2e8ba904 100644 --- a/packages/rollup/test/integration/BUILD.bazel +++ b/packages/rollup/test/integration/BUILD.bazel @@ -25,7 +25,11 @@ _BUNDLE_FORMATS = [ sourcemap = "true", deps = [ "//%s/fum:fumlib" % package_name(), - "//%s/foo:foolib" % package_name(), + "//%s/foo:foo_lib" % package_name(), + "//%s/foo_a:foo_lib_a" % package_name(), + "//%s/foo_aaa:foo_lib_a_a_a" % package_name(), + "//%s/far/a/b/c" % package_name(), + "//%s/far/a" % package_name(), "@npm//hello", "@npm//rollup-plugin-commonjs", "@npm//rollup-plugin-json", diff --git a/packages/rollup/test/integration/far/a/BUILD.bazel b/packages/rollup/test/integration/far/a/BUILD.bazel new file mode 100644 index 0000000000..0a163b9d97 --- /dev/null +++ b/packages/rollup/test/integration/far/a/BUILD.bazel @@ -0,0 +1,9 @@ +load("@npm_bazel_typescript//:index.from_src.bzl", "ts_library") + +package(default_visibility = ["//packages/rollup:__subpackages__"]) + +ts_library( + name = "a", + srcs = ["index.ts"], + module_name = "@far/a", +) diff --git a/packages/rollup/test/integration/far/a/b/c/BUILD.bazel b/packages/rollup/test/integration/far/a/b/c/BUILD.bazel new file mode 100644 index 0000000000..653d5e5c43 --- /dev/null +++ b/packages/rollup/test/integration/far/a/b/c/BUILD.bazel @@ -0,0 +1,9 @@ +load("@npm_bazel_typescript//:index.from_src.bzl", "ts_library") + +package(default_visibility = ["//packages/rollup:__subpackages__"]) + +ts_library( + name = "c", + srcs = ["index.ts"], + module_name = "@far/a/b/c", +) diff --git a/packages/rollup/test/integration/far/a/b/c/index.ts b/packages/rollup/test/integration/far/a/b/c/index.ts new file mode 100644 index 0000000000..9075058eeb --- /dev/null +++ b/packages/rollup/test/integration/far/a/b/c/index.ts @@ -0,0 +1 @@ +export default `@far/a/b/c`; diff --git a/packages/rollup/test/integration/far/a/index.ts b/packages/rollup/test/integration/far/a/index.ts new file mode 100644 index 0000000000..4ff5142dfd --- /dev/null +++ b/packages/rollup/test/integration/far/a/index.ts @@ -0,0 +1 @@ +export default `@far/a`; diff --git a/packages/rollup/test/integration/foo.js b/packages/rollup/test/integration/foo.js index 6dee3b2104..81083b2de2 100644 --- a/packages/rollup/test/integration/foo.js +++ b/packages/rollup/test/integration/foo.js @@ -1,4 +1,8 @@ -import {foo} from 'foolib'; +import far_a from '@far/a'; +import far_a_b_c from '@far/a/b/c'; +import {foo} from '@foo/lib'; +import {foo as foo_a} from '@foo/lib/a'; +import {foo as foo_a_a_a} from '@foo/lib/a/a/a'; import {fum} from 'fumlib'; import hello from 'hello'; import {thing} from 'some_global_var'; @@ -6,7 +10,8 @@ import {thing} from 'some_global_var'; import {name} from './bar'; import {json_key} from './some.json'; -console.log(`${hello}, ${name} in ${fum} ${foo} ${json_key}`); +console.log( + `${hello}, ${name} in ${fum} ${foo} ${foo_a} ${foo_a_a_a} ${far_a} ${far_a_b_c} ${json_key}`); // Tests for @PURE annotations /*@__PURE__*/ diff --git a/packages/rollup/test/integration/foo/BUILD.bazel b/packages/rollup/test/integration/foo/BUILD.bazel index 0f61277b50..4fb9dbee90 100644 --- a/packages/rollup/test/integration/foo/BUILD.bazel +++ b/packages/rollup/test/integration/foo/BUILD.bazel @@ -14,14 +14,11 @@ genrule( ) ts_library( - name = "foolib", + name = "foo_lib", srcs = [ "index.ts", "user.d.ts", ], - module_name = "foolib", - # Don't allow deep imports under here, - # and give it the AMD name "foolib", not "foolib/index" - module_root = "index.d.ts", + module_name = "@foo/lib", deps = ["@npm//date-fns"], ) diff --git a/packages/rollup/test/integration/foo_a/BUILD.bazel b/packages/rollup/test/integration/foo_a/BUILD.bazel new file mode 100644 index 0000000000..1fa184ce51 --- /dev/null +++ b/packages/rollup/test/integration/foo_a/BUILD.bazel @@ -0,0 +1,10 @@ +load("@npm_bazel_typescript//:index.from_src.bzl", "ts_library") + +package(default_visibility = ["//packages/rollup:__subpackages__"]) + +ts_library( + name = "foo_lib_a", + srcs = ["index.ts"], + module_name = "@foo/lib/a", + deps = ["@npm//date-fns"], +) diff --git a/packages/rollup/test/integration/foo_a/index.ts b/packages/rollup/test/integration/foo_a/index.ts new file mode 100644 index 0000000000..fffb2223b5 --- /dev/null +++ b/packages/rollup/test/integration/foo_a/index.ts @@ -0,0 +1,4 @@ +import {format} from 'date-fns'; + +const date: string = format(new Date(2019, 4, 7), 'MMMM D, YYYY'); +export const foo = `@foo/lib/a ${date}`; diff --git a/packages/rollup/test/integration/foo_aaa/BUILD.bazel b/packages/rollup/test/integration/foo_aaa/BUILD.bazel new file mode 100644 index 0000000000..9781517497 --- /dev/null +++ b/packages/rollup/test/integration/foo_aaa/BUILD.bazel @@ -0,0 +1,10 @@ +load("@npm_bazel_typescript//:index.from_src.bzl", "ts_library") + +package(default_visibility = ["//packages/rollup:__subpackages__"]) + +ts_library( + name = "foo_lib_a_a_a", + srcs = ["index.ts"], + module_name = "@foo/lib/a/a/a", + deps = ["@npm//date-fns"], +) diff --git a/packages/rollup/test/integration/foo_aaa/index.ts b/packages/rollup/test/integration/foo_aaa/index.ts new file mode 100644 index 0000000000..42bb64970f --- /dev/null +++ b/packages/rollup/test/integration/foo_aaa/index.ts @@ -0,0 +1,4 @@ +import {format} from 'date-fns'; + +const date: string = format(new Date(2019, 4, 7), 'MMMM D, YYYY'); +export const foo = `@foo/lib/a/a/a ${date}`; diff --git a/packages/rollup/test/integration/fum/BUILD.bazel b/packages/rollup/test/integration/fum/BUILD.bazel index 937081ace4..1b1a3e4b3d 100644 --- a/packages/rollup/test/integration/fum/BUILD.bazel +++ b/packages/rollup/test/integration/fum/BUILD.bazel @@ -7,7 +7,4 @@ js_library( srcs = ["index.js"], module_from_src = True, module_name = "fumlib", - # Don't allow deep imports under here, - # and give it the AMD name "fumlib", not "fumlib/index" - module_root = "index.d.ts", ) diff --git a/packages/rollup/test/integration/golden.amd.js_ b/packages/rollup/test/integration/golden.amd.js_ index e0e9deb672..371bac271a 100644 --- a/packages/rollup/test/integration/golden.amd.js_ +++ b/packages/rollup/test/integration/golden.amd.js_ @@ -5,6 +5,10 @@ define(['exports', 'some_global_var'], function (exports, some_global_var) { 'use strict'; + var far_a = `@far/a`; + + var far_a_b_c = `@far/a/b/c`; + var MILLISECONDS_IN_MINUTE = 60000; /** @@ -5643,6 +5647,12 @@ define(['exports', 'some_global_var'], function (exports, some_global_var) { 'us const date = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); const foo = `Sunnyvale ${user} ${date}`; + const date$1 = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); + const foo$1 = `@foo/lib/a ${date$1}`; + + const date$2 = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); + const foo$2 = `@foo/lib/a/a/a ${date$2}`; + const fum = 'Wonderland'; var hello = 'Hello'; @@ -5651,7 +5661,8 @@ define(['exports', 'some_global_var'], function (exports, some_global_var) { 'us const json_key = "json_value"; - console.log(`${hello}, ${name} in ${fum} ${foo} ${json_key}`); + console.log( + `${hello}, ${name} in ${fum} ${foo} ${foo$1} ${foo$2} ${far_a} ${far_a_b_c} ${json_key}`); // Test for sequences = false class A { diff --git a/packages/rollup/test/integration/golden.cjs.js_ b/packages/rollup/test/integration/golden.cjs.js_ index 372927067e..174f2b49b8 100644 --- a/packages/rollup/test/integration/golden.cjs.js_ +++ b/packages/rollup/test/integration/golden.cjs.js_ @@ -9,6 +9,10 @@ Object.defineProperty(exports, '__esModule', { value: true }); var some_global_var = require('some_global_var'); +var far_a = `@far/a`; + +var far_a_b_c = `@far/a/b/c`; + var MILLISECONDS_IN_MINUTE = 60000; /** @@ -5647,6 +5651,12 @@ const user = "user"; const date = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); const foo = `Sunnyvale ${user} ${date}`; +const date$1 = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); +const foo$1 = `@foo/lib/a ${date$1}`; + +const date$2 = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); +const foo$2 = `@foo/lib/a/a/a ${date$2}`; + const fum = 'Wonderland'; var hello = 'Hello'; @@ -5655,7 +5665,8 @@ const name = 'Alice'; const json_key = "json_value"; -console.log(`${hello}, ${name} in ${fum} ${foo} ${json_key}`); +console.log( + `${hello}, ${name} in ${fum} ${foo} ${foo$1} ${foo$2} ${far_a} ${far_a_b_c} ${json_key}`); // Test for sequences = false class A { diff --git a/packages/rollup/test/integration/golden.esm.js_ b/packages/rollup/test/integration/golden.esm.js_ index 14a02cecb1..f4c737126c 100644 --- a/packages/rollup/test/integration/golden.esm.js_ +++ b/packages/rollup/test/integration/golden.esm.js_ @@ -5,6 +5,10 @@ import { thing } from 'some_global_var'; +var far_a = `@far/a`; + +var far_a_b_c = `@far/a/b/c`; + var MILLISECONDS_IN_MINUTE = 60000; /** @@ -5643,6 +5647,12 @@ const user = "user"; const date = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); const foo = `Sunnyvale ${user} ${date}`; +const date$1 = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); +const foo$1 = `@foo/lib/a ${date$1}`; + +const date$2 = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); +const foo$2 = `@foo/lib/a/a/a ${date$2}`; + const fum = 'Wonderland'; var hello = 'Hello'; @@ -5651,7 +5661,8 @@ const name = 'Alice'; const json_key = "json_value"; -console.log(`${hello}, ${name} in ${fum} ${foo} ${json_key}`); +console.log( + `${hello}, ${name} in ${fum} ${foo} ${foo$1} ${foo$2} ${far_a} ${far_a_b_c} ${json_key}`); // Test for sequences = false class A { diff --git a/packages/rollup/test/integration/golden.iife.js_ b/packages/rollup/test/integration/golden.iife.js_ index 8d65be5ad5..5b0c526e55 100644 --- a/packages/rollup/test/integration/golden.iife.js_ +++ b/packages/rollup/test/integration/golden.iife.js_ @@ -6,6 +6,10 @@ var bundle = (function (exports, some_global_var) { 'use strict'; + var far_a = `@far/a`; + + var far_a_b_c = `@far/a/b/c`; + var MILLISECONDS_IN_MINUTE = 60000; /** @@ -5644,6 +5648,12 @@ var bundle = (function (exports, some_global_var) { const date = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); const foo = `Sunnyvale ${user} ${date}`; + const date$1 = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); + const foo$1 = `@foo/lib/a ${date$1}`; + + const date$2 = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); + const foo$2 = `@foo/lib/a/a/a ${date$2}`; + const fum = 'Wonderland'; var hello = 'Hello'; @@ -5652,7 +5662,8 @@ var bundle = (function (exports, some_global_var) { const json_key = "json_value"; - console.log(`${hello}, ${name} in ${fum} ${foo} ${json_key}`); + console.log( + `${hello}, ${name} in ${fum} ${foo} ${foo$1} ${foo$2} ${far_a} ${far_a_b_c} ${json_key}`); // Test for sequences = false class A { diff --git a/packages/rollup/test/integration/golden.system.js_ b/packages/rollup/test/integration/golden.system.js_ index 154cfeca1e..9bc8618467 100644 --- a/packages/rollup/test/integration/golden.system.js_ +++ b/packages/rollup/test/integration/golden.system.js_ @@ -12,6 +12,10 @@ System.register('bundle', ['some_global_var'], function (exports, module) { }], execute: function () { + var far_a = `@far/a`; + + var far_a_b_c = `@far/a/b/c`; + var MILLISECONDS_IN_MINUTE = 60000; /** @@ -5650,6 +5654,12 @@ System.register('bundle', ['some_global_var'], function (exports, module) { const date = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); const foo = `Sunnyvale ${user} ${date}`; + const date$1 = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); + const foo$1 = `@foo/lib/a ${date$1}`; + + const date$2 = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); + const foo$2 = `@foo/lib/a/a/a ${date$2}`; + const fum = 'Wonderland'; var hello = 'Hello'; @@ -5658,7 +5668,8 @@ System.register('bundle', ['some_global_var'], function (exports, module) { const json_key = "json_value"; - console.log(`${hello}, ${name} in ${fum} ${foo} ${json_key}`); + console.log( + `${hello}, ${name} in ${fum} ${foo} ${foo$1} ${foo$2} ${far_a} ${far_a_b_c} ${json_key}`); // Test for sequences = false class A { diff --git a/packages/rollup/test/integration/golden.umd.js_ b/packages/rollup/test/integration/golden.umd.js_ index d0ff90f180..95437eca74 100644 --- a/packages/rollup/test/integration/golden.umd.js_ +++ b/packages/rollup/test/integration/golden.umd.js_ @@ -9,6 +9,10 @@ (global = global || self, factory(global.bundle = {}, global.runtime_name_of_global_var)); }(this, function (exports, some_global_var) { 'use strict'; + var far_a = `@far/a`; + + var far_a_b_c = `@far/a/b/c`; + var MILLISECONDS_IN_MINUTE = 60000; /** @@ -5647,6 +5651,12 @@ const date = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); const foo = `Sunnyvale ${user} ${date}`; + const date$1 = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); + const foo$1 = `@foo/lib/a ${date$1}`; + + const date$2 = dateFns_50(new Date(2019, 4, 7), 'MMMM D, YYYY'); + const foo$2 = `@foo/lib/a/a/a ${date$2}`; + const fum = 'Wonderland'; var hello = 'Hello'; @@ -5655,7 +5665,8 @@ const json_key = "json_value"; - console.log(`${hello}, ${name} in ${fum} ${foo} ${json_key}`); + console.log( + `${hello}, ${name} in ${fum} ${foo} ${foo$1} ${foo$2} ${far_a} ${far_a_b_c} ${json_key}`); // Test for sequences = false class A { diff --git a/packages/rollup/test/integration/golden.umd.sha256_ b/packages/rollup/test/integration/golden.umd.sha256_ index 2fc68acd53..87c83156eb 100755 --- a/packages/rollup/test/integration/golden.umd.sha256_ +++ b/packages/rollup/test/integration/golden.umd.sha256_ @@ -1 +1 @@ -a5354270e4b26942df1c0f59d3a80c472b6a29206e85e8edac6f3244bf3f3d22 \ No newline at end of file +adc91355f275b6013a06b9f3c7b76f5fce606653396af3540794886799e0b78b \ No newline at end of file