From a067fbd4bd4c73588f908843b5a1f74707a43a87 Mon Sep 17 00:00:00 2001 From: daimond113 <72147841+daimond113@users.noreply.github.com> Date: Fri, 22 Nov 2024 15:52:54 +0100 Subject: [PATCH] fix: correctly resolve peer dependencies --- CHANGELOG.md | 4 ++++ src/cli/commands/install.rs | 4 ++-- src/cli/commands/outdated.rs | 2 +- src/cli/commands/publish.rs | 2 +- src/cli/mod.rs | 4 ++-- src/download.rs | 2 +- src/linking/mod.rs | 2 +- src/lockfile.rs | 8 ++++---- src/resolver.rs | 23 +++++++++++++---------- 9 files changed, 29 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7dfe0a..699d394 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] +### Fixed +- Fix peer dependencies being resolved incorrectly by @daimond113 + ## [0.5.0-rc.11] - 2024-11-20 ### Fixed - Add back mistakenly removed updates check caching by @daimond113 diff --git a/src/cli/commands/install.rs b/src/cli/commands/install.rs index 4dc7b22..3a6df0f 100644 --- a/src/cli/commands/install.rs +++ b/src/cli/commands/install.rs @@ -231,7 +231,7 @@ impl InstallCommand { ( n, v.into_iter() - .filter(|(_, n)| n.node.ty != DependencyType::Dev) + .filter(|(_, n)| n.node.resolved_ty != DependencyType::Dev) .collect(), ) }) @@ -268,7 +268,7 @@ impl InstallCommand { .flat_map(|versions| versions.values()) .filter(|node| node.target.bin_path().is_some()) .filter_map(|node| node.node.direct.as_ref()) - .map(|(alias, _)| alias) + .map(|(alias, _, _)| alias) .filter(|alias| { if *alias == env!("CARGO_BIN_NAME") { log::warn!( diff --git a/src/cli/commands/outdated.rs b/src/cli/commands/outdated.rs index 8ca531a..828493b 100644 --- a/src/cli/commands/outdated.rs +++ b/src/cli/commands/outdated.rs @@ -60,7 +60,7 @@ impl OutdatedCommand { .map(|(current_version_id, node)| { let project = project.clone(); async move { - let Some((alias, mut specifier)) = node.node.direct else { + let Some((alias, mut specifier, _)) = node.node.direct else { return Ok::<(), anyhow::Error>(()); }; diff --git a/src/cli/commands/publish.rs b/src/cli/commands/publish.rs index 4ae26a5..57872c8 100644 --- a/src/cli/commands/publish.rs +++ b/src/cli/commands/publish.rs @@ -81,7 +81,7 @@ impl PublishCommand { .filter_map(|(_, node)| node.node.direct.as_ref().map(|_| node)) .any(|node| { node.target.build_files().is_none() - && !matches!(node.node.ty, DependencyType::Dev) + && !matches!(node.node.resolved_ty, DependencyType::Dev) }) { anyhow::bail!("roblox packages may not depend on non-roblox packages"); diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 5c68e5e..fc2bdba 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -79,7 +79,7 @@ pub async fn up_to_date_lockfile(project: &Project) -> anyhow::Result>(); @@ -87,7 +87,7 @@ pub async fn up_to_date_lockfile(project: &Project) -> anyhow::Result {} Err(e) => { diff --git a/src/linking/mod.rs b/src/linking/mod.rs index f3cb11c..3e330c3 100644 --- a/src/linking/mod.rs +++ b/src/linking/mod.rs @@ -137,7 +137,7 @@ impl Project { version_id.version(), ); - if let Some((alias, _)) = &node.node.direct.as_ref() { + if let Some((alias, _, _)) = &node.node.direct.as_ref() { if let Some((lib_file, types)) = node.target.lib_path().and_then(|lib_file| { package_types diff --git a/src/lockfile.rs b/src/lockfile.rs index 2df59cf..e776b66 100644 --- a/src/lockfile.rs +++ b/src/lockfile.rs @@ -24,14 +24,14 @@ pub type Graph = BTreeMap>; /// A dependency graph node #[derive(Serialize, Deserialize, Debug, Clone)] pub struct DependencyGraphNode { - /// The alias and specifiers for the dependency, if it is a direct dependency (i.e. used by the current project) + /// 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<(String, DependencySpecifiers)>, + pub direct: Option<(String, DependencySpecifiers, DependencyType)>, /// The dependencies of the package #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub dependencies: BTreeMap, - /// The type of the dependency - pub ty: DependencyType, + /// The resolved (transformed, for example Peer -> Standard) type of the dependency + pub resolved_ty: DependencyType, /// The package reference pub pkg_ref: PackageRefs, } diff --git a/src/resolver.rs b/src/resolver.rs index c799bc5..c11c392 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -37,7 +37,7 @@ impl Project { if let Some(previous_graph) = previous_graph { for (name, versions) in previous_graph { for (version, node) in versions { - let Some((_, specifier)) = &node.direct else { + let Some((_, specifier, source_ty)) = &node.direct else { // this is not a direct dependency, will be added if it's still being used later continue; }; @@ -47,7 +47,8 @@ impl Project { continue; } - let Some(alias) = all_specifiers.remove(&(specifier.clone(), node.ty)) else { + let Some(alias) = all_specifiers.remove(&(specifier.clone(), *source_ty)) + else { log::debug!( "dependency {name}@{version} from old dependency graph is no longer in the manifest", ); @@ -60,7 +61,7 @@ impl Project { name.clone(), version.clone(), DependencyGraphNode { - direct: Some((alias.clone(), specifier.clone())), + direct: Some((alias.clone(), specifier.clone(), *source_ty)), ..node.clone() }, true, @@ -218,7 +219,7 @@ impl Project { ))); }; - let ty = if depth == 0 && ty == DependencyType::Peer { + let resolved_ty = if depth == 0 && ty == DependencyType::Peer { DependencyType::Standard } else { ty @@ -255,12 +256,14 @@ impl Project { ); } - if already_resolved.ty == DependencyType::Peer && ty == DependencyType::Standard { - already_resolved.ty = ty; + if already_resolved.resolved_ty == DependencyType::Peer + && resolved_ty == DependencyType::Standard + { + already_resolved.resolved_ty = resolved_ty; } if already_resolved.direct.is_none() && depth == 0 { - already_resolved.direct = Some((alias.clone(), specifier.clone())); + already_resolved.direct = Some((alias.clone(), specifier.clone(), ty)); } continue; @@ -268,13 +271,13 @@ impl Project { let node = DependencyGraphNode { direct: if depth == 0 { - Some((alias.clone(), specifier.clone())) + Some((alias.clone(), specifier.clone(), ty)) } else { None }, pkg_ref: pkg_ref.clone(), dependencies: Default::default(), - ty, + resolved_ty, }; insert_node( &mut graph, @@ -335,7 +338,7 @@ impl Project { for (name, versions) in &graph { for (version_id, node) in versions { - if node.ty == DependencyType::Peer { + if node.resolved_ty == DependencyType::Peer { log::warn!("peer dependency {name}@{version_id} was not resolved"); } }