From 3a061a9fbe5d635b616fdde348dbb36fbde97355 Mon Sep 17 00:00:00 2001 From: daimond113 <72147841+daimond113@users.noreply.github.com> Date: Mon, 25 Mar 2024 17:29:31 +0100 Subject: [PATCH] refactor(resolver): :art: improve lockfile format --- src/cli/root.rs | 45 ++-- src/dependencies/mod.rs | 14 +- src/dependencies/resolution.rs | 363 +++++++++++++++++---------------- src/index.rs | 6 +- src/lib.rs | 4 + src/linking_file.rs | 59 ++++-- src/manifest.rs | 27 ++- src/patches.rs | 7 +- src/project.rs | 38 ++-- tests/resolver.rs | 10 +- 10 files changed, 294 insertions(+), 279 deletions(-) diff --git a/src/cli/root.rs b/src/cli/root.rs index 5c644b8..aa9e116 100644 --- a/src/cli/root.rs +++ b/src/cli/root.rs @@ -80,15 +80,10 @@ pub fn root_command(cmd: Command) -> anyhow::Result<()> { .indices .clone() .into_iter() - .map(|(k, v)| (k, Box::new(clone_index(&v)) as Box)); + .map(|(k, v)| (k, Box::new(clone_index(&v)) as Box)) + .collect::>(); - Project::new( - CWD.to_path_buf(), - CLI_CONFIG.cache_dir(), - HashMap::from_iter(indices), - manifest, - ) - .unwrap() + Project::new(CWD.to_path_buf(), CLI_CONFIG.cache_dir(), indices, manifest).unwrap() }); match cmd { @@ -104,18 +99,18 @@ pub fn root_command(cmd: Command) -> anyhow::Result<()> { } let manifest = project.manifest().clone(); - let resolved_versions_map = manifest.dependency_tree(&mut project, locked)?; + let lockfile = manifest.dependency_graph(&mut project, locked)?; - let download_job = project.download(resolved_versions_map.clone())?; + let download_job = project.download(&lockfile)?; multithreaded_bar( download_job, - resolved_versions_map.len() as u64, + lockfile.children.len() as u64, "Downloading packages".to_string(), )?; #[allow(unused_variables)] - project.convert_manifests(&resolved_versions_map, |path| { + project.convert_manifests(&lockfile, |path| { cfg_if! { if #[cfg(feature = "wally")] { if let Some(sourcemap_generator) = &manifest.sourcemap_generator { @@ -145,7 +140,7 @@ pub fn root_command(cmd: Command) -> anyhow::Result<()> { InstallOptions::new() .locked(locked) .auto_download(false) - .resolved_versions_map(resolved_versions_map), + .lockfile(lockfile), )?; } Command::Run { package, args } => { @@ -153,17 +148,18 @@ pub fn root_command(cmd: Command) -> anyhow::Result<()> { .lockfile()? .ok_or(anyhow::anyhow!("lockfile not found"))?; - let (_, resolved_pkg) = lockfile + let resolved_pkg = lockfile + .children .get(&package.into()) - .and_then(|versions| versions.iter().find(|(_, pkg_ref)| pkg_ref.is_root)) + .and_then(|versions| { + versions + .values() + .find(|pkg_ref| lockfile.root_specifier(pkg_ref).is_some()) + }) .ok_or(anyhow::anyhow!( "package not found in lockfile (or isn't root)" ))?; - if !resolved_pkg.is_root { - anyhow::bail!("package is not a root package"); - } - let pkg_path = resolved_pkg.directory(project.path()).1; let manifest = Manifest::from_path(&pkg_path)?; @@ -278,6 +274,7 @@ pub fn root_command(cmd: Command) -> anyhow::Result<()> { .ok_or(anyhow::anyhow!("lockfile not found"))?; let resolved_pkg = lockfile + .children .get(&package.0) .and_then(|versions| versions.get(&package.1)) .ok_or(anyhow::anyhow!("package not found in lockfile"))?; @@ -509,15 +506,15 @@ pub fn root_command(cmd: Command) -> anyhow::Result<()> { let project = Lazy::force_mut(&mut project); let manifest = project.manifest().clone(); - let dependency_tree = manifest.dependency_tree(project, false)?; + let lockfile = manifest.dependency_graph(project, false)?; - for (name, versions) in dependency_tree { + for (name, versions) in &lockfile.children { for (version, resolved_pkg) in versions { - if !resolved_pkg.is_root { + if lockfile.root_specifier(resolved_pkg).is_none() { continue; } - if let PackageRef::Registry(ref registry) = resolved_pkg.pkg_ref { + if let PackageRef::Registry(registry) = &resolved_pkg.pkg_ref { let latest_version = send_request(REQWEST_CLIENT.get(format!( "{}/v0/packages/{}/{}/versions", resolved_pkg.pkg_ref.get_index(project).config()?.api(), @@ -533,7 +530,7 @@ pub fn root_command(cmd: Command) -> anyhow::Result<()> { "failed to get latest version of {name}@{version}" ))?; - if latest_version > version { + if &latest_version > version { println!( "{name}@{version} is outdated. latest version: {latest_version}" ); diff --git a/src/dependencies/mod.rs b/src/dependencies/mod.rs index 831bc6e..e6fdaa9 100644 --- a/src/dependencies/mod.rs +++ b/src/dependencies/mod.rs @@ -18,7 +18,7 @@ use crate::{ dependencies::{ git::{GitDependencySpecifier, GitPackageRef}, registry::{RegistryDependencySpecifier, RegistryPackageRef}, - resolution::ResolvedVersionsMap, + resolution::RootLockfileNode, }, index::{CredentialsFn, Index}, manifest::{Manifest, Realm}, @@ -274,11 +274,11 @@ impl Project { /// Downloads the project's dependencies pub fn download( &mut self, - map: ResolvedVersionsMap, + lockfile: &RootLockfileNode, ) -> Result, InstallProjectError> { let (job, tx) = MultithreadedJob::new(); - for (name, versions) in map.clone() { + for (name, versions) in lockfile.children.clone() { for (version, resolved_package) in versions { let (_, source) = resolved_package.directory(self.path()); @@ -319,7 +319,7 @@ impl Project { #[cfg(feature = "wally")] pub fn convert_manifests( &self, - map: &ResolvedVersionsMap, + lockfile: &RootLockfileNode, generate_sourcemap: F, ) -> Result<(), ConvertManifestsError> { #[derive(Deserialize)] @@ -329,7 +329,7 @@ impl Project { file_paths: Vec, } - for versions in map.values() { + for versions in lockfile.children.values() { for resolved_package in versions.values() { let source = match &resolved_package.pkg_ref { PackageRef::Wally(_) | PackageRef::Git(_) => { @@ -373,10 +373,10 @@ impl Project { #[cfg(not(feature = "wally"))] pub fn convert_manifests( &self, - map: &ResolvedVersionsMap, + lockfile: &RootLockfileNode, _generate_sourcemap: F, ) -> Result<(), ConvertManifestsError> { - for versions in map.values() { + for versions in lockfile.children.values() { for resolved_package in versions.values() { let source = match &resolved_package.pkg_ref { PackageRef::Git(_) => resolved_package.directory(self.path()).1, diff --git a/src/dependencies/resolution.rs b/src/dependencies/resolution.rs index e9b2095..f07b797 100644 --- a/src/dependencies/resolution.rs +++ b/src/dependencies/resolution.rs @@ -1,45 +1,68 @@ use std::{ - collections::{BTreeMap, BTreeSet, VecDeque}, + collections::{BTreeMap, BTreeSet, HashSet, VecDeque}, fmt::Display, path::{Path, PathBuf}, }; use log::debug; -use semver::Version; +use semver::{Version, VersionReq}; use serde::{Deserialize, Serialize}; use thiserror::Error; -#[cfg(feature = "wally")] -use crate::index::Index; use crate::{ dependencies::{ git::{GitDownloadError, GitPackageRef}, registry::RegistryPackageRef, DependencySpecifier, PackageRef, }, - index::IndexPackageError, + index::{Index, IndexFileEntry, IndexPackageError}, manifest::{DependencyType, Manifest, Realm}, package_name::PackageName, project::{get_index, get_index_by_url, Project, ReadLockfileError}, DEV_PACKAGES_FOLDER, INDEX_FOLDER, PACKAGES_FOLDER, SERVER_PACKAGES_FOLDER, }; -/// A node in the dependency tree +/// A mapping of packages to something +pub type PackageMap = BTreeMap>; + +/// The root node of the dependency graph +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash, Default)] +#[serde(deny_unknown_fields)] +pub struct RootLockfileNode { + /// The specifiers of the root packages + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub specifiers: PackageMap, + + /// All nodes in the dependency graph + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub children: PackageMap, +} + +impl RootLockfileNode { + /// Returns the specifier of the root package + pub fn root_specifier( + &self, + resolved_package: &ResolvedPackage, + ) -> Option<&DependencySpecifier> { + self.specifiers + .get(&resolved_package.pkg_ref.name()) + .and_then(|versions| versions.get(resolved_package.pkg_ref.version())) + } +} + +/// A node in the dependency graph #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash)] #[serde(deny_unknown_fields)] pub struct ResolvedPackage { /// The reference to the package pub pkg_ref: PackageRef, - /// The specifier that resolved to this package - pub specifier: DependencySpecifier, /// The dependencies of the package #[serde(default, skip_serializing_if = "BTreeSet::is_empty")] pub dependencies: BTreeSet<(PackageName, Version)>, - /// Whether the package is a root package (top-level dependency) - pub is_root: bool, /// The realm of the package pub realm: Realm, /// The type of the dependency + #[serde(default, skip_serializing_if = "crate::is_default")] pub dep_type: DependencyType, } @@ -49,7 +72,7 @@ impl Display for ResolvedPackage { } } -pub(crate) fn packages_folder(realm: &Realm) -> &str { +pub(crate) fn packages_folder<'a>(realm: Realm) -> &'a str { match realm { Realm::Shared => PACKAGES_FOLDER, Realm::Server => SERVER_PACKAGES_FOLDER, @@ -59,7 +82,7 @@ pub(crate) fn packages_folder(realm: &Realm) -> &str { impl ResolvedPackage { pub(crate) fn packages_folder(&self) -> &str { - packages_folder(&self.realm) + packages_folder(self.realm) } /// Returns the directory of the package in the project, and the parent of the directory @@ -76,18 +99,44 @@ impl ResolvedPackage { } } -/// A flat resolved map, a map of package names to versions to resolved packages -pub type ResolvedVersionsMap = BTreeMap>; - macro_rules! find_highest { - ($iter:expr, $dep:expr) => { + ($iter:expr, $version:expr) => { $iter - .filter(|v| $dep.version.matches(v)) + .filter(|v| $version.matches(v)) .max_by(|a, b| a.cmp(&b)) .cloned() }; } +fn find_version_from_index( + root: &mut RootLockfileNode, + index: &dyn Index, + specifier: &DependencySpecifier, + name: PackageName, + version_req: &VersionReq, +) -> Result { + let index_entries = index + .package(&name) + .map_err(|e| ResolveError::IndexPackage(e, name.to_string()))? + .ok_or_else(|| ResolveError::PackageNotFound(name.to_string()))?; + + let resolved_versions = root.children.entry(name).or_default(); + + // try to find the highest already downloaded version that satisfies the requirement, otherwise find the highest satisfying version in the index + let Some(version) = find_highest!(resolved_versions.keys(), version_req) + .or_else(|| find_highest!(index_entries.iter().map(|v| &v.version), version_req)) + else { + return Err(ResolveError::NoSatisfyingVersion(Box::new( + specifier.clone(), + ))); + }; + + Ok(index_entries + .into_iter() + .find(|e| e.version.eq(&version)) + .unwrap()) +} + fn find_realm(a: &Realm, b: &Realm) -> Realm { if a == b { return *a; @@ -96,38 +145,6 @@ fn find_realm(a: &Realm, b: &Realm) -> Realm { Realm::Shared } -fn add_to_map( - map: &mut ResolvedVersionsMap, - name: &PackageName, - version: &Version, - resolved_package: &ResolvedPackage, - lockfile: &ResolvedVersionsMap, - depth: usize, -) -> Result<(), ResolveError> { - debug!( - "{}resolved {resolved_package} from lockfile", - "\t".repeat(depth) - ); - - map.entry(name.clone()) - .or_default() - .insert(version.clone(), resolved_package.clone()); - - for (dep_name, dep_version) in &resolved_package.dependencies { - if map.get(dep_name).and_then(|v| v.get(dep_version)).is_none() { - let dep = lockfile.get(dep_name).and_then(|v| v.get(dep_version)); - - match dep { - Some(dep) => add_to_map(map, dep_name, dep_version, dep, lockfile, depth + 1)?, - // the lockfile is malformed - None => return Err(ResolveError::OutOfDateLockfile), - } - } - } - - Ok(()) -} - /// An error that occurred while resolving dependencies #[derive(Debug, Error)] pub enum ResolveError { @@ -186,63 +203,95 @@ pub enum ResolveError { } impl Manifest { - /// Resolves the dependency tree for the project - pub fn dependency_tree( + /// Resolves the dependency graph for the project + pub fn dependency_graph( &self, project: &mut Project, locked: bool, - ) -> Result { - debug!("resolving dependency tree for project {}", self.name); + ) -> Result { + debug!("resolving dependency graph for project {}", self.name); // try to reuse versions (according to semver specifiers) to decrease the amount of downloads and storage - let mut resolved_versions_map: ResolvedVersionsMap = BTreeMap::new(); + let mut root = RootLockfileNode::default(); - let tree = if let Some(lockfile) = project.lockfile()? { + let graph = if let Some(old_root) = project.lockfile()? { debug!("lockfile found, resolving dependencies from it"); let mut missing = Vec::new(); - // resolve all root dependencies (and their dependencies) from the lockfile - for (name, versions) in &lockfile { + let current_dependencies = self.dependencies(); + let current_specifiers = current_dependencies + .iter() + .map(|(d, _)| d) + .collect::>(); + + // populate the new lockfile with all root dependencies (and their dependencies) from the old lockfile + for (name, versions) in &old_root.children { for (version, resolved_package) in versions { - if !resolved_package.is_root - || !self - .dependencies() - .into_iter() - .any(|(spec, _)| spec == resolved_package.specifier) - { + let specifier = old_root.root_specifier(resolved_package); + + if !specifier.is_some_and(|specifier| current_specifiers.contains(specifier)) { continue; } - add_to_map( - &mut resolved_versions_map, - name, - version, - resolved_package, - &lockfile, - 1, - )?; + root.specifiers + .entry(name.clone()) + .or_default() + .insert(version.clone(), specifier.unwrap().clone()); + + let mut queue = VecDeque::from([resolved_package]); + + while let Some(resolved_package) = queue.pop_front() { + debug!("resolved {resolved_package} from lockfile"); + + root.children + .entry(name.clone()) + .or_default() + .insert(version.clone(), resolved_package.clone()); + + for (dep_name, dep_version) in &resolved_package.dependencies { + if root + .children + .get(dep_name) + .and_then(|v| v.get(dep_version)) + .is_none() + { + let dep = old_root + .children + .get(dep_name) + .and_then(|v| v.get(dep_version)); + + match dep { + Some(dep) => queue.push_back(dep), + // the lockfile is out of date + None => return Err(ResolveError::OutOfDateLockfile), + } + } + } + } } } - // resolve new, or modified, dependencies from the lockfile - 'outer: for (dep, dep_type) in self.dependencies() { - for versions in resolved_versions_map.values() { - for resolved_package in versions.values() { - if resolved_package.specifier == dep && resolved_package.is_root { - continue 'outer; - } - } + let old_specifiers = old_root + .specifiers + .values() + .flat_map(|v| v.values()) + .collect::>(); + + // resolve new, or modified, dependencies from the manifest + for (specifier, dep_type) in current_dependencies { + if old_specifiers.contains(&specifier) { + continue; } if locked { return Err(ResolveError::OutOfDateLockfile); } - missing.push((dep.clone(), dep_type)); + missing.push((specifier.clone(), dep_type)); } debug!( "resolved {} dependencies from lockfile. new dependencies: {}", - resolved_versions_map.len(), + old_root.children.len(), missing.len() ); @@ -252,16 +301,19 @@ impl Manifest { self.dependencies() }; - if tree.is_empty() { + if graph.is_empty() { debug!("no dependencies left to resolve, finishing..."); - return Ok(resolved_versions_map); + return Ok(root); } - debug!("resolving {} dependencies from index", tree.len()); + debug!("resolving {} dependencies from index", graph.len()); - let mut queue = VecDeque::from_iter(self.dependencies().into_iter().map(|d| (d, None))); + let mut queue = graph + .into_iter() + .map(|(specifier, dep_type)| (specifier, dep_type, None)) + .collect::>(); - while let Some(((specifier, dep_type), dependant)) = queue.pop_front() { + while let Some((specifier, dep_type, dependant)) = queue.pop_front() { let (pkg_ref, default_realm, dependencies) = match &specifier { DependencySpecifier::Registry(registry_dependency) => { let index = if dependant.is_none() { @@ -269,49 +321,24 @@ impl Manifest { } else { get_index_by_url(project.indices(), ®istry_dependency.index.parse()?) }; - let pkg_name: PackageName = registry_dependency.name.clone().into(); - let index_entries = index - .package(&pkg_name) - .map_err(|e| { - ResolveError::IndexPackage(e, registry_dependency.name.to_string()) - })? - .ok_or_else(|| { - ResolveError::PackageNotFound(registry_dependency.name.to_string()) - })?; - - let resolved_versions = resolved_versions_map.entry(pkg_name).or_default(); - - // try to find the highest already downloaded version that satisfies the requirement, otherwise find the highest satisfying version in the index - let Some(version) = - find_highest!(resolved_versions.keys(), registry_dependency).or_else( - || { - find_highest!( - index_entries.iter().map(|v| &v.version), - registry_dependency - ) - }, - ) - else { - return Err(ResolveError::NoSatisfyingVersion(Box::new( - specifier.clone(), - ))); - }; - - let entry = index_entries - .into_iter() - .find(|e| e.version.eq(&version)) - .unwrap(); + let entry = find_version_from_index( + &mut root, + index, + &specifier, + registry_dependency.name.clone().into(), + ®istry_dependency.version, + )?; debug!( "resolved registry dependency {} to {}", - registry_dependency.name, version + registry_dependency.name, entry.version ); ( PackageRef::Registry(RegistryPackageRef { name: registry_dependency.name.clone(), - version: version.clone(), + version: entry.version, index_url: index.url().clone(), }), entry.realm, @@ -346,47 +373,24 @@ impl Manifest { project.indices_mut(), &wally_dependency.index_url, )?; - let pkg_name = wally_dependency.name.clone().into(); - let index_entries = index - .package(&pkg_name) - .map_err(|e| { - ResolveError::IndexPackage(e, wally_dependency.name.to_string()) - })? - .ok_or_else(|| { - ResolveError::PackageNotFound(wally_dependency.name.to_string()) - })?; - - let resolved_versions = resolved_versions_map.entry(pkg_name).or_default(); - - // try to find the highest already downloaded version that satisfies the requirement, otherwise find the highest satisfying version in the index - let Some(version) = find_highest!(resolved_versions.keys(), wally_dependency) - .or_else(|| { - find_highest!( - index_entries.iter().map(|v| &v.version), - wally_dependency - ) - }) - else { - return Err(ResolveError::NoSatisfyingVersion(Box::new( - specifier.clone(), - ))); - }; - - let entry = index_entries - .into_iter() - .find(|e| e.version.eq(&version)) - .unwrap(); + let entry = find_version_from_index( + &mut root, + &index, + &specifier, + wally_dependency.name.clone().into(), + &wally_dependency.version, + )?; debug!( - "resolved registry dependency {} to {}", - wally_dependency.name, version + "resolved wally dependency {} to {}", + wally_dependency.name, entry.version ); ( PackageRef::Wally(crate::dependencies::wally::WallyPackageRef { name: wally_dependency.name.clone(), - version: version.clone(), + version: entry.version, index_url: index.url().clone(), }), entry.realm, @@ -395,26 +399,30 @@ impl Manifest { } }; - let is_root = dependant.is_none(); // if the dependency is a root dependency, it can be thought of as a normal dependency - let dep_type = if is_root { - DependencyType::Normal - } else { + let dep_type = if dependant.is_some() { dep_type + } else { + DependencyType::Normal }; + let specifier_realm = specifier.realm().copied(); + if let Some((dependant_name, dependant_version)) = dependant { - resolved_versions_map + root.children .get_mut(&dependant_name) .and_then(|v| v.get_mut(&dependant_version)) .unwrap() .dependencies .insert((pkg_ref.name(), pkg_ref.version().clone())); + } else { + root.specifiers + .entry(pkg_ref.name()) + .or_default() + .insert(pkg_ref.version().clone(), specifier); } - let resolved_versions = resolved_versions_map - .entry(pkg_ref.name().clone()) - .or_default(); + let resolved_versions = root.children.entry(pkg_ref.name()).or_default(); if let Some(previously_resolved) = resolved_versions.get_mut(pkg_ref.version()) { match (&pkg_ref, &previously_resolved.pkg_ref) { @@ -443,15 +451,13 @@ impl Manifest { continue; } - if specifier - .realm() - .is_some_and(|realm| realm == &Realm::Shared) + if specifier_realm.is_some_and(|realm| realm == Realm::Shared) && default_realm.is_some_and(|realm| realm == Realm::Server) { return Err(ResolveError::IncompatibleRealms( pkg_ref.name().to_string(), default_realm.unwrap(), - *specifier.realm().unwrap(), + specifier_realm.unwrap(), )); } @@ -459,29 +465,26 @@ impl Manifest { pkg_ref.version().clone(), ResolvedPackage { pkg_ref: pkg_ref.clone(), - specifier: specifier.clone(), dependencies: BTreeSet::new(), - is_root, - realm: *specifier - .realm() - .copied() + realm: specifier_realm .unwrap_or_default() - .or(&default_realm.unwrap_or_default()), + .or(default_realm.unwrap_or_default()), dep_type, }, ); - for dependency in dependencies { + for (specifier, ty) in dependencies { queue.push_back(( - dependency, - Some((pkg_ref.name().clone(), pkg_ref.version().clone())), + specifier, + ty, + Some((pkg_ref.name(), pkg_ref.version().clone())), )); } } debug!("resolving realms and peer dependencies..."); - for (name, versions) in resolved_versions_map.clone() { + for (name, versions) in root.children.clone() { for (version, resolved_package) in versions { if resolved_package.dep_type == DependencyType::Peer { return Err(ResolveError::PeerNotInstalled( @@ -493,16 +496,14 @@ impl Manifest { let mut realm = resolved_package.realm; for (dep_name, dep_version) in &resolved_package.dependencies { - let dep = resolved_versions_map - .get(dep_name) - .and_then(|v| v.get(dep_version)); + let dep = root.children.get(dep_name).and_then(|v| v.get(dep_version)); if let Some(dep) = dep { realm = find_realm(&realm, &dep.realm); } } - resolved_versions_map + root.children .get_mut(&name) .and_then(|v| v.get_mut(&version)) .unwrap() @@ -510,8 +511,8 @@ impl Manifest { } } - debug!("finished resolving dependency tree"); + debug!("finished resolving dependency graph"); - Ok(resolved_versions_map) + Ok(root) } } diff --git a/src/index.rs b/src/index.rs index 3a1eaa0..e45620c 100644 --- a/src/index.rs +++ b/src/index.rs @@ -707,13 +707,13 @@ impl Index for WallyIndex { .collect::, _>>() .map_err(|e| IndexPackageError::Other(Box::new(e)))?; - Ok(Some(BTreeSet::from_iter( + Ok(Some( manifest_stream .into_iter() .map(|m| m.try_into()) - .collect::, _>>() + .collect::, _>>() .map_err(|e| IndexPackageError::Other(Box::new(e)))?, - ))) + )) } fn create_package_version( diff --git a/src/lib.rs b/src/lib.rs index 3d7cbf4..9db2de4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -45,3 +45,7 @@ pub const IGNORED_FOLDERS: &[&str] = &[ SERVER_PACKAGES_FOLDER, ".git", ]; + +pub(crate) fn is_default(t: &T) -> bool { + t == &Default::default() +} diff --git a/src/linking_file.rs b/src/linking_file.rs index 590843c..356910b 100644 --- a/src/linking_file.rs +++ b/src/linking_file.rs @@ -14,7 +14,7 @@ use semver::Version; use thiserror::Error; use crate::{ - dependencies::resolution::{packages_folder, ResolvedPackage, ResolvedVersionsMap}, + dependencies::resolution::{packages_folder, ResolvedPackage, RootLockfileNode}, manifest::{Manifest, ManifestReadError, PathStyle, Realm}, package_name::PackageName, project::Project, @@ -125,6 +125,7 @@ pub enum LinkingError { pub(crate) fn link, Q: AsRef>( project: &Project, resolved_pkg: &ResolvedPackage, + lockfile: &RootLockfileNode, destination_dir: P, parent_dependency_packages_dir: Q, only_name: bool, @@ -146,12 +147,15 @@ pub(crate) fn link, Q: AsRef>( let pkg_name = resolved_pkg.pkg_ref.name(); let name = pkg_name.name(); - let destination_dir = if resolved_pkg.is_root { - project.path().join(packages_folder( - &resolved_pkg.specifier.realm().cloned().unwrap_or_default(), - )) - } else { - destination_dir.as_ref().to_path_buf() + let destination_dir = match lockfile + .specifiers + .get(&pkg_name) + .and_then(|v| v.get(resolved_pkg.pkg_ref.version())) + { + Some(specifier) => project.path().join(packages_folder( + specifier.realm().copied().unwrap_or_default(), + )), + None => destination_dir.as_ref().to_path_buf(), }; let destination_file = destination_dir.join(format!( @@ -230,19 +234,26 @@ pub struct LinkingDependenciesError( Version, ); +fn is_duplicate_in(item: T, items: &[T]) -> bool { + let mut count = 0u8; + items.iter().any(|i| { + if i == &item { + count += 1; + } + count > 1 + }) +} + impl Project { /// Links the dependencies of the project pub fn link_dependencies( &self, - map: &ResolvedVersionsMap, + lockfile: &RootLockfileNode, ) -> Result<(), LinkingDependenciesError> { - let root_deps: HashSet = HashSet::from_iter( - map.iter() - .flat_map(|(_, v)| v) - .filter_map(|(_, v)| v.is_root.then_some(v.pkg_ref.name().name().to_string())), - ); + let root_deps = lockfile.specifiers.keys().collect::>(); + let root_dep_names = root_deps.iter().map(|n| n.name()).collect::>(); - for (name, versions) in map { + for (name, versions) in &lockfile.children { for (version, resolved_pkg) in versions { let (container_dir, _) = resolved_pkg.directory(self.path()); @@ -251,8 +262,15 @@ impl Project { container_dir.display() ); + let resolved_pkg_dep_names = resolved_pkg + .dependencies + .iter() + .map(|(n, _)| n.name()) + .collect::>(); + for (dep_name, dep_version) in &resolved_pkg.dependencies { - let dep = map + let dep = lockfile + .children .get(dep_name) .and_then(|versions| versions.get(dep_version)) .unwrap(); @@ -260,12 +278,10 @@ impl Project { link( self, dep, + lockfile, &container_dir, &self.path().join(resolved_pkg.packages_folder()), - resolved_pkg - .dependencies - .iter() - .any(|(n, _)| n.name() == dep_name.name()), + !is_duplicate_in(dep_name.name(), &resolved_pkg_dep_names), ) .map_err(|e| { LinkingDependenciesError( @@ -278,7 +294,7 @@ impl Project { })?; } - if resolved_pkg.is_root { + if root_deps.contains(&name) { let linking_dir = &self.path().join(resolved_pkg.packages_folder()); debug!( @@ -289,9 +305,10 @@ impl Project { link( self, resolved_pkg, + lockfile, linking_dir, linking_dir, - root_deps.contains(name.name()), + !is_duplicate_in(name.name(), &root_dep_names), ) .map_err(|e| { LinkingDependenciesError( diff --git a/src/manifest.rs b/src/manifest.rs index 16a1e88..beab16c 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -72,7 +72,7 @@ pub enum Realm { impl Realm { /// Returns the most restrictive realm - pub fn or<'a>(&'a self, other: &'a Self) -> &'a Self { + pub fn or(self, other: Self) -> Self { match self { Realm::Shared => other, _ => self, @@ -115,6 +115,18 @@ pub struct Manifest { pub name: StandardPackageName, /// The version of the package. Must be [semver](https://semver.org) compatible. The registry will not accept non-semver versions and the CLI will not handle such packages pub version: Version, + /// A short description of the package + #[serde(default, skip_serializing_if = "Option::is_none")] + pub description: Option, + /// The license of the package + #[serde(default, skip_serializing_if = "Option::is_none")] + pub license: Option, + /// The authors of the package + #[serde(default, skip_serializing_if = "Option::is_none")] + pub authors: Option>, + /// The repository of the package + #[serde(default, skip_serializing_if = "Option::is_none")] + pub repository: Option, /// The files exported by the package #[serde(default)] pub exports: Exports, @@ -139,19 +151,6 @@ pub struct Manifest { /// The peer dependencies of the package #[serde(default, skip_serializing_if = "Vec::is_empty")] pub peer_dependencies: Vec, - - /// A short description of the package - #[serde(default, skip_serializing_if = "Option::is_none")] - pub description: Option, - /// The license of the package - #[serde(default, skip_serializing_if = "Option::is_none")] - pub license: Option, - /// The authors of the package - #[serde(default, skip_serializing_if = "Option::is_none")] - pub authors: Option>, - /// The repository of the package - #[serde(default, skip_serializing_if = "Option::is_none")] - pub repository: Option, } /// An error that occurred while reading the manifest diff --git a/src/patches.rs b/src/patches.rs index 2097b8d..b4409e9 100644 --- a/src/patches.rs +++ b/src/patches.rs @@ -9,7 +9,7 @@ use semver::Version; use thiserror::Error; use crate::{ - dependencies::resolution::ResolvedVersionsMap, + dependencies::resolution::RootLockfileNode, package_name::{FromEscapedStrPackageNameError, PackageName}, project::Project, PATCHES_FOLDER, @@ -141,7 +141,7 @@ pub enum ApplyPatchesError { impl Project { /// Applies patches for the project - pub fn apply_patches(&self, map: &ResolvedVersionsMap) -> Result<(), ApplyPatchesError> { + pub fn apply_patches(&self, lockfile: &RootLockfileNode) -> Result<(), ApplyPatchesError> { let patches_dir = self.path().join(PATCHES_FOLDER); if !patches_dir.exists() { return Ok(()); @@ -170,7 +170,8 @@ impl Project { let version = Version::parse(version)?; - let resolved_pkg = map + let resolved_pkg = lockfile + .children .get(&package_name) .ok_or_else(|| ApplyPatchesError::PackageNotFound(package_name.clone()))? .get(&version) diff --git a/src/project.rs b/src/project.rs index ff7ce47..7a8a9b1 100644 --- a/src/project.rs +++ b/src/project.rs @@ -10,7 +10,7 @@ use thiserror::Error; use url::Url; use crate::{ - dependencies::{resolution::ResolvedVersionsMap, DownloadError, UrlResolveError}, + dependencies::{resolution::RootLockfileNode, DownloadError, UrlResolveError}, index::Index, linking_file::LinkingDependenciesError, manifest::{Manifest, ManifestReadError}, @@ -34,7 +34,7 @@ pub struct Project { pub struct InstallOptions { locked: bool, auto_download: bool, - resolved_versions_map: Option, + lockfile: Option, } impl Default for InstallOptions { @@ -42,7 +42,7 @@ impl Default for InstallOptions { Self { locked: false, auto_download: true, - resolved_versions_map: None, + lockfile: None, } } } @@ -57,7 +57,7 @@ impl InstallOptions { pub fn locked(&self, locked: bool) -> Self { Self { locked, - resolved_versions_map: self.resolved_versions_map.clone(), + lockfile: self.lockfile.clone(), ..*self } } @@ -67,16 +67,16 @@ impl InstallOptions { pub fn auto_download(&self, auto_download: bool) -> Self { Self { auto_download, - resolved_versions_map: self.resolved_versions_map.clone(), + lockfile: self.lockfile.clone(), ..*self } } - /// Makes the installation to use the given resolved versions map + /// Makes the installation to use the given lockfile /// Having this set to Some is only useful if you're using auto_download = false - pub fn resolved_versions_map(&self, resolved_versions_map: ResolvedVersionsMap) -> Self { + pub fn lockfile(&self, lockfile: RootLockfileNode) -> Self { Self { - resolved_versions_map: Some(resolved_versions_map), + lockfile: Some(lockfile), ..*self } } @@ -97,9 +97,9 @@ pub enum ReadLockfileError { /// An error that occurred while downloading a project #[derive(Debug, Error)] pub enum InstallProjectError { - /// An error that occurred while resolving the dependency tree - #[error("failed to resolve dependency tree")] - ResolveTree(#[from] crate::dependencies::resolution::ResolveError), + /// An error that occurred while resolving the dependency graph + #[error("failed to resolve dependency graph")] + ResolveGraph(#[from] crate::dependencies::resolution::ResolveError), /// An error that occurred while downloading a package #[error("failed to download package")] @@ -273,12 +273,12 @@ impl Project { } /// Returns the lockfile of the project - pub fn lockfile(&self) -> Result, ReadLockfileError> { + pub fn lockfile(&self) -> Result, ReadLockfileError> { let lockfile_path = self.path.join(LOCKFILE_FILE_NAME); Ok(if lockfile_path.exists() { let lockfile_contents = read(&lockfile_path)?; - let lockfile: ResolvedVersionsMap = serde_yaml::from_slice(&lockfile_contents) + let lockfile: RootLockfileNode = serde_yaml::from_slice(&lockfile_contents) .map_err(ReadLockfileError::LockfileDeser)?; Some(lockfile) @@ -289,25 +289,25 @@ impl Project { /// Downloads the project's dependencies, applies patches, and links the dependencies pub fn install(&mut self, install_options: InstallOptions) -> Result<(), InstallProjectError> { - let map = match install_options.resolved_versions_map { + let lockfile = match install_options.lockfile { Some(map) => map, None => { let manifest = self.manifest.clone(); - manifest.dependency_tree(self, install_options.locked)? + manifest.dependency_graph(self, install_options.locked)? } }; if install_options.auto_download { - self.download(map.clone())?.wait()?; + self.download(&lockfile)?.wait()?; } - self.apply_patches(&map)?; + self.apply_patches(&lockfile)?; - self.link_dependencies(&map)?; + self.link_dependencies(&lockfile)?; if !install_options.locked { - serde_yaml::to_writer(File::create(self.path.join(LOCKFILE_FILE_NAME))?, &map) + serde_yaml::to_writer(File::create(self.path.join(LOCKFILE_FILE_NAME))?, &lockfile) .map_err(InstallProjectError::LockfileSer)?; } diff --git a/tests/resolver.rs b/tests/resolver.rs index 252eca7..6e7fedf 100644 --- a/tests/resolver.rs +++ b/tests/resolver.rs @@ -104,9 +104,9 @@ fn test_resolves_package() { .unwrap(); let manifest = project.manifest().clone(); - let tree = manifest.dependency_tree(&mut project, false).unwrap(); - assert_eq!(tree.len(), 1); - let versions = tree.get(&pkg_name.clone().into()).unwrap(); + let graph = manifest.dependency_graph(&mut project, false).unwrap(); + assert_eq!(graph.children.len(), 1); + let versions = graph.children.get(&pkg_name.clone().into()).unwrap(); assert_eq!(versions.len(), 2); let resolved_pkg = versions.get(&version).unwrap(); assert_eq!( @@ -117,9 +117,7 @@ fn test_resolves_package() { version: version.clone(), index_url: index.url().clone(), }), - specifier, dependencies: Default::default(), - is_root: true, realm: Realm::Shared, dep_type: DependencyType::Normal, } @@ -133,9 +131,7 @@ fn test_resolves_package() { version: version_2.clone(), index_url: index.url().clone(), }), - specifier: specifier_2, dependencies: Default::default(), - is_root: true, realm: Realm::Shared, dep_type: DependencyType::Normal, }