From dd1faee650e0ab190a10f21d56910f08cf91b303 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 3 Feb 2025 19:48:43 +0100 Subject: [PATCH 1/2] fix: do not resolve fallback descriptor when `packageManager` is defined --- sources/Engine.ts | 31 +++++++++++++++++++++++-------- sources/types.ts | 15 +++++++++++++++ 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/sources/Engine.ts b/sources/Engine.ts index b9a12dea7..142dd0e6c 100644 --- a/sources/Engine.ts +++ b/sources/Engine.ts @@ -14,9 +14,9 @@ import * as folderUtils from './folderUtil import type {NodeError} from './nodeUtils'; import * as semverUtils from './semverUtils'; import * as specUtils from './specUtils'; -import {Config, Descriptor, Locator, PackageManagerSpec} from './types'; +import {Config, Descriptor, LazyLocator, Locator} from './types'; import {SupportedPackageManagers, SupportedPackageManagerSet} from './types'; -import {isSupportedPackageManager} from './types'; +import {isSupportedPackageManager, PackageManagerSpec} from './types'; export type PreparedPackageManagerInfo = Awaited>; @@ -161,6 +161,7 @@ export class Engine { async getDefaultDescriptors() { const locators: Array = []; + debugUtils.log(`getLastKnownGood getDefaultVersion getDefaultDescriptors`); for (const name of SupportedPackageManagerSet as Set) locators.push({name, range: await this.getDefaultVersion(name)}); @@ -244,12 +245,16 @@ export class Engine { * project using the default package managers, and configure it so that we * don't need to ask again in the future. */ - async findProjectSpec(initialCwd: string, locator: Locator, {transparent = false}: {transparent?: boolean} = {}): Promise { + async findProjectSpec(initialCwd: string, locator: Locator | LazyLocator, {transparent = false}: {transparent?: boolean} = {}): Promise { // A locator is a valid descriptor (but not the other way around) const fallbackDescriptor = {name: locator.name, range: `${locator.reference}`}; - if (process.env.COREPACK_ENABLE_PROJECT_SPEC === `0`) + if (process.env.COREPACK_ENABLE_PROJECT_SPEC === `0`) { + if (typeof locator.reference === `function`) + fallbackDescriptor.range = await locator.reference(); + return fallbackDescriptor; + } if (process.env.COREPACK_ENABLE_STRICT === `0`) transparent = true; @@ -258,11 +263,18 @@ export class Engine { const result = await specUtils.loadSpec(initialCwd); switch (result.type) { - case `NoProject`: + case `NoProject`: { + if (typeof locator.reference === `function`) + fallbackDescriptor.range = await locator.reference(); + debugUtils.log(`Falling back to ${fallbackDescriptor.name}@${fallbackDescriptor.range} as no project manifest were found`); return fallbackDescriptor; + } case `NoSpec`: { + if (typeof locator.reference === `function`) + fallbackDescriptor.range = await locator.reference(); + if (process.env.COREPACK_ENABLE_AUTO_PIN !== `0`) { const resolved = await this.resolveDescriptor(fallbackDescriptor, {allowTags: true}); if (resolved === null) @@ -284,6 +296,9 @@ export class Engine { case `Found`: { if (result.spec.name !== locator.name) { if (transparent) { + if (typeof locator.reference === `function`) + fallbackDescriptor.range = await locator.reference(); + debugUtils.log(`Falling back to ${fallbackDescriptor.name}@${fallbackDescriptor.range} in a ${result.spec.name}@${result.spec.range} project`); return fallbackDescriptor; } else { @@ -299,14 +314,14 @@ export class Engine { } async executePackageManagerRequest({packageManager, binaryName, binaryVersion}: PackageManagerRequest, {cwd, args}: {cwd: string, args: Array}): Promise { - let fallbackLocator: Locator = { + let fallbackLocator: Locator | LazyLocator = { name: binaryName as SupportedPackageManagers, reference: undefined as any, }; let isTransparentCommand = false; if (packageManager != null) { - const defaultVersion = binaryVersion || await this.getDefaultVersion(packageManager); + const defaultVersion = binaryVersion || (() => this.getDefaultVersion(packageManager)); const definition = this.config.definitions[packageManager]!; // If all leading segments match one of the patterns defined in the `transparent` @@ -325,7 +340,7 @@ export class Engine { fallbackLocator = { name: packageManager, - reference: fallbackReference, + reference: fallbackReference as string, }; } diff --git a/sources/types.ts b/sources/types.ts index 08ed7699b..b9fe3e370 100644 --- a/sources/types.ts +++ b/sources/types.ts @@ -145,3 +145,18 @@ export interface Locator { */ reference: string; } + +/** + * + */ +export interface LazyLocator { + /** + * The name of the package manager required. + */ + name: string; + + /** + * The exact version required. + */ + reference: () => Promise; +} From cfae5f54648e68004383d175bd770096ba2d5af1 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 4 Feb 2025 13:02:25 +0100 Subject: [PATCH 2/2] add tests --- sources/Engine.ts | 1 - tests/main.test.ts | 102 +++++++++++++++++++++++---------------------- 2 files changed, 53 insertions(+), 50 deletions(-) diff --git a/sources/Engine.ts b/sources/Engine.ts index 142dd0e6c..f34f2fa26 100644 --- a/sources/Engine.ts +++ b/sources/Engine.ts @@ -161,7 +161,6 @@ export class Engine { async getDefaultDescriptors() { const locators: Array = []; - debugUtils.log(`getLastKnownGood getDefaultVersion getDefaultDescriptors`); for (const name of SupportedPackageManagerSet as Set) locators.push({name, range: await this.getDefaultVersion(name)}); diff --git a/tests/main.test.ts b/tests/main.test.ts index ac4ae802a..92cc0e687 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -497,15 +497,10 @@ describe(`read-only and offline environment`, () => { exitCode: 0, }); - // Let corepack discover the latest yarn version. - // BUG: This should not be necessary with a fully specified version in package.json plus populated corepack cache. - // Engine.executePackageManagerRequest needs to defer the fallback work. This requires a big refactoring. - await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ - exitCode: 0, - }); - // Make COREPACK_HOME ro const home = npath.toPortablePath(folderUtils.getCorepackHomeFolder()); + // Make a lastKnownGood.json file with not JSON-parsable content: + await xfs.writeFilePromise(ppath.join(home, `lastKnownGood.json`), `{`); await xfs.chmodPromise(ppath.join(home, `lastKnownGood.json`), 0o444); await xfs.chmodPromise(home, 0o555); @@ -967,54 +962,63 @@ for (const authType of [`COREPACK_NPM_REGISTRY`, `COREPACK_NPM_TOKEN`, `COREPACK describe(`handle integrity checks`, () => { beforeEach(() => { process.env.AUTH_TYPE = `COREPACK_NPM_TOKEN`; // See `_registryServer.mjs` - process.env.COREPACK_DEFAULT_TO_LATEST = `1`; }); - it(`should return no error when signature matches`, async () => { - process.env.TEST_INTEGRITY = `valid`; // See `_registryServer.mjs` - - await xfs.mktempPromise(async cwd => { - await Promise.all([ - expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({ - exitCode: 0, - stdout: `pnpm: Hello from custom registry\n`, - stderr: ``, - }), - expect(runCli(cwd, [`yarn@1.x`, `--version`], true)).resolves.toMatchObject({ - exitCode: 0, - stdout: `yarn: Hello from custom registry\n`, - stderr: ``, - }), - expect(runCli(cwd, [`yarn@5.x`, `--version`], true)).resolves.toMatchObject({ - exitCode: 0, - stdout: `yarn: Hello from custom registry\n`, - stderr: ``, - }), - ]); + describe(`when signature matches`, () => { + beforeEach(() => { + process.env.TEST_INTEGRITY = `valid`; // See `_registryServer.mjs` + }); + it(`should return no error when calling 'corepack use'`, async () => { + await xfs.mktempPromise(async cwd => { // Skip rest of the test on Windows & Node.js 18.x as it inevitably times out otherwise. - if (process.version.startsWith(`v18.`) && os.platform() === `win32`) return; + if (process.version.startsWith(`v18.`) && os.platform() === `win32`) return; + + // Removing home directory to force the "re-download" + await xfs.rmPromise(process.env.COREPACK_HOME as any, {recursive: true}); + + await Promise.all([ + expect(runCli(cwd, [`use`, `pnpm`], true)).resolves.toMatchObject({ + exitCode: 0, + stdout: `Installing pnpm@1.9998.9999 in the project...\n\npnpm: Hello from custom registry\n`, + stderr: ``, + }), + expect(runCli(cwd, [`use`, `yarn@1.x`], true)).resolves.toMatchObject({ + exitCode: 0, + stdout: `Installing yarn@1.9998.9999 in the project...\n\nyarn: Hello from custom registry\n`, + stderr: ``, + }), + expect(runCli(cwd, [`use`, `yarn@latest`], true)).resolves.toMatchObject({ + exitCode: 0, + stdout: `Installing yarn@5.9999.9999 in the project...\n\nyarn: Hello from custom registry\n`, + stderr: ``, + }), + ]); + }); + }); - // Removing home directory to force the "re-download" - await xfs.rmPromise(process.env.COREPACK_HOME as any, {recursive: true}); - await Promise.all([ - expect(runCli(cwd, [`use`, `pnpm`], true)).resolves.toMatchObject({ - exitCode: 0, - stdout: `Installing pnpm@1.9998.9999 in the project...\n\npnpm: Hello from custom registry\n`, - stderr: ``, - }), - expect(runCli(cwd, [`use`, `yarn@1.x`], true)).resolves.toMatchObject({ - exitCode: 0, - stdout: `Installing yarn@1.9998.9999 in the project...\n\nyarn: Hello from custom registry\n`, - stderr: ``, - }), - expect(runCli(cwd, [`use`, `yarn@latest`], true)).resolves.toMatchObject({ - exitCode: 0, - stdout: `Installing yarn@5.9999.9999 in the project...\n\nyarn: Hello from custom registry\n`, - stderr: ``, - }), - ]); + it(`should return no error when fetching latest version`, async () => { + process.env.COREPACK_DEFAULT_TO_LATEST = `1`; + await xfs.mktempPromise(async cwd => { + await Promise.all([ + expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({ + exitCode: 0, + stdout: `pnpm: Hello from custom registry\n`, + stderr: ``, + }), + expect(runCli(cwd, [`yarn@1.x`, `--version`], true)).resolves.toMatchObject({ + exitCode: 0, + stdout: `yarn: Hello from custom registry\n`, + stderr: ``, + }), + expect(runCli(cwd, [`yarn@5.x`, `--version`], true)).resolves.toMatchObject({ + exitCode: 0, + stdout: `yarn: Hello from custom registry\n`, + stderr: ``, + }), + ]); + }); }); }); it(`should return an error when signature does not match with a tag`, async () => {