fix: do not resolve fallback descriptor when `packageManager` is defined (#632)

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<ReturnType<Engine[`ensurePackageManager`]>>;
 
@@ -244,12 +244,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<Descriptor> {
+  async findProjectSpec(initialCwd: string, locator: Locator | LazyLocator, {transparent = false}: {transparent?: boolean} = {}): Promise<Descriptor> {
     // 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 +262,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 +295,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 +313,14 @@ export class Engine {
   }
 
   async executePackageManagerRequest({packageManager, binaryName, binaryVersion}: PackageManagerRequest, {cwd, args}: {cwd: string, args: Array<string>}): Promise<void> {
-    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 +339,7 @@ export class Engine {
 
       fallbackLocator = {
         name: packageManager,
-        reference: fallbackReference,
+        reference: fallbackReference as string,
       };
     }
 

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<string>;
+}

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,31 +962,15 @@ 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 () => {
+  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 => {
-      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: ``,
-        }),
-      ]);
-
       // 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;
 
@@ -1017,6 +996,31 @@ describe(`handle integrity checks`, () => {
         ]);
       });
     });
+
+
+    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 () => {
     process.env.TEST_INTEGRITY = `invalid_signature`; // See `_registryServer.mjs`