diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b7302b..71418aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Download engines in install step rather than lazily by @daimond113 +- Rewrite dependency type system to solve multiple issues by @daimond113 ### Performance - Remove unnecessary `Arc`s from codebase by @daimond113 diff --git a/src/cli/commands/publish.rs b/src/cli/commands/publish.rs index ee9c36c..bb11ade 100644 --- a/src/cli/commands/publish.rs +++ b/src/cli/commands/publish.rs @@ -172,7 +172,10 @@ impl PublishCommand { Ok::<_, anyhow::Error>( target.build_files().is_none() - && !matches!(node.resolved_ty, DependencyType::Dev), + && node + .direct + .as_ref() + .is_some_and(|(_, _, ty)| *ty != DependencyType::Dev), ) } }) diff --git a/src/cli/install.rs b/src/cli/install.rs index 65b7221..49a2507 100644 --- a/src/cli/install.rs +++ b/src/cli/install.rs @@ -53,7 +53,7 @@ impl DownloadAndLinkHooks for InstallHooks { let aliases = graph .iter() .flat_map(|(_, node)| node.node.dependencies.iter()) - .filter_map(|(id, alias)| binary_packages.contains(id).then_some(alias.as_str())) + .filter_map(|(id, (alias, _))| binary_packages.contains(id).then_some(alias.as_str())) .chain( graph .iter() @@ -202,6 +202,8 @@ pub async fn install( .await .context("failed to build dependency graph")?; + check_peers_satisfied(&graph); + let mut tasks = graph .iter() .filter_map(|(id, node)| { @@ -537,3 +539,58 @@ pub fn print_package_diff(prefix: &str, old_graph: &DependencyGraph, new_graph: println!(); } } + +pub fn check_peers_satisfied(graph: &DependencyGraph) { + for (id, node) in graph { + let Some((alias, _, _)) = &node.direct else { + continue; + }; + + let mut queue = node + .dependencies + .iter() + .map(|(dep_id, (dep_alias, dep_ty))| (vec![(id, alias)], (dep_id, dep_alias), *dep_ty)) + .collect::>(); + + while let Some((path, (dep_id, dep_alias), dep_ty)) = queue.pop() { + if dep_ty == DependencyType::Peer { + let mut iter = path + .iter() + .map(|(id, _)| id) + .rev() + // skip our parent since we're always going to be descendants of it + .skip(1) + .take(2); + + let satisfied = if iter.len() > 0 { + iter.any(|id| graph[id].dependencies.contains_key(dep_id)) + } else { + graph.get(dep_id).is_some_and(|node| node.direct.is_some()) + }; + + if !satisfied { + eprintln!( + "{WARN_PREFIX}: peer dependency {}>{dep_alias} is not satisfied", + path.iter() + .map(|(_, alias)| alias.as_str()) + .collect::>() + .join(">"), + ); + } + } + + queue.extend(graph[dep_id].dependencies.iter().map( + |(inner_dep_id, (inner_dep_alias, inner_dep_ty))| { + ( + path.iter() + .copied() + .chain(std::iter::once((dep_id, dep_alias))) + .collect(), + (inner_dep_id, inner_dep_alias), + *inner_dep_ty, + ) + }, + )); + } + } +} diff --git a/src/download_and_link.rs b/src/download_and_link.rs index 92d8888..6d2f85a 100644 --- a/src/download_and_link.rs +++ b/src/download_and_link.rs @@ -268,7 +268,9 @@ impl Project { let mut queue = graph .iter() .filter(|(_, node)| { - node.direct.is_some() && install_dependencies_mode.fits(node.resolved_ty) + node.direct + .as_ref() + .is_some_and(|(_, _, ty)| install_dependencies_mode.fits(*ty)) }) .collect::>(); diff --git a/src/graph.rs b/src/graph.rs index b326abc..d5807bd 100644 --- a/src/graph.rs +++ b/src/graph.rs @@ -25,12 +25,7 @@ pub struct DependencyGraphNode { pub direct: Option<(Alias, DependencySpecifiers, DependencyType)>, /// The dependencies of the package #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - pub dependencies: BTreeMap, - /// The resolved (transformed, for example Peer -> Standard) type of the dependency - pub resolved_ty: DependencyType, - /// Whether the resolved type should be Peer if this isn't depended on - #[serde(default, skip_serializing_if = "std::ops::Not::not")] - pub is_peer: bool, + pub dependencies: BTreeMap, /// The package reference pub pkg_ref: PackageRefs, } diff --git a/src/linking/mod.rs b/src/linking/mod.rs index da04291..5d1ac44 100644 --- a/src/linking/mod.rs +++ b/src/linking/mod.rs @@ -307,7 +307,7 @@ impl Project { (container_folder, base_folder) }; - for (dep_id, dep_alias) in &node.node.dependencies { + for (dep_id, (dep_alias, _)) in &node.node.dependencies { let dep_id = dep_id.clone(); let dep_alias = dep_alias.clone(); let dep_node = graph.get(&dep_id).cloned(); diff --git a/src/lockfile.rs b/src/lockfile.rs index 653f1fa..c455488 100644 --- a/src/lockfile.rs +++ b/src/lockfile.rs @@ -3,15 +3,15 @@ use crate::{ graph::DependencyGraph, manifest::{overrides::OverrideKey, target::TargetKind}, names::PackageName, - source::{ids::PackageId, specifiers::DependencySpecifiers}, + source::specifiers::DependencySpecifiers, }; use relative_path::RelativePathBuf; use semver::Version; use serde::{Deserialize, Serialize}; -use std::{collections::BTreeMap, fmt::Debug}; +use std::collections::BTreeMap; /// The current format of the lockfile -pub const CURRENT_FORMAT: usize = 1; +pub const CURRENT_FORMAT: usize = 2; /// A lockfile #[derive(Serialize, Deserialize, Debug, Clone)] @@ -47,120 +47,12 @@ pub fn parse_lockfile(lockfile: &str) -> Result { - let this = match toml::from_str(lockfile) { - Ok(lockfile) => return Ok(lockfile), - Err(e) => match toml::from_str::(lockfile) { - Ok(this) => this, - Err(_) => return Err(errors::ParseLockfileError::De(e)), - }, - }; - - Ok(Lockfile { - name: this.name, - version: this.version, - target: this.target, - overrides: this.overrides, - workspace: this.workspace, - graph: this - .graph - .into_iter() - .flat_map(|(name, versions)| { - versions.into_iter().map(move |(version, node)| { - ( - PackageId(name.clone(), version), - crate::graph::DependencyGraphNode { - direct: node.node.direct, - dependencies: node - .node - .dependencies - .into_iter() - .map(|(name, (version, alias))| { - (PackageId(name, version), alias) - }) - .collect(), - resolved_ty: node.node.resolved_ty, - is_peer: node.node.is_peer, - pkg_ref: node.node.pkg_ref, - }, - ) - }) - }) - .collect(), - }) - } CURRENT_FORMAT => toml::de::from_str(lockfile).map_err(Into::into), + format if format < CURRENT_FORMAT => Err(errors::ParseLockfileError::TooOld(format)), format => Err(errors::ParseLockfileError::TooNew(format)), } } -/// Lockfile v0 -pub mod v0 { - use crate::{ - manifest::{ - overrides::OverrideKey, - target::{Target, TargetKind}, - Alias, DependencyType, - }, - names::{PackageName, PackageNames}, - source::{ids::VersionId, refs::PackageRefs, specifiers::DependencySpecifiers}, - }; - use relative_path::RelativePathBuf; - use semver::Version; - use serde::{Deserialize, Serialize}; - use std::collections::BTreeMap; - - /// A dependency graph node - #[derive(Serialize, Deserialize, Debug, Clone)] - pub struct DependencyGraphNode { - /// The alias, specifier, and original (as in the manifest) type for the dependency, if it is a direct dependency (i.e. used by the current project) - #[serde(default, skip_serializing_if = "Option::is_none")] - pub direct: Option<(Alias, DependencySpecifiers, DependencyType)>, - /// The dependencies of the package - #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - pub dependencies: BTreeMap, - /// The resolved (transformed, for example Peer -> Standard) type of the dependency - pub resolved_ty: DependencyType, - /// Whether the resolved type should be Peer if this isn't depended on - #[serde(default, skip_serializing_if = "std::ops::Not::not")] - pub is_peer: bool, - /// The package reference - pub pkg_ref: PackageRefs, - } - - /// A downloaded dependency graph node, i.e. a `DependencyGraphNode` with a `Target` - #[derive(Serialize, Deserialize, Debug, Clone)] - pub struct DownloadedDependencyGraphNode { - /// The target of the package - pub target: Target, - /// The node - #[serde(flatten)] - pub node: DependencyGraphNode, - } - - /// A lockfile - #[derive(Serialize, Deserialize, Debug, Clone)] - pub struct Lockfile { - /// The name of the package - pub name: PackageName, - /// The version of the package - pub version: Version, - /// The target of the package - pub target: TargetKind, - /// The overrides of the package - #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - pub overrides: BTreeMap, - - /// The workspace members - #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - pub workspace: BTreeMap>, - - /// The graph of dependencies - #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - pub graph: BTreeMap>, - } -} - /// Errors that can occur when working with lockfiles pub mod errors { use thiserror::Error; @@ -173,6 +65,10 @@ pub mod errors { #[error("lockfile format {} is too new. newest supported format: {}", .0, super::CURRENT_FORMAT)] TooNew(usize), + /// The lockfile format is too old + #[error("lockfile format {} is too old. manual deletion is required. current format: {}", .0, super::CURRENT_FORMAT)] + TooOld(usize), + /// Deserializing the lockfile failed #[error("deserializing the lockfile failed")] De(#[from] toml::de::Error), diff --git a/src/resolver.rs b/src/resolver.rs index 7d60b14..2051600 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -114,7 +114,9 @@ impl Project { let mut queue = node .dependencies .iter() - .map(|(id, dep_alias)| (id, vec![alias.to_string(), dep_alias.to_string()])) + .map(|(id, (dep_alias, _))| { + (id, vec![alias.to_string(), dep_alias.to_string()]) + }) .collect::>(); while let Some((dep_id, path)) = queue.pop_front() { @@ -135,7 +137,7 @@ impl Project { dep_node .dependencies .iter() - .map(|(id, alias)| { + .map(|(id, (alias, _))| { ( id, path.iter() @@ -270,19 +272,12 @@ impl Project { ))); }; - let resolved_ty = if (is_published_package || depth == 0) && ty == DependencyType::Peer - { - DependencyType::Standard - } else { - ty - }; - if let Some(dependant_id) = dependant { graph .get_mut(&dependant_id) .expect("dependant package not found in graph") .dependencies - .insert(package_id.clone(), alias.clone()); + .insert(package_id.clone(), (alias.clone(), ty)); } let pkg_ref = &resolved[package_id.version_id()]; @@ -296,14 +291,6 @@ impl Project { ); } - if already_resolved.resolved_ty == DependencyType::Peer { - already_resolved.resolved_ty = resolved_ty; - } - - if ty == DependencyType::Peer && depth == 0 { - already_resolved.is_peer = true; - } - if already_resolved.direct.is_none() && depth == 0 { already_resolved.direct = Some((alias.clone(), specifier.clone(), ty)); } @@ -315,12 +302,6 @@ impl Project { direct: (depth == 0).then(|| (alias.clone(), specifier.clone(), ty)), pkg_ref: pkg_ref.clone(), dependencies: Default::default(), - resolved_ty, - is_peer: if depth == 0 { - false - } else { - ty == DependencyType::Peer - }, }; insert_node( &mut graph, @@ -388,16 +369,6 @@ impl Project { .await?; } - for (id, node) in &mut graph { - if node.is_peer && node.direct.is_none() { - node.resolved_ty = DependencyType::Peer; - } - - if node.resolved_ty == DependencyType::Peer { - tracing::warn!("peer dependency {id} was not resolved"); - } - } - Ok(graph) } }