Fixes couple of release blockers (#5730)

**What's the problem this PR addresses?**

- CRA broke when [we started required that `--cwd` be put before any
other
argument](#5600 (comment)).
CRA is mostly deprecated / unmaintained by now, but it has a decent
amount of downloads (more than `create-next-app`), so it doesn't cost us
much to special-case a fix for the v4 that we can then drop in v5.

- When a machine had a hot cache for package X in both cache versions A
and B (each variant having its own checksum), when migrating a project
cache version from A to B, Yarn would mistakenly try to validate the
variant B using the checksum from variant A.

- We already have a mechanism to tolerate checksum changes when going
from one cache key to the next, but before
#5564 we used to retrieve the cache
key from the file name, whereas we now retrieve it from the lockfile
instead. A branch of code that relied on the assumption that the cache
key could be checked later became invalid, hence the problem.

**How did you fix it?**

- The `--cwd` flag is now allowed (as a special case) at the end of
`yarn add`.

- I refactored the "is this locator compatible with the current cache
key" function outside of `getLocatorPath`.

**Checklist**
<!--- Don't worry if you miss something, chores are automatically
tested. -->
<!--- This checklist exists to help you remember doing the chores when
you submit a PR. -->
<!--- Put an `x` in all the boxes that apply. -->
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).

<!-- See
https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released
for more details. -->
<!-- Check with `yarn version check` and fix with `yarn version check
-i` -->
- [x] I have set the packages that need to be released for my changes to
be effective.

<!-- The "Testing chores" workflow validates that your PR follows our
guidelines. -->
<!-- If it doesn't pass, click on it to see details as to what your PR
might be missing. -->
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.

.github/workflows/e2e-cra-workflow.yml

@@ -1,3 +1,4 @@
+#
 on:
   schedule:
   - cron: '0 */4 * * *'

.yarn/versions/eec3be1d.yml

@@ -0,0 +1,34 @@
+releases:
+  "@yarnpkg/cli": patch
+  "@yarnpkg/core": patch
+
+declined:
+  - "@yarnpkg/plugin-compat"
+  - "@yarnpkg/plugin-constraints"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-essentials"
+  - "@yarnpkg/plugin-exec"
+  - "@yarnpkg/plugin-file"
+  - "@yarnpkg/plugin-git"
+  - "@yarnpkg/plugin-github"
+  - "@yarnpkg/plugin-http"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-link"
+  - "@yarnpkg/plugin-nm"
+  - "@yarnpkg/plugin-npm"
+  - "@yarnpkg/plugin-npm-cli"
+  - "@yarnpkg/plugin-pack"
+  - "@yarnpkg/plugin-patch"
+  - "@yarnpkg/plugin-pnp"
+  - "@yarnpkg/plugin-pnpm"
+  - "@yarnpkg/plugin-stage"
+  - "@yarnpkg/plugin-typescript"
+  - "@yarnpkg/plugin-version"
+  - "@yarnpkg/plugin-workspace-tools"
+  - "@yarnpkg/builder"
+  - "@yarnpkg/doctor"
+  - "@yarnpkg/extensions"
+  - "@yarnpkg/nm"
+  - "@yarnpkg/pnpify"
+  - "@yarnpkg/sdks"

packages/acceptance-tests/pkg-tests-specs/sources/features/__snapshots__/mergeConflictResolution.test.ts.snap

@@ -41,7 +41,7 @@ exports[`Features Merge Conflict Resolution it should properly fix merge conflic
 ➤ YN0000: ┌ Resolution step
 ➤ YN0000: └ Completed
 ➤ YN0000: ┌ Fetch step
-➤ YN0013: │ No new packages added to the cache, but one was removed (- 0.78 KiB).
+➤ YN0013: │ No new packages added to the project, but one was removed (- 0.78 KiB).
 ➤ YN0000: └ Completed
 ➤ YN0000: ┌ Link step
 ➤ YN0000: └ Completed
@@ -141,7 +141,7 @@ exports[`Features Merge Conflict Resolution it should support fixing cherry-pick
 ➤ YN0000: ┌ Resolution step
 ➤ YN0000: └ Completed
 ➤ YN0000: ┌ Fetch step
-➤ YN0013: │ No new packages added to the cache, but one was removed (- 0.78 KiB).
+➤ YN0013: │ No new packages added to the project, but one was removed (- 0.78 KiB).
 ➤ YN0000: └ Completed
 ➤ YN0000: ┌ Link step
 ➤ YN0000: └ Completed
@@ -191,7 +191,7 @@ exports[`Features Merge Conflict Resolution it should support fixing rebase conf
 ➤ YN0000: ┌ Resolution step
 ➤ YN0000: └ Completed
 ➤ YN0000: ┌ Fetch step
-➤ YN0013: │ No new packages added to the cache, but one was removed (- 0.78 KiB).
+➤ YN0013: │ No new packages added to the project, but one was removed (- 0.78 KiB).
 ➤ YN0000: └ Completed
 ➤ YN0000: ┌ Link step
 ➤ YN0000: └ Completed
@@ -241,7 +241,7 @@ exports[`Features Merge Conflict Resolution it shouldn't re-fetch the lockfile m
 ➤ YN0000: ┌ Resolution step
 ➤ YN0000: └ Completed
 ➤ YN0000: ┌ Fetch step
-➤ YN0013: │ No new packages added to the cache, but one was removed (- 0.78 KiB).
+➤ YN0013: │ No new packages added to the project, but one was removed (- 0.78 KiB).
 ➤ YN0000: └ Completed
 ➤ YN0000: ┌ Link step
 ➤ YN0000: └ Completed

packages/acceptance-tests/pkg-tests-specs/sources/features/cache.test.ts

@@ -1,4 +1,4 @@
-import {ppath, xfs} from '@yarnpkg/fslib';
+import {Filename, ppath, xfs} from '@yarnpkg/fslib';
 
 describe(`Cache`, () => {
   test(
@@ -114,6 +114,41 @@ describe(`Cache`, () => {
     }),
   );
 
+  test(
+    `it should ignore checksum mismatches and regenerate archives when their cache key is different from Yarn's own cache key, if cacheMigrationMode=always (global cache, hot)`,
+    makeTemporaryEnv({
+      dependencies: {
+        [`no-deps`]: `1.0.0`,
+      },
+    }, {
+      cacheMigrationMode: `always`,
+      enableGlobalCache: true,
+    }, async ({path, run, source}) => {
+      await run(`install`, {
+        cacheVersionOverride: `2`,
+      });
+
+      const cacheFiles = await xfs.readdirPromise(ppath.join(path, `.yarn/global/cache`));
+      const cacheFile = ppath.join(path, `.yarn/global/cache`, cacheFiles.find(name => name.startsWith(`no-deps-`))!);
+
+      // Adding some data to give it a different checksum than what we'll have for
+      // "cache key v1"; zip archives allow pseudo-arbitrary content at their end
+      await xfs.appendFilePromise(cacheFile, `corrupted archive`);
+
+      // Removing the lockfile to make sure it'll be populated with "cache key v1" data
+      await xfs.removePromise(ppath.join(path, Filename.lockfile));
+
+      await run(`install`, {
+        cacheVersionOverride: `1`,
+      });
+
+      await run(`install`, {
+        cacheVersionOverride: `2`,
+        cacheCheckpointOverride: `1`,
+      });
+    }),
+  );
+
   test(
     `it should update the cache files when changing the compression level, if cacheMigrationMode=match-spec`,
     makeTemporaryEnv({

packages/yarnpkg-cli/sources/lib.ts

@@ -99,6 +99,12 @@ function checkCwd(cli: YarnCli, argv: Array<string>) {
   } else if (argv.length >= 1 && argv[0].startsWith(`--cwd=`)) {
     cwd = npath.toPortablePath(argv[0].slice(6));
     postCwdArgv = argv.slice(1);
+  } else if (argv[0] === `add` && argv[argv.length - 2] === `--cwd`) {
+    // CRA adds `--cwd` at the end of the command; it's not ideal, but since
+    // it's unlikely to receive more releases we can just special-case it
+    // TODO v5: remove this special case
+    cwd = npath.toPortablePath(argv[argv.length - 1]);
+    postCwdArgv = argv.slice(0, argv.length - 2);
   }
 
   cli.defaultContext.cwd = cwd !== null

packages/yarnpkg-core/sources/Cache.ts

@@ -144,41 +144,51 @@ export class Cache {
     return `${structUtils.slugifyLocator(locator)}-${significantChecksum}.zip` as Filename;
   }
 
-  getLocatorPath(locator: Locator, expectedChecksum: string | null, opts: CacheOptions = {}) {
-    // If there is no mirror, then the local cache *is* the mirror, in which
-    // case we use the versioned filename pattern. Same if the package is
-    // unstable, meaning it may be there or not depending on the environment,
-    // so we can't rely on its checksum to get a stable location.
-    if (this.mirrorCwd === null || opts.unstablePackages?.has(locator.locatorHash))
-      return ppath.resolve(this.cwd, this.getVersionFilename(locator));
-
+  isChecksumCompatible(checksum: string) {
     // If we don't yet know the checksum, discard the path resolution for now
     // until the checksum can be obtained from somewhere (mirror or network).
-    if (expectedChecksum === null)
-      return null;
+    if (checksum === null)
+      return false;
 
     const {
       cacheVersion,
       cacheSpec,
-    } = splitChecksumComponents(expectedChecksum);
+    } = splitChecksumComponents(checksum);
 
     if (cacheVersion === null)
-      return null;
+      return false;
 
     // The cache keys must always be at least as old as the last checkpoint.
     if (cacheVersion < CACHE_CHECKPOINT)
-      return null;
+      return false;
 
     const migrationMode = this.configuration.get(`cacheMigrationMode`);
 
     // If the global cache is used, then the lockfile must always be up-to-date,
     // so the archives must be regenerated each time the version changes.
     if (cacheVersion < CACHE_VERSION && migrationMode === `always`)
-      return null;
+      return false;
 
     // If the cache spec changed, we may need to regenerate the archive
     if (cacheSpec !== this.cacheSpec && migrationMode !== `required-only`)
-      return null;
+      return false;
+
+    return true;
+  }
+
+  getLocatorPath(locator: Locator, expectedChecksum: string | null, opts: CacheOptions = {}) {
+    // When using the global cache we want the archives to be named as per
+    // the cache key rather than the hash, as otherwise we wouldn't be able
+    // to find them if we didn't have the hash (which is the case when adding
+    // new dependencies to a project).
+    if (this.mirrorCwd === null)
+      return ppath.resolve(this.cwd, this.getVersionFilename(locator));
+
+    // Same thing if we don't know the checksum; it means that the package
+    // doesn't support being checksum'd (unstablePackage), so we fallback
+    // on the versioned filename.
+    if (expectedChecksum === null)
+      return ppath.resolve(this.cwd, this.getVersionFilename(locator));
 
     return ppath.resolve(this.cwd, this.getChecksumFilename(locator, expectedChecksum));
   }
@@ -351,14 +361,11 @@ export class Cache {
       const {path: packagePath, source: packageSource} = await loadPackageThroughMirror();
 
       // Do this before moving the file so that we don't pollute the cache with corrupted archives
-      const checksum = (await validateFile(packagePath, {
+      const {hash: checksum} = await validateFile(packagePath, {
         isColdHit: true,
-      })).hash;
+      });
 
       const cachePath = this.getLocatorPath(locator, checksum, opts);
-      if (!cachePath)
-        throw new Error(`Assertion failed: Expected the cache path to be available`);
-
       const copyProcess: Array<() => Promise<void>> = [];
 
       // Copy the package into the mirror
@@ -394,10 +401,11 @@ export class Cache {
 
     const loadPackageThroughMutex = async () => {
       const mutexedLoad = async () => {
-        // We don't yet know whether the cache path can be computed yet, since that
-        // depends on whether the cache is actually the mirror or not, and whether
-        // the checksum is known or not.
-        const tentativeCachePath = this.getLocatorPath(locator, expectedChecksum, opts);
+        const isUnstablePackage = opts.unstablePackages?.has(locator.locatorHash);
+
+        const tentativeCachePath = isUnstablePackage || expectedChecksum && this.isChecksumCompatible(expectedChecksum)
+          ? this.getLocatorPath(locator, expectedChecksum, opts)
+          : null;
 
         const cacheFileExists = tentativeCachePath !== null
           ? this.markedFiles.has(tentativeCachePath) || await baseFs.existsPromise(tentativeCachePath)
@@ -414,7 +422,7 @@ export class Cache {
           action();
 
         if (!isCacheHit) {
-          if (this.immutable && opts.unstablePackages?.has(locator.locatorHash))
+          if (this.immutable && isUnstablePackage)
             throw new ReportError(MessageName.IMMUTABLE_CACHE, `Cache entry required but missing for ${structUtils.prettyLocator(this.configuration, locator)}; consider defining ${formatUtils.pretty(this.configuration, `supportedArchitectures`, formatUtils.Type.CODE)} to cache packages for multiple systems`);
 
           return loadPackage();

packages/yarnpkg-core/sources/Project.ts

@@ -1151,7 +1151,7 @@ export class Project {
         zero: `No new packages`,
         one: `A package was`,
         more: `${formatUtils.pretty(this.configuration, addedCount, formatUtils.Type.NUMBER)} packages were`,
-      })} added to the cache`;
+      })} added to the project`;
 
       const removedLine = `${miscUtils.plural(removedCount, {
         zero: `none were`,
@@ -2690,7 +2690,7 @@ function emitPeerDependencyWarnings(project: Project, report: Report) {
       formatUtils.pretty(project.configuration, warning.hash, formatUtils.Type.CODE)
     }), requested by ${
       structUtils.prettyIdent(project.configuration, warning.requester)
-    }`;
+    }.`;
   }) ?? [];
 
   report.startSectionSync({

packages/yarnpkg-core/sources/formatUtils.ts

@@ -476,7 +476,7 @@ export function prettyTruncatedLocatorList(configuration: Configuration, locator
       .slice(0, -2);
 
   const mark = `X`.repeat(locatorsCopy.length.toString().length);
-  const suffix = `and ${mark} more`;
+  const suffix = `and ${mark} more.`;
 
   let otherCount = locatorsCopy.length;
   while (named.length > 1 && remainingLength < suffix.length) {

packages/yarnpkg-core/tests/formatUtils.test.ts

@@ -27,11 +27,11 @@ describe(`formatUtils`, () => {
     });
 
     it(`should return cut the list of locators if needed`, () => {
-      expect(formatUtils.prettyTruncatedLocatorList(configuration, LOCATORS, 99)).toEqual(`foo@npm:1.0.0, foo@npm:2.0.0, foo@npm:3.0.0, foo@npm:4.0.0, foo@npm:5.0.0, and 2 more`);
+      expect(formatUtils.prettyTruncatedLocatorList(configuration, LOCATORS, 100)).toEqual(`foo@npm:1.0.0, foo@npm:2.0.0, foo@npm:3.0.0, foo@npm:4.0.0, foo@npm:5.0.0, and 2 more.`);
     });
 
     it(`should return cut the list of locators if needed (right on edge)`, () => {
-      expect(formatUtils.prettyTruncatedLocatorList(configuration, LOCATORS, 100)).toEqual(`foo@npm:1.0.0, foo@npm:2.0.0, foo@npm:3.0.0, foo@npm:4.0.0, foo@npm:5.0.0, foo@npm:6.0.0, and 1 more`);
+      expect(formatUtils.prettyTruncatedLocatorList(configuration, LOCATORS, 101)).toEqual(`foo@npm:1.0.0, foo@npm:2.0.0, foo@npm:3.0.0, foo@npm:4.0.0, foo@npm:5.0.0, foo@npm:6.0.0, and 1 more.`);
     });
   });
 });