diff --git a/sources/Engine.ts b/sources/Engine.ts index c818cb2b9..890d6e6a7 100644 --- a/sources/Engine.ts +++ b/sources/Engine.ts @@ -276,9 +276,12 @@ export class Engine { } case `NoSpec`: { - if (typeof locator.reference === `function`) + if (result.devEnginesValue) + fallbackDescriptor.range = result.devEnginesValue.range; + else if (typeof locator.reference === `function`) fallbackDescriptor.range = await locator.reference(); + if (process.env.COREPACK_ENABLE_AUTO_PIN === `1`) { const resolved = await this.resolveDescriptor(fallbackDescriptor, {allowTags: true}); if (resolved === null) diff --git a/sources/commands/Base.ts b/sources/commands/Base.ts index c2c9ea2de..5a8b3088b 100644 --- a/sources/commands/Base.ts +++ b/sources/commands/Base.ts @@ -16,10 +16,11 @@ export abstract class BaseCommand extends Command { throw new UsageError(`Couldn't find a project in the local directory - please specify the package manager to pack, or run this command from a valid project`); case `NoSpec`: + if (lookup.devEnginesValue) return [lookup.devEnginesValue]; 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.getSpec()]; + return [lookup.devEnginesValue ?? lookup.getSpec()]; } } } else { diff --git a/sources/commands/deprecated/Prepare.ts b/sources/commands/deprecated/Prepare.ts index 49705b900..2b73cd28d 100644 --- a/sources/commands/deprecated/Prepare.ts +++ b/sources/commands/deprecated/Prepare.ts @@ -39,6 +39,10 @@ export class PrepareCommand extends Command { throw new UsageError(`Couldn't find a project in the local directory - please specify the package manager to pack, or run this command from a valid project`); case `NoSpec`: + if (lookup.devEnginesValue) { + specs.push(lookup.devEnginesValue); + break; + } throw new UsageError(`The local project doesn't feature a 'packageManager' field - please specify the package manager to pack, or update the manifest to reference it`); default: { diff --git a/sources/specUtils.ts b/sources/specUtils.ts index 29f61f59a..d29b619fd 100644 --- a/sources/specUtils.ts +++ b/sources/specUtils.ts @@ -101,7 +101,7 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) { return pm; } - debugUtils.log(`devEngines.packageManager defines that ${name}@${version} is the local package manager`); + debugUtils.log(`devEngines.packageManager defines that ${name}${version ? `@${version}` : ``} should the local package manager`); if (pm) { if (!pm.startsWith?.(`${name}@`)) @@ -113,8 +113,9 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) { return pm; } - - return `${name}@${version ?? `*`}`; + return {spec: `${name}@${version ?? `*`}`, name, version, toString() { + return this.spec; + }}; } return pm; @@ -123,14 +124,15 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) { export async function setLocalPackageManager(cwd: string, info: PreparedPackageManagerInfo) { const lookup = await loadSpecAndEnv(cwd); - const range = `range` in lookup && lookup.range; + const projectFound = lookup.type !== `NoProject`; + const range = projectFound && lookup.devEnginesValue; 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` + const content = projectFound ? await fs.promises.readFile(lookup.target, `utf8`) : ``; @@ -151,12 +153,12 @@ interface FoundSpecResult { type: `Found`; target: string; getSpec: (options?: {enforceExactVersion?: boolean}) => Descriptor; - range?: Descriptor & {onFail?: DevEngineDependency[`onFail`]}; + devEnginesValue?: Descriptor & {onFail?: DevEngineDependency[`onFail`]}; envFilePath?: string; } export type LoadSpecResult = | {type: `NoProject`, target: string, envFilePath?: string} - | {type: `NoSpec`, target: string, envFilePath?: string} + | {type: `NoSpec`, target: string, envFilePath?: string, devEnginesValue?: FoundSpecResult[`devEnginesValue`]} | FoundSpecResult; async function loadEnvFileIfExists(cwd: string): Promise<{env: LocalEnvFile, path: string} | void> { @@ -238,18 +240,26 @@ export async function loadSpecAndEnv(initialCwd: string, {envOnly} = {envOnly: f if (typeof rawPmSpec === `undefined`) return {type: `NoSpec`, target: selection.manifestPath, envFilePath: localEnv?.path}; - debugUtils.log(`${selection.manifestPath} defines ${rawPmSpec} as local package manager`); + const devEnginesValue = selection.data.devEngines?.packageManager?.version && { + name: selection.data.devEngines.packageManager.name, + range: selection.data.devEngines.packageManager.version, + onFail: selection.data.devEngines.packageManager.onFail, + }; + + if (typeof rawPmSpec === `object` && !semverValid(rawPmSpec.version)) { + debugUtils.log(`${selection.manifestPath} devEngines does not specify a specific version`); + return {type: `NoSpec`, target: selection.manifestPath, envFilePath: localEnv?.path, devEnginesValue}; + } + + const hasPackageManagerField = typeof rawPmSpec === `string`; + debugUtils.log(`${selection.manifestPath} defines ${rawPmSpec} as local package manager${hasPackageManagerField ? `using packageManager field` : ``}`); return { type: `Found`, target: selection.manifestPath, envFilePath: localEnv?.path, - range: selection.data.devEngines?.packageManager?.version && { - name: selection.data.devEngines.packageManager.name, - range: selection.data.devEngines.packageManager.version, - onFail: selection.data.devEngines.packageManager.onFail, - }, + devEnginesValue, // Lazy-loading it so we do not throw errors on commands that do not need valid spec. - getSpec: ({enforceExactVersion = true} = {}) => parseSpec(rawPmSpec, path.relative(initialCwd, selection.manifestPath), {enforceExactVersion}), + getSpec: ({enforceExactVersion = true} = {}) => parseSpec(`${rawPmSpec}`, path.relative(initialCwd, selection.manifestPath), {enforceExactVersion}), }; } diff --git a/tests/main.test.ts b/tests/main.test.ts index e6f7d7200..a4662b0f1 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -272,23 +272,6 @@ 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: `Invalid package manager specification in package.json (yarn@*); expected a semver version\n`, - stdout: ``, - }); - }); - }); it(`throw on invalid version`, async () => { await xfs.mktempPromise(async cwd => { await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { @@ -380,8 +363,8 @@ it(`should use hash from "packageManager" even when "devEngines" defines a diffe }); }); -describe(`should accept range in devEngines only if a specific version is provided`, () => { - it(`either in package.json#packageManager field`, async () => { +describe(`should accept range in devEngines`, () => { + it(`should accept if package.json#packageManager field matches`, async () => { await xfs.mktempPromise(async cwd => { await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), { devEngines: { @@ -390,18 +373,19 @@ describe(`should accept range in devEngines only if a specific version is provid version: `6.x`, }, }, + packageManager: `pnpm@6.6.2+sha224.eb5c0acad3b0f40ecdaa2db9aa5a73134ad256e17e22d1419a2ab073`, }); 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: ``, + exitCode: 0, + 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`, - version: `6.x`, }, }, packageManager: `pnpm@6.6.2+sha224.eb5c0acad3b0f40ecdaa2db9aa5a73134ad256e17e22d1419a2ab073`, @@ -411,20 +395,60 @@ 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), { + it(`should accept without a package.json#packageManager field`, async () => { + process.env.AUTH_TYPE = `COREPACK_NPM_TOKEN`; + process.env.TEST_INTEGRITY = `valid`; + + await xfs.mktempPromise(async cwd => { + // When no user version is specified, range versions in devEngines should still cause error + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), { devEngines: { packageManager: { name: `pnpm`, + version: `^1.0.0`, }, }, - packageManager: `pnpm@6.6.2+sha224.eb5c0acad3b0f40ecdaa2db9aa5a73134ad256e17e22d1419a2ab073`, }); - await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({ + + // Without user-specified version, should still fail due to range version in devEngines + await expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({ exitCode: 0, stderr: ``, - stdout: `6.6.2\n`, + stdout: `pnpm: Hello from custom registry\n`, + }); + }); + }); + + it(`should pin a specific if COREPACK_ENABLE_AUTO_PIN is set`, async () => { + process.env.AUTH_TYPE = `COREPACK_NPM_TOKEN`; + process.env.TEST_INTEGRITY = `valid`; + process.env.COREPACK_ENABLE_AUTO_PIN = `1`; + + await xfs.mktempPromise(async cwd => { + const devEngines = { + packageManager: { + name: `pnpm`, + version: `^1.0.0`, + }, + }; + + await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), { + devEngines, + }); + + // Without user-specified version, should still fail due to range version in devEngines + await expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({ + exitCode: 0, + stderr: expect.stringContaining(`local project doesn't define a 'packageManager' field`), + stdout: `pnpm: Hello from custom registry\n`, + }); + + await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json` as Filename))).resolves.toMatchObject({ + packageManager: expect.stringMatching(/^pnpm@1\.9998\.9999\+sha512\.[0-9a-z]{128}$/), + devEngines, }); }); }); @@ -1824,24 +1848,3 @@ describe(`allow range versions in devEngines.packageManager.version when user sp }); } }); - -it(`should still validate devEngines.packageManager.version format when no user version specified`, async () => { - await xfs.mktempPromise(async cwd => { - // When no user version is specified, range versions in devEngines should still cause error - await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), { - devEngines: { - packageManager: { - name: `npm`, - version: `^6.14.2`, - }, - }, - }); - - // Without user-specified version, should still fail due to range version in devEngines - await expect(runCli(cwd, [`npm`, `--version`])).resolves.toMatchObject({ - exitCode: 1, - stderr: expect.stringContaining(`Invalid package manager specification in package.json (npm@^6.14.2); expected a semver version`), - stdout: ``, - }); - }); -});