Skip to content

Commit

Permalink
fix: berry prune including all builtin patch descriptors (#4770)
Browse files Browse the repository at this point in the history
### Description

Addresses the issue @erj826 found in
[#2791](#2791 (comment))

The issue came from us including all descriptors for patches included in
the pruned lockfile. This is fine for patches created by `yarn patch`
since this uses `resolutions` which results in the patch having a single
descriptor. Yarn also includes [some
patches](https://github.com/yarnpkg/berry/tree/master/packages/plugin-compat)
by default if they appear in your lockfile. These patches don't go
through `resolutions` and instead get routed through the
[`reduceDependency`
hook](https://yarnpkg.com/advanced/plugin-tutorial#hook-reduceDependency)
which produces a new descriptor for each descriptor used for the
original package.

The fix was to only include patch descriptors if the embedded descriptor
is present in the pruned lockfile.

### Testing Instructions

Added a new test fixture where multiple workspaces depend on `resolve`,
but use different ranges.
  • Loading branch information
chris-olszewski committed May 2, 2023
1 parent ac765e7 commit 2c7bad5
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 8 deletions.
103 changes: 103 additions & 0 deletions crates/turborepo-lockfiles/fixtures/berry-builtin.lock
@@ -0,0 +1,103 @@
# This file is generated by running "yarn install" inside your project.
# Manual changes might be lost - proceed with caution!

__metadata:
version: 6
cacheKey: 8

"a@workspace:packages/a":
version: 0.0.0-use.local
resolution: "a@workspace:packages/a"
dependencies:
c: "*"
resolve: ^1.22.2
languageName: unknown
linkType: soft

"b@workspace:packages/b":
version: 0.0.0-use.local
resolution: "b@workspace:packages/b"
dependencies:
c: "workspace:*"
resolve: 1.22.3
languageName: unknown
linkType: soft

"c@*, c@workspace:*, c@workspace:packages/c":
version: 0.0.0-use.local
resolution: "c@workspace:packages/c"
dependencies:
resolve: ^1.22.0
languageName: unknown
linkType: soft

"function-bind@npm:^1.1.1":
version: 1.1.1
resolution: "function-bind@npm:1.1.1"
checksum: b32fbaebb3f8ec4969f033073b43f5c8befbb58f1a79e12f1d7490358150359ebd92f49e72ff0144f65f2c48ea2a605bff2d07965f548f6474fd8efd95bf361a
languageName: node
linkType: hard

"has@npm:^1.0.3":
version: 1.0.3
resolution: "has@npm:1.0.3"
dependencies:
function-bind: ^1.1.1
checksum: b9ad53d53be4af90ce5d1c38331e712522417d017d5ef1ebd0507e07c2fbad8686fffb8e12ddecd4c39ca9b9b47431afbb975b8abf7f3c3b82c98e9aad052792
languageName: node
linkType: hard

"is-core-module@npm:^2.12.0":
version: 2.12.0
resolution: "is-core-module@npm:2.12.0"
dependencies:
has: ^1.0.3
checksum: f7f7eb2ab71fd769ee9fb2385c095d503aa4b5ce0028c04557de03f1e67a87c85e5bac1f215945fc3c955867a139a415a3ec4c4234a0bffdf715232660f440a6
languageName: node
linkType: hard

"path-parse@npm:^1.0.7":
version: 1.0.7
resolution: "path-parse@npm:1.0.7"
checksum: 49abf3d81115642938a8700ec580da6e830dde670be21893c62f4e10bd7dd4c3742ddc603fe24f898cba7eb0c6bc1777f8d9ac14185d34540c6d4d80cd9cae8a
languageName: node
linkType: hard

"prune-edge@workspace:.":
version: 0.0.0-use.local
resolution: "prune-edge@workspace:."
languageName: unknown
linkType: soft

"resolve@npm:1.22.3, resolve@npm:^1.22.0, resolve@npm:^1.22.2":
version: 1.22.3
resolution: "resolve@npm:1.22.3"
dependencies:
is-core-module: ^2.12.0
path-parse: ^1.0.7
supports-preserve-symlinks-flag: ^1.0.0
bin:
resolve: bin/resolve
checksum: fb834b81348428cb545ff1b828a72ea28feb5a97c026a1cf40aa1008352c72811ff4d4e71f2035273dc536dcfcae20c13604ba6283c612d70fa0b6e44519c374
languageName: node
linkType: hard

"resolve@patch:resolve@1.22.3#~builtin<compat/resolve>, resolve@patch:resolve@^1.22.0#~builtin<compat/resolve>, resolve@patch:resolve@^1.22.2#~builtin<compat/resolve>":
version: 1.22.3
resolution: "resolve@patch:resolve@npm%3A1.22.3#~builtin<compat/resolve>::version=1.22.3&hash=c3c19d"
dependencies:
is-core-module: ^2.12.0
path-parse: ^1.0.7
supports-preserve-symlinks-flag: ^1.0.0
bin:
resolve: bin/resolve
checksum: ad59734723b596d0891321c951592ed9015a77ce84907f89c9d9307dd0c06e11a67906a3e628c4cae143d3e44898603478af0ddeb2bba3f229a9373efe342665
languageName: node
linkType: hard

"supports-preserve-symlinks-flag@npm:^1.0.0":
version: 1.0.0
resolution: "supports-preserve-symlinks-flag@npm:1.0.0"
checksum: 53b1e247e68e05db7b3808b99b892bd36fb096e6fba213a06da7fab22045e97597db425c724f2bbd6c99a3c295e1e73f3e4de78592289f38431049e1277ca0ae
languageName: node
linkType: hard
39 changes: 31 additions & 8 deletions crates/turborepo-lockfiles/src/berry/mod.rs
Expand Up @@ -290,18 +290,13 @@ impl<'a> BerryLockfile<'a> {
resolutions.insert(dependency, dep_locator.clone());
}

// If the package has an associated patch we include it in the subgraph
if let Some(patch_locator) = self.patches.get(&locator) {
patches.insert(locator.as_owned(), patch_locator.clone());
let patch_descriptors = reverse_lookup
.get(patch_locator)
.unwrap_or_else(|| panic!("No descriptors found for {patch_locator}"));
for patch_descriptor in patch_descriptors {
resolutions.insert((*patch_descriptor).clone(), patch_locator.clone());
}
}
}

for patch in self.patches.values() {
for patch in patches.values() {
let patch_descriptors = reverse_lookup
.get(patch)
.unwrap_or_else(|| panic!("Unable to find {patch} in reverse lookup"));
Expand Down Expand Up @@ -780,7 +775,7 @@ mod test {
}

#[test]
fn test_workspace_collission() {
fn test_workspace_collision() {
let data = LockfileData::from_bytes(include_bytes!(
"../../fixtures/berry-protocol-collision.lock"
))
Expand Down Expand Up @@ -819,4 +814,32 @@ mod test {
.collect::<HashSet<_>>())
);
}

#[test]
fn test_builtin_patch_descriptors() {
let data =
LockfileData::from_bytes(include_bytes!("../../fixtures/berry-builtin.lock")).unwrap();
let lockfile = BerryLockfile::new(&data, None).unwrap();
let subgraph = lockfile
.subgraph(
&["packages/a".into(), "packages/c".into()],
&["resolve@npm:1.22.3".into()],
)
.unwrap();
let subgraph_data = subgraph.lockfile().unwrap();
let (resolve_key, _) = subgraph_data
.packages
.iter()
.find(|(_, v)| {
v.resolution
== "resolve@patch:resolve@npm%3A1.22.3#~builtin<compat/resolve>::version=1.22.\
3&hash=c3c19d"
})
.unwrap();
assert_eq!(
resolve_key,
"resolve@patch:resolve@^1.22.0#~builtin<compat/resolve>, \
resolve@patch:resolve@^1.22.2#~builtin<compat/resolve>"
);
}
}

0 comments on commit 2c7bad5

Please sign in to comment.