Skip to content

Commit 302a2f7

Browse files
fix(config/reader): don't warn when packageManager and devEngines.packageManager match (#12287)
When `package.json` sets both `packageManager` and `devEngines.packageManager` to the same pnpm version with the same integrity hash pnpm prints a spurious warning on every command. For example, a `package.json` file that looks like: ```json { "packageManager": "pnpm@11.5.1+sha512.93f7b57422ea7068257235b4c16eb60762eb68e1dc23723199cc739043ea9be2c4143274a399d8c6defa2b1176226d9ca1c4b63482d6200c1a8fbaa78c1d1485", "devEngines": { "packageManager": { "name": "pnpm", "version": "11.5.1+sha512.93f7b57422ea7068257235b4c16eb60762eb68e1dc23723199cc739043ea9be2c4143274a399d8c6defa2b1176226d9ca1c4b63482d6200c1a8fbaa78c1d1485", "onFail": "ignore" }, "runtime": [ { "name": "node", "version": "26.3.0", "onFail": "ignore" } ] } } ``` Issues a warning on every `pnpm` command: > Cannot use both "packageManager" and "devEngines.packageManager" in package.json. "packageManager" will be ignored. ## Root cause `getWantedPackageManager` compares the two fields to decide whether to warn, but the two sides were normalized differently: - `parsePackageManager` **strips the integrity hash** from the legacy `packageManager` field → `11.5.1` - the `devEngines.packageManager` version was compared **with its hash intact** → `11.5.1+sha512.93f7b57…` So, `"11.5.1" !== "11.5.1+sha512…"` was always true and the warning fired, even for identical specs. An earlier fix in #11307 only suppressed the warning when *neither* side carried a hash. ## Fix `parsePackageManager` now also returns the hash (via a shared `splitPackageManagerVersion`), and `getPackageManagerConflictWarning` compares the fields structurally. The warning is suppressed **only when the two specifiers are identical** (name + version + hash, both-absent counts as equal): | name | version | hash | result | |------|---------|------|--------| | same | same | both absent, or both present & equal | ✅ no warning | | same | same | present on **one side only** | ⚠️ generic "Cannot use both…" | | same | same | both present & **differ** | ⚠️ "…contradictory integrity hashes" | | same | **differ** | — | ⚠️ "…different versions of pnpm" | | **differ** | — | — | ⚠️ "…different package managers" | A hash on only one side is still a divergence — dropping the ignored `packageManager` field would lose that hash — so it warns with the original generic message. Two contradictory hashes for one version (a likely wrong-hash mistake) get a dedicated message. The generic single message is otherwise replaced by one tailored to each conflict, each ending with `"packageManager" will be ignored`. Closes #12028. --------- Signed-off-by: C. Spencer Beggs <spencer@beggs.codes> Signed-off-by: Zoltan Kochan <z@kochan.io> Co-authored-by: Zoltan Kochan <z@kochan.io>
1 parent 817f99d commit 302a2f7

4 files changed

