Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions workspaces/arborist/lib/arborist/isolated-reifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ module.exports = cls => class IsolatedReifier extends cls {
}

binNames.forEach(bn => {
target.binPaths.push(join(from.realpath, 'node_modules', '.bin', bn))
target.binPaths.push(join(dep.root.localPath, nmFolder, '.bin', bn))
})

const link = {
Expand All @@ -428,7 +428,10 @@ module.exports = cls => class IsolatedReifier extends cls {
path: join(dep.root.localPath, nmFolder, dep.name),
realpath: target.path,
name: toKey,
resolved: dep.resolved,
version: dep.version,
resolved: external
? `file:.store/${toKey}/node_modules/${dep.packageName}`
: dep.resolved,
top: { path: dep.root.localPath },
children: [],
fsChildren: [],
Expand Down
107 changes: 106 additions & 1 deletion workspaces/arborist/lib/arborist/reify.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const { callLimit: promiseCallLimit } = require('promise-call-limit')
const { depth: dfwalk } = require('treeverse')
const { dirname, resolve, relative, join } = require('node:path')
const { log, time } = require('proc-log')
const { existsSync } = require('node:fs')
const { lstat, mkdir, rm, symlink } = require('node:fs/promises')
const { moveFile } = require('@npmcli/fs')
const { subset, intersects } = require('semver')
Expand Down Expand Up @@ -75,6 +76,7 @@ module.exports = cls => class Reifier extends cls {
#shrinkwrapInflated = new Set()
#sparseTreeDirs = new Set()
#sparseTreeRoots = new Set()
#linkedActualForDiff = null

constructor (options) {
super(options)
Expand Down Expand Up @@ -116,6 +118,9 @@ module.exports = cls => class Reifier extends cls {
// of Node/Link trees
log.warn('reify', 'The "linked" install strategy is EXPERIMENTAL and may contain bugs.')
this.idealTree = await this[_createIsolatedTree]()
this.#linkedActualForDiff = this.#buildLinkedActualForDiff(
this.idealTree, this.actualTree
)
}
await this[_diffTrees]()
await this.#reifyPackages()
Expand All @@ -125,6 +130,7 @@ module.exports = cls => class Reifier extends cls {
this.idealTree = oldTree
}
await this[_saveIdealTree](options)
this.#linkedActualForDiff = null
// clean inert
for (const node of this.idealTree.inventory.values()) {
if (node.inert) {
Expand Down Expand Up @@ -450,7 +456,7 @@ module.exports = cls => class Reifier extends cls {
omit: this.#omit,
shrinkwrapInflated: this.#shrinkwrapInflated,
filterNodes,
actual: this.actualTree,
actual: this.#linkedActualForDiff || this.actualTree,
ideal: this.idealTree,
})

Expand Down Expand Up @@ -573,6 +579,7 @@ module.exports = cls => class Reifier extends cls {
// if the directory already exists, made will be undefined. if that's the case
// we don't want to remove it because we aren't the ones who created it so we
// omit it from the #sparseTreeRoots
/* istanbul ignore next -- mkdir returns path only when dir is new */
if (made) {
this.#sparseTreeRoots.add(made)
}
Expand Down Expand Up @@ -789,6 +796,104 @@ module.exports = cls => class Reifier extends cls {
return join(filePath)
}

// 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.
#buildLinkedActualForDiff (idealTree, actualTree) {
// Combined Map keyed by path (how allChildren() in diff.js keys)
const combined = new Map()

// Add actual tree's children (the top-level symlinks)
for (const child of actualTree.children.values()) {
combined.set(child.path, child)
}

// Add synthetic entries for store nodes and store links that exist on disk.
// The proxy tree is flat — all store entries (isInStore) and store links
// (isStoreLink) are direct children of root. The actual tree only has
// top-level links as root children, so store entries and store links
// need synthetic actual entries for the diff to match them.
for (const child of idealTree.children) {
if (!combined.has(child.path) && (child.isInStore || child.isStoreLink) &&
existsSync(child.path)) {
const entry = {
global: false,
globalTop: false,
isProjectRoot: false,
isTop: false,
location: child.location,
name: child.name,
optional: child.optional,
top: child.top,
children: [],
edgesIn: new Set(),
edgesOut: new Map(),
binPaths: [],
fsChildren: [],
/* istanbul ignore next -- emulate Node */
getBundler () {
return null
},
hasShrinkwrap: false,
inDepBundle: false,
integrity: null,
isLink: Boolean(child.isLink),
isRoot: false,
isInStore: Boolean(child.isInStore),
path: child.path,
realpath: child.realpath,
resolved: child.resolved,
version: child.version,
package: child.package,
}
entry.target = entry
if (child.isLink && combined.has(child.realpath)) {
entry.target = combined.get(child.realpath)
}
combined.set(child.path, entry)
}
}

// Proxy .get(name) to original actual tree for filterNodes compatibility
// (scoped workspace installs use .get(name), allChildren uses .values())
const origGet = actualTree.children.get.bind(actualTree.children)
const combinedGet = combined.get.bind(combined)
/* istanbul ignore next -- only reached during scoped workspace installs */
combined.get = (key) => combinedGet(key) || origGet(key)

const wrapper = {
isRoot: true,
isLink: actualTree.isLink,
target: actualTree.target,
fsChildren: actualTree.fsChildren,
path: actualTree.path,
realpath: actualTree.realpath,
edgesOut: actualTree.edgesOut,
inventory: actualTree.inventory,
package: actualTree.package,
resolved: actualTree.resolved,
version: actualTree.version,
integrity: actualTree.integrity,
binPaths: actualTree.binPaths,
hasShrinkwrap: false,
inDepBundle: false,
parent: null,
children: combined,
}

// Set parent/root on synthetic entries for consistency
for (const child of combined.values()) {
if (!child.parent) {
child.parent = wrapper
child.root = wrapper
}
}

return wrapper
}

#registryResolved (resolved) {
// the default registry url is a magic value meaning "the currently
// configured registry".
Expand Down
74 changes: 74 additions & 0 deletions workspaces/arborist/test/isolated-mode.js
Original file line number Diff line number Diff line change
Expand Up @@ -1637,6 +1637,80 @@ tap.test('bins are installed', async t => {
t.ok(binFromBarToWhich)
})

tap.test('subsequent linked install is a no-op', async t => {
const graph = {
registry: [
{ name: 'which', version: '1.0.0', bin: './bin.js', dependencies: { isexe: '^1.0.0' } },
{ name: 'isexe', version: '1.0.0' },
],
root: {
name: 'myproject',
version: '1.0.0',
dependencies: { which: '1.0.0' },
},
}
const { dir, registry } = await getRepo(graph)
const cache = fs.mkdtempSync(`${getTempDir()}/test-`)

// First install
const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
await arb1.reify({ installStrategy: 'linked' })

// Verify packages are installed
t.ok(fs.lstatSync(path.join(dir, 'node_modules', 'which')).isSymbolicLink(),
'which is a symlink after first install')

// Second install — should detect everything is up-to-date
const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
await arb2.reify({ installStrategy: 'linked' })

// Verify the diff has zero actionable leaves
const leaves = arb2.diff?.leaves || []
const actions = leaves.filter(l => l.action)
t.equal(actions.length, 0, 'second install should have no diff actions')

// Verify unchanged nodes were detected
t.ok(arb2.diff.unchanged.length > 0, 'second install should have unchanged nodes')

// Verify packages are still correctly installed
t.ok(fs.lstatSync(path.join(dir, 'node_modules', 'which')).isSymbolicLink(),
'which is still a symlink after second install')
t.ok(setupRequire(dir)('which'), 'which is requireable after second install')
})

tap.test('workspace links are not affected by store resolved fix', async t => {
const graph = {
registry: [
{ name: 'abbrev', version: '1.0.0' },
],
root: {
name: 'myproject',
version: '1.0.0',
dependencies: { abbrev: '1.0.0' },
},
workspaces: [
{ name: 'mypkg', version: '1.0.0', dependencies: { abbrev: '1.0.0' } },
],
}
const { dir, registry } = await getRepo(graph)
const cache = fs.mkdtempSync(`${getTempDir()}/test-`)

// First install
const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
await arb1.reify({ installStrategy: 'linked' })

// Second install
const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
await arb2.reify({ installStrategy: 'linked' })

// Verify workspace is still correctly linked
t.ok(setupRequire(dir)('mypkg'), 'workspace is requireable after second install')
t.ok(setupRequire(dir)('abbrev'), 'registry dep is requireable after second install')

// Verify the diff has unchanged nodes (store entries are correctly matched)
t.ok(arb2.diff.unchanged.length > 0, 'second install should have unchanged nodes')
})

function setupRequire (cwd) {
return function requireChain (...chain) {
return chain.reduce((path, name) => {
Expand Down
Loading