diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index 960510a86531e..4b10e78ff1c3f 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -26,7 +26,8 @@ const optionalSet = require('../optional-set.js') const relpath = require('../relpath.js') const retirePath = require('../retire-path.js') const treeCheck = require('../tree-check.js') -const { defaultLockfileVersion } = require('../shrinkwrap.js') +const Shrinkwrap = require('../shrinkwrap.js') +const { defaultLockfileVersion } = Shrinkwrap const { saveTypeMap, hasSubKey } = require('../add-rm-pkg-deps.js') const { IsolatedNode, IsolatedLink } = require('../isolated-classes.js') @@ -115,12 +116,15 @@ module.exports = cls => class Reifier extends cls { await this[_loadTrees](options) const oldTree = this.idealTree + // Kept to serialize the hidden lockfile from the on-disk .store/symlink layout. + let isolatedTree = null if (linked) { // swap out the tree with the isolated tree // this is currently technical debt which will be resolved in a refactor // of Node/Link trees log.warn('reify', 'The "linked" install strategy is EXPERIMENTAL and may contain bugs.') this.idealTree = await this.createIsolatedTree() + isolatedTree = this.idealTree if (this.actualTree) { this.#linkedActualForDiff = this.#buildLinkedActualForDiff( this.idealTree, this.actualTree @@ -241,17 +245,24 @@ module.exports = cls => class Reifier extends cls { calcDepFlags(this.idealTree) } - // save the ideal's meta as a hidden lockfile after we actualize it - this.idealTree.meta.filename = - this.idealTree.realpath + '/node_modules/.package-lock.json' - this.idealTree.meta.hiddenLockfile = true - this.idealTree.meta.lockfileVersion = defaultLockfileVersion + // save the ideal's meta as a hidden lockfile after we actualize it. + // Under linked the logical tree is the hoisted layout, so the hidden lockfile is serialized from the isolated tree instead. + if (!linked) { + this.idealTree.meta.filename = + this.idealTree.realpath + '/node_modules/.package-lock.json' + this.idealTree.meta.hiddenLockfile = true + this.idealTree.meta.lockfileVersion = defaultLockfileVersion + } this.actualTree = this.idealTree this.idealTree = null if (!this.options.global && !this.options.dryRun) { - await this.actualTree.meta.save() + if (linked) { + await this.#saveLinkedHiddenLockfile(isolatedTree) + } else { + await this.actualTree.meta.save() + } const ignoreScripts = !!this.options.ignoreScripts // if we aren't doing a dry run or ignoring scripts and we actually made changes to the dep // tree, then run the dependencies scripts @@ -834,6 +845,47 @@ module.exports = cls => class Reifier extends cls { return join(filePath) } + // Serialize the hidden lockfile from the isolated tree, which mirrors the on-disk .store/symlink layout. + // Its children are every materialized node_modules entry: store package dirs and all symlinks. + async #saveLinkedHiddenLockfile (isolatedTree) { + const path = isolatedTree.realpath + const meta = new Shrinkwrap({ + path, + hiddenLockfile: true, + lockfileVersion: defaultLockfileVersion, + resolveOptions: this.options, + }) + meta.reset() + meta.filename = resolve(path, 'node_modules/.package-lock.json') + const storeRe = /^(.*\/\.store\/.+?)\/node_modules\// + const containers = new Set() + const nodes = new Set() + for (const node of isolatedTree.children.values()) { + // Tree-only undeclared workspace self-links aren't on disk. + if (node.isUndeclaredWorkspaceLink) { + continue + } + nodes.add(node) + // Record the enclosing .store/ dir so loadVirtual can resolve a store package's sibling deps. + // node.location uses the platform separator; lockfile keys are posix. + const m = node.location.replace(/\\/g, '/').match(storeRe) + if (m) { + containers.add(m[1]) + } + } + // Workspace dirs hold their own dep symlinks; record them so the cache can validate those subtrees. + for (const ws of isolatedTree.fsChildren) { + nodes.add(ws) + } + for (const node of nodes) { + meta.add(node) + } + for (const loc of containers) { + meta.data.packages[loc] = {} + } + await meta.save() + } + // Build a flat actual tree wrapper for linked installs so the diff can correctly match store entries that already exist on disk. // The proxy tree from createIsolatedTree() is flat (all children on root), but loadActual() produces a nested tree where store entries are deep link targets. // This wrapper surfaces them at the root level for comparison. diff --git a/workspaces/arborist/lib/shrinkwrap.js b/workspaces/arborist/lib/shrinkwrap.js index cf27bcf4ac726..03cc86edbd4e9 100644 --- a/workspaces/arborist/lib/shrinkwrap.js +++ b/workspaces/arborist/lib/shrinkwrap.js @@ -172,6 +172,35 @@ const assertNoNewer = async (path, data, lockTime, dir, seen) => { return } + // The walk above can't reach two linked-strategy layouts: a store package's sibling deps under .store//node_modules (.store is a skipped dot-dir), and an undeclared workspace not symlinked into root node_modules. Walk those dirs, derived from the lockfile entries. + // A dir reachable through a link entry is skipped: it was already reached (or, if the symlink is stale, correctly stays unseen so the lockfile is rejected). This keeps the hoisted strategy's stale-symlink detection intact, since there every workspace is a link target. + const linkTargets = new Set() + for (const loc in data.packages) { + const { link, resolved } = data.packages[loc] + if (link && resolved) { + linkTargets.add(resolved.replace(/\\/g, '/')) + } + } + const extraDirs = new Set() + for (const loc in data.packages) { + const store = loc.match(/^(.*\/\.store\/.+?)\/node_modules\//) + if (store) { + // .store/ is never walked but has no entry, so mark it seen. + seen.add(store[1]) + extraDirs.add(`${store[1]}/node_modules`) + continue + } + // A workspace/fsChild dir outside node_modules, e.g. packages/a. + const i = loc.indexOf('/node_modules/') + const root = i === -1 ? loc : loc.slice(0, i) + if (root && !/(^|\/)node_modules(\/|$)/.test(root) && !linkTargets.has(root)) { + extraDirs.add(root) + } + } + for (const rel of extraDirs) { + await assertNoNewer(path, data, lockTime, resolve(path, rel), seen) + } + // assert that all the entries in the lockfile were seen for (const loc in data.packages) { if (!seen.has(loc)) { diff --git a/workspaces/arborist/test/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js index becde0b6fa4c2..e16563a7722bf 100644 --- a/workspaces/arborist/test/arborist/reify.js +++ b/workspaces/arborist/test/arborist/reify.js @@ -4333,6 +4333,119 @@ t.test('install strategy linked', async (t) => { t.ok(abbrev.isSymbolicLink(), 'abbrev got installed') }) + t.test('hidden lockfile records the linked .store layout and round-trips', async t => { + // Regression for #9612: the hidden lockfile must record the on-disk .store/symlink layout so it round-trips as a valid cache. + const Shrinkwrap = require('../../lib/shrinkwrap.js') + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + // once depends on wrappy, so the store has a transitive symlink to validate + dependencies: { once: '1.4.0' }, + }), + }) + + createRegistry(t, true) + await reify(path, { installStrategy: 'linked' }) + + const hidden = require(resolve(path, 'node_modules/.package-lock.json')) + const locs = Object.keys(hidden.packages) + // the layout is recorded at .store paths, not hoisted node_modules/ + t.ok(locs.some(l => /^node_modules\/\.store\/once@/.test(l)), + 'once is recorded under .store') + t.ok(locs.some(l => /^node_modules\/\.store\/.*\/node_modules\/wrappy$/.test(l)), + 'the transitive wrappy symlink is recorded inside the store') + t.notOk(locs.includes('node_modules/wrappy'), + 'wrappy is not recorded at a hoisted path') + + // the cache is accepted on reload: assertNoNewer matches it against the real disk layout + const meta = await Shrinkwrap.load({ path, hiddenLockfile: true }) + t.equal(meta.loadedFromDisk, true, 'hidden lockfile is a valid cache of the disk layout') + + // loadActual must reconstruct the tree from the cache with once->wrappy resolved through the store. + const actual = await newArb({ path, installStrategy: 'linked' }).loadActual() + const onceNode = [...actual.inventory.values()].find(n => n.name === 'once' && !n.isLink) + t.ok(onceNode, 'once is in the cached actual tree') + const wrappyEdge = onceNode.edgesOut.get('wrappy') + t.ok(wrappyEdge && !wrappyEdge.missing, 'once resolves its wrappy dep through the cached store layout') + }) + + t.test('hidden lockfile round-trips with an undeclared workspace', async t => { + // Regression for #9612: an undeclared workspace materializes deps in its own node_modules but isn't linked into root, and the cache must still validate that subtree. + const Shrinkwrap = require('../../lib/shrinkwrap.js') + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + workspaces: ['packages/a'], + // root does not depend on the workspace, so it stays undeclared + }), + packages: { + a: { + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + dependencies: { once: '1.4.0' }, + }), + }, + }, + }) + + createRegistry(t, true) + await reify(path, { installStrategy: 'linked' }) + + // the workspace's dep is materialized under its own node_modules, not the root's + t.ok(fs.lstatSync(resolve(path, 'packages/a/node_modules/once')).isSymbolicLink(), + 'once is symlinked into the workspace node_modules') + t.notOk(fs.existsSync(resolve(path, 'node_modules/a')), + 'the undeclared workspace is not symlinked into the root node_modules') + + const meta = await Shrinkwrap.load({ path, hiddenLockfile: true }) + t.equal(meta.loadedFromDisk, true, 'hidden lockfile validates the undeclared workspace subtree') + }) + + t.test('hidden lockfile round-trips with an undeclared workspace and no store entries', async t => { + // Regression for #9612: with only local deps there is no .store, so the cache must still walk the undeclared workspace subtree to validate it. + const Shrinkwrap = require('../../lib/shrinkwrap.js') + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + workspaces: ['packages/w', 'packages/a', 'packages/b'], + // only w is declared; a and b stay undeclared, and a depends on b locally + dependencies: { w: '1.0.0' }, + }), + packages: { + w: { 'package.json': JSON.stringify({ name: 'w', version: '1.0.0' }) }, + a: { + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + dependencies: { b: '1.0.0' }, + }), + }, + b: { 'package.json': JSON.stringify({ name: 'b', version: '1.0.0' }) }, + }, + }) + + createRegistry(t, false) + await reify(path, { installStrategy: 'linked' }) + + t.notOk(fs.existsSync(resolve(path, 'node_modules/.store')), + 'no store is created for an all-local graph') + t.ok(fs.lstatSync(resolve(path, 'packages/a/node_modules/b')).isSymbolicLink(), + 'the undeclared workspace links its local dep') + + const meta = await Shrinkwrap.load({ path, hiddenLockfile: true }) + t.equal(meta.loadedFromDisk, true, 'hidden lockfile validates the subtree without any store entry') + + // loadActual must reconstruct the undeclared workspace from the cache with its local dep resolved. + const actual = await newArb({ path, installStrategy: 'linked' }).loadActual() + const aNode = [...actual.inventory.values()].find(n => n.name === 'a' && !n.isLink) + const bEdge = aNode && aNode.edgesOut.get('b') + t.ok(bEdge && !bEdge.missing, 'the undeclared workspace resolves its local dep through the cache') + }) + t.test('does not re-create a workspace dir removed from manifest', async t => { // Regression test for https://github.com/npm/cli/issues/9331 const path = t.testdir({