From 98432f3cc0edb36de2886854370de5b6dcb0201f Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Fri, 28 Apr 2023 15:59:45 -0700 Subject: [PATCH] allow for workplace descriptors without a protocol --- .../fixtures/berry-protocol-collision.lock | 33 ++++++++++++ crates/turborepo-lockfiles/src/berry/mod.rs | 50 +++++++++++++++++-- .../src/berry/protocol_resolver.rs | 38 ++++++++++++-- 3 files changed, 113 insertions(+), 8 deletions(-) create mode 100644 crates/turborepo-lockfiles/fixtures/berry-protocol-collision.lock diff --git a/crates/turborepo-lockfiles/fixtures/berry-protocol-collision.lock b/crates/turborepo-lockfiles/fixtures/berry-protocol-collision.lock new file mode 100644 index 0000000000000..0450964d02950 --- /dev/null +++ b/crates/turborepo-lockfiles/fixtures/berry-protocol-collision.lock @@ -0,0 +1,33 @@ +# This file is generated by running "yarn install" inside your project. +# Manual changes might be lost - proceed with caution! + +__metadata: + version: 6 + +"a@workspace:packages/a": + version: 0.0.0-use.local + resolution: "a@workspace:packages/a" + dependencies: + c: "*" + languageName: unknown + linkType: soft + +"b@workspace:packages/b": + version: 0.0.0-use.local + resolution: "b@workspace:packages/b" + dependencies: + c: "workspace:*" + languageName: unknown + linkType: soft + +"c@*, c@workspace:*, c@workspace:packages/c": + version: 0.0.0-use.local + resolution: "c@workspace:packages/c" + languageName: unknown + linkType: soft + +"prune-edge@workspace:.": + version: 0.0.0-use.local + resolution: "prune-edge@workspace:." + languageName: unknown + linkType: soft diff --git a/crates/turborepo-lockfiles/src/berry/mod.rs b/crates/turborepo-lockfiles/src/berry/mod.rs index 7e1172383a1dc..1380cca4bb1f6 100644 --- a/crates/turborepo-lockfiles/src/berry/mod.rs +++ b/crates/turborepo-lockfiles/src/berry/mod.rs @@ -266,11 +266,10 @@ impl<'a> BerryLockfile<'a> { resolutions.insert(dependency, dep_locator.clone()); } - if let Some(descriptors) = reverse_lookup.get(locator) { - for descriptor in descriptors { - resolutions.insert((*descriptor).clone(), locator.clone()); - } - } + // Included workspaces will always have their locator listed as a descriptor. + // All other descriptors should show up in the other workspace package + // dependencies. + resolutions.insert(Descriptor::from(locator.clone()), locator.clone()); } } @@ -779,4 +778,45 @@ mod test { version: "4.4.1".into() })); } + + #[test] + fn test_workspace_collission() { + let data = LockfileData::from_bytes(include_bytes!( + "../../fixtures/berry-protocol-collision.lock" + )) + .unwrap(); + let lockfile = BerryLockfile::new(&data, None).unwrap(); + let no_proto = Descriptor::try_from("c@*").unwrap(); + let workspace_proto = Descriptor::try_from("c@workspace:*").unwrap(); + let full_path = Descriptor::try_from("c@workspace:packages/c").unwrap(); + let a_lockfile = lockfile + .subgraph(&["packages/a".into(), "packages/c".into()], &[]) + .unwrap(); + let a_reverse_lookup = a_lockfile.locator_to_descriptors(); + let a_c_descriptors = a_reverse_lookup + .get(&Locator::try_from("c@workspace:packages/c").unwrap()) + .unwrap(); + + assert_eq!( + a_c_descriptors, + &(vec![&no_proto, &full_path] + .into_iter() + .collect::>()) + ); + + let b_lockfile = lockfile + .subgraph(&["packages/b".into(), "packages/c".into()], &[]) + .unwrap(); + let b_reverse_lookup = b_lockfile.locator_to_descriptors(); + let b_c_descriptors = b_reverse_lookup + .get(&Locator::try_from("c@workspace:packages/c").unwrap()) + .unwrap(); + + assert_eq!( + b_c_descriptors, + &(vec![&workspace_proto, &full_path] + .into_iter() + .collect::>()) + ); + } } diff --git a/crates/turborepo-lockfiles/src/berry/protocol_resolver.rs b/crates/turborepo-lockfiles/src/berry/protocol_resolver.rs index b773284d79db5..fec7eaa9929ad 100644 --- a/crates/turborepo-lockfiles/src/berry/protocol_resolver.rs +++ b/crates/turborepo-lockfiles/src/berry/protocol_resolver.rs @@ -5,7 +5,7 @@ use super::identifiers::{Descriptor, Ident}; /// A data structure for resolving descriptors when the protocol isn't known #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct DescriptorResolver<'a> { - mapping: HashMap, &'a str>, + mapping: HashMap, Entry<'a>>, } #[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] @@ -14,18 +14,25 @@ struct Key<'a> { range: &'a str, } +#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Default)] +struct Entry<'a> { + without: Option<&'a str>, + with: Option<&'a str>, +} + impl<'a> DescriptorResolver<'a> { /// Add a descriptor to the resolver pub fn insert(&mut self, descriptor: &Descriptor<'a>) -> Option<&'a str> { let key = Key::new(descriptor)?; - self.mapping.insert(key, descriptor.range()?) + let entry = self.mapping.entry(key).or_default(); + entry.insert_descriptor(descriptor) } /// If given a descriptor without a protocol it will return all matching /// descriptors with a protocol pub fn get(&self, descriptor: &Descriptor) -> Option<&'a str> { let key = Key::new(descriptor)?; - self.mapping.get(&key).copied() + self.mapping.get(&key).and_then(|e| e.get(descriptor)) } } @@ -37,6 +44,28 @@ impl<'a> Key<'a> { } } +impl<'a> Entry<'a> { + // Insert the given descriptor's range into the correct slot depending if it is + // with or without a protocol + fn insert_descriptor(&mut self, descriptor: &Descriptor<'a>) -> Option<&'a str> { + let range = descriptor.range()?; + match descriptor.protocol().is_some() { + true => self.with.replace(range), + false => self.without.replace(range), + } + } + + fn get(&self, descriptor: &Descriptor) -> Option<&'a str> { + // We only return the without protocol range if `without` is present + // and the given descriptor is also without a protocol + if self.without.is_some() && descriptor.protocol().is_none() { + self.without + } else { + self.with + } + } +} + #[cfg(test)] mod test { use super::*; @@ -58,7 +87,10 @@ mod test { fn test_descriptors_without_protocols() { let mut resolver = DescriptorResolver::default(); let workspace = Descriptor::new("internal-workspace", "*").unwrap(); + let workspace_with_protocol = Descriptor::new("internal-workspace", "workspace:*").unwrap(); assert!(resolver.insert(&workspace).is_none()); + assert!(resolver.insert(&workspace_with_protocol).is_none()); assert_eq!(resolver.get(&workspace), Some("*")); + assert_eq!(resolver.get(&workspace_with_protocol), Some("workspace:*")); } }