Skip to content

Commit fe66535

Browse files
zkochanzkochan
authored
fix: converge incremental install by refreshing stale transitive pins (#12472)
When a package is reused from the lockfile, its child edges are taken verbatim and bypass the preferred-versions walk, so a transitive dependency can stay pinned to an older version even after a direct dependency resolved to a higher version that satisfies the same range — leaving the lockfile non-convergent (an incremental install keeps a duplicate that a fresh install would not). The resolver now refreshes such a stale pin to the higher direct-dependency version during resolution, via `preferredVersion` (singular), which overrides the EXISTING_VERSION_SELECTOR_WEIGHT stability bias. The older version is never resolved or fetched, and the incremental result converges to what a fresh install produces. The pick is anchored to direct dependencies (which resolve first), so it restores the automatic dedupe removed in #11110 without reintroducing its non-determinism, and unlike the post-pass in #11502 it does not over-fetch. pacquet is ported in the same change. Its full-subtree lockfile reuse is coarser than pnpm's per-edge reuse, so it records per importer which direct deps changed and their resolved versions, declines full-subtree reuse for a parent that depends on a changed direct dep, and forces the higher version in the child walk. Range satisfaction uses plain semver (not prerelease-inclusive), matching pnpm's semver.satisfies(.., true).
1 parent ff9a1cf commit fe66535

7 files changed

Lines changed: 585 additions & 5 deletions

File tree

‎.changeset/refresh-stale-lockfile-pins.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@pnpm/installing.deps-resolver": patch
3+
"pnpm": patch
4+
---
5+
6+
Fixed a lockfile non-convergence bug where an incremental install kept a duplicate transitive dependency that a fresh install would not produce. When a package is reused from the lockfile, its child edges are taken verbatim and bypass the preferred-versions walk, so a transitive dependency could stay pinned to an older version even after a direct dependency resolved to a higher version that satisfies the same range. The resolver now refreshes such a stale pin to the higher direct-dependency version during resolution — so the older version is never resolved or fetched, and the incremental result converges to the fresh one.

‎installing/deps-installer/test/install/dedupe.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,89 @@ test('ignore version of root dependency when it is incompatible with the indirec
212212
expect(lockfile.packages).toHaveProperty(['@pnpm.e2e/dep-of-pkg-with-1-dep@101.0.0'])
213213
})
214214