Lines changed: 261 additions & 15 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"@pnpm/config.reader": patch
3+
"pnpm": patch
4+
---
5+
6+
No longer warn about using both `packageManager` and `devEngines.packageManager` when the two fields pin the same package manager at the same version with the same integrity hash (e.g. both `pnpm@11.5.1+sha512.…`). Previously the hash was stripped from the legacy `packageManager` field but not from `devEngines.packageManager`, so even identical specifications looked like a mismatch [#12028](https://github.com/pnpm/pnpm/issues/12028).
7+
8+
The warning still fires on any genuine divergence, and several cases now state the specific reason instead of a single generic message: a different package manager, a different version, or contradictory integrity hashes for the same version.

config/reader/src/index.ts

Lines changed: 89 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import fs from 'node:fs'
22
import os from 'node:os'
33
import path from 'node:path'
4+
import { stripVTControlCharacters } from 'node:util'
45

56
import { getCatalogsFromWorkspaceManifest } from '@pnpm/catalogs.config'
67
import { createMatcher } from '@pnpm/config.matcher'
@@ -793,8 +794,12 @@ function getWantedPackageManager (manifest: ProjectManifest): { pm?: WantedPacka
793794
}
794795
if (manifest.packageManager) {
795796
const legacyPm = parsePackageManager(manifest.packageManager)
796-
if (legacyPm.name !== pmFromDevEngines.name || legacyPm.version !== pmFromDevEngines.version) {
797-
warnings.push('Cannot use both "packageManager" and "devEngines.packageManager" in package.json. "packageManager" will be ignored')
797+
const conflictWarning = getPackageManagerConflictWarning(legacyPm, {
798+
name: pmFromDevEngines.name,
799+
...splitPackageManagerVersion(pmFromDevEngines.version),
800+
})
801+
if (conflictWarning) {
802+
warnings.push(conflictWarning)
798803
}
799804
}
800805
return { pm: { ...pmFromDevEngines, fromDevEngines: true }, warnings }
@@ -848,17 +853,90 @@ function getIgnoredPnpmFieldKeys (manifest: ProjectManifest): string[] {
848853
return Object.keys(legacyField as Record<string, unknown>).filter(k => MIGRATED_PNPM_FIELD_KEYS.has(k))
849854
}
850855

851-
export function parsePackageManager (packageManager: string): { name: string, version: string | undefined } {
852-
if (!packageManager.includes('@')) return { name: packageManager, version: undefined }
853-
const [name, pmReference] = packageManager.split('@')
856+
export interface ParsedPackageManager {
857+
name: string
858+
version: string | undefined
859+
hash: string | undefined
860+
}
861+
862+
export function parsePackageManager (packageManager: string): ParsedPackageManager {
863+
// Split on the `@` that separates the name from the reference. A leading `@`
864+
// belongs to a scoped name (e.g. `@scope/pm@1.2.3`), so skip it; otherwise
865+
// the first `@` is the separator. The first `@` (not the last) is used so a
866+
// reference that is a URL containing `@` (e.g. credentials) stays intact.
867+
const separatorIndex = packageManager.startsWith('@')
868+
? packageManager.indexOf('@', 1)
869+
: packageManager.indexOf('@')
870+
if (separatorIndex === -1) return { name: packageManager, version: undefined, hash: undefined }
871+
const name = packageManager.slice(0, separatorIndex)
872+
const pmReference = packageManager.slice(separatorIndex + 1)
854873
// pmReference is semantic versioning, not URL
855-
if (pmReference.includes(':')) return { name, version: undefined }
856-
// Remove the integrity hash. Ex: "pnpm@9.5.0+sha512.140036830124618d624a2187b50d04289d5a087f326c9edfc0ccd733d76c4f52c3a313d4fc148794a2a9d81553016004e6742e8cf850670268a7387fc220c903"
857-
const [version] = pmReference.split('+')
858-
return {
859-
name,
860-
version,
874+
if (pmReference.includes(':')) return { name, version: undefined, hash: undefined }
875+
return { name, ...splitPackageManagerVersion(pmReference) }
876+
}
877+
878+
/**
879+
* Splits a package manager version reference into its semver part and the
880+
* integrity hash carried as semver build metadata, e.g.
881+
* "9.5.0+sha512.140036830124618d624a2187b50d04289d5a087f326c9edfc0ccd733d76c4f52c3a313d4fc148794a2a9d81553016004e6742e8cf850670268a7387fc220c903"
882+
* becomes `{ version: "9.5.0", hash: "sha512.14003…" }`. A reference without a
883+
* hash yields an undefined hash; an undefined reference yields both undefined.
884+
*/
885+
function splitPackageManagerVersion (reference: string | undefined): { version: string | undefined, hash: string | undefined } {
886+
if (reference == null) return { version: undefined, hash: undefined }
887+
// Split on the first `+` only. The integrity hash is semver build metadata —
888+
// everything after that `+` — and must be preserved whole, so a reference is
889+
// never truncated at a later `+`.
890+
const hashIndex = reference.indexOf('+')
891+
if (hashIndex === -1) return { version: reference, hash: undefined }
892+
return { version: reference.slice(0, hashIndex), hash: reference.slice(hashIndex + 1) }
893+
}
894+
895+
/**
896+
* Describes how the legacy `packageManager` field disagrees with
897+
* `devEngines.packageManager`, or returns undefined when the two specifiers are
898+
* identical (so keeping both fields in sync produces no warning). Any
899+
* divergence warns — including an integrity hash (semver build metadata) on
900+
* only one side, since dropping the ignored `packageManager` field would lose
901+
* it. In every conflict `devEngines.packageManager` wins and `packageManager`
902+
* is ignored.
903+
*/
904+
function getPackageManagerConflictWarning (legacy: ParsedPackageManager, devEngines: ParsedPackageManager): string | undefined {
905+
const ignoredSuffix = '. "packageManager" will be ignored'
906+
const genericWarning = `Cannot use both "packageManager" and "devEngines.packageManager" in package.json${ignoredSuffix}`
907+
if (legacy.name !== devEngines.name) {
908+
return `"packageManager" (${sanitizeManifestValue(legacy.name)}) and "devEngines.packageManager" (${sanitizeManifestValue(devEngines.name)}) specify different package managers in package.json${ignoredSuffix}`
909+
}
910+
if (legacy.version !== devEngines.version) {
911+
// "different versions" only makes sense when both sides are concrete
912+
// versions. If one side has no semver version — e.g. the legacy field is a
913+
// URL or a bare name — fall back to the generic notice rather than claiming
914+
// a version mismatch.
915+
if (legacy.version == null || devEngines.version == null) return genericWarning
916+
return `"packageManager" and "devEngines.packageManager" specify different versions of ${sanitizeManifestValue(legacy.name)} in package.json${ignoredSuffix}`
861917
}
918+
if (legacy.hash !== devEngines.hash) {
919+
// Same name and version, but the integrity hashes differ. Two distinct
920+
// hashes for one version is a likely wrong-hash mistake, so call it out
921+
// specifically; a hash on only one side is a softer mismatch (the version
922+
// still agrees) and gets the generic notice.
923+
if (legacy.hash != null && devEngines.hash != null && legacy.version != null) {
924+
return `"packageManager" and "devEngines.packageManager" specify ${sanitizeManifestValue(legacy.name)}@${sanitizeManifestValue(legacy.version)} with different integrity hashes in package.json${ignoredSuffix}`
925+
}
926+
return genericWarning
927+
}
928+
return undefined
929+
}
930+
931+
/**
932+
* Renders a package.json-controlled value safe to embed in a warning printed to
933+
* the terminal. Strips ANSI escape sequences and replaces remaining control
934+
* characters (including newlines) with spaces so a malicious manifest cannot
935+
* forge or rewrite terminal/CI log output.
936+
*/
937+
function sanitizeManifestValue (value: string): string {
938+
// eslint-disable-next-line no-control-regex
939+
return stripVTControlCharacters(value).replace(/[\u0000-\u001f\u007f]/g, ' ')
862940
}
863941

864942
/**

config/reader/test/index.ts

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { writeYamlFileSync } from 'write-yaml-file'
1313

1414
jest.unstable_mockModule('@pnpm/network.git-utils', () => ({ getCurrentBranch: jest.fn() }))
1515

16-
const { getConfig } = await import('@pnpm/config.reader')
16+
const { getConfig, parsePackageManager } = await import('@pnpm/config.reader')
1717
const { getCurrentBranch } = await import('@pnpm/network.git-utils')
1818

1919
// To override any local settings,
@@ -207,6 +207,109 @@ test('devEngines.packageManager with explicit onFail is respected (regression gu
207207
expect(context.wantedPackageManager?.onFail).toBe('error')
208208
})
209209

210+
describe('"packageManager" / "devEngines.packageManager" conflict warning', () => {
211+
const HASH_A = 'sha512.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
212+
const HASH_B = 'sha512.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'
213+
const IGNORED = '. "packageManager" will be ignored'
214+
215+
async function warningsFor (packageManager: string, devEnginesVersion: string): Promise<string[]> {
216+
prepare({
217+
packageManager,
218+
devEngines: {
219+
packageManager: { name: 'pnpm', version: devEnginesVersion, onFail: 'ignore' },
220+
},
221+
})
222+
const { warnings } = await getConfig({
223+
cliOptions: {},
224+
packageManager: { name: 'pnpm', version: '1.2.3' },
225+
})
226+
return warnings
227+
}
228+
229+
test.each([
230+
{ caseName: 'same version without a hash', packageManager: 'pnpm@1.2.3', devEnginesVersion: '1.2.3' },
231+
{ caseName: 'same version with the same hash', packageManager: `pnpm@1.2.3+${HASH_A}`, devEnginesVersion: `1.2.3+${HASH_A}` },
232+
])('does not warn when the specifiers are identical: $caseName', async ({ packageManager, devEnginesVersion }) => {
233+
const warnings = await warningsFor(packageManager, devEnginesVersion)
234+
expect(warnings.some(w => w.includes(IGNORED))).toBe(false)
235+
})
236+
237+
test.each([
238+
{ caseName: 'hash on the legacy field only', packageManager: `pnpm@1.2.3+${HASH_A}`, devEnginesVersion: '1.2.3' },
239+
{ caseName: 'hash on the devEngines field only', packageManager: 'pnpm@1.2.3', devEnginesVersion: `1.2.3+${HASH_A}` },
240+
])('warns generically when the integrity hash is present on only one field: $caseName', async ({ packageManager, devEnginesVersion }) => {
241+
const warnings = await warningsFor(packageManager, devEnginesVersion)
242+
expect(warnings).toContain(`Cannot use both "packageManager" and "devEngines.packageManager" in package.json${IGNORED}`)
243+
})
244+
245+
test('warns about contradictory integrity hashes for the same version', async () => {
246+
const warnings = await warningsFor(`pnpm@1.2.3+${HASH_A}`, `1.2.3+${HASH_B}`)
247+
expect(warnings).toContain(`"packageManager" and "devEngines.packageManager" specify pnpm@1.2.3 with different integrity hashes in package.json${IGNORED}`)
248+
})
249+
250+
test.each([
251+
{ caseName: 'the legacy field is a URL reference', packageManager: 'pnpm@https://github.com/pnpm/pnpm' },
252+
{ caseName: 'the legacy field is a bare name with no version', packageManager: 'pnpm' },
253+
])('warns generically rather than claiming a version mismatch when one side is not a concrete version: $caseName', async ({ packageManager }) => {
254+
const warnings = await warningsFor(packageManager, '1.2.3')
255+
expect(warnings).toContain(`Cannot use both "packageManager" and "devEngines.packageManager" in package.json${IGNORED}`)
256+
expect(warnings.some(w => w.includes('different versions'))).toBe(false)
257+
})
258+
259+
test.each([
260+
{ caseName: 'different exact versions', devEnginesVersion: '1.2.4' },
261+
{ caseName: 'exact version versus a range', devEnginesVersion: '>=1.0.0' },
262+
])('warns about a version mismatch: $caseName', async ({ devEnginesVersion }) => {
263+
const warnings = await warningsFor('pnpm@1.2.3', devEnginesVersion)
264+
expect(warnings).toContain(`"packageManager" and "devEngines.packageManager" specify different versions of pnpm in package.json${IGNORED}`)
265+
})
266+
267+
test('warns when the fields name different package managers', async () => {
268+
prepare({
269+
packageManager: 'yarn@1.2.3',
270+
devEngines: {
271+
packageManager: { name: 'pnpm', version: '1.2.3', onFail: 'ignore' },
272+
},
273+
})
274+
const { warnings } = await getConfig({
275+
cliOptions: {},
276+
packageManager: { name: 'pnpm', version: '1.2.3' },
277+
})
278+
expect(warnings).toContain(`"packageManager" (yarn) and "devEngines.packageManager" (pnpm) specify different package managers in package.json${IGNORED}`)
279+
})
280+
281+
test('strips control characters from package.json values embedded in the warning', async () => {
282+
prepare({
283+
packageManager: 'ev\u001b[31mi\nl@1.2.3',
284+
devEngines: {
285+
packageManager: { name: 'pnpm', version: '1.2.3', onFail: 'ignore' },
286+
},
287+
})
288+
const { warnings } = await getConfig({
289+
cliOptions: {},
290+
packageManager: { name: 'pnpm', version: '1.2.3' },
291+
})
292+
const warning = warnings.find(w => w.includes('different package managers'))
293+
expect(warning).toBeDefined()
294+
// eslint-disable-next-line no-control-regex
295+
expect(warning).not.toMatch(/[\u0000-\u001f\u007f]/)
296+
expect(warning).toContain('"packageManager" (evi l)')
297+
})
298+
})
299+
300+
describe('parsePackageManager', () => {
301+
test.each([
302+
{ input: 'pnpm@9.5.0', expected: { name: 'pnpm', version: '9.5.0', hash: undefined } },
303+
{ input: 'pnpm@9.5.0+sha512.abc123', expected: { name: 'pnpm', version: '9.5.0', hash: 'sha512.abc123' } },
304+
{ input: 'pnpm@9.5.0+a+b', expected: { name: 'pnpm', version: '9.5.0', hash: 'a+b' } },
305+
{ input: 'pnpm', expected: { name: 'pnpm', version: undefined, hash: undefined } },
306+
{ input: '@scope/pm@1.2.3', expected: { name: '@scope/pm', version: '1.2.3', hash: undefined } },
307+
{ input: 'pnpm@https://github.com/pnpm/pnpm', expected: { name: 'pnpm', version: undefined, hash: undefined } },
308+
])('parses $input', ({ input, expected }) => {
309+
expect(parsePackageManager(input)).toEqual(expected)
310+
})
311+
})
312+
210313
test('throw error if --link-workspace-packages is used with --global', async () => {
211314
await expect(getConfig({
212315
cliOptions: {

pnpm/test/packageManagerCheck.test.ts

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,62 @@ test('no warning when packageManager and devEngines.packageManager specify the s
506506

507507
const { stderr } = execPnpmSync(['install'])
508508

509-
expect(stderr.toString()).not.toContain('Cannot use both')
509+
expect(stderr.toString()).not.toContain('"packageManager" will be ignored')
510+
})
511+
512+
test('no warning when packageManager and devEngines.packageManager specify the same version with the same integrity hash', async () => {
513+
const hash = 'sha512.93f7b57422ea7068257235b4c16eb60762eb68e1dc23723199cc739043ea9be2c4143274a399d8c6defa2b1176226d9ca1c4b63482d6200c1a8fbaa78c1d1485'
514+
prepare({
515+
packageManager: `pnpm@1.2.3+${hash}`,
516+
devEngines: {
517+
packageManager: {
518+
name: 'pnpm',
519+
version: `1.2.3+${hash}`,
520+
onFail: 'ignore',
521+
},
522+
},
523+
})
524+
525+
const { stderr } = execPnpmSync(['install'])
526+
527+
expect(stderr.toString()).not.toContain('"packageManager" will be ignored')
528+
})
529+
530+
test('warns when packageManager and devEngines.packageManager pin the same version but different integrity hashes', async () => {
531+
prepare({
532+
packageManager: 'pnpm@1.2.3+sha512.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
533+
devEngines: {
534+
packageManager: {
535+
name: 'pnpm',
536+
version: '1.2.3+sha512.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb',
537+
onFail: 'ignore',
538+
},
539+
},
540+
})
541+
542+
const { stderr } = execPnpmSync(['install'])
543+
544+
expect(stderr.toString()).toContain('different integrity hashes')
545+
expect(stderr.toString()).toContain('"packageManager" will be ignored')
546+
})
547+
548+
test('warns when the integrity hash is present on packageManager but not devEngines.packageManager', async () => {
549+
const hash = 'sha512.93f7b57422ea7068257235b4c16eb60762eb68e1dc23723199cc739043ea9be2c4143274a399d8c6defa2b1176226d9ca1c4b63482d6200c1a8fbaa78c1d1485'
550+
prepare({
551+
packageManager: `pnpm@1.2.3+${hash}`,
552+
devEngines: {
553+
packageManager: {
554+
name: 'pnpm',
555+
version: '1.2.3',
556+
onFail: 'ignore',
557+
},
558+
},
559+
})
560+
561+
const { stderr } = execPnpmSync(['install'])
562+
563+
expect(stderr.toString()).toContain('Cannot use both "packageManager" and "devEngines.packageManager"')
564+
expect(stderr.toString()).toContain('"packageManager" will be ignored')
510565
})
511566

512567
test('warns when packageManager specifies a different package manager from devEngines.packageManager', async () => {
@@ -523,7 +578,8 @@ test('warns when packageManager specifies a different package manager from devEn
523578

524579
const { stderr } = execPnpmSync(['install'])
525580

526-
expect(stderr.toString()).toContain('Cannot use both "packageManager" and "devEngines.packageManager"')
581+
expect(stderr.toString()).toContain('specify different package managers')
582+
expect(stderr.toString()).toContain('"packageManager" will be ignored')
527583
})
528584

529585
test('warns when packageManager version does not match the devEngines.packageManager version string exactly', async () => {
@@ -540,7 +596,8 @@ test('warns when packageManager version does not match the devEngines.packageMan
540596

541597
const { stderr } = execPnpmSync(['install'])
542598

543-
expect(stderr.toString()).toContain('Cannot use both "packageManager" and "devEngines.packageManager"')
599+
expect(stderr.toString()).toContain('specify different versions of pnpm')
600+
expect(stderr.toString()).toContain('"packageManager" will be ignored')
544601
})
545602

546603
test('pmOnFail=ignore via env var bypasses the devEngines.packageManager check', async () => {

0 commit comments

Comments
 (0)