From b36c7f0c09ef5189079d0c2e7fdd619f03cb0fee Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 8 Feb 2025 19:21:57 +0100 Subject: [PATCH 01/17] feat: add limited support for `devEngines` --- sources/commands/Base.ts | 2 +- sources/specUtils.ts | 51 ++++++++++++++++- tests/Up.test.ts | 58 ++++++++++++++----- tests/main.test.ts | 119 ++++++++++++++++++++++++++++++++++++--- 4 files changed, 204 insertions(+), 26 deletions(-) diff --git a/sources/commands/Base.ts b/sources/commands/Base.ts index ef7c0e4c8..ca0550129 100644 --- a/sources/commands/Base.ts +++ b/sources/commands/Base.ts @@ -19,7 +19,7 @@ export abstract class BaseCommand extends Command { throw new UsageError(`The local project doesn't feature a 'packageManager' field - please explicit the package manager to pack, or update the manifest to reference it`); default: { - return [lookup.spec]; + return [lookup.range ?? lookup.spec]; } } } diff --git a/sources/specUtils.ts b/sources/specUtils.ts index a75b10ffd..1fe6fef18 100644 --- a/sources/specUtils.ts +++ b/sources/specUtils.ts @@ -1,6 +1,7 @@ import {UsageError} from 'clipanion'; import fs from 'fs'; import path from 'path'; +import semverSatisfies from 'semver/functions/satisfies'; import semverValid from 'semver/functions/valid'; import {PreparedPackageManagerInfo} from './Engine'; @@ -52,6 +53,46 @@ export function parseSpec(raw: unknown, source: string, {enforceExactVersion = t }; } +type CorepackPackageJSON = { + packageManager?: string; + devEngines?: { packageManager?: DevEngineDependency }; +}; + +interface DevEngineDependency { + name: string; + version: string; +} +function parsePackageJSON(packageJSONContent: CorepackPackageJSON) { + if (packageJSONContent.devEngines?.packageManager) { + const {packageManager} = packageJSONContent.devEngines; + + if (Array.isArray(packageManager)) + throw new UsageError(`Providing several package managers is currently not supported`); + + const {version} = packageManager; + if (!version) + throw new UsageError(`Providing no version nor ranger for package manager is currently not supported`); + + debugUtils.log(`devEngines defines that ${packageManager.name}@${version} is the local package manager`); + + const {packageManager: pm} = packageJSONContent; + if (pm) { + if (!pm.startsWith(`${packageManager.name}@`)) + throw new UsageError(`"packageManager" field is set to ${JSON.stringify(pm)} which does not match the "devEngines.packageManager" field set to ${JSON.stringify(packageManager.name)}`); + + if (!semverSatisfies(pm.slice(packageManager.name.length + 1), version)) + throw new UsageError(`"packageManager" field is set to ${JSON.stringify(pm)} which does not match the value defined in "devEngines.packageManager" for ${JSON.stringify(packageManager.name)} of ${JSON.stringify(version)}`); + + return pm; + } + + + return `${packageManager.name}@${version}`; + } + + return packageJSONContent.packageManager; +} + export async function setLocalPackageManager(cwd: string, info: PreparedPackageManagerInfo) { const lookup = await loadSpec(cwd); @@ -75,7 +116,7 @@ export async function setLocalPackageManager(cwd: string, info: PreparedPackageM export type LoadSpecResult = | {type: `NoProject`, target: string} | {type: `NoSpec`, target: string} - | {type: `Found`, target: string, spec: Descriptor}; + | {type: `Found`, target: string, spec: Descriptor, range?: Descriptor}; export async function loadSpec(initialCwd: string): Promise { let nextCwd = initialCwd; @@ -117,13 +158,17 @@ export async function loadSpec(initialCwd: string): Promise { if (selection === null) return {type: `NoProject`, target: path.join(initialCwd, `package.json`)}; - const rawPmSpec = selection.data.packageManager; + const rawPmSpec = parsePackageJSON(selection.data); if (typeof rawPmSpec === `undefined`) return {type: `NoSpec`, target: selection.manifestPath}; + debugUtils.log(`${selection.manifestPath} defines ${rawPmSpec} as local package manager`); + + const spec = parseSpec(rawPmSpec, path.relative(initialCwd, selection.manifestPath)); return { type: `Found`, target: selection.manifestPath, - spec: parseSpec(rawPmSpec, path.relative(initialCwd, selection.manifestPath)), + spec, + range: selection.data.devEngines?.packageManager?.version && {...spec, range: selection.data.devEngines.packageManager.version}, }; } diff --git a/tests/Up.test.ts b/tests/Up.test.ts index d0d102400..fa6045c4e 100644 --- a/tests/Up.test.ts +++ b/tests/Up.test.ts @@ -11,24 +11,54 @@ beforeEach(async () => { }); describe(`UpCommand`, () => { - it(`should upgrade the package manager from the current project`, async () => { - await xfs.mktempPromise(async cwd => { - await xfs.writeJsonPromise(ppath.join(cwd, `package.json`), { - packageManager: `yarn@2.1.0`, - }); + describe(`should update the "packageManager" field from the current project`, () => { + it(`to the same major if no devEngines range`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json`), { + packageManager: `yarn@2.1.0`, + }); - await expect(runCli(cwd, [`up`])).resolves.toMatchObject({ - exitCode: 0, - stderr: ``, - }); + await expect(runCli(cwd, [`up`])).resolves.toMatchObject({ + exitCode: 0, + stderr: ``, + }); - await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ - packageManager: `yarn@2.4.3+sha512.8dd9fedc5451829619e526c56f42609ad88ae4776d9d3f9456d578ac085115c0c2f0fb02bb7d57fd2e1b6e1ac96efba35e80a20a056668f61c96934f67694fd0`, + await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ + packageManager: `yarn@2.4.3+sha512.8dd9fedc5451829619e526c56f42609ad88ae4776d9d3f9456d578ac085115c0c2f0fb02bb7d57fd2e1b6e1ac96efba35e80a20a056668f61c96934f67694fd0`, + }); + + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stdout: `2.4.3\n`, + }); }); + }); + + it(`to whichever range devEngines defines`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json`), { + packageManager: `yarn@1.1.0`, + devEngines: { + packageManager: { + name: `yarn`, + version: `1.x || 2.x`, + }, + }, + }); + + await expect(runCli(cwd, [`up`])).resolves.toMatchObject({ + exitCode: 0, + stderr: ``, + }); + + await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ + packageManager: `yarn@2.4.3+sha512.8dd9fedc5451829619e526c56f42609ad88ae4776d9d3f9456d578ac085115c0c2f0fb02bb7d57fd2e1b6e1ac96efba35e80a20a056668f61c96934f67694fd0`, + }); - await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ - exitCode: 0, - stdout: `2.4.3\n`, + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stdout: `2.4.3\n`, + }); }); }); }); diff --git a/tests/main.test.ts b/tests/main.test.ts index 92cc0e687..fca4477cc 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -16,16 +16,33 @@ beforeEach(async () => { process.env.COREPACK_DEFAULT_TO_LATEST = `0`; }); -it(`should refuse to download a package manager if the hash doesn't match`, async () => { - await xfs.mktempPromise(async cwd => { - await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), { - packageManager: `yarn@1.22.4+sha1.deadbeef`, +describe(`should refuse to download a package manager if the hash doesn't match`, () => { + it(`the one defined in "devEngines.packageManager" field`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), { + devEngines: { + packageManager: {name: `yarn`, version: `1.22.4+sha1.deadbeef`}, + }, + }); + + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 1, + stderr: expect.stringContaining(`Mismatch hashes`), + stdout: ``, + }); }); + }); + it(`the one defined in "packageManager" field`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), { + packageManager: `yarn@1.22.4+sha1.deadbeef`, + }); - await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ - exitCode: 1, - stderr: /Mismatch hashes/, - stdout: ``, + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 1, + stderr: expect.stringContaining(`Mismatch hashes`), + stdout: ``, + }); }); }); }); @@ -150,6 +167,16 @@ for (const [name, version, expectedVersion = version.split(`+`, 1)[0]] of tested stderr: ``, stdout: `${expectedVersion}\n`, }); + + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), { + devEngines: {packageManager: {name, version}}, + }); + + await expect(runCli(cwd, [name, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stderr: ``, + stdout: `${expectedVersion}\n`, + }); }); }); } @@ -231,6 +258,82 @@ it(`should ignore the packageManager field when found within a node_modules vend }); }); +it(`should use hash from "packageManager" even when "devEngines" defines a different one`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + packageManager: `yarn@3.0.0-rc.2+sha1.11111`, + devEngines: { + packageManager: { + name: `yarn`, + version: `3.0.0-rc.2+sha1.22222`, + }, + }, + }); + + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 1, + stderr: expect.stringContaining(`Mismatch hashes. Expected 11111, got`), + stdout: ``, + }); + }); +}); + +describe(`should accept range in devEngines only if a specific version is provided`, () => { + it(`either in package.json#packageManager field`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + devEngines: { + packageManager: { + name: `pnpm`, + version: `6.x`, + }, + }, + }); + await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({ + exitCode: 1, + stderr: `Invalid package manager specification in package.json (pnpm@6.x); expected a semver version\n`, + stdout: ``, + }); + + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + devEngines: { + packageManager: { + name: `pnpm`, + version: `6.x`, + }, + }, + packageManager: `pnpm@6.6.2+sha224.eb5c0acad3b0f40ecdaa2db9aa5a73134ad256e17e22d1419a2ab073`, + }); + await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stderr: ``, + stdout: `6.6.2\n`, + }); + }); + }); +}); + +describe(`should reject if range in devEngines does not match version provided`, () => { + it(`in package.json#packageManager field`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + devEngines: { + packageManager: { + name: `pnpm`, + version: `10.x`, + }, + }, + packageManager: `pnpm@6.6.2+sha1.7b4d6b176c1b93b5670ed94c24babb7d80c13854`, + }); + await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({ + exitCode: 1, + stderr: `"packageManager" field is set to "pnpm@6.6.2+sha1.7b4d6b176c1b93b5670ed94c24babb7d80c13854" which does not match the value defined in "devEngines.packageManager" for "pnpm" of "10.x"\n`, + stdout: ``, + }); + }); + }); +}); + it(`should use the closest matching packageManager field`, async () => { await xfs.mktempPromise(async cwd => { await xfs.mkdirPromise(ppath.join(cwd, `foo` as PortablePath), {recursive: true}); From 073440e59a3189aa43cce796d397e5471de38ba6 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 9 Feb 2025 08:13:37 +0100 Subject: [PATCH 02/17] Update error message Co-authored-by: Geoffrey Booth --- sources/specUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sources/specUtils.ts b/sources/specUtils.ts index 1fe6fef18..33bdbdf78 100644 --- a/sources/specUtils.ts +++ b/sources/specUtils.ts @@ -71,7 +71,7 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) { const {version} = packageManager; if (!version) - throw new UsageError(`Providing no version nor ranger for package manager is currently not supported`); + throw new UsageError(`Package manager version or version range is required`); debugUtils.log(`devEngines defines that ${packageManager.name}@${version} is the local package manager`); From e9f6adfd0621712af5b93eef905439e4e97ea696 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 10 Feb 2025 11:55:28 +0100 Subject: [PATCH 03/17] change throw into warn --- sources/specUtils.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sources/specUtils.ts b/sources/specUtils.ts index 33bdbdf78..45d1f29e9 100644 --- a/sources/specUtils.ts +++ b/sources/specUtils.ts @@ -66,14 +66,16 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) { if (packageJSONContent.devEngines?.packageManager) { const {packageManager} = packageJSONContent.devEngines; - if (Array.isArray(packageManager)) - throw new UsageError(`Providing several package managers is currently not supported`); + if (Array.isArray(packageManager)) { + console.warn(`! Corepack does not currently supported array values for devEngines.packageManager`); + return packageJSONContent.packageManager; + } const {version} = packageManager; if (!version) throw new UsageError(`Package manager version or version range is required`); - debugUtils.log(`devEngines defines that ${packageManager.name}@${version} is the local package manager`); + debugUtils.log(`devEngines.packageManager defines that ${packageManager.name}@${version} is the local package manager`); const {packageManager: pm} = packageJSONContent; if (pm) { From 675452131755a6074cb903ab9d2b0c9ee1794d40 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 10 Feb 2025 12:17:10 +0100 Subject: [PATCH 04/17] increase coverage and improve error path --- sources/specUtils.ts | 13 +++++-- tests/main.test.ts | 89 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 4 deletions(-) diff --git a/sources/specUtils.ts b/sources/specUtils.ts index 45d1f29e9..e7dd67690 100644 --- a/sources/specUtils.ts +++ b/sources/specUtils.ts @@ -3,6 +3,7 @@ import fs from 'fs'; import path from 'path'; import semverSatisfies from 'semver/functions/satisfies'; import semverValid from 'semver/functions/valid'; +import semverValidRange from 'semver/ranges/valid'; import {PreparedPackageManagerInfo} from './Engine'; import * as debugUtils from './debugUtils'; @@ -63,17 +64,21 @@ interface DevEngineDependency { version: string; } function parsePackageJSON(packageJSONContent: CorepackPackageJSON) { - if (packageJSONContent.devEngines?.packageManager) { + if (packageJSONContent.devEngines?.packageManager != null) { const {packageManager} = packageJSONContent.devEngines; + if (typeof packageManager !== `object`) { + console.warn(`! Corepack only supports objects as valid value for devEngines.packageManager. The current value (${JSON.stringify(packageManager)}) will be ignored.`); + return packageJSONContent.packageManager; + } if (Array.isArray(packageManager)) { - console.warn(`! Corepack does not currently supported array values for devEngines.packageManager`); + console.warn(`! Corepack does not currently support array values for devEngines.packageManager`); return packageJSONContent.packageManager; } const {version} = packageManager; - if (!version) - throw new UsageError(`Package manager version or version range is required`); + if (!version || !semverValidRange(version)) + throw new UsageError(`Version or version range is required in packageManager.devEngines.version`); debugUtils.log(`devEngines.packageManager defines that ${packageManager.name}@${version} is the local package manager`); diff --git a/tests/main.test.ts b/tests/main.test.ts index fca4477cc..6df094100 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -258,6 +258,95 @@ it(`should ignore the packageManager field when found within a node_modules vend }); }); +describe(`should handle invalid devEngines values`, () => { + it(`throw on missing version`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + devEngines: { + packageManager: { + name: `yarn`, + }, + }, + }); + + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 1, + stderr: expect.stringContaining(`Version or version range is required in packageManager.devEngines.version`), + stdout: ``, + }); + }); + }); + it(`throw on invalid version`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + devEngines: { + packageManager: { + name: `yarn`, + version: `yarn@1.x`, + }, + }, + }); + + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 1, + stderr: expect.stringContaining(`Version or version range is required in packageManager.devEngines.version`), + stdout: ``, + }); + }); + }); + it(`warn on array values`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + packageManager: `yarn@1.22.4+sha1.01c1197ca5b27f21edc8bc472cd4c8ce0e5a470e`, + devEngines: { + packageManager: [{ + name: `pnpm`, + version: `10.x`, + }, + ]}, + }); + + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stderr: `! Corepack does not currently support array values for devEngines.packageManager\n`, + stdout: `1.22.4\n`, + }); + }); + }); + it(`warn on string values`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + packageManager: `yarn@1.22.4+sha1.01c1197ca5b27f21edc8bc472cd4c8ce0e5a470e`, + devEngines: { + packageManager: `pnpm@10.x`, + }, + }); + + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stderr: `! Corepack only supports objects as valid value for devEngines.packageManager. The current value ("pnpm@10.x") will be ignored.\n`, + stdout: `1.22.4\n`, + }); + }); + }); + it(`warn on number values`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + packageManager: `yarn@1.22.4+sha1.01c1197ca5b27f21edc8bc472cd4c8ce0e5a470e`, + devEngines: { + packageManager: 10, + }, + }); + + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stderr: `! Corepack only supports objects as valid value for devEngines.packageManager. The current value (10) will be ignored.\n`, + stdout: `1.22.4\n`, + }); + }); + }); +}); + it(`should use hash from "packageManager" even when "devEngines" defines a different one`, async () => { await xfs.mktempPromise(async cwd => { await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { From 32cca551374efa00be1236ad756fcb08f9136f3c Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 12 Feb 2025 11:32:42 +0100 Subject: [PATCH 05/17] fix test when version is a URL --- tests/main.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/main.test.ts b/tests/main.test.ts index 9bd0b4ef2..de62b8701 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -172,7 +172,11 @@ for (const [name, version, expectedVersion = version.split(`+`, 1)[0]] of tested devEngines: {packageManager: {name, version}}, }); - await expect(runCli(cwd, [name, `--version`])).resolves.toMatchObject({ + await expect(runCli(cwd, [name, `--version`])).resolves.toMatchObject(URL.canParse(version) ? { + exitCode: 1, + stderr: `Version or version range is required in packageManager.devEngines.version\n`, + stdout: ``, + } : { exitCode: 0, stderr: ``, stdout: `${expectedVersion}\n`, From 8a53d7de8e4d6e566a7f405582ef988e04bfc6ed Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 15 Feb 2025 19:38:18 +0100 Subject: [PATCH 06/17] add limited support for `onFail` --- sources/specUtils.ts | 41 +++++++++++++++++++++++++++++++---------- tests/Up.test.ts | 29 +++++++++++++++++++++++++++++ tests/main.test.ts | 40 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 99 insertions(+), 11 deletions(-) diff --git a/sources/specUtils.ts b/sources/specUtils.ts index e7dd67690..7f53cc2fe 100644 --- a/sources/specUtils.ts +++ b/sources/specUtils.ts @@ -62,6 +62,18 @@ type CorepackPackageJSON = { interface DevEngineDependency { name: string; version: string; + onFail?: 'ignore' | 'warn' | 'error'; +} +function warnOrThrow(errorMessage: string, onFail?: DevEngineDependency['onFail']) { + switch (onFail) { + case `ignore`: + break; + case `error`: + case undefined: + throw new UsageError(errorMessage); + default: + console.warn(`! Corepack validation warning: ${errorMessage}`); + } } function parsePackageJSON(packageJSONContent: CorepackPackageJSON) { if (packageJSONContent.devEngines?.packageManager != null) { @@ -76,25 +88,31 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) { return packageJSONContent.packageManager; } - const {version} = packageManager; - if (!version || !semverValidRange(version)) - throw new UsageError(`Version or version range is required in packageManager.devEngines.version`); + const {name, version, onFail} = packageManager; + if (typeof name !== `string` || name.includes(`@`)) { + warnOrThrow(`The value of devEngines.packageManager.name ${JSON.stringify(name)} is not a supported string value`, onFail); + return packageJSONContent.packageManager; + } + if (version != null && (typeof version !== `string` || !semverValidRange(version))) { + warnOrThrow(`The value of devEngines.packageManager.version ${JSON.stringify(version)} is not a valid semver range`, onFail); + return packageJSONContent.packageManager; + } - debugUtils.log(`devEngines.packageManager defines that ${packageManager.name}@${version} is the local package manager`); + debugUtils.log(`devEngines.packageManager defines that ${name}@${version} is the local package manager`); const {packageManager: pm} = packageJSONContent; if (pm) { - if (!pm.startsWith(`${packageManager.name}@`)) - throw new UsageError(`"packageManager" field is set to ${JSON.stringify(pm)} which does not match the "devEngines.packageManager" field set to ${JSON.stringify(packageManager.name)}`); + if (!pm.startsWith(`${name}@`)) + warnOrThrow(`"packageManager" field is set to ${JSON.stringify(pm)} which does not match the "devEngines.packageManager" field set to ${JSON.stringify(name)}`, onFail); - if (!semverSatisfies(pm.slice(packageManager.name.length + 1), version)) - throw new UsageError(`"packageManager" field is set to ${JSON.stringify(pm)} which does not match the value defined in "devEngines.packageManager" for ${JSON.stringify(packageManager.name)} of ${JSON.stringify(version)}`); + else if (version != null && !semverSatisfies(pm.slice(packageManager.name.length + 1), version)) + warnOrThrow(`"packageManager" field is set to ${JSON.stringify(pm)} which does not match the value defined in "devEngines.packageManager" for ${JSON.stringify(name)} of ${JSON.stringify(version)}`, onFail); return pm; } - return `${packageManager.name}@${version}`; + return `${name}@${version ?? `*`}`; } return packageJSONContent.packageManager; @@ -176,6 +194,9 @@ export async function loadSpec(initialCwd: string): Promise { type: `Found`, target: selection.manifestPath, spec, - range: selection.data.devEngines?.packageManager?.version && {...spec, range: selection.data.devEngines.packageManager.version}, + range: selection.data.devEngines?.packageManager?.version && { + name: selection.data.devEngines.packageManager.name, + range: selection.data.devEngines.packageManager.version, + }, }; } diff --git a/tests/Up.test.ts b/tests/Up.test.ts index fa6045c4e..09e75c41d 100644 --- a/tests/Up.test.ts +++ b/tests/Up.test.ts @@ -61,5 +61,34 @@ describe(`UpCommand`, () => { }); }); }); + + it(`to whichever range devEngines defines even if onFail is set to ignore`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json`), { + packageManager: `pnpm@10.1.0`, + devEngines: { + packageManager: { + name: `yarn`, + version: `1.x || 2.x`, + onFail: `ignore`, + }, + }, + }); + + await expect(runCli(cwd, [`up`])).resolves.toMatchObject({ + exitCode: 0, + stderr: ``, + }); + + await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ + packageManager: `yarn@2.4.3+sha512.8dd9fedc5451829619e526c56f42609ad88ae4776d9d3f9456d578ac085115c0c2f0fb02bb7d57fd2e1b6e1ac96efba35e80a20a056668f61c96934f67694fd0`, + }); + + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stdout: `2.4.3\n`, + }); + }); + }); }); }); diff --git a/tests/main.test.ts b/tests/main.test.ts index de62b8701..ad2dacd79 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -174,7 +174,7 @@ for (const [name, version, expectedVersion = version.split(`+`, 1)[0]] of tested await expect(runCli(cwd, [name, `--version`])).resolves.toMatchObject(URL.canParse(version) ? { exitCode: 1, - stderr: `Version or version range is required in packageManager.devEngines.version\n`, + stderr: expect.stringMatching(/^The value of devEngines\.packageManager\.version ".+" is not a valid semver range\n$/), stdout: ``, } : { exitCode: 0, @@ -407,6 +407,44 @@ describe(`should accept range in devEngines only if a specific version is provid }); describe(`should reject if range in devEngines does not match version provided`, () => { + it(`unless onFail is set to "ignore"`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + devEngines: { + packageManager: { + name: `pnpm`, + version: `10.x`, + onFail: `ignore`, + }, + }, + packageManager: `pnpm@6.6.2+sha1.7b4d6b176c1b93b5670ed94c24babb7d80c13854`, + }); + await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stderr: ``, + stdout: `6.6.2\n`, + }); + }); + }); + it(`unless onFail is set to "warn"`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + devEngines: { + packageManager: { + name: `pnpm`, + version: `10.x`, + onFail: `warn`, + }, + }, + packageManager: `pnpm@6.6.2+sha1.7b4d6b176c1b93b5670ed94c24babb7d80c13854`, + }); + await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stderr: `! Corepack validation warning: "packageManager" field is set to "pnpm@6.6.2+sha1.7b4d6b176c1b93b5670ed94c24babb7d80c13854" which does not match the value defined in "devEngines.packageManager" for "pnpm" of "10.x"\n`, + stdout: `6.6.2\n`, + }); + }); + }); it(`in package.json#packageManager field`, async () => { await xfs.mktempPromise(async cwd => { await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { From 21f9fecbf456b03d647911f564e4e1bceea5e3d8 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 15 Feb 2025 19:38:23 +0100 Subject: [PATCH 07/17] docs --- README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/README.md b/README.md index fea1e2c8f..be21dd4e9 100644 --- a/README.md +++ b/README.md @@ -113,6 +113,19 @@ use in the archive). } ``` +#### `devEngines.packageManager` + +When a `devEngines.packageManager` field is defined, and is an object containing +a `"name"` field (can also optionally contain `version` and `onFail` fields), +Corepack will use it to validate you're using a compatible package manager. + +Depending on the value of `devEngines.packageManager.onFail`: + +- if set to `ignore`, Corepack won't print any warning or error. +- if unset or set to `error`, Corepack will throw an error in case of a mismatch. +- if set to `warn` or some other value, Corepack will print a warning in case + of mismatch. + ## Known Good Releases When running Corepack within projects that don't list a supported package @@ -246,6 +259,7 @@ it. Unlike `corepack use` this command doesn't take a package manager name nor a version range, as it will always select the latest available version from the +range specified in `devEngines.packageManager.version`, or fallback to the same major line. Should you need to upgrade to a new major, use an explicit `corepack use {name}@latest` call (or simply `corepack use {name}`). From 54be2977cde9557c0bffb64ff312053042503e4a Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 15 Feb 2025 19:49:56 +0100 Subject: [PATCH 08/17] doc what happens when only `devEngines` is defined --- README.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/README.md b/README.md index be21dd4e9..308b80632 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,22 @@ Depending on the value of `devEngines.packageManager.onFail`: - if set to `warn` or some other value, Corepack will print a warning in case of mismatch. +If the top-level `packageManager` field is missing, Corepack will use the +package manager defined in `devEngines.packageManager` – in which case you must +provide a specific version in `devEngines.packageManager.version`, ideally with +a hash, as explained in the previous section: + +```json +{ + "devEngines":{ + "packageManager": { + "name": "yarn", + "version": "3.2.3+sha224.953c8233f7a92884eee2de69a1b92d1f2ec1655e66d08071ba9a02fa" + } + } +} +``` + ## Known Good Releases When running Corepack within projects that don't list a supported package From bf09e9a2fec765a9665b3b68da22d6b9a517ab0c Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 15 Feb 2025 19:58:52 +0100 Subject: [PATCH 09/17] fix tests --- tests/main.test.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/tests/main.test.ts b/tests/main.test.ts index ad2dacd79..1d7225681 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -275,7 +275,7 @@ describe(`should handle invalid devEngines values`, () => { await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ exitCode: 1, - stderr: expect.stringContaining(`Version or version range is required in packageManager.devEngines.version`), + stderr: `Invalid package manager specification in package.json (yarn@*); expected a semver version\n`, stdout: ``, }); }); @@ -293,7 +293,7 @@ describe(`should handle invalid devEngines values`, () => { await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ exitCode: 1, - stderr: expect.stringContaining(`Version or version range is required in packageManager.devEngines.version`), + stderr: `The value of devEngines.packageManager.version "yarn@1.x" is not a valid semver range\n`, stdout: ``, }); }); @@ -402,6 +402,21 @@ describe(`should accept range in devEngines only if a specific version is provid stderr: ``, stdout: `6.6.2\n`, }); + + // No version should also work + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + devEngines: { + packageManager: { + name: `pnpm`, + }, + }, + packageManager: `pnpm@6.6.2+sha224.eb5c0acad3b0f40ecdaa2db9aa5a73134ad256e17e22d1419a2ab073`, + }); + await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stderr: ``, + stdout: `6.6.2\n`, + }); }); }); }); From b5c28a8b410f8954c740fb87be28c11663db8e83 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 15 Feb 2025 20:02:08 +0100 Subject: [PATCH 10/17] increase coverage --- tests/main.test.ts | 74 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/tests/main.test.ts b/tests/main.test.ts index 1d7225681..c2838f757 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -421,6 +421,80 @@ describe(`should accept range in devEngines only if a specific version is provid }); }); +describe(`when devEngines.packageManager.name does not match packageManager`, () => { + it(`should ignore if devEngines.packageManager.onFail is set to "ignore"`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + devEngines: { + packageManager: { + name: `yarn`, + onFail: `ignore`, + }, + }, + packageManager: `pnpm@6.6.2+sha1.7b4d6b176c1b93b5670ed94c24babb7d80c13854`, + }); + await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stderr: ``, + stdout: `6.6.2\n`, + }); + }); + }); + it(`should warn if devEngines.packageManager.onFail is set to "warn"`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + devEngines: { + packageManager: { + name: `yarn`, + onFail: `warn`, + }, + }, + packageManager: `pnpm@6.6.2+sha1.7b4d6b176c1b93b5670ed94c24babb7d80c13854`, + }); + await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stderr: `! Corepack validation warning: "packageManager" field is set to "pnpm@6.6.2+sha1.7b4d6b176c1b93b5670ed94c24babb7d80c13854" which does not match the "devEngines.packageManager" field set to "yarn"\n`, + stdout: `6.6.2\n`, + }); + }); + }); + it(`should throw if devEngines.packageManager.onFail is set to "error"`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + devEngines: { + packageManager: { + name: `yarn`, + onFail: `error`, + }, + }, + packageManager: `pnpm@6.6.2+sha1.7b4d6b176c1b93b5670ed94c24babb7d80c13854`, + }); + await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({ + exitCode: 1, + stderr: `"packageManager" field is set to "pnpm@6.6.2+sha1.7b4d6b176c1b93b5670ed94c24babb7d80c13854" which does not match the "devEngines.packageManager" field set to "yarn"\n`, + stdout: ``, + }); + }); + }); + it(`should throw by default`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { + devEngines: { + packageManager: { + name: `yarn`, + }, + }, + packageManager: `pnpm@6.6.2+sha1.7b4d6b176c1b93b5670ed94c24babb7d80c13854`, + }); + await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({ + exitCode: 1, + stderr: `"packageManager" field is set to "pnpm@6.6.2+sha1.7b4d6b176c1b93b5670ed94c24babb7d80c13854" which does not match the "devEngines.packageManager" field set to "yarn"\n`, + stdout: ``, + }); + }); + }); +}); + describe(`should reject if range in devEngines does not match version provided`, () => { it(`unless onFail is set to "ignore"`, async () => { await xfs.mktempPromise(async cwd => { From 8994e0ac5352f8fab17d2968681870be7b4500e1 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 25 Feb 2025 19:09:43 +0100 Subject: [PATCH 11/17] clarify return value is `pm` --- sources/specUtils.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/sources/specUtils.ts b/sources/specUtils.ts index 7f53cc2fe..f0a66534f 100644 --- a/sources/specUtils.ts +++ b/sources/specUtils.ts @@ -76,33 +76,33 @@ function warnOrThrow(errorMessage: string, onFail?: DevEngineDependency['onFail' } } function parsePackageJSON(packageJSONContent: CorepackPackageJSON) { + const {packageManager: pm} = packageJSONContent; if (packageJSONContent.devEngines?.packageManager != null) { const {packageManager} = packageJSONContent.devEngines; if (typeof packageManager !== `object`) { console.warn(`! Corepack only supports objects as valid value for devEngines.packageManager. The current value (${JSON.stringify(packageManager)}) will be ignored.`); - return packageJSONContent.packageManager; + return pm; } if (Array.isArray(packageManager)) { console.warn(`! Corepack does not currently support array values for devEngines.packageManager`); - return packageJSONContent.packageManager; + return pm; } const {name, version, onFail} = packageManager; if (typeof name !== `string` || name.includes(`@`)) { warnOrThrow(`The value of devEngines.packageManager.name ${JSON.stringify(name)} is not a supported string value`, onFail); - return packageJSONContent.packageManager; + return pm; } if (version != null && (typeof version !== `string` || !semverValidRange(version))) { warnOrThrow(`The value of devEngines.packageManager.version ${JSON.stringify(version)} is not a valid semver range`, onFail); - return packageJSONContent.packageManager; + return pm; } debugUtils.log(`devEngines.packageManager defines that ${name}@${version} is the local package manager`); - const {packageManager: pm} = packageJSONContent; if (pm) { - if (!pm.startsWith(`${name}@`)) + if (!pm.startsWith?.(`${name}@`)) warnOrThrow(`"packageManager" field is set to ${JSON.stringify(pm)} which does not match the "devEngines.packageManager" field set to ${JSON.stringify(name)}`, onFail); else if (version != null && !semverSatisfies(pm.slice(packageManager.name.length + 1), version)) @@ -115,7 +115,7 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) { return `${name}@${version ?? `*`}`; } - return packageJSONContent.packageManager; + return pm; } export async function setLocalPackageManager(cwd: string, info: PreparedPackageManagerInfo) { From 81419f38c59e13d1c221c208ca2805d9931dfe2b Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 26 Feb 2025 09:49:39 +0100 Subject: [PATCH 12/17] Update error message --- sources/commands/Base.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sources/commands/Base.ts b/sources/commands/Base.ts index ca0550129..41746f8ca 100644 --- a/sources/commands/Base.ts +++ b/sources/commands/Base.ts @@ -16,7 +16,7 @@ export abstract class BaseCommand extends Command { throw new UsageError(`Couldn't find a project in the local directory - please explicit the package manager to pack, or run this command from a valid project`); case `NoSpec`: - throw new UsageError(`The local project doesn't feature a 'packageManager' field - please explicit the package manager to pack, or update the manifest to reference it`); + throw new UsageError(`The local project doesn't feature a 'packageManager' field nor a 'devEngines.packageManager' field - please explicit the package manager to pack, or update the manifest to reference it`); default: { return [lookup.range ?? lookup.spec]; From b6e8a4fedc6ce61675b520b3d4723f08bf611306 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 26 Feb 2025 11:02:53 +0100 Subject: [PATCH 13/17] Update sources/commands/Base.ts --- sources/commands/Base.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sources/commands/Base.ts b/sources/commands/Base.ts index 41746f8ca..25e2c7f58 100644 --- a/sources/commands/Base.ts +++ b/sources/commands/Base.ts @@ -16,7 +16,7 @@ export abstract class BaseCommand extends Command { throw new UsageError(`Couldn't find a project in the local directory - please explicit the package manager to pack, or run this command from a valid project`); case `NoSpec`: - throw new UsageError(`The local project doesn't feature a 'packageManager' field nor a 'devEngines.packageManager' field - please explicit the package manager to pack, or update the manifest to reference it`); + throw new UsageError(`The local project doesn't feature a 'packageManager' field nor a 'devEngines.packageManager' field - please specify the package manager to pack, or update the manifest to reference it`); default: { return [lookup.range ?? lookup.spec]; From e28e3c0a8a90c9ec76d1471cb4d42befc3d23100 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 26 Feb 2025 19:23:52 +0100 Subject: [PATCH 14/17] Add more tests for `corepack up` and `corepack use` --- tests/Up.test.ts | 26 ++++++++++++ tests/Use.test.ts | 101 ++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 115 insertions(+), 12 deletions(-) diff --git a/tests/Up.test.ts b/tests/Up.test.ts index 09e75c41d..08b4a4f3b 100644 --- a/tests/Up.test.ts +++ b/tests/Up.test.ts @@ -21,6 +21,7 @@ describe(`UpCommand`, () => { await expect(runCli(cwd, [`up`])).resolves.toMatchObject({ exitCode: 0, stderr: ``, + stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n(.*\n)+➤ YN0000: Done in \d+s \d+ms\n$/), }); await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ @@ -30,6 +31,7 @@ describe(`UpCommand`, () => { await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ exitCode: 0, stdout: `2.4.3\n`, + stderr: ``, }); }); }); @@ -49,6 +51,7 @@ describe(`UpCommand`, () => { await expect(runCli(cwd, [`up`])).resolves.toMatchObject({ exitCode: 0, stderr: ``, + stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n(.*\n)+➤ YN0000: Done in \d+s \d+ms\n$/), }); await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ @@ -58,6 +61,7 @@ describe(`UpCommand`, () => { await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ exitCode: 0, stdout: `2.4.3\n`, + stderr: ``, }); }); }); @@ -78,6 +82,7 @@ describe(`UpCommand`, () => { await expect(runCli(cwd, [`up`])).resolves.toMatchObject({ exitCode: 0, stderr: ``, + stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n(.*\n)+➤ YN0000: Done in \d+s \d+ms\n$/), }); await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ @@ -87,6 +92,27 @@ describe(`UpCommand`, () => { await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ exitCode: 0, stdout: `2.4.3\n`, + stderr: ``, + }); + }); + }); + + it(`should fail if no top-level 'packageManager' field`, async () => { + await xfs.mktempPromise(async cwd => { + process.env.NO_COLOR = `1`; + await xfs.writeJsonPromise(ppath.join(cwd, `package.json`), { + devEngines: { + packageManager: { + name: `yarn`, + version: `1.x || 2.x`, + }, + }, + }); + + await expect(runCli(cwd, [`up`])).resolves.toMatchObject({ + exitCode: 1, + stderr: ``, + stdout: `Usage Error: Invalid package manager specification in package.json (yarn@1.x || 2.x); expected a semver version\n\n$ corepack up\n`, }); }); }); diff --git a/tests/Use.test.ts b/tests/Use.test.ts index e4d528efc..ed1b9aa0d 100644 --- a/tests/Use.test.ts +++ b/tests/Use.test.ts @@ -11,23 +11,100 @@ beforeEach(async () => { }); describe(`UseCommand`, () => { - it(`should set the package manager in the current project`, async () => { - await xfs.mktempPromise(async cwd => { - await xfs.writeJsonPromise(ppath.join(cwd, `package.json`), { - packageManager: `yarn@1.0.0`, - }); + describe(`should set the package manager in the current project`, () => { + it(`With an existing 'packageManager' field`, async () => { + await xfs.mktempPromise(async cwd => { + await xfs.writeJsonPromise(ppath.join(cwd, `package.json`), { + packageManager: `yarn@1.0.0`, + license: `MIT`, + }); - await expect(runCli(cwd, [`use`, `yarn@1.22.4`])).resolves.toMatchObject({ - exitCode: 0, + await expect(runCli(cwd, [`use`, `yarn@1.22.4`])).resolves.toMatchObject({ + exitCode: 0, + stdout: expect.stringMatching(/^Installing yarn@1\.22\.4 in the project\.\.\.\n\n(.*\n)+Done in \d+\.\d+s\.\n$/), + stderr: ``, + }); + + await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ + packageManager: `yarn@1.22.4+sha512.a1833b862fe52169bd6c2a033045a07df5bc6a23595c259e675fed1b2d035ab37abe6ce309720abb6636d68f03615054b6292dc0a70da31c8697fda228b50d18`, + }); + + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stdout: `1.22.4\n`, + stderr: ``, + }); }); + }); + it(`with 'devEngines.packageManager' field`, async () => { + await xfs.mktempPromise(async cwd => { + process.env.NO_COLOR = `1`; + const devEngines = {packageManager: {name: `yarn`, version: `2.x`}}; + await xfs.writeJsonPromise(ppath.join(cwd, `package.json`), { + devEngines, + }); - await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ - packageManager: `yarn@1.22.4+sha512.a1833b862fe52169bd6c2a033045a07df5bc6a23595c259e675fed1b2d035ab37abe6ce309720abb6636d68f03615054b6292dc0a70da31c8697fda228b50d18`, + // Should refuse to install an incompatible version: + await expect(runCli(cwd, [`use`, `yarn@1.22.4`])).resolves.toMatchObject({ + exitCode: 1, + stderr: ``, + stdout: `Installing yarn@1.22.4 in the project...\nUsage Error: The requested version of yarn@1.22.4+sha512.a1833b862fe52169bd6c2a033045a07df5bc6a23595c259e675fed1b2d035ab37abe6ce309720abb6636d68f03615054b6292dc0a70da31c8697fda228b50d18 does not match the devEngines specification (yarn@2.x)\n\n$ corepack use \n`, + }); + + // Should accept setting to a compatible version: + await expect(runCli(cwd, [`use`, `yarn@2.4.3`])).resolves.toMatchObject({ + exitCode: 0, + stderr: ``, + stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n(.*\n)+➤ YN0000: Done in \d+s \d+ms\n$/), + }); + + await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ + devEngines, + packageManager: `yarn@2.4.3+sha512.8dd9fedc5451829619e526c56f42609ad88ae4776d9d3f9456d578ac085115c0c2f0fb02bb7d57fd2e1b6e1ac96efba35e80a20a056668f61c96934f67694fd0`, + }); + + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stdout: `2.4.3\n`, + stderr: ``, + }); }); + }); - await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ - exitCode: 0, - stdout: `1.22.4\n`, + it(`with 'devEngines.packageManager' and 'packageManager' fields`, async () => { + await xfs.mktempPromise(async cwd => { + process.env.NO_COLOR = `1`; + const devEngines = {packageManager: {name: `yarn`, version: `1.x || 2.x`}}; + await xfs.writeJsonPromise(ppath.join(cwd, `package.json`), { + devEngines, + packageManager: `yarn@1.1.0`, + license: `MIT`, + }); + + // Should refuse to install an incompatible version: + await expect(runCli(cwd, [`use`, `yarn@1.22.4`])).resolves.toMatchObject({ + exitCode: 0, + stderr: ``, + stdout: expect.stringMatching(/^Installing yarn@1\.22\.4 in the project\.\.\.\n\n(.*\n)+Done in \d+\.\d+s\.\n$/), + }); + + // Should accept setting to a compatible version: + await expect(runCli(cwd, [`use`, `yarn@2.4.3`])).resolves.toMatchObject({ + exitCode: 0, + stderr: ``, + stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n(.*\n)+➤ YN0000: Done with warnings in \d+s \d+ms\n$/), + }); + + await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ + devEngines, + packageManager: `yarn@2.4.3+sha512.8dd9fedc5451829619e526c56f42609ad88ae4776d9d3f9456d578ac085115c0c2f0fb02bb7d57fd2e1b6e1ac96efba35e80a20a056668f61c96934f67694fd0`, + }); + + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stdout: `2.4.3\n`, + stderr: ``, + }); }); }); }); From 573b6889586a7c39e32080f2df75771dedf004f9 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 26 Feb 2025 19:27:06 +0100 Subject: [PATCH 15/17] validate range in `corepack use` --- sources/specUtils.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/sources/specUtils.ts b/sources/specUtils.ts index f0a66534f..f10883a05 100644 --- a/sources/specUtils.ts +++ b/sources/specUtils.ts @@ -121,13 +121,20 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) { export async function setLocalPackageManager(cwd: string, info: PreparedPackageManagerInfo) { const lookup = await loadSpec(cwd); + const range = `range` in lookup && lookup.range; + if (range) { + if (info.locator.name !== range.name || !semverSatisfies(info.locator.reference, range.range)) { + warnOrThrow(`The requested version of ${info.locator.name}@${info.locator.reference} does not match the devEngines specification (${range.name}@${range.range})`, range.onFail); + } + } + const content = lookup.type !== `NoProject` ? await fs.promises.readFile(lookup.target, `utf8`) : ``; const {data, indent} = nodeUtils.readPackageJson(content); - const previousPackageManager = data.packageManager ?? `unknown`; + const previousPackageManager = data.packageManager ?? (range ? `${range.name}@${range.range}` : `unknown`); data.packageManager = `${info.locator.name}@${info.locator.reference}`; const newContent = nodeUtils.normalizeLineEndings(content, `${JSON.stringify(data, null, indent)}\n`); @@ -141,7 +148,7 @@ export async function setLocalPackageManager(cwd: string, info: PreparedPackageM export type LoadSpecResult = | {type: `NoProject`, target: string} | {type: `NoSpec`, target: string} - | {type: `Found`, target: string, spec: Descriptor, range?: Descriptor}; + | {type: `Found`, target: string, spec: Descriptor, range?: Descriptor & {onFail?: DevEngineDependency['onFail']}}; export async function loadSpec(initialCwd: string): Promise { let nextCwd = initialCwd; @@ -197,6 +204,7 @@ export async function loadSpec(initialCwd: string): Promise { range: selection.data.devEngines?.packageManager?.version && { name: selection.data.devEngines.packageManager.name, range: selection.data.devEngines.packageManager.version, + onFail: selection.data.devEngines.packageManager.onFail, }, }; } From 32407e7a541c7d6c684c53a19caeb7554c7ec916 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 28 Feb 2025 19:25:09 +0100 Subject: [PATCH 16/17] fixup --- sources/specUtils.ts | 1 - tests/Up.test.ts | 16 +++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/sources/specUtils.ts b/sources/specUtils.ts index f26c4b6b1..36af0372a 100644 --- a/sources/specUtils.ts +++ b/sources/specUtils.ts @@ -196,7 +196,6 @@ export async function loadSpec(initialCwd: string): Promise { debugUtils.log(`${selection.manifestPath} defines ${rawPmSpec} as local package manager`); - const spec = parseSpec(rawPmSpec, path.relative(initialCwd, selection.manifestPath)); return { type: `Found`, target: selection.manifestPath, diff --git a/tests/Up.test.ts b/tests/Up.test.ts index 5cc4f4fea..3f30d2a95 100644 --- a/tests/Up.test.ts +++ b/tests/Up.test.ts @@ -103,7 +103,7 @@ describe(`UpCommand`, () => { }); }); - it(`should fail if no top-level 'packageManager' field`, async () => { + it(`should succeed even if no 'packageManager' field`, async () => { await xfs.mktempPromise(async cwd => { process.env.NO_COLOR = `1`; await xfs.writeJsonPromise(ppath.join(cwd, `package.json`), { @@ -116,9 +116,19 @@ describe(`UpCommand`, () => { }); await expect(runCli(cwd, [`up`])).resolves.toMatchObject({ - exitCode: 1, + exitCode: 0, + stderr: ``, + stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n(.*\n)+➤ YN0000: Done in \d+s \d+ms\n$/), + }); + + await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ + packageManager: `yarn@2.4.3+sha512.8dd9fedc5451829619e526c56f42609ad88ae4776d9d3f9456d578ac085115c0c2f0fb02bb7d57fd2e1b6e1ac96efba35e80a20a056668f61c96934f67694fd0`, + }); + + await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ + exitCode: 0, + stdout: `2.4.3\n`, stderr: ``, - stdout: `Usage Error: Invalid package manager specification in package.json (yarn@1.x || 2.x); expected a semver version\n\n$ corepack up\n`, }); }); }); From f761471052411eab1819577520b3d0baa81eeacb Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 28 Feb 2025 19:48:29 +0100 Subject: [PATCH 17/17] fixup --- tests/Up.test.ts | 8 ++++---- tests/Use.test.ts | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/Up.test.ts b/tests/Up.test.ts index 3f30d2a95..dac6e9d96 100644 --- a/tests/Up.test.ts +++ b/tests/Up.test.ts @@ -27,7 +27,7 @@ describe(`UpCommand`, () => { await expect(runCli(cwd, [`up`])).resolves.toMatchObject({ exitCode: 0, stderr: ``, - stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n(.*\n)+➤ YN0000: Done in \d+s \d+ms\n$/), + stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n/), }); await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ @@ -57,7 +57,7 @@ describe(`UpCommand`, () => { await expect(runCli(cwd, [`up`])).resolves.toMatchObject({ exitCode: 0, stderr: ``, - stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n(.*\n)+➤ YN0000: Done in \d+s \d+ms\n$/), + stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n/), }); await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ @@ -88,7 +88,7 @@ describe(`UpCommand`, () => { await expect(runCli(cwd, [`up`])).resolves.toMatchObject({ exitCode: 0, stderr: ``, - stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n(.*\n)+➤ YN0000: Done in \d+s \d+ms\n$/), + stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n/), }); await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ @@ -118,7 +118,7 @@ describe(`UpCommand`, () => { await expect(runCli(cwd, [`up`])).resolves.toMatchObject({ exitCode: 0, stderr: ``, - stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n(.*\n)+➤ YN0000: Done in \d+s \d+ms\n$/), + stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n/), }); await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ diff --git a/tests/Use.test.ts b/tests/Use.test.ts index a92caf46d..978c2e3a8 100644 --- a/tests/Use.test.ts +++ b/tests/Use.test.ts @@ -27,7 +27,7 @@ describe(`UseCommand`, () => { await expect(runCli(cwd, [`use`, `yarn@1.22.4`])).resolves.toMatchObject({ exitCode: 0, - stdout: expect.stringMatching(/^Installing yarn@1\.22\.4 in the project\.\.\.\n\n(.*\n)+Done in \d+\.\d+s\.\n$/), + stdout: expect.stringMatching(/^Installing yarn@1\.22\.4 in the project\.\.\.\n\n/), stderr: ``, }); @@ -61,7 +61,7 @@ describe(`UseCommand`, () => { await expect(runCli(cwd, [`use`, `yarn@2.4.3`])).resolves.toMatchObject({ exitCode: 0, stderr: ``, - stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n(.*\n)+➤ YN0000: Done in \d+s \d+ms\n$/), + stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n/), }); await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ @@ -91,14 +91,14 @@ describe(`UseCommand`, () => { await expect(runCli(cwd, [`use`, `yarn@1.22.4`])).resolves.toMatchObject({ exitCode: 0, stderr: ``, - stdout: expect.stringMatching(/^Installing yarn@1\.22\.4 in the project\.\.\.\n\n(.*\n)+Done in \d+\.\d+s\.\n$/), + stdout: expect.stringMatching(/^Installing yarn@1\.22\.4 in the project\.\.\.\n\n/), }); // Should accept setting to a compatible version: await expect(runCli(cwd, [`use`, `yarn@2.4.3`])).resolves.toMatchObject({ exitCode: 0, stderr: ``, - stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n(.*\n)+➤ YN0000: Done with warnings in \d+s \d+ms\n$/), + stdout: expect.stringMatching(/^Installing yarn@2\.4\.3 in the project\.\.\.\n\n/), }); await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({ @@ -177,7 +177,7 @@ describe(`UseCommand`, () => { await expect(runCli(cwd, [`use`, `yarn@1.22.4`])).resolves.toMatchObject({ exitCode: 0, stderr: ``, - stdout: expect.stringMatching(/^Installing yarn@1\.22\.4 in the project\.\.\.\n\nyarn install v1\.22\.4\ninfo No lockfile found\.\n(.*\n)+Done in \d+\.\d+s\.\n$/), + stdout: expect.stringMatching(/^Installing yarn@1\.22\.4 in the project\.\.\.\n\n/), }); await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({