Skip to content

Commit 0217724

Browse files
jbedardalexeagle
authored andcommitted
fix(builtin): link module_name to directories recursively to avoid directory clashes (#1432)
ensure all FS test cases have equivelent reduceModules cases update fs module-name tests to test unique link fs-layouts, not unique module_name=>link conversions Fixes #1411
1 parent f3bb56b commit 0217724

3 files changed

Lines changed: 748 additions & 239 deletions

File tree

internal/linker/index.js

Lines changed: 153 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,9 @@ Include as much of the build output as you can without disclosing anything confi
6464
}
6565
});
6666
}
67-
function symlink(target, path) {
67+
function symlink(target, p) {
6868
return __awaiter(this, void 0, void 0, function* () {
69-
log_verbose(`symlink( ${path} -> ${target} )`);
69+
log_verbose(`symlink( ${p} -> ${target} )`);
7070
// Check if the target exists before creating the symlink.
7171
// This is an extra filesystem access on top of the symlink but
7272
// 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
8181
// Use junction on Windows since symlinks require elevated permissions.
8282
// We only link to directories so junctions work for us.
8383
try {
84-
yield fs.promises.symlink(target, path, 'junction');
84+
yield fs.promises.symlink(target, p, 'junction');
8585
return true;
8686
}
8787
catch (e) {
@@ -95,9 +95,9 @@ Include as much of the build output as you can without disclosing anything confi
9595
// Be verbose about creating a bad symlink
9696
// Maybe this should fail in production as well, but again we want to avoid
9797
// any unneeded file I/O
98-
if (!(yield exists(path))) {
98+
if (!(yield exists(p))) {
9999
log_verbose('ERROR\n***\nLooks like we created a bad symlink:' +
100-
`\n pwd ${process.cwd()}\n target ${target}\n path ${path}\n***`);
100+
`\n pwd ${process.cwd()}\n target ${target}\n path ${p}\n***`);
101101
}
102102
}
103103
return false;
@@ -246,60 +246,140 @@ Include as much of the build output as you can without disclosing anything confi
246246
}
247247
});
248248
}
249-
function groupAndReduceModules(modules) {
250-
// Group nested modules names as these need to be symlinked in order.
251-
// For example, given a list of module keys such as:
252-
// ['a', '@foo/c/c/c/c', 'b/b', 'b', '@foo/c', '@foo/c/c']
253-
// this reducer should output the groups list:
254-
// [ [ '@foo/c', '@foo/c/c', '@foo/c/c/c/c' ], [ 'a' ], [ 'b', 'b/b' ] ]
255-
const grouped = Object.keys(modules).sort().reduce((grouped, module, index, array) => {
256-
if (index > 0 && module.startsWith(`${array[index - 1]}/`)) {
257-
grouped[grouped.length - 1].push(module);
249+
/**
250+
* Given a set of module aliases returns an array of recursive `LinkerTreeElement`.
251+
*
252+
* The tree nodes represent the FS links required to represent the module aliases.
253+
* Each node of the tree hierarchy depends on its parent node having been setup first.
254+
* Each sibling node can be processed concurrently.
255+
*
256+
* The number of symlinks is minimized in situations such as:
257+
*
258+
* Shared parent path to lowest common denominator:
259+
* `@foo/b/c => /path/to/a/b/c`
260+
*
261+
* can be represented as
262+
*
263+
* `@foo => /path/to/a`
264+
*
265+
* Shared parent path across multiple module names:
266+
* `@foo/p/a => /path/to/x/a`
267+
* `@foo/p/c => /path/to/x/a`
268+
*
269+
* can be represented as a single parent
270+
*
271+
* `@foo/p => /path/to/x`
272+
*/
273+
function reduceModules(modules) {
274+
return buildModuleHierarchy(Object.keys(modules).sort(), modules, '/').children || [];
275+
}
276+
exports.reduceModules = reduceModules;
277+
function buildModuleHierarchy(moduleNames, modules, elementPath) {
278+
let element = {
279+
name: elementPath.slice(0, -1),
280+
link: modules[elementPath.slice(0, -1)],
281+
children: [],
282+
};
283+
for (let i = 0; i < moduleNames.length;) {
284+
const moduleName = moduleNames[i];
285+
const next = moduleName.indexOf('/', elementPath.length + 1);
286+
const moduleGroup = (next === -1) ? (moduleName + '/') : moduleName.slice(0, next + 1);
287+
// An exact match (direct child of element) then it is the element parent, skip it
288+
if (next === -1) {
289+
i++;
258290
}
259-
else {
260-
grouped.push([module]);
291+
const siblings = [];
292+
while (i < moduleNames.length && moduleNames[i].startsWith(moduleGroup)) {
293+
siblings.push(moduleNames[i++]);
261294
}
262-
return grouped;
263-
}, []);
264-
// Reduce links such as `@foo/b/c => /path/to/a/b/c` to their
265-
// lowest common denominator `@foo => /path/to/a` & then remove
266-
// duplicates.
267-
return grouped.map(group => {
268-
return group
269-
.map(name => {
270-
let [kind, modulePath] = modules[name];
271-
for (;;) {
272-
const bn = path.basename(name);
273-
const bmp = path.basename(modulePath);
274-
if (bn == bmp && bn !== name && bmp !== modulePath) {
275-
// strip off the last segment as it is common
276-
name = path.dirname(name);
277-
modulePath = path.dirname(modulePath);
278-
log_verbose(`module mapping ( ${name}/${bn} => ${modulePath}/${bmp} ) reduced to ( ${name} => ${modulePath} )`);
279-
}
280-
else {
281-
break;
282-
}
283-
}
284-
return { name, root: kind, modulePath };
285-
})
286-
.reduce((result, current) => {
287-
if (result.length > 0) {
288-
const last = result[result.length - 1];
289-
if (current.name === last.name && current.modulePath === last.modulePath) {
290-
// duplicate mapping after reduction
291-
if (current.root !== last.root) {
292-
throw new Error(`conflicting module mappings for '${last.name}' => '${last.modulePath}' of kind '${last.root}' and '${current.root}'`);
293-
}
294-
return result;
295-
}
296-
}
297-
result.push(current);
298-
return result;
299-
}, []);
300-
});
295+
let childElement = buildModuleHierarchy(siblings, modules, moduleGroup);
296+
for (let cur = childElement; (cur = liftElement(childElement)) !== childElement;) {
297+
childElement = cur;
298+
}
299+
element.children.push(childElement);
300+
}
301+
// Cleanup empty children+link
302+
if (!element.link) {
303+
delete element.link;
304+
}
305+
if (!element.children || element.children.length === 0) {
306+
delete element.children;
307+
}
308+
return element;
309+
}
310+
function liftElement(element) {
311+
let { name, link, children } = element;
312+
if (!children || !children.length) {
313+
return element;
314+
}
315+
// This element has a link and all the child elements have aligning links
316+
// => this link alone represents that structure
317+
if (link && allElementsAlignUnder(name, link, children)) {
318+
return { name, link };
319+
}
320+
// No link but all child elements have aligning links
321+
// => the link can be lifted to here
322+
if (!link && allElementsAlign(name, children)) {
323+
return {
324+
name,
325+
link: toParentLink(children[0].link),
326+
};
327+
}
328+
// Only a single child and this element is just a directory (no link) => only need the child link
329+
// Do this last only after trying to lift child links up
330+
if (children.length === 1 && !link) {
331+
return children[0];
332+
}
333+
return element;
334+
}
335+
function toParentLink(link) {
336+
return [link[0], path.dirname(link[1])];
337+
}
338+
function allElementsAlign(name, elements) {
339+
if (!elements[0].link) {
340+
return false;
341+
}
342+
const parentLink = toParentLink(elements[0].link);
343+
// Every child needs a link with aligning parents
344+
if (!elements.every(e => !!e.link && isDirectChildLink(parentLink, e.link))) {
345+
return false;
346+
}
347+
return !!elements[0].link && allElementsAlignUnder(name, parentLink, elements);
348+
}
349+
function allElementsAlignUnder(parentName, parentLink, elements) {
350+
for (const { name, link, children } of elements) {
351+
if (!link || children) {
352+
return false;
353+
}
354+
if (!isDirectChildPath(parentName, name)) {
355+
return false;
356+
}
357+
if (!isDirectChildLink(parentLink, link)) {
358+
return false;
359+
}
360+
if (!isNameLinkPathTopAligned(name, link)) {
361+
return false;
362+
}
363+
}
364+
return true;
365+
}
366+
function isDirectChildPath(parent, child) {
367+
return parent === path.dirname(child);
368+
}
369+
function isDirectChildLink([parentRel, parentPath], [childRel, childPath]) {
370+
// Ensure same link-relation type
371+
if (parentRel !== childRel) {
372+
return false;
373+
}
374+
// Ensure child path is a direct-child of the parent path
375+
if (!isDirectChildPath(parentPath, childPath)) {
376+
return false;
377+
}
378+
return true;
379+
}
380+
function isNameLinkPathTopAligned(namePath, [, linkPath]) {
381+
return path.basename(namePath) === path.basename(linkPath);
301382
}
302-
exports.groupAndReduceModules = groupAndReduceModules;
303383
function main(args, runfiles) {
304384
return __awaiter(this, void 0, void 0, function* () {
305385
if (!args || args.length < 1)
@@ -332,39 +412,37 @@ Include as much of the build output as you can without disclosing anything confi
332412
process.chdir(rootDir);
333413
// Symlinks to packages need to reach back to the workspace/runfiles directory
334414
const workspaceAbs = path.resolve(workspaceDir);
335-
// Now add symlinks to each of our first-party packages so they appear under the node_modules tree
336-
const links = [];
337-
function linkModules(modules) {
415+
function linkModules(m) {
338416
return __awaiter(this, void 0, void 0, function* () {
339-
for (const m of modules) {
417+
// ensure the parent directory exist
418+
yield mkdirp(path.dirname(m.name));
419+
if (m.link) {
420+
const [root, modulePath] = m.link;
340421
let target = '<package linking failed>';
341-
switch (m.root) {
422+
switch (root) {
342423
case 'bin':
343424
// FIXME(#1196)
344-
target = path.join(workspaceAbs, bin, toWorkspaceDir(m.modulePath));
425+
target = path.join(workspaceAbs, bin, toWorkspaceDir(modulePath));
345426
break;
346427
case 'src':
347-
target = path.join(workspaceAbs, toWorkspaceDir(m.modulePath));
428+
target = path.join(workspaceAbs, toWorkspaceDir(modulePath));
348429
break;
349430
case 'runfiles':
350-
target = runfiles.resolve(m.modulePath) || '<runfiles resolution failed>';
431+
target = runfiles.resolve(modulePath) || '<runfiles resolution failed>';
351432
break;
352433
}
353-
// ensure the subdirectories exist
354-
yield mkdirp(path.dirname(m.name));
355434
yield symlink(target, m.name);
356435
}
436+
// Process each child branch concurrently
437+
if (m.children) {
438+
yield Promise.all(m.children.map(linkModules));
439+
}
357440
});
358441
}
359-
const groupedMappings = groupAndReduceModules(modules);
360-
log_verbose(`grouped mappings ${JSON.stringify(groupedMappings)}`);
361-
for (const mappings of groupedMappings) {
362-
// ensure that common directories between groups exists
363-
// to prevent race conditions between parallelized linkModules
364-
yield mkdirp(path.dirname(mappings[0].name));
365-
// call linkModules for each group
366-
links.push(linkModules(mappings));
367-
}
442+
const moduleHeirarchy = reduceModules(modules);
443+
log_verbose(`mapping hierarchy ${JSON.stringify(moduleHeirarchy)}`);
444+
// Process each root branch concurrently
445+
const links = moduleHeirarchy.map(linkModules);
368446
let code = 0;
369447
yield Promise.all(links).catch(e => {
370448
log_error(e);

0 commit comments

Comments
 (0)