From 4d7913bfe3c08bcbe1980cfcac2434c027fd504c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 5 Dec 2024 10:03:27 -0600 Subject: [PATCH 01/18] refactor(source): Qualify what alternatives we look for --- crates/resolver-tests/src/lib.rs | 2 +- src/cargo/core/resolver/errors.rs | 2 +- src/cargo/sources/directory.rs | 2 +- src/cargo/sources/path.rs | 4 ++-- src/cargo/sources/registry/mod.rs | 6 +++--- src/cargo/sources/source.rs | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index 0fda8cf62c9..d77cbda97f2 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -143,7 +143,7 @@ pub fn resolve_with_global_context_raw( for summary in self.list.iter() { let matched = match kind { QueryKind::Exact => dep.matches(summary), - QueryKind::Alternatives => true, + QueryKind::AlternativeNames => true, QueryKind::Normalized => true, }; if matched { diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index 3c5504bdf39..88c44a93dfd 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -304,7 +304,7 @@ pub(super) fn activation_error( // Maybe the user mistyped the name? Like `dep-thing` when `Dep_Thing` // was meant. So we try asking the registry for a `fuzzy` search for suggestions. let candidates = loop { - match registry.query_vec(&new_dep, QueryKind::Alternatives) { + match registry.query_vec(&new_dep, QueryKind::AlternativeNames) { Poll::Ready(Ok(candidates)) => break candidates, Poll::Ready(Err(e)) => return to_resolve_err(e), Poll::Pending => match registry.block_until_ready() { diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index 6fff8ed2763..166624b8dcd 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -109,7 +109,7 @@ impl<'gctx> Source for DirectorySource<'gctx> { let packages = self.packages.values().map(|p| &p.0); let matches = packages.filter(|pkg| match kind { QueryKind::Exact => dep.matches(pkg.summary()), - QueryKind::Alternatives => true, + QueryKind::AlternativeNames => true, QueryKind::Normalized => dep.matches(pkg.summary()), }); for summary in matches.map(|pkg| pkg.summary().clone()) { diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 829f63ce103..e9dc0a0fbbf 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -146,7 +146,7 @@ impl<'gctx> Source for PathSource<'gctx> { if let Some(s) = self.package.as_ref().map(|p| p.summary()) { let matched = match kind { QueryKind::Exact => dep.matches(s), - QueryKind::Alternatives => true, + QueryKind::AlternativeNames => true, QueryKind::Normalized => dep.matches(s), }; if matched { @@ -333,7 +333,7 @@ impl<'gctx> Source for RecursivePathSource<'gctx> { { let matched = match kind { QueryKind::Exact => dep.matches(s), - QueryKind::Alternatives => true, + QueryKind::AlternativeNames => true, QueryKind::Normalized => dep.matches(s), }; if matched { diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index ed36c31f331..5bb5b2f4ebd 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -804,7 +804,7 @@ impl<'gctx> Source for RegistrySource<'gctx> { dep.matches(s.as_summary()) } } - QueryKind::Alternatives => true, + QueryKind::AlternativeNames => true, QueryKind::Normalized => true, }; if !matched { @@ -839,7 +839,7 @@ impl<'gctx> Source for RegistrySource<'gctx> { return Poll::Ready(Ok(())); } let mut any_pending = false; - if kind == QueryKind::Alternatives || kind == QueryKind::Normalized { + if kind == QueryKind::AlternativeNames || kind == QueryKind::Normalized { // Attempt to handle misspellings by searching for a chain of related // names to the original name. The resolver will later // reject any candidates that have the wrong name, and with this it'll @@ -859,7 +859,7 @@ impl<'gctx> Source for RegistrySource<'gctx> { .query_inner(name_permutation, &req, &mut *self.ops, &mut |s| { if !s.is_yanked() { f(s); - } else if kind == QueryKind::Alternatives { + } else if kind == QueryKind::AlternativeNames { f(s); } })? diff --git a/src/cargo/sources/source.rs b/src/cargo/sources/source.rs index 377ac385e11..bdf6fb6870a 100644 --- a/src/cargo/sources/source.rs +++ b/src/cargo/sources/source.rs @@ -185,7 +185,7 @@ pub enum QueryKind { /// Path/Git sources may return all dependencies that are at that URI, /// whereas an `Registry` source may return dependencies that have the same /// canonicalization. - Alternatives, + AlternativeNames, /// Match a dependency in all ways and will normalize the package name. /// Each source defines what normalizing means. Normalized, From 31a884ef9e5d48899f2c1c4edf3f5825b70a69eb Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 5 Dec 2024 10:09:27 -0600 Subject: [PATCH 02/18] refactor(resolver): Qualify what kind of alt candidates we're discussing --- src/cargo/core/resolver/errors.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index 88c44a93dfd..5ee38916842 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -303,7 +303,7 @@ pub(super) fn activation_error( } else { // Maybe the user mistyped the name? Like `dep-thing` when `Dep_Thing` // was meant. So we try asking the registry for a `fuzzy` search for suggestions. - let candidates = loop { + let name_candidates = loop { match registry.query_vec(&new_dep, QueryKind::AlternativeNames) { Poll::Ready(Ok(candidates)) => break candidates, Poll::Ready(Err(e)) => return to_resolve_err(e), @@ -314,30 +314,33 @@ pub(super) fn activation_error( } }; - let mut candidates: Vec<_> = candidates.into_iter().map(|s| s.into_summary()).collect(); + let mut name_candidates: Vec<_> = name_candidates + .into_iter() + .map(|s| s.into_summary()) + .collect(); - candidates.sort_unstable_by_key(|a| a.name()); - candidates.dedup_by(|a, b| a.name() == b.name()); - let mut candidates: Vec<_> = candidates + name_candidates.sort_unstable_by_key(|a| a.name()); + name_candidates.dedup_by(|a, b| a.name() == b.name()); + let mut name_candidates: Vec<_> = name_candidates .iter() .filter_map(|n| Some((edit_distance(&*new_dep.package_name(), &*n.name(), 3)?, n))) .collect(); - candidates.sort_by_key(|o| o.0); + name_candidates.sort_by_key(|o| o.0); let mut msg: String; - if candidates.is_empty() { + if name_candidates.is_empty() { msg = format!("no matching package named `{}` found\n", dep.package_name()); } else { msg = format!( "no matching package found\nsearched package name: `{}`\n", dep.package_name() ); - let mut names = candidates + let mut names = name_candidates .iter() .take(3) .map(|c| c.1.name().as_str()) .collect::>(); - if candidates.len() > 3 { + if name_candidates.len() > 3 { names.push("..."); } // Vertically align first suggestion with missing crate name @@ -347,7 +350,7 @@ pub(super) fn activation_error( String::default(), |acc, (i, el)| match i { 0 => acc + el, - i if names.len() - 1 == i && candidates.len() <= 3 => acc + " or " + el, + i if names.len() - 1 == i && name_candidates.len() <= 3 => acc + " or " + el, _ => acc + ", " + el, }, )); From eb0daec9915d029fd52cb77f629a32588286eeb3 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 5 Dec 2024 10:10:38 -0600 Subject: [PATCH 03/18] refactor(resolver): Group related alternatives logic --- src/cargo/core/resolver/errors.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index 5ee38916842..8e55f1662b5 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -313,12 +313,10 @@ pub(super) fn activation_error( }, } }; - let mut name_candidates: Vec<_> = name_candidates .into_iter() .map(|s| s.into_summary()) .collect(); - name_candidates.sort_unstable_by_key(|a| a.name()); name_candidates.dedup_by(|a, b| a.name() == b.name()); let mut name_candidates: Vec<_> = name_candidates @@ -326,6 +324,7 @@ pub(super) fn activation_error( .filter_map(|n| Some((edit_distance(&*new_dep.package_name(), &*n.name(), 3)?, n))) .collect(); name_candidates.sort_by_key(|o| o.0); + let mut msg: String; if name_candidates.is_empty() { msg = format!("no matching package named `{}` found\n", dep.package_name()); From efe17de8a1748f42477b2fb421bc1959437f3a1d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 5 Dec 2024 10:10:17 -0600 Subject: [PATCH 04/18] refactor(resolver): Make no alternatives the fall-through --- src/cargo/core/resolver/errors.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index 8e55f1662b5..cd73db4cc79 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -326,9 +326,7 @@ pub(super) fn activation_error( name_candidates.sort_by_key(|o| o.0); let mut msg: String; - if name_candidates.is_empty() { - msg = format!("no matching package named `{}` found\n", dep.package_name()); - } else { + if !name_candidates.is_empty() { msg = format!( "no matching package found\nsearched package name: `{}`\n", dep.package_name() @@ -354,6 +352,8 @@ pub(super) fn activation_error( }, )); msg.push('\n'); + } else { + msg = format!("no matching package named `{}` found\n", dep.package_name()); } let mut location_searched_msg = registry.describe_source(dep.source_id()); From 271df34996edb103f679668bf6113b694bd2dfcb Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 5 Dec 2024 10:32:41 -0600 Subject: [PATCH 05/18] refactor(resolver): Switch some code from string construction to writing --- src/cargo/core/resolver/errors.rs | 40 ++++++++++++++++--------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index cd73db4cc79..946378ccb60 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -1,4 +1,5 @@ use std::fmt; +use std::fmt::Write as _; use std::task::Poll; use crate::core::{Dependency, PackageId, Registry, Summary}; @@ -325,12 +326,10 @@ pub(super) fn activation_error( .collect(); name_candidates.sort_by_key(|o| o.0); - let mut msg: String; + let mut msg = String::new(); if !name_candidates.is_empty() { - msg = format!( - "no matching package found\nsearched package name: `{}`\n", - dep.package_name() - ); + let _ = writeln!(&mut msg, "no matching package found",); + let _ = writeln!(&mut msg, "searched package name: `{}`", dep.package_name()); let mut names = name_candidates .iter() .take(3) @@ -342,18 +341,21 @@ pub(super) fn activation_error( } // Vertically align first suggestion with missing crate name // so a typo jumps out at you. - msg.push_str("perhaps you meant: "); - msg.push_str(&names.iter().enumerate().fold( - String::default(), - |acc, (i, el)| match i { + let suggestions = names + .iter() + .enumerate() + .fold(String::default(), |acc, (i, el)| match i { 0 => acc + el, i if names.len() - 1 == i && name_candidates.len() <= 3 => acc + " or " + el, _ => acc + ", " + el, - }, - )); - msg.push('\n'); + }); + let _ = writeln!(&mut msg, "perhaps you meant: {suggestions}"); } else { - msg = format!("no matching package named `{}` found\n", dep.package_name()); + let _ = writeln!( + &mut msg, + "no matching package named `{}` found", + dep.package_name() + ); } let mut location_searched_msg = registry.describe_source(dep.source_id()); @@ -361,12 +363,12 @@ pub(super) fn activation_error( location_searched_msg = format!("{}", dep.source_id()); } - msg.push_str(&format!("location searched: {}\n", location_searched_msg)); - msg.push_str("required by "); - msg.push_str(&describe_path_in_context( - resolver_ctx, - &parent.package_id(), - )); + let _ = writeln!(&mut msg, "location searched: {}", location_searched_msg); + let _ = write!( + &mut msg, + "required by {}", + describe_path_in_context(resolver_ctx, &parent.package_id()), + ); msg }; From 0160cb7537e8d752e315e82f9293599fcc67a92f Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 5 Dec 2024 10:48:43 -0600 Subject: [PATCH 06/18] refactor(registry): Move IndexSummary filtering from Index to RegistrySource --- src/cargo/sources/registry/index/mod.rs | 18 ++------------ src/cargo/sources/registry/mod.rs | 32 ++++++++++++++++++------- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/src/cargo/sources/registry/index/mod.rs b/src/cargo/sources/registry/index/mod.rs index 0bcd2009c6f..b2a23139b41 100644 --- a/src/cargo/sources/registry/index/mod.rs +++ b/src/cargo/sources/registry/index/mod.rs @@ -37,7 +37,7 @@ use std::collections::HashMap; use std::path::Path; use std::str; use std::task::{ready, Poll}; -use tracing::{debug, info}; +use tracing::info; mod cache; use self::cache::CacheManager; @@ -370,21 +370,7 @@ impl<'gctx> RegistryIndex<'gctx> { .filter_map(move |(k, v)| if req.matches(k) { Some(v) } else { None }) .filter_map(move |maybe| { match maybe.parse(raw_data, source_id, bindeps) { - Ok(sum @ IndexSummary::Candidate(_) | sum @ IndexSummary::Yanked(_)) => { - Some(sum) - } - Ok(IndexSummary::Unsupported(summary, v)) => { - debug!( - "unsupported schema version {} ({} {})", - v, - summary.name(), - summary.version() - ); - None - } - Ok(IndexSummary::Offline(_)) => { - unreachable!("We do not check for off-line until later") - } + Ok(sum) => Some(sum), Err(e) => { info!("failed to parse `{}` registry package: {}", name, e); None diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 5bb5b2f4ebd..6242c1e26ec 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -779,7 +779,9 @@ impl<'gctx> Source for RegistrySource<'gctx> { ready!(self .index .query_inner(dep.package_name(), &req, &mut *self.ops, &mut |s| { - if dep.matches(s.as_summary()) { + if matches!(s, IndexSummary::Candidate(_) | IndexSummary::Yanked(_)) + && dep.matches(s.as_summary()) + { // We are looking for a package from a lock file so we do not care about yank callback(s) } @@ -813,13 +815,27 @@ impl<'gctx> Source for RegistrySource<'gctx> { // Next filter out all yanked packages. Some yanked packages may // leak through if they're in a whitelist (aka if they were // previously in `Cargo.lock` - if !s.is_yanked() { - callback(s); - } else if self.yanked_whitelist.contains(&s.package_id()) { - callback(s); - } else if req.is_precise() { - precise_yanked_in_use = true; - callback(s); + match s { + s @ IndexSummary::Candidate(_) => callback(s), + s @ IndexSummary::Yanked(_) => { + if self.yanked_whitelist.contains(&s.package_id()) { + callback(s); + } else if req.is_precise() { + precise_yanked_in_use = true; + callback(s); + } + } + IndexSummary::Unsupported(summary, v) => { + tracing::debug!( + "unsupported schema version {} ({} {})", + v, + summary.name(), + summary.version() + ); + } + IndexSummary::Offline(summary) => { + tracing::debug!("offline ({} {})", summary.name(), summary.version()); + } } }))?; if precise_yanked_in_use { From e48a00825a2e04aa7f7b77760e7a5ebc1863f2bc Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 5 Dec 2024 11:03:21 -0600 Subject: [PATCH 07/18] refactor(registry): Pass along original IndexSummary state for deps --- src/cargo/core/registry.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 37af3d7a130..50010772a84 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -733,8 +733,8 @@ impl<'gctx> Registry for PackageRegistry<'gctx> { return; } } - let summary = summary.into_summary(); - f(IndexSummary::Candidate(lock(locked, all_patches, summary))) + let summary = summary.map_summary(|summary| lock(locked, all_patches, summary)); + f(summary) }; return source.query(dep, kind, callback); } From 015e9802dacc20a2b187e66493d7f38993602b51 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 5 Dec 2024 11:03:47 -0600 Subject: [PATCH 08/18] refactor(registry): Pass along original IndexSummary state for overrides --- src/cargo/core/registry.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 50010772a84..52cb1b0cc38 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -674,9 +674,10 @@ impl<'gctx> Registry for PackageRegistry<'gctx> { let patch = patches.remove(0); match override_summary { Some(override_summary) => { - let override_summary = override_summary.into_summary(); - self.warn_bad_override(&override_summary, &patch)?; - f(IndexSummary::Candidate(self.lock(override_summary))); + self.warn_bad_override(override_summary.as_summary(), &patch)?; + let override_summary = + override_summary.map_summary(|summary| self.lock(summary)); + f(override_summary); } None => f(IndexSummary::Candidate(patch)), } @@ -760,11 +761,11 @@ impl<'gctx> Registry for PackageRegistry<'gctx> { "found an override with a non-locked list" ))); } - let override_summary = override_summary.into_summary(); if let Some(to_warn) = to_warn { - self.warn_bad_override(&override_summary, to_warn.as_summary())?; + self.warn_bad_override(override_summary.as_summary(), to_warn.as_summary())?; } - f(IndexSummary::Candidate(self.lock(override_summary))); + let override_summary = override_summary.map_summary(|summary| self.lock(summary)); + f(override_summary); } } From d2fcf6b13f13e5b3aa53ca5581e1e140b15efad1 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 5 Dec 2024 11:30:56 -0600 Subject: [PATCH 09/18] fix(resolver): Report unmatched versions, rather than saying no package --- crates/resolver-tests/src/lib.rs | 1 + src/cargo/core/resolver/errors.rs | 46 ++++++++++++++++++++++++++++++- src/cargo/sources/directory.rs | 2 +- src/cargo/sources/path.rs | 4 +-- src/cargo/sources/registry/mod.rs | 3 +- src/cargo/sources/source.rs | 7 +++++ tests/testsuite/registry.rs | 15 ++++++---- 7 files changed, 68 insertions(+), 10 deletions(-) diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index d77cbda97f2..f34dd737650 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -143,6 +143,7 @@ pub fn resolve_with_global_context_raw( for summary in self.list.iter() { let matched = match kind { QueryKind::Exact => dep.matches(summary), + QueryKind::AlternativeVersions => dep.matches(summary), QueryKind::AlternativeNames => true, QueryKind::Normalized => true, }; diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index 946378ccb60..fdf5db2501e 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -4,6 +4,7 @@ use std::task::Poll; use crate::core::{Dependency, PackageId, Registry, Summary}; use crate::sources::source::QueryKind; +use crate::sources::IndexSummary; use crate::util::edit_distance::edit_distance; use crate::util::{GlobalContext, OptVersionReq, VersionExt}; use anyhow::Error; @@ -302,6 +303,19 @@ pub(super) fn activation_error( msg } else { + // Maybe something is wrong with the available versions + let mut version_candidates = loop { + match registry.query_vec(&new_dep, QueryKind::AlternativeVersions) { + Poll::Ready(Ok(candidates)) => break candidates, + Poll::Ready(Err(e)) => return to_resolve_err(e), + Poll::Pending => match registry.block_until_ready() { + Ok(()) => continue, + Err(e) => return to_resolve_err(e), + }, + } + }; + version_candidates.sort_unstable_by_key(|a| a.as_summary().version().clone()); + // Maybe the user mistyped the name? Like `dep-thing` when `Dep_Thing` // was meant. So we try asking the registry for a `fuzzy` search for suggestions. let name_candidates = loop { @@ -327,7 +341,37 @@ pub(super) fn activation_error( name_candidates.sort_by_key(|o| o.0); let mut msg = String::new(); - if !name_candidates.is_empty() { + if !version_candidates.is_empty() { + let _ = writeln!( + &mut msg, + "no matching versions for `{}` found", + dep.package_name() + ); + for candidate in version_candidates { + match candidate { + IndexSummary::Candidate(summary) => { + // HACK: If this was a real candidate, we wouldn't hit this case. + // so it must be a patch which get normalized to being a candidate + let _ = + writeln!(&mut msg, " version {} is unavailable", summary.version()); + } + IndexSummary::Yanked(summary) => { + let _ = writeln!(&mut msg, " version {} is yanked", summary.version()); + } + IndexSummary::Offline(summary) => { + let _ = writeln!(&mut msg, " version {} is not cached", summary.version()); + } + IndexSummary::Unsupported(summary, schema_version) => { + let _ = writeln!( + &mut msg, + " version {} requires a Cargo version that supports index version {}", + summary.version(), + schema_version + ); + } + } + } + } else if !name_candidates.is_empty() { let _ = writeln!(&mut msg, "no matching package found",); let _ = writeln!(&mut msg, "searched package name: `{}`", dep.package_name()); let mut names = name_candidates diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index 166624b8dcd..b9c34690c52 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -108,7 +108,7 @@ impl<'gctx> Source for DirectorySource<'gctx> { } let packages = self.packages.values().map(|p| &p.0); let matches = packages.filter(|pkg| match kind { - QueryKind::Exact => dep.matches(pkg.summary()), + QueryKind::Exact | QueryKind::AlternativeVersions => dep.matches(pkg.summary()), QueryKind::AlternativeNames => true, QueryKind::Normalized => dep.matches(pkg.summary()), }); diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index e9dc0a0fbbf..f47225af63c 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -145,7 +145,7 @@ impl<'gctx> Source for PathSource<'gctx> { self.load()?; if let Some(s) = self.package.as_ref().map(|p| p.summary()) { let matched = match kind { - QueryKind::Exact => dep.matches(s), + QueryKind::Exact | QueryKind::AlternativeVersions => dep.matches(s), QueryKind::AlternativeNames => true, QueryKind::Normalized => dep.matches(s), }; @@ -332,7 +332,7 @@ impl<'gctx> Source for RecursivePathSource<'gctx> { .map(|p| p.summary()) { let matched = match kind { - QueryKind::Exact => dep.matches(s), + QueryKind::Exact | QueryKind::AlternativeVersions => dep.matches(s), QueryKind::AlternativeNames => true, QueryKind::Normalized => dep.matches(s), }; diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 6242c1e26ec..6a1451feb0c 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -799,7 +799,7 @@ impl<'gctx> Source for RegistrySource<'gctx> { .index .query_inner(dep.package_name(), &req, &mut *self.ops, &mut |s| { let matched = match kind { - QueryKind::Exact => { + QueryKind::Exact | QueryKind::AlternativeVersions => { if req.is_precise() && self.gctx.cli_unstable().unstable_options { dep.matches_prerelease(s.as_summary()) } else { @@ -816,6 +816,7 @@ impl<'gctx> Source for RegistrySource<'gctx> { // leak through if they're in a whitelist (aka if they were // previously in `Cargo.lock` match s { + s @ _ if kind == QueryKind::AlternativeVersions => callback(s), s @ IndexSummary::Candidate(_) => callback(s), s @ IndexSummary::Yanked(_) => { if self.yanked_whitelist.contains(&s.package_id()) { diff --git a/src/cargo/sources/source.rs b/src/cargo/sources/source.rs index bdf6fb6870a..f4a29016bb9 100644 --- a/src/cargo/sources/source.rs +++ b/src/cargo/sources/source.rs @@ -183,6 +183,13 @@ pub enum QueryKind { /// Each source gets to define what `close` means for it. /// /// Path/Git sources may return all dependencies that are at that URI, + /// whereas an `Registry` source may return dependencies that are yanked or invalid. + AlternativeVersions, + /// A query for packages close to the given dependency requirement. + /// + /// Each source gets to define what `close` means for it. + /// + /// Path/Git sources may return all dependencies that are at that URI, /// whereas an `Registry` source may return dependencies that have the same /// canonicalization. AlternativeNames, diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 896aab44bb8..c8697dcc06c 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -924,7 +924,8 @@ fn yanks_in_lockfiles_are_ok_http() { "#]], str![[r#" [UPDATING] `dummy-registry` index -[ERROR] no matching package named `bar` found +[ERROR] no matching versions for `bar` found + version 0.0.1 is yanked location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `foo v0.0.1 ([ROOT]/foo)` @@ -941,7 +942,8 @@ fn yanks_in_lockfiles_are_ok_git() { "#]], str![[r#" [UPDATING] `dummy-registry` index -[ERROR] no matching package named `bar` found +[ERROR] no matching versions for `bar` found + version 0.0.1 is yanked location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `foo v0.0.1 ([ROOT]/foo)` @@ -993,7 +995,8 @@ fn yanks_in_lockfiles_are_ok_for_other_update_http() { "#]], str![[r#" [UPDATING] `dummy-registry` index -[ERROR] no matching package named `bar` found +[ERROR] no matching versions for `bar` found + version 0.0.1 is yanked location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `foo v0.0.1 ([ROOT]/foo)` @@ -1016,7 +1019,8 @@ fn yanks_in_lockfiles_are_ok_for_other_update_git() { "#]], str![[r#" [UPDATING] `dummy-registry` index -[ERROR] no matching package named `bar` found +[ERROR] no matching versions for `bar` found + version 0.0.1 is yanked location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `foo v0.0.1 ([ROOT]/foo)` @@ -3225,7 +3229,8 @@ fn unknown_index_version_error() { .with_status(101) .with_stderr_data(str![[r#" [UPDATING] `dummy-registry` index -[ERROR] no matching package named `bar` found +[ERROR] no matching versions for `bar` found + version 1.0.1 requires a Cargo version that supports index version 4294967295 location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `foo v0.1.0 ([ROOT]/foo)` From 06b5ec5765e31223f00bfc404c935d61d86e0453 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 5 Dec 2024 11:32:32 -0600 Subject: [PATCH 10/18] test(resolver): Show error with MSRV set --- tests/testsuite/registry.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index c8697dcc06c..56ca49dea3c 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -3238,6 +3238,43 @@ required by package `foo v0.1.0 ([ROOT]/foo)` .run(); } +#[cargo_test] +fn unknown_index_version_with_msrv_error() { + // If the version field is not understood, it is ignored. + Package::new("bar", "1.0.1") + .schema_version(u32::MAX) + .rust_version("1.2345") + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + + [dependencies] + bar = "1.0" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("generate-lockfile") + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] `dummy-registry` index +[ERROR] no matching versions for `bar` found + version 1.0.1 requires a Cargo version that supports index version 4294967295 +location searched: `dummy-registry` index (which is replacing registry `crates-io`) +required by package `foo v0.1.0 ([ROOT]/foo)` + +"#]]) + .run(); +} + #[cargo_test] fn protocol() { cargo_process("install bar") From 0ed5c215a20fcc3d99d6256aa5757afad58edc11 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 5 Dec 2024 11:35:10 -0600 Subject: [PATCH 11/18] fix(resolver): Prefer rust-version over schema version --- src/cargo/core/resolver/errors.rs | 13 ++++++++++++- tests/testsuite/registry.rs | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index fdf5db2501e..74eb871c8d4 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -362,12 +362,23 @@ pub(super) fn activation_error( let _ = writeln!(&mut msg, " version {} is not cached", summary.version()); } IndexSummary::Unsupported(summary, schema_version) => { - let _ = writeln!( + if let Some(rust_version) = summary.rust_version() { + // HACK: technically its unsupported and we shouldn't make assumptions + // about the entry but this is limited and for diagnostics purposes + let _ = writeln!( + &mut msg, + " version {} requires cargo {}", + summary.version(), + rust_version + ); + } else { + let _ = writeln!( &mut msg, " version {} requires a Cargo version that supports index version {}", summary.version(), schema_version ); + } } } } diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 56ca49dea3c..2c3da8261a2 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -3267,7 +3267,7 @@ fn unknown_index_version_with_msrv_error() { .with_stderr_data(str![[r#" [UPDATING] `dummy-registry` index [ERROR] no matching versions for `bar` found - version 1.0.1 requires a Cargo version that supports index version 4294967295 + version 1.0.1 requires cargo 1.2345 location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `foo v0.1.0 ([ROOT]/foo)` From 674e609a0ec2dc431575c48989a7bd1953ff2ab0 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 19 Jun 2024 20:07:37 -0400 Subject: [PATCH 12/18] refactor: make room for adapting rustc-stable-hasher --- src/cargo/core/compiler/build_runner/compilation_files.rs | 4 ++-- src/cargo/core/compiler/compile_kind.rs | 2 +- src/cargo/core/compiler/fingerprint/mod.rs | 2 +- src/cargo/ops/cargo_compile/mod.rs | 2 +- src/cargo/util/hex.rs | 4 ++-- src/cargo/util/rustc.rs | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/cargo/core/compiler/build_runner/compilation_files.rs b/src/cargo/core/compiler/build_runner/compilation_files.rs index 80100f322ed..d9b6676d10a 100644 --- a/src/cargo/core/compiler/build_runner/compilation_files.rs +++ b/src/cargo/core/compiler/build_runner/compilation_files.rs @@ -718,8 +718,8 @@ fn compute_metadata( } } - let c_metadata = UnitHash(c_metadata_hasher.finish()); - let c_extra_filename = UnitHash(c_extra_filename_hasher.finish()); + let c_metadata = UnitHash(Hasher::finish(&c_metadata_hasher)); + let c_extra_filename = UnitHash(Hasher::finish(&c_extra_filename_hasher)); let unit_id = c_extra_filename; let c_extra_filename = use_extra_filename.then_some(c_extra_filename); diff --git a/src/cargo/core/compiler/compile_kind.rs b/src/cargo/core/compiler/compile_kind.rs index 222732ddebc..deb518afb48 100644 --- a/src/cargo/core/compiler/compile_kind.rs +++ b/src/cargo/core/compiler/compile_kind.rs @@ -195,6 +195,6 @@ impl CompileTarget { self.name.hash(&mut hasher); } } - hasher.finish() + Hasher::finish(&hasher) } } diff --git a/src/cargo/core/compiler/fingerprint/mod.rs b/src/cargo/core/compiler/fingerprint/mod.rs index 67e4aaab438..ac29b0d15d9 100644 --- a/src/cargo/core/compiler/fingerprint/mod.rs +++ b/src/cargo/core/compiler/fingerprint/mod.rs @@ -1561,7 +1561,7 @@ fn calculate_normal( local: Mutex::new(local), memoized_hash: Mutex::new(None), metadata, - config: config.finish(), + config: Hasher::finish(&config), compile_kind, rustflags: extra_flags, fs_status: FsStatus::Stale, diff --git a/src/cargo/ops/cargo_compile/mod.rs b/src/cargo/ops/cargo_compile/mod.rs index 4e011e7ec5a..c89c064c780 100644 --- a/src/cargo/ops/cargo_compile/mod.rs +++ b/src/cargo/ops/cargo_compile/mod.rs @@ -657,7 +657,7 @@ fn traverse_and_share( .collect(); // Here, we have recursively traversed this unit's dependencies, and hashed them: we can // finalize the dep hash. - let new_dep_hash = dep_hash.finish(); + let new_dep_hash = Hasher::finish(&dep_hash); // This is the key part of the sharing process: if the unit is a runtime dependency, whose // target is the same as the host, we canonicalize the compile kind to `CompileKind::Host`. diff --git a/src/cargo/util/hex.rs b/src/cargo/util/hex.rs index 2d06d9b5939..c0583e4a3af 100644 --- a/src/cargo/util/hex.rs +++ b/src/cargo/util/hex.rs @@ -10,7 +10,7 @@ pub fn to_hex(num: u64) -> String { pub fn hash_u64(hashable: H) -> u64 { let mut hasher = StableHasher::new(); hashable.hash(&mut hasher); - hasher.finish() + Hasher::finish(&hasher) } pub fn hash_u64_file(mut file: &File) -> std::io::Result { @@ -23,7 +23,7 @@ pub fn hash_u64_file(mut file: &File) -> std::io::Result { } hasher.write(&buf[..n]); } - Ok(hasher.finish()) + Ok(Hasher::finish(&hasher)) } pub fn short_hash(hashable: &H) -> String { diff --git a/src/cargo/util/rustc.rs b/src/cargo/util/rustc.rs index 19aa447edd9..66218dc8541 100644 --- a/src/cargo/util/rustc.rs +++ b/src/cargo/util/rustc.rs @@ -381,7 +381,7 @@ fn rustc_fingerprint( _ => (), } - Ok(hasher.finish()) + Ok(Hasher::finish(&hasher)) } fn process_fingerprint(cmd: &ProcessBuilder, extra_fingerprint: u64) -> u64 { @@ -391,5 +391,5 @@ fn process_fingerprint(cmd: &ProcessBuilder, extra_fingerprint: u64) -> u64 { let mut env = cmd.get_envs().iter().collect::>(); env.sort_unstable(); env.hash(&mut hasher); - hasher.finish() + Hasher::finish(&hasher) } From 5eb7480565adfaf79bfdf90de6a7ff5ea5dcd0eb Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Mon, 18 Nov 2024 23:29:01 -0500 Subject: [PATCH 13/18] feat(SourceId): use stable hash from rustc-stable-hash This helps `-Ztrim-paths` build a stable cross-platform path for the registry and git sources. Sources files then can be found from the same path when debugging. It also helps cache registry index all at once for all platforms, for example the use case in rust-lang/cargo#14795 (despite they should use `cargo vendor` instead IMO). Some caveats: * Newer cargo will need to re-download files for global caches (index files, git/registry sources). The old cache is still kept and used when running with older cargoes. * Windows is not really covered by the "cross-platform" hash, because path prefix components like `C:` are always there. That means hashes of some sources kind, like local registry and local path, are not going to be real cross-platform stable. There might be hash collisions if you have two registries under the same domain. This won't happen to crates.io, as the infra would have to intentionally put another registry on index.crates.io to collide. We don't consider this is an actual threat model, so we are not going to use any cryptographically secure hash algorithm like BLAKE3. See also --- Cargo.lock | 7 ++ Cargo.toml | 2 + src/cargo/core/source_id.rs | 91 +++++++++++++++---------- src/cargo/util/hasher.rs | 23 +------ tests/testsuite/global_cache_tracker.rs | 16 ++++- 5 files changed, 80 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bdba69d5f51..5518d5fcb96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -357,6 +357,7 @@ dependencies = [ "regex", "rusqlite", "rustc-hash 2.0.0", + "rustc-stable-hash", "rustfix", "same-file", "semver", @@ -3069,6 +3070,12 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "583034fd73374156e66797ed8e5b0d5690409c9226b22d87cb7f19821c05d152" +[[package]] +name = "rustc-stable-hash" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2febf9acc5ee5e99d1ad0afcdbccc02d87aa3f857a1f01f825b80eacf8edfcd1" + [[package]] name = "rustfix" version = "0.9.0" diff --git a/Cargo.toml b/Cargo.toml index 8e676d2984d..bafde065c09 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -82,6 +82,7 @@ rand = "0.8.5" regex = "1.10.5" rusqlite = { version = "0.32.0", features = ["bundled"] } rustc-hash = "2.0.0" +rustc-stable-hash = "0.1.1" rustfix = { version = "0.9.0", path = "crates/rustfix" } same-file = "1.0.6" schemars = "0.8.21" @@ -194,6 +195,7 @@ rand.workspace = true regex.workspace = true rusqlite.workspace = true rustc-hash.workspace = true +rustc-stable-hash.workspace = true rustfix.workspace = true same-file.workspace = true semver.workspace = true diff --git a/src/cargo/core/source_id.rs b/src/cargo/core/source_id.rs index 663f69b5751..501c88cba72 100644 --- a/src/cargo/core/source_id.rs +++ b/src/cargo/core/source_id.rs @@ -782,70 +782,89 @@ mod tests { // Otherwise please just leave a comment in your PR as to why the hash value is // changing and why the old value can't be easily preserved. // - // The hash value depends on endianness and bit-width, so we only run this test on - // little-endian 64-bit CPUs (such as x86-64 and ARM64) where it matches the - // well-known value. + // The hash value should be stable across platforms, and doesn't depend on + // endianness and bit-width. One caveat is that absolute paths on Windows + // are inherently different than on Unix-like platforms. Unless we omit or + // strip the prefix components (e.g. `C:`), there is not way to have a true + // cross-platform stable hash for absolute paths. #[test] - #[cfg(all(target_endian = "little", target_pointer_width = "64"))] - fn test_cratesio_hash() { - let gctx = GlobalContext::default().unwrap(); - let crates_io = SourceId::crates_io(&gctx).unwrap(); - assert_eq!(crate::util::hex::short_hash(&crates_io), "1ecc6299db9ec823"); - } - - // See the comment in `test_cratesio_hash`. - // - // Only test on non-Windows as paths on Windows will get different hashes. - #[test] - #[cfg(all(target_endian = "little", target_pointer_width = "64", not(windows)))] fn test_stable_hash() { use std::hash::Hasher; use std::path::Path; + use crate::util::StableHasher; + + #[cfg(not(windows))] + let ws_root = Path::new("/tmp/ws"); + #[cfg(windows)] + let ws_root = Path::new(r"C:\\tmp\ws"); + let gen_hash = |source_id: SourceId| { - let mut hasher = std::collections::hash_map::DefaultHasher::new(); - source_id.stable_hash(Path::new("/tmp/ws"), &mut hasher); - hasher.finish() + let mut hasher = StableHasher::new(); + source_id.stable_hash(ws_root, &mut hasher); + Hasher::finish(&hasher) }; + let source_id = SourceId::crates_io(&GlobalContext::default().unwrap()).unwrap(); + assert_eq!(gen_hash(source_id), 7062945687441624357); + assert_eq!(crate::util::hex::short_hash(&source_id), "25cdd57fae9f0462"); + let url = "https://my-crates.io".into_url().unwrap(); let source_id = SourceId::for_registry(&url).unwrap(); - assert_eq!(gen_hash(source_id), 18108075011063494626); - assert_eq!(crate::util::hex::short_hash(&source_id), "fb60813d6cb8df79"); + assert_eq!(gen_hash(source_id), 8310250053664888498); + assert_eq!(crate::util::hex::short_hash(&source_id), "b2d65deb64f05373"); let url = "https://your-crates.io".into_url().unwrap(); let source_id = SourceId::for_alt_registry(&url, "alt").unwrap(); - assert_eq!(gen_hash(source_id), 12862859764592646184); - assert_eq!(crate::util::hex::short_hash(&source_id), "09c10fd0cbd74bce"); + assert_eq!(gen_hash(source_id), 14149534903000258933); + assert_eq!(crate::util::hex::short_hash(&source_id), "755952de063f5dc4"); let url = "sparse+https://my-crates.io".into_url().unwrap(); let source_id = SourceId::for_registry(&url).unwrap(); - assert_eq!(gen_hash(source_id), 8763561830438022424); - assert_eq!(crate::util::hex::short_hash(&source_id), "d1ea0d96f6f759b5"); + assert_eq!(gen_hash(source_id), 16249512552851930162); + assert_eq!(crate::util::hex::short_hash(&source_id), "327cfdbd92dd81e1"); let url = "sparse+https://your-crates.io".into_url().unwrap(); let source_id = SourceId::for_alt_registry(&url, "alt").unwrap(); - assert_eq!(gen_hash(source_id), 5159702466575482972); - assert_eq!(crate::util::hex::short_hash(&source_id), "135d23074253cb78"); + assert_eq!(gen_hash(source_id), 6156697384053352292); + assert_eq!(crate::util::hex::short_hash(&source_id), "64a713b6a6fb7055"); let url = "file:///tmp/ws/crate".into_url().unwrap(); let source_id = SourceId::for_git(&url, GitReference::DefaultBranch).unwrap(); - assert_eq!(gen_hash(source_id), 15332537265078583985); - assert_eq!(crate::util::hex::short_hash(&source_id), "73a808694abda756"); - - let path = Path::new("/tmp/ws/crate"); + assert_eq!(gen_hash(source_id), 473480029881867801); + assert_eq!(crate::util::hex::short_hash(&source_id), "199e591d94239206"); + let path = &ws_root.join("crate"); let source_id = SourceId::for_local_registry(path).unwrap(); - assert_eq!(gen_hash(source_id), 18446533307730842837); - assert_eq!(crate::util::hex::short_hash(&source_id), "52a84cc73f6fd48b"); + #[cfg(not(windows))] + { + assert_eq!(gen_hash(source_id), 11515846423845066584); + assert_eq!(crate::util::hex::short_hash(&source_id), "58d73c154f81d09f"); + } + #[cfg(windows)] + { + assert_eq!(gen_hash(source_id), 6146331155906064276); + assert_eq!(crate::util::hex::short_hash(&source_id), "946fb2239f274c55"); + } let source_id = SourceId::for_path(path).unwrap(); - assert_eq!(gen_hash(source_id), 8764714075439899829); - assert_eq!(crate::util::hex::short_hash(&source_id), "e1ddd48578620fc1"); + assert_eq!(gen_hash(source_id), 215644081443634269); + #[cfg(not(windows))] + assert_eq!(crate::util::hex::short_hash(&source_id), "64bace89c92b101f"); + #[cfg(windows)] + assert_eq!(crate::util::hex::short_hash(&source_id), "01e1e6c391813fb6"); let source_id = SourceId::for_directory(path).unwrap(); - assert_eq!(gen_hash(source_id), 17459999773908528552); - assert_eq!(crate::util::hex::short_hash(&source_id), "6568fe2c2fab5bfe"); + #[cfg(not(windows))] + { + assert_eq!(gen_hash(source_id), 6127590343904940368); + assert_eq!(crate::util::hex::short_hash(&source_id), "505191d1f3920955"); + } + #[cfg(windows)] + { + assert_eq!(gen_hash(source_id), 10423446877655960172); + assert_eq!(crate::util::hex::short_hash(&source_id), "6c8ad69db585a790"); + } } #[test] diff --git a/src/cargo/util/hasher.rs b/src/cargo/util/hasher.rs index 87586c0900f..93c88bb4bf9 100644 --- a/src/cargo/util/hasher.rs +++ b/src/cargo/util/hasher.rs @@ -1,25 +1,6 @@ -//! Implementation of a hasher that produces the same values across releases. +//! A hasher that produces the same values across releases and platforms. //! //! The hasher should be fast and have a low chance of collisions (but is not //! sufficient for cryptographic purposes). -#![allow(deprecated)] -use std::hash::{Hasher, SipHasher}; - -#[derive(Clone)] -pub struct StableHasher(SipHasher); - -impl StableHasher { - pub fn new() -> StableHasher { - StableHasher(SipHasher::new()) - } -} - -impl Hasher for StableHasher { - fn finish(&self) -> u64 { - self.0.finish() - } - fn write(&mut self, bytes: &[u8]) { - self.0.write(bytes) - } -} +pub use rustc_stable_hash::StableSipHasher128 as StableHasher; diff --git a/tests/testsuite/global_cache_tracker.rs b/tests/testsuite/global_cache_tracker.rs index dfd698f5f8b..9a20ef50c18 100644 --- a/tests/testsuite/global_cache_tracker.rs +++ b/tests/testsuite/global_cache_tracker.rs @@ -2004,7 +2004,16 @@ fn compatible_with_older_cargo() { assert_eq!(get_registry_names("src"), ["middle-1.0.0", "new-1.0.0"]); assert_eq!( get_registry_names("cache"), - ["middle-1.0.0.crate", "new-1.0.0.crate", "old-1.0.0.crate"] + // Duplicate crates from two different cache location + // because we're changing how SourceId is hashed. + // This change should be reverted once rust-lang/cargo#14917 lands. + [ + "middle-1.0.0.crate", + "middle-1.0.0.crate", + "new-1.0.0.crate", + "new-1.0.0.crate", + "old-1.0.0.crate" + ] ); // T-0 months: Current version, make sure it can read data from stable, @@ -2027,7 +2036,10 @@ fn compatible_with_older_cargo() { assert_eq!(get_registry_names("src"), ["new-1.0.0"]); assert_eq!( get_registry_names("cache"), - ["middle-1.0.0.crate", "new-1.0.0.crate"] + // Duplicate crates from two different cache location + // because we're changing how SourceId is hashed. + // This change should be reverted once rust-lang/cargo#14917 lands. + ["middle-1.0.0.crate", "new-1.0.0.crate", "new-1.0.0.crate"] ); } From b45ca32bff294922bfe3df99deae508b70be9b13 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 11 Dec 2024 10:48:14 -0600 Subject: [PATCH 14/18] refactor(source): Rename AlternativeVersions to RejectedVersions Really, when we switch the req to a wildcard, it is an alternative version, so for yanked and unsupported we need a different name. --- crates/resolver-tests/src/lib.rs | 2 +- src/cargo/core/resolver/errors.rs | 2 +- src/cargo/sources/directory.rs | 2 +- src/cargo/sources/path.rs | 4 ++-- src/cargo/sources/registry/mod.rs | 4 ++-- src/cargo/sources/source.rs | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index f34dd737650..359090540bd 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -143,7 +143,7 @@ pub fn resolve_with_global_context_raw( for summary in self.list.iter() { let matched = match kind { QueryKind::Exact => dep.matches(summary), - QueryKind::AlternativeVersions => dep.matches(summary), + QueryKind::RejectedVersions => dep.matches(summary), QueryKind::AlternativeNames => true, QueryKind::Normalized => true, }; diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index 74eb871c8d4..45dea512b8f 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -305,7 +305,7 @@ pub(super) fn activation_error( } else { // Maybe something is wrong with the available versions let mut version_candidates = loop { - match registry.query_vec(&new_dep, QueryKind::AlternativeVersions) { + match registry.query_vec(&new_dep, QueryKind::RejectedVersions) { Poll::Ready(Ok(candidates)) => break candidates, Poll::Ready(Err(e)) => return to_resolve_err(e), Poll::Pending => match registry.block_until_ready() { diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index b9c34690c52..6f676a79850 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -108,7 +108,7 @@ impl<'gctx> Source for DirectorySource<'gctx> { } let packages = self.packages.values().map(|p| &p.0); let matches = packages.filter(|pkg| match kind { - QueryKind::Exact | QueryKind::AlternativeVersions => dep.matches(pkg.summary()), + QueryKind::Exact | QueryKind::RejectedVersions => dep.matches(pkg.summary()), QueryKind::AlternativeNames => true, QueryKind::Normalized => dep.matches(pkg.summary()), }); diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index f47225af63c..1d4a533a3c4 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -145,7 +145,7 @@ impl<'gctx> Source for PathSource<'gctx> { self.load()?; if let Some(s) = self.package.as_ref().map(|p| p.summary()) { let matched = match kind { - QueryKind::Exact | QueryKind::AlternativeVersions => dep.matches(s), + QueryKind::Exact | QueryKind::RejectedVersions => dep.matches(s), QueryKind::AlternativeNames => true, QueryKind::Normalized => dep.matches(s), }; @@ -332,7 +332,7 @@ impl<'gctx> Source for RecursivePathSource<'gctx> { .map(|p| p.summary()) { let matched = match kind { - QueryKind::Exact | QueryKind::AlternativeVersions => dep.matches(s), + QueryKind::Exact | QueryKind::RejectedVersions => dep.matches(s), QueryKind::AlternativeNames => true, QueryKind::Normalized => dep.matches(s), }; diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 6a1451feb0c..3ff8fad344b 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -799,7 +799,7 @@ impl<'gctx> Source for RegistrySource<'gctx> { .index .query_inner(dep.package_name(), &req, &mut *self.ops, &mut |s| { let matched = match kind { - QueryKind::Exact | QueryKind::AlternativeVersions => { + QueryKind::Exact | QueryKind::RejectedVersions => { if req.is_precise() && self.gctx.cli_unstable().unstable_options { dep.matches_prerelease(s.as_summary()) } else { @@ -816,7 +816,7 @@ impl<'gctx> Source for RegistrySource<'gctx> { // leak through if they're in a whitelist (aka if they were // previously in `Cargo.lock` match s { - s @ _ if kind == QueryKind::AlternativeVersions => callback(s), + s @ _ if kind == QueryKind::RejectedVersions => callback(s), s @ IndexSummary::Candidate(_) => callback(s), s @ IndexSummary::Yanked(_) => { if self.yanked_whitelist.contains(&s.package_id()) { diff --git a/src/cargo/sources/source.rs b/src/cargo/sources/source.rs index f4a29016bb9..045684008ab 100644 --- a/src/cargo/sources/source.rs +++ b/src/cargo/sources/source.rs @@ -184,7 +184,7 @@ pub enum QueryKind { /// /// Path/Git sources may return all dependencies that are at that URI, /// whereas an `Registry` source may return dependencies that are yanked or invalid. - AlternativeVersions, + RejectedVersions, /// A query for packages close to the given dependency requirement. /// /// Each source gets to define what `close` means for it. From 8ec9b1ef883ef3a7a53dbeb0e33fc390cf6b09c6 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 11 Dec 2024 11:41:30 -0600 Subject: [PATCH 15/18] refactor(resolver): Clarify which dep spec is used for alternatives --- src/cargo/core/resolver/errors.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index 45dea512b8f..c29a4c6a267 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -222,11 +222,11 @@ pub(super) fn activation_error( // Maybe the user mistyped the ver_req? Like `dep="2"` when `dep="0.2"` // was meant. So we re-query the registry with `dep="*"` so we can // list a few versions that were actually found. - let mut new_dep = dep.clone(); - new_dep.set_version_req(OptVersionReq::Any); + let mut wild_dep = dep.clone(); + wild_dep.set_version_req(OptVersionReq::Any); let candidates = loop { - match registry.query_vec(&new_dep, QueryKind::Exact) { + match registry.query_vec(&wild_dep, QueryKind::Exact) { Poll::Ready(Ok(candidates)) => break candidates, Poll::Ready(Err(e)) => return to_resolve_err(e), Poll::Pending => match registry.block_until_ready() { @@ -305,7 +305,7 @@ pub(super) fn activation_error( } else { // Maybe something is wrong with the available versions let mut version_candidates = loop { - match registry.query_vec(&new_dep, QueryKind::RejectedVersions) { + match registry.query_vec(&wild_dep, QueryKind::RejectedVersions) { Poll::Ready(Ok(candidates)) => break candidates, Poll::Ready(Err(e)) => return to_resolve_err(e), Poll::Pending => match registry.block_until_ready() { @@ -319,7 +319,7 @@ pub(super) fn activation_error( // Maybe the user mistyped the name? Like `dep-thing` when `Dep_Thing` // was meant. So we try asking the registry for a `fuzzy` search for suggestions. let name_candidates = loop { - match registry.query_vec(&new_dep, QueryKind::AlternativeNames) { + match registry.query_vec(&wild_dep, QueryKind::AlternativeNames) { Poll::Ready(Ok(candidates)) => break candidates, Poll::Ready(Err(e)) => return to_resolve_err(e), Poll::Pending => match registry.block_until_ready() { @@ -336,7 +336,7 @@ pub(super) fn activation_error( name_candidates.dedup_by(|a, b| a.name() == b.name()); let mut name_candidates: Vec<_> = name_candidates .iter() - .filter_map(|n| Some((edit_distance(&*new_dep.package_name(), &*n.name(), 3)?, n))) + .filter_map(|n| Some((edit_distance(&*wild_dep.package_name(), &*n.name(), 3)?, n))) .collect(); name_candidates.sort_by_key(|o| o.0); From 3aab14575f2740719ae6328c447ff4ce986b4823 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 11 Dec 2024 11:39:47 -0600 Subject: [PATCH 16/18] fix(resolver): Don't report all versions as rejected When I copy/pasted the alternative-names code, I didn't notice that the wildcard dependency was used. This does not have a test yet because overly-constrictive dep specs have higher precedence atm (which I plan to also fix). --- src/cargo/core/resolver/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index c29a4c6a267..a44485adaa9 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -305,7 +305,7 @@ pub(super) fn activation_error( } else { // Maybe something is wrong with the available versions let mut version_candidates = loop { - match registry.query_vec(&wild_dep, QueryKind::RejectedVersions) { + match registry.query_vec(&dep, QueryKind::RejectedVersions) { Poll::Ready(Ok(candidates)) => break candidates, Poll::Ready(Err(e)) => return to_resolve_err(e), Poll::Pending => match registry.block_until_ready() { From ab7469e3b8fea6d7e7febe5d278d570254142e78 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 11 Dec 2024 16:53:01 -0600 Subject: [PATCH 17/18] fix(script): Don't override the release profile This has been around since #12224 and isn't in the RFC, so its being removed. This is part of #12207. --- src/cargo/util/toml/embedded.rs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 126188588b2..686fcb77a22 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -141,19 +141,6 @@ fn expand_manifest_( toml::Value::Array(vec![toml::Value::Table(bin)]), ); - let release = manifest - .entry("profile".to_owned()) - .or_insert_with(|| toml::Value::Table(Default::default())) - .as_table_mut() - .ok_or_else(|| anyhow::format_err!("`profile` must be a table"))? - .entry("release".to_owned()) - .or_insert_with(|| toml::Value::Table(Default::default())) - .as_table_mut() - .ok_or_else(|| anyhow::format_err!("`profile.release` must be a table"))?; - release - .entry("strip".to_owned()) - .or_insert_with(|| toml::Value::Boolean(true)); - Ok(manifest) } @@ -588,9 +575,6 @@ build = false edition = "2024" name = "test-" -[profile.release] -strip = true - [workspace] "#]] @@ -626,9 +610,6 @@ build = false edition = "2024" name = "test-" -[profile.release] -strip = true - [workspace] "#]] From 71f81734657aab2e1fcfda8232c4ab4ca22f7f5f Mon Sep 17 00:00:00 2001 From: Aatif Syed Date: Wed, 11 Dec 2024 23:12:35 +0000 Subject: [PATCH 18/18] fix: emit_serialized_unit_graph uses the configured shell --- src/cargo/core/compiler/unit_graph.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/cargo/core/compiler/unit_graph.rs b/src/cargo/core/compiler/unit_graph.rs index 14bab76d65d..553996bcdc0 100644 --- a/src/cargo/core/compiler/unit_graph.rs +++ b/src/cargo/core/compiler/unit_graph.rs @@ -10,7 +10,6 @@ use crate::util::interning::InternedString; use crate::util::CargoResult; use crate::GlobalContext; use std::collections::HashMap; -use std::io::Write; /// The dependency graph of Units. pub type UnitGraph = HashMap>; @@ -121,15 +120,10 @@ pub fn emit_serialized_unit_graph( } }) .collect(); - let s = SerializedUnitGraph { + + gctx.shell().print_json(&SerializedUnitGraph { version: VERSION, units: ser_units, roots, - }; - - let stdout = std::io::stdout(); - let mut lock = stdout.lock(); - serde_json::to_writer(&mut lock, &s)?; - drop(writeln!(lock)); - Ok(()) + }) }