From b7e550762d0f2e833e5d7f175b3b88aa32636e3a Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Fri, 6 Dec 2019 16:33:35 -0800 Subject: [PATCH] fix(builtin): link module_name to directories recursively to avoid directory clashes Fixes #1411 --- internal/linker/index.js | 227 +++++--- internal/linker/link_node_modules.ts | 277 +++++++--- .../linker/test/link_node_modules.spec.ts | 513 +++++++++++++++--- 3 files changed, 778 insertions(+), 239 deletions(-) diff --git a/internal/linker/index.js b/internal/linker/index.js index 3aa580d984..4979466fbe 100644 --- a/internal/linker/index.js +++ b/internal/linker/index.js @@ -64,9 +64,9 @@ Include as much of the build output as you can without disclosing anything confi } }); } - function symlink(target, path) { + function symlink(target, p) { return __awaiter(this, void 0, void 0, function* () { - log_verbose(`symlink( ${path} -> ${target} )`); + log_verbose(`symlink( ${p} -> ${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. @@ -81,7 +81,7 @@ Include as much of the build output as you can without disclosing anything confi // 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'); + yield fs.promises.symlink(target, p, 'junction'); return true; } catch (e) { @@ -95,9 +95,9 @@ Include as much of the build output as you can without disclosing anything confi // 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))) { + if (!(yield exists(p))) { log_verbose('ERROR\n***\nLooks like we created a bad symlink:' + - `\n pwd ${process.cwd()}\n target ${target}\n path ${path}\n***`); + `\n pwd ${process.cwd()}\n target ${target}\n path ${p}\n***`); } } return false; @@ -246,60 +246,139 @@ 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); + /** + * Given a set of module aliases returns an array of recursive `LinkerTreeElement`s. + * + * The tree nodes represent the FS links required. Each node of the tree hierarchy + * depends on its parent node having been setup first. Each sibling node can be processed + * concurrently. + * + * The number of symlinks is minimized in situations such as: + * + * Shared parent path to lowest common denominator: + * `@foo/b/c => /path/to/a/b/c` + * + * can be represented as + * + * `@foo => /path/to/a` + * + * Shared parent directory: + * `@foo/p/a => /path/to/x/a` + * `@foo/p/c => /path/to/x/a` + * + * can be represented as a single parent + * + * `@foo/p => /path/to/x` + */ + function reduceModules(modules) { + return buildModuleHierarchy(Object.keys(modules).sort(), modules, '/').children || []; + } + exports.reduceModules = reduceModules; + function buildModuleHierarchy(moduleNames, modules, elementPath) { + let element = { + name: elementPath.slice(0, -1), + link: modules[elementPath.slice(0, -1)], + children: [], + }; + for (let i = 0; i < moduleNames.length;) { + const moduleName = moduleNames[i]; + const next = moduleName.indexOf('/', elementPath.length + 1); + const moduleGroup = (next === -1) ? (moduleName + '/') : moduleName.slice(0, next + 1); + // If the first was an exact match (direct child of element) then it is the element parent, skip + // it + if (next === -1) { + i++; } - else { - grouped.push([module]); + const siblings = []; + while (i < moduleNames.length && moduleNames[i].startsWith(moduleGroup)) { + siblings.push(moduleNames[i++]); } - 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; - }, []); - }); + let childElement = buildModuleHierarchy(siblings, modules, moduleGroup); + for (let cur = childElement; (cur = liftElement(childElement)) !== childElement;) { + childElement = cur; + } + element.children.push(childElement); + } + // Cleanup empty children+link + if (!element.link) { + delete element.link; + } + if (!element.children || element.children.length === 0) { + delete element.children; + } + return element; + } + function liftElement(element) { + let { name, link, children } = element; + if (!children || !children.length) { + return element; + } + // A link and all the child links align under => this link alone represents that + if (link && allElementsAlignUnder(name, link, children)) { + return { name, link }; + } + // No link but all children align => the link can be lifted to here + if (!link && allElementsAlign(name, children)) { + return { + name, + link: toParentLink(children[0].link), + }; + } + // Only a single child and this element is just a directory (no link) => only need the child link + // Do this last only after trying to lift child links up + if (children.length === 1 && !link) { + return children[0]; + } + return element; + } + function toParentLink(link) { + return [link[0], path.dirname(link[1])]; + } + function allElementsAlign(name, elements) { + if (!elements[0].link) { + return false; + } + const parentLink = toParentLink(elements[0].link); + // Every child needs a link with aligning parents + if (!elements.every(e => !!e.link && isDirectChildLink(parentLink, e.link))) { + return false; + } + return !!elements[0].link && allElementsAlignUnder(name, parentLink, elements); + } + function allElementsAlignUnder(parentName, parentLink, elements) { + for (const { name, link, children } of elements) { + if (!link || children) { + return false; + } + if (!isDirectChildPath(parentName, name)) { + return false; + } + if (!isDirectChildLink(parentLink, link)) { + return false; + } + if (!isNameLinkPathTopAligned(name, link)) { + return false; + } + } + return true; + } + function isDirectChildPath(parent, child) { + return parent === path.dirname(child); + } + function isDirectChildLink([parentRel, parentPath], [childRel, childPath]) { + // Same link-relation type + if (parentRel !== childRel) { + return false; + } + // Child path is a directly-child of the parent path + if (!isDirectChildPath(parentPath, childPath)) { + return false; + } + return true; + } + function isNameLinkPathTopAligned(namePath, [, linkPath]) { + return path.basename(namePath) === path.basename(linkPath); } - exports.groupAndReduceModules = groupAndReduceModules; function main(args, runfiles) { return __awaiter(this, void 0, void 0, function* () { if (!args || args.length < 1) @@ -332,39 +411,37 @@ Include as much of the build output as you can without disclosing anything confi process.chdir(rootDir); // Symlinks to packages need to reach back to the workspace/runfiles directory 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 = []; - function linkModules(modules) { + function linkModules(m) { return __awaiter(this, void 0, void 0, function* () { - for (const m of modules) { + // ensure the parent directory exist + yield mkdirp(path.dirname(m.name)); + if (m.link) { + const [root, modulePath] = m.link; let target = ''; - switch (m.root) { + switch (root) { case 'bin': // FIXME(#1196) - target = path.join(workspaceAbs, bin, toWorkspaceDir(m.modulePath)); + target = path.join(workspaceAbs, bin, toWorkspaceDir(modulePath)); break; case 'src': - target = path.join(workspaceAbs, toWorkspaceDir(m.modulePath)); + target = path.join(workspaceAbs, toWorkspaceDir(modulePath)); break; case 'runfiles': - target = runfiles.resolve(m.modulePath) || ''; + target = runfiles.resolve(modulePath) || ''; break; } - // ensure the subdirectories exist - yield mkdirp(path.dirname(m.name)); yield symlink(target, m.name); } + // Process each child branch concurrently + if (m.children) { + yield Promise.all(m.children.map(linkModules)); + } }); } - 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)); - } + const moduleHeirarchy = reduceModules(modules); + log_verbose(`mapping hierarchy ${JSON.stringify(moduleHeirarchy)}`); + // Process each root branch concurrently + const links = moduleHeirarchy.map(linkModules); let code = 0; yield Promise.all(links).catch(e => { log_error(e); diff --git a/internal/linker/link_node_modules.ts b/internal/linker/link_node_modules.ts index 0f961120aa..937b493c3a 100644 --- a/internal/linker/link_node_modules.ts +++ b/internal/linker/link_node_modules.ts @@ -47,8 +47,8 @@ async function mkdirp(p: string) { } } -async function symlink(target: string, path: string): Promise { - log_verbose(`symlink( ${path} -> ${target} )`); +async function symlink(target: string, p: string): Promise { + log_verbose(`symlink( ${p} -> ${target} )`); // Check if the target exists before creating the symlink. // This is an extra filesystem access on top of the symlink but @@ -65,7 +65,7 @@ async function symlink(target: string, path: string): Promise { // 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'); + await fs.promises.symlink(target, p, 'junction'); return true; } catch (e) { if (e.code !== 'EEXIST') { @@ -79,10 +79,10 @@ async function symlink(target: string, path: string): Promise { // 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)) { + if (!await exists(p)) { log_verbose( 'ERROR\n***\nLooks like we created a bad symlink:' + - `\n pwd ${process.cwd()}\n target ${target}\n path ${path}\n***`); + `\n pwd ${process.cwd()}\n target ${target}\n path ${p}\n***`); } } return false; @@ -252,61 +252,169 @@ 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; - }, []); - }); +/** + * Given a set of module aliases returns an array of recursive `LinkerTreeElement`s. + * + * The tree nodes represent the FS links required. Each node of the tree hierarchy + * depends on its parent node having been setup first. Each sibling node can be processed + * concurrently. + * + * The number of symlinks is minimized in situations such as: + * + * Shared parent path to lowest common denominator: + * `@foo/b/c => /path/to/a/b/c` + * + * can be represented as + * + * `@foo => /path/to/a` + * + * Shared parent directory: + * `@foo/p/a => /path/to/x/a` + * `@foo/p/c => /path/to/x/a` + * + * can be represented as a single parent + * + * `@foo/p => /path/to/x` + */ +export function reduceModules(modules: LinkerAliases): LinkerTreeElement[] { + return buildModuleHierarchy(Object.keys(modules).sort(), modules, '/').children || []; +} + +function buildModuleHierarchy( + moduleNames: string[], modules: LinkerAliases, elementPath: string): LinkerTreeElement { + let element: LinkerTreeElement = { + name: elementPath.slice(0, -1), + link: modules[elementPath.slice(0, -1)], + children: [], + }; + + for (let i = 0; i < moduleNames.length;) { + const moduleName = moduleNames[i]; + const next = moduleName.indexOf('/', elementPath.length + 1); + const moduleGroup = (next === -1) ? (moduleName + '/') : moduleName.slice(0, next + 1); + + // If the first was an exact match (direct child of element) then it is the element parent, skip + // it + if (next === -1) { + i++; + } + + const siblings: string[] = []; + while (i < moduleNames.length && moduleNames[i].startsWith(moduleGroup)) { + siblings.push(moduleNames[i++]); + } + + let childElement = buildModuleHierarchy(siblings, modules, moduleGroup); + + for (let cur = childElement; (cur = liftElement(childElement)) !== childElement;) { + childElement = cur; + } + + element.children!.push(childElement); + } + + // Cleanup empty children+link + if (!element.link) { + delete element.link; + } + if (!element.children || element.children.length === 0) { + delete element.children; + } + + return element; +} + +function liftElement(element: LinkerTreeElement): LinkerTreeElement { + let {name, link, children} = element; + + if (!children || !children.length) { + return element; + } + + // A link and all the child links align under => this link alone represents that + if (link && allElementsAlignUnder(name, link, children)) { + return {name, link}; + } + + // No link but all children align => the link can be lifted to here + if (!link && allElementsAlign(name, children)) { + return { + name, + link: toParentLink(children[0].link!), + }; + } + + // Only a single child and this element is just a directory (no link) => only need the child link + // Do this last only after trying to lift child links up + if (children.length === 1 && !link) { + return children[0]; + } + + return element; +} + +function toParentLink(link: Link): Link { + return [link[0], path.dirname(link[1])]; +} + +function allElementsAlign(name: string, elements: LinkerTreeElement[]): boolean { + if (!elements[0].link) { + return false; + } + + const parentLink = toParentLink(elements[0].link!); + + // Every child needs a link with aligning parents + if (!elements.every(e => !!e.link && isDirectChildLink(parentLink, e.link))) { + return false; + } + + return !!elements[0].link && allElementsAlignUnder(name, parentLink, elements); +} + +function allElementsAlignUnder( + parentName: string, parentLink: Link, elements: LinkerTreeElement[]) { + for (const {name, link, children} of elements) { + if (!link || children) { + return false; + } + + if (!isDirectChildPath(parentName, name)) { + return false; + } + + if (!isDirectChildLink(parentLink, link)) { + return false; + } + + if (!isNameLinkPathTopAligned(name, link)) { + return false; + } + } + + return true; +} + +function isDirectChildPath(parent: string, child: string) { + return parent === path.dirname(child); +} + +function isDirectChildLink([parentRel, parentPath]: Link, [childRel, childPath]: Link) { + // Same link-relation type + if (parentRel !== childRel) { + return false; + } + + // Child path is a directly-child of the parent path + if (!isDirectChildPath(parentPath, childPath)) { + return false; + } + + return true; +} + +function isNameLinkPathTopAligned(namePath: string, [, linkPath]: Link) { + return path.basename(namePath) === path.basename(linkPath); } // See link_node_modules.bzl where these three strings @@ -315,7 +423,16 @@ export function groupAndReduceModules(modules: {[name: string]: any[]}): // bin: bazel-bin/path/to/package // src: workspace/path/to/package // runfiles: look in the runfiles dir/manifest -type LinkerRoot = 'bin'|'src'|'runfiles'; +export type Link = [LinkerRoot, string]; +export type LinkerTreeElement = { + name: string, + link?: Link, + children?: LinkerTreeElement[], +}; +export type LinkerRoot = 'bin'|'src'|'runfiles'; +export type LinkerAliases = { + [name: string]: Link +}; export async function main(args: string[], runfiles: Runfiles) { if (!args || args.length < 1) throw new Error('requires one argument: modulesManifest path'); @@ -357,41 +474,41 @@ export async function main(args: string[], runfiles: Runfiles) { // Symlinks to packages need to reach back to the workspace/runfiles directory 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 = []; + async function linkModules(m: LinkerTreeElement) { + // ensure the parent directory exist + await mkdirp(path.dirname(m.name)); + + if (m.link) { + const [root, modulePath] = m.link; - async function linkModules(modules: {name: string, root: LinkerRoot, modulePath: string}[]) { - for (const m of modules) { let target: string = ''; - switch (m.root) { + switch (root) { case 'bin': // FIXME(#1196) - target = path.join(workspaceAbs, bin, toWorkspaceDir(m.modulePath)); + target = path.join(workspaceAbs, bin, toWorkspaceDir(modulePath)); break; case 'src': - target = path.join(workspaceAbs, toWorkspaceDir(m.modulePath)); + target = path.join(workspaceAbs, toWorkspaceDir(modulePath)); break; case 'runfiles': - target = runfiles.resolve(m.modulePath) || ''; + target = runfiles.resolve(modulePath) || ''; break; } - // ensure the subdirectories exist - await mkdirp(path.dirname(m.name)); await symlink(target, m.name); } + + // Process each child branch concurrently + if (m.children) { + await Promise.all(m.children.map(linkModules)); + } } - 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)); + const moduleHeirarchy = reduceModules(modules); + log_verbose(`mapping hierarchy ${JSON.stringify(moduleHeirarchy)}`); - // call linkModules for each group - links.push(linkModules(mappings)); - } + // Process each root branch concurrently + const links = moduleHeirarchy.map(linkModules); let code = 0; await Promise.all(links).catch(e => { diff --git a/internal/linker/test/link_node_modules.spec.ts b/internal/linker/test/link_node_modules.spec.ts index 7fce3aefe1..b681510dc9 100644 --- a/internal/linker/test/link_node_modules.spec.ts +++ b/internal/linker/test/link_node_modules.spec.ts @@ -2,6 +2,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as linker from '../link_node_modules'; +import {LinkerAliases, LinkerTreeElement} from '../link_node_modules'; const BIN_DIR = `bazel-out/my-platform-fastbuild/bin`; @@ -32,47 +33,206 @@ 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)); - }) + function readWorkspaceNodeModules(...parts: string[]) { + const filePath = path.join(process.env['TEST_TMPDIR']!, workspace, 'node_modules', ...parts); + return fs.readFileSync(filePath, 'utf-8') + } + + describe('reduceModules', () => { + it('should support no links', () => { + expect(linker.reduceModules({})).toEqual([]); + }); + + it('should pull aligned child paths up', () => { + const IN: LinkerAliases = { + '@foo/a/1': ['bin', 'root/sub/a/1'], + '@foo/a/2': ['bin', 'root/sub/a/2'], + }; + const OUT: LinkerTreeElement[] = [{ + name: '@foo', + link: ['bin', 'root/sub'], + }]; + + expect(linker.reduceModules(IN)).toEqual(OUT); + }); + + it('should pull deep aligned child paths up', () => { + const IN: LinkerAliases = { + '@foo/a/b/1': ['bin', 'root/sub/a/b/1'], + '@foo/a/b/2': ['bin', 'root/sub/a/b/2'], + }; + const OUT: LinkerTreeElement[] = [{ + name: '@foo', + link: ['bin', 'root/sub'], + }]; + + expect(linker.reduceModules(IN)).toEqual(OUT); + }); + + it('should not change aligned paths up with a mis-aligned parent', () => { + const IN: LinkerAliases = { + '@foo/a/b/1': ['bin', 'root/sub/other/a/b/1'], + '@foo/a/b/2': ['bin', 'root/sub/a/b/2'], + }; + const OUT: LinkerTreeElement[] = [{ + 'name': '@foo/a/b', + 'children': [ + {'name': '@foo/a/b/1', 'link': ['bin', 'root/sub/other/a/b/1']}, + {'name': '@foo/a/b/2', 'link': ['bin', 'root/sub/a/b/2']} + ] + }]; + + expect(linker.reduceModules(IN)).toEqual(OUT); + }); + + it('should not reduce parent/child when parent linking to different path', () => { + const IN: LinkerAliases = { + '@foo/a': ['bin', 'root/foo'], + '@foo/a/b/1': ['bin', 'root/sub/a/b/1'], + '@foo/a/b/2': ['bin', 'root/sub/a/b/2'], + }; + const OUT: LinkerTreeElement[] = [{ + name: '@foo/a', + link: ['bin', 'root/foo'], + children: [{ + name: '@foo/a/b', + link: ['bin', 'root/sub/a/b'], + }] + }]; + + expect(linker.reduceModules(IN)).toEqual(OUT); + }); + + it('should reduce aligned parent+children aliases to single parent alias', () => { + const IN: LinkerAliases = { + '@foo/a': ['bin', 'root/sub/a'], + '@foo/a/1': ['bin', 'root/sub/a/1'], + '@foo/a/2': ['bin', 'root/sub/a/2'], + }; + const OUT: LinkerTreeElement[] = [{ + name: '@foo', + link: ['bin', 'root/sub'], + }]; + + expect(linker.reduceModules(IN)).toEqual(OUT); + }); + + it('should reduce parent+children aliases aligned to different roots ', () => { + const IN: LinkerAliases = { + '@foo/a': ['bin', 'root/sub/a'], + '@foo/a/1': ['runfiles', 'root/sub/a/1'], + '@foo/a/2': ['bin', 'root/sub/a/2'], + }; + const OUT: LinkerTreeElement[] = [{ + name: '@foo/a', + link: ['bin', 'root/sub/a'], + children: [ + {name: '@foo/a/1', link: ['runfiles', 'root/sub/a/1']}, { + name: '@foo/a/2', + link: ['bin', 'root/sub/a/2'], + } + ] + }]; + + expect(linker.reduceModules(IN)).toEqual(OUT); + }); + + it('should reduce deeply-aligned siblings', () => { + const IN: LinkerAliases = { + '@foo/a/b/c/d1': ['bin', 'root/sub/a/b/c/d1'], + '@foo/a/b/c/d2': ['bin', 'root/sub/a/b/c/d2'], + }; + const OUT: LinkerTreeElement[] = [{ + name: '@foo', + link: ['bin', 'root/sub'], + }]; + + expect(linker.reduceModules(IN)).toEqual(OUT); + }); + + it('should not reduce parent/child with different link paths', () => { + const IN: LinkerAliases = { + 'b/b': ['src', 'other_wksp/path/to/lib_bb'], + 'b': ['src', 'other_wksp/path/to/lib_b'], + }; + const OUT: LinkerTreeElement[] = [{ + name: 'b', + link: ['src', 'other_wksp/path/to/lib_b'], + children: [{ + name: 'b/b', + link: ['src', 'other_wksp/path/to/lib_bb'], + }], + }]; + + expect(linker.reduceModules(IN)).toEqual(OUT); + }); + + it('should not reduce aligned paths when link has extra dir', () => { + const IN: LinkerAliases = { + '@foo/lib/a': ['bin', `path/to/lib/noseeme/a`], + '@foo/lib/b': ['bin', `path/to/lib/noseeme/b`], + '@foo/lib/c': ['bin', `path/to/lib/noseeme/c`], + }; + const OUT: LinkerTreeElement[] = [{ + name: '@foo/lib', + link: ['bin', 'path/to/lib/noseeme'], + }]; + + expect(linker.reduceModules(IN)).toEqual(OUT); + }) + + it('should reduce complicated example', () => { + const IN: LinkerAliases = { + '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 linker should derive that symlink + // from the lowest common denominator of the module name & module path + }; + const OUT: LinkerTreeElement[] = [ + { + name: '@foo', + children: [ + { + name: '@foo/c', + link: ['bin', `${workspace}/path/to/foo_c`], + children: [{ + name: '@foo/c/c', + link: ['bin', `${workspace}/path/to/foo_cc`], + children: [{ + name: '@foo/c/c/c/c', + link: ['bin', `${workspace}/path/to/foo_cccc`], + }] + }] + }, + { + name: '@foo/d', + link: ['bin', `${workspace}/path/to/foo_d`], + }, + ], + }, + { + name: 'a', + link: ['src', `${workspace}/path/to/lib_a`], + }, + { + name: 'b', + link: ['src', 'other_wksp/path/to/lib_b'], + children: [{ + name: 'b/b', + link: ['src', 'other_wksp/path/to/lib_bb'], + }], + } + ]; + + expect(linker.reduceModules(IN)).toEqual(OUT); + }); + }); it('should report when modules manifest absent', async () => { try { @@ -150,55 +310,240 @@ describe('link_node_modules', () => { // The linker expects to run as its own process, so it changes the wd process.chdir(path.join()); - 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')) + expect(readWorkspaceNodeModules('a', 'index.js')).toEqual('/*a*/exports = {}'); + expect(readWorkspaceNodeModules('b', 'index.js')).toEqual('/*b*/exports = {}'); + expect(readWorkspaceNodeModules('b', 'b', 'index.js')).toEqual('/*b/b*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'c', 'index.js')).toEqual('/*@foo/c*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'c', 'c', 'index.js')) .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')) + expect(readWorkspaceNodeModules('@foo', 'c', 'c', 'c', 'c', 'index.js')) .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')) + expect(readWorkspaceNodeModules('@foo', 'd', 'index.js')).toEqual('/*@foo/d*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'd', 'bar', 'index.js')) .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')) + expect(readWorkspaceNodeModules('@foo', 'd', 'bar', 'fum', 'far', 'index.js')) .toEqual('/*@foo/d/bar/fum/far*/exports = {}'); }); + it('should handle first-party packages with sibling directories', async () => { + // Set the cwd() like Bazel would in the execroot + process.chdir(workspace); + + // Create sub-packages to a lib in the user workspace + mkdirp(`${BIN_DIR}/path/to/lib/a`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/a/index.js`, '/*a*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/lib/b`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/b/index.js`, '/*b*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/lib/c`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/c/index.js`, '/*c*/exports = {}', 'utf-8'); + + writeManifest({ + 'bin': BIN_DIR, + 'modules': { + '@foo/lib/a': ['bin', `${workspace}/path/to/lib/a`], + '@foo/lib/b': ['bin', `${workspace}/path/to/lib/b`], + '@foo/lib/c': ['bin', `${workspace}/path/to/lib/c`], + }, + 'workspace': workspace, + }); + + // TODO(alexeagle): test should control the environment, not just pass through + await linker.main(['manifest.json'], new linker.Runfiles(process.env)); + + // The linker expects to run as its own process, so it changes the wd + process.chdir(path.join()); + expect(readWorkspaceNodeModules('@foo', 'lib', 'a', 'index.js')).toEqual('/*a*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'lib', 'b', 'index.js')).toEqual('/*b*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'lib', 'c', 'index.js')).toEqual('/*c*/exports = {}'); + }); + + it('should handle first-party packages with sibling directories with different link-types', + async () => { + // Set the cwd() like Bazel would in the execroot + process.chdir(workspace); + + // Create sub-packages to a lib in the user workspace + mkdirp(`${BIN_DIR}/path/to/lib/a`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/a/index.js`, '/*a*/exports = {}', 'utf-8'); + mkdirp(`path/to/lib/b`); + fs.writeFileSync(`path/to/lib/b/index.js`, '/*b*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/lib/c`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/c/index.js`, '/*c*/exports = {}', 'utf-8'); + + writeManifest({ + 'bin': BIN_DIR, + 'modules': { + '@foo/lib/a': ['bin', `${workspace}/path/to/lib/a`], + '@foo/lib/b': ['src', `${workspace}/path/to/lib/b`], + '@foo/lib/c': ['bin', `${workspace}/path/to/lib/c`], + }, + 'workspace': workspace, + }); + + // TODO(alexeagle): test should control the environment, not just pass through + await linker.main(['manifest.json'], new linker.Runfiles(process.env)); + + // The linker expects to run as its own process, so it changes the wd + process.chdir(path.join()); + expect(readWorkspaceNodeModules('@foo', 'lib', 'a', 'index.js')) + .toEqual('/*a*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'lib', 'b', 'index.js')) + .toEqual('/*b*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'lib', 'c', 'index.js')) + .toEqual('/*c*/exports = {}'); + }); + + it('should handle first-party packages with sibling nested directories', async () => { + // Set the cwd() like Bazel would in the execroot + process.chdir(workspace); + + // Create sub-packages to a lib in the user workspace + mkdirp(`${BIN_DIR}/path/to/lib`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/index.js`, '/*root*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/lib/a`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/a/index.js`, '/*a*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/lib/b`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/b/index.js`, '/*b*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/lib/c`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/c/index.js`, '/*c*/exports = {}', 'utf-8'); + + writeManifest({ + 'bin': BIN_DIR, + 'modules': { + '@foo/lib': ['bin', `${workspace}/path/to/lib`], + '@foo/lib/a': ['bin', `${workspace}/path/to/lib/a`], + '@foo/lib/b': ['bin', `${workspace}/path/to/lib/b`], + '@foo/lib/c': ['bin', `${workspace}/path/to/lib/c`], + }, + 'workspace': workspace, + }); + + // TODO(alexeagle): test should control the environment, not just pass through + await linker.main(['manifest.json'], new linker.Runfiles(process.env)); + + // The linker expects to run as its own process, so it changes the wd + process.chdir(path.join()); + expect(readWorkspaceNodeModules('@foo', 'lib', 'a', 'index.js')).toEqual('/*a*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'lib', 'b', 'index.js')).toEqual('/*b*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'lib', 'c', 'index.js')).toEqual('/*c*/exports = {}'); + }); + + it('should handle first-party packages with sibling mappings and inconsistent directories', + async () => { + // Set the cwd() like Bazel would in the execroot + process.chdir(workspace); + + // Create sub-packages to a lib in the user workspace + mkdirp(`${BIN_DIR}/path/to/lib/x/a`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/x/a/index.js`, '/*a*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/lib/b`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/b/index.js`, '/*b*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/lib/c`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/c/index.js`, '/*c*/exports = {}', 'utf-8'); + + writeManifest({ + 'bin': BIN_DIR, + 'modules': { + '@foo/lib/a': ['bin', `${workspace}/path/to/lib/x/a`], + '@foo/lib/b': ['bin', `${workspace}/path/to/lib/b`], + '@foo/lib/c': ['bin', `${workspace}/path/to/lib/c`], + }, + 'workspace': workspace, + }); + + // TODO(alexeagle): test should control the environment, not just pass through + await linker.main(['manifest.json'], new linker.Runfiles(process.env)); + + // The linker expects to run as its own process, so it changes the wd + process.chdir(path.join()); + expect(readWorkspaceNodeModules('@foo', 'lib', 'a', 'index.js')) + .toEqual('/*a*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'lib', 'b', 'index.js')) + .toEqual('/*b*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'lib', 'c', 'index.js')) + .toEqual('/*c*/exports = {}'); + }); + + it('should handle first-party packages with sibling dirs with a dir skipped in the module name', + async () => { + // Set the cwd() like Bazel would in the execroot + process.chdir(workspace); + + // Create sub-packages to a lib in the user workspace + mkdirp(`${BIN_DIR}/path/to/lib/noseeme/a`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/noseeme/a/index.js`, '/*a*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/lib/noseeme/b`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/noseeme/b/index.js`, '/*b*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/lib/noseeme/c`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/noseeme/c/index.js`, '/*c*/exports = {}', 'utf-8'); + + writeManifest({ + 'bin': BIN_DIR, + 'modules': { + '@foo/lib/a': ['bin', `${workspace}/path/to/lib/noseeme/a`], + '@foo/lib/b': ['bin', `${workspace}/path/to/lib/noseeme/b`], + '@foo/lib/c': ['bin', `${workspace}/path/to/lib/noseeme/c`], + }, + 'workspace': workspace, + }); + + // TODO(alexeagle): test should control the environment, not just pass through + await linker.main(['manifest.json'], new linker.Runfiles(process.env)); + + // The linker expects to run as its own process, so it changes the wd + process.chdir(path.join()); + expect(readWorkspaceNodeModules('@foo', 'lib', 'a', 'index.js')) + .toEqual('/*a*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'lib', 'b', 'index.js')) + .toEqual('/*b*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'lib', 'c', 'index.js')) + .toEqual('/*c*/exports = {}'); + }); + + it('should handle first-party packages nested sub-package style tree', async () => { + // Set the cwd() like Bazel would in the execroot + process.chdir(workspace); + + // Create nested sub-packages in the user workspace + mkdirp(`${BIN_DIR}/path/to/lib/a`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/a/index.js`, '/*a*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/lib/a/1`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/a/1/index.js`, '/*a1*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/lib/b`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/b/index.js`, '/*b*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/lib/c`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/c/index.js`, '/*c*/exports = {}', 'utf-8'); + mkdirp(`${BIN_DIR}/path/to/lib/c/1`); + fs.writeFileSync(`${BIN_DIR}/path/to/lib/c/1/index.js`, '/*c1*/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': { + '@foo/lib/c/1': ['bin', `${workspace}/path/to/lib/c/1`], + '@foo/lib/a': ['bin', `${workspace}/path/to/lib/a`], + '@foo/lib/b': ['bin', `${workspace}/path/to/lib/b`], + '@foo/lib/c': ['bin', `${workspace}/path/to/lib/c`], + '@foo/lib/a/1': ['bin', `${workspace}/path/to/lib/a/1`], + }, + 'workspace': workspace, + }); + + // TODO(alexeagle): test should control the environment, not just pass through + await linker.main(['manifest.json'], new linker.Runfiles(process.env)); + + // The linker expects to run as its own process, so it changes the wd + process.chdir(path.join()); + expect(readWorkspaceNodeModules('@foo', 'lib', 'c', '1', 'index.js')) + .toEqual('/*c1*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'lib', 'a', 'index.js')).toEqual('/*a*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'lib', 'b', 'index.js')).toEqual('/*b*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'lib', 'c', 'index.js')).toEqual('/*c*/exports = {}'); + expect(readWorkspaceNodeModules('@foo', 'lib', 'a', '1', 'index.js')) + .toEqual('/*a1*/exports = {}'); + }); + it('should handle third-party packages in runfiles', async () => { mkdirp('npm/node_modules/some-package'); const idx = 'npm/node_modules/some-package/index.js';