From 45aa1b847f388a0875a2d3877a358ce94ffd0328 Mon Sep 17 00:00:00 2001 From: merceyz Date: Fri, 15 Mar 2024 20:13:09 +0100 Subject: [PATCH 1/3] fix!: call `executePackageManagerRequest` directly --- sources/main.ts | 81 +++++++++++++++++++--------------------------- tests/main.test.ts | 35 ++++++++++---------- 2 files changed, 51 insertions(+), 65 deletions(-) diff --git a/sources/main.ts b/sources/main.ts index d58dcb4f..8849e883 100644 --- a/sources/main.ts +++ b/sources/main.ts @@ -1,23 +1,23 @@ -import {BaseContext, Builtins, Cli, Command, Option} from 'clipanion'; - -import {version as corepackVersion} from '../package.json'; - -import {Engine, PackageManagerRequest} from './Engine'; -import {CacheCommand} from './commands/Cache'; -import {DisableCommand} from './commands/Disable'; -import {EnableCommand} from './commands/Enable'; -import {InstallGlobalCommand} from './commands/InstallGlobal'; -import {InstallLocalCommand} from './commands/InstallLocal'; -import {PackCommand} from './commands/Pack'; -import {UpCommand} from './commands/Up'; -import {UseCommand} from './commands/Use'; -import {HydrateCommand} from './commands/deprecated/Hydrate'; -import {PrepareCommand} from './commands/deprecated/Prepare'; +import {BaseContext, Builtins, Cli} from 'clipanion'; + +import {version as corepackVersion} from '../package.json'; + +import {Engine, PackageManagerRequest} from './Engine'; +import {CacheCommand} from './commands/Cache'; +import {DisableCommand} from './commands/Disable'; +import {EnableCommand} from './commands/Enable'; +import {InstallGlobalCommand} from './commands/InstallGlobal'; +import {InstallLocalCommand} from './commands/InstallLocal'; +import {PackCommand} from './commands/Pack'; +import {UpCommand} from './commands/Up'; +import {UseCommand} from './commands/Use'; +import {HydrateCommand} from './commands/deprecated/Hydrate'; +import {PrepareCommand} from './commands/deprecated/Prepare'; export type CustomContext = {cwd: string, engine: Engine}; export type Context = BaseContext & CustomContext; -function getPackageManagerRequestFromCli(parameter: string | undefined, context: CustomContext & Partial): PackageManagerRequest | null { +function getPackageManagerRequestFromCli(parameter: string | undefined, engine: Engine): PackageManagerRequest | null { if (!parameter) return null; @@ -26,7 +26,7 @@ function getPackageManagerRequestFromCli(parameter: string | undefined, context: return null; const [, binaryName, binaryVersion] = match; - const packageManager = context.engine.getPackageManagerFor(binaryName)!; + const packageManager = engine.getPackageManagerFor(binaryName)!; if (packageManager == null && binaryVersion == null) return null; @@ -38,17 +38,10 @@ function getPackageManagerRequestFromCli(parameter: string | undefined, context: } export async function runMain(argv: Array) { - // Because we load the binaries in the same process, we don't support custom contexts. - const context = { - ...Cli.defaultContext, - cwd: process.cwd(), - engine: new Engine(), - }; + const engine = new Engine(); const [firstArg, ...restArgs] = argv; - const request = getPackageManagerRequestFromCli(firstArg, context); - - let code: number; + const request = getPackageManagerRequestFromCli(firstArg, engine); if (!request) { // If the first argument doesn't match any supported package manager, we fallback to the standard Corepack CLI @@ -74,29 +67,21 @@ export async function runMain(argv: Array) { cli.register(HydrateCommand); cli.register(PrepareCommand); - code = await cli.run(argv, context); - } else { - // Otherwise, we create a single-command CLI to run the specified package manager (we still use Clipanion in order to pretty-print usage errors). - const cli = new Cli({ - binaryLabel: `'${request.binaryName}', via Corepack`, - binaryName: request.binaryName, - binaryVersion: `corepack/${corepackVersion}`, - }); - - cli.register(class BinaryCommand extends Command { - proxy = Option.Proxy(); - async execute() { - return this.context.engine.executePackageManagerRequest(request, { - cwd: this.context.cwd, - args: this.proxy, - }); - } - }); + const context = { + ...Cli.defaultContext, + cwd: process.cwd(), + engine, + }; - code = await cli.run(restArgs, context); - } + const code = await cli.run(argv, context); - if (code !== 0) { - process.exitCode ??= code; + if (code !== 0) { + process.exitCode ??= code; + } + } else { + await engine.executePackageManagerRequest(request, { + cwd: process.cwd(), + args: restArgs, + }); } } diff --git a/tests/main.test.ts b/tests/main.test.ts index f07cb85b..59053e04 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -24,8 +24,8 @@ it(`should refuse to download a package manager if the hash doesn't match`, asyn await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ exitCode: 1, - stderr: ``, - stdout: /Mismatch hashes/, + stderr: /Mismatch hashes/, + stdout: ``, }); }); }); @@ -35,8 +35,8 @@ it(`should refuse to download a known package manager from a URL`, async () => { // Package managers known by Corepack cannot be loaded from a URL. await expect(runCli(cwd, [`yarn@https://registry.npmjs.com/yarn/-/yarn-1.22.21.tgz`, `--version`])).resolves.toMatchObject({ exitCode: 1, - stderr: ``, - stdout: /Illegal use of URL for known package manager/, + stderr: /Illegal use of URL for known package manager/, + stdout: ``, }); // Unknown package managers can be loaded from a URL. @@ -57,8 +57,8 @@ it.failing(`should refuse to download a known package manager from a URL in pack await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ exitCode: 1, - stderr: ``, - stdout: /Illegal use of URL for known package manager/, + stderr: /Illegal use of URL for known package manager/, + stdout: ``, }); // Unknown package managers can be loaded from a URL. @@ -82,8 +82,8 @@ it(`should require a version to be specified`, async () => { await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ exitCode: 1, - stderr: ``, - stdout: /expected a semver version/, + stderr: /expected a semver version/, + stdout: ``, }); await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), { @@ -92,8 +92,8 @@ it(`should require a version to be specified`, async () => { await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ exitCode: 1, - stderr: ``, - stdout: /expected a semver version/, + stderr: /expected a semver version/, + stdout: ``, }); await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), { @@ -102,8 +102,8 @@ it(`should require a version to be specified`, async () => { await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ exitCode: 1, - stderr: ``, - stdout: /expected a semver version/, + stderr: /expected a semver version/, + stdout: ``, }); }); }); @@ -272,7 +272,7 @@ it(`shouldn't allow using regular Yarn commands on npm-configured projects`, asy await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ exitCode: 1, - stderr: ``, + stderr: expect.stringContaining(`This project is configured to use npm`), }); }); }); @@ -419,9 +419,10 @@ it(`should refuse to run a different package manager within a configured project process.env.FORCE_COLOR = `0`; await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({ - stdout: `Usage Error: This project is configured to use yarn because ${ + stdout: ``, + stderr: expect.stringContaining(`This project is configured to use yarn because ${ npath.fromPortablePath(ppath.join(cwd, `package.json` as Filename)) - } has a "packageManager" field\n\n$ pnpm ...\n`, + } has a "packageManager" field`), exitCode: 1, }); @@ -471,8 +472,8 @@ it(`should support disabling the network accesses from the environment`, async ( }); await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ - stdout: expect.stringContaining(`Network access disabled by the environment`), - stderr: ``, + stdout: ``, + stderr: expect.stringContaining(`Network access disabled by the environment`), exitCode: 1, }); }); From 6e62f292789ea87f0ffaa36848a256a446824009 Mon Sep 17 00:00:00 2001 From: Kristoffer K Date: Fri, 15 Mar 2024 21:30:17 +0100 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Antoine du Hamel --- tests/main.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/main.test.ts b/tests/main.test.ts index 59053e04..3820bd17 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -272,7 +272,7 @@ it(`shouldn't allow using regular Yarn commands on npm-configured projects`, asy await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ exitCode: 1, - stderr: expect.stringContaining(`This project is configured to use npm`), + stderr: /This project is configured to use npm/, }); }); }); @@ -473,7 +473,7 @@ it(`should support disabling the network accesses from the environment`, async ( await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({ stdout: ``, - stderr: expect.stringContaining(`Network access disabled by the environment`), + stderr: /Network access disabled by the environment/, exitCode: 1, }); }); From a82e453c1355e86bbe21468b14c4c5f878c2573d Mon Sep 17 00:00:00 2001 From: merceyz Date: Sat, 20 Apr 2024 15:02:18 +0200 Subject: [PATCH 3/3] test: update assertions --- tests/main.test.ts | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/tests/main.test.ts b/tests/main.test.ts index 3820bd17..fdb9b30e 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -999,13 +999,13 @@ describe(`handle integrity checks`, () => { await xfs.mktempPromise(async cwd => { await expect(runCli(cwd, [`pnpm@1.x`, `--version`], true)).resolves.toMatchObject({ exitCode: 1, - stdout: /Signature does not match/, - stderr: ``, + stderr: /Signature does not match/, + stdout: ``, }); await expect(runCli(cwd, [`yarn@stable`, `--version`], true)).resolves.toMatchObject({ exitCode: 1, - stdout: /Signature does not match/, - stderr: ``, + stderr: /Signature does not match/, + stdout: ``, }); }); }); @@ -1015,19 +1015,19 @@ describe(`handle integrity checks`, () => { await xfs.mktempPromise(async cwd => { await expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({ exitCode: 1, - stdout: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/, - stderr: ``, + stderr: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/, + stdout: ``, }); // A second time to validate the invalid version was not cached. await expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({ exitCode: 1, - stdout: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/, - stderr: ``, + stderr: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/, + stdout: ``, }); await expect(runCli(cwd, [`yarn`, `--version`], true)).resolves.toMatchObject({ exitCode: 1, - stdout: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/, - stderr: ``, + stderr: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/, + stdout: ``, }); await expect(runCli(cwd, [`use`, `pnpm`], true)).resolves.toMatchObject({ exitCode: 1, @@ -1042,19 +1042,19 @@ describe(`handle integrity checks`, () => { await xfs.mktempPromise(async cwd => { await expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({ exitCode: 1, - stdout: /Signature does not match/, - stderr: ``, + stderr: /Signature does not match/, + stdout: ``, }); // A second time to validate the invalid version was not cached. await expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({ exitCode: 1, - stdout: /Signature does not match/, - stderr: ``, + stderr: /Signature does not match/, + stdout: ``, }); await expect(runCli(cwd, [`yarn`, `--version`], true)).resolves.toMatchObject({ exitCode: 1, - stdout: /Signature does not match/, - stderr: ``, + stderr: /Signature does not match/, + stdout: ``, }); await expect(runCli(cwd, [`use`, `pnpm`], true)).resolves.toMatchObject({ exitCode: 1, @@ -1069,8 +1069,8 @@ describe(`handle integrity checks`, () => { await xfs.mktempPromise(async cwd => { await expect(runCli(cwd, [`yarn@1.9998.9999`, `--version`], true)).resolves.toMatchObject({ exitCode: 1, - stdout: /Signature does not match/, - stderr: ``, + stderr: /Signature does not match/, + stdout: ``, }); await expect(runCli(cwd, [`use`, `yarn@1.9998.9999`], true)).resolves.toMatchObject({ exitCode: 1, @@ -1085,8 +1085,8 @@ describe(`handle integrity checks`, () => { await xfs.mktempPromise(async cwd => { await expect(runCli(cwd, [`yarn@1.9998.9999`, `--version`], true)).resolves.toMatchObject({ exitCode: 1, - stdout: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/, - stderr: ``, + stderr: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/, + stdout: ``, }); await expect(runCli(cwd, [`use`, `yarn@1.9998.9999`], true)).resolves.toMatchObject({ exitCode: 1, @@ -1102,10 +1102,10 @@ describe(`handle integrity checks`, () => { const result = await runCli(cwd, [`yarn@1.9998.9999+sha1.deadbeef`, `--version`], true); expect(result).toMatchObject({ exitCode: 1, - stderr: ``, + stdout: ``, }); - const match = /Mismatch hashes. Expected deadbeef, got ([a-f0-9]{40})/.exec(result.stdout); - if (match == null) throw new Error(`Invalid output`, {cause: result.stdout}); + const match = /Mismatch hashes. Expected deadbeef, got ([a-f0-9]{40})/.exec(result.stderr); + if (match == null) throw new Error(`Invalid output`, {cause: result.stderr}); await expect(runCli(cwd, [`yarn@1.9998.9999+sha1.${match[1]}`, `--version`], true)).resolves.toMatchObject({ exitCode: 0, stdout: `yarn: Hello from custom registry\n`,