215+
test('refreshes a stale transitive pin to a higher direct-dependency version at resolution time', async () => {
216+
// The stale transitive pin is refreshed during resolution, so the older
217+
// version is never resolved or fetched (no post-resolution pruning).
218+
const project = prepareEmpty()
219+
220+
await addDistTag({ package: '@pnpm.e2e/dep-of-pkg-with-1-dep', version: '100.0.0', distTag: 'latest' })
221+
222+
const { updatedManifest: manifest } = await addDependenciesToPackage(
223+
{},
224+
['@pnpm.e2e/dep-of-pkg-with-1-dep@100.0.0', '@pnpm.e2e/pkg-with-1-dep@100.0.0'],
225+
testDefaults()
226+
)
227+
228+
expect(project.readLockfile().packages).toHaveProperty(['@pnpm.e2e/dep-of-pkg-with-1-dep@100.0.0'])
229+
230+
await addDistTag({ package: '@pnpm.e2e/dep-of-pkg-with-1-dep', version: '100.1.0', distTag: 'latest' })
231+
232+
await addDependenciesToPackage(
233+
manifest,
234+
['@pnpm.e2e/dep-of-pkg-with-1-dep@100.1.0'],
235+
testDefaults()
236+
)
237+
238+
const lockfile = project.readLockfile()
239+
expect(lockfile.packages).toHaveProperty(['@pnpm.e2e/dep-of-pkg-with-1-dep@100.1.0'])
240+
expect(lockfile.packages).not.toHaveProperty(['@pnpm.e2e/dep-of-pkg-with-1-dep@100.0.0'])
241+
})
242+
243+
test('does not refresh an aliased transitive dependency', async () => {
244+
// pkg-with-1-aliased-dep depends on `dep: npm:@pnpm.e2e/dep-of-pkg-with-1-dep@^100.0.0`.
245+
// An `npm:` specifier is not a plain semver range, so the refresh skips
246+
// the edge and the older version is kept (no misfire on aliases).
247+
const project = prepareEmpty()
248+
249+
await addDistTag({ package: '@pnpm.e2e/dep-of-pkg-with-1-dep', version: '100.0.0', distTag: 'latest' })
250+
251+
const { updatedManifest: manifest } = await addDependenciesToPackage(
252+
{},
253+
['@pnpm.e2e/dep-of-pkg-with-1-dep@100.0.0', '@pnpm.e2e/pkg-with-1-aliased-dep@100.0.0'],
254+
testDefaults()
255+
)
256+
257+
await addDistTag({ package: '@pnpm.e2e/dep-of-pkg-with-1-dep', version: '100.1.0', distTag: 'latest' })
258+
259+
await addDependenciesToPackage(
260+
manifest,
261+
['@pnpm.e2e/dep-of-pkg-with-1-dep@100.1.0'],
262+
testDefaults()
263+
)
264+
265+
const lockfile = project.readLockfile()
266+
expect(lockfile.packages).toHaveProperty(['@pnpm.e2e/dep-of-pkg-with-1-dep@100.1.0'])
267+
expect(lockfile.packages).toHaveProperty(['@pnpm.e2e/dep-of-pkg-with-1-dep@100.0.0'])
268+
})
269+
270+
test('refreshing a stale transitive pin is idempotent', async () => {
271+
const project = prepareEmpty()
272+
273+
await addDistTag({ package: '@pnpm.e2e/dep-of-pkg-with-1-dep', version: '100.0.0', distTag: 'latest' })
274+
275+
const { updatedManifest: manifest } = await addDependenciesToPackage(
276+
{},
277+
['@pnpm.e2e/dep-of-pkg-with-1-dep@100.0.0', '@pnpm.e2e/pkg-with-1-dep@100.0.0'],
278+
testDefaults()
279+
)
280+
281+
await addDistTag({ package: '@pnpm.e2e/dep-of-pkg-with-1-dep', version: '100.1.0', distTag: 'latest' })
282+
283+
const { updatedManifest: manifestWithBoth } = await addDependenciesToPackage(
284+
manifest,
285+
['@pnpm.e2e/dep-of-pkg-with-1-dep@100.1.0'],
286+
testDefaults()
287+
)
288+
289+
const convergedPackages = project.readLockfile().packages
290+
expect(convergedPackages).not.toHaveProperty(['@pnpm.e2e/dep-of-pkg-with-1-dep@100.0.0'])
291+
292+
// A second install over the converged lockfile must not reintroduce or
293+
// churn the refreshed edge.
294+
await install(manifestWithBoth, testDefaults())
295+
expect(project.readLockfile().packages).toStrictEqual(convergedPackages)
296+
})
297+
215298
test('prefer dist-tag specified for top dependency', async () => {
216299
const project = prepareEmpty()
217300

‎installing/deps-installer/test/install/multipleImporters.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1962,3 +1962,54 @@ require("fs").writeFileSync("created-by-prepare", "", "utf8")`)
19621962

19631963
expect(fs.existsSync('project-1/created-by-prepare')).toBeTruthy()
19641964
})
1965+
1966+
test("a direct-dependency bump in one importer converges another importer's transitive pin", async () => {
1967+
preparePackages([
1968+
{ location: 'project-1', package: { name: 'project-1' } },
1969+
{ location: 'project-2', package: { name: 'project-2' } },
1970+
])
1971+
1972+
const importers: MutatedProject[] = [
1973+
{ mutation: 'install', rootDir: path.resolve('project-1') as ProjectRootDir },
1974+
{ mutation: 'install', rootDir: path.resolve('project-2') as ProjectRootDir },
1975+
]
1976+
// project-1 pulls dep-of-pkg-with-1-dep transitively (via pkg-with-1-dep);
1977+
// project-2 depends on it directly. Only project-2 ever bumps it.
1978+
const allProjects = (directDepOfVersion: string) => [
1979+
{
1980+
buildIndex: 0,
1981+
manifest: {
1982+
name: 'project-1',
1983+
version: '1.0.0',
1984+
dependencies: { '@pnpm.e2e/pkg-with-1-dep': '100.0.0' },
1985+
},
1986+
rootDir: path.resolve('project-1') as ProjectRootDir,
1987+
},
1988+
{
1989+
buildIndex: 0,
1990+
manifest: {
1991+
name: 'project-2',
1992+
version: '1.0.0',
1993+
dependencies: { '@pnpm.e2e/dep-of-pkg-with-1-dep': directDepOfVersion },
1994+
},
1995+
rootDir: path.resolve('project-2') as ProjectRootDir,
1996+
},
1997+
]
1998+
1999+
await addDistTag({ package: '@pnpm.e2e/dep-of-pkg-with-1-dep', version: '100.0.0', distTag: 'latest' })
2000+
await mutateModules(importers, testDefaults({ allProjects: allProjects('100.0.0') }))
2001+
2002+
await addDistTag({ package: '@pnpm.e2e/dep-of-pkg-with-1-dep', version: '100.1.0', distTag: 'latest' })
2003+
await mutateModules(importers, testDefaults({ allProjects: allProjects('100.1.0') }))
2004+
2005+
const lockfile = readYamlFileSync<any>(WANTED_LOCKFILE) // eslint-disable-line @typescript-eslint/no-explicit-any
2006+
// All importers' direct deps resolve before any transitive, so project-2's
2007+
// bumped 100.1.0 is in scope when project-1's transitive edge re-resolves.
2008+
// The refresh converges that edge to 100.1.0 too — matching a fresh install
2009+
// (no pin), where `^100.0.0` resolves to the latest 100.1.0 — and the stale
2010+
// 100.0.0 is pruned.
2011+
expect(lockfile.importers['project-2'].dependencies['@pnpm.e2e/dep-of-pkg-with-1-dep'].version).toBe('100.1.0')
2012+
expect(lockfile.snapshots['@pnpm.e2e/pkg-with-1-dep@100.0.0'].dependencies['@pnpm.e2e/dep-of-pkg-with-1-dep']).toBe('100.1.0')
2013+
expect(lockfile.packages).toHaveProperty(['@pnpm.e2e/dep-of-pkg-with-1-dep@100.1.0'])
2014+
expect(lockfile.packages).not.toHaveProperty(['@pnpm.e2e/dep-of-pkg-with-1-dep@100.0.0'])
2015+
})

‎installing/deps-resolver/src/resolveDependencies.ts‎

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,7 @@ export async function resolveDependencies (
714714
): Promise<ResolvedDependenciesResult> {
715715
const extendedWantedDeps = getDepsToResolve(wantedDependencies, ctx.wantedLockfile, {
716716
preferredDependencies: options.preferredDependencies,
717+
preferredVersions,
717718
prefix: options.prefix,
718719
proceed: options.proceed || ctx.forceFullResolution,
719720
registries: ctx.registries,
@@ -1411,6 +1412,7 @@ function getDepsToResolve (
14111412
wantedLockfile: LockfileObject,
14121413
options: {
14131414
preferredDependencies?: ResolvedDependencies
1415+
preferredVersions?: PreferredVersions
14141416
prefix: string
14151417
proceed: boolean
14161418
registries: Registries
@@ -1430,14 +1432,34 @@ function getDepsToResolve (
14301432
})
14311433
for (const wantedDependency of wantedDependencies) {
14321434
let reference = undefined as undefined | string
1435+
let preferredVersion = undefined as undefined | string
14331436
let proceed = proceedAll
14341437
if (wantedDependency.alias) {
14351438
const satisfiesWanted = satisfiesWanted2Args.bind(null, wantedDependency)
14361439
if (
14371440
resolvedDependencies[wantedDependency.alias] &&
14381441
(satisfiesWanted(resolvedDependencies[wantedDependency.alias]) || resolvedDependencies[wantedDependency.alias].startsWith('file:'))
14391442
) {
1440-
reference = resolvedDependencies[wantedDependency.alias]
1443+
const pinnedRef = resolvedDependencies[wantedDependency.alias]
1444+
// Reusing a lockfile pin verbatim bypasses the preferred-versions
1445+
// walk, so a transitive edge stays on a stale lower version even
1446+
// when a direct dependency resolved to a higher version in range.
1447+
const pinned = pinnedRef.startsWith('file:')
1448+
? undefined
1449+
: getPinnedNameVer(wantedLockfile, pinnedRef, wantedDependency.alias)
1450+
const higherDirectVersion = pinned == null
1451+
? undefined
1452+
: findHigherDirectDepVersion(options.preferredVersions, pinned.name, pinned.version, wantedDependency.bareSpecifier)
1453+
if (higherDirectVersion != null) {
1454+
// `preferredVersion` (singular) overrides the
1455+
// EXISTING_VERSION_SELECTOR_WEIGHT stability bias that would
1456+
// otherwise re-pick the lower version. The lower version is then
1457+
// never resolved or fetched.
1458+
proceed = true
1459+
preferredVersion = higherDirectVersion
1460+
} else {
1461+
reference = pinnedRef
1462+
}
14411463
} else if (
14421464
// If dependencies that were used by the previous version of the package
14431465
// satisfy the newer version's requirements, then pnpm tries to keep
@@ -1474,6 +1496,7 @@ function getDepsToResolve (
14741496
}
14751497
extendedWantedDeps.push({
14761498
infoFromLockfile,
1499+
preferredVersion,
14771500
proceed,
14781501
wantedDependency,
14791502
})
@@ -1506,6 +1529,51 @@ function referenceSatisfiesWantedSpec (
15061529
return semver.satisfies(version, wantedDep.bareSpecifier, true)
15071530
}
15081531

1532+
function getPinnedNameVer (
1533+
lockfile: LockfileObject,
1534+
reference: string,
1535+
alias: string
1536+
): { name: string, version: string } | undefined {
1537+
const depPath = dp.refToRelative(reference, alias)
1538+
if (depPath === null) return undefined
1539+
const pkgSnapshot = lockfile.packages?.[depPath]
1540+
if (pkgSnapshot == null) return undefined
1541+
return nameVerFromPkgSnapshot(depPath, pkgSnapshot)
1542+
}
1543+
1544+
// The highest version a direct dependency (the only deterministic,
1545+
// resolved-first anchor) resolved to that is higher than the
1546+
// lockfile-pinned `pinnedVersion` and still satisfies the edge's range, or
1547+
// `undefined` when none exists. Direct-dependency versions are marked with
1548+
// DIRECT_DEP_SELECTOR_WEIGHT in preferredVersions.
1549+
function findHigherDirectDepVersion (
1550+
preferredVersions: PreferredVersions | undefined,
1551+
pinnedName: string,
1552+
pinnedVersion: string,
1553+
bareSpecifier: string
1554+
): string | undefined {
1555+
if (preferredVersions == null) return undefined
1556+
if (!semver.valid(pinnedVersion)) return undefined
1557+
if (semver.validRange(bareSpecifier) === null) return undefined
1558+
const selectors = preferredVersions[pinnedName]
1559+
if (selectors == null) return undefined
1560+
let best: string | undefined
1561+
for (const [candidate, selector] of Object.entries(selectors)) {
1562+
if (
1563+
typeof selector === 'object' &&
1564+
selector.selectorType === 'version' &&
1565+
selector.weight === DIRECT_DEP_SELECTOR_WEIGHT &&
1566+
semver.valid(candidate) &&
1567+
semver.gt(candidate, pinnedVersion) &&
1568+
semver.satisfies(candidate, bareSpecifier, true) &&
1569+
(best == null || semver.gt(candidate, best))
1570+
) {
1571+
best = candidate
1572+
}
1573+
}
1574+
return best
1575+
}
1576+
15091577
export interface LockedPeerContext {
15101578
[peerName: string]: DepPath
15111579
}

0 commit comments

Comments
 (0)