From ac48fbe18a5ca426efe8dbb276fc8e5844d8055b Mon Sep 17 00:00:00 2001 From: daimond113 <72147841+daimond113@users.noreply.github.com> Date: Wed, 27 Mar 2024 20:56:19 +0100 Subject: [PATCH] feat(manifest): :sparkles: add dependency names --- registry/src/endpoints/packages.rs | 2 +- src/cli/root.rs | 30 ++++++++++++++--- src/dependencies/resolution.rs | 54 ++++++++++++++++++------------ src/dependencies/wally.rs | 25 ++++++++------ src/index.rs | 39 +++++++++++---------- src/linking_file.rs | 35 ++++--------------- src/manifest.rs | 26 +++++++++----- tests/resolver.rs | 10 +++--- 8 files changed, 124 insertions(+), 97 deletions(-) diff --git a/registry/src/endpoints/packages.rs b/registry/src/endpoints/packages.rs index ee72e8d..6d9d9a4 100644 --- a/registry/src/endpoints/packages.rs +++ b/registry/src/endpoints/packages.rs @@ -84,7 +84,7 @@ pub async fn create_package( let mut index = app_state.index.lock().unwrap(); let config = index.config()?; - for (dependency, _) in manifest.dependencies() { + for (dependency, _) in manifest.dependencies().into_values() { match dependency { DependencySpecifier::Git(_) => { if !config.git_allowed { diff --git a/src/cli/root.rs b/src/cli/root.rs index 6256b13..e3d1b4f 100644 --- a/src/cli/root.rs +++ b/src/cli/root.rs @@ -436,7 +436,7 @@ pub fn root_command(cmd: Command) -> anyhow::Result<()> { } => { let mut manifest = project.manifest().clone(); - let specifier = match package.0 { + let specifier = match package.0.clone() { PackageName::Standard(name) => { DependencySpecifier::Registry(RegistryDependencySpecifier { name, @@ -456,10 +456,32 @@ pub fn root_command(cmd: Command) -> anyhow::Result<()> { ), }; + fn insert_into( + deps: &mut BTreeMap, + specifier: DependencySpecifier, + name: PackageName, + ) { + macro_rules! not_taken { + ($key:expr) => { + (!deps.contains_key(&$key)).then_some($key) + }; + } + + let key = not_taken!(name.name().to_string()) + .or_else(|| not_taken!(format!("{}/{}", name.scope(), name.name()))) + .or_else(|| not_taken!(name.to_string())) + .unwrap(); + deps.insert(key, specifier); + } + if peer { - manifest.peer_dependencies.push(specifier); + insert_into( + &mut manifest.peer_dependencies, + specifier, + package.0.clone(), + ); } else { - manifest.dependencies.push(specifier); + insert_into(&mut manifest.dependencies, specifier, package.0.clone()); } serde_yaml::to_writer( @@ -471,7 +493,7 @@ pub fn root_command(cmd: Command) -> anyhow::Result<()> { let mut manifest = project.manifest().clone(); for dependencies in [&mut manifest.dependencies, &mut manifest.peer_dependencies] { - dependencies.retain(|d| { + dependencies.retain(|_, d| { if let DependencySpecifier::Registry(registry) = d { match &package { PackageName::Standard(name) => ®istry.name != name, diff --git a/src/dependencies/resolution.rs b/src/dependencies/resolution.rs index 7a6c1cb..ae4c9dd 100644 --- a/src/dependencies/resolution.rs +++ b/src/dependencies/resolution.rs @@ -1,5 +1,5 @@ use std::{ - collections::{BTreeMap, BTreeSet, HashMap, HashSet, VecDeque}, + collections::{BTreeMap, HashMap, HashSet, VecDeque}, fmt::Display, path::{Path, PathBuf}, }; @@ -35,7 +35,7 @@ pub struct RootLockfileNode { /// The specifiers of the root packages #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - pub specifiers: PackageMap, + pub specifiers: PackageMap<(DependencySpecifier, String)>, /// All nodes in the dependency graph #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] @@ -47,7 +47,7 @@ impl RootLockfileNode { pub fn root_specifier( &self, resolved_package: &ResolvedPackage, - ) -> Option<&DependencySpecifier> { + ) -> Option<&(DependencySpecifier, String)> { self.specifiers .get(&resolved_package.pkg_ref.name()) .and_then(|versions| versions.get(resolved_package.pkg_ref.version())) @@ -61,8 +61,8 @@ pub struct ResolvedPackage { /// The reference to the package pub pkg_ref: PackageRef, /// The dependencies of the package - #[serde(default, skip_serializing_if = "BTreeSet::is_empty")] - pub dependencies: BTreeSet<(PackageName, Version)>, + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub dependencies: BTreeMap, /// The realm of the package pub realm: Realm, /// The type of the dependency @@ -212,7 +212,7 @@ impl Manifest { root: &mut RootLockfileNode, locked: bool, project: &Project, - ) -> Result, ResolveError> { + ) -> Result, ResolveError> { Ok(if let Some(old_root) = project.lockfile()? { if self.overrides != old_root.overrides { // TODO: resolve only the changed dependencies (will this be worth it?) @@ -221,12 +221,12 @@ impl Manifest { } debug!("lockfile found, resolving dependencies from it"); - let mut missing = Vec::new(); + let mut missing = BTreeMap::new(); let current_dependencies = self.dependencies(); let current_specifiers = current_dependencies - .iter() - .map(|(d, _)| d) + .values() + .map(|(specifier, _)| specifier) .collect::>(); // populate the new lockfile with all root dependencies (and their dependencies) from the old lockfile @@ -234,7 +234,9 @@ impl Manifest { for (version, resolved_package) in versions { let specifier = old_root.root_specifier(resolved_package); - if !specifier.is_some_and(|specifier| current_specifiers.contains(specifier)) { + if !specifier + .is_some_and(|(specifier, _)| current_specifiers.contains(specifier)) + { continue; } @@ -256,7 +258,7 @@ impl Manifest { .or_default() .insert(version.clone(), resolved_package.clone()); - for (dep_name, dep_version) in &resolved_package.dependencies { + for (dep_name, (dep_version, _)) in &resolved_package.dependencies { if root .children .get(dep_name) @@ -283,10 +285,11 @@ impl Manifest { .specifiers .values() .flat_map(|v| v.values()) + .map(|(specifier, _)| specifier) .collect::>(); // resolve new, or modified, dependencies from the manifest - for (specifier, dep_type) in current_dependencies { + for (desired_name, (specifier, dep_type)) in current_dependencies { if old_specifiers.contains(&specifier) { continue; } @@ -295,7 +298,7 @@ impl Manifest { return Err(ResolveError::OutOfDateLockfile); } - missing.push((specifier.clone(), dep_type)); + missing.insert(desired_name, (specifier.clone(), dep_type)); } debug!( @@ -341,10 +344,13 @@ impl Manifest { let mut queue = missing_dependencies .into_iter() - .map(|(specifier, dep_type)| (specifier, dep_type, None, vec![])) + .map(|(desired_name, (specifier, dep_type))| { + (desired_name, specifier, dep_type, None, vec![]) + }) .collect::>(); - while let Some((specifier, dep_type, dependant, mut path)) = queue.pop_front() { + while let Some((desired_name, specifier, dep_type, dependant, mut path)) = queue.pop_front() + { let depth = path.len(); let (pkg_ref, default_realm, dependencies) = match &specifier { @@ -452,12 +458,15 @@ impl Manifest { .and_then(|v| v.get_mut(&dependant_version)) .unwrap() .dependencies - .insert((pkg_ref.name(), pkg_ref.version().clone())); + .insert( + pkg_ref.name(), + (pkg_ref.version().clone(), desired_name.clone()), + ); } else { root.specifiers .entry(pkg_ref.name()) .or_default() - .insert(pkg_ref.version().clone(), specifier); + .insert(pkg_ref.version().clone(), (specifier, desired_name.clone())); } let resolved_versions = root.children.entry(pkg_ref.name()).or_default(); @@ -503,7 +512,7 @@ impl Manifest { pkg_ref.version().clone(), ResolvedPackage { pkg_ref: pkg_ref.clone(), - dependencies: BTreeSet::new(), + dependencies: Default::default(), realm: specifier_realm .unwrap_or_default() .or(default_realm.unwrap_or_default()), @@ -511,15 +520,16 @@ impl Manifest { }, ); - path.push(pkg_ref.name().to_string()); + path.push(desired_name); - for (specifier, ty) in dependencies { + for (desired_name, (specifier, ty)) in dependencies { let overridden = overrides.iter().find_map(|(k_path, spec)| { - (path == k_path[..k_path.len() - 1] && k_path.last() == Some(&specifier.name())) + (path == k_path[..k_path.len() - 1] && k_path.last() == Some(&desired_name)) .then_some(spec) }); queue.push_back(( + desired_name, overridden.cloned().unwrap_or(specifier), ty, Some((pkg_ref.name(), pkg_ref.version().clone())), @@ -541,7 +551,7 @@ impl Manifest { let mut realm = resolved_package.realm; - for (dep_name, dep_version) in &resolved_package.dependencies { + for (dep_name, (dep_version, _)) in &resolved_package.dependencies { let dep = root.children.get(dep_name).and_then(|v| v.get(dep_version)); if let Some(dep) = dep { diff --git a/src/dependencies/wally.rs b/src/dependencies/wally.rs index 7b84cbe..a3f8a8d 100644 --- a/src/dependencies/wally.rs +++ b/src/dependencies/wally.rs @@ -314,7 +314,7 @@ pub enum WallyManifestDependencyError { pub(crate) fn parse_wally_dependencies( manifest: WallyManifest, -) -> Result, WallyManifestDependencyError> { +) -> Result, WallyManifestDependencyError> { [ (manifest.dependencies, Realm::Shared), (manifest.server_dependencies, Realm::Server), @@ -322,22 +322,25 @@ pub(crate) fn parse_wally_dependencies( ] .into_iter() .flat_map(|(deps, realm)| { - deps.into_values() - .map(|specifier| { + deps.into_iter() + .map(move |(desired_name, specifier)| (desired_name, specifier, realm)) + .map(|(desired_name, specifier, realm)| { let (name, req) = specifier.split_once('@').ok_or_else(|| { WallyManifestDependencyError::InvalidDependencySpecifier(specifier.clone()) })?; let name: WallyPackageName = name.parse()?; let req: VersionReq = req.parse()?; - Ok(DependencySpecifier::Wally(WallyDependencySpecifier { - name, - version: req, - index_url: manifest.package.registry.clone(), - realm: Some(realm), - })) + Ok(( + desired_name, + DependencySpecifier::Wally(WallyDependencySpecifier { + name, + version: req, + index_url: manifest.package.registry.clone(), + realm: Some(realm), + }), + )) }) - .collect::>() }) .collect() } @@ -348,7 +351,7 @@ impl TryFrom for IndexFileEntry { fn try_from(value: WallyManifest) -> Result { let dependencies = parse_wally_dependencies(value.clone())? .into_iter() - .map(|d| (d, DependencyType::Normal)) + .map(|(desired_name, specifier)| (desired_name, (specifier, DependencyType::Normal))) .collect(); Ok(IndexFileEntry { diff --git a/src/index.rs b/src/index.rs index e45620c..30967e9 100644 --- a/src/index.rs +++ b/src/index.rs @@ -1,6 +1,6 @@ use std::{ any::Any, - collections::BTreeSet, + collections::{BTreeMap, BTreeSet}, fmt::Debug, fs::create_dir_all, hash::Hash, @@ -578,8 +578,8 @@ pub struct IndexFileEntry { pub description: Option, /// The dependencies of the package - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub dependencies: Vec<(DependencySpecifier, DependencyType)>, + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub dependencies: BTreeMap, } /// An error that occurred while converting a manifest to an index file entry @@ -606,21 +606,24 @@ impl TryFrom for IndexFileEntry { dependencies: dependencies .into_iter() - .map(|(dep, ty)| { - Ok(match dep { - DependencySpecifier::Registry(mut registry) => { - registry.index = indices - .get(®istry.index) - .ok_or_else(|| { - FromManifestIndexFileEntry::IndexNotSpecified( - registry.index.clone(), - ) - })? - .clone(); - (DependencySpecifier::Registry(registry), ty) - } - d => (d, ty), - }) + .map(|(desired_name, (dep, ty))| { + Ok(( + desired_name, + match dep { + DependencySpecifier::Registry(mut registry) => { + registry.index = indices + .get(®istry.index) + .ok_or_else(|| { + FromManifestIndexFileEntry::IndexNotSpecified( + registry.index.clone(), + ) + })? + .clone(); + (DependencySpecifier::Registry(registry), ty) + } + d => (d, ty), + }, + )) }) .collect::>()?, }) diff --git a/src/linking_file.rs b/src/linking_file.rs index 58a1d06..9561e21 100644 --- a/src/linking_file.rs +++ b/src/linking_file.rs @@ -128,7 +128,7 @@ pub(crate) fn link, Q: AsRef>( lockfile: &RootLockfileNode, destination_dir: P, parent_dependency_packages_dir: Q, - only_name: bool, + desired_name: &str, as_root: bool, ) -> Result<(), LinkingError> { let (_, source_dir) = resolved_pkg.directory(project.path()); @@ -153,17 +153,13 @@ pub(crate) fn link, Q: AsRef>( .get(&pkg_name) .and_then(|v| v.get(resolved_pkg.pkg_ref.version())) { - Some(specifier) if as_root => project.path().join(packages_folder( + Some((specifier, _)) if as_root => project.path().join(packages_folder( specifier.realm().copied().unwrap_or_default(), )), _ => destination_dir.as_ref().to_path_buf(), }; - let destination_file = destination_dir.join(format!( - "{}{}.lua", - if only_name { "" } else { pkg_name.prefix() }, - name - )); + let destination_file = destination_dir.join(desired_name.to_string() + ".lua"); let realm_folder = project.path().join(resolved_pkg.packages_folder()); let in_different_folders = realm_folder != parent_dependency_packages_dir.as_ref(); @@ -235,16 +231,6 @@ 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( @@ -252,7 +238,6 @@ impl Project { lockfile: &RootLockfileNode, ) -> Result<(), LinkingDependenciesError> { let root_deps = lockfile.specifiers.keys().collect::>(); - let root_dep_names = root_deps.iter().map(|n| n.name()).collect::>(); for (name, versions) in &lockfile.children { for (version, resolved_pkg) in versions { @@ -263,13 +248,7 @@ 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 { + for (dep_name, (dep_version, desired_name)) in &resolved_pkg.dependencies { let dep = lockfile .children .get(dep_name) @@ -282,7 +261,7 @@ impl Project { lockfile, &container_dir, &self.path().join(resolved_pkg.packages_folder()), - !is_duplicate_in(dep_name.name(), &resolved_pkg_dep_names), + desired_name, false, ) .map_err(|e| { @@ -297,7 +276,7 @@ impl Project { } if root_deps.contains(&name) { - let specifier = lockfile.root_specifier(resolved_pkg).unwrap(); + let (specifier, desired_name) = lockfile.root_specifier(resolved_pkg).unwrap(); let linking_dir = &self.path().join(packages_folder( specifier.realm().copied().unwrap_or_default(), )); @@ -313,7 +292,7 @@ impl Project { lockfile, linking_dir, self.path().join(resolved_pkg.packages_folder()), - !is_duplicate_in(name.name(), &root_dep_names), + desired_name, true, ) .map_err(|e| { diff --git a/src/manifest.rs b/src/manifest.rs index cc376ea..8f46dc3 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -189,11 +189,11 @@ pub struct Manifest { pub overrides: BTreeMap, /// The dependencies of the package - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub dependencies: Vec, + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub dependencies: BTreeMap, /// The peer dependencies of the package - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub peer_dependencies: Vec, + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub peer_dependencies: BTreeMap, } /// An error that occurred while reading the manifest @@ -338,7 +338,7 @@ impl Manifest { overrides: BTreeMap::new(), dependencies, - peer_dependencies: Vec::new(), + peer_dependencies: Default::default(), description: wally_manifest.package.description, license: wally_manifest.package.license, authors: wally_manifest.package.authors, @@ -361,14 +361,24 @@ impl Manifest { } /// Returns all dependencies - pub fn dependencies(&self) -> Vec<(DependencySpecifier, DependencyType)> { + pub fn dependencies(&self) -> BTreeMap { self.dependencies .iter() - .map(|dep| (dep.clone(), DependencyType::Normal)) + .map(|(desired_name, specifier)| { + ( + desired_name.clone(), + (specifier.clone(), DependencyType::Normal), + ) + }) .chain( self.peer_dependencies .iter() - .map(|dep| (dep.clone(), DependencyType::Peer)), + .map(|(desired_name, specifier)| { + ( + desired_name.clone(), + (specifier.clone(), DependencyType::Peer), + ) + }), ) .collect() } diff --git a/tests/resolver.rs b/tests/resolver.rs index 359be44..a7e748e 100644 --- a/tests/resolver.rs +++ b/tests/resolver.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeSet, HashMap}; +use std::collections::{BTreeMap, BTreeSet, HashMap}; use semver::Version; use tempfile::tempdir; @@ -45,8 +45,8 @@ fn test_resolves_package() { sourcemap_generator: None, overrides: Default::default(), - dependencies: vec![], - peer_dependencies: vec![], + dependencies: Default::default(), + peer_dependencies: Default::default(), description: Some(description.to_string()), license: None, authors: None, @@ -86,8 +86,8 @@ fn test_resolves_package() { sourcemap_generator: None, overrides: Default::default(), - dependencies: vec![specifier.clone()], - peer_dependencies: vec![specifier_2.clone()], + dependencies: BTreeMap::from([("test".to_string(), specifier.clone())]), + peer_dependencies: BTreeMap::from([("test2".to_string(), specifier_2.clone())]), description: Some(description.to_string()), license: None, authors: None,