feat(lockfile) hide berry lockfile lifetime (#5436)

### Description

In order to let the package graph hold a lockfile we need to avoid
exposing a lifetime in the berry implementation. This PR changes the
internal representations to all be completely owned.

Alternatives considered:
- Continue with the self-referential struct, but have the lockfile
pinned and then use `mem::transmute` to mutate the required lifetimes to
be `static`. Attempted this, but ran into enough issues that it wasn't
looking like a quick refactor.
- Use
[ouroboros](https://docs.rs/ouroboros/0.17.0/ouroboros/attr.self_referencing.html)
to allow for the self-referential struct via the macro. In the future
this might be worth a look, but I didn't want to block on verifying that
we currently follow and in the future will follow all of the
restrictions this macro imposes.

### Testing Instructions

This shouldn't change any behavior, just makes it so doesn't require any
additional data.

`cargo instruments --example berry_resolutions -t time` shows that
execution is `68ms` which is still very reasonable. These changes do
increase memory pressure somewhat, but it's not unreasonable at all.

---------

Co-authored-by: Chris Olszewski <Chris Olszewski>

crates/turborepo-ffi/src/lockfile.rs

@@ -85,7 +85,7 @@ fn berry_transitive_closure_inner(
  let resolutions =
  resolutions.map(|r| turborepo_lockfiles::BerryManifest::with_resolutions(r.resolutions));
  let data = LockfileData::from_bytes(contents.as_slice())?;
- let lockfile = BerryLockfile::new(&data, resolutions.as_ref())?;
+ let lockfile = BerryLockfile::new(data, resolutions)?;
  let dependencies = turborepo_lockfiles::all_transitive_closures(
  &lockfile,
  workspaces.into_iter().map(|(k, v)| (k, v.into())).collect(),
@@ -183,7 +183,7 @@ fn patches_internal(buf: Buffer) -> Result<proto::Patches, Error> {
  let patches = match request.package_manager() {
  proto::PackageManager::Berry => {
  let data = LockfileData::from_bytes(&request.contents)?;
- let lockfile = BerryLockfile::new(&data, None)?;
+ let lockfile = BerryLockfile::new(data, None)?;
  Ok(lockfile
  .patches()
  .into_iter()

crates/turborepo-lockfiles/Cargo.toml

@@ -12,7 +12,7 @@ pest = "2.5.6"
 pest_derive = "2.5.6"
 regex = "1"
 semver = "1.0.17"
-serde = { version = "1.0.126", features = ["derive"] }
+serde = { version = "1.0.126", features = ["derive", "rc"] }
 serde_json = "1.0.86"
 serde_yaml = "0.9"
 thiserror = "1.0.38"

crates/turborepo-lockfiles/examples/berry_resolutions.rs

@@ -4,7 +4,7 @@ fn main() {
  let manifest = generate_manifest("foobar", 100);
  let lockfile_bytes = include_bytes!("yarn.lock");
  let data = LockfileData::from_bytes(lockfile_bytes.as_slice()).unwrap();
- let lockfile = BerryLockfile::new(&data, Some(&manifest)).unwrap();
+ let lockfile = BerryLockfile::new(data, Some(manifest)).unwrap();
  let key = "debug@npm:3.2.7";
  println!(
  "Dependencies of {key}: {}",

crates/turborepo-lockfiles/src/berry/mod.rs

@@ -8,6 +8,7 @@ use std::{
  collections::{HashMap, HashSet},
  iter,
  path::Path,
+ rc::Rc,
 };
 
 use de::SemverString;
@@ -38,18 +39,18 @@ pub enum Error {
 // We depend on BTree iteration being sorted for correct serialization
 type Map<K, V> = std::collections::BTreeMap<K, V>;
 
-pub struct BerryLockfile<'a> {
- data: &'a LockfileData,
- resolutions: Map<Descriptor<'a>, Locator<'a>>,
+pub struct BerryLockfile {
+ data: Rc<LockfileData>,
+ resolutions: Map<Descriptor<'static>, Locator<'static>>,
  // A mapping from descriptors without protocols to a range with a protocol
- resolver: DescriptorResolver<'a>,
- locator_package: Map<Locator<'a>, &'a BerryPackage>,
+ resolver: DescriptorResolver,
+ locator_package: Map<Locator<'static>, Rc<BerryPackage>>,
  // Map of regular locators to patch locators that apply to them
- patches: Map<Locator<'static>, Locator<'a>>,
+ patches: Map<Locator<'static>, Locator<'static>>,
  // Descriptors that come from default package extensions that ship with berry
  extensions: HashSet<Descriptor<'static>>,
  // Package overrides
- overrides: Map<Resolution<'a>, &'a str>,
+ overrides: Map<Resolution, String>,
 }
 
 // This is the direct representation of the lockfile as it appears on disk.
@@ -59,7 +60,7 @@ pub struct LockfileData {
  #[serde(rename = "__metadata")]
  metadata: Metadata,
  #[serde(flatten)]
- packages: Map<String, BerryPackage>,
+ packages: Map<String, Rc<BerryPackage>>,
 }
 
 #[derive(Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Clone)]
@@ -97,11 +98,8 @@ pub struct BerryManifest {
  resolutions: Option<Map<String, String>>,
 }
 
-impl<'a> BerryLockfile<'a> {
- pub fn new(
- lockfile: &'a LockfileData,
- manifest: Option<&'a BerryManifest>,
- ) -> Result<Self, Error> {
+impl BerryLockfile {
+ pub fn new(lockfile: LockfileData, manifest: Option<BerryManifest>) -> Result<Self, Error> {
  let mut patches = Map::new();
  let mut locator_package = Map::new();
  let mut descriptor_locator = Map::new();
@@ -113,17 +111,17 @@ impl<'a> BerryLockfile<'a> {
  let original_locator = locator
  .patched_locator()
  .ok_or_else(|| Error::PatchMissingOriginalLocator(locator.as_owned()))?;
- patches.insert(original_locator.as_owned(), locator.clone());
+ patches.insert(original_locator.as_owned(), locator.as_owned());
  }
 
- locator_package.insert(locator.clone(), package);
+ locator_package.insert(locator.as_owned(), package.clone());
 
  for descriptor in Descriptor::from_lockfile_key(key) {
  let descriptor = descriptor?;
  if let Some(other) = resolver.insert(&descriptor) {
  panic!("Descriptor collision {descriptor} and {other}");
  }
- descriptor_locator.insert(descriptor, locator.clone());
+ descriptor_locator.insert(descriptor.into_owned(), locator.as_owned());
  }
  }
 
@@ -133,7 +131,7 @@ impl<'a> BerryLockfile<'a> {
  .unwrap_or_default();
 
  let mut this = Self {
- data: lockfile,
+ data: Rc::new(lockfile),
  resolutions: descriptor_locator,
  locator_package,
  resolver,
@@ -184,7 +182,7 @@ impl<'a> BerryLockfile<'a> {
  }
 
  // Helper function for inverting the resolution map
- fn locator_to_descriptors(&self) -> HashMap<&Locator<'a>, HashSet<&Descriptor<'a>>> {
+ fn locator_to_descriptors(&self) -> HashMap<&Locator<'static>, HashSet<&Descriptor<'static>>> {
  let mut reverse_lookup: HashMap<&Locator, HashSet<&Descriptor>> =
  HashMap::with_capacity(self.locator_package.len());
 
@@ -200,7 +198,7 @@ impl<'a> BerryLockfile<'a> {
 
  /// Constructs a new lockfile data ready to be serialized
  pub fn lockfile(&self) -> Result<LockfileData, Error> {
- let mut packages: std::collections::BTreeMap<String, BerryPackage> = Map::new();
+ let mut packages = Map::new();
  let mut metadata = self.data.metadata.clone();
  let reverse_lookup = self.locator_to_descriptors();
 
@@ -216,7 +214,7 @@ impl<'a> BerryLockfile<'a> {
  .locator_package
  .get(locator)
  .ok_or_else(|| Error::MissingPackageForLocator(locator.as_owned()))?;
- packages.insert(key, (*package).clone());
+ packages.insert(key, package.clone());
  }
 
  // If there aren't any checksums in the lockfile, then cache key is omitted
@@ -242,7 +240,7 @@ impl<'a> BerryLockfile<'a> {
  &self,
  workspace_packages: &[String],
  packages: &[String],
- ) -> Result<BerryLockfile<'a>, Error> {
+ ) -> Result<BerryLockfile, Error> {
  let reverse_lookup = self.locator_to_descriptors();
 
  let mut resolutions = Map::new();
@@ -279,6 +277,7 @@ impl<'a> BerryLockfile<'a> {
  let package = self
  .locator_package
  .get(&locator)
+ .cloned()
  .ok_or_else(|| Error::MissingPackageForLocator(locator.as_owned()))?;
 
  for (name, range) in package.dependencies.iter().flatten() {
@@ -327,7 +326,7 @@ impl<'a> BerryLockfile<'a> {
  }
 
  Ok(Self {
- data: self.data,
+ data: self.data.clone(),
  resolutions,
  patches,
  // We clone the following structures without any alterations and
@@ -342,9 +341,9 @@ impl<'a> BerryLockfile<'a> {
  fn resolve_dependency(
  &self,
  locator: &Locator,
- name: &'a str,
- range: &'a str,
- ) -> Result<Descriptor<'a>, Error> {
+ name: &str,
+ range: &str,
+ ) -> Result<Descriptor<'static>, Error> {
  let mut dependency = Descriptor::new(name, range)?;
  // If there's no protocol we attempt to find a known one
  if dependency.protocol().is_none() {
@@ -362,11 +361,12 @@ impl<'a> BerryLockfile<'a> {
  }
  }
 
- Ok(dependency)
+ // TODO Could we dedupe and wrap in Rc?
+ Ok(dependency.into_owned())
  }
 }
 
-impl<'a> Lockfile for BerryLockfile<'a> {
+impl Lockfile for BerryLockfile {
  fn resolve_package(
  &self,
  workspace_path: &str,
@@ -449,13 +449,13 @@ impl BerryManifest {
  Self { resolutions }
  }
 
- pub fn resolutions(&self) -> Option<Result<Map<Resolution, &str>, Error>> {
- self.resolutions.as_ref().map(|resolutions| {
+ pub fn resolutions(self) -> Option<Result<Map<Resolution, String>, Error>> {
+ self.resolutions.map(|resolutions| {
  resolutions
- .iter()
+ .into_iter()
  .map(|(resolution, reference)| {
- let res = parse_resolution(resolution)?;
- Ok((res, reference.as_str()))
+ let res = parse_resolution(&resolution)?;
+ Ok((res, reference))
  })
  .collect()
  })
@@ -470,7 +470,7 @@ pub fn berry_subgraph(
 ) -> Result<Vec<u8>, Error> {
  let manifest = resolutions.map(BerryManifest::with_resolutions);
  let data = LockfileData::from_bytes(contents)?;
- let lockfile = BerryLockfile::new(&data, manifest.as_ref())?;
+ let lockfile = BerryLockfile::new(data, manifest)?;
  let pruned_lockfile = lockfile.subgraph(workspace_packages, packages)?;
  let new_contents = pruned_lockfile.lockfile()?.to_string().into_bytes();
  Ok(new_contents)
@@ -510,7 +510,7 @@ mod test {
  fn test_resolve_package() {
  let data: LockfileData =
  serde_yaml::from_str(include_str!("../../fixtures/berry.lock")).unwrap();
- let lockfile = BerryLockfile::new(&data, None).unwrap();
+ let lockfile = BerryLockfile::new(data, None).unwrap();
 
  assert_eq!(
  lockfile
@@ -551,7 +551,7 @@ mod test {
  fn test_all_dependencies() {
  let data: LockfileData =
  serde_yaml::from_str(include_str!("../../fixtures/berry.lock")).unwrap();
- let lockfile = BerryLockfile::new(&data, None).unwrap();
+ let lockfile = BerryLockfile::new(data, None).unwrap();
 
  let pkg = lockfile
  .resolve_package("apps/docs", "react-dom", "18.2.0")
@@ -574,7 +574,7 @@ mod test {
  fn test_package_extension_detection() {
  let data: LockfileData =
  serde_yaml::from_str(include_str!("../../fixtures/berry.lock")).unwrap();
- let lockfile = BerryLockfile::new(&data, None).unwrap();
+ let lockfile = BerryLockfile::new(data, None).unwrap();
 
  assert_eq!(
  &lockfile.extensions,
@@ -589,7 +589,7 @@ mod test {
  fn test_patch_list() {
  let data: LockfileData =
  serde_yaml::from_str(include_str!("../../fixtures/berry.lock")).unwrap();
- let lockfile = BerryLockfile::new(&data, None).unwrap();
+ let lockfile = BerryLockfile::new(data, None).unwrap();
 
  let locator = Locator::try_from("resolve@npm:2.0.0-next.4").unwrap();
 
@@ -602,7 +602,7 @@ mod test {
  fn test_empty_patch_list() {
  let data =
  LockfileData::from_bytes(include_bytes!("../../fixtures/minimal-berry.lock")).unwrap();
- let lockfile = BerryLockfile::new(&data, None).unwrap();
+ let lockfile = BerryLockfile::new(data, None).unwrap();
 
  let empty_vec: Vec<&Path> = Vec::new();
  assert_eq!(lockfile.patches(), empty_vec);
@@ -612,7 +612,7 @@ mod test {
  fn test_basic_descriptor_prune() {
  let data: LockfileData =
  serde_yaml::from_str(include_str!("../../fixtures/minimal-berry.lock")).unwrap();
- let lockfile = BerryLockfile::new(&data, None).unwrap();
+ let lockfile = BerryLockfile::new(data, None).unwrap();
 
  let pruned_lockfile = lockfile
  .subgraph(
@@ -648,7 +648,7 @@ mod test {
  "lodash@^4.17.21".into(),
  "patch:lodash@npm%3A4.17.21#./.yarn/patches/lodash-npm-4.17.21-6382451519.patch".into(),
  )]);
- let lockfile = BerryLockfile::new(&data, Some(&resolutions)).unwrap();
+ let lockfile = BerryLockfile::new(data, Some(resolutions)).unwrap();
  let closure = crate::transitive_closure(
  &lockfile,
  "apps/docs",
@@ -676,7 +676,7 @@ mod test {
  .collect(),
  ),
  };
- let lockfile = BerryLockfile::new(&data, Some(&manifest)).unwrap();
+ let lockfile = BerryLockfile::new(data, Some(manifest)).unwrap();
 
  let pkg = lockfile
  .resolve_package("packages/b", "debug", "^4.3.4")
@@ -709,7 +709,7 @@ mod test {
  .collect(),
  ),
  };
- let lockfile = BerryLockfile::new(&data, Some(&manifest)).unwrap();
+ let lockfile = BerryLockfile::new(data, Some(manifest)).unwrap();
 
  let deps = lockfile
  .all_dependencies("debug@npm:1.0.0")
@@ -749,7 +749,7 @@ mod test {
  .collect(),
  ),
  };
- let lockfile = BerryLockfile::new(&data, Some(&manifest)).unwrap();
+ let lockfile = BerryLockfile::new(data, Some(manifest)).unwrap();
 
  let unresolved_deps = vec![
  ("@types/react-dom", "^17.0.11"),
@@ -780,7 +780,7 @@ mod test {
  "../../fixtures/berry-protocol-collision.lock"
  ))
  .unwrap();
- let lockfile = BerryLockfile::new(&data, None).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();
@@ -819,7 +819,7 @@ mod 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 lockfile = BerryLockfile::new(data, None).unwrap();
  let subgraph = lockfile
  .subgraph(
  &["packages/a".into(), "packages/c".into()],