Skip to content

Commit bd545d8

Browse files
authored
Deprecates implicit cache keys (#6083)
**What's the problem this PR addresses?** The lockfile has a very small optimization feature: instead of repeating the cache key across all checksums, it keeps a global cache key at the top of the file, which it uses everytime a checksum is missing a cache key. This saves a few bytes. However, I found out this behaviour causes breakages under some situations: - Alice creates a feature branch from main and add a new dependency `foo` - Bob upgrades Yarn to a new version that compiles zip files differently - Alice merges the new main into her feature branch - Git merges the two lockfiles together; let's say there are no conflicts - The `foo` entry added by Alice has no cache key, since it was added at a time its cache key matched the global one - However it doesn't match the new global one, which was updated at the top of the file **How did you fix it?** The checksum cache key is now always set. **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.
1 parent 3724cf3 commit bd545d8

File tree

9 files changed

+1923
-1829
lines changed

9 files changed

+1923
-1829
lines changed

.pnp.cjs

Lines changed: 4 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.yarn/versions/c9849542.yml

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
releases:
2+
"@yarnpkg/cli": patch
3+
"@yarnpkg/core": patch
4+
"@yarnpkg/libzip": patch
5+
6+
declined:
7+
- "@yarnpkg/plugin-compat"
8+
- "@yarnpkg/plugin-constraints"
9+
- "@yarnpkg/plugin-dlx"
10+
- "@yarnpkg/plugin-essentials"
11+
- "@yarnpkg/plugin-exec"
12+
- "@yarnpkg/plugin-file"
13+
- "@yarnpkg/plugin-git"
14+
- "@yarnpkg/plugin-github"
15+
- "@yarnpkg/plugin-http"
16+
- "@yarnpkg/plugin-init"
17+
- "@yarnpkg/plugin-interactive-tools"
18+
- "@yarnpkg/plugin-link"
19+
- "@yarnpkg/plugin-nm"
20+
- "@yarnpkg/plugin-npm"
21+
- "@yarnpkg/plugin-npm-cli"
22+
- "@yarnpkg/plugin-pack"
23+
- "@yarnpkg/plugin-patch"
24+
- "@yarnpkg/plugin-pnp"
25+
- "@yarnpkg/plugin-pnpm"
26+
- "@yarnpkg/plugin-stage"
27+
- "@yarnpkg/plugin-typescript"
28+
- "@yarnpkg/plugin-version"
29+
- "@yarnpkg/plugin-workspace-tools"
30+
- vscode-zipfs
31+
- "@yarnpkg/builder"
32+
- "@yarnpkg/doctor"
33+
- "@yarnpkg/extensions"
34+
- "@yarnpkg/fslib"
35+
- "@yarnpkg/nm"
36+
- "@yarnpkg/pnp"
37+
- "@yarnpkg/pnpify"
38+
- "@yarnpkg/sdks"

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

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import {execUtils} from '@yarnpkg/core';
12
import {Filename, ppath, xfs} from '@yarnpkg/fslib';
23
import {tests} from 'pkg-tests-core';
34

@@ -198,6 +199,71 @@ describe(`Cache`, () => {
198199
}),
199200
);
200201

202+
test(`it should not confuse the cache key when merging an upgraded cache key branch into a feature branch`, makeTemporaryEnv({
203+
dependencies: {
204+
[`depA`]: `npm:[email protected]`,
205+
[`depB`]: `npm:[email protected]`,
206+
[`depC`]: `npm:[email protected]`,
207+
},
208+
}, async ({path, run, source}) => {
209+
await run(`install`);
210+
211+
// First we create the main branch; it contains a bunch of various dependencies
212+
213+
await execUtils.execvp(`git`, [`init`], {cwd: path});
214+
215+
await execUtils.execvp(`git`, [`add`, `.`], {cwd: path});
216+
await execUtils.execvp(`git`, [`commit`, `-m`, `test`], {cwd: path});
217+
218+
// We now create a new feature branch derived from the base one, and we add a new dependency
219+
220+
await execUtils.execvp(`git`, [`checkout`, `-b`, `feature`], {cwd: path});
221+
222+
await run(`add`, `depX@npm:[email protected]`);
223+
224+
await execUtils.execvp(`git`, [`add`, `.`], {cwd: path});
225+
await execUtils.execvp(`git`, [`commit`, `-m`, `new dep`], {cwd: path});
226+
227+
// Meanwhile, the base branch is updated with a new Yarn version:
228+
//
229+
// - we add an extra byte at the end of each file in the cache, to simulate a change in how zip archives are generated
230+
// - we run an install with a new cache version and we persist the new checksums
231+
232+
await execUtils.execvp(`git`, [`checkout`, `-`], {cwd: path});
233+
234+
await run(`install`, {
235+
cacheVersionOverride: `2`,
236+
zipDataEpilogue: `<arbitrary data>`,
237+
});
238+
239+
//throw new Error(`lol`);
240+
241+
await execUtils.execvp(`git`, [`add`, `.`], {cwd: path});
242+
await execUtils.execvp(`git`, [`commit`, `-m`, `fake upgrade`], {cwd: path});
243+
244+
// Going back to our feature branch, we now merge the updated base branch into it
245+
246+
await execUtils.execvp(`git`, [`checkout`, `-`], {cwd: path});
247+
248+
await execUtils.execvp(`git`, [`merge`, `-`], {cwd: path});
249+
250+
// Running an install should work fine
251+
252+
await run(`install`, {
253+
cacheVersionOverride: `2`,
254+
zipDataEpilogue: `<arbitrary data>`,
255+
});
256+
257+
// However, once we remove the cache and try to reinstall, we used to get an error because the wrong cache key had been encoded in the lockfile
258+
259+
await xfs.removePromise(ppath.join(path, `.yarn/cache`));
260+
261+
await run(`install`, {
262+
cacheVersionOverride: `2`,
263+
zipDataEpilogue: `<arbitrary data>`,
264+
});
265+
}));
266+
201267
test(
202268
`it should ignore changes in the cache compression, if cacheMigrationMode=required-only`,
203269
makeTemporaryEnv({

packages/yarnpkg-core/sources/Configuration.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ const IGNORED_ENV_VARIABLES = new Set([
6969
`injectNpmUser`,
7070
`injectNpmPassword`,
7171
`injectNpm2FaToken`,
72+
`zipDataEpilogue`,
7273
`cacheCheckpointOverride`,
7374
`cacheVersionOverride`,
7475
`lockfileVersionOverride`,

packages/yarnpkg-core/sources/Project.ts

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1969,23 +1969,6 @@ export class Project {
19691969

19701970
manifest.bin = new Map(pkg.bin);
19711971

1972-
let entryChecksum: string | undefined;
1973-
const checksum = this.storedChecksums.get(pkg.locatorHash);
1974-
if (typeof checksum !== `undefined`) {
1975-
const cacheKeyIndex = checksum.indexOf(`/`);
1976-
if (cacheKeyIndex === -1)
1977-
throw new Error(`Assertion failed: Expected the checksum to reference its cache key`);
1978-
1979-
const entryCacheKey = checksum.slice(0, cacheKeyIndex);
1980-
const hash = checksum.slice(cacheKeyIndex + 1);
1981-
1982-
if (entryCacheKey === cacheKey) {
1983-
entryChecksum = hash;
1984-
} else {
1985-
entryChecksum = checksum;
1986-
}
1987-
}
1988-
19891972
optimizedLockfile[key] = {
19901973
...manifest.exportTo({}, {
19911974
compatibilityMode: false,
@@ -1994,7 +1977,7 @@ export class Project {
19941977
linkType: pkg.linkType.toLowerCase(),
19951978

19961979
resolution: structUtils.stringifyLocator(pkg),
1997-
checksum: entryChecksum,
1980+
checksum: this.storedChecksums.get(pkg.locatorHash),
19981981

19991982
conditions: pkg.conditions || undefined,
20001983
};

packages/yarnpkg-core/sources/worker-zip/index.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/yarnpkg-libzip/sources/ZipFS.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,12 @@ export class ZipFS extends BasePortableFakeFS {
295295
else if (rc > size)
296296
throw new Error(`Overread`);
297297

298-
const memory = this.libzip.HEAPU8.subarray(buffer, buffer + size);
299-
return Buffer.from(memory);
298+
let result = Buffer.from(this.libzip.HEAPU8.subarray(buffer, buffer + size));
299+
300+
if (process.env.YARN_IS_TEST_ENV && process.env.YARN_ZIP_DATA_EPILOGUE)
301+
result = Buffer.concat([result, Buffer.from(process.env.YARN_ZIP_DATA_EPILOGUE)]);
302+
303+
return result;
300304
} finally {
301305
this.libzip.free(buffer);
302306
}

packages/yarnpkg-pnp/sources/hook.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)