Visitar URL original
fix(arborist): repair wrong-but-existing symlink target under linked … · npm/cli@d3fa3d2 · GitHub
Skip to content

Commit d3fa3d2

Browse files
committed
fix(arborist): repair wrong-but-existing symlink target under linked strategy
1 parent 989f571 commit d3fa3d2

2 files changed

Lines changed: 47 additions & 1 deletion

File tree

‎workspaces/arborist/lib/arborist/reify.js‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const { callLimit: promiseCallLimit } = require('promise-call-limit')
1111
const { depth: dfwalk } = require('treeverse')
1212
const { dirname, resolve, relative, join, sep } = require('node:path')
1313
const { log, time } = require('proc-log')
14-
const { existsSync } = require('node:fs')
14+
const { existsSync, realpathSync } = require('node:fs')
1515
const { lstat, mkdir, readdir, readlink, rm, symlink } = require('node:fs/promises')
1616
const { moveFile } = require('@npmcli/fs')
1717
const { subset, intersects } = require('semver')
@@ -869,6 +869,10 @@ module.exports = cls => class Reifier extends cls {
869869
if (child.isLink && child.resolved?.startsWith('file:.store/') && !existsSync(child.realpath)) {
870870
continue
871871
}
872+
// Skip a link whose on-disk target is a valid-but-wrong store key (e.g. an interrupted update), so the diff repoints it via ADD.
873+
if (child.isLink && this.#linkTargetMismatch(child)) {
874+
continue
875+
}
872876
let entry
873877
if (child.isLink) {
874878
entry = new IsolatedLink(child)
@@ -908,6 +912,12 @@ module.exports = cls => class Reifier extends cls {
908912
return wrapper
909913
}
910914

915+
// True when the link's on-disk target resolves to a different path than its ideal target.
916+
// The caller only invokes this once both paths exist, so realpathSync won't throw.
917+
#linkTargetMismatch (child) {
918+
return realpathSync(child.path) !== realpathSync(child.realpath)
919+
}
920+
911921
// When extracting a registry-resolved package, the spec we hand to pacote is name@URL.
912922
// pacote re-parses that with npa and gets spec.type === 'remote', so without an override the allow-remote gate would fire on every registry tarball (both =none and =root mis-fire).
913923
// Returns true only when we are confident this is a registry-mediated install.

‎workspaces/arborist/test/isolated-mode.js‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,42 @@ tap.test('idempotent install with legacyPeerDeps and workspace peer deps', async
414414
}
415415
})
416416

417+
tap.test('reinstall repairs a wrong-but-existing top-level symlink target', async t => {
418+
// Regression for https://github.com/npm/cli/issues/9611: an interrupted update can leave a top-level symlink pointing at a valid-but-wrong store key. Reinstall must repoint it, not treat it as already-correct.
419+
// root depends on a@1.0.0 directly and on b@1.0.0, which pulls a@2.0.0 — so both versions of `a` live in the store.
420+
const graph = {
421+
registry: [
422+
{ name: 'a', version: '1.0.0' },
423+
{ name: 'a', version: '2.0.0' },
424+
{ name: 'b', version: '1.0.0', dependencies: { a: '2.0.0' } },
425+
],
426+
root: {
427+
name: 'myroot', version: '1.0.0', dependencies: { a: '1.0.0', b: '1.0.0' },
428+
},
429+
}
430+
431+
const { dir, registry } = await getRepo(graph)
432+
const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
433+
const opts = { path: dir, registry, packumentCache: new Map(), cache }
434+
435+
await new Arborist(opts).reify({ installStrategy: 'linked' })
436+
437+
const topLink = path.join(dir, 'node_modules', 'a')
438+
const readVersion = p => JSON.parse(fs.readFileSync(path.join(p, 'package.json'), 'utf8')).version
439+
t.equal(readVersion(fs.realpathSync(topLink)), '1.0.0', 'top-level a resolves to 1.0.0 after install')
440+
441+
// Simulate an interrupted update: repoint node_modules/a at the wrong (but real) a@2.0.0 store key.
442+
const storeDir = path.join(dir, 'node_modules', '.store')
443+
const wrongKey = fs.readdirSync(storeDir).find(k => k.startsWith('a@2.0.0'))
444+
fs.unlinkSync(topLink)
445+
fs.symlinkSync(path.join('.store', wrongKey, 'node_modules', 'a'), topLink, 'junction')
446+
t.equal(readVersion(fs.realpathSync(topLink)), '2.0.0', 'symlink corrupted to 2.0.0 before reinstall')
447+
448+
await new Arborist({ ...opts, packumentCache: new Map() }).reify({ installStrategy: 'linked' })
449+
450+
t.equal(readVersion(fs.realpathSync(topLink)), '1.0.0', 'reinstall repaired the symlink back to 1.0.0')
451+
})
452+
417453
tap.test('Lock file is same in hoisted and in isolated mode', async t => {
418454
const graph = {
419455
registry: [

0 commit comments

Comments
 (0)