diff --git a/.changeset/backtrack-across-compatibility-lines.md b/.changeset/backtrack-across-compatibility-lines.md new file mode 100644 index 0000000000..7828aa3601 --- /dev/null +++ b/.changeset/backtrack-across-compatibility-lines.md @@ -0,0 +1,5 @@ +--- +"pacquet": patch +--- + +`pnpm install` now falls back to an older semver-incompatible version of a crate when the newest one a dependency range allows cannot be resolved. Ranges such as `>=1, <3` span several of them [pnpm/pnpm#14962](https://github.com/pnpm/pnpm/issues/14962). diff --git a/pnpm/crates/cargo-resolver/src/features.rs b/pnpm/crates/cargo-resolver/src/features.rs index 5dd5679659..4701c4b8ee 100644 --- a/pnpm/crates/cargo-resolver/src/features.rs +++ b/pnpm/crates/cargo-resolver/src/features.rs @@ -1,6 +1,7 @@ use crate::{ model::{DependencyKind, FeatureSelection, PackageKey, RegistryDependency, RegistryVersion}, - registry::{Registry, newest_compatibility}, + packages::{newest_line_package, selected_package}, + registry::Registry, }; use miette::Result; use pubgrub::SelectedDependencies; @@ -177,27 +178,44 @@ fn collect_feature_selections( while let Some(dependency) = pending.pop_front() { registry.validate_dependency_source(dependency.registry.as_deref())?; let versions = registry.package(&dependency.name)?; - let Some(compatibility) = newest_compatibility(versions, &dependency.requirement) else { - continue; + // Before there is a solution the line is a prediction, and the + // solver reaches for the newest first. Once there is one, the line + // it settled on is the only one this dependency asked anything of. + let package = match solution { + Some(solution) => selected_package(registry, &dependency, solution)?, + None => newest_line_package(registry, &dependency)?, }; - let package = PackageKey::Registry { name: dependency.name.clone(), compatibility }; - let requested = dependency.feature_selection(); - let previous = selections.get(&package).cloned(); - let selection = selections.entry(package.clone()).or_default(); - selection.default_features |= requested.default_features; - selection.features.extend(requested.features); - if previous.as_ref() == Some(selection) { + let Some(package) = package else { continue }; + if !widen_line_selection(&mut selections, &dependency, &package) { continue; } let Some(selected_version) = solution.and_then(|solution| solution.get(&package)) else { continue; }; let selected = indexed_version(versions, &dependency.name, selected_version)?; - pending.extend(active_dependencies(selected, selection)?); + let selection = selections + .get(&package) + .cloned() + .unwrap_or_default(); + pending.extend(active_dependencies(selected, &selection)?); } Ok(selections) } +/// Fold what `dependency` asks for into `package`'s selection, reporting +/// whether that added anything, which is what makes it worth walking again. +fn widen_line_selection( + selections: &mut BTreeMap, + dependency: &RegistryDependency, + package: &PackageKey, +) -> bool { + let previous = selections.get(package).cloned(); + let selection = selections.entry(package.clone()).or_default(); + selection.default_features |= dependency.default_features; + selection.features.extend(dependency.features.iter().cloned()); + previous.as_ref() != Some(selection) +} + pub(crate) fn indexed_version<'v>( versions: &'v [RegistryVersion], name: &str, diff --git a/pnpm/crates/cargo-resolver/src/lib.rs b/pnpm/crates/cargo-resolver/src/lib.rs index 50df970ea2..3e2f8a387c 100644 --- a/pnpm/crates/cargo-resolver/src/lib.rs +++ b/pnpm/crates/cargo-resolver/src/lib.rs @@ -14,6 +14,7 @@ mod features; mod lockfile; mod metadata; mod model; +mod packages; mod registry; mod resolution; diff --git a/pnpm/crates/cargo-resolver/src/lockfile.rs b/pnpm/crates/cargo-resolver/src/lockfile.rs index 61c0b30935..8c03c565af 100644 --- a/pnpm/crates/cargo-resolver/src/lockfile.rs +++ b/pnpm/crates/cargo-resolver/src/lockfile.rs @@ -2,9 +2,8 @@ use crate::{ features::{active_dependencies, indexed_version}, metadata::{active_metadata_dependencies, root_dependencies}, model::{CargoMetadata, FeatureSelection, PackageKey, RegistryDependency, RegistryVersion}, - registry::{ - CRATES_IO_SOURCE, Registry, compatibility_line, is_crates_io_source, matching_versions, - }, + packages::selected_package, + registry::{CRATES_IO_SOURCE, Registry, is_crates_io_source}, }; use cargo_lock::{Checksum, Dependency, Lockfile, Metadata, Name, Package, Patch, ResolveVersion}; use miette::{IntoDiagnostic, Result, WrapErr}; @@ -26,7 +25,7 @@ pub(crate) fn lockfile_from_solution( configured: &str, ) -> Result { let selected = solution.iter().collect::>(); - let sources = locked_sources(metadata, registry, &selected, feature_selections, configured)?; + let sources = locked_sources(metadata, registry, solution, feature_selections, configured)?; let mut packages = Vec::new(); for (key, version) in &selected { @@ -42,7 +41,7 @@ pub(crate) fn lockfile_from_solution( registry_version, &selection, registry, - &selected, + solution, &sources, )?; packages.push(Package { @@ -55,7 +54,7 @@ pub(crate) fn lockfile_from_solution( }); } - packages.extend(workspace_packages(metadata, registry, &selected, &sources)?); + packages.extend(workspace_packages(metadata, registry, solution, &sources)?); packages.sort(); let lockfile = Lockfile { @@ -72,7 +71,7 @@ pub(crate) fn lockfile_from_solution( fn workspace_packages( metadata: &CargoMetadata, registry: &Registry, - selected: &BTreeMap<&PackageKey, &Version>, + solution: &pubgrub::SelectedDependencies, sources: &BTreeMap, ) -> Result> { let mut packages = Vec::new(); @@ -84,13 +83,7 @@ fn workspace_packages( .iter() .map(|dependency| { if dependency.registry.is_some() { - locked_dependency( - &dependency.name, - &dependency.requirement, - registry, - selected, - sources, - ) + locked_dependency(dependency, registry, solution, sources) } else { locked_workspace_dependency(&dependency.name, &dependency.requirement, metadata) } @@ -113,50 +106,37 @@ fn locked_registry_dependencies( package: &RegistryVersion, selection: &FeatureSelection, registry: &Registry, - selected: &BTreeMap<&PackageKey, &Version>, + solution: &pubgrub::SelectedDependencies, sources: &BTreeMap, ) -> Result> { let mut dependencies = BTreeSet::new(); for dependency in active_dependencies(package, selection)? { registry.validate_dependency_source(dependency.registry.as_deref())?; - dependencies.insert(locked_dependency( - &dependency.name, - &dependency.requirement, - registry, - selected, - sources, - )?); + dependencies.insert(locked_dependency(&dependency, registry, solution, sources)?); } Ok(dependencies.into_iter().collect()) } fn locked_dependency( - name: &str, - requirement: &VersionReq, + dependency: &RegistryDependency, registry: &Registry, - selected: &BTreeMap<&PackageKey, &Version>, + solution: &pubgrub::SelectedDependencies, sources: &BTreeMap, ) -> Result { - let key = resolved_key(registry, name, requirement)?; - let version = selected - .get(&key) + let name = dependency.name.as_str(); + let (package, version) = selected_package(registry, dependency, solution)? + .and_then(|package| { + let version = solution.get(&package)?.clone(); + Some((package, version)) + }) .ok_or_else(|| miette::miette!("resolver did not select dependency {name}"))?; Ok(Dependency { name: Name::from_str(name).into_diagnostic()?, - version: (*version).clone(), - source: Some(package_source(sources, &key)?), + version, + source: Some(package_source(sources, &package)?), }) } -/// The package a requirement resolves against. -fn resolved_key(registry: &Registry, name: &str, requirement: &VersionReq) -> Result { - let compatibility = matching_versions(registry.package(name)?, requirement) - .next_back() - .map(|version| compatibility_line(&version.version)) - .ok_or_else(|| miette::miette!("no version of {name} satisfies {requirement}"))?; - Ok(PackageKey::Registry { name: name.to_string(), compatibility }) -} - fn package_source( sources: &BTreeMap, package: &PackageKey, @@ -180,7 +160,7 @@ fn package_source( fn locked_sources( metadata: &CargoMetadata, registry: &Registry, - selected: &BTreeMap<&PackageKey, &Version>, + solution: &pubgrub::SelectedDependencies, feature_selections: &BTreeMap, configured: &str, ) -> Result> { @@ -192,8 +172,9 @@ fn locked_sources( while let Some((dependency, inherited)) = pending.pop_front() { let source = declared_source(&dependency, &inherited, configured); - let key = resolved_key(registry, &dependency.name, &dependency.requirement)?; - let Some(version) = selected.get(&key) else { continue }; + let key = selected_package(registry, &dependency, solution)?; + let Some(key) = key else { continue }; + let Some(version) = solution.get(&key) else { continue }; match sources.entry(key.clone()) { Entry::Occupied(known) if *known.get() == source => continue, Entry::Occupied(known) => { diff --git a/pnpm/crates/cargo-resolver/src/model.rs b/pnpm/crates/cargo-resolver/src/model.rs index 3989bbcc4b..6d6e6de4b9 100644 --- a/pnpm/crates/cargo-resolver/src/model.rs +++ b/pnpm/crates/cargo-resolver/src/model.rs @@ -60,6 +60,15 @@ pub(crate) struct RegistryDependency { pub(crate) features: BTreeSet, } +impl RegistryDependency { + pub(crate) fn feature_selection(&self) -> FeatureSelection { + FeatureSelection { + default_features: self.default_features, + features: self.features.clone(), + } + } +} + #[derive(Debug, Default, Clone, PartialEq, Eq)] pub(crate) struct FeatureSelection { pub(crate) default_features: bool, @@ -96,15 +105,6 @@ impl<'de> Deserialize<'de> for DependencyKind { } } -impl RegistryDependency { - pub(crate) fn feature_selection(&self) -> FeatureSelection { - FeatureSelection { - default_features: self.default_features, - features: self.features.clone(), - } - } -} - #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub(crate) enum PackageKey { Root, @@ -112,6 +112,20 @@ pub(crate) enum PackageKey { name: String, compatibility: String, }, + /// A requirement met on more than one compatibility line. Its versions + /// stand for those lines, each depending on the line it names, so the + /// solver can try the newest and backtrack to an older one. + /// + /// The features are part of the key: two dependencies asking the same + /// crate for different features may need different lines, and a version + /// that suits both still unifies them, because both choices lead to the + /// same line package. + Requirement { + name: String, + requirement: String, + default_features: bool, + features: Vec, + }, /// A dependency nothing in the index meets. Nothing is ever registered /// under it, so the solver finds no version to pick. Unsatisfiable { @@ -125,6 +139,9 @@ impl fmt::Display for PackageKey { match self { Self::Root => formatter.write_str("pnpm Cargo workspace"), Self::Registry { name, compatibility } => write!(formatter, "{name}@{compatibility}"), + Self::Requirement { name, requirement, .. } => { + write!(formatter, "{name} {requirement}") + } Self::Unsatisfiable { name, requirement } => { write!(formatter, "{name} {requirement} (no version available)") } diff --git a/pnpm/crates/cargo-resolver/src/packages.rs b/pnpm/crates/cargo-resolver/src/packages.rs new file mode 100644 index 0000000000..8fef7c3fe9 --- /dev/null +++ b/pnpm/crates/cargo-resolver/src/packages.rs @@ -0,0 +1,113 @@ +use crate::{ + features::supports_features, + model::{FeatureSelection, PackageKey, RegistryDependency}, + registry::{Registry, compatibility_line, matching_lines, matching_versions}, +}; +use miette::Result; +use pubgrub::SelectedDependencies; +use semver::{Version, VersionReq}; + +/// The solver package a requirement resolves against. +/// +/// A requirement met on one compatibility line names that line's package +/// directly. One met on several names a [`PackageKey::Requirement`] package +/// standing for the choice between them, which is how the solver backtracks +/// from a line that cannot be satisfied to an older one, as `cargo` does. +/// A requirement nothing meets names a package with no versions at all. +/// +/// The caller is responsible for [`Registry::validate_dependency_source`]; +/// this only maps a name and a requirement onto a package. +pub(crate) fn package_key( + registry: &Registry, + dependency: &RegistryDependency, +) -> Result { + let name = dependency.name.as_str(); + let requirement = &dependency.requirement; + let unsatisfiable = || PackageKey::Unsatisfiable { + name: name.to_string(), + requirement: requirement.to_string(), + }; + let Some(versions) = registry.versions(name) else { + return Ok(unsatisfiable()); + }; + let mut lines = matching_lines(versions, requirement); + if lines.len() > 1 { + return Ok(PackageKey::Requirement { + name: name.to_string(), + requirement: requirement.to_string(), + default_features: dependency.default_features, + features: dependency.features + .iter() + .cloned() + .collect(), + }); + } + match lines.pop() { + Some((compatibility, _)) => { + Ok(PackageKey::Registry { name: name.to_string(), compatibility }) + } + None => Ok(unsatisfiable()), + } +} + +/// The line package a requirement resolves to before there is a solution to +/// read the choice from: the newest line it is met on, which is the one the +/// solver reaches for first. +pub(crate) fn newest_line_package( + registry: &Registry, + dependency: &RegistryDependency, +) -> Result> { + let Some(versions) = registry.versions(&dependency.name) else { + return Ok(None); + }; + let selection = dependency.feature_selection(); + Ok(matching_lines(versions, &dependency.requirement) + .into_iter() + .rfind(|(compatibility, _)| { + admits_features(versions, &dependency.requirement, compatibility, &selection) + }) + .map(|(compatibility, _)| PackageKey::Registry { + name: dependency.name.clone(), + compatibility, + })) +} + +/// Whether `compatibility` carries a version meeting `requirement` that +/// supports `selection`. A line that does not is not a line the dependency +/// asking for those features can settle on. +pub(crate) fn admits_features( + versions: &[crate::model::RegistryVersion], + requirement: &VersionReq, + compatibility: &str, + selection: &FeatureSelection, +) -> bool { + matching_versions(versions, requirement) + .filter(|version| compatibility_line(&version.version) == compatibility) + .any(|version| supports_features(version, selection)) +} + +/// The line package a [`PackageKey::Requirement`] choice settled on, named +/// by the version the solver picked for it. +pub(crate) fn chosen_line(name: &str, representative: &Version) -> PackageKey { + PackageKey::Registry { + name: name.to_string(), + compatibility: compatibility_line(representative), + } +} + +/// The registry package a requirement resolved to, reading the chosen +/// compatibility line out of `solution` when the requirement spans several. +/// `None` when the solution does not reach it. +pub(crate) fn selected_package( + registry: &Registry, + dependency: &RegistryDependency, + solution: &SelectedDependencies, +) -> Result> { + Ok(match package_key(registry, dependency)? { + line @ PackageKey::Registry { .. } => Some(line), + choice @ PackageKey::Requirement { .. } => solution + .get(&choice) + .map(|representative| chosen_line(&dependency.name, representative)), + PackageKey::Root | PackageKey::Unsatisfiable { .. } => None, + }) +} diff --git a/pnpm/crates/cargo-resolver/src/registry.rs b/pnpm/crates/cargo-resolver/src/registry.rs index 8bd50c8c01..44c3a3f42b 100644 --- a/pnpm/crates/cargo-resolver/src/registry.rs +++ b/pnpm/crates/cargo-resolver/src/registry.rs @@ -175,15 +175,22 @@ fn registry_dependency_from_index(dependency: IndexDependency<'_>) -> Result Option { - matching_versions(versions, requirement) - .next_back() - .map(|version| compatibility_line(&version.version)) +) -> Vec<(String, Version)> { + // Ascending, so the last version recorded for a line is its newest. + let newest = matching_versions(versions, requirement) + .map(|version| (compatibility_line(&version.version), version.version.clone())) + .collect::>(); + let mut lines = newest.into_iter().collect::>(); + lines.sort_by(|left, right| left.1.cmp(&right.1)); + lines } pub(crate) fn matching_versions<'a>( diff --git a/pnpm/crates/cargo-resolver/src/resolution.rs b/pnpm/crates/cargo-resolver/src/resolution.rs index 3004924fe7..96318c09f0 100644 --- a/pnpm/crates/cargo-resolver/src/resolution.rs +++ b/pnpm/crates/cargo-resolver/src/resolution.rs @@ -1,18 +1,19 @@ use crate::{ features::{ - active_dependencies, feature_selections_for_solution, root_feature_selections, - supports_features, + active_dependencies, feature_selections_for_solution, indexed_version, + root_feature_selections, supports_features, }, lockfile::lockfile_from_solution, metadata::{parse_metadata, root_dependencies}, model::{FeatureSelection, PackageKey, RegistryDependency, RegistryVersion}, - registry::{Registry, compatibility_line, matching_versions, newest_compatibility}, + packages::{chosen_line, package_key}, + registry::{Registry, compatibility_line, matching_lines, matching_versions}, }; -use miette::Result; +use miette::{IntoDiagnostic, Result, WrapErr}; use pubgrub::{ DefaultStringReporter, OfflineDependencyProvider, PubGrubError, Ranges, Reporter, resolve, }; -use semver::Version; +use semver::{Version, VersionReq}; use std::collections::{BTreeMap, BTreeSet, VecDeque}; /// What discovery has reached a crate with so far: the features every @@ -73,28 +74,34 @@ fn unified_dependencies( dependency: &RegistryDependency, versions: &[RegistryVersion], ) -> Result> { - let Some(compatibility) = newest_compatibility(versions, &dependency.requirement) else { - return Ok(Vec::new()); - }; - let selectable = matching_versions(versions, &dependency.requirement) - .filter(|version| compatibility_line(&version.version) == compatibility) - .map(|version| version.version.clone()) - .collect::>(); - let package = PackageKey::Registry { name: dependency.name.clone(), compatibility }; - let entry = discovered.entry(package).or_default(); - let previous = entry.selection.clone(); - entry.selection.default_features |= dependency.default_features; - entry.selection.features.extend(dependency.features.iter().cloned()); - let unwalked = &selectable - &entry.versions; - entry.versions.extend(selectable); - let walk = if entry.selection == previous { unwalked } else { entry.versions.clone() }; let mut reached = Vec::new(); - for version in versions - .iter() - .filter(|version| walk.contains(&version.version)) - .filter(|version| supports_features(version, &entry.selection)) - { - reached.extend(active_dependencies(version, &entry.selection)?); + let selection = dependency.feature_selection(); + for (compatibility, _) in matching_lines(versions, &dependency.requirement) { + // Only what this dependency could settle on: a version missing a + // feature it asks for is not one it can select, even though another + // dependency on the same line may select it. + let selectable = matching_versions(versions, &dependency.requirement) + .filter(|version| compatibility_line(&version.version) == compatibility) + .filter(|version| supports_features(version, &selection)) + .map(|version| version.version.clone()) + .collect::>(); + let package = PackageKey::Registry { name: dependency.name.clone(), compatibility }; + let entry = discovered.entry(package).or_default(); + let previous = entry.selection.clone(); + entry.selection.default_features |= dependency.default_features; + entry.selection.features.extend(dependency.features.iter().cloned()); + let unwalked = &selectable - &entry.versions; + entry.versions.extend(selectable); + let walk = if entry.selection == previous { unwalked } else { entry.versions.clone() }; + // The features of every dependency reaching the line, because one + // may turn on a weak feature of another's. A version that has none + // of them simply activates nothing extra. + for version in versions + .iter() + .filter(|version| walk.contains(&version.version)) + { + reached.extend(active_dependencies(version, &entry.selection)?); + } } Ok(reached) } @@ -150,7 +157,8 @@ fn validate_selected_graph( let mut pending = VecDeque::from(root_dependencies.to_vec()); while let Some(dependency) = pending.pop_front() { - let package = package_key(registry, &dependency)?; + let package = validated_package(registry, &dependency, solution, &mut validated)?; + let Some(package) = package else { return Ok(None) }; let Some(selected_version) = solution.get(&package) else { return Ok(None) }; if !dependency.requirement.matches(selected_version) { return Ok(None); @@ -158,17 +166,7 @@ fn validate_selected_graph( if validated.contains_key(&package) { continue; } - let selected = registry - .package(&dependency.name)? - .iter() - .find(|candidate| !candidate.yanked && candidate.version == *selected_version) - .ok_or_else(|| { - miette::miette!( - "selected {} {} is absent from the index", - dependency.name, - selected_version, - ) - })?; + let selected = offered_version(registry, &dependency.name, selected_version)?; let selection = feature_selections .get(&package) .cloned() @@ -183,6 +181,46 @@ fn validate_selected_graph( Ok(Some(validated.into_iter().collect())) } +/// The line package `dependency` settled on. +/// +/// A requirement spanning several compatibility lines resolves through its +/// choice package, whose entry is recorded in `validated` so the lockfile +/// can read the chosen line back out. +/// The index entry for a version the solver selected. Only versions the +/// index still offers are registered, so a yanked one means the index the +/// solution was built against is not the one being read. +fn offered_version<'v>( + registry: &'v Registry, + name: &str, + version: &Version, +) -> Result<&'v RegistryVersion> { + let offered = indexed_version(registry.package(name)?, name, version)?; + if offered.yanked { + return Err(miette::miette!("selected {name} {version} is yanked")); + } + Ok(offered) +} + +fn validated_package( + registry: &Registry, + dependency: &RegistryDependency, + solution: &pubgrub::SelectedDependencies, + validated: &mut BTreeMap, +) -> Result> { + registry.validate_dependency_source(dependency.registry.as_deref())?; + Ok(match package_key(registry, dependency)? { + line @ PackageKey::Registry { .. } => Some(line), + choice @ PackageKey::Requirement { .. } => solution + .get(&choice) + .map(|representative| { + let line = chosen_line(&dependency.name, representative); + validated.insert(choice.clone(), representative.clone()); + line + }), + PackageKey::Root | PackageKey::Unsatisfiable { .. } => None, + }) +} + fn resolve_with_features( registry: &Registry, root_dependencies: &[RegistryDependency], @@ -198,11 +236,7 @@ fn resolve_with_features( if !registered.insert(package.clone()) { continue; } - let selection = feature_selections - .get(&package) - .cloned() - .unwrap_or_default(); - register_candidates(registry, &package, &selection, &mut provider, &mut pending)?; + register(registry, &package, feature_selections, &mut provider, &mut pending)?; } match resolve(&provider, PackageKey::Root, Version::new(0, 0, 0)) { @@ -219,18 +253,42 @@ fn resolve_with_features( } } -/// Offer the solver every version of `package` that the selected features -/// admit, queueing each one's own dependencies. +/// Tell the solver what `package` offers, which depends on what kind of +/// package it is. +fn register( + registry: &Registry, + package: &PackageKey, + feature_selections: &BTreeMap, + provider: &mut OfflineDependencyProvider>, + pending: &mut VecDeque, +) -> Result<()> { + match package { + PackageKey::Registry { .. } => { + register_candidates(registry, package, feature_selections, provider, pending) + } + PackageKey::Requirement { .. } => { + register_compatibility_lines(registry, package, provider, pending) + } + PackageKey::Root | PackageKey::Unsatisfiable { .. } => Ok(()), + } +} + +/// Offer the solver the versions of `package` that every dependency able to +/// settle on it can support, queueing each one's own dependencies. fn register_candidates( registry: &Registry, package: &PackageKey, - selection: &FeatureSelection, + feature_selections: &BTreeMap, provider: &mut OfflineDependencyProvider>, pending: &mut VecDeque, ) -> Result<()> { let PackageKey::Registry { name, compatibility } = package else { return Ok(()); }; + let resolved = feature_selections + .get(package) + .cloned() + .unwrap_or_default(); let versions = registry.package(name)?; let candidates = versions .iter() @@ -238,16 +296,62 @@ fn register_candidates( !version.yanked && compatibility_line(&version.version) == *compatibility }); for version in candidates { - if !supports_features(version, selection) { - continue; - } - let dependencies = active_dependencies(version, selection)?; + let dependencies = active_dependencies(version, &resolved)?; let constraints = constraints_for(registry, &dependencies, pending)?; provider.add_dependencies(package.clone(), version.version.clone(), constraints); } Ok(()) } +/// Offer the solver one version per compatibility line the requirement is +/// met on, each standing for that line and depending on the versions it +/// admits there. Ordered by version, so the solver reaches for the newest +/// line first and backtracks to an older one, as `cargo` does. +/// +/// A line is offered only the versions that support what the lines's +/// dependants ask, so a line that cannot support them is not a choice at +/// all rather than one the solver takes and later has to leave. +fn register_compatibility_lines( + registry: &Registry, + package: &PackageKey, + provider: &mut OfflineDependencyProvider>, + pending: &mut VecDeque, +) -> Result<()> { + let PackageKey::Requirement { + name, + requirement, + default_features, + features, + } = package + else { + return Ok(()); + }; + let requested = FeatureSelection { + default_features: *default_features, + features: features.iter().cloned().collect(), + }; + let requirement = VersionReq::parse(requirement) + .into_diagnostic() + .wrap_err_with(|| format!("parse requirement for {name}"))?; + let versions = registry.package(name)?; + for (compatibility, representative) in matching_lines(versions, &requirement) { + let line = + PackageKey::Registry { name: name.clone(), compatibility: compatibility.clone() }; + let admitted = matching_versions(versions, &requirement) + .filter(|version| compatibility_line(&version.version) == compatibility) + .filter(|version| supports_features(version, &requested)) + .fold(Ranges::empty(), |range, version| { + range.union(&Ranges::singleton(version.version.clone())) + }); + if admitted == Ranges::empty() { + continue; + } + provider.add_dependencies(package.clone(), representative, [(line.clone(), admitted)]); + pending.push_back(line); + } + Ok(()) +} + fn constraints_for( registry: &Registry, dependencies: &[RegistryDependency], @@ -255,10 +359,16 @@ fn constraints_for( ) -> Result)>> { let mut constraints = BTreeMap::>::new(); for dependency in dependencies { + registry.validate_dependency_source(dependency.registry.as_deref())?; let package = package_key(registry, dependency)?; + // A version missing a feature this dependency asks for is not one it + // can settle on, which is how a requirement reaches past a line to an + // older one. + let selection = dependency.feature_selection(); let allowed = if let PackageKey::Registry { compatibility, .. } = &package { matching_versions(registry.package(&dependency.name)?, &dependency.requirement) .filter(|version| compatibility_line(&version.version) == *compatibility) + .filter(|version| supports_features(version, &selection)) .fold(Ranges::empty(), |range, version| { range.union(&Ranges::singleton(version.version.clone())) }) @@ -273,25 +383,3 @@ fn constraints_for( } Ok(constraints.into_iter().collect()) } - -/// The solver package `dependency` resolves against. -/// -/// A dependency the index cannot meet still gets a package of its own, one -/// the solver finds no version under. That keeps a dead-end candidate a -/// plain incompatibility the solver backtracks over and can name in its -/// report, rather than a failure of the whole resolution. -fn package_key(registry: &Registry, dependency: &RegistryDependency) -> Result { - registry.validate_dependency_source(dependency.registry.as_deref())?; - let compatibility = registry - .versions(&dependency.name) - .and_then(|versions| newest_compatibility(versions, &dependency.requirement)); - Ok(match compatibility { - Some(compatibility) => { - PackageKey::Registry { name: dependency.name.clone(), compatibility } - } - None => PackageKey::Unsatisfiable { - name: dependency.name.clone(), - requirement: dependency.requirement.to_string(), - }, - }) -} diff --git a/pnpm/crates/cargo-resolver/src/tests.rs b/pnpm/crates/cargo-resolver/src/tests.rs index 4575fb98fb..87cefe763d 100644 --- a/pnpm/crates/cargo-resolver/src/tests.rs +++ b/pnpm/crates/cargo-resolver/src/tests.rs @@ -518,6 +518,26 @@ fn rejects_a_dependency_from_a_third_party_registry() { assert!(error.contains("other.example.test"), "{error}"); } +/// A workspace asking for `foo` across two compatibility lines. +const SPANNING_METADATA: &str = r#"{ + "packages": [{ + "id": "path+file:///workspace#app@0.1.0", + "name": "app", + "version": "0.1.0", + "dependencies": [{ + "name": "foo", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "req": ">=1, <3" + }] + }], + "workspace_members": ["path+file:///workspace#app@0.1.0"] +}"#; + +/// `foo` 2.0.0 needs a `bar` that does not exist, so only `foo` 1.0.0 and +/// the `bar ^2` it needs can resolve. +const SPANNING_FOO_INDEX: &str = r#"{"name":"foo","vers":"1.0.0","deps":[{"name":"bar","req":"^2","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":null}],"cksum":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","features":{},"yanked":false} +{"name":"foo","vers":"2.0.0","deps":[{"name":"bar","req":"^9","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":null}],"cksum":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","features":{},"yanked":false}"#; + /// `bar` 2.0.0 resolves, while the whole 3 compatibility line is yanked. const PARTLY_YANKED_BAR_INDEX: &str = r#"{"name":"bar","vers":"2.0.0","deps":[],"cksum":"cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc","features":{},"yanked":false} {"name":"bar","vers":"3.0.0","deps":[],"cksum":"dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd","features":{},"yanked":true}"#; @@ -706,30 +726,30 @@ fn discovers_a_crate_only_unified_features_activate() { ); } -/// The package key holds one compatibility line, so a version outside it -/// can never be selected and the crates only it needs are not fetched. #[test] -fn leaves_a_crate_only_an_unselectable_version_needs_unfetched() { - const METADATA: &str = r#"{ - "packages": [{ - "id": "path+file:///workspace#app@0.1.0", - "name": "app", - "version": "0.1.0", - "dependencies": [{ - "name": "foo", - "source": "registry+https://github.com/rust-lang/crates.io-index", - "req": ">=0.9" - }] - }], - "workspace_members": ["path+file:///workspace#app@0.1.0"] -}"#; - let foo_index = r#"{"name":"foo","vers":"0.9.0","deps":[{"name":"legacy","req":"^1","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":null}],"cksum":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","features":{},"yanked":false} -{"name":"foo","vers":"1.0.0","deps":[],"cksum":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","features":{},"yanked":false}"#; +fn fetches_what_every_admissible_line_needs() { + // Each line needs a crate of its own, so walking only the newest would + // leave the other unfetched. + let foo_index = r#"{"name":"foo","vers":"1.0.0","deps":[{"name":"legacy","req":"^1","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":null}],"cksum":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","features":{},"yanked":false} +{"name":"foo","vers":"2.0.0","deps":[{"name":"modern","req":"^1","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":null}],"cksum":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","features":{},"yanked":false}"#; let files = BTreeMap::from([("foo".to_string(), foo_index.to_string())]); - assert!(missing_index_names(METADATA, &files, CRATES_IO_SOURCE).unwrap().is_empty()); + assert_eq!( + missing_index_names(SPANNING_METADATA, &files, CRATES_IO_SOURCE).unwrap(), + ["legacy", "modern"], + ); +} + +#[test] +fn backtracks_to_an_older_compatibility_line() { + let files = BTreeMap::from([ + ("bar".to_string(), BAR_INDEX.to_string()), + ("foo".to_string(), SPANNING_FOO_INDEX.to_string()), + ]); + let lockfile = - Lockfile::from_str(&resolve_lockfile(METADATA, &files, CRATES_IO_SOURCE).unwrap()).unwrap(); + Lockfile::from_str(&resolve_lockfile(SPANNING_METADATA, &files, CRATES_IO_SOURCE).unwrap()) + .unwrap(); dbg!(&lockfile.packages); assert!( @@ -739,4 +759,277 @@ fn leaves_a_crate_only_an_unselectable_version_needs_unfetched() { package.name.as_str() == "foo" && package.version == semver::Version::new(1, 0, 0) }), ); + assert!( + lockfile.packages + .iter() + .any(|package| { + package.name.as_str() == "bar" && package.version == semver::Version::new(2, 0, 0) + }), + ); +} + +#[test] +fn prefers_the_newest_compatibility_line_that_resolves() { + let foo_index = SPANNING_FOO_INDEX.replace(r#""req":"^9""#, r#""req":"^2""#); + let files = BTreeMap::from([ + ("bar".to_string(), BAR_INDEX.to_string()), + ("foo".to_string(), foo_index), + ]); + + let lockfile = + Lockfile::from_str(&resolve_lockfile(SPANNING_METADATA, &files, CRATES_IO_SOURCE).unwrap()) + .unwrap(); + + dbg!(&lockfile.packages); + assert!( + lockfile.packages + .iter() + .any(|package| { + package.name.as_str() == "foo" && package.version == semver::Version::new(2, 0, 0) + }), + ); +} + +/// Both lines of `foo` carry an `extra` feature, so only the selection each +/// line was actually asked for decides whether `baz` is activated there. +#[test] +fn keeps_requested_features_on_the_line_that_asked_for_them() { + const METADATA: &str = r#"{ + "packages": [ + { + "id": "path+file:///workspace#wide@0.1.0", + "name": "wide", + "version": "0.1.0", + "dependencies": [{ + "name": "foo", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "req": ">=0.9", + "features": ["extra"] + }] + }, + { + "id": "path+file:///workspace#narrow@0.1.0", + "name": "narrow", + "version": "0.1.0", + "dependencies": [{ + "name": "foo", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "req": "^0.9" + }] + } + ], + "workspace_members": [ + "path+file:///workspace#wide@0.1.0", + "path+file:///workspace#narrow@0.1.0" + ] +}"#; + let foo_index = r#"{"name":"foo","vers":"0.9.0","deps":[{"name":"baz","req":"^1","features":[],"optional":true,"default_features":true,"target":null,"kind":"normal","registry":null}],"cksum":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","features":{"extra":["dep:baz"]},"yanked":false} +{"name":"foo","vers":"1.0.0","deps":[{"name":"baz","req":"^1","features":[],"optional":true,"default_features":true,"target":null,"kind":"normal","registry":null}],"cksum":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","features":{"extra":["dep:baz"]},"yanked":false}"#; + let files = BTreeMap::from([ + ("baz".to_string(), BAZ_INDEX.to_string()), + ("foo".to_string(), foo_index.to_string()), + ]); + + let lockfile = + Lockfile::from_str(&resolve_lockfile(METADATA, &files, CRATES_IO_SOURCE).unwrap()).unwrap(); + + dbg!(&lockfile.packages); + let dependencies = |version: semver::Version| { + lockfile.packages + .iter() + .find(|package| package.name.as_str() == "foo" && package.version == version) + .map(|package| { + package.dependencies + .iter() + .map(|dependency| dependency.name.to_string()) + .collect::>() + }) + }; + assert_eq!(dependencies(semver::Version::new(1, 0, 0)), Some(vec!["baz".to_string()])); + assert_eq!(dependencies(semver::Version::new(0, 9, 0)), Some(Vec::new())); +} + +/// The `rand` example from the Cargo book: a requirement spanning two lines +/// and one pinned to the older line do not unify, and `cargo` builds both. +#[test] +fn keeps_two_compatibility_lines_of_one_crate_apart() { + const METADATA: &str = r#"{ + "packages": [ + { + "id": "path+file:///workspace#wide@0.1.0", + "name": "wide", + "version": "0.1.0", + "dependencies": [{ + "name": "rand", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "req": ">=0.6, <0.8.0" + }] + }, + { + "id": "path+file:///workspace#narrow@0.1.0", + "name": "narrow", + "version": "0.1.0", + "dependencies": [{ + "name": "rand", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "req": "^0.6" + }] + } + ], + "workspace_members": [ + "path+file:///workspace#wide@0.1.0", + "path+file:///workspace#narrow@0.1.0" + ] +}"#; + let rand_index = r#"{"name":"rand","vers":"0.6.5","deps":[],"cksum":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","features":{},"yanked":false} +{"name":"rand","vers":"0.7.3","deps":[],"cksum":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","features":{},"yanked":false}"#; + let files = BTreeMap::from([("rand".to_string(), rand_index.to_string())]); + + let lockfile = + Lockfile::from_str(&resolve_lockfile(METADATA, &files, CRATES_IO_SOURCE).unwrap()).unwrap(); + + dbg!(&lockfile.packages); + let selected = lockfile.packages + .iter() + .filter(|package| package.name.as_str() == "rand") + .map(|package| package.version.to_string()) + .collect::>(); + assert_eq!(selected, ["0.6.5", "0.7.3"]); +} + +/// Only the older line carries `extra`, so the newer one is not a choice +/// this requirement can take, and `baz` comes with the line that is. +#[test] +fn selects_the_older_line_when_the_newer_lacks_a_requested_feature() { + const METADATA: &str = r#"{ + "packages": [{ + "id": "path+file:///workspace#app@0.1.0", + "name": "app", + "version": "0.1.0", + "dependencies": [{ + "name": "foo", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "req": ">=0.9", + "features": ["extra"] + }] + }], + "workspace_members": ["path+file:///workspace#app@0.1.0"] +}"#; + let foo_index = r#"{"name":"foo","vers":"0.9.0","deps":[{"name":"baz","req":"^1","features":[],"optional":true,"default_features":true,"target":null,"kind":"normal","registry":null}],"cksum":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","features":{"extra":["dep:baz"]},"yanked":false} +{"name":"foo","vers":"1.0.0","deps":[],"cksum":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","features":{},"yanked":false}"#; + let files = BTreeMap::from([ + ("baz".to_string(), BAZ_INDEX.to_string()), + ("foo".to_string(), foo_index.to_string()), + ]); + + let lockfile = + Lockfile::from_str(&resolve_lockfile(METADATA, &files, CRATES_IO_SOURCE).unwrap()).unwrap(); + + dbg!(&lockfile.packages); + assert!( + lockfile.packages + .iter() + .any(|package| { + package.name.as_str() == "foo" && package.version == semver::Version::new(0, 9, 0) + }), + ); + assert!( + lockfile.packages + .iter() + .any(|package| package.name.as_str() == "baz"), + ); +} + +/// No line supports both features, so one shared choice would rule both +/// lines out. +#[test] +fn lets_requirements_asking_for_different_features_take_different_lines() { + const METADATA: &str = r#"{ + "packages": [ + { + "id": "path+file:///workspace#reads@0.1.0", + "name": "reads", + "version": "0.1.0", + "dependencies": [{ + "name": "foo", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "req": ">=1, <3", + "features": ["read"] + }] + }, + { + "id": "path+file:///workspace#writes@0.1.0", + "name": "writes", + "version": "0.1.0", + "dependencies": [{ + "name": "foo", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "req": ">=1, <3", + "features": ["write"] + }] + } + ], + "workspace_members": [ + "path+file:///workspace#reads@0.1.0", + "path+file:///workspace#writes@0.1.0" + ] +}"#; + let foo_index = r#"{"name":"foo","vers":"1.0.0","deps":[],"cksum":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","features":{"read":[]},"yanked":false} +{"name":"foo","vers":"2.0.0","deps":[],"cksum":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","features":{"write":[]},"yanked":false}"#; + let files = BTreeMap::from([("foo".to_string(), foo_index.to_string())]); + + let lockfile = + Lockfile::from_str(&resolve_lockfile(METADATA, &files, CRATES_IO_SOURCE).unwrap()).unwrap(); + + dbg!(&lockfile.packages); + let selected = lockfile.packages + .iter() + .filter(|package| package.name.as_str() == "foo") + .map(|package| package.version.to_string()) + .collect::>(); + assert_eq!(selected, ["1.0.0", "2.0.0"]); +} + +/// A line walked only under the features of every requirement reaching it +/// would reach neither crate. +#[test] +fn fetches_what_each_requirement_activates_on_its_own_line() { + const METADATA: &str = r#"{ + "packages": [ + { + "id": "path+file:///workspace#reads@0.1.0", + "name": "reads", + "version": "0.1.0", + "dependencies": [{ + "name": "foo", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "req": ">=1, <3", + "features": ["read"] + }] + }, + { + "id": "path+file:///workspace#writes@0.1.0", + "name": "writes", + "version": "0.1.0", + "dependencies": [{ + "name": "foo", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "req": ">=1, <3", + "features": ["write"] + }] + } + ], + "workspace_members": [ + "path+file:///workspace#reads@0.1.0", + "path+file:///workspace#writes@0.1.0" + ] +}"#; + let foo_index = r#"{"name":"foo","vers":"1.0.0","deps":[{"name":"reader","req":"^1","features":[],"optional":true,"default_features":true,"target":null,"kind":"normal","registry":null}],"cksum":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","features":{"read":["dep:reader"]},"yanked":false} +{"name":"foo","vers":"2.0.0","deps":[{"name":"writer","req":"^1","features":[],"optional":true,"default_features":true,"target":null,"kind":"normal","registry":null}],"cksum":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","features":{"write":["dep:writer"]},"yanked":false}"#; + let files = BTreeMap::from([("foo".to_string(), foo_index.to_string())]); + + assert_eq!( + missing_index_names(METADATA, &files, CRATES_IO_SOURCE).unwrap(), + ["reader", "writer"], + ); }