diff --git a/Cargo.lock b/Cargo.lock index a8f0e5e5eed..835a3229769 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -426,6 +426,7 @@ dependencies = [ "pasetors", "pathdiff", "portable-atomic", + "pubgrub", "rand 0.10.1", "regex", "rusqlite", @@ -607,6 +608,7 @@ dependencies = [ "anstyle-hyperlink", "anstyle-progress", "anyhow", + "cargo-util", "libc", "serde", "serde_json", @@ -3010,9 +3012,9 @@ checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" [[package]] name = "libgit2-sys" -version = "0.18.5+1.9.4" +version = "0.18.7+1.9.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "005d6ae6eac1912906073e069f7db60b1fa98e052a68227824afe3e3a1c59ca2" +checksum = "23c7391e4b9f4ffab1a624223cc1d7385ff9a678f490768add717de7ea2f4d89" dependencies = [ "cc", "libc", @@ -3811,6 +3813,17 @@ dependencies = [ "elliptic-curve", ] +[[package]] +name = "priority-queue" +version = "2.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93980406f12d9f8140ed5abe7155acb10bb1e69ea55c88960b9c2f117445ef96" +dependencies = [ + "equivalent", + "indexmap", + "serde", +] + [[package]] name = "proc-macro-error-attr2" version = "2.0.0" @@ -3870,6 +3883,20 @@ dependencies = [ "unarray", ] +[[package]] +name = "pubgrub" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a1c3256d319f1ed35251223140ab8f29fd6b0528a1216344efd904c651fecd5e" +dependencies = [ + "indexmap", + "log", + "priority-queue", + "rustc-hash 2.1.2", + "thiserror 2.0.18", + "version-ranges", +] + [[package]] name = "pulldown-cmark" version = "0.13.4" @@ -5305,6 +5332,15 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68ed610a8d5e63d9c0e31300e8fdb55104c5f21e422743a9dc74848fa8317fd2" +[[package]] +name = "version-ranges" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "31e9bd4e9c9ff6a2a9b5969462ba26216af3e010df0377dad8320ab515262ef8" +dependencies = [ + "smallvec", +] + [[package]] name = "version_check" version = "0.9.5" diff --git a/Cargo.toml b/Cargo.toml index b2fc229f6d9..39f983e6473 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -85,6 +85,7 @@ pathdiff = "0.2.3" portable-atomic = "1.13.1" percent-encoding = "2.3.2" proptest = "1.11.0" +pubgrub = "0.4.0" pulldown-cmark = { version = "0.13.3", default-features = false, features = ["html"] } rand = "0.10.1" regex = "1.12.3" @@ -203,6 +204,7 @@ os_info.workspace = true pasetors.workspace = true pathdiff.workspace = true portable-atomic.workspace = true +pubgrub.workspace = true rand.workspace = true regex.workspace = true rusqlite = { workspace = true, features = ["fallible_uint"] } diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index 55d345dc2e4..04471cadbba 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/crates/cargo-test-support/src/lib.rs @@ -1439,6 +1439,10 @@ pub trait TestEnvCommandExt: Sized { .env("CARGO_INCREMENTAL", "0") // Don't read the system git config which is out of our control. .env("GIT_CONFIG_NOSYSTEM", "1") + // See: https://github.com/rust-lang/rust/pull/159857#issuecomment-5119325932 + // This should be removed once the new build-dir layout is stabilized and the old layout + // is removed. + .env("__CARGO_TEMPORARY_BUILD_DIR_NEW_LAYOUT_OPT_OUT", "1") .env_remove("CI") .env_remove("__CARGO_DEFAULT_LIB_METADATA") .env_remove("ALL_PROXY") diff --git a/crates/cargo-util-terminal/Cargo.toml b/crates/cargo-util-terminal/Cargo.toml index 428c7d4bc41..7957b7945c0 100644 --- a/crates/cargo-util-terminal/Cargo.toml +++ b/crates/cargo-util-terminal/Cargo.toml @@ -14,6 +14,7 @@ anstyle.workspace = true anstyle-hyperlink = { workspace = true, features = ["file"] } anstyle-progress.workspace = true anyhow.workspace = true +cargo-util.workspace = true serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["raw_value"] } supports-hyperlinks.workspace = true diff --git a/crates/cargo-util-terminal/src/shell.rs b/crates/cargo-util-terminal/src/shell.rs index e39d231e7dd..ecea8547a61 100644 --- a/crates/cargo-util-terminal/src/shell.rs +++ b/crates/cargo-util-terminal/src/shell.rs @@ -363,6 +363,21 @@ impl Shell { url.map(|u| self.err_hyperlink(u)).unwrap_or_default() } + /// Query terminal capability, independent of user configuration + pub fn progress_supported(&self) -> bool { + // report no progress when -q (for quiet) or TERM=dumb are set + // or if running on Continuous Integration service like Travis where the + // output logs get mangled. + if self.verbosity == Verbosity::Quiet + || std::env::var("TERM").as_deref() == Ok("dumb") + || cargo_util::is_ci() + { + false + } else { + true + } + } + fn unstable_flags_rustc_unicode(&self) -> bool { match &self.output { ShellOut::Write(_) => false, diff --git a/crates/cargo-util/src/paths.rs b/crates/cargo-util/src/paths.rs index 28f77d6a3e5..d7d4f6a32ed 100644 --- a/crates/cargo-util/src/paths.rs +++ b/crates/cargo-util/src/paths.rs @@ -57,6 +57,8 @@ pub fn dylib_path_envvar() -> &'static str { "DYLD_FALLBACK_LIBRARY_PATH" } else if cfg!(target_os = "aix") { "LIBPATH" + } else if cfg!(target_os = "haiku") { + "LIBRARY_PATH" } else { "LD_LIBRARY_PATH" } diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index e624fdd9e95..a024956fc8b 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -33,9 +33,38 @@ use proptest::prelude::*; use proptest::sample::Index; use proptest::string::string_regex; +/// Builds the [`GlobalContext`] used by the convenience resolve helpers. +/// +/// When the `__CARGO_TEST_PUBGRUB` environment variable is set, the experimental +/// `-Zpubgrub-resolver` is enabled, so the entire curated resolver test suite +/// can be re-run against the PubGrub resolver for differential validation: +/// +/// ```sh +/// __CARGO_TEST_PUBGRUB=1 cargo test -p resolver-tests +/// ``` +pub fn test_global_context() -> GlobalContext { + let mut gctx = GlobalContext::default().unwrap(); + if gctx.get_env_os("__CARGO_TEST_PUBGRUB").is_some() { + gctx.nightly_features_allowed = true; + gctx.configure( + 0, + false, + None, + false, + false, + false, + &None, + &["pubgrub-resolver".to_string()], + &[], + ) + .unwrap(); + } + gctx +} + pub fn resolve(deps: Vec, registry: &[Summary]) -> CargoResult> { Ok( - resolve_with_global_context(deps, registry, &GlobalContext::default().unwrap())? + resolve_with_global_context(deps, registry, &test_global_context())? .into_iter() .map(|(pkg, _)| pkg) .collect(), @@ -61,7 +90,7 @@ pub fn resolve_and_validated_raw( deps.clone(), registry, root_pkg_id, - &GlobalContext::default().unwrap(), + &test_global_context(), ); match resolve { @@ -129,6 +158,31 @@ pub fn resolve_with_global_context_raw( registry: &[Summary], root_pkg_id: PackageId, gctx: &GlobalContext, +) -> CargoResult { + resolve_with_prefs_raw( + deps, + registry, + root_pkg_id, + gctx, + VersionPreferences::default(), + ) +} + +/// Like [`resolve_with_global_context_raw`], but lets the caller seed the +/// [`VersionPreferences`]. +/// +/// This is how the conservative-update paths are exercised offline: an existing +/// lockfile (and `cargo update -p `) is modeled by preferring the +/// previously selected [`PackageId`]s (see [`prefs_from_lock`]), and `--precise` +/// is modeled by preferring an exact [`Dependency`]. The production glue in +/// `ops::resolve` builds the same [`VersionPreferences`] before handing off to +/// the resolver, so this is the faithful resolver-level slice of those flows. +pub fn resolve_with_prefs_raw( + deps: Vec, + registry: &[Summary], + root_pkg_id: PackageId, + gctx: &GlobalContext, + mut version_prefs: VersionPreferences, ) -> CargoResult { struct MyRegistry<'a> { list: &'a [Summary], @@ -193,7 +247,6 @@ pub fn resolve_with_global_context_raw( let opts = ResolveOpts::everything(); let start = Instant::now(); - let mut version_prefs = VersionPreferences::default(); if gctx.cli_unstable().minimal_versions { version_prefs.version_ordering(VersionOrdering::MinimumVersionsFirst) } @@ -213,6 +266,26 @@ pub fn resolve_with_global_context_raw( resolve } +/// Build [`VersionPreferences`] that reproduce a previous resolution, modeling +/// an existing `Cargo.lock` (and `cargo update -p `) at the resolver +/// level. +/// +/// Every previously selected package is preferred via +/// [`VersionPreferences::prefer_package_id`], *except* those whose name is in +/// `unlock`. Passing an empty `unlock` models building against an untouched lock +/// (everything kept); passing a single name models `cargo update -p ` +/// (that crate is free to move, the rest are pinned). +pub fn prefs_from_lock(resolve: &Resolve, unlock: &[&str]) -> VersionPreferences { + let mut prefs = VersionPreferences::default(); + for id in resolve.iter() { + if unlock.contains(&id.name().as_str()) { + continue; + } + prefs.prefer_package_id(id); + } + prefs +} + /// By default `Summary` and `Dependency` have a very verbose `Debug` representation. /// This replaces with a representation that uses constructors from this file. /// diff --git a/crates/resolver-tests/tests/pubgrub_graph.rs b/crates/resolver-tests/tests/pubgrub_graph.rs new file mode 100644 index 00000000000..b485faabf54 --- /dev/null +++ b/crates/resolver-tests/tests/pubgrub_graph.rs @@ -0,0 +1,114 @@ +//! Compares the full resolved dependency *graph* (edges, not just the package +//! set) produced by the pubgrub resolver against the default resolver. + +use std::collections::BTreeSet; + +use cargo::resolver::Resolve; +use cargo::workspace::{Dependency, Summary}; +use cargo::util::GlobalContext; + +use resolver_tests::{ + helpers::{ToDep, dep, pkg, pkg_dep, pkg_dep_with, pkg_id, registry}, + resolve_with_global_context_raw, +}; + +fn pubgrub_gctx() -> GlobalContext { + let mut gctx = GlobalContext::default().unwrap(); + gctx.nightly_features_allowed = true; + gctx.configure( + 0, + false, + None, + false, + false, + false, + &None, + &["pubgrub-resolver".to_string()], + &[], + ) + .unwrap(); + gctx +} + +fn edges(r: &Resolve) -> BTreeSet<(String, String)> { + let mut e = BTreeSet::new(); + for p in r.iter() { + for (dp, _) in r.deps(p) { + e.insert(( + format!("{}/{}", p.name(), p.version()), + format!("{}/{}", dp.name(), dp.version()), + )); + } + } + e +} + +#[track_caller] +fn assert_same_graph(deps: Vec, reg: &[Summary]) { + let default = resolve_with_global_context_raw( + deps.clone(), + reg, + pkg_id("root"), + &GlobalContext::default().unwrap(), + ); + let pubgrub = resolve_with_global_context_raw(deps, reg, pkg_id("root"), &pubgrub_gctx()); + match (default, pubgrub) { + (Ok(d), Ok(p)) => { + let de = edges(&d); + let pe = edges(&p); + let missing: Vec<_> = de.difference(&pe).collect(); + let extra: Vec<_> = pe.difference(&de).collect(); + assert!( + missing.is_empty() && extra.is_empty(), + "graph mismatch:\n missing in pubgrub: {missing:?}\n extra in pubgrub: {extra:?}", + ); + } + (Err(_), Err(_)) => {} + (d, p) => panic!( + "resolvers disagree on solvability: default={:?} pubgrub={:?}", + d.is_ok(), + p.is_ok() + ), + } +} + +/// An optional dependency that IS activated (via `features = [..]` on the dep) +/// must appear as an edge. +#[test] +fn activated_optional_edge_present() { + let reg = registry(vec![ + pkg(("serde", "1.0.0")), + pkg_dep_with("bstr", vec!["serde".opt()], &[]), + pkg_dep(("consumer", "1.0.0"), vec!["bstr".with(&["serde"])]), + ]); + assert_same_graph(vec![dep("consumer")], ®); +} + +/// A weak dependency feature (`dep?/feat`) on an enabled feature still records +/// the optional dependency as an edge in the lock graph (mirrors bstr's +/// `std = ["serde?/std"]`), matching Cargo's v1 lock resolver. +#[test] +fn weak_dep_feature_records_edge() { + let reg = registry(vec![ + pkg_dep_with("serde", vec![], &[("std", &[])]), + pkg_dep_with("bstr", vec!["serde".opt()], &[("std", &["serde?/std"])]), + pkg_dep(("consumer", "1.0.0"), vec!["bstr".with(&["std"])]), + ]); + assert_same_graph(vec![dep("consumer")], ®); +} + +/// An optional dependency that is NOT activated must NOT create an edge, even +/// if its target is otherwise present in the lock (regression for the +/// schemars->url cycle). +#[test] +fn unactivated_optional_edge_absent() { + let reg = registry(vec![ + // b optionally depends on a, but that optional dep is never enabled. + pkg_dep_with("b", vec!["a".opt()], &[]), + // a depends on b normally; a is also independently in the graph. + pkg_dep(("a", "1.0.0"), vec![dep("b")]), + ]); + // Resolving `a` pulls in b; b's optional `a` is not enabled, so there must + // be no b->a edge (which would be a cycle). + assert_same_graph(vec![dep("a")], ®); +} diff --git a/crates/resolver-tests/tests/pubgrub_prop.rs b/crates/resolver-tests/tests/pubgrub_prop.rs new file mode 100644 index 00000000000..6ba2620ab98 --- /dev/null +++ b/crates/resolver-tests/tests/pubgrub_prop.rs @@ -0,0 +1,153 @@ +use std::io::IsTerminal; + +use cargo::util::GlobalContext; +use cargo_util::is_ci; + +use cargo::workspace::PackageId; +use cargo::resolver::Resolve; +use cargo::resolver::VersionPreferences; +use cargo::util::interning::InternedString; + +use resolver_tests::{ + PrettyPrintRegistry, + helpers::{dep_req, pkg_id, registry}, + prefs_from_lock, registry_strategy, resolve_with_global_context, resolve_with_prefs_raw, + sat::SatResolver, +}; + +use proptest::prelude::*; + +/// Project a [`Resolve`] into the `(PackageId, features)` shape the SAT +/// reference resolver validates. +fn collect_features(resolve: &Resolve) -> Vec<(PackageId, Vec)> { + resolve + .sort() + .iter() + .map(|&pkg| (pkg, resolve.features(pkg).to_vec())) + .collect() +} + +fn pubgrub_gctx() -> GlobalContext { + let mut gctx = GlobalContext::default().unwrap(); + gctx.nightly_features_allowed = true; + gctx.configure( + 0, + false, + None, + false, + false, + false, + &None, + &["pubgrub-resolver".to_string()], + &[], + ) + .unwrap(); + gctx +} + +proptest! { + #![proptest_config(ProptestConfig { + max_shrink_iters: + if is_ci() || !std::io::stderr().is_terminal() { + 0 + } else { + u32::MAX + }, + result_cache: prop::test_runner::basic_result_cache, + .. ProptestConfig::default() + })] + + /// The pubgrub resolver must agree with the SAT reference resolver: every + /// solution it produces is valid, and it only fails when there is no + /// solution. + /// + /// NOTE: this is a form of fuzz testing; a failure indicates a real + /// problem, but passing does not prove correctness. + #[test] + fn prop_pubgrub_passes_validation( + PrettyPrintRegistry(input) in registry_strategy(50, 20, 60) + ) { + let reg = registry(input.clone()); + let gctx = pubgrub_gctx(); + let mut sat = SatResolver::new(®); + for this in input.iter().rev().take(20) { + let deps = vec![dep_req(&this.name(), &format!("={}", this.version()))]; + match resolve_with_global_context(deps.clone(), ®, &gctx) { + Ok(out) => prop_assert!( + sat.sat_is_valid_solution(&out), + "pubgrub solution rejected by SAT for `{}={}`:\n{:?}\n{:?}", + this.name(), this.version(), out, PrettyPrintRegistry(input.clone()), + ), + Err(_) => prop_assert!( + !sat.sat_resolve(&deps), + "pubgrub failed but SAT says solvable for `{}={}`\n{:?}", + this.name(), this.version(), PrettyPrintRegistry(input.clone()), + ), + } + } + } + + /// The conservative-update paths (building against a lock, `cargo update + /// -p`) must stay correct: after resolving once and feeding the result back + /// as version preferences, a second pubgrub resolution must still produce a + /// SAT-valid solution. + /// + /// Preferences only reorder the candidates pubgrub considers; they must + /// never let it accept an invalid solution nor fail when one exists. We + /// cross-check the locked re-resolution against the default resolver run + /// with the same preferences so the two resolvers agree on solvability. + #[test] + fn prop_pubgrub_locked_reresolve_passes_validation( + PrettyPrintRegistry(input) in registry_strategy(50, 20, 60) + ) { + let reg = registry(input.clone()); + let gctx = pubgrub_gctx(); + let default_gctx = GlobalContext::default().unwrap(); + let mut sat = SatResolver::new(®); + + for this in input.iter().rev().take(20) { + let deps = vec![dep_req(&this.name(), &format!("={}", this.version()))]; + + // First pass: a fresh pubgrub resolution acts as the "lock". + let Ok(locked) = resolve_with_prefs_raw( + deps.clone(), ®, pkg_id("root"), &gctx, VersionPreferences::default(), + ) else { + continue; + }; + + // Re-resolve with everything preferred (building against the lock) + // and with the requested dependency freed (`cargo update -p `). + let unlock_dep = this.name(); + for unlock in [Vec::new(), vec![unlock_dep.as_str()]] { + let prefs = || prefs_from_lock(&locked, &unlock); + + let pubgrub = resolve_with_prefs_raw( + deps.clone(), ®, pkg_id("root"), &gctx, prefs(), + ); + let default = resolve_with_prefs_raw( + deps.clone(), ®, pkg_id("root"), &default_gctx, prefs(), + ); + + match pubgrub { + Ok(out) => { + prop_assert!( + sat.sat_is_valid_solution(&collect_features(&out)), + "locked pubgrub solution rejected by SAT for `{}={}` (unlock={:?}):\n{:?}", + this.name(), this.version(), unlock, PrettyPrintRegistry(input.clone()), + ); + prop_assert!( + default.is_ok(), + "locked pubgrub resolved but default failed for `{}={}` (unlock={:?})\n{:?}", + this.name(), this.version(), unlock, PrettyPrintRegistry(input.clone()), + ); + } + Err(_) => prop_assert!( + default.is_err(), + "locked pubgrub failed but default resolved for `{}={}` (unlock={:?})\n{:?}", + this.name(), this.version(), unlock, PrettyPrintRegistry(input.clone()), + ), + } + } + } + } +} diff --git a/crates/resolver-tests/tests/pubgrub_smoke.rs b/crates/resolver-tests/tests/pubgrub_smoke.rs new file mode 100644 index 00000000000..11bcde38ae6 --- /dev/null +++ b/crates/resolver-tests/tests/pubgrub_smoke.rs @@ -0,0 +1,46 @@ +use cargo::util::GlobalContext; + +use resolver_tests::{ + helpers::{dep, dep_req, pkg, pkg_dep, registry}, + resolve_with_global_context, +}; + +fn pubgrub_gctx() -> GlobalContext { + let mut gctx = GlobalContext::default().unwrap(); + gctx.nightly_features_allowed = true; + gctx.configure( + 0, + false, + None, + false, + false, + false, + &None, + &["pubgrub-resolver".to_string()], + &[], + ) + .unwrap(); + gctx +} + +#[test] +fn smoke_single() { + let reg = registry(vec![pkg(("a", "1.0.0"))]); + let gctx = pubgrub_gctx(); + let res = resolve_with_global_context(vec![dep("a")], ®, &gctx); + eprintln!("RESULT: {res:?}"); + assert!(res.is_ok(), "{res:?}"); +} + +#[test] +fn smoke_transitive() { + let reg = registry(vec![ + pkg_dep(("a", "1.0.0"), vec![dep_req("b", "^1.0")]), + pkg(("b", "1.2.0")), + pkg(("b", "1.0.0")), + ]); + let gctx = pubgrub_gctx(); + let res = resolve_with_global_context(vec![dep("a")], ®, &gctx); + eprintln!("RESULT: {res:?}"); + assert!(res.is_ok(), "{res:?}"); +} diff --git a/crates/resolver-tests/tests/pubgrub_update.rs b/crates/resolver-tests/tests/pubgrub_update.rs new file mode 100644 index 00000000000..8645c362698 --- /dev/null +++ b/crates/resolver-tests/tests/pubgrub_update.rs @@ -0,0 +1,281 @@ +//! Differential tests for the conservative-update paths of the pubgrub +//! resolver. +//! +//! Building against an existing `Cargo.lock`, `cargo update -p `, and +//! `--precise` all reach the resolver as [`VersionPreferences`]: a set of +//! previously selected packages (or exact dependencies) that resolution should +//! reuse when still valid. The production glue in `ops::resolve` constructs +//! those preferences; here we construct them directly with [`prefs_from_lock`] +//! and a hand-built [`VersionPreferences`], which is the faithful +//! resolver-level slice of those flows. +//! +//! Each test follows the same shape: +//! +//! 1. resolve a manifest fresh, +//! 2. derive preferences from that first resolution (the "lock"), +//! 3. mutate the registry and/or the root manifest (publish a new version, free +//! one crate, add a dependency, pin an exact version), +//! 4. re-resolve *with those preferences* under both the default resolver and +//! the pubgrub resolver, +//! 5. assert the two resolvers agree on the full graph (nodes **and** edges). +//! +//! The default resolver is the oracle: these tests pin down that pubgrub honors +//! preferences the same way Cargo already does, rather than asserting a +//! particular hand-computed lock (which would re-encode the very logic under +//! test). + +use std::collections::BTreeSet; + +use cargo::resolver::Resolve; +use cargo::resolver::VersionPreferences; +use cargo::workspace::{Dependency, Summary}; +use cargo::util::GlobalContext; + +use resolver_tests::{ + helpers::{dep, dep_req, pkg, pkg_dep, pkg_id, registry}, + prefs_from_lock, resolve_with_prefs_raw, +}; + +fn default_gctx() -> GlobalContext { + GlobalContext::default().unwrap() +} + +fn pubgrub_gctx() -> GlobalContext { + let mut gctx = GlobalContext::default().unwrap(); + gctx.nightly_features_allowed = true; + gctx.configure( + 0, + false, + None, + false, + false, + false, + &None, + &["pubgrub-resolver".to_string()], + &[], + ) + .unwrap(); + gctx +} + +/// `name/version` for every resolved package. +fn nodes(r: &Resolve) -> BTreeSet { + r.iter() + .map(|p| format!("{}/{}", p.name(), p.version())) + .collect() +} + +/// `parent -> child` for every resolved edge. +fn edges(r: &Resolve) -> BTreeSet<(String, String)> { + let mut e = BTreeSet::new(); + for p in r.iter() { + for (dp, _) in r.deps(p) { + e.insert(( + format!("{}/{}", p.name(), p.version()), + format!("{}/{}", dp.name(), dp.version()), + )); + } + } + e +} + +/// Resolve `deps` against `reg` with the default resolver and no preferences; +/// this stands in for a fresh `generate-lockfile`. +#[track_caller] +fn lock(deps: Vec, reg: &[Summary]) -> Resolve { + resolve_with_prefs_raw( + deps, + reg, + pkg_id("root"), + &default_gctx(), + VersionPreferences::default(), + ) + .expect("initial resolution should succeed") +} + +/// Re-resolve `deps` against `reg` with `prefs` under both resolvers and assert +/// the resulting graphs are identical. +#[track_caller] +fn assert_same_update( + deps: Vec, + reg: &[Summary], + prefs: impl Fn() -> VersionPreferences, +) { + let default = + resolve_with_prefs_raw(deps.clone(), reg, pkg_id("root"), &default_gctx(), prefs()); + let pubgrub = resolve_with_prefs_raw(deps, reg, pkg_id("root"), &pubgrub_gctx(), prefs()); + match (default, pubgrub) { + (Ok(d), Ok(p)) => { + assert_eq!( + nodes(&d), + nodes(&p), + "node set mismatch:\n default: {:?}\n pubgrub: {:?}", + nodes(&d), + nodes(&p), + ); + assert_eq!( + edges(&d), + edges(&p), + "edge set mismatch:\n default: {:?}\n pubgrub: {:?}", + edges(&d), + edges(&p), + ); + } + (Err(_), Err(_)) => {} + (d, p) => panic!( + "resolvers disagree on solvability: default={} pubgrub={}", + d.is_ok(), + p.is_ok() + ), + } +} + +/// Building against an untouched lock keeps the previously selected version +/// even after a newer one is published. +#[test] +fn keeps_locked_version_when_newer_published() { + // Lock against an index that only has foo 1.0.0. + let old_reg = registry(vec![pkg(("foo", "1.0.0"))]); + let locked = lock(vec![dep_req("foo", "^1")], &old_reg); + + // foo 1.1.0 is now published, but the lock prefers 1.0.0. + let new_reg = registry(vec![pkg(("foo", "1.0.0")), pkg(("foo", "1.1.0"))]); + assert_same_update(vec![dep_req("foo", "^1")], &new_reg, || { + prefs_from_lock(&locked, &[]) + }); +} + +/// `cargo update -p foo` frees `foo` (only) to advance to the newest +/// compatible version while everything else stays put. +#[test] +fn update_single_package_advances_only_it() { + let old_reg = registry(vec![pkg(("foo", "1.0.0")), pkg(("bar", "1.0.0"))]); + let locked = lock(vec![dep_req("foo", "^1"), dep_req("bar", "^1")], &old_reg); + + // Both foo and bar have newer releases now; updating only foo should move + // foo to 1.1.0 while bar stays at 1.0.0. + let new_reg = registry(vec![ + pkg(("foo", "1.0.0")), + pkg(("foo", "1.1.0")), + pkg(("bar", "1.0.0")), + pkg(("bar", "1.1.0")), + ]); + assert_same_update( + vec![dep_req("foo", "^1"), dep_req("bar", "^1")], + &new_reg, + || prefs_from_lock(&locked, &["foo"]), + ); +} + +/// Adding a brand-new dependency to the manifest leaves the locked packages +/// untouched and only selects the new crate (and its subtree). +#[test] +fn adding_a_dependency_keeps_the_rest_locked() { + let old_reg = registry(vec![pkg(("foo", "1.0.0")), pkg(("foo", "1.1.0"))]); + let locked = lock(vec![dep_req("foo", "^1")], &old_reg); + + // Now `bar` is added to the manifest (and bar depends on foo too). foo must + // stay at its locked version even though bar would otherwise pull the newest. + let new_reg = registry(vec![ + pkg(("foo", "1.0.0")), + pkg(("foo", "1.1.0")), + pkg_dep(("bar", "1.0.0"), vec![dep_req("foo", "^1")]), + ]); + assert_same_update( + vec![dep_req("foo", "^1"), dep_req("bar", "^1")], + &new_reg, + || prefs_from_lock(&locked, &[]), + ); +} + +/// A locked transitive dependency shared by two parents stays pinned across a +/// re-resolve when a newer compatible version appears. +#[test] +fn shared_transitive_stays_locked() { + let old_reg = registry(vec![ + pkg(("baz", "1.0.0")), + pkg_dep(("foo", "1.0.0"), vec![dep_req("baz", "^1")]), + pkg_dep(("bar", "1.0.0"), vec![dep_req("baz", "^1")]), + ]); + let locked = lock(vec![dep_req("foo", "^1"), dep_req("bar", "^1")], &old_reg); + + let new_reg = registry(vec![ + pkg(("baz", "1.0.0")), + pkg(("baz", "1.1.0")), + pkg_dep(("foo", "1.0.0"), vec![dep_req("baz", "^1")]), + pkg_dep(("bar", "1.0.0"), vec![dep_req("baz", "^1")]), + ]); + assert_same_update( + vec![dep_req("foo", "^1"), dep_req("bar", "^1")], + &new_reg, + || prefs_from_lock(&locked, &[]), + ); +} + +/// `--precise` pins an exact version via a preferred dependency; resolution +/// must select it even when a newer one is available. +#[test] +fn precise_pins_exact_version() { + let reg = registry(vec![ + pkg(("foo", "1.0.0")), + pkg(("foo", "1.1.0")), + pkg(("foo", "1.2.0")), + ]); + // `cargo update foo --precise 1.1.0` is modeled as preferring `foo =1.1.0`. + assert_same_update(vec![dep_req("foo", "^1")], ®, || { + let mut prefs = VersionPreferences::default(); + prefs.prefer_dependency(dep_req("foo", "=1.1.0")); + prefs + }); +} + +/// Unlocking everything (`cargo update` with no `-p`) lets all packages move to +/// the newest compatible versions. +#[test] +fn full_update_advances_everything() { + let old_reg = registry(vec![pkg(("foo", "1.0.0")), pkg(("bar", "1.0.0"))]); + let locked = lock(vec![dep_req("foo", "^1"), dep_req("bar", "^1")], &old_reg); + + let new_reg = registry(vec![ + pkg(("foo", "1.0.0")), + pkg(("foo", "1.1.0")), + pkg(("bar", "1.0.0")), + pkg(("bar", "1.1.0")), + ]); + // Free both crates: prefs become empty, so this is a fresh resolution. + assert_same_update( + vec![dep_req("foo", "^1"), dep_req("bar", "^1")], + &new_reg, + || prefs_from_lock(&locked, &["foo", "bar"]), + ); +} + +/// A locked version that is no longer valid (the manifest constraint moved to a +/// new major) must be dropped in favor of a compatible one, despite the +/// preference. +#[test] +fn stale_lock_is_overridden_by_constraint() { + let old_reg = registry(vec![pkg(("foo", "1.0.0"))]); + let locked = lock(vec![dep_req("foo", "^1")], &old_reg); + + // The manifest now requires foo ^2; the locked 1.0.0 cannot satisfy it. + let new_reg = registry(vec![pkg(("foo", "1.0.0")), pkg(("foo", "2.0.0"))]); + assert_same_update(vec![dep_req("foo", "^2")], &new_reg, || { + prefs_from_lock(&locked, &[]) + }); +} + +/// Sanity check that `dep` (wildcard) and feature-free graphs round-trip the +/// preference path too, not just exact requirements. +#[test] +fn wildcard_dep_keeps_locked_version() { + let old_reg = registry(vec![pkg(("foo", "1.0.0"))]); + let locked = lock(vec![dep("foo")], &old_reg); + + let new_reg = registry(vec![ + pkg(("foo", "1.0.0")), + pkg(("foo", "1.1.0")), + pkg(("foo", "2.0.0")), + ]); + assert_same_update(vec![dep("foo")], &new_reg, || prefs_from_lock(&locked, &[])); +} diff --git a/crates/resolver-tests/tests/pubgrub_validated.rs b/crates/resolver-tests/tests/pubgrub_validated.rs new file mode 100644 index 00000000000..c43cfc918be --- /dev/null +++ b/crates/resolver-tests/tests/pubgrub_validated.rs @@ -0,0 +1,156 @@ +//! Validates the `-Zpubgrub-resolver` resolver against the SAT reference +//! resolver over a spread of resolution scenarios. + +use cargo::workspace::Dependency; +use cargo::util::GlobalContext; + +use resolver_tests::{ + helpers::{ToDep, dep, dep_req, pkg, pkg_dep, pkg_dep_link, pkg_dep_with, registry}, + resolve_with_global_context, + sat::SatResolver, +}; + +fn pubgrub_gctx() -> GlobalContext { + let mut gctx = GlobalContext::default().unwrap(); + gctx.nightly_features_allowed = true; + gctx.configure( + 0, + false, + None, + false, + false, + false, + &None, + &["pubgrub-resolver".to_string()], + &[], + ) + .unwrap(); + gctx +} + +/// Resolve `deps` against `reg` with pubgrub and check the result agrees with +/// the SAT reference resolver. +#[track_caller] +fn check(deps: Vec, reg: &[cargo::workspace::Summary]) { + let gctx = pubgrub_gctx(); + let mut sat = SatResolver::new(reg); + match resolve_with_global_context(deps.clone(), reg, &gctx) { + Ok(out) => assert!( + sat.sat_is_valid_solution(&out), + "pubgrub produced a solution the SAT resolver rejects:\n{out:?}", + ), + Err(e) => assert!( + !sat.sat_resolve(&deps), + "pubgrub failed but SAT says it is solvable:\n{e:?}\n{}", + sat.used_packages().unwrap_or_default(), + ), + } +} + +#[test] +fn transitive() { + let reg = registry(vec![ + pkg_dep(("a", "1.0.0"), vec![dep_req("b", "^1.0")]), + pkg(("b", "1.2.0")), + pkg(("b", "1.0.0")), + ]); + check(vec![dep("a")], ®); +} + +#[test] +fn incompatible_majors_coexist() { + let reg = registry(vec![ + pkg_dep(("a", "1.0.0"), vec![dep_req("b", "^1.0")]), + pkg_dep(("c", "1.0.0"), vec![dep_req("b", "^2.0")]), + pkg(("b", "1.0.0")), + pkg(("b", "2.0.0")), + ]); + check(vec![dep("a"), dep("c")], ®); +} + +#[test] +fn pick_highest_compatible() { + let reg = registry(vec![ + pkg(("a", "1.0.0")), + pkg(("a", "1.1.0")), + pkg(("a", "1.2.0")), + pkg(("a", "2.0.0")), + ]); + check(vec![dep_req("a", "^1.0")], ®); +} + +#[test] +fn named_feature() { + let reg = registry(vec![ + pkg(("b", "1.0.0")), + pkg_dep_with("a", vec!["b".opt()], &[("f", &["b"])]), + ]); + check(vec!["a".with(&["f"])], ®); +} + +#[test] +fn default_feature() { + let reg = registry(vec![ + pkg(("b", "1.0.0")), + pkg_dep_with("a", vec!["b".opt()], &[("default", &["b"])]), + ]); + check(vec![dep("a")], ®); +} + +#[test] +fn dep_colon_feature() { + let reg = registry(vec![ + pkg(("b", "1.0.0")), + pkg_dep_with("a", vec!["b".opt()], &[("f", &["dep:b"])]), + ]); + check(vec!["a".with(&["f"])], ®); +} + +#[test] +fn dep_slash_feature() { + let reg = registry(vec![ + pkg_dep_with("b", vec![], &[("inner", &[])]), + pkg_dep_with("a", vec!["b".to_dep()], &[("f", &["b/inner"])]), + ]); + check(vec!["a".with(&["f"])], ®); +} + +#[test] +fn unselected_optional_dep() { + let reg = registry(vec![ + pkg(("b", "1.0.0")), + pkg_dep_with("a", vec!["b".opt()], &[("f", &["b"])]), + ]); + // `f` not enabled, so `b` must not be pulled in. + check(vec![dep("a")], ®); +} + +#[test] +fn links_conflict_is_unsat() { + let reg = registry(vec![ + pkg_dep_link("foo", "foo", vec![]), + pkg_dep_link("bar", "foo", vec![]), + ]); + check(vec![dep("foo"), dep("bar")], ®); +} + +#[test] +fn missing_dependency_is_unsat() { + let reg = registry(vec![ + pkg_dep(("a", "1.0.0"), vec![dep_req("b", "^2.0")]), + pkg(("b", "1.0.0")), + ]); + check(vec![dep("a")], ®); +} + +#[test] +fn diamond() { + let reg = registry(vec![ + pkg_dep(("a", "1.0.0"), vec![dep("b"), dep("c")]), + pkg_dep(("b", "1.0.0"), vec![dep_req("d", "^1.0")]), + pkg_dep(("c", "1.0.0"), vec![dep_req("d", "^1.0")]), + pkg(("d", "1.0.0")), + pkg(("d", "1.5.0")), + ]); + check(vec![dep("a")], ®); +} diff --git a/crates/resolver-tests/tests/resolve.rs b/crates/resolver-tests/tests/resolve.rs index 20d32fbf884..7ad080155fb 100644 --- a/crates/resolver-tests/tests/resolve.rs +++ b/crates/resolver-tests/tests/resolve.rs @@ -878,6 +878,13 @@ fn resolving_but_no_exists() { let res = resolve(vec![dep_req("foo", "1")], ®); assert!(res.is_err()); + // The PubGrub resolver reports conflicts via its own derivation-tree + // formatter rather than Cargo's native messages; only assert the outcome. + #[expect(clippy::disallowed_methods, reason = "no GlobalContext in scope")] + if std::env::var_os("__CARGO_TEST_PUBGRUB").is_some() { + return; + } + assert_eq!( res.err().unwrap().to_string(), "no matching package named `foo` found\n\ @@ -1017,6 +1024,12 @@ fn shortest_path_in_error_message() { ]; let error = resolve(vec![dep("A")], ®istry(input)).unwrap_err(); println!("{}", error); + // The PubGrub resolver formats conflicts differently; only assert that + // resolution fails (above), not the exact Cargo-native message. + #[expect(clippy::disallowed_methods, reason = "no GlobalContext in scope")] + if std::env::var_os("__CARGO_TEST_PUBGRUB").is_some() { + return; + } assert_data_eq!( error.to_string(), str![[r#" diff --git a/crates/xtask-lint-docs/src/main.rs b/crates/xtask-lint-docs/src/main.rs index 98b413e8d2a..b2265a24a49 100644 --- a/crates/xtask-lint-docs/src/main.rs +++ b/crates/xtask-lint-docs/src/main.rs @@ -40,7 +40,16 @@ fn main() -> anyhow::Result<()> { writeln!(buf, "# Lints\n")?; writeln!( buf, - "Note: [Cargo's linting system is unstable](unstable.md#lintscargo) and can only be used on nightly toolchains" + "> [!NOTE] +> This chapter is about lints emitted by `cargo` itself. +> +> See [the lints section in the Manifest Format +> chapter](manifest.md#the-lints-section) to configure lint levels for tools +> such as `rustc` or `clippy`. + +> [!WARNING] +> [Cargo's linting system is unstable](unstable.md#lintscargo) and can only be used on nightly +> toolchains." )?; writeln!(buf)?; diff --git a/design-docs/pubgrub-resolver.md b/design-docs/pubgrub-resolver.md new file mode 100644 index 00000000000..6f024016dea --- /dev/null +++ b/design-docs/pubgrub-resolver.md @@ -0,0 +1,636 @@ +# PubGrub Resolver for Cargo (`-Zpubgrub-resolver`) — Design & Handoff + +> Status: **experimental, working for fresh full-graph resolution.** This document +> is a handoff for the next agent/engineer. It captures the architecture, the +> hard-won correctness insights, how to build/test, what is verified, and the +> prioritized next steps. + +Branch: `pubgrub` (off `rust-lang/cargo` master). + +> Rebased onto upstream `master` after the `src/` flattening +> (rust-lang/cargo#17230, #17231). Paths in this document reflect the **new** +> layout: the module lives at `src/resolver/pubgrub/`, not +> `src/cargo/core/resolver/pubgrub/`. See §13 if another such move lands. + +--- + +## 1. Goal + +Replace Cargo's hand-rolled backtracking dependency resolver with one built on +the [`pubgrub`](https://crates.io/crates/pubgrub) v0.4 crate, **side by side** +with the existing resolver, gated behind the unstable flag +`-Zpubgrub-resolver`. When the flag is off, resolution is completely unchanged. + +Initial acceptance bar: **resolve Cargo's own dependency tree** and produce a +lockfile identical to the default resolver. + +--- + +## 2. Current status (verified) + +- **Full-tree parity against an *unmodified* `rust-lang/cargo`.** Using the + pubgrub-enabled binary against a **pristine clone** of `rust-lang/cargo` (whose + `Cargo.toml` has no pubgrub dependency), with no pre-existing `Cargo.lock`, + both resolvers produce a **byte-identical** lockfile (latest run: 5907 lines, + 542 packages, `diff` = 0). See §3 for the exact procedure. + - Verified two ways, because `diff == 0` alone is *not* sufficient (it is also + consistent with the `-Z` flag being a silent no-op that runs the default + resolver twice): + 1. **Dispatch proof** — a temporary marker in `pubgrub::resolve` printed 0 + times without the flag and 1 time with it, proving the flag routes to the + pubgrub code path. (Also: `generate-lockfile` does a **single** resolve + pass, so the marker fires once.) + 2. **Parity proof** — the byte-identical lockfile above. + - NOTE: resolving the manifest *inside this branch's repo* is a weaker check: + this branch adds `pubgrub`/`version-ranges` to the workspace `Cargo.toml`, so + its lockfile is ~5942 lines (the extra ~35 are those added deps). Always test + against a pristine clone to avoid that confound. +- Validation in `crates/resolver-tests`: + - `pubgrub_smoke.rs` — basic end-to-end (2 tests). + - `pubgrub_validated.rs` — SAT-validated scenarios: features, `dep:`/`dep/feat`, + incompatible majors, links conflicts, diamonds, missing deps (11 tests). + - `pubgrub_graph.rs` — **graph/edge** comparison vs the default resolver, + including regressions for the cycle and weak-dependency cases (3 tests). + - `pubgrub_prop.rs` — property tests vs the SAT reference resolver over 256 + randomly generated registries: a fresh-resolution check and a + conservative-update check (resolve, feed the result back as + `VersionPreferences`, re-resolve both kept and with the requested crate + freed, and re-validate against SAT + the default resolver). + - `pubgrub_update.rs` — deterministic offline differential tests for the + conservative-update paths: building against an untouched lock, `cargo + update -p `, `cargo update` (free everything), adding a dependency, + shared transitive pinning, stale-lock override, and `--precise`. Each + resolves, derives `VersionPreferences` from the first resolution, mutates + one input, and asserts the pubgrub graph (nodes **and** edges) matches the + default resolver (8 tests). + - **Curated suite via `__CARGO_TEST_PUBGRUB=1`** — the harness convenience + helpers route through `-Zpubgrub-resolver` when this env var is set, so the + pre-existing curated suites run on PubGrub: + - `tests/resolve.rs`: **37/37 pass**. + - `tests/pubgrub.rs`: **28/28 pass** (weak deps, feature unification, cyclic + features — all SAT-validated where applicable). + - Two `resolve.rs` tests have their *exact error-text* assertions gated off + under PubGrub (it uses its own derivation-tree formatter); the resolution + *outcome* is identical. + +### Caveats on the verification +- Parity is verified against the **current crates.io index state**; index drift + changes selected versions for both resolvers (it stays a 0-line diff because + both drift together, but it is not a hermetic golden-file test). +- Full-lockfile parity is verified for **`generate-lockfile` (fresh)** only. The + conservative-update paths are now exercised at the **resolver level** (where + they reduce to `VersionPreferences`) by `pubgrub_update.rs` and the new + `pubgrub_prop.rs` case — see §9.3. Still unverified end-to-end: the + `ops::resolve` glue that *builds* those preferences from a real `Cargo.lock`, + and the registry-side exact pinning that `--precise` performs on top of the + preference (the harness models `--precise` only as a preferred `=x.y.z` + dependency). +- The package-set/SAT tests do **not** check graph edges; only `pubgrub_graph.rs` + does. Edge correctness is where the subtle bugs lived (see §6). + +--- + +## 3. How to build & test (IMPORTANT) + +This workspace needs OpenSSL/curl/libgit2 from a Nix dev shell. **All** cargo +commands must run inside it: + +```sh +nix develop ~/dev/dotfiles#cargo --command bash -c '' +``` + +Common commands: + +```sh +# Build the library +nix develop ~/dev/dotfiles#cargo --command bash -c 'cargo build -p cargo --lib' + +# Build the cargo binary (needed for real lockfile tests) +nix develop ~/dev/dotfiles#cargo --command bash -c 'cargo build --bin cargo' + +# Unit tests for the semver conversion +nix develop ~/dev/dotfiles#cargo --command bash -c 'cargo test -p cargo --lib resolver::pubgrub' + +# Resolver-test suites +nix develop ~/dev/dotfiles#cargo --command bash -c \ + 'cargo test -p resolver-tests --test pubgrub_graph --test pubgrub_validated --test pubgrub_smoke' + +# Property test (slow, ~60-70s) +nix develop ~/dev/dotfiles#cargo --command bash -c 'cargo test -p resolver-tests --test pubgrub_prop' + +# Re-run the ENTIRE curated suite through the PubGrub resolver +nix develop ~/dev/dotfiles#cargo --command bash -c \ + '__CARGO_TEST_PUBGRUB=1 cargo test -p resolver-tests --test resolve --test pubgrub' + +# Re-run the FULL integration testsuite through PubGrub. `__CARGO_TEST_PUBGRUB` +# is honored at the resolver dispatch fork (resolver::resolve), independent of +# the nightly-gated `-Zpubgrub-resolver` flag, and is inherited by the child +# cargo processes the testsuite spawns. Expect failures: many testsuite cases +# assert exact error text / version-selection ordering the PubGrub path does +# not reproduce. This is a survey of the gap, not a pass/fail gate. +nix develop ~/dev/dotfiles#cargo --command bash -c \ + '__CARGO_TEST_PUBGRUB=1 cargo test -p cargo --test testsuite' +``` + +### Reproducing the full-tree parity check (the real acceptance test) + +Test against a **pristine clone** of `rust-lang/cargo`, *not* this branch's repo +(this branch's `Cargo.toml` adds the pubgrub dependency — a confound). + +```sh +nix develop ~/dev/dotfiles#cargo --command bash -c ' + cd /local/home/whlo/dev/cargo + cargo build --bin cargo + CARGO=$(pwd)/target/debug/cargo + + rm -rf /tmp/cargo-clean + git clone --depth 1 https://github.com/rust-lang/cargo /tmp/cargo-clean + cd /tmp/cargo-clean + grep -c pubgrub Cargo.toml # expect 0 (pristine manifest) + + rm -f Cargo.lock; $CARGO generate-lockfile >/dev/null 2>&1; cp Cargo.lock /tmp/d.lock + rm -f Cargo.lock; $CARGO -Zpubgrub-resolver generate-lockfile >/dev/null 2>&1; cp Cargo.lock /tmp/p.lock + diff /tmp/d.lock /tmp/p.lock && echo IDENTICAL +' +``` +> - Always `rm -f Cargo.lock` before *each* run. A present lock seeds +> `version_prefs` and masks fresh-resolution bugs (this exact mistake produced +> a false "it works" claim early on). +> - `diff == 0` is necessary but **not** sufficient — it cannot tell "pubgrub +> matched" from "flag is a no-op, default ran twice". To prove the pubgrub path +> actually executed, run with the flag and +> `CARGO_LOG=cargo::resolver::pubgrub=debug`; you should see +> `pubgrub resolver active: resolving N workspace member(s)` (a permanent +> `tracing::debug!` in `pubgrub::resolve`). It does not print without the flag. + +--- + +## 4. Architecture + +### 4.1 Dispatch (the only fork point) +`src/resolver/mod.rs::resolve()` checks +`gctx.cli_unstable().pubgrub_resolver` and, if set, calls +`pubgrub::resolve(...)` with the identical signature. Flag is declared in +`src/workspace/features.rs` (`unstable_cli_options!` + parse arm +`"pubgrub-resolver"`). The single upstream call site is +`src/ops/resolve.rs` (~line 505), unchanged. + +### 4.2 Module layout — `src/resolver/pubgrub/` + +| File | Responsibility | +|---|---| +| `mod.rs` | Entry `resolve()`. Builds `RegistryQueryer`, the `Root`s from workspace members + their requested features, the `Provider`, runs `pubgrub::resolve(Provider, Root, 0.0.0)`, then reconstructs via `solution`. Translates `PubGrubError` (stashed real errors take precedence over `NoSolution`). | +| `semver_pubgrub.rs` | `SemverPubgrub`: a `pubgrub::VersionSet` over `semver::Version`. Ported & specialized from the `semver-pubgrub` crate, adapted to published pubgrub 0.4 (`Range`/`VersionSet`). Bug-for-bug compatible with `VersionReq::matches`. Also `SemverCompatibility` (the compat-bucket enum) + `only_one_compatibility_range`, `as_singleton`. | +| `package.rs` | `PubGrubPackage` — the encoding (see §5). Plus `FeatureNamespace`, `BucketName`, `WideName`, and `OptVersionReq -> SemverPubgrub` conversion. | +| `provider.rs` | `Provider`: implements `pubgrub::DependencyProvider`. Wraps Cargo's async `RegistryQueryer` with a **blocking** poll loop. `choose_version`, `prioritize`, `get_dependencies` (the big translation from `Summary`/`Dependency`/`FeatureValue` into the encoding). | +| `solution.rs` | `into_resolve`: projects pubgrub's `SelectedDependencies` back into a Cargo `Resolve` (graph nodes, edges, features, checksums, replacements). Reuses the default resolver's `check_cycles` / `check_duplicate_pkgs_in_lockfile`. Handles `[patch]`/`[replace]` node identity (see §6). | +| `error.rs` | The standalone error-reporting bridge: `report_error` turns a `PubGrubError` into a typed `ResolveError`, reusing the v1 resolver's own renderers for byte-identical text. Defines `UnavailableReason` (the structured `M`). See §8. | + +### 4.3 Data flow +``` +ops::resolve → resolver::resolve --flag--> pubgrub::resolve + │ + RegistryQueryer (async, poll) │ build Roots from (Summary, ResolveOpts) + ▲ blocking bridge ▼ + Provider: DependencyProvider ──► pubgrub::resolve(Root, 0.0.0) + │ SelectedDependencies + ▼ + solution::into_resolve → Resolve → Cargo.lock +``` + +--- + +## 5. The encoding (the crux) + +PubGrub selects **one version per package**. Cargo needs (a) the same crate at +multiple semver-incompatible versions and (b) feature unification. We encode +both into a richer package identity (`PubGrubPackage`), adapted from +`Eh2406/pubgrub-crates-benchmark`'s `Names` enum, extended to carry `SourceId` +(Cargo has multiple sources) and to own its data: + +- `Root` — synthetic; its deps are the workspace members. +- `Bucket { name: (crate, source, SemverCompatibility), member, all_features }` + — a concrete crate within one compat bucket. Distinct buckets coexist ⇒ + incompatible majors allowed. `member` ⇒ include dev-deps. `all_features` ⇒ + enable every feature (lockfile pass). +- `BucketFeatures { bucket, FeatureNamespace }` — "this feature (Feat) or + optional-dep activation (Dep) is enabled". Feature unification falls out of + version solving over these virtual packages. +- `BucketDefaultFeatures { bucket }` — default features enabled. +- `Wide { name, req, from, from_compat }` (+ `WideFeatures`, + `WideDefaultFeatures`) — used when a requirement could span **multiple** compat + buckets (rare; e.g. `>=1, <3`). Defers bucket choice to a second step. +- `Links { links }` — enforces global uniqueness of a `links` value. + +`semver::Version` is used directly as pubgrub's `V` (it already implements +`Ord/Clone/Debug/Display`). pubgrub 0.4's `Package` trait needs only +`Clone+Eq+Hash+Debug+Display` (no `Ord`), so `PubGrubPackage` does not implement +`Ord`. + +### Key encoding rules in `get_dependencies` (provider.rs) +- A feature/default-feature package depends on its `Bucket` pinned to the same + exact version (singleton range) → ties feature selection to the crate version. +- `Bucket` with `all_features` enables every key in `summary.features()` (the + feature map already contains implicit features for optional deps). +- Optional deps are pulled in only via `BucketFeatures{Dep(..)}` packages, except + in the `all_features` bucket. +- **Weak dep features (`dep?/feat`)**: still activate the optional dependency + (record the edge); the `weak` flag only suppresses enabling the dep's own + *implicit feature*. This mirrors Cargo's v1 lock resolver — see §6. + +### Reconstruction rules in `solution.rs` +- Real nodes = `Bucket` packages → `PackageId(name, version, source)`. +- A package's enabled features = the `Feat(..)` + `default` activations in the + solution. +- Edges: for each resolved package, walk its `summary.dependencies()`; include an + edge when: + - dev-dependency: only if the package is a workspace `member`; + - optional: only if activated (`BucketFeatures{Dep(name_in_toml)}` present); + - otherwise (normal/build, non-optional): always. +- The child version is found via `from_dep` (re-derives the bucket; for `Wide` + packages it reads the chosen bucket from the solution). + +--- + +## 6. Hard-won correctness insights (READ THIS) + +These cost real debugging time; do not regress them. + +1. **Workspace members are not in the registry.** They are provided directly. + The provider seeds its version cache with the root summaries in + `Provider::new`; otherwise `choose_version` queries the registry for a member + and finds nothing → immediate `NoSolution`. + +2. **Cargo's lockfile graph is activation-gated, NOT feature-agnostic.** An + optional-dependency edge appears only if the optional dep is activated. + Drawing edges for any present optional dep (a tempting "fix") creates cycles + such as `schemars → url` and fails `check_cycles`. + +3. **Weak dependency features still create the edge.** Cargo's v1 lock resolver + (`dep_cache.rs::Requirements::require_dep_feature`) runs + `self.deps.entry(package).or_default().insert(feat)` **unconditionally** — so a + `dep?/feat` reference in an *enabled* feature records the optional dependency + in the lock graph. The `weak` flag only gates whether the dep's own implicit + feature is enabled. Example: bstr's `std = ["serde?/std"]` causes + `bstr → serde` to appear in the lock even though `serde` is never + feature-activated (confirmed: even `cargo tree --all-features` shows bstr + without `serde`, yet the lock has the edge). The v1 lock resolver is a + deliberately coarse over-approximation; the precise feature resolver + (`features.rs::FeatureResolver`) refines features at build time. **We are + replacing the v1 lock resolver, so we must match its coarse behavior.** + +4. **The SAT/scenario tests do not check edges.** They validate the package set + and feature set. The cycle and weak-dep bugs only showed up via full-lockfile + diff and the new `pubgrub_graph.rs`. Always add edge-level tests for graph + bugs. + +5. **Always `rm -f Cargo.lock` before a fresh-resolution test.** A present lock + seeds `version_prefs` and hides bugs. + +6. **The reconstructed node identity is the *selected summary's* `PackageId`, + not the bucket's.** `[patch]` redirects a query to a summary from a different + source, so building the node from the bucket's `(name, source)` records the + wrong source ("patch not used" + checksum errors). Use + `summary.package_id()`. `[replace]` is the inverse: keep the original as the + node but *also* register the replacement target as a resolved node, else + `Resolve::deps`' replacement redirection points at a package missing from the + set ("couldn't find … in package set"). + +7. **A feature listing itself is a cycle PubGrub won't catch.** + `default = ["default"]` becomes a self-dependency, which the solver treats as + trivially satisfiable. Detect `*f == feat` explicitly to match the default + resolver's `cyclic feature dependency` error. Mutual cycles (`A → B → A` + across distinct features) are *legal* and must still resolve. + +### How I root-caused #3 (technique worth reusing) +Temporarily instrumented `dep_cache.rs::resolve_features` to print, for a target +crate (env-gated), `parent`, `opts.features`, and `reqs.deps`. Running the +**default** resolver showed `serde` in bstr's `reqs.deps` despite features being +only `{std, unicode}` → pointed straight at `serde?/std`. (Instrumentation has +been removed; re-add ad hoc if needed.) + +--- + +## 7. Reference material reused + +- `pubgrub-rs/semver-pubgrub` — source ported/specialized into + `semver_pubgrub.rs` (it targets pubgrub's git `dev` branch; adapted to + published 0.4). MPL-2.0 — note for upstreaming. +- `Eh2406/pubgrub-crates-benchmark` — the `Names` encoding + `DependencyProvider` + shape was the model for `package.rs`/`provider.rs`. Also a ready-made harness + to resolve thousands of real crates with both resolvers (great for §8.4). +- pubgrub 0.4 published API notes: `DependencyConstraints` is a `Vec` + newtype (build via `FromIterator`; no `entry`/`insert`). + `SelectedDependencies` has `iter()`/`get()`. `Dependencies::{Available, + Unavailable}`. `Range` is re-exported from `version-ranges` 0.1. + +--- + +## 8. Known limitations / open questions + +- **Conservative updates verified at the resolver level, not end-to-end.** + Building against a lock, `cargo update -p`, and `--precise` all reach the + resolver as `VersionPreferences`; `pubgrub_update.rs` + the new + `pubgrub_prop.rs` case confirm pubgrub honors those preferences exactly like + the default resolver (`choose_version` iterates `version_prefs`-sorted + candidates). Not yet covered: the `ops::resolve` glue that constructs the + preferences from a real `Cargo.lock`, and the registry-side version pinning + `--precise` applies in addition to the preference. +- **`[patch]`/`[replace]`** — now handled in `solution.rs`. `[patch]` uses the + selected summary's real `PackageId` (carrying the patched source) as the node + identity; `[replace]` registers the replacement target as a resolved node + (graph + summary + checksum), mirroring the default resolver's activation of + the replacement summary. Brought `patch::` 43→25 and `replace::` 20→9 under + `__CARGO_TEST_PUBGRUB`. Remaining `patch::` failures are a *spurious + `[UPDATING]` index refresh* (the non-locked wildcard query defeats the + locked-patch short-circuit in `PackageRegistry::query`), not misresolution. +- **Error reporting** — a standalone bridge now lives in `pubgrub/error.rs` + (the only place that formats resolver errors). It returns a typed + `ResolveError` and reuses the v1 resolver's own renderers for byte-identical + text, via three extracted helpers in `errors.rs`: + - `RequirementError::into_activate_error(None, …)` — root/CLI + missing/cyclic-feature and missing-dependency errors; + - `no_candidates_error` — the "no matching package / version / yanked / typo" + family (the trigger recovers the failing `Dependency` and checks whether + *any* candidate matches its req, so both absent-crate and wrong-version + cases route here); + - `version_conflict_error` — the "candidates exist but conflict" family, used + so far for a dependency requesting a feature its target lacks (the bridge + reuses `into_activate_error(Some(parent), …)` to get Cargo's own + `ConflictReason`). + + PubGrub's custom incompatibility metadata `M` is a structured + `UnavailableReason`, not a string, so the provider never bakes prose. + **Still falling back** to pubgrub's `DefaultStringReporter` (wrapped as + `ResolveError`): *semver* and *links* conflicts (deliberately not bridged — + their text needs the full multi-hop dependency path the derivation tree + doesn't preserve; see §9.6), and the offline-mode hint (the provider carries + no `GlobalContext`). +- **Performance** is not tuned: blocking poll loop in `Provider::candidates`, no + reuse of the provider across Cargo's two resolve passes, `RefCell` caches. +- **`Wide` packages** (multi-bucket requirements) are implemented but lightly + exercised; most real reqs are single-bucket. +- **`features` map fidelity** in the reconstructed `Resolve` is approximate + (Feat names + `default`); the lockfile itself doesn't store features, but + downstream `cargo build` feature unification reads this map — verify it. +- **Public/private deps, artifact (bindeps), platform `cfg` deps** not + specifically validated. + +--- + +## 9. Prioritized next steps + +1. ~~Run `tests/resolve.rs` through pubgrub.~~ **DONE** via `__CARGO_TEST_PUBGRUB` + (see §2/§3). `resolve.rs` 37/37, `pubgrub.rs` 28/28; `proptests.rs` also + passes 5/5 under the env var at the default 256 cases. The env var is now + *also* honored at the resolver dispatch fork (not just in the resolver-tests + harness), so the full `cargo test -p cargo --test testsuite` can be run on + PubGrub — see §12 for the current survey results. Next: wire a curated + green subset into CI (the full testsuite is not yet pass/fail-clean). +2. **Scale the property test** (bump cases way up; loop it). It is the Cargo + team's de-facto correctness gate. +3. ~~**Verify conservative-update paths**: existing-lock reuse, `cargo update + -p`, `--precise`. Add tests that resolve, mutate one dep, and re-resolve.~~ + **DONE at the resolver level** via `pubgrub_update.rs` (8 deterministic + differential tests) and a new `pubgrub_prop.rs` case. All three paths reduce + to `VersionPreferences` at the resolver boundary, so the tests build prefs + from a first resolution and re-resolve both kept and freed, comparing + pubgrub against the default resolver (graph nodes + edges) and SAT. Next: + close the end-to-end gap — drive a real `Cargo.lock` through `ops::resolve` + and the `--precise` registry pinning (see §8), e.g. via a cargo-test + integration test rather than the resolver harness. +4. **Real-world differential testing** via `Eh2406/pubgrub-crates-benchmark` — + resolve many crates.io crates with both resolvers and diff. +5. **Weak-dep + feature-map fidelity** — stress more `dep?/feat` shapes and + confirm the `Resolve.features` map matches the default resolver, not just the + lockfile graph. +6. **Cargo-native error reporting** — *partially done; remainder deliberately + deferred.* The standalone `pubgrub/error.rs` bridge (see §8) covers the + missing/cyclic-feature, missing-dependency, no-candidates (incl. + wrong-version), and dependency-requested-feature-conflict families with + byte-identical text via the v1 renderers. + + **Not pursued (by design):** the *semver* ("all possible versions conflict") + and *links* conflict families — 7 tests total. Their expected text embeds the + **full multi-hop dependency chain** of both the failing package *and* the + competing already-selected package (e.g. `foo → qux → bad` vs `foo → baz → + bad`, each edge with its exact `Dependency`). The default resolver has this + from `ResolverContext::parents` (the real resolution graph); PubGrub's + derivation tree records *incompatibilities*, not that path, so reconstructing + it would be guesswork tuned to one observed tree shape — i.e. overfitting on + the smallest remaining bucket. The right fix is architectural: thread the + actual resolution path through, or design PubGrub-native reporting; not a + tree-shape bridge. Until then these fall back to `DefaultStringReporter`. + + Also still falling back: the offline-mode hint (the provider carries no + `GlobalContext`). +7. **Spurious `[UPDATING]` index refresh.** The provider always queries with a + non-locked wildcard `Dependency`, defeating the `patches.len() == 1 && + dep.is_locked()` short-circuit in `PackageRegistry::query`. This makes + `cargo` print an extra `[UPDATING]`/download line vs. the default resolver — + the bulk of the remaining `patch::` testsuite failures. Resolution is + correct; only the index-access side effect differs. +8. **Performance** — defer until correctness is solid. + +--- + +## 10. Commit history (this branch) + +Newest first. The branch was rebased onto upstream `master` after the +`src/` flattening (rust-lang/cargo#17230, #17231), so all hashes below are +post-rebase; pre-rebase hashes referenced in older notes no longer resolve. + +``` +1d962d2d feat(resolver): Bridge dependency-requested feature conflicts +11bb57a3 refactor(resolver)!: Extract version_conflict_error from activation_error +a0790c64 feat(resolver): Bridge wrong-version errors to Cargo-native text +544a958f docs: Update handoff for patch/replace/cyclic fixes and error bridge +2adfdc99 feat(resolver): Bridge no-candidates errors to Cargo-native text +03e1ad2c refactor(resolver)!: Extract no_candidates_error from activation_error +a032be9d feat(resolver): Add Cargo-native error-reporting bridge for pubgrub +0be1ff1e refactor(resolver): Expose RequirementError for reuse by pubgrub +18d8bf9b fix(resolver): Detect self-referential feature cycles in pubgrub +dd2eb59d fix(resolver): Register [replace] targets as nodes in pubgrub lockfile +c019576a fix(resolver): Track patched source in pubgrub lockfile reconstruction +2a325f3d docs(resolver): Fix broken intra-doc links in the pubgrub module +479dfaba fix(resolver): Read __CARGO_TEST_PUBGRUB via GlobalContext, not std::env +cc424abf style(resolver): Run rustfmt over the pubgrub module and tests +ba89e626 test(resolver): Update -Z help snapshot for the pubgrub-resolver flag +3ffa9692 docs: Record conservative-update verification and full-testsuite survey +0e883888 test(resolver): Add CARGO_TEST_PUBGRUB escape hatch at the dispatch fork +2b121462 test(resolver): Add conservative-update property test for pubgrub +c8fd93d9 test(resolver): Add conservative-update differential tests for pubgrub +0da50059 refactor(resolver): Allow seeding VersionPreferences in the raw resolve helper +f78a9f60 docs: Update handoff doc for cleaner verification methodology +8be9ef27 feat(resolver): Add observable trace when the pubgrub resolver runs +ac7cea9a docs: Record curated-suite validation results for pubgrub resolver +91b8becb test(resolver): Skip exact error-text assertions under pubgrub +66f80090 test(resolver): Allow running the curated suite through pubgrub +c38f1881 docs: Add PubGrub resolver design & handoff doc +d20c932b fix(resolver): Match v1 lock graph for weak dependency features +8979a653 docs(unstable): Document -Zpubgrub-resolver flag +f2427df5 test(resolver): Add pubgrub vs SAT property test +46a70382 fix(resolver): Record feature-agnostic dependency edges in pubgrub lock +cb61ba39 test(resolver): Add SAT-validated pubgrub resolution suite +4c292038 fix(resolver): Seed workspace members into the pubgrub version cache +7b50b495 feat(resolver): Wire up pubgrub resolution and reconstruct Resolve +a2cae053 feat(resolver): Implement pubgrub DependencyProvider over the registry +15ceb3f3 feat(resolver): Add PubGrubPackage encoding for the pubgrub resolver +94a474f8 feat(resolver): Add semver-to-pubgrub VersionSet conversion +eaaccf9c feat(resolver): Add -Zpubgrub-resolver flag and module skeleton +``` + +> Note on history: commit `46a70382` ("feature-agnostic edges") was a wrong +> turn; it is corrected by `d20c932b`. The current `solution.rs`/`provider.rs` +> reflect the corrected (activation-gated + weak-records-edge) behavior. + +--- + +## 11. Quick orientation for the next agent + +- Start in `src/resolver/pubgrub/mod.rs`, then `provider.rs` + (`get_dependencies` is the heart), then `solution.rs`. +- To debug an edge mismatch: instrument `dep_cache.rs::resolve_features` + (default resolver) and `solution.rs` (pubgrub) for a target crate, compare. +- The acceptance command is in §3; remember `rm -f Cargo.lock` each run. +- Before claiming "it works," test **fresh** (no lock) and **diff the full + lockfile**, not just exit codes. + +--- + +## 12. Full-testsuite survey under PubGrub + +Run the entire integration testsuite through PubGrub via the dispatch hook +(§3). Run under **nightly** so the ~376 nightly-gated tests are un-ignored: + +```sh +RUSTUP_TOOLCHAIN=nightly __CARGO_TEST_PUBGRUB=1 cargo +nightly test -p cargo --test testsuite +``` + +> ⚠️ **Methodology note.** The env var must match the dispatch hook exactly +> (`__CARGO_TEST_PUBGRUB`, two leading underscores). An earlier survey used the +> wrong name and so silently ran the *default* resolver, producing a bogus +> "3872 passed, 4 failed". Always confirm the pubgrub path actually ran (e.g. +> a known error-text test should fail) before trusting a survey number. + +Results in this environment (nightly), tracking the correctness/error-reporting +work in §10: + +| Survey | passed | failed | ignored | +|---|---|---|---| +| Before this work (baseline) | 4019 | 233 | 28 | +| After `[patch]`/`[replace]`/cyclic + error bridge | ~4075 | ~177 | 28 | +| After wrong-version (`alt_versions`) bridge | ~4088 | ~164 | 28 | +| After dependency-requested feature-conflict bridge | ~4094 | **~158** | 28 | + +The failed count wobbles by ~1 between runs (≈158–159); the delta is entirely +in env-flaky tests (`artifact_dep::*` cross-compile, an `update::*` timing +case), **not** the resolver — diffing two runs shows only those swap in/out. + +Per-module failure drops (baseline → now): `registry` 36→12, `replace` 20→9, +`package_features` 9→3, `features` 9→3, `package` 4→0, plus `member_errors`, +`generate_lockfile`, `source_replacement`, `features_namespaced` → 0, and +smaller drops across `build`/`directory`/`install`/`path`/`publish`/`update`. +(`patch` stays ~24 — those are the spurious-`[UPDATING]` issue, §9.7.) + +The remaining ~158 failures are dominated by these known, non-correctness +causes: + +1. **Spurious `[UPDATING]` index refresh** (§9.7) — bulk of `patch::`, and a + chunk of `registry`/`offline`/`git`. +2. **Remaining conflict-family error text** (§8, §9.6) — the *semver* ("all + possible versions conflict") and *links* conflicts. These need recovering + *which already-selected package* conflicts from the derivation tree, which is + fragile, so they still fall back. (The missing-feature conflict family is now + byte-matched.) +3. **`metadata`/`build_script`/auth modules** — a mix of output-shape diffs and + env-gated cases not yet individually triaged. + +Both are output/formatting, not misresolution. The 28 still-ignored are +genuinely unavailable (network/container/`hg`/manual-only), not nightly-gated. + +--- + +## 13. Rebasing across an upstream file move + +Upstream flattened `src/` in two PRs +([#17230](https://github.com/rust-lang/cargo/pull/17230): `src/cargo`→`src`, +`src/doc`→`doc`, `src/etc`→`etc`; +[#17231](https://github.com/rust-lang/cargo/pull/17231): elevate +`resolver`/`compiler`/`context` to top-level mods, rename `core`→`workspace`). +A plain `rebase --onto` does **not** survive this. +The technique below did, and is worth reusing if another move lands. + +### Why plain rebase fails + +Git's rename detection relocates *modified* files correctly — it found +`src/resolver/mod.rs` and `src/workspace/features.rs` on its own. +But it will not relocate a **newly added directory**: +our `pubgrub/` module landed back at the dead +`src/cargo/core/resolver/pubgrub/` path. +`merge.directoryRenames=true` did not help (and it was not a rename-limit +issue — 503 renames were detected fine). + +### The recipe + +Rewrite paths *first*, then rebase. +Do the whole thing in a throwaway clone before touching the real repo: + +```sh +git clone --no-local /path/to/repo /tmp/pgtest +``` + +1. **`git filter-branch --index-filter`** over `$FORK_POINT..pubgrub`, rewriting + the index with `git ls-files -s | sed …` to move our paths + (`src/cargo/core/resolver/`→`src/resolver/`, + `src/cargo/core/features.rs`→`src/workspace/features.rs`, + `src/doc/src/reference/unstable.md`→`doc/book/src/reference/unstable.md`). +2. In the **same** filter, rewrite stale module paths *inside the files we own* + (`crate::core::resolver::`→`crate::resolver::`, + `crate::core::summary::`→`crate::workspace::summary::`, + `cargo::core::Resolve`→`cargo::resolver::Resolve`, …). + Folding these into the filter keeps every commit individually buildable, so + `git bisect` still works. +3. **Only then** `git rebase --onto master@origin $FORK_POINT`. + +### The critical constraint (cost real time) + +**Do not rewrite upstream-owned files inside the filter.** +An early attempt also rewrote `src/resolver/{mod,dep_cache,errors}.rs` and +`crates/resolver-tests/src/lib.rs`; that destroys the common ancestor git needs +for a 3-way merge and turned 1 conflict into 5. +Restrict the filter to files we exclusively own, and fix our few inserted lines +in upstream files as a normal follow-up edit. + +One exception is safe: a string that exists in **neither** base is provably ours +alone, so a blanket rewrite cannot corrupt upstream content. +`pub(in crate::core::resolver)` qualified (verify with `git show $BASE:$FILE` +before relying on this). + +### Generated files: regenerate, never hand-merge + +Both remaining conflicts were in generated artifacts: + +- `Cargo.lock` — resolve with `--ours`, then let cargo rewrite it + (`cargo metadata >/dev/null`). It correctly re-adds `pubgrub`, + `version-ranges`, and `priority-queue` while keeping upstream's `getrandom` + bump. +- `tests/testsuite/cargo/z_help/stdout.term.svg` — regenerate with + `SNAPSHOTS=overwrite cargo test -p cargo --test testsuite -- cargo::z_help`. + Taking `--theirs` silently **drops upstream's newly added `-Z` flags**; + regenerating restored 55 lines while keeping our `pubgrub-resolver` entry. + +### Unrelated upstream change caught by the rebase + +Upstream moved hot maps to the `Fx` hasher, so `Resolve::new` now takes +`HashMap<_, _, FxBuildHasher>`. +`solution.rs` must import `crate::util::data_structures::{HashMap, HashSet}` +(not `std::collections`) and construct with `::default()` rather than `::new()`. + +### Verification after the rebase + +Re-run the §3 suites and compare against the pre-rebase numbers — they should +match exactly (they did: 3/2/8/11 for +`pubgrub_graph`/`smoke`/`update`/`validated`, and 37/37 + 28/28 for the curated +suites under `__CARGO_TEST_PUBGRUB=1`). +Also spot-check that mid-branch commits still build, to confirm bisectability. diff --git a/doc/book/src/commands/cargo-fetch.md b/doc/book/src/commands/cargo-fetch.md index 279558343b0..e1c960d2516 100644 --- a/doc/book/src/commands/cargo-fetch.md +++ b/doc/book/src/commands/cargo-fetch.md @@ -19,10 +19,6 @@ file before fetching the dependencies. If `--target` is not specified, then all target dependencies are fetched. -See also the [cargo-prefetch](https://crates.io/crates/cargo-prefetch) -plugin which adds a command to download popular crates. This may be useful if -you plan to use Cargo without a network with the `--offline` flag. - ## OPTIONS ### Fetch options diff --git a/doc/book/src/reference/lints.md b/doc/book/src/reference/lints.md index 4a3f03c5df9..120eedbd158 100644 --- a/doc/book/src/reference/lints.md +++ b/doc/book/src/reference/lints.md @@ -1,6 +1,15 @@ # Lints -Note: [Cargo's linting system is unstable](unstable.md#lintscargo) and can only be used on nightly toolchains +> [!NOTE] +> This chapter is about lints emitted by `cargo` itself. +> +> See [the lints section in the Manifest Format +> chapter](manifest.md#the-lints-section) to configure lint levels for tools +> such as `rustc` or `clippy`. + +> [!WARNING] +> [Cargo's linting system is unstable](unstable.md#lintscargo) and can only be used on nightly +> toolchains. @@ -20,7 +29,6 @@ Note: [Cargo's linting system is unstable](unstable.md#lintscargo) and can only ## Allowed-by-default These lints are all set to the 'allow' level by default. -- [`implicit_minimum_version_req`](#implicit_minimum_version_req) - [`non_kebab_case_features`](#non_kebab_case_features) - [`non_kebab_case_packages`](#non_kebab_case_packages) - [`non_snake_case_features`](#non_snake_case_features) @@ -77,58 +85,6 @@ hint-mostly-unused = true ``` -## `implicit_minimum_version_req` - -- Group: `pedantic` -- Level: `allow` - - -### What it does - -Checks for dependency version requirements -that do not explicitly specify a full `major.minor.patch` version requirement, -such as `serde = "1"` or `serde = "1.0"`. - -This lint currently only applies to caret requirements -(the [default requirements](specifying-dependencies.md#default-requirements)). - -### Why is this bad? - -Version requirements without an explicit full version -can be misleading about the actual minimum supported version. -For example, -`serde = "1"` has an implicit minimum bound of `1.0.0`. -If your code actually requires features from `1.0.219`, -the implicit minimum bound of `1.0.0` gives a false impression about compatibility. - -Specifying the full version helps with: - -- Accurate minimum version documentation -- Better compatibility with `-Z minimal-versions` -- Clearer dependency constraints for consumers - -### Drawbacks - -Even with a fully specified version, -the minimum bound might still be incorrect if untested. -This lint helps make the minimum version requirement explicit -but doesn't guarantee correctness. - -### Example - -```toml -[dependencies] -serde = "1" -``` - -Should be written as a full specific version: - -```toml -[dependencies] -serde = "1.0.219" -``` - - ## `missing_lints_inheritance` - Group: `suspicious` diff --git a/doc/book/src/reference/unstable.md b/doc/book/src/reference/unstable.md index 80599ba093c..6d444787b4a 100644 --- a/doc/book/src/reference/unstable.md +++ b/doc/book/src/reference/unstable.md @@ -2438,3 +2438,20 @@ Support for `resolver.lockfile-path` config field has been stabilized in Rust 1. ## warnings The `build.warnings` config field has been stabilized in Rust 1.97. + +## pubgrub-resolver +* Tracking Issue: [#14930](https://github.com/rust-lang/cargo/issues/14930) + +The `-Zpubgrub-resolver` flag switches Cargo's dependency resolution from the +default hand-rolled backtracking resolver to an experimental resolver built on +the [PubGrub](https://crates.io/crates/pubgrub) algorithm. + +```sh +cargo +nightly -Zpubgrub-resolver generate-lockfile +``` + +This is an experimental, side-by-side implementation intended for evaluation and +correctness comparison against the default resolver. It is not yet feature +complete and should not be relied upon for production lock files. When the flag +is not passed, resolution is completely unaffected. + diff --git a/doc/book/src/reference/workspaces.md b/doc/book/src/reference/workspaces.md index a923e3ebd18..fb1cc6e6efd 100644 --- a/doc/book/src/reference/workspaces.md +++ b/doc/book/src/reference/workspaces.md @@ -105,6 +105,10 @@ should be an array of strings containing directories with `Cargo.toml` files. The `members` list also supports [globs] to match multiple paths, using typical filename glob patterns like `*` and `?`. +**Recommendation:** Keep all member packages in a flat directory (commonly `crates/`) +and use a glob pattern for the `members` field, e.g. +`members = ["crates/*"]`. This minimizes churn in maintaining the `members` list. + The `exclude` key can be used to prevent paths from being included in a workspace. This can be useful if some path dependencies aren't desired to be in the workspace at all, or using a glob pattern and you want to remove a diff --git a/doc/man/cargo-fetch.md b/doc/man/cargo-fetch.md index dc85c9ac9f1..2ae0304b7ce 100644 --- a/doc/man/cargo-fetch.md +++ b/doc/man/cargo-fetch.md @@ -24,10 +24,6 @@ file before fetching the dependencies. If `--target` is not specified, then all target dependencies are fetched. -See also the [cargo-prefetch](https://crates.io/crates/cargo-prefetch) -plugin which adds a command to download popular crates. This may be useful if -you plan to use Cargo without a network with the `--offline` flag. - ## OPTIONS ### Fetch options diff --git a/doc/man/generated_txt/cargo-fetch.txt b/doc/man/generated_txt/cargo-fetch.txt index 0ab31bfef45..d93c20dda2b 100644 --- a/doc/man/generated_txt/cargo-fetch.txt +++ b/doc/man/generated_txt/cargo-fetch.txt @@ -17,11 +17,6 @@ DESCRIPTION If --target is not specified, then all target dependencies are fetched. - See also the cargo-prefetch - plugin which adds a command to download popular crates. This may be - useful if you plan to use Cargo without a network with the --offline - flag. - OPTIONS Fetch options --target triple diff --git a/etc/_cargo b/etc/_cargo index b964c4b2638..06fdb5c4095 100644 --- a/etc/_cargo +++ b/etc/_cargo @@ -89,6 +89,7 @@ _cargo() { '--branch=[branch to use when adding from git]:branch' \ '--git=[specify URL from which to add the crate]:url:_urls' \ '--path=[local filesystem path to crate to add]: :_directories' \ + '(-p --package)'{-p+,--package=}'[specify package to add dependencies to]:package:_cargo_package_names' \ '--rev=[specific commit to use when adding from git]:commit' \ '--tag=[tag to use when adding from git]:tag' \ '--ignore-rust-version[Ignore rust-version specification in packages]' \ diff --git a/etc/man/cargo-fetch.1 b/etc/man/cargo-fetch.1 index 41fa1d60b1e..226ba20e17d 100644 --- a/etc/man/cargo-fetch.1 +++ b/etc/man/cargo-fetch.1 @@ -16,10 +16,6 @@ If the lock file is not available, then this command will generate the lock file before fetching the dependencies. .sp If \fB\-\-target\fR is not specified, then all target dependencies are fetched. -.sp -See also the \fIcargo\-prefetch\fR -plugin which adds a command to download popular crates. This may be useful if -you plan to use Cargo without a network with the \fB\-\-offline\fR flag. .SH "OPTIONS" .SS "Fetch options" .sp diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs index bab475b88e6..b6039f4062f 100644 --- a/src/bin/cargo/cli.rs +++ b/src/bin/cargo/cli.rs @@ -739,18 +739,22 @@ See 'cargo help <>' for more information } fn get_toolchains_from_rustup() -> Vec { - let output = std::process::Command::new("rustup") + let Ok(output) = std::process::Command::new("rustup") .arg("toolchain") .arg("list") .arg("-q") .output() - .unwrap(); + else { + return vec![]; + }; if !output.status.success() { return vec![]; } - let stdout = String::from_utf8(output.stdout).unwrap(); + let Ok(stdout) = String::from_utf8(output.stdout) else { + return vec![]; + }; stdout.lines().map(|line| format!("+{}", line)).collect() } diff --git a/src/compiler/compilation.rs b/src/compiler/compilation.rs index 5e4e335cf1b..7af0e7e4101 100644 --- a/src/compiler/compilation.rs +++ b/src/compiler/compilation.rs @@ -5,6 +5,7 @@ use std::collections::BTreeSet; use std::ffi::{OsStr, OsString}; use std::path::Path; use std::path::PathBuf; +use std::rc::Rc; use cargo_platform::CfgExpr; use cargo_util::{ProcessBuilder, paths}; @@ -117,6 +118,9 @@ pub struct Compilation<'gctx> { /// See `-Zrustdoc-mergeable-info` for more. pub rustdoc_fingerprints: Option>, + /// Extra flags to pass to rustdoc for each host or target. + pub rustdocflags: HashMap>, + /// The target host triple. pub host: String, @@ -145,6 +149,12 @@ impl<'gctx> Compilation<'gctx> { let primary_rustc_process = bcx.build_config.primary_unit_rustc.clone(); let rustc_workspace_wrapper_process = bcx.rustc().workspace_process(); let host = bcx.host_triple().to_string(); + let sysroot_target_libdir = get_sysroot_target_libdir(bcx)?; + let rustdocflags = bcx + .all_kinds + .iter() + .map(|&kind| (kind, bcx.target_data.info(kind).rustdocflags.clone())) + .collect(); // When `target-applies-to-host=false`, and without `--target`, // there will be only `CompileKind::Host` in requested_kinds. @@ -183,7 +193,7 @@ impl<'gctx> Compilation<'gctx> { native_dirs: BTreeSet::new(), root_output: HashMap::default(), deps_output: HashMap::default(), - sysroot_target_libdir: get_sysroot_target_libdir(bcx)?, + sysroot_target_libdir, tests: Vec::new(), binaries: Vec::new(), cdylibs: Vec::new(), @@ -191,6 +201,7 @@ impl<'gctx> Compilation<'gctx> { extra_env: HashMap::default(), to_doc_test: Vec::new(), rustdoc_fingerprints: None, + rustdocflags, gctx: bcx.gctx, host, rustc_process, diff --git a/src/compiler/fingerprint/rustdoc.rs b/src/compiler/fingerprint/rustdoc.rs index 42e7d4640fd..11336b55ee6 100644 --- a/src/compiler/fingerprint/rustdoc.rs +++ b/src/compiler/fingerprint/rustdoc.rs @@ -25,13 +25,13 @@ struct RustdocFingerprintJson { /// Structure used to deal with Rustdoc fingerprinting /// /// This is important because the `.js`/`.html` & `.css` files -/// that are generated by Rustc don't have any versioning yet +/// that are generated by Rustdoc don't have any versioning yet /// (see ). /// Therefore, we can end up with weird bugs and behaviours /// if we mix different versions of these files. /// /// We need to make sure that if there were any previous docs already compiled, -/// they were compiled with the same Rustc version that we're currently using. +/// they were compiled with the same Rustdoc version that we're currently using. /// Otherwise we must remove the `doc/` folder and compile again forcing a rebuild. #[derive(Debug)] pub struct RustdocFingerprint { diff --git a/src/compiler/mod.rs b/src/compiler/mod.rs index 18b5179f13a..3f406a15e93 100644 --- a/src/compiler/mod.rs +++ b/src/compiler/mod.rs @@ -1858,6 +1858,13 @@ fn add_dep_arg<'a, 'b: 'a>( continue; } map.insert(&dep.unit, build_runner.files().deps_dir(&dep.unit)); + + // Proc macros are statically linked, so when including a proc-macro dependency we can skip + // adding it's dependencies. Note that we still do add them when we are compiling the + // proc-macro itself. + if dep.unit.target.proc_macro() { + continue; + } add_dep_arg(map, build_runner, &dep.unit); } } diff --git a/src/compiler/timings/mod.rs b/src/compiler/timings/mod.rs index 0d0b0f7b0d8..3649e28a3bb 100644 --- a/src/compiler/timings/mod.rs +++ b/src/compiler/timings/mod.rs @@ -274,7 +274,7 @@ impl<'gctx> Timings<'gctx> { let filename = timings_path.join(format!("cargo-timing-{run_id}.html")); let mut f = BufWriter::new(paths::create(&filename)?); - let mut ctx = prepare_context(logs.into_iter(), run_id)?; + let mut ctx = prepare_context(logs.into_iter(), run_id, false)?; ctx.error = error; ctx.cpu_usage = &self.cpu_usage; report::write_html(ctx, &mut f)?; diff --git a/src/context/mod.rs b/src/context/mod.rs index 15674c8b797..00131cfba97 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -134,7 +134,6 @@ mod environment; use environment::Env; mod schema; -use crate::workspace::features::EmbedMetadata; pub use schema::*; /// Helper macro for creating typed access methods. @@ -1261,11 +1260,7 @@ impl GlobalContext { } pub fn should_embed_metadata(&self) -> bool { - match self.cli_unstable().embed_metadata { - EmbedMetadata::Embed => true, - EmbedMetadata::DoNotEmbed => false, - EmbedMetadata::Unset => true, - } + self.cli_unstable().embed_metadata.unwrap_or(true) } pub fn network_allowed(&self) -> bool { diff --git a/src/diagnostics/rules/implicit_minimum_version_req.rs b/src/diagnostics/rules/implicit_minimum_version_req.rs deleted file mode 100644 index 786358d29d5..00000000000 --- a/src/diagnostics/rules/implicit_minimum_version_req.rs +++ /dev/null @@ -1,361 +0,0 @@ -use crate::util::data_structures::HashMap; -use std::path::Path; - -use cargo_platform::Platform; -use cargo_util_schemas::manifest::TomlDependency; -use cargo_util_terminal::report::AnnotationKind; -use cargo_util_terminal::report::Group; -use cargo_util_terminal::report::Level; -use cargo_util_terminal::report::Origin; -use cargo_util_terminal::report::Patch; -use cargo_util_terminal::report::Snippet; -use toml::de::DeValue; -use tracing::instrument; - -use super::PEDANTIC; -use crate::CargoResult; -use crate::GlobalContext; -use crate::diagnostics::Lint; -use crate::diagnostics::LintLevel; -use crate::diagnostics::LintLevelProduct; -use crate::diagnostics::LintLevelSource; -use crate::diagnostics::ScopedDiagnosticStats; -use crate::diagnostics::get_key_value; -use crate::diagnostics::workspace_rel_path; -use crate::util::OptVersionReq; -use crate::workspace::Manifest; -use crate::workspace::MaybePackage; -use crate::workspace::Package; -use crate::workspace::Workspace; - -pub static LINT: &Lint = &Lint { - name: "implicit_minimum_version_req", - desc: "dependency version requirement without an explicit minimum version", - primary_group: &PEDANTIC, - msrv: None, - feature_gate: None, - docs: Some( - r#" -### What it does - -Checks for dependency version requirements -that do not explicitly specify a full `major.minor.patch` version requirement, -such as `serde = "1"` or `serde = "1.0"`. - -This lint currently only applies to caret requirements -(the [default requirements](specifying-dependencies.md#default-requirements)). - -### Why is this bad? - -Version requirements without an explicit full version -can be misleading about the actual minimum supported version. -For example, -`serde = "1"` has an implicit minimum bound of `1.0.0`. -If your code actually requires features from `1.0.219`, -the implicit minimum bound of `1.0.0` gives a false impression about compatibility. - -Specifying the full version helps with: - -- Accurate minimum version documentation -- Better compatibility with `-Z minimal-versions` -- Clearer dependency constraints for consumers - -### Drawbacks - -Even with a fully specified version, -the minimum bound might still be incorrect if untested. -This lint helps make the minimum version requirement explicit -but doesn't guarantee correctness. - -### Example - -```toml -[dependencies] -serde = "1" -``` - -Should be written as a full specific version: - -```toml -[dependencies] -serde = "1.0.219" -``` -"#, - ), -}; - -#[instrument(skip_all)] -pub(crate) fn lint_package( - ws: &Workspace<'_>, - pkg: &Package, - manifest_path: &Path, - level: LintLevelProduct, - pkg_stats: &mut ScopedDiagnosticStats<'_>, - gctx: &GlobalContext, -) -> CargoResult<()> { - let LintLevelProduct { - level: lint_level, - source, - } = level; - - let manifest_path = workspace_rel_path(ws, manifest_path); - - let manifest = pkg.manifest(); - - let document = manifest.document(); - let contents = manifest.contents(); - let target_key_for_platform = target_key_for_platform(&manifest); - - let mut emit_source = true; - for dep in manifest.dependencies().iter() { - let version_req = dep.version_req(); - let Some(suggested_req) = get_suggested_version_req(&version_req) else { - continue; - }; - - let name_in_toml = dep.name_in_toml().as_str(); - let key_path = - if let Some(cfg) = dep.platform().and_then(|p| target_key_for_platform.get(p)) { - &["target", &cfg, dep.kind().kind_table(), name_in_toml][..] - } else { - &[dep.kind().kind_table(), name_in_toml][..] - }; - - let Some(report) = report( - lint_level, - source, - contents, - document, - key_path, - &manifest_path, - &suggested_req, - emit_source, - ) else { - continue; - }; - - if emit_source { - emit_source = false; - } - - pkg_stats.record_lint(lint_level); - gctx.shell().print_report(&report, lint_level.force())?; - } - - Ok(()) -} - -#[instrument(skip_all)] -pub(crate) fn lint_workspace( - ws: &Workspace<'_>, - maybe_pkg: &MaybePackage, - manifest_path: &Path, - level: LintLevelProduct, - pkg_stats: &mut ScopedDiagnosticStats<'_>, - gctx: &GlobalContext, -) -> CargoResult<()> { - let LintLevelProduct { - level: lint_level, - source, - } = level; - - let manifest_path = workspace_rel_path(ws, manifest_path); - - let document = maybe_pkg.document(); - let contents = maybe_pkg.contents(); - let toml = match maybe_pkg { - MaybePackage::Package(p) => p.manifest().normalized_toml(), - MaybePackage::Virtual(vm) => vm.normalized_toml(), - }; - let dep_iter = toml - .workspace - .as_ref() - .and_then(|ws| ws.dependencies.as_ref()) - .into_iter() - .flat_map(|deps| deps.iter()) - .map(|(name, dep)| { - let name = name.as_str(); - let ver = match dep { - TomlDependency::Simple(ver) => ver, - TomlDependency::Detailed(detailed) => { - let Some(ver) = detailed.version.as_ref() else { - return (name, OptVersionReq::Any); - }; - ver - } - }; - let req = semver::VersionReq::parse(ver) - .map(Into::into) - .unwrap_or(OptVersionReq::Any); - (name, req) - }); - - let mut emit_source = true; - for (name_in_toml, version_req) in dep_iter { - let Some(suggested_req) = get_suggested_version_req(&version_req) else { - continue; - }; - - let key_path = ["workspace", "dependencies", name_in_toml]; - - let Some(report) = report( - lint_level, - source, - contents, - document, - &key_path, - &manifest_path, - &suggested_req, - emit_source, - ) else { - continue; - }; - - if emit_source { - emit_source = false; - } - - pkg_stats.record_lint(lint_level); - gctx.shell().print_report(&report, lint_level.force())?; - } - - Ok(()) -} - -pub(crate) fn span_of_version_req<'doc>( - document: &'doc toml::Spanned>, - path: &[&str], -) -> Option> { - let (_key, value) = get_key_value(document, path)?; - - match value.as_ref() { - DeValue::String(_) => Some(value.span()), - DeValue::Table(map) if map.get("workspace").is_some() => { - // We only lint non-workspace-inherited dependencies - None - } - DeValue::Table(map) => { - let Some(v) = map.get("version") else { - panic!("version must be specified or workspace-inherited"); - }; - Some(v.span()) - } - _ => unreachable!("dependency must be string or table"), - } -} - -fn report<'a>( - lint_level: LintLevel, - source: LintLevelSource, - contents: Option<&'a str>, - document: Option<&toml::Spanned>>, - key_path: &[&str], - manifest_path: &str, - suggested_req: &str, - emit_source: bool, -) -> Option<[Group<'a>; 2]> { - let level = lint_level.to_diagnostic_level(); - let emitted_source = LINT.emitted_source(lint_level, source); - let replacement = format!(r#""{suggested_req}""#); - let label = "missing full version components"; - let secondary_title = "consider specifying full `major.minor.patch` version components"; - - let mut desc = Group::with_title(level.primary_title(LINT.desc)); - let mut help = Group::with_title(Level::HELP.secondary_title(secondary_title)); - - if let Some(document) = document - && let Some(contents) = contents - { - let Some(span) = span_of_version_req(document, key_path) else { - return None; - }; - desc = desc.element( - Snippet::source(contents) - .path(manifest_path.to_owned()) - .annotation(AnnotationKind::Primary.span(span.clone()).label(label)), - ); - - help = help.element(Snippet::source(contents).patch(Patch::new(span.clone(), replacement))); - } else { - desc = desc.element(Origin::path(manifest_path.to_owned())); - } - - if emit_source { - desc = desc.element(Level::NOTE.message(emitted_source)); - } - - Some([desc, help]) -} - -fn get_suggested_version_req(req: &OptVersionReq) -> Option { - use semver::Op; - let OptVersionReq::Req(req) = req else { - return None; - }; - let mut has_suggestions = false; - let mut comparators = Vec::new(); - - for mut cmp in req.comparators.iter().cloned() { - match cmp.op { - Op::Caret | Op::GreaterEq => { - // Only focus on comparator that has only `major` or `major.minor` - if cmp.minor.is_some() && cmp.patch.is_some() { - comparators.push(cmp); - continue; - } else { - has_suggestions = true; - cmp.minor.get_or_insert(0); - cmp.patch.get_or_insert(0); - comparators.push(cmp); - } - } - Op::Exact | Op::Tilde | Op::Wildcard | Op::Greater | Op::Less | Op::LessEq => { - comparators.push(cmp); - continue; - } - _ => panic!("unknown comparator in `{cmp}`"), - } - } - - if !has_suggestions { - return None; - } - - // This is a lossy suggestion that - // - // * extra spaces are removed - // * caret operator `^` is stripped - let mut suggestion = String::new(); - - for cmp in &comparators { - if !suggestion.is_empty() { - suggestion.push_str(", "); - } - let s = cmp.to_string(); - - if cmp.op == Op::Caret { - suggestion.push_str(s.strip_prefix('^').unwrap_or(&s)); - } else { - suggestion.push_str(&s); - } - } - - Some(suggestion) -} - -/// A map from parsed `Platform` to their original TOML key strings. -/// This is needed for constructing TOML key paths in diagnostics. -/// -/// This is only relevant for package dependencies. -fn target_key_for_platform(manifest: &Manifest) -> HashMap { - manifest - .normalized_toml() - .target - .as_ref() - .map(|map| { - map.keys() - .map(|k| (k.parse().expect("already parsed"), k.clone())) - .collect() - }) - .unwrap_or_default() -} diff --git a/src/diagnostics/rules/mod.rs b/src/diagnostics/rules/mod.rs index 8640a79869d..1d0731f713a 100644 --- a/src/diagnostics/rules/mod.rs +++ b/src/diagnostics/rules/mod.rs @@ -1,7 +1,6 @@ mod blanket_hint_mostly_unused; mod deferred_parse_diagnostics; mod im_a_teapot; -mod implicit_minimum_version_req; mod missing_lints_features; mod missing_lints_inheritance; mod non_kebab_case_bins; @@ -54,10 +53,6 @@ pub const PARSE_PASS_RULES: &[ParsePassRule<'static>] = &[ rule: unused_workspace_package_fields::lint_workspace, lint: unused_workspace_package_fields::LINT, }, - ParsePassRule::LintWorkspace { - rule: implicit_minimum_version_req::lint_workspace, - lint: implicit_minimum_version_req::LINT, - }, // `warn` ParsePassRule::LintPackage { rule: missing_lints_inheritance::lint_package, @@ -84,10 +79,6 @@ pub const PARSE_PASS_RULES: &[ParsePassRule<'static>] = &[ lint: im_a_teapot::LINT, }, // `allow` - ParsePassRule::LintPackage { - rule: implicit_minimum_version_req::lint_package, - lint: implicit_minimum_version_req::LINT, - }, ParsePassRule::LintPackage { rule: non_kebab_case_features::lint_package, lint: non_kebab_case_features::LINT, @@ -108,7 +99,6 @@ pub const PARSE_PASS_RULES: &[ParsePassRule<'static>] = &[ pub static LINTS: &[&crate::diagnostics::Lint] = &[ blanket_hint_mostly_unused::LINT, - implicit_minimum_version_req::LINT, im_a_teapot::LINT, missing_lints_inheritance::LINT, non_kebab_case_bins::LINT, diff --git a/src/diagnostics/rules/unused_dependencies.rs b/src/diagnostics/rules/unused_dependencies.rs index 312d8d09939..1016fce3c8b 100644 --- a/src/diagnostics/rules/unused_dependencies.rs +++ b/src/diagnostics/rules/unused_dependencies.rs @@ -1,6 +1,7 @@ +use std::collections::VecDeque; use std::path::Path; -use crate::util::data_structures::IndexMap; +use crate::util::data_structures::{HashSet, IndexMap}; use cargo_util_schemas::manifest; use cargo_util_schemas::manifest::TomlPackageBuild; use cargo_util_terminal::report::AnnotationKind; @@ -366,7 +367,8 @@ fn is_transitive_dep( seen_units: &Vec, bcx: &BuildContext<'_, '_>, ) -> bool { - let mut queue = std::collections::VecDeque::new(); + let mut queue = VecDeque::new(); + let mut visited: HashSet<&Unit> = HashSet::default(); for root_unit in seen_units { for unit_dep in &bcx.unit_graph[root_unit] { if root_unit.pkg.package_id() == unit_dep.unit.pkg.package_id() { @@ -375,7 +377,9 @@ fn is_transitive_dep( if unit_dep.unit == *direct_dep_unit { continue; } - queue.push_back(&unit_dep.unit); + if visited.insert(&unit_dep.unit) { + queue.push_back(&unit_dep.unit); + } } } @@ -384,7 +388,9 @@ fn is_transitive_dep( if unit_dep.unit == *direct_dep_unit { return true; } - queue.push_back(&unit_dep.unit); + if visited.insert(&unit_dep.unit) { + queue.push_back(&unit_dep.unit); + } } } diff --git a/src/ops/cargo_doc.rs b/src/ops/cargo_doc.rs index 9de066dcf45..f088ae8ad33 100644 --- a/src/ops/cargo_doc.rs +++ b/src/ops/cargo_doc.rs @@ -158,6 +158,7 @@ fn merge_cross_crate_info(ws: &Workspace<'_>, compilation: &Compilation<'_>) -> cmd.arg("-o") .arg(rustdoc_artifact_dir.as_path_unlocked()) .arg("-Zunstable-options"); + cmd.args(&compilation.rustdocflags[kind]); for parts_dir in doc_parts_dirs { let mut include_arg = OsString::from("--read-doc-meta-dir="); include_arg.push(parts_dir); diff --git a/src/ops/cargo_report/timings.rs b/src/ops/cargo_report/timings.rs index daa2480a101..3ac97963e87 100644 --- a/src/ops/cargo_report/timings.rs +++ b/src/ops/cargo_report/timings.rs @@ -46,6 +46,8 @@ struct UnitEntry { data: UnitData, sections: IndexMap, rmeta_time: Option, + /// Whether the job queue actually ran this unit. + started: bool, } pub fn report_timings( @@ -88,7 +90,7 @@ pub fn report_timings( None } }); - let ctx = prepare_context(iter, &run_id) + let ctx = prepare_context(iter, &run_id, true) .with_context(|| format!("failed to analyze log at `{}`", log.display()))?; // If we are in a workspace, @@ -130,7 +132,11 @@ pub fn report_timings( Ok(()) } -pub(crate) fn prepare_context(log: I, run_id: &RunId) -> CargoResult> +pub(crate) fn prepare_context( + log: I, + run_id: &RunId, + error_if_no_units: bool, +) -> CargoResult> where I: Iterator, { @@ -230,6 +236,7 @@ where data, sections: IndexMap::default(), rmeta_time: None, + started: false, }, ); } @@ -241,7 +248,10 @@ where LogMessage::UnitStarted { index, elapsed } => { units .entry(index) - .and_modify(|unit| unit.data.start = elapsed) + .and_modify(|unit| { + unit.data.start = elapsed; + unit.started = true; + }) .or_insert_with(|| { unreachable!("unit {index} must have been registered first") }); @@ -331,6 +341,16 @@ where } } + // A build can legitimately have no units at all, such as an idle + // `cargo test` with `test = false` and `doctest = false`, + // so `--timings` always renders its report. + // `cargo report timings` instead treats a unit-less + // log as corrupted or truncated and errors out. + // See https://github.com/rust-lang/cargo/issues/17212. + if error_if_no_units && units.is_empty() { + anyhow::bail!("no timing data found in log"); + } + ctx.root_units = { let mut root_map: IndexMap<_, Vec<_>> = IndexMap::default(); for index in requested_units { @@ -356,15 +376,34 @@ where .collect() }; + let started: HashSet = units + .iter() + .filter(|(_, entry)| entry.started) + .map(|(index, _)| *index) + .collect(); + + // Units that never started, like fresh units, and + // units that were run outside the job queue are + // excluded so they don't show up as zero-duration rows. + // See https://github.com/rust-lang/cargo/issues/17212. let unit_data: Vec<_> = units .into_values() + .filter(|entry| entry.started) .map( |UnitEntry { target: _, mut data, sections, rmeta_time, + started: _, }| { + // A finished unit can unblock units that never started, like + // fresh units. Drop them so rows never reference units absent + // from the report. + data.unblocked_units.retain(|index| started.contains(index)); + data.unblocked_rmeta_units + .retain(|index| started.contains(index)); + // Post-processing for compilation sections we've collected so far. data.sections = aggregate_sections(sections, data.duration, rmeta_time); data.start = round_to_centisecond(data.start); @@ -375,10 +414,6 @@ where .sorted_unstable_by(|a, b| a.start.partial_cmp(&b.start).unwrap()) .collect(); - if unit_data.is_empty() { - anyhow::bail!("no timing data found in log"); - } - ctx.unit_data = unit_data; ctx.concurrency = compute_concurrency(&ctx.unit_data); ctx.requested_targets = platform_targets.into_iter().sorted_unstable().collect(); diff --git a/src/resolver/dep_cache.rs b/src/resolver/dep_cache.rs index f2a928fe231..b97e99309b2 100644 --- a/src/resolver/dep_cache.rs +++ b/src/resolver/dep_cache.rs @@ -478,7 +478,12 @@ struct Requirements<'a> { /// /// This will later be converted to an `ActivateError` depending on whether or /// not this is a dependency or a root package. -enum RequirementError { +/// +/// Exposed to the `pubgrub` resolver so its error-reporting bridge can reuse +/// [`RequirementError::into_activate_error`] verbatim, keeping the two +/// resolvers' messages byte-identical. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(in crate::resolver) enum RequirementError { /// The package does not have the requested feature. MissingFeature(InternedString), /// The package does not have the requested dependency. @@ -572,8 +577,25 @@ impl Requirements<'_> { } } +impl std::fmt::Display for RequirementError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + RequirementError::MissingFeature(feat) => write!(f, "no feature `{feat}`"), + RequirementError::MissingDependency(dep) => write!(f, "no dependency `{dep}`"), + RequirementError::Cycle(feat) => write!( + f, + "cyclic feature dependency: feature `{feat}` depends on itself" + ), + } + } +} + impl RequirementError { - fn into_activate_error(self, parent: Option, summary: &Summary) -> ActivateError { + pub(in crate::resolver) fn into_activate_error( + self, + parent: Option, + summary: &Summary, + ) -> ActivateError { match self { RequirementError::MissingFeature(feat) => { let deps: Vec<_> = summary diff --git a/src/resolver/errors.rs b/src/resolver/errors.rs index 69fadfd69a5..433ef37b714 100644 --- a/src/resolver/errors.rs +++ b/src/resolver/errors.rs @@ -1,7 +1,9 @@ use std::fmt; use std::fmt::Write as _; +use std::path::{Path, PathBuf}; use crate::sources::IndexSummary; +use crate::sources::path::RecursivePathSource; use crate::sources::source::QueryKind; use crate::util::edit_distance::{closest, edit_distance}; use crate::util::errors::CargoResult; @@ -82,158 +84,228 @@ pub(super) fn activation_error( candidates: &[Summary], gctx: Option<&GlobalContext>, ) -> ResolveError { - let to_resolve_err = |err| { - ResolveError::new( - err, - resolver_ctx - .parents - .path_to_bottom(&parent.package_id()) - .into_iter() - .map(|(node, _)| node) - .cloned() - .collect(), - ) - }; - if !candidates.is_empty() { - let mut msg = format!("failed to select a version for `{}`.", dep.package_name()); - msg.push_str("\n ... required by "); - msg.push_str(&describe_path_in_context( - resolver_ctx, - &parent.package_id(), - )); - - msg.push_str("\nversions that meet the requirements `"); - msg.push_str(&dep.version_req().to_string()); - msg.push_str("` "); - - if let Some(v) = dep.version_req().locked_version() { - msg.push_str("(locked to "); - msg.push_str(&v.to_string()); - msg.push_str(") "); - } - - msg.push_str("are: "); - msg.push_str( - &candidates - .iter() - .map(|v| v.version()) - .map(|v| v.to_string()) - .collect::>() - .join(", "), + let package_path = resolver_ctx + .parents + .path_to_bottom(&parent.package_id()) + .into_iter() + .map(|(node, _)| node) + .cloned() + .collect(); + let required_by = describe_path_in_context(resolver_ctx, &parent.package_id()); + // Pre-render the dependency-chain description for every conflicting + // package, since the shared helper has no `ResolverContext`. + let conflict_paths = conflicting_activations + .keys() + .map(|p| (*p, describe_path_in_context(resolver_ctx, p))) + .collect(); + return version_conflict_error( + dep, + candidates, + conflicting_activations, + package_path, + &required_by, + &conflict_paths, ); + } - let mut conflicting_activations: Vec<_> = conflicting_activations.iter().collect(); - conflicting_activations.sort_unstable(); - // This is reversed to show the newest versions first. I don't know if there is - // a strong reason to do this, but that is how the code previously worked - // (see https://github.com/rust-lang/cargo/pull/5037) and I don't feel like changing it. - conflicting_activations.reverse(); - // Flag used for grouping all semver errors together. - let mut has_semver = false; + // We didn't actually find any candidates, so we need to give an error + // message that nothing was found. The body is shared with the PubGrub + // resolver's error bridge, which reconstructs the path differently. + let required_by = describe_path_in_context(resolver_ctx, &parent.package_id()); + let package_path = resolver_ctx + .parents + .path_to_bottom(&parent.package_id()) + .into_iter() + .map(|(node, _)| node) + .cloned() + .collect(); + no_candidates_error( + registry, + dep, + version_prefs, + package_path, + &required_by, + gctx, + ) +} - for (p, r) in &conflicting_activations { - match r { - ConflictReason::Semver => { - has_semver = true; - } - ConflictReason::Links(link) => { - msg.push_str("\n\npackage `"); - msg.push_str(&*dep.package_name()); - msg.push_str("` links to the native library `"); - msg.push_str(link); - msg.push_str("`, but it conflicts with a previous package which links to `"); - msg.push_str(link); - msg.push_str("` as well:\n"); - msg.push_str(&describe_path_in_context(resolver_ctx, p)); - msg.push_str("\nnote: only one package in the dependency graph may specify the same links value to ensure that only one copy of a native library is linked in the final binary"); - msg.push_str("\nfor more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links"); - msg.push_str("\nhelp: try to adjust your dependencies so that only one package uses the `links = \""); - msg.push_str(link); - msg.push_str("\"` value"); - } - ConflictReason::MissingFeature(feature) => { - msg.push_str("\n\npackage `"); - msg.push_str(&*p.name()); - msg.push_str("` depends on `"); - msg.push_str(&*dep.package_name()); - msg.push_str("` with feature `"); - msg.push_str(feature); - msg.push_str("` but `"); - msg.push_str(&*dep.package_name()); - msg.push_str("` does not have that feature.\n"); - let latest = candidates.last().expect("in the non-empty branch"); - if let Some(closest) = closest(feature, latest.features().keys(), |k| k) { - msg.push_str("help: there is a feature `"); - msg.push_str(closest); - msg.push_str("` with a similar name\n"); - } else if !latest.features().is_empty() { - let mut features: Vec<_> = - latest.features().keys().map(|f| f.as_str()).collect(); - features.sort(); - msg.push_str("help: available features: "); - msg.push_str(&features.join(", ")); - msg.push_str("\n"); - } - // p == parent so the full path is redundant. - } - ConflictReason::RequiredDependencyAsFeature(feature) => { - msg.push_str("\n\npackage `"); - msg.push_str(&*p.name()); - msg.push_str("` depends on `"); - msg.push_str(&*dep.package_name()); - msg.push_str("` with feature `"); - msg.push_str(feature); - msg.push_str("` but `"); - msg.push_str(&*dep.package_name()); - msg.push_str("` does not have that feature.\n"); - msg.push_str( - "note: a required dependency with that name exists, \ - but only optional dependencies can be used as features.\n", - ); - // p == parent so the full path is redundant. +/// Build the "candidates exist but conflict" resolver error: versions of `dep` +/// were found, but each conflicts with an already-selected package (semver +/// clash, `links` collision, or a requested feature the dependency lacks). +/// +/// Split out of [`activation_error`] so the PubGrub resolver's error bridge can +/// reuse the exact same rendering. The caller supplies the resolved +/// `package_path` (for [`ResolveError`]), the pre-rendered `required_by` +/// dependency-chain description for the parent, and `conflict_paths` mapping +/// each conflicting [`PackageId`] to its rendered chain (used by the `Links` +/// and `Semver` arms), since the two resolvers recover those differently. +pub(in crate::resolver) fn version_conflict_error( + dep: &Dependency, + candidates: &[Summary], + conflicting_activations: &ConflictMap, + package_path: Vec, + required_by: &str, + conflict_paths: &std::collections::HashMap, +) -> ResolveError { + let to_resolve_err = |err| ResolveError::new(err, package_path.clone()); + + let mut msg = format!("failed to select a version for `{}`.", dep.package_name()); + msg.push_str("\n ... required by "); + msg.push_str(required_by); + + msg.push_str("\nversions that meet the requirements `"); + msg.push_str(&dep.version_req().to_string()); + msg.push_str("` "); + + if let Some(v) = dep.version_req().locked_version() { + msg.push_str("(locked to "); + msg.push_str(&v.to_string()); + msg.push_str(") "); + } + + msg.push_str("are: "); + msg.push_str( + &candidates + .iter() + .map(|v| v.version()) + .map(|v| v.to_string()) + .collect::>() + .join(", "), + ); + + let mut conflicting_activations: Vec<_> = conflicting_activations.iter().collect(); + conflicting_activations.sort_unstable(); + // This is reversed to show the newest versions first. I don't know if there is + // a strong reason to do this, but that is how the code previously worked + // (see https://github.com/rust-lang/cargo/pull/5037) and I don't feel like changing it. + conflicting_activations.reverse(); + // Flag used for grouping all semver errors together. + let mut has_semver = false; + + for (p, r) in &conflicting_activations { + match r { + ConflictReason::Semver => { + has_semver = true; + } + ConflictReason::Links(link) => { + msg.push_str("\n\npackage `"); + msg.push_str(&*dep.package_name()); + msg.push_str("` links to the native library `"); + msg.push_str(link); + msg.push_str("`, but it conflicts with a previous package which links to `"); + msg.push_str(link); + msg.push_str("` as well:\n"); + if let Some(path) = conflict_paths.get(p) { + msg.push_str(path); } - ConflictReason::NonImplicitDependencyAsFeature(feature) => { - msg.push_str("\n\npackage `"); - msg.push_str(&*p.name()); - msg.push_str("` depends on `"); - msg.push_str(&*dep.package_name()); - msg.push_str("` with feature `"); - msg.push_str(feature); - msg.push_str("` but `"); - msg.push_str(&*dep.package_name()); - msg.push_str("` does not have that feature.\n"); - msg.push_str( - "note: an optional dependency with that name exists, \ - but that dependency uses the \"dep:\" \ - syntax in the features table, so it does not have an \ - implicit feature with that name.\n", - ); - // p == parent so the full path is redundant. + msg.push_str("\nnote: only one package in the dependency graph may specify the same links value to ensure that only one copy of a native library is linked in the final binary"); + msg.push_str("\nfor more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links"); + msg.push_str("\nhelp: try to adjust your dependencies so that only one package uses the `links = \""); + msg.push_str(link); + msg.push_str("\"` value"); + } + ConflictReason::MissingFeature(feature) => { + msg.push_str("\n\npackage `"); + msg.push_str(&*p.name()); + msg.push_str("` depends on `"); + msg.push_str(&*dep.package_name()); + msg.push_str("` with feature `"); + msg.push_str(feature); + msg.push_str("` but `"); + msg.push_str(&*dep.package_name()); + msg.push_str("` does not have that feature.\n"); + let latest = candidates.last().expect("in the non-empty branch"); + if let Some(closest) = closest(feature, latest.features().keys(), |k| k) { + msg.push_str("help: there is a feature `"); + msg.push_str(closest); + msg.push_str("` with a similar name\n"); + } else if !latest.features().is_empty() { + let mut features: Vec<_> = + latest.features().keys().map(|f| f.as_str()).collect(); + features.sort(); + msg.push_str("help: available features: "); + msg.push_str(&features.join(", ")); + msg.push_str("\n"); } + // p == parent so the full path is redundant. + } + ConflictReason::RequiredDependencyAsFeature(feature) => { + msg.push_str("\n\npackage `"); + msg.push_str(&*p.name()); + msg.push_str("` depends on `"); + msg.push_str(&*dep.package_name()); + msg.push_str("` with feature `"); + msg.push_str(feature); + msg.push_str("` but `"); + msg.push_str(&*dep.package_name()); + msg.push_str("` does not have that feature.\n"); + msg.push_str( + "note: a required dependency with that name exists, \ + but only optional dependencies can be used as features.\n", + ); + // p == parent so the full path is redundant. + } + ConflictReason::NonImplicitDependencyAsFeature(feature) => { + msg.push_str("\n\npackage `"); + msg.push_str(&*p.name()); + msg.push_str("` depends on `"); + msg.push_str(&*dep.package_name()); + msg.push_str("` with feature `"); + msg.push_str(feature); + msg.push_str("` but `"); + msg.push_str(&*dep.package_name()); + msg.push_str("` does not have that feature.\n"); + msg.push_str( + "note: an optional dependency with that name exists, \ + but that dependency uses the \"dep:\" \ + syntax in the features table, so it does not have an \ + implicit feature with that name.\n", + ); + // p == parent so the full path is redundant. } } + } - if has_semver { - // Group these errors together. - msg.push_str("\n\nall possible versions conflict with previously selected packages"); - for (p, r) in &conflicting_activations { - if let ConflictReason::Semver = r { - msg.push_str("\n\n previously selected "); - msg.push_str(&describe_path_in_context(resolver_ctx, p)); + if has_semver { + // Group these errors together. + msg.push_str("\n\nall possible versions conflict with previously selected packages"); + for (p, r) in &conflicting_activations { + if let ConflictReason::Semver = r { + msg.push_str("\n\n previously selected "); + if let Some(path) = conflict_paths.get(p) { + msg.push_str(path); } } } + } - msg.push_str("\n\nfailed to select a version for `"); - msg.push_str(&*dep.package_name()); - msg.push_str("` which could resolve this conflict"); + msg.push_str("\n\nfailed to select a version for `"); + msg.push_str(&*dep.package_name()); + msg.push_str("` which could resolve this conflict"); - return to_resolve_err(anyhow::format_err!("{}", msg)); - } + to_resolve_err(anyhow::format_err!("{}", msg)) +} + +/// Build the "no candidates found" resolver error (no version of `dep` exists, +/// is yanked, has a typo'd name, etc.). +/// +/// Split out of [`activation_error`] so the PubGrub resolver's error bridge can +/// reuse the exact same message rendering. The caller supplies the resolved +/// `package_path` (for [`ResolveError`]) and the pre-rendered `required_by` +/// dependency-chain description, since the two resolvers recover those +/// differently. `version_prefs` is used to flag candidates rejected for being +/// newer than `min-publish-age`. +pub(in crate::resolver) fn no_candidates_error( + registry: &impl Registry, + dep: &Dependency, + version_prefs: &VersionPreferences, + package_path: Vec, + required_by: &str, + gctx: Option<&GlobalContext>, +) -> ResolveError { + let to_resolve_err = |err| ResolveError::new(err, package_path.clone()); - // We didn't actually find any candidates, so we need to - // give an error message that nothing was found. let mut msg = String::new(); let mut hints = String::new(); // Whether any candidate was rejected for being newer than `min-publish-age`, @@ -375,6 +447,51 @@ pub(super) fn activation_error( "\nnote: perhaps a crate was updated and forgotten to be re-vendored?" ); } + } else if let Some(packages) = alt_paths(dep, gctx) { + let path = dep.source_id().url().to_file_path().unwrap(); + let _ = writeln!( + &mut msg, + "no matching package named `{}` found", + dep.package_name() + ); + + let mut exact_match: Option = None; + let mut found_dir: Option = None; + let mut names_found: Vec<(String, PathBuf)> = vec![]; + + for pkg in &packages { + let manifest_dir = pkg.manifest_path().parent().unwrap(); + let p_name = pkg.name().as_str(); + if p_name == dep.package_name().as_str() { + exact_match = Some(manifest_dir.to_path_buf()); + break; + } else if manifest_dir == path { + found_dir = Some(p_name.to_string()); + } else { + names_found.push((p_name.to_string(), manifest_dir.to_path_buf())); + } + } + + let mut add_hint = |name: &str, p: &Path| { + let _ = writeln!(&mut hints); + let _ = write!( + &mut hints, + "help: package `{}` exists at `{}`", + name, + p.display() + ); + }; + + if let Some(dir) = exact_match { + add_hint(dep.package_name().as_str(), &dir); + } else if let Some(dir_pkg) = found_dir { + add_hint(&dir_pkg, &path); + } else { + names_found.sort_by(|a, b| a.0.cmp(&b.0)); + for (name, p) in names_found.iter() { + add_hint(name, p); + } + } } else if let Some(name_candidates) = alt_names(registry, dep) { let name_candidates = match name_candidates { Ok(c) => c, @@ -420,11 +537,7 @@ pub(super) fn activation_error( location_searched_msg = format!("{}", dep.source_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()), - ); + let _ = write!(&mut msg, "required by {}", required_by); if has_too_new { let downgrade_to = @@ -548,6 +661,31 @@ fn alt_names( } } +/// For path dependencies, scan the dependency directory for any packages +/// that exist in subdirectories. This helps when the user points to a +/// directory without a Cargo.toml or with the wrong package name. +fn alt_paths( + dep: &Dependency, + gctx: Option<&GlobalContext>, +) -> Option> { + let gctx = gctx?; + if !dep.source_id().is_path() { + return None; + } + let path = dep.source_id().url().to_file_path().ok()?; + if !path.is_dir() { + return None; + } + let source_id = dep.source_id(); + let mut source = RecursivePathSource::new(&path, source_id, gctx); + let packages = source.read_packages().ok()?; + if packages.is_empty() { + None + } else { + Some(packages) + } +} + /// Returns String representation of dependency chain for a particular `pkgid` /// within given context. pub(super) fn describe_path_in_context(cx: &ResolverContext, id: &PackageId) -> String { diff --git a/src/resolver/mod.rs b/src/resolver/mod.rs index 9f1cfd800f9..904e4f8399c 100644 --- a/src/resolver/mod.rs +++ b/src/resolver/mod.rs @@ -96,6 +96,8 @@ mod resolve; mod types; mod version_prefs; +mod pubgrub; + /// Builds the list of all packages required to build the first argument. /// /// * `summaries` - the list of package summaries along with how to resolve @@ -131,6 +133,26 @@ pub fn resolve( resolve_version: ResolveVersion, gctx: Option<&GlobalContext>, ) -> CargoResult { + // `__CARGO_TEST_PUBGRUB` is a test-only escape hatch that routes every + // resolution through the experimental PubGrub resolver, independent of the + // `-Zpubgrub-resolver` flag (which requires nightly). It lets the entire + // integration testsuite be re-run on PubGrub for differential validation; + // it is read here, at the single dispatch fork, so child `cargo` processes + // spawned by the testsuite inherit it. It is never set in production. + let use_pubgrub = gctx.is_some_and(|gctx| { + gctx.cli_unstable().pubgrub_resolver || gctx.get_env_os("__CARGO_TEST_PUBGRUB").is_some() + }); + if use_pubgrub { + return pubgrub::resolve( + summaries, + replacements, + registry, + version_prefs, + resolve_version, + gctx, + ); + } + let first_version = match gctx { Some(config) if config.cli_unstable().direct_minimal_versions => { Some(VersionOrdering::MinimumVersionsFirst) diff --git a/src/resolver/pubgrub/error.rs b/src/resolver/pubgrub/error.rs new file mode 100644 index 00000000000..60399c1bb9b --- /dev/null +++ b/src/resolver/pubgrub/error.rs @@ -0,0 +1,381 @@ +//! Error-reporting bridge between PubGrub and Cargo's resolver errors. +//! +//! This module is deliberately self-contained: it is the *only* place that +//! turns a PubGrub failure into a Cargo [`ResolveError`], and it does so by +//! reusing the v1 resolver's own message rendering +//! ([`RequirementError::into_activate_error`]) rather than re-implementing it. +//! Keeping the translation here means the rest of the PubGrub resolver never +//! formats user-facing prose, and the layer can be dropped or rewritten without +//! touching resolution logic. +//! +//! # How reasons flow +//! +//! When [`super::provider::Provider::get_dependencies`] decides a package is +//! unusable it returns [`pubgrub::Dependencies::Unavailable`] carrying an +//! [`UnavailableReason`] (PubGrub's custom incompatibility metadata `M`). That +//! reason lands in the derivation tree as an [`pubgrub::report::External::Custom`] +//! leaf, where [`report_error`] can recover it and render Cargo-native text. + +use std::fmt; + +use pubgrub::{DefaultStringReporter, DerivationTree, External, PubGrubError, Reporter}; + +use crate::resolver::dep_cache::RequirementError; +use crate::resolver::errors::{ + ActivateError, ResolveError, describe_path, no_candidates_error, version_conflict_error, +}; +use crate::resolver::types::ConflictMap; +use crate::workspace::{Dependency, Registry}; + +use super::package::PubGrubPackage; +use super::provider::Provider; +use super::semver_pubgrub::SemverPubgrub; + +/// PubGrub's custom incompatibility metadata (the `M` type). +/// +/// Rather than baking a prose string at the throw site, the provider records +/// *why* a package is unusable in a structured form, so [`report_error`] can map +/// it back to Cargo's own error rendering. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum UnavailableReason { + /// No version of the package satisfied the request (e.g. the exact version + /// the solution asked for is not in the registry). + NoVersion, + /// A feature/dependency requirement could not be met. Carries the v1 + /// resolver's own [`RequirementError`] so the message is rendered + /// identically. + Requirement(RequirementError), +} + +impl fmt::Display for UnavailableReason { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + // Matches the legacy provider string so any fallback rendering is + // unchanged. + UnavailableReason::NoVersion => write!(f, "no such version"), + UnavailableReason::Requirement(req) => write!(f, "{req}"), + } + } +} + +/// Turn a PubGrub error into a Cargo [`ResolveError`]. +/// +/// For the common shape — a single [`External::Custom`] leaf carrying an +/// [`UnavailableReason::Requirement`] — this reproduces the v1 resolver's exact +/// message via [`RequirementError::into_activate_error`]. Anything else falls +/// back to PubGrub's [`DefaultStringReporter`], wrapped so the resolution +/// outcome is still a typed `ResolveError`. +pub(super) fn report_error( + provider: &Provider<'_, T>, + err: PubGrubError>, +) -> anyhow::Error { + match err { + PubGrubError::NoSolution(mut derivation_tree) => { + derivation_tree.collapse_no_versions(); + let package_path = package_path(provider, &derivation_tree); + // A feature requested by a *dependency* renders as a version + // conflict ("... depends on X with feature Y but ..."); check this + // before the root-level `native_error` form. + if let Some(err) = feature_conflict_native(provider, &derivation_tree) { + return err.into(); + } + if let Some(err) = native_error(provider, &derivation_tree, &package_path) { + return err.into(); + } + if let Some(err) = no_candidates_native(provider, &derivation_tree) { + return err.into(); + } + // Fallback: PubGrub's own rendering, still surfaced as a + // `ResolveError` so callers that downcast keep working. + ResolveError::new( + anyhow::anyhow!( + "failed to select a version for the requirement\n{}", + DefaultStringReporter::report(&derivation_tree) + ), + package_path, + ) + .into() + } + other => anyhow::anyhow!("pubgrub resolution failed: {other}"), + } +} + +/// Try to render a Cargo-native error for the recognized single-cause shapes. +/// +/// Returns `None` when the tree is not a shape we translate, so the caller can +/// fall back to PubGrub's reporter. +fn native_error( + provider: &Provider<'_, T>, + tree: &DerivationTree, + package_path: &[crate::workspace::PackageId], +) -> Option { + let (pkg, reason) = single_custom_leaf(tree)?; + let UnavailableReason::Requirement(req) = reason else { + return None; + }; + // Recover the failing package's summary so the v1 renderer can build the + // exact message (feature lists, "dep:" help text, etc.). + let name = pkg.base_name()?; + let summary = provider.any_summary(name.name, name.source)?; + match req.clone().into_activate_error(None, &summary) { + ActivateError::Fatal(e) => Some(ResolveError::new(e, package_path.to_vec())), + // The `parent: None` arms of `into_activate_error` only ever produce + // `Fatal`, so a `Conflict` here means our assumption broke; fall back. + ActivateError::Conflict(..) => None, + } +} + +/// Render a "package depends on X with feature Y but X does not have that +/// feature" conflict, when the missing feature was requested by a *dependency* +/// (rather than the root/CLI, which [`native_error`] handles). +/// +/// Reuses `RequirementError::into_activate_error(Some(parent), …)` to obtain the +/// exact `ConflictReason` Cargo would (distinguishing missing / required-dep / +/// `dep:`-syntax), then delegates to [`version_conflict_error`]. +fn feature_conflict_native( + provider: &Provider<'_, T>, + tree: &DerivationTree, +) -> Option { + // The failing leaf: a feature of `child` is unavailable; its parent is the + // crate that requested that feature. + let (parent, child, req) = feature_requirement_edge(tree)?; + let parent_name = parent.base_name()?; + let parent_summary = provider.any_summary(parent_name.name, parent_name.source)?; + let child_name = child.base_name()?; + let child_summary = provider.any_summary(child_name.name, child_name.source)?; + + // Only the *feature* requirement errors render as this conflict; a cyclic + // self-feature is a different (root-level) message. + if matches!(req, RequirementError::Cycle(_)) { + return None; + } + + // The dependency edge from parent to child, for the "required by" line and + // the candidate version list. + let dep = parent_summary + .dependencies() + .iter() + .find(|d| d.package_name() == child_name.name && d.source_id() == child_name.source)? + .clone(); + let candidates = provider.matching_summaries(&dep)?; + if candidates.is_empty() { + return None; + } + + let parent_id = parent_summary.package_id(); + // Reuse Cargo's own reason classification (optional vs required vs `dep:`). + let reason = match req + .clone() + .into_activate_error(Some(parent_id), &child_summary) + { + ActivateError::Conflict(_, reason) => reason, + ActivateError::Fatal(_) => return None, + }; + let mut conflicts = ConflictMap::new(); + conflicts.insert(parent_id, reason); + + let required_by = describe_path(std::iter::once((&parent_id, None))); + Some(version_conflict_error( + &dep, + &candidates, + &conflicts, + vec![parent_id], + &required_by, + // The `MissingFeature` family arms do not consult conflict paths + // (`p == parent`), so an empty map suffices. + &std::collections::HashMap::new(), + )) +} + +/// Find a `parent depends on child/feature` edge whose feature requirement +/// failed, returning the parent package, the child package, and the +/// requirement error. +/// +/// The `Custom(child, Requirement(..))` leaf and the +/// `FromDependencyOf(parent, child)` edge that introduced `child` are generally +/// at *different* depths in the tree, so each is searched independently and +/// matched by package identity. +fn feature_requirement_edge<'a>( + tree: &'a DerivationTree, +) -> Option<(&'a PubGrubPackage, &'a PubGrubPackage, &'a RequirementError)> { + let (child, req) = find_custom_requirement(tree)?; + let parent = find_dependency_parent(tree, child)?; + Some((parent, child, req)) +} + +/// Find the first `Custom(pkg, _, Requirement(req))` leaf anywhere in the tree. +fn find_custom_requirement<'a>( + tree: &'a DerivationTree, +) -> Option<(&'a PubGrubPackage, &'a RequirementError)> { + match tree { + DerivationTree::External(External::Custom(pkg, _, UnavailableReason::Requirement(req))) => { + Some((pkg, req)) + } + DerivationTree::Derived(derived) => find_custom_requirement(&derived.cause1) + .or_else(|| find_custom_requirement(&derived.cause2)), + _ => None, + } +} + +/// Find the parent of `child` via a `FromDependencyOf(parent, _, child, _)` +/// external anywhere in the tree. +fn find_dependency_parent<'a>( + tree: &'a DerivationTree, + child: &PubGrubPackage, +) -> Option<&'a PubGrubPackage> { + match tree { + DerivationTree::External(External::FromDependencyOf(parent, _, dep_child, _)) + if dep_child == child => + { + Some(parent) + } + DerivationTree::Derived(derived) => find_dependency_parent(&derived.cause1, child) + .or_else(|| find_dependency_parent(&derived.cause2, child)), + _ => None, + } +} + +/// Render the "no candidates found" family (no matching package / version / +/// yanked / typo) by reconstructing the failing dependency and its parent from +/// the derivation tree, then delegating to the v1 resolver's +/// [`no_candidates_error`] for byte-identical text. +/// +/// Returns `None` if the tree is not a recognizable "parent depends on a +/// missing child" shape, so the caller can fall back to PubGrub's reporter. +fn no_candidates_native( + provider: &Provider<'_, T>, + tree: &DerivationTree, +) -> Option { + // Find a `parent depends on dep` edge where no candidate satisfies `dep`. + let (parent_id, dep) = unsatisfiable_dependency(provider, tree)?; + let required_by = describe_path(std::iter::once((&parent_id, None))); + let registry = provider.registry(); + Some(no_candidates_error( + registry.registry(), + &dep, + provider.version_prefs(), + vec![parent_id], + &required_by, + // The provider does not carry a `GlobalContext`, so the offline-mode + // hint is omitted; it only adds an advisory note. + None, + )) +} + +/// Find a dependency edge in the tree where the depended-on crate has **no +/// candidate version satisfying the requirement** — either the crate is absent +/// entirely or every published version is out of range. +/// +/// Returns the parent's resolved [`PackageId`] and the original [`Dependency`]. +/// This deliberately excludes the "some candidate matches but conflicts with +/// another selection" case (handled by the conflict branch, not here), so the +/// caller only produces the "no candidates" message when it is truly accurate. +fn unsatisfiable_dependency( + provider: &Provider<'_, T>, + tree: &DerivationTree, +) -> Option<(crate::workspace::PackageId, Dependency)> { + match tree { + DerivationTree::External(External::FromDependencyOf(parent, _, child, _)) => { + let parent_name = parent.base_name()?; + let parent_summary = provider.any_summary(parent_name.name, parent_name.source)?; + let child_name = child.base_name()?; + // Recover the original `Dependency` from the parent's manifest. + let dep = parent_summary + .dependencies() + .iter() + .find(|d| { + d.package_name() == child_name.name && d.source_id() == child_name.source + })? + .clone(); + // Only claim "no candidates" when nothing actually matches the req. + let satisfiable = provider + .matching_summaries(&dep) + .is_some_and(|summaries| !summaries.is_empty()); + if satisfiable { + return None; + } + Some((parent_summary.package_id(), dep)) + } + DerivationTree::Derived(derived) => unsatisfiable_dependency(provider, &derived.cause1) + .or_else(|| unsatisfiable_dependency(provider, &derived.cause2)), + _ => None, + } +} + +/// Best-effort reconstruction of the [`ResolveError`] package path: the +/// workspace member(s) whose requirements led to the failure. +/// +/// The default resolver reports the path from the failing package up to the +/// root. PubGrub's derivation tree does not preserve that ordering, but the +/// workspace members referenced in the tree are recoverable and are what +/// consumers (e.g. `cargo metadata`/member diagnostics) key on. +fn package_path( + provider: &Provider<'_, T>, + tree: &DerivationTree, +) -> Vec { + let mut members = Vec::new(); + collect_members(provider, tree, &mut members); + members +} + +/// Collect the resolved [`PackageId`]s of member `Bucket` packages in the tree. +fn collect_members( + provider: &Provider<'_, T>, + tree: &DerivationTree, + out: &mut Vec, +) { + match tree { + DerivationTree::External(ext) => { + for pkg in external_packages(ext) { + if let PubGrubPackage::Bucket { + name, member: true, .. + } = pkg + { + if let Some(summary) = provider.any_summary(name.name, name.source) { + let id = summary.package_id(); + if !out.contains(&id) { + out.push(id); + } + } + } + } + } + DerivationTree::Derived(derived) => { + collect_members(provider, &derived.cause1, out); + collect_members(provider, &derived.cause2, out); + } + } +} + +/// The packages referenced by an [`External`] incompatibility. +fn external_packages( + ext: &External, +) -> Vec<&PubGrubPackage> { + match ext { + External::NotRoot(p, _) | External::NoVersions(p, _) | External::Custom(p, _, _) => { + vec![p] + } + External::FromDependencyOf(p1, _, p2, _) => vec![p1, p2], + } +} + +/// If the whole derivation tree reduces to a single [`External::Custom`] leaf, +/// return its package and reason. +fn single_custom_leaf<'a>( + tree: &'a DerivationTree, +) -> Option<(&'a PubGrubPackage, &'a UnavailableReason)> { + match tree { + DerivationTree::External(External::Custom(pkg, _, reason)) => Some((pkg, reason)), + DerivationTree::Derived(derived) => { + // Walk through single-cause derivations (the other cause being a + // trivially-true "not root" / dependency-of link). + let c1 = single_custom_leaf(&derived.cause1); + let c2 = single_custom_leaf(&derived.cause2); + match (c1, c2) { + (Some(found), None) | (None, Some(found)) => Some(found), + _ => None, + } + } + _ => None, + } +} diff --git a/src/resolver/pubgrub/mod.rs b/src/resolver/pubgrub/mod.rs new file mode 100644 index 00000000000..dc87f6317e4 --- /dev/null +++ b/src/resolver/pubgrub/mod.rs @@ -0,0 +1,124 @@ +//! An alternative dependency resolver built on the [`pubgrub`] crate. +//! +//! This is an experimental, side-by-side implementation of Cargo's dependency +//! resolver gated behind the `-Zpubgrub-resolver` unstable flag. The default +//! resolver in the parent [`super`] module is the hand-rolled backtracking +//! solver; this module instead encodes Cargo's resolution problem into the +//! PubGrub algorithm. +//! +//! # Encoding +//! +//! PubGrub natively allows only a single version of each "package" to be +//! selected. Cargo, however, allows the same crate to appear multiple times in +//! the graph at semver-incompatible versions, and performs feature +//! unification. To bridge this gap we use a richer notion of a "package", see +//! [`package::PubGrubPackage`]: +//! +//! * the real crate, bucketed by its semver-compatibility range, so that +//! semver-incompatible versions are distinct PubGrub packages and may +//! coexist; +//! * a virtual package per crate feature, so that feature unification falls out +//! of normal version solving; +//! * a synthetic `root` package representing the set of workspace members being +//! resolved. +//! +//! See the individual submodules for the details of each piece. + +use crate::resolver::Resolve; +use crate::resolver::ResolveVersion; +use crate::resolver::VersionPreferences; +use crate::resolver::dep_cache::RegistryQueryer; +use crate::resolver::features::{CliFeatures, RequestedFeatures}; +use crate::resolver::types::ResolveOpts; +use crate::workspace::{Dependency, PackageIdSpec, Registry, Summary}; +use crate::context::GlobalContext; +use crate::util::errors::CargoResult; +use crate::util::interning::InternedString; + +mod error; +mod package; +mod provider; +mod semver_pubgrub; +mod solution; + +use self::package::PubGrubPackage; +use self::provider::{Provider, Root, root_version}; + +/// Resolve the dependency graph using the PubGrub algorithm. +/// +/// This mirrors the signature of [`super::resolve()`] so the two resolvers are +/// drop-in interchangeable at the call site in `ops::resolve`. +pub(super) fn resolve( + summaries: &[(Summary, ResolveOpts)], + replacements: &[(PackageIdSpec, Dependency)], + registry: &impl Registry, + version_prefs: &VersionPreferences, + resolve_version: ResolveVersion, + _gctx: Option<&GlobalContext>, +) -> CargoResult { + let registry = RegistryQueryer::new(registry, replacements, version_prefs); + + tracing::debug!( + target: "cargo::resolver::pubgrub", + "pubgrub resolver active: resolving {} workspace member(s)", + summaries.len(), + ); + + let roots = summaries + .iter() + .map(|(summary, opts)| root_from_opts(summary.clone(), opts)) + .collect(); + + let provider = Provider::new(registry, version_prefs, roots); + + match pubgrub::resolve(&provider, PubGrubPackage::Root, root_version()) { + Ok(solution) => solution::into_resolve(&provider, &solution, resolve_version), + Err(err) => { + // A real (e.g. network) error stashed during a callback takes + // precedence over PubGrub's own error. + if let Some(err) = provider.take_error() { + return Err(err); + } + Err(error::report_error(&provider, err)) + } + } +} + +/// Build a [`Root`] describing how a workspace member's features were requested. +fn root_from_opts(summary: Summary, opts: &ResolveOpts) -> Root { + let (all_features, default_features, features) = match &opts.features { + RequestedFeatures::CliFeatures(CliFeatures { + features, + all_features, + uses_default_features, + }) => { + let names = features + .iter() + .filter_map(|fv| match fv { + crate::workspace::summary::FeatureValue::Feature(f) => Some(*f), + // `dep:`/`dep/feat` CLI features are uncommon for workspace + // members; treat their base name as a requested feature. + crate::workspace::summary::FeatureValue::Dep { dep_name } => Some(*dep_name), + crate::workspace::summary::FeatureValue::DepFeature { dep_feature, .. } => { + Some(*dep_feature) + } + }) + .collect(); + (*all_features, *uses_default_features, names) + } + RequestedFeatures::DepFeatures { + features, + uses_default_features, + } => { + let names: Vec = features.iter().copied().collect(); + (false, *uses_default_features, names) + } + }; + Root { + summary, + dev_deps: opts.dev_deps, + all_features, + default_features, + features, + } +} diff --git a/src/resolver/pubgrub/package.rs b/src/resolver/pubgrub/package.rs new file mode 100644 index 00000000000..ea4ebbe8c29 --- /dev/null +++ b/src/resolver/pubgrub/package.rs @@ -0,0 +1,251 @@ +//! The PubGrub "package" encoding for Cargo's resolution problem. +//! +//! PubGrub selects at most one version per package. Cargo needs to (a) allow a +//! crate to appear at several semver-incompatible versions and (b) perform +//! feature unification. We encode both into a richer package identity, +//! [`PubGrubPackage`], adapted from the encoding used by +//! `Eh2406/pubgrub-crates-benchmark` (the `Names` enum) but extended to carry a +//! [`SourceId`] (Cargo resolves across multiple sources) and to own its data. +//! +//! The variants are: +//! +//! * [`PubGrubPackage::Root`] — a synthetic package whose dependencies are the +//! workspace members being resolved. +//! * [`PubGrubPackage::Bucket`] — a concrete crate, identified by name, source +//! and [`SemverCompatibility`] bucket. Selecting a version of a bucket is +//! selecting a concrete crate version. Distinct buckets may coexist, which is +//! how incompatible majors are allowed. +//! * [`PubGrubPackage::BucketFeatures`] — a virtual package standing for "this +//! feature (or optional dependency) of the bucket is enabled". Feature +//! unification falls out of normal version solving over these packages. +//! * [`PubGrubPackage::BucketDefaultFeatures`] — "default features of the bucket +//! are enabled". +//! * [`PubGrubPackage::Wide`] (+ feature variants) — used when a dependency's +//! version requirement could span more than one compatibility bucket. The +//! wide package defers the choice of bucket to a second resolution step. +//! * [`PubGrubPackage::Links`] — enforces the global uniqueness of a `links` +//! attribute value. + +use std::fmt::{self, Display}; + +use semver::VersionReq; + +use crate::workspace::SourceId; +use crate::util::OptVersionReq; +use crate::util::interning::InternedString; + +use super::semver_pubgrub::{SemverCompatibility, SemverPubgrub}; + +/// Distinguishes the two feature "namespaces" Cargo uses: a real feature name +/// (`Feat`) versus an optional dependency activated via `dep:` (`Dep`). +#[derive(Clone, Copy, Eq, PartialEq, Hash, PartialOrd, Ord)] +pub enum FeatureNamespace { + /// A named feature (`feature = [..]`) or `crate/feat`. + Feat(InternedString), + /// An optional dependency named with `dep:name`. + Dep(InternedString), +} + +impl Display for FeatureNamespace { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + FeatureNamespace::Dep(n) => write!(f, "dep:{n}"), + FeatureNamespace::Feat(n) => write!(f, "{n}"), + } + } +} + +/// Identity of a concrete crate within a single compatibility bucket. +#[derive(Clone, Eq, PartialEq, Hash)] +pub struct BucketName { + pub name: InternedString, + pub source: SourceId, + pub compat: SemverCompatibility, +} + +impl Display for BucketName { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}@{:?}", self.name, self.compat) + } +} + +/// Identity of a "wide" dependency whose requirement may span multiple buckets. +/// +/// The requirement and the requesting parent's bucket are part of the identity +/// so that two parents requesting the same crate with different wide +/// requirements remain distinct packages. +#[derive(Clone, Eq, PartialEq, Hash)] +pub struct WideName { + pub name: InternedString, + pub source: SourceId, + pub req: VersionReq, + pub from: InternedString, + pub from_compat: SemverCompatibility, +} + +impl Display for WideName { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "{}(from {}@{:?}):{}", + self.name, self.from, self.from_compat, self.req + ) + } +} + +/// A PubGrub package: the unit over which PubGrub selects a single version. +#[derive(Clone, Eq, PartialEq, Hash)] +pub enum PubGrubPackage { + /// Synthetic root; its dependencies are the workspace members. + Root, + /// A concrete crate bucket. `member` is true for workspace members being + /// resolved directly (which also pull in their dev-dependencies). + /// `all_features` is true when every feature/optional dependency should be + /// activated (the lock-file resolution pass), as opposed to a specific set + /// selected through [`PubGrubPackage::BucketFeatures`]. + Bucket { + name: BucketName, + member: bool, + all_features: bool, + }, + /// "Feature (or optional dep) of the bucket is enabled". + BucketFeatures { + name: BucketName, + feature: FeatureNamespace, + }, + /// "Default features of the bucket are enabled". + BucketDefaultFeatures { name: BucketName }, + /// A wide dependency spanning multiple buckets. + Wide { name: WideName }, + /// A wide dependency with a feature enabled. + WideFeatures { + name: WideName, + feature: FeatureNamespace, + }, + /// A wide dependency with default features enabled. + WideDefaultFeatures { name: WideName }, + /// Enforces global uniqueness of a `links` value. + Links { links: InternedString }, +} + +impl PubGrubPackage { + /// The same bucket package, with default features enabled. + pub fn with_default_features(&self) -> Self { + match self { + PubGrubPackage::Bucket { name, .. } + | PubGrubPackage::BucketFeatures { name, .. } + | PubGrubPackage::BucketDefaultFeatures { name } => { + PubGrubPackage::BucketDefaultFeatures { name: name.clone() } + } + PubGrubPackage::Wide { name } + | PubGrubPackage::WideFeatures { name, .. } + | PubGrubPackage::WideDefaultFeatures { name } => { + PubGrubPackage::WideDefaultFeatures { name: name.clone() } + } + PubGrubPackage::Root | PubGrubPackage::Links { .. } => { + panic!("with_default_features on non-crate package") + } + } + } + + /// The crate `(name, source)` this package refers to, if any. + /// + /// Returns `None` for the synthetic [`PubGrubPackage::Root`] and + /// [`PubGrubPackage::Links`] packages, which have no crate identity. Used by + /// the error-reporting bridge to recover the failing package's summary. + pub fn base_name(&self) -> Option { + match self { + PubGrubPackage::Bucket { name, .. } + | PubGrubPackage::BucketFeatures { name, .. } + | PubGrubPackage::BucketDefaultFeatures { name } => Some(name.clone()), + PubGrubPackage::Wide { name } + | PubGrubPackage::WideFeatures { name, .. } + | PubGrubPackage::WideDefaultFeatures { name } => Some(BucketName { + name: name.name, + source: name.source, + // The wide package has not yet committed to a compat bucket; + // the value is unused by the error bridge (it only needs + // `name`/`source`). + compat: SemverCompatibility::Patch(0), + }), + PubGrubPackage::Root | PubGrubPackage::Links { .. } => None, + } + } + + /// The same bucket package, with the given feature enabled. + pub fn with_feature(&self, feature: FeatureNamespace) -> Self { + match self { + PubGrubPackage::Bucket { name, .. } + | PubGrubPackage::BucketFeatures { name, .. } + | PubGrubPackage::BucketDefaultFeatures { name } => PubGrubPackage::BucketFeatures { + name: name.clone(), + feature, + }, + PubGrubPackage::Wide { name } + | PubGrubPackage::WideFeatures { name, .. } + | PubGrubPackage::WideDefaultFeatures { name } => PubGrubPackage::WideFeatures { + name: name.clone(), + feature, + }, + PubGrubPackage::Root | PubGrubPackage::Links { .. } => { + panic!("with_feature on non-crate package") + } + } + } +} + +impl Display for PubGrubPackage { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + PubGrubPackage::Root => f.write_str("root"), + PubGrubPackage::Bucket { + name, + member, + all_features, + } => { + write!( + f, + "{name}{}{}", + if *member { " (member)" } else { "" }, + if *all_features { " (all-features)" } else { "" }, + ) + } + PubGrubPackage::BucketFeatures { name, feature } => write!(f, "{name}/{feature}"), + PubGrubPackage::BucketDefaultFeatures { name } => write!(f, "{name}/default"), + PubGrubPackage::Wide { name } => write!(f, "wide:{name}"), + PubGrubPackage::WideFeatures { name, feature } => write!(f, "wide:{name}/{feature}"), + PubGrubPackage::WideDefaultFeatures { name } => write!(f, "wide:{name}/default"), + PubGrubPackage::Links { links } => write!(f, "links:{links}"), + } + } +} + +impl fmt::Debug for PubGrubPackage { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + Display::fmt(self, f) + } +} + +/// Convert a dependency's [`OptVersionReq`] into a PubGrub [`SemverPubgrub`]. +/// +/// `Locked`/`Precise` reqs pin to an exact version (they carry a concrete +/// [`semver::Version`]); everything else uses the underlying [`VersionReq`]. +pub fn opt_version_req_to_pubgrub(req: &OptVersionReq) -> SemverPubgrub { + match req { + OptVersionReq::Any => SemverPubgrub::full(), + OptVersionReq::Req(req) => SemverPubgrub::from(req), + OptVersionReq::Locked(v, _) | OptVersionReq::Precise(v, _) => { + SemverPubgrub::singleton(v.clone()) + } + } +} + +/// Extract a [`VersionReq`] from an [`OptVersionReq`] for use in [`WideName`]. +pub fn opt_version_req_to_version_req(req: &OptVersionReq) -> VersionReq { + match req { + OptVersionReq::Any => VersionReq::STAR, + OptVersionReq::Req(req) + | OptVersionReq::Locked(_, req) + | OptVersionReq::Precise(_, req) => req.clone(), + } +} diff --git a/src/resolver/pubgrub/provider.rs b/src/resolver/pubgrub/provider.rs new file mode 100644 index 00000000000..6dc451449e0 --- /dev/null +++ b/src/resolver/pubgrub/provider.rs @@ -0,0 +1,758 @@ +//! The pubgrub [`DependencyProvider`] backed by Cargo's registry. +//! +//! This bridges two impedance mismatches between Cargo and pubgrub: +//! +//! * **async vs. sync.** Cargo's [`RegistryQueryer`] is poll-based and driven by +//! an outer `wait()` loop, while pubgrub drives resolution synchronously by +//! calling back into the provider. We block on the poll loop inside +//! [`Provider::candidates`], reusing the queryer's caching. +//! * **registry data vs. the package encoding.** Cargo describes crates with +//! [`Summary`]/[`Dependency`]/[`FeatureValue`]; we translate those into the +//! [`PubGrubPackage`] encoding on demand in [`Provider::get_dependencies`]. +//! +//! The translation logic mirrors the encoding used by +//! `Eh2406/pubgrub-crates-benchmark`, adapted to Cargo's types and multiple +//! sources. + +use std::cell::RefCell; +use std::cmp::Reverse; +use std::collections::HashMap; +use std::error::Error; +use std::fmt; +use std::rc::Rc; +use std::task::Poll; + +use pubgrub::{Dependencies, DependencyProvider, PackageResolutionStatistics}; +use semver::Version; + +use crate::workspace::dependency::DepKind; +use crate::resolver::VersionPreferences; +use crate::resolver::dep_cache::RegistryQueryer; +use crate::workspace::summary::FeatureValue; +use crate::workspace::{Dependency, Registry, SourceId, Summary}; +use crate::util::interning::InternedString; + +use super::error::UnavailableReason; +use super::package::{ + BucketName, FeatureNamespace, PubGrubPackage, WideName, opt_version_req_to_pubgrub, + opt_version_req_to_version_req, +}; +use super::semver_pubgrub::{SemverCompatibility, SemverPubgrub}; +use crate::resolver::dep_cache::RequirementError; + +/// The version PubGrub assigns to the synthetic [`PubGrubPackage::Root`]. +pub fn root_version() -> Version { + Version::new(0, 0, 0) +} + +/// Error surfaced from the [`DependencyProvider`] callbacks. +/// +/// Real (e.g. network) errors from the registry are stashed on the provider and +/// re-surfaced by the caller; this type is just the pubgrub-facing sentinel. +#[derive(Debug)] +pub struct ProviderError(String); + +impl fmt::Display for ProviderError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + +impl Error for ProviderError {} + +/// A single workspace member to resolve, plus how its features were requested. +pub struct Root { + pub summary: Summary, + /// Whether dev-dependencies of this member should be included. + pub dev_deps: bool, + /// Whether every feature should be enabled (lock-file resolution). + pub all_features: bool, + /// Whether default features are enabled. + pub default_features: bool, + /// Specific features requested (when `all_features` is false). + pub features: Vec, +} + +pub struct Provider<'a, T: Registry> { + registry: RefCell>, + version_prefs: &'a VersionPreferences, + roots: Vec, + /// Cache of candidate summaries per crate, in preference order. + versions: RefCell>>>, + /// Stashed real error from the registry, re-surfaced by the caller. + error: RefCell>, +} + +impl<'a, T: Registry> Provider<'a, T> { + pub fn new( + registry: RegistryQueryer<'a, T>, + version_prefs: &'a VersionPreferences, + roots: Vec, + ) -> Self { + // Workspace members are provided directly rather than queried from the + // registry (they are typically path/local sources). Seed the version + // cache with their summaries so `candidates`/`summary_for` find them. + let mut versions: HashMap<(InternedString, SourceId), Rc>> = HashMap::new(); + { + let mut grouped: HashMap<(InternedString, SourceId), Vec> = HashMap::new(); + for root in &roots { + grouped + .entry((root.summary.name(), root.summary.source_id())) + .or_default() + .push(root.summary.clone()); + } + for (key, summaries) in grouped { + versions.insert(key, Rc::new(summaries)); + } + } + Provider { + registry: RefCell::new(registry), + version_prefs, + roots, + versions: RefCell::new(versions), + error: RefCell::new(None), + } + } + + pub fn take_error(&self) -> Option { + self.error.borrow_mut().take() + } + + pub fn registry(&self) -> std::cell::Ref<'_, RegistryQueryer<'a, T>> { + self.registry.borrow() + } + + /// The version preferences in effect, used by the error bridge to flag + /// candidates rejected for being too new (`min-publish-age`). + pub(super) fn version_prefs(&self) -> &VersionPreferences { + self.version_prefs + } + + /// Stash a real error and return the pubgrub sentinel. + fn fail(&self, context: impl Into, err: anyhow::Error) -> ProviderError { + let msg = context.into(); + if self.error.borrow().is_none() { + *self.error.borrow_mut() = Some(err); + } + ProviderError(msg) + } + + /// Blocking enumeration of all candidate versions of a crate, in preference + /// order (preferred/locked first, then highest version, honoring + /// minimal-versions and publish-time filters via [`VersionPreferences`]). + fn candidates( + &self, + name: InternedString, + source: SourceId, + ) -> Result>, ProviderError> { + if let Some(c) = self.versions.borrow().get(&(name, source)) { + return Ok(c.clone()); + } + // A wildcard dependency to enumerate every version of the crate. + let dep = Dependency::parse(name, None, source) + .map_err(|e| self.fail(format!("failed to query `{name}`"), e))?; + let summaries = { + let mut registry = self.registry.borrow_mut(); + loop { + match registry.query(&dep, None) { + Poll::Ready(Ok(s)) => break s, + Poll::Ready(Err(e)) => { + return Err(self.fail(format!("failed to query `{name}`"), e)); + } + Poll::Pending => { + if let Err(e) = registry.wait() { + return Err(self.fail(format!("failed to query `{name}`"), e)); + } + } + } + } + }; + let mut summaries = (*summaries).clone(); + // Order by Cargo's version preferences so `choose_version` selects the + // same candidate the default resolver would prefer. + self.version_prefs.sort_summaries(&mut summaries, None); + let summaries = Rc::new(summaries); + self.versions + .borrow_mut() + .insert((name, source), summaries.clone()); + Ok(summaries) + } + + /// The summary for an exact (name, source, version), if it exists. + pub(super) fn summary_for( + &self, + name: InternedString, + source: SourceId, + version: &Version, + ) -> Result, ProviderError> { + let candidates = self.candidates(name, source)?; + Ok(candidates.iter().find(|s| s.version() == version).cloned()) + } + + /// Any candidate summary for `(name, source)`. + /// + /// Used by the error-reporting bridge, which needs a representative summary + /// (for its feature/dependency tables) to render a Cargo-native message; the + /// exact version is not significant for those messages. + pub(super) fn any_summary(&self, name: InternedString, source: SourceId) -> Option { + self.candidates(name, source).ok()?.first().cloned() + } + + /// Candidate summaries that satisfy `dep`'s version requirement. + /// + /// Used by the error-reporting bridge to tell "no version matches the + /// requirement" (render a "no candidates" error) apart from "a version + /// matches but conflicts" (a different message it does not yet produce). + /// Returns `None` if the crate could not be queried at all. + pub(super) fn matching_summaries(&self, dep: &Dependency) -> Option> { + let candidates = self.candidates(dep.package_name(), dep.source_id()).ok()?; + Some( + candidates + .iter() + .filter(|s| dep.matches(s)) + .cloned() + .collect(), + ) + } + + /// If every available version matching `dep` lies in a single compatibility + /// bucket, return it. Used to decide between a plain bucket and a wide + /// package. + fn only_one_compat_in_data(&self, dep: &Dependency) -> Option { + let pubgrub_req = opt_version_req_to_pubgrub(dep.version_req()); + let candidates = self.candidates(dep.package_name(), dep.source_id()).ok()?; + let mut iter = candidates + .iter() + .map(|s| s.version()) + .filter(|v| pubgrub_req.contains(v)) + .map(SemverCompatibility::from); + let first = iter.next()?; + if iter.any(|c| c != first) { + None + } else { + Some(first) + } + } + + /// Map a Cargo [`Dependency`] to the PubGrub package + version range that + /// represents it. + pub(super) fn from_dep( + &self, + dep: &Dependency, + from: InternedString, + from_version: &Version, + ) -> (PubGrubPackage, SemverPubgrub) { + let pubgrub_req = opt_version_req_to_pubgrub(dep.version_req()); + let compat = pubgrub_req + .only_one_compatibility_range() + .or_else(|| self.only_one_compat_in_data(dep)); + match compat { + Some(compat) => ( + PubGrubPackage::Bucket { + name: BucketName { + name: dep.package_name(), + source: dep.source_id(), + compat, + }, + member: false, + all_features: false, + }, + pubgrub_req, + ), + None => ( + PubGrubPackage::Wide { + name: WideName { + name: dep.package_name(), + source: dep.source_id(), + req: opt_version_req_to_version_req(dep.version_req()), + from, + from_compat: SemverCompatibility::from(from_version), + }, + }, + SemverPubgrub::full(), + ), + } + } + + /// Count candidate versions of a crate that fall in `range`. + fn count_matches(&self, range: &SemverPubgrub, name: InternedString, source: SourceId) -> u32 { + self.candidates(name, source) + .map(|c| c.iter().filter(|s| range.contains(s.version())).count() as u32) + .unwrap_or(0) + } +} + +/// Insert a dependency constraint, intersecting with any existing one for the +/// same package. +fn deps_insert( + deps: &mut HashMap, + pkg: PubGrubPackage, + range: SemverPubgrub, +) { + deps.entry(pkg) + .and_modify(|old| *old = old.intersection(&range)) + .or_insert(range); +} + +/// A deterministic synthetic version for a `links` package, unique to a given +/// crate version, so that two crates declaring the same `links` value conflict. +fn links_version(pkg: &PubGrubPackage, version: &Version) -> Version { + use std::hash::{Hash, Hasher}; + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + pkg.hash(&mut hasher); + version.hash(&mut hasher); + let h = hasher.finish(); + let b = h.to_be_bytes(); + Version::new( + u16::from_be_bytes([b[0], b[1]]) as u64, + u16::from_be_bytes([b[2], b[3]]) as u64, + u16::from_be_bytes([b[4], b[5]]) as u64, + ) +} + +impl<'a, T: Registry> DependencyProvider for Provider<'a, T> { + type P = PubGrubPackage; + type V = Version; + type VS = SemverPubgrub; + type M = UnavailableReason; + type Err = ProviderError; + type Priority = (u32, Reverse); + + fn choose_version( + &self, + package: &PubGrubPackage, + range: &SemverPubgrub, + ) -> Result, ProviderError> { + Ok(match package { + PubGrubPackage::Root => Some(root_version()), + PubGrubPackage::Links { .. } => { + use std::ops::Bound; + match range.bounding_range() { + Some((_, Bound::Included(v))) => Some(v.clone()), + _ => { + return Err(ProviderError( + "links package has no concrete version".into(), + )); + } + } + } + PubGrubPackage::Wide { name } + | PubGrubPackage::WideFeatures { name, .. } + | PubGrubPackage::WideDefaultFeatures { name } => { + // Pick the canonical version of the first compatibility bucket + // that matches the wide requirement and lies in `range`. + let candidates = self.candidates(name.name, name.source)?; + candidates + .iter() + .map(|s| s.version()) + .filter(|v| name.req.matches(v)) + .map(|v| SemverCompatibility::from(v).canonical()) + .find(|v| range.contains(v)) + } + PubGrubPackage::Bucket { name, .. } + | PubGrubPackage::BucketFeatures { name, .. } + | PubGrubPackage::BucketDefaultFeatures { name } => { + let candidates = self.candidates(name.name, name.source)?; + candidates + .iter() + .map(|s| s.version()) + .find(|v| range.contains(v)) + .cloned() + } + }) + } + + fn prioritize( + &self, + package: &PubGrubPackage, + range: &SemverPubgrub, + stats: &PackageResolutionStatistics, + ) -> Self::Priority { + let conflicts = stats.conflict_count(); + match package { + PubGrubPackage::Root => (conflicts, Reverse(0)), + // Decide links last: it only rubber-stamps uniqueness. + PubGrubPackage::Links { .. } => (conflicts, Reverse(u32::MAX)), + PubGrubPackage::Bucket { name, .. } => { + if range.as_singleton().is_some() { + (conflicts, Reverse(1)) + } else { + ( + conflicts, + Reverse(self.count_matches(range, name.name, name.source)), + ) + } + } + PubGrubPackage::BucketFeatures { name, .. } + | PubGrubPackage::BucketDefaultFeatures { name } => { + if range.as_singleton().is_some() { + (conflicts, Reverse(0)) + } else { + ( + conflicts, + Reverse( + self.count_matches(range, name.name, name.source) + .saturating_add(1), + ), + ) + } + } + PubGrubPackage::Wide { name } + | PubGrubPackage::WideFeatures { name, .. } + | PubGrubPackage::WideDefaultFeatures { name } => ( + conflicts, + Reverse( + self.count_matches(range, name.name, name.source) + .saturating_add(1), + ), + ), + } + } + + fn get_dependencies( + &self, + package: &PubGrubPackage, + version: &Version, + ) -> Result, ProviderError> { + let mut deps: HashMap = HashMap::new(); + match package { + PubGrubPackage::Root => { + for root in &self.roots { + let summary = &root.summary; + let name = BucketName { + name: summary.name(), + source: summary.source_id(), + compat: SemverCompatibility::from(summary.version()), + }; + let singleton = SemverPubgrub::singleton(summary.version().clone()); + // The member itself, pinned to its exact version. + deps_insert( + &mut deps, + PubGrubPackage::Bucket { + name: name.clone(), + member: root.dev_deps, + all_features: root.all_features, + }, + singleton.clone(), + ); + if root.all_features { + // The all-features bucket pulls in every feature itself. + } else { + if root.default_features { + deps_insert( + &mut deps, + PubGrubPackage::BucketDefaultFeatures { name: name.clone() }, + singleton.clone(), + ); + } + for feat in &root.features { + deps_insert( + &mut deps, + PubGrubPackage::BucketFeatures { + name: name.clone(), + feature: FeatureNamespace::Feat(*feat), + }, + singleton.clone(), + ); + } + } + } + return Ok(Dependencies::Available(deps.into_iter().collect())); + } + + PubGrubPackage::Links { .. } => { + return Ok(Dependencies::Available(deps.into_iter().collect())); + } + + PubGrubPackage::Bucket { + name, + member, + all_features, + } => { + let Some(summary) = self.summary_for(name.name, name.source, version)? else { + return Ok(Dependencies::Unavailable(UnavailableReason::NoVersion)); + }; + // `links` uniqueness. + if let Some(link) = summary.links() { + deps.insert( + PubGrubPackage::Links { links: link }, + SemverPubgrub::singleton(links_version(package, version)), + ); + } + for dep in summary.dependencies() { + let is_dev = dep.kind() == DepKind::Development; + if is_dev && !*member { + continue; + } + if dep.is_optional() && !*all_features { + // Optional deps are activated via feature packages. + continue; + } + let (cray, range) = self.from_dep(dep, name.name, version); + deps_insert(&mut deps, cray.clone(), range.clone()); + if dep.uses_default_features() { + deps_insert(&mut deps, cray.with_default_features(), range.clone()); + } + for f in dep.features() { + deps_insert( + &mut deps, + cray.with_feature(FeatureNamespace::Feat(*f)), + range.clone(), + ); + } + } + if *all_features { + // Enable every feature (the implicit features of optional + // dependencies are included in the feature map, so this + // also activates all optional dependencies). + for feat in summary.features().keys() { + deps_insert( + &mut deps, + PubGrubPackage::BucketFeatures { + name: name.clone(), + feature: FeatureNamespace::Feat(*feat), + }, + SemverPubgrub::singleton(version.clone()), + ); + } + } + return Ok(Dependencies::Available(deps.into_iter().collect())); + } + + PubGrubPackage::BucketFeatures { + name, + feature: FeatureNamespace::Feat(feat), + } => { + let Some(summary) = self.summary_for(name.name, name.source, version)? else { + return Ok(Dependencies::Unavailable(UnavailableReason::NoVersion)); + }; + // A feature implies the crate at this exact version. + deps.insert( + PubGrubPackage::Bucket { + name: name.clone(), + member: false, + all_features: false, + }, + SemverPubgrub::singleton(version.clone()), + ); + let Some(values) = summary.features().get(feat) else { + return Ok(Dependencies::Unavailable(UnavailableReason::Requirement( + RequirementError::MissingFeature(*feat), + ))); + }; + let singleton = SemverPubgrub::singleton(version.clone()); + for fv in values { + match fv { + // A feature that lists itself is a cycle. PubGrub would + // treat the resulting self-dependency as trivially + // satisfiable, so detect it explicitly to match the + // default resolver's `cyclic feature dependency` error. + FeatureValue::Feature(f) if f == feat => { + return Ok(Dependencies::Unavailable(UnavailableReason::Requirement( + RequirementError::Cycle(*feat), + ))); + } + FeatureValue::Feature(f) => deps_insert( + &mut deps, + package.with_feature(FeatureNamespace::Feat(*f)), + singleton.clone(), + ), + FeatureValue::Dep { dep_name } => deps_insert( + &mut deps, + package.with_feature(FeatureNamespace::Dep(*dep_name)), + singleton.clone(), + ), + FeatureValue::DepFeature { + dep_name, + dep_feature, + weak, + } => { + for dep in summary + .dependencies() + .iter() + .filter(|d| d.name_in_toml() == *dep_name) + { + if dep.kind() == DepKind::Development { + continue; + } + let (cray, range) = self.from_dep(dep, name.name, version); + if dep.is_optional() { + // Cargo's v1 lock resolver records the + // optional dependency as part of the graph + // for ANY `dep/feat` reference, including + // weak `dep?/feat` ones. The `weak` flag + // only controls whether the dependency's + // own implicit feature is enabled. + deps_insert( + &mut deps, + package.with_feature(FeatureNamespace::Dep(*dep_name)), + singleton.clone(), + ); + if !*weak + && *dep_name != *feat + && summary.features().contains_key(dep_name) + { + deps_insert( + &mut deps, + package.with_feature(FeatureNamespace::Feat(*dep_name)), + singleton.clone(), + ); + } + } + deps_insert( + &mut deps, + cray.with_feature(FeatureNamespace::Feat(*dep_feature)), + range, + ); + } + } + } + } + return Ok(Dependencies::Available(deps.into_iter().collect())); + } + + PubGrubPackage::BucketFeatures { + name, + feature: FeatureNamespace::Dep(dep_name), + } => { + let Some(summary) = self.summary_for(name.name, name.source, version)? else { + return Ok(Dependencies::Unavailable(UnavailableReason::NoVersion)); + }; + deps.insert( + PubGrubPackage::Bucket { + name: name.clone(), + member: false, + all_features: false, + }, + SemverPubgrub::singleton(version.clone()), + ); + let mut found = false; + for dep in summary + .dependencies() + .iter() + .filter(|d| d.name_in_toml() == *dep_name) + { + if !dep.is_optional() || dep.kind() == DepKind::Development { + continue; + } + found = true; + let (cray, range) = self.from_dep(dep, name.name, version); + deps_insert(&mut deps, cray.clone(), range.clone()); + if dep.uses_default_features() { + deps_insert(&mut deps, cray.with_default_features(), range.clone()); + } + for f in dep.features() { + deps_insert( + &mut deps, + cray.with_feature(FeatureNamespace::Feat(*f)), + range.clone(), + ); + } + } + if found { + return Ok(Dependencies::Available(deps.into_iter().collect())); + } else { + return Ok(Dependencies::Unavailable(UnavailableReason::Requirement( + RequirementError::MissingDependency(*dep_name), + ))); + } + } + + PubGrubPackage::BucketDefaultFeatures { name } => { + let Some(summary) = self.summary_for(name.name, name.source, version)? else { + return Ok(Dependencies::Unavailable(UnavailableReason::NoVersion)); + }; + deps.insert( + PubGrubPackage::Bucket { + name: name.clone(), + member: false, + all_features: false, + }, + SemverPubgrub::singleton(version.clone()), + ); + if summary.features().contains_key("default") { + deps_insert( + &mut deps, + package + .with_feature(FeatureNamespace::Feat(InternedString::new("default"))), + SemverPubgrub::singleton(version.clone()), + ); + } + return Ok(Dependencies::Available(deps.into_iter().collect())); + } + + PubGrubPackage::Wide { name } => { + let compat = SemverCompatibility::from(version); + let range = + opt_req_range(&name.req).intersection(&SemverPubgrub::compatibility(&compat)); + deps_insert( + &mut deps, + PubGrubPackage::Bucket { + name: BucketName { + name: name.name, + source: name.source, + compat, + }, + member: false, + all_features: false, + }, + range, + ); + return Ok(Dependencies::Available(deps.into_iter().collect())); + } + + PubGrubPackage::WideFeatures { name, feature } => { + let compat = SemverCompatibility::from(version); + let range = + opt_req_range(&name.req).intersection(&SemverPubgrub::compatibility(&compat)); + // Tie this wide-feature decision to the underlying wide package. + deps_insert( + &mut deps, + PubGrubPackage::Wide { name: name.clone() }, + SemverPubgrub::singleton(version.clone()), + ); + deps_insert( + &mut deps, + PubGrubPackage::BucketFeatures { + name: BucketName { + name: name.name, + source: name.source, + compat, + }, + feature: *feature, + }, + range, + ); + return Ok(Dependencies::Available(deps.into_iter().collect())); + } + + PubGrubPackage::WideDefaultFeatures { name } => { + let compat = SemverCompatibility::from(version); + let range = + opt_req_range(&name.req).intersection(&SemverPubgrub::compatibility(&compat)); + deps_insert( + &mut deps, + PubGrubPackage::Wide { name: name.clone() }, + SemverPubgrub::singleton(version.clone()), + ); + deps_insert( + &mut deps, + PubGrubPackage::BucketDefaultFeatures { + name: BucketName { + name: name.name, + source: name.source, + compat, + }, + }, + range, + ); + return Ok(Dependencies::Available(deps.into_iter().collect())); + } + } + } +} + +/// The PubGrub range for a bare [`semver::VersionReq`]. +fn opt_req_range(req: &semver::VersionReq) -> SemverPubgrub { + SemverPubgrub::from(req) +} diff --git a/src/resolver/pubgrub/semver_pubgrub.rs b/src/resolver/pubgrub/semver_pubgrub.rs new file mode 100644 index 00000000000..f1d49681928 --- /dev/null +++ b/src/resolver/pubgrub/semver_pubgrub.rs @@ -0,0 +1,762 @@ +//! Compatibility between semver's [`VersionReq`] and PubGrub's [`VersionSet`]. +//! +//! PubGrub needs more operations on version requirements than the `semver` +//! crate provides on [`VersionReq`] (notably negation, intersection and union). +//! [`SemverPubgrub`] is a representation of a [`VersionReq`] that supports those +//! operations while remaining bug-for-bug compatible with semver's `matches`. +//! +//! This is a specialization (to [`semver::Version`]) and port of the +//! `semver-pubgrub` crate , +//! adapted to the published `pubgrub` 0.4 API. The structure deliberately +//! mirrors `semver`'s `eval.rs` so the two can be kept in sync. + +use std::cmp::{max, min}; +use std::fmt::{self, Display}; +use std::num::NonZeroU64; +use std::ops::Bound; + +use pubgrub::{Range, VersionSet}; +use semver::{BuildMetadata, Comparator, Op, Prerelease, Version, VersionReq}; + +/// A [`VersionReq`] re-expressed as a pair of PubGrub [`Range`]s, split into the +/// versions matched among normal releases and among pre-releases. +/// +/// semver applies different rules to pre-release versions, so we track the two +/// kinds of matches separately and recombine them in [`Self::contains`]. +#[derive(Debug, PartialEq, Eq, Clone, Hash)] +pub struct SemverPubgrub { + normal: Range, + pre: Range, +} + +impl SemverPubgrub { + pub fn empty() -> Self { + SemverPubgrub { + normal: Range::empty(), + pre: Range::empty(), + } + } + + pub fn full() -> Self { + SemverPubgrub { + normal: Range::full(), + pre: Range::full(), + } + } + + pub fn singleton(v: Version) -> Self { + let is_pre = !v.pre.is_empty(); + let singleton = Range::::singleton(v); + if !is_pre { + SemverPubgrub { + normal: singleton, + pre: Range::empty(), + } + } else { + SemverPubgrub { + normal: Range::empty(), + pre: singleton, + } + } + } + + pub fn complement(&self) -> Self { + SemverPubgrub { + normal: self.normal.complement(), + pre: self.pre.complement(), + } + } + + pub fn intersection(&self, other: &Self) -> Self { + SemverPubgrub { + normal: self.normal.intersection(&other.normal), + pre: self.pre.intersection(&other.pre), + } + } + + pub fn contains(&self, v: &Version) -> bool { + // Must be bug-for-bug compatible with `matches_req`: + // https://github.com/dtolnay/semver/blob/master/src/eval.rs + if v.pre.is_empty() { + self.normal.contains(v) + } else { + self.pre.contains(v) + } + } + + pub fn union(&self, other: &Self) -> Self { + SemverPubgrub { + normal: self.normal.union(&other.normal), + pre: self.pre.union(&other.pre), + } + } + + pub fn is_disjoint(&self, other: &Self) -> bool { + self.normal.is_disjoint(&other.normal) && self.pre.is_disjoint(&other.pre) + } + + pub fn subset_of(&self, other: &Self) -> bool { + self.normal.subset_of(&other.normal) && self.pre.subset_of(&other.pre) + } + + /// If this set is exactly one version, return it. + pub fn as_singleton(&self) -> Option<&Version> { + self.normal.as_singleton().xor(self.pre.as_singleton()) + } + + /// A range covering all versions in a single semver-compatibility bucket. + pub fn compatibility(compat: &SemverCompatibility) -> Self { + let r = compat.to_range(); + SemverPubgrub { + normal: r.clone(), + pre: r, + } + } + + /// If every version this set can match falls within a single + /// semver-compatibility bucket, return that bucket. + /// + /// Most requirements in the ecosystem (caret/tilde) match only one bucket, + /// which lets the encoding use a plain [`super::package::PubGrubPackage::Bucket`] + /// instead of the heavier "wide" packages. + pub fn only_one_compatibility_range(&self) -> Option { + use Bound::*; + let normal_bound = self.normal.bounding_range(); + let pre_bound = self.pre.bounding_range(); + if normal_bound.is_none() && pre_bound.is_none() { + return Some(SemverCompatibility::Patch(0)); + } + let normal_start = normal_bound.map(|(s, _)| match s { + Included(v) | Excluded(v) => v.into(), + Unbounded => SemverCompatibility::Patch(0), + }); + let pre_start = pre_bound.map(|(s, _)| match s { + Included(v) | Excluded(v) => v.into(), + Unbounded => SemverCompatibility::Patch(0), + }); + if normal_start.is_some() && pre_start.is_some() && normal_start != pre_start { + return None; + } + let start = normal_start.or(pre_start).unwrap(); + if let Some(next) = start.next() { + if let Some((_, pe)) = pre_bound { + match (pe, next.minimum()) { + (Unbounded, _) => return None, + (Included(e), m) => { + if e >= &m { + return None; + } + } + (Excluded(e), m) => { + if e > &m { + return None; + } + } + } + } + if let Some((_, ne)) = normal_bound { + match (ne, next.canonical()) { + (Unbounded, _) => return None, + (Included(e), m) => { + if e >= &m { + return None; + } + } + (Excluded(e), m) => { + if e > &m { + return None; + } + } + } + } + } + + Some(start) + } + + /// Convert to a pair of bounds usable with + /// [`BTreeMap::range`](std::collections::BTreeMap::range). Every version + /// contained in `self` falls within the output, but the output may be + /// wider than `self`. Returns `None` when the range is empty. + pub fn bounding_range(&self) -> Option<(Bound<&Version>, Bound<&Version>)> { + use Bound::*; + let Some((ns, ne)) = self.normal.bounding_range() else { + return self.pre.bounding_range(); + }; + let Some((ps, pe)) = self.pre.bounding_range() else { + return Some((ns, ne)); + }; + let start = match (ns, ps) { + (Included(n), Included(p)) => Included(min(n, p)), + (Included(i), Excluded(e)) | (Excluded(e), Included(i)) => { + if e < i { + Excluded(e) + } else { + Included(i) + } + } + (Excluded(n), Excluded(p)) => Excluded(min(n, p)), + (Unbounded, _) | (_, Unbounded) => Unbounded, + }; + let end = match (ne, pe) { + (Included(n), Included(p)) => Included(max(n, p)), + (Included(i), Excluded(e)) | (Excluded(e), Included(i)) => { + if i < e { + Excluded(e) + } else { + Included(i) + } + } + (Excluded(n), Excluded(p)) => Excluded(max(n, p)), + (Unbounded, _) | (_, Unbounded) => Unbounded, + }; + Some((start, end)) + } +} + +impl Display for SemverPubgrub { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "SemverPubgrub {{ normal: {}, pre: {} }}", + self.normal, self.pre + ) + } +} + +impl VersionSet for SemverPubgrub { + type V = Version; + + fn empty() -> Self { + Self::empty() + } + + fn full() -> Self { + Self::full() + } + + fn singleton(v: Self::V) -> Self { + Self::singleton(v) + } + + fn complement(&self) -> Self { + self.complement() + } + + fn intersection(&self, other: &Self) -> Self { + self.intersection(other) + } + + fn contains(&self, v: &Self::V) -> bool { + self.contains(v) + } + + fn union(&self, other: &Self) -> Self { + self.union(other) + } + + fn is_disjoint(&self, other: &Self) -> bool { + self.is_disjoint(other) + } + + fn subset_of(&self, other: &Self) -> bool { + self.subset_of(other) + } +} + +impl From<&VersionReq> for SemverPubgrub { + fn from(req: &VersionReq) -> Self { + let mut out = Self::full(); + // The normal range is the intersection of all comparators. + for cmp in &req.comparators { + out = out.intersection(&matches_impl(cmp)); + } + // The pre-release range is the union of each comparator's pre window, + // then intersected with the normal pre range. + let mut pre = Range::empty(); + for cmp in &req.comparators { + pre = pre.union(&pre_is_compatible(cmp)); + } + out.pre = pre.intersection(&out.pre); + out + } +} + +// ----- semver `eval.rs` port ------------------------------------------------- + +fn matches_impl(cmp: &Comparator) -> SemverPubgrub { + match cmp.op { + Op::Exact | Op::Wildcard => matches_exact(cmp), + Op::Greater => matches_greater(cmp), + Op::GreaterEq => matches_exact(cmp).union(&matches_greater(cmp)), + Op::Less => matches_less(cmp), + Op::LessEq => matches_exact(cmp).union(&matches_less(cmp)), + Op::Tilde => matches_tilde(cmp), + Op::Caret => matches_caret(cmp), + _ => unreachable!("update to a semver version that supports this Op"), + } +} + +fn matches_exact(cmp: &Comparator) -> SemverPubgrub { + let low = Version { + major: cmp.major, + minor: cmp.minor.unwrap_or(0), + patch: cmp.patch.unwrap_or(0), + pre: cmp.pre.clone(), + build: BuildMetadata::EMPTY, + }; + if !cmp.pre.is_empty() { + return SemverPubgrub { + normal: Range::empty(), + pre: between(low, bump_pre), + }; + } + let normal = if cmp.patch.is_some() { + between(low, bump_patch) + } else if cmp.minor.is_some() { + between(low, bump_minor) + } else { + between(low, bump_major) + }; + + SemverPubgrub { + normal: simplified_to_normal(&normal), + pre: Range::empty(), + } +} + +fn matches_greater(cmp: &Comparator) -> SemverPubgrub { + let low = Version { + major: cmp.major, + minor: cmp.minor.unwrap_or(0), + patch: cmp.patch.unwrap_or(0), + pre: cmp.pre.clone(), + build: BuildMetadata::EMPTY, + }; + let bump = if cmp.patch.is_some() { + bump_pre(&low) + } else if cmp.minor.is_some() { + bump_minor(&low) + } else { + bump_major(&low) + }; + let low_bound = match bump { + Bound::Included(_) => unreachable!(), + Bound::Excluded(v) => Bound::Included(v), + Bound::Unbounded => return SemverPubgrub::empty(), + }; + let out = Range::from_range_bounds((low_bound, Bound::Unbounded)); + SemverPubgrub { + normal: simplified_to_normal(&out), + pre: out, + } +} + +fn matches_less(cmp: &Comparator) -> SemverPubgrub { + let out = Range::strictly_lower_than(Version { + major: cmp.major, + minor: cmp.minor.unwrap_or(0), + patch: cmp.patch.unwrap_or(0), + pre: if cmp.patch.is_some() { + cmp.pre.clone() + } else { + Prerelease::new("0").unwrap() + }, + build: BuildMetadata::EMPTY, + }); + SemverPubgrub { + normal: simplified_to_normal(&out), + pre: out, + } +} + +fn matches_tilde(cmp: &Comparator) -> SemverPubgrub { + let low = Version { + major: cmp.major, + minor: cmp.minor.unwrap_or(0), + patch: cmp.patch.unwrap_or(0), + pre: cmp.pre.clone(), + build: BuildMetadata::EMPTY, + }; + if cmp.patch.is_some() { + let out = between(low, bump_minor); + return SemverPubgrub { + normal: simplified_to_normal(&out), + pre: out, + }; + } + let normal = if cmp.minor.is_some() { + between(low, bump_minor) + } else { + between(low, bump_major) + }; + SemverPubgrub { + normal: simplified_to_normal(&normal), + pre: Range::empty(), + } +} + +fn matches_caret(cmp: &Comparator) -> SemverPubgrub { + let low = Version { + major: cmp.major, + minor: cmp.minor.unwrap_or(0), + patch: cmp.patch.unwrap_or(0), + pre: if cmp.patch.is_some() { + cmp.pre.clone() + } else { + Prerelease::new("0").unwrap() + }, + build: BuildMetadata::EMPTY, + }; + let Some(minor) = cmp.minor else { + let out = between(low, bump_major); + return SemverPubgrub { + normal: simplified_to_normal(&out), + pre: out, + }; + }; + + if cmp.patch.is_none() { + let out = if cmp.major > 0 { + between(low, bump_major) + } else { + between(low, bump_minor) + }; + return SemverPubgrub { + normal: simplified_to_normal(&out), + pre: out, + }; + }; + + let out = if cmp.major > 0 { + between(low, bump_major) + } else if minor > 0 { + between(low, bump_minor) + } else { + between(low, bump_patch) + }; + SemverPubgrub { + normal: simplified_to_normal(&out), + pre: out, + } +} + +fn pre_is_compatible(cmp: &Comparator) -> Range { + if cmp.pre.is_empty() { + return Range::empty(); + } + let (Some(minor), Some(patch)) = (cmp.minor, cmp.patch) else { + return Range::empty(); + }; + + Range::between( + Version { + major: cmp.major, + minor, + patch, + pre: Prerelease::new("0").unwrap(), + build: BuildMetadata::EMPTY, + }, + Version::new(cmp.major, minor, patch), + ) +} + +// ----- bump helpers (port of semver-pubgrub `bump_helpers.rs`) --------------- + +fn bump_major(v: &Version) -> Bound { + match v.major.checked_add(1) { + Some(new) => Bound::Excluded(Version { + major: new, + minor: 0, + patch: 0, + pre: Prerelease::new("0").unwrap(), + build: BuildMetadata::EMPTY, + }), + None => Bound::Unbounded, + } +} + +fn bump_minor(v: &Version) -> Bound { + match v.minor.checked_add(1) { + Some(new) => Bound::Excluded(Version { + major: v.major, + minor: new, + patch: 0, + pre: Prerelease::new("0").unwrap(), + build: BuildMetadata::EMPTY, + }), + None => bump_major(v), + } +} + +fn bump_patch(v: &Version) -> Bound { + match v.patch.checked_add(1) { + Some(new) => Bound::Excluded(Version { + major: v.major, + minor: v.minor, + patch: new, + pre: Prerelease::new("0").unwrap(), + build: BuildMetadata::EMPTY, + }), + None => bump_minor(v), + } +} + +fn bump_pre(v: &Version) -> Bound { + if !v.pre.is_empty() { + Bound::Excluded(Version { + major: v.major, + minor: v.minor, + patch: v.patch, + pre: Prerelease::new(&format!("{}.0", v.pre)).unwrap(), + build: BuildMetadata::EMPTY, + }) + } else { + bump_patch(v) + } +} + +fn between(low: Version, into: impl Fn(&Version) -> Bound) -> Range { + let high = into(&low); + Range::from_range_bounds((Bound::Included(low), high)) +} + +fn bump_up_to_normal(v: &Version) -> Option { + if v.pre.is_empty() { + None + } else { + Some(Version { + major: v.major, + minor: v.minor, + patch: v.patch, + pre: Prerelease::EMPTY, + build: BuildMetadata::EMPTY, + }) + } +} + +fn simplified_bounds_to_normal( + bounds: (Bound, Bound), +) -> (Bound, Bound) { + let (mut from, mut to) = bounds; + if let Bound::Included(f) | Bound::Excluded(f) = &from { + if let Some(n) = bump_up_to_normal(f) { + from = Bound::Included(n) + } + }; + if let Bound::Included(f) | Bound::Excluded(f) = &to { + if let Some(n) = bump_up_to_normal(f) { + to = Bound::Excluded(n) + } + }; + (from, to) +} + +fn simplified_to_normal(input: &Range) -> Range { + Range::from_iter( + input + .iter() + .map(|(from, to)| simplified_bounds_to_normal((from.clone(), to.clone()))), + ) +} + +// ----- semver compatibility buckets ------------------------------------------ + +/// Describes when Cargo treats two versions as compatible: versions `a` and `b` +/// are compatible when their left-most nonzero component is equal. +/// +/// PubGrub allows only one version of a package; Cargo allows one per +/// compatibility bucket. We therefore encode the bucket into the PubGrub +/// package identity (see [`super::package`]). +#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug, PartialOrd, Ord)] +pub enum SemverCompatibility { + Patch(u64), + Minor(NonZeroU64), + Major(NonZeroU64), +} + +impl SemverCompatibility { + /// The smallest (pre-release) version contained in this bucket. + pub fn minimum(&self) -> Version { + match *self { + Self::Major(new) => Version { + major: new.into(), + minor: 0, + patch: 0, + pre: Prerelease::new("0").unwrap(), + build: BuildMetadata::EMPTY, + }, + Self::Minor(new) => Version { + major: 0, + minor: new.into(), + patch: 0, + pre: Prerelease::new("0").unwrap(), + build: BuildMetadata::EMPTY, + }, + Self::Patch(new) => Version { + major: 0, + minor: 0, + patch: new, + pre: Prerelease::new("0").unwrap(), + build: BuildMetadata::EMPTY, + }, + } + } + + /// The smallest non-pre-release version contained in this bucket. + pub fn canonical(&self) -> Version { + match *self { + Self::Major(new) => Version::new(new.into(), 0, 0), + Self::Minor(new) => Version::new(0, new.into(), 0), + Self::Patch(new) => Version::new(0, 0, new), + } + } + + pub fn next(&self) -> Option { + let one = NonZeroU64::new(1).unwrap(); + match *self { + Self::Patch(s) => Some( + s.checked_add(1) + .map(Self::Patch) + .unwrap_or_else(|| Self::Minor(one)), + ), + Self::Minor(s) => Some( + s.checked_add(1) + .map(Self::Minor) + .unwrap_or_else(|| Self::Major(one)), + ), + Self::Major(s) => s.checked_add(1).map(Self::Major), + } + } + + fn maximum_bound(&self) -> Bound { + if let Some(next) = self.next() { + Bound::Excluded(next.minimum()) + } else { + Bound::Unbounded + } + } + + /// The PubGrub range matching exactly this compatibility bucket. + pub fn to_range(&self) -> Range { + Range::from_range_bounds((Bound::Included(self.minimum()), self.maximum_bound())) + } +} + +impl From<&Version> for SemverCompatibility { + fn from(ver: &Version) -> Self { + if let Some(m) = NonZeroU64::new(ver.major) { + return SemverCompatibility::Major(m); + } + if let Some(m) = NonZeroU64::new(ver.minor) { + return SemverCompatibility::Minor(m); + } + SemverCompatibility::Patch(ver.patch) + } +} + +#[cfg(test)] +mod test { + use super::*; + + const OPS: &[&str] = &["^", "~", "=", "<", ">", "<=", ">="]; + + /// `SemverPubgrub::contains` must agree with semver's `VersionReq::matches`. + #[test] + fn contains_matches_semver() { + let reqs = [ + "^1", + "^1.2", + "^1.2.3", + "~1.2", + "~1.2.3", + "=1.2.3", + ">1.2.3", + ">=1.2.3", + "<1.2.3", + "<=1.2.3", + "^0.2", + "^0.0.3", + "^0", + "*", + "1.*", + "1.2.*", + ">=1.2, <1.5", + "^1.2.3-alpha", + "=1.2.3-beta.1", + ]; + let vers = [ + "0.0.1", + "0.2.0", + "0.2.5", + "1.0.0", + "1.2.0", + "1.2.3", + "1.2.4", + "1.4.9", + "1.5.0", + "2.0.0", + "1.2.3-alpha", + "1.2.3-beta.1", + "1.2.3-beta.2", + ]; + for raw_req in reqs { + let req = VersionReq::parse(raw_req).unwrap(); + let pg: SemverPubgrub = (&req).into(); + for raw_ver in vers { + let ver = Version::parse(raw_ver).unwrap(); + assert_eq!( + req.matches(&ver), + pg.contains(&ver), + "mismatch for req `{raw_req}` and version `{raw_ver}`", + ); + } + } + } + + /// Exhaustive-ish cross-check across a grid of operators and operands. + #[test] + fn contains_matches_semver_grid() { + let operands = [ + "0", "0.0", "0.0.3", "0.2", "0.2.5", "1", "1.2", "1.2.3", "2.0.0", + ]; + let vers: Vec = [ + "0.0.1", "0.0.3", "0.2.0", "0.2.5", "1.0.0", "1.2.0", "1.2.3", "2.0.0", "2.1.0", + ] + .iter() + .map(|v| Version::parse(v).unwrap()) + .collect(); + for op in OPS { + for operand in operands { + let raw_req = format!("{op}{operand}"); + let Ok(req) = VersionReq::parse(&raw_req) else { + continue; + }; + let pg: SemverPubgrub = (&req).into(); + for ver in &vers { + assert_eq!( + req.matches(ver), + pg.contains(ver), + "mismatch for req `{raw_req}` and version `{ver}`", + ); + } + } + } + } + + #[test] + fn compatibility_buckets() { + assert_eq!( + SemverCompatibility::from(&Version::parse("1.2.3").unwrap()), + SemverCompatibility::Major(NonZeroU64::new(1).unwrap()) + ); + assert_eq!( + SemverCompatibility::from(&Version::parse("0.2.3").unwrap()), + SemverCompatibility::Minor(NonZeroU64::new(2).unwrap()) + ); + assert_eq!( + SemverCompatibility::from(&Version::parse("0.0.3").unwrap()), + SemverCompatibility::Patch(3) + ); + } +} diff --git a/src/resolver/pubgrub/solution.rs b/src/resolver/pubgrub/solution.rs new file mode 100644 index 00000000000..3c58da58ab5 --- /dev/null +++ b/src/resolver/pubgrub/solution.rs @@ -0,0 +1,260 @@ +//! Reconstruct a Cargo [`Resolve`] from a PubGrub solution. +//! +//! PubGrub returns a [`SelectedDependencies`] mapping each [`PubGrubPackage`] to +//! the version it selected. We project that back onto Cargo's model: +//! +//! * concrete [`PubGrubPackage::Bucket`] packages become the resolved +//! [`PackageId`]s and graph nodes; +//! * the feature/default-feature packages tell us which features each package +//! ended up with; +//! * graph edges are recovered by walking each resolved summary's +//! dependencies, keeping the ones that the feature solution activated, and +//! linking them to the selected child version. + +use std::collections::{BTreeMap, BTreeSet}; + +use crate::util::data_structures::{HashMap, HashSet}; + +use semver::Version; + +use pubgrub::SelectedDependencies; + +use crate::workspace::dependency::DepKind; +use crate::resolver::Resolve; +use crate::resolver::ResolveVersion; +use crate::workspace::{Dependency, PackageId, Registry, SourceId, Summary}; +use crate::util::Graph; +use crate::util::errors::CargoResult; +use crate::util::interning::{INTERNED_DEFAULT, InternedString}; + +use super::package::{BucketName, FeatureNamespace, PubGrubPackage}; +use super::provider::Provider; +use super::semver_pubgrub::SemverCompatibility; + +/// Per-package activation facts gathered from the PubGrub solution. +#[derive(Default)] +struct Activation { + /// Activated named features (including `default`). + features: BTreeSet, + /// Activated optional dependencies (the toml names that appeared as + /// `BucketFeatures{.., Dep(name)}` in the solution). + deps: HashSet, + /// Whether this package was resolved as a workspace member (dev-deps). + member: bool, +} + +pub(super) fn into_resolve( + provider: &Provider<'_, T>, + solution: &SelectedDependencies, + resolve_version: ResolveVersion, +) -> CargoResult { + // (name, source) -> selected packages (one per compatibility bucket). The + // key is the *bucket* identity (the source the dependency named), while the + // value is the resolved [`PackageId`], whose source may differ when the + // package was redirected by `[patch]` (e.g. a `crates-io` dep satisfied by + // a path patch). See [`bucket_pid`]. + let mut selected: HashMap<(InternedString, SourceId), BTreeSet> = HashMap::default(); + // PackageId -> activation facts. + let mut activations: HashMap = HashMap::default(); + let mut package_ids: BTreeSet = BTreeSet::new(); + // Resolved summary per node. Captured here (keyed by the patched + // [`PackageId`]) so later passes don't re-query by a source that no longer + // matches the queryer's bucket cache. + let mut summaries: HashMap = HashMap::default(); + + for (pkg, version) in solution.iter() { + match pkg { + PubGrubPackage::Bucket { + name, + member, + all_features: _, + } => { + let (pid, summary) = bucket_pid(provider, name, version)?; + package_ids.insert(pid); + summaries.insert(pid, summary); + selected + .entry((name.name, name.source)) + .or_default() + .insert(pid); + let act = activations.entry(pid).or_default(); + act.member |= *member; + } + PubGrubPackage::BucketFeatures { name, feature } => { + let (pid, _) = bucket_pid(provider, name, version)?; + let act = activations.entry(pid).or_default(); + match feature { + FeatureNamespace::Feat(f) => { + act.features.insert(*f); + } + // Optional-dependency activations don't contribute to the + // user-facing feature list, but do gate optional edges. + FeatureNamespace::Dep(d) => { + act.deps.insert(*d); + } + } + } + PubGrubPackage::BucketDefaultFeatures { name } => { + let (pid, _) = bucket_pid(provider, name, version)?; + activations + .entry(pid) + .or_default() + .features + .insert(INTERNED_DEFAULT); + } + // Wide/links/root packages are not real graph nodes. + PubGrubPackage::Root + | PubGrubPackage::Wide { .. } + | PubGrubPackage::WideFeatures { .. } + | PubGrubPackage::WideDefaultFeatures { .. } + | PubGrubPackage::Links { .. } => {} + } + } + + // Build the dependency graph. + let mut graph: Graph> = Graph::new(); + for pid in &package_ids { + graph.add(*pid); + } + + for pid in &package_ids { + let Some(summary) = summaries.get(pid) else { + anyhow::bail!("pubgrub selected `{pid}` but it has no summary"); + }; + let act = activations.get(pid); + let member = act.is_some_and(|a| a.member); + for dep in summary.dependencies() { + // Determine whether this dependency is part of the resolved graph: + // + // * dev-dependencies are only recorded for workspace members; + // * optional dependencies are recorded only when activated (some + // feature turned them on), so that unactivated optional deps do + // not introduce spurious edges (and cycles); + // * all other dependencies are always recorded. + let active = match dep.kind() { + DepKind::Development => member, + _ => { + !dep.is_optional() || act.is_some_and(|a| a.deps.contains(&dep.name_in_toml())) + } + }; + if !active { + continue; + } + let Some(child) = resolve_child(provider, dep, pid, solution, &selected) else { + // An active dependency with no resolved child indicates a bug + // in the encoding rather than a benign skip. + anyhow::bail!( + "pubgrub could not map dependency `{}` of `{pid}` to a resolved package", + dep.package_name() + ); + }; + graph.link(*pid, child).insert(dep.clone()); + } + } + + // Checksums, features and replacements. + let mut cksums = HashMap::default(); + let mut features: HashMap> = HashMap::default(); + let mut replacements = HashMap::default(); + // Replacement targets (the `to` side of `[replace]`) to fold in as resolved + // packages once the registry borrow below is released. + let mut replacement_targets: Vec = Vec::new(); + { + let registry = provider.registry(); + for pid in &package_ids { + let summary = &summaries[pid]; + cksums.insert(*pid, summary.checksum().map(|s| s.to_string())); + if let Some((from, to)) = registry.used_replacement_for(*pid) { + replacements.insert(from, to); + // Unlike `[patch]`, `[replace]` keeps the replaced package as a + // graph node and redirects dependency edges to the replacement + // via `Resolve::deps`/`replacement`. The replacement package + // must therefore also be a resolved node (with its summary and + // checksum) so the package set can find it, mirroring how the + // default resolver activates the replacement summary. + if let Some(replacement) = registry.replacement_summary(*pid) { + replacement_targets.push(replacement); + } + } + if let Some(act) = activations.get(pid) { + let mut feats: Vec = act.features.iter().copied().collect(); + feats.sort_unstable(); + features.insert(*pid, feats); + } + } + } + for replacement in replacement_targets { + let to = replacement.package_id(); + graph.add(to); + cksums + .entry(to) + .or_insert_with(|| replacement.checksum().map(|s| s.to_string())); + summaries.entry(to).or_insert(replacement); + } + + let resolve = Resolve::new( + graph, + replacements, + features, + cksums, + BTreeMap::new(), + Vec::new(), + resolve_version, + summaries, + ); + + super::super::check_cycles(&resolve)?; + super::super::check_duplicate_pkgs_in_lockfile(&resolve)?; + Ok(resolve) +} + +/// Find the resolved child [`PackageId`] that satisfies `dep` from `parent`. +/// +/// The lookup is keyed by the *bucket* `(name, source)` the dependency named, +/// but the returned [`PackageId`] is the one actually selected for that bucket, +/// whose source may differ when the dependency was redirected by `[patch]`. +fn resolve_child( + provider: &Provider<'_, T>, + dep: &Dependency, + parent: &PackageId, + solution: &SelectedDependencies, + selected: &HashMap<(InternedString, SourceId), BTreeSet>, +) -> Option { + let (cray, _) = provider.from_dep(dep, parent.name(), parent.version()); + let (name, source, compat) = match cray { + PubGrubPackage::Bucket { ref name, .. } => (name.name, name.source, name.compat), + PubGrubPackage::Wide { ref name } => { + // The wide package chose a bucket; read it from the solution. + let chosen = solution.get(&cray)?; + (name.name, name.source, SemverCompatibility::from(chosen)) + } + _ => return None, + }; + let pids = selected.get(&(name, source))?; + pids.iter() + .find(|pid| SemverCompatibility::from(pid.version()) == compat) + .copied() +} + +/// Resolve a [`BucketName`] + version to the selected package and its summary. +/// +/// The bucket names a `(crate, source)`, but `[patch]` can redirect a query to a +/// summary from a *different* source (e.g. a `crates-io` requirement satisfied +/// by a path patch). [`Provider::summary_for`] returns that real summary; we use +/// its [`PackageId`] — carrying the patched source — as the node identity, so +/// the lockfile records the package the patch actually provided rather than the +/// nominal registry source. +fn bucket_pid( + provider: &Provider<'_, T>, + name: &BucketName, + version: &Version, +) -> CargoResult<(PackageId, Summary)> { + let Some(summary) = provider.summary_for(name.name, name.source, version)? else { + anyhow::bail!( + "pubgrub selected `{} {}` from `{}` but it has no summary", + name.name, + version, + name.source + ); + }; + Ok((summary.package_id(), summary)) +} diff --git a/src/sources/git/utils.rs b/src/sources/git/utils.rs index 69f33ae3f16..0c9b04977d6 100644 --- a/src/sources/git/utils.rs +++ b/src/sources/git/utils.rs @@ -1,6 +1,7 @@ //! Utilities for handling git repositories, mainly around //! authentication/cloning. +use crate::context::ProgressWhen; use crate::sources::git::fetch::RemoteKind; use crate::sources::git::oxide; use crate::sources::git::oxide::cargo_config_to_gitoxide_overrides; @@ -881,6 +882,7 @@ where /// `git reset --hard` to the given `obj` for the `repo`. /// /// The `obj` is a commit-ish to which the head should be moved. +#[tracing::instrument(skip_all)] fn reset(repo: &git2::Repository, obj: &git2::Object<'_>, gctx: &GlobalContext) -> CargoResult<()> { let mut pb = Progress::new("Checkout", gctx); let mut opts = git2::build::CheckoutBuilder::new(); @@ -1005,6 +1007,7 @@ pub fn with_fetch_options( /// at this time. It could be extended when libgit2 supports shallow clones. /// /// [`-Zgitoxide`]: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#gitoxide +#[tracing::instrument(skip_all)] pub fn fetch( repo: &mut git2::Repository, remote_url: &str, @@ -1135,6 +1138,7 @@ fn has_shallow_lock_file(err: &crate::sources::git::fetch::Error) -> bool { /// speed and portability of using `libgit2`. /// /// [1]: https://doc.rust-lang.org/nightly/cargo/reference/config.html#netgit-fetch-with-cli +#[tracing::instrument(skip(repo, gctx))] fn fetch_with_cli( repo: &mut git2::Repository, url: &str, @@ -1156,15 +1160,25 @@ fn fetch_with_cli( let depth = 0i32.saturating_add_unsigned(depth.get()); cmd.arg(format!("--depth={depth}")); } - match gctx.shell().verbosity() { - Verbosity::Normal => {} - Verbosity::Verbose => { - cmd.arg("--verbose"); - } - Verbosity::Quiet => { - cmd.arg("--quiet"); + + let progress_config = gctx.progress_config(); + let progress = match progress_config.when { + ProgressWhen::Always => true, + ProgressWhen::Never => false, + ProgressWhen::Auto => { + // Recreate the same conditions used with `Progress` + let width = progress_config + .width + .or_else(|| gctx.shell().err_width().progress_max_width()); + gctx.shell().progress_supported() && width.is_some() } + }; + if gctx.shell().verbosity() == Verbosity::Verbose { + cmd.arg("--verbose"); + } else if !progress { + cmd.arg("--quiet"); } + cmd.arg("--force") // handle force pushes .arg("--update-head-ok") // see discussion in #2078 .arg(url) @@ -1183,13 +1197,21 @@ fn fetch_with_cli( gctx.shell() .verbose(|s| s.status("Running", &cmd.to_string()))?; network::retry::with_retry(gctx, || { - cmd.exec() - .map_err(|error| GitCliError::new(error, true).into()) + cmd.exec().map_err(|error| { + GitCliError::new(error) + .spurious(true) + .workaround( + "help: re-try with `net.git-fetch-with-cli = false` to see if it resolves the problem +https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli", + ) + .into() + }) })?; Ok(()) } +#[tracing::instrument(skip(repo, gctx))] fn fetch_with_gitoxide( repo: &mut git2::Repository, remote_url: &str, @@ -1301,6 +1323,7 @@ fn fetch_with_gitoxide( res } +#[tracing::instrument(skip(repo, gctx))] fn fetch_with_libgit2( repo: &mut git2::Repository, remote_url: &str, @@ -1552,6 +1575,7 @@ enum FastPathRev { /// this function and move forward on the normal path. /// /// [^1]: +#[tracing::instrument(skip(repo, gctx))] fn github_fast_path( repo: &mut git2::Repository, url: &str, diff --git a/src/sources/path.rs b/src/sources/path.rs index 6c7ddd7dd0a..051ed94e63f 100644 --- a/src/sources/path.rs +++ b/src/sources/path.rs @@ -36,8 +36,8 @@ pub struct PathSource<'gctx> { source_id: SourceId, /// The root path of this source. path: PathBuf, - /// Packages that this sources has discovered. - package: RefCell>, + /// The package discovered in this source, if any. + package: RefCell>>, gctx: &'gctx GlobalContext, } @@ -63,23 +63,23 @@ impl<'gctx> PathSource<'gctx> { Self { source_id, path, - package: RefCell::new(Some(pkg)), + package: RefCell::new(Some(Some(pkg))), gctx, } } - /// Gets the package on the root path. + /// Returns the root package, or an error if it is missing or failed to load. pub fn root_package(&mut self) -> CargoResult { trace!("root_package; source={:?}", self); self.load()?; match &*self.package.borrow() { - Some(pkg) => Ok(pkg.clone()), - None => Err(internal(format!( - "no package found in source {:?}", - self.path - ))), + Some(Some(pkg)) => Ok(pkg.clone()), + Some(None) | None => Err(anyhow::format_err!( + "failed to read `{}`", + self.path.join("Cargo.toml").display() + )), } } @@ -124,10 +124,14 @@ impl<'gctx> PathSource<'gctx> { Ok(()) } - fn read_package(&self) -> CargoResult { + /// Reads the manifest. Returning `Ok(None)` if missing allows the resolver + /// to handle it as "not found" instead of an early IO error. + fn read_package(&self) -> CargoResult> { let path = self.path.join("Cargo.toml"); - let pkg = ops::read_package(&path, self.source_id, self.gctx)?; - Ok(pkg) + if !path.exists() { + return Ok(None); + } + Ok(Some(ops::read_package(&path, self.source_id, self.gctx)?)) } } @@ -146,7 +150,8 @@ impl<'gctx> Source for PathSource<'gctx> { f: &mut dyn FnMut(IndexSummary), ) -> CargoResult<()> { self.load()?; - if let Some(s) = self.package.borrow().as_ref().map(|p| p.summary()) { + if let Some(Some(p)) = &*self.package.borrow() { + let s = p.summary(); let matched = match kind { QueryKind::Exact | QueryKind::RejectedVersions => dep.matches(s), QueryKind::AlternativeNames => true, @@ -175,7 +180,10 @@ impl<'gctx> Source for PathSource<'gctx> { trace!("getting packages; id={}", id); self.load()?; let pkg = self.package.borrow(); - let pkg = pkg.iter().find(|pkg| pkg.package_id() == id); + let pkg = pkg + .as_ref() + .and_then(|p| p.as_ref()) + .filter(|pkg| pkg.package_id() == id); pkg.cloned() .map(MaybePackage::Ready) .ok_or_else(|| internal(format!("failed to find {} in path source", id))) diff --git a/src/sources/registry/download.rs b/src/sources/registry/download.rs index b4b15e389b1..453d8f6db60 100644 --- a/src/sources/registry/download.rs +++ b/src/sources/registry/download.rs @@ -1,7 +1,7 @@ -//! Shared download logic between [`HttpRegistry`] and [`RemoteRegistry`]. +//! Shared download logic between [`HttpRegistry`] and [`GitRegistry`]. //! //! [`HttpRegistry`]: super::http_remote::HttpRegistry -//! [`RemoteRegistry`]: super::remote::RemoteRegistry +//! [`GitRegistry`]: super::git_remote::GitRegistry use crate::util::interning::InternedString; use anyhow::Context as _; diff --git a/src/sources/registry/remote.rs b/src/sources/registry/git_remote.rs similarity index 95% rename from src/sources/registry/remote.rs rename to src/sources/registry/git_remote.rs index 402e1aac276..b030ee5d123 100644 --- a/src/sources/registry/remote.rs +++ b/src/sources/registry/git_remote.rs @@ -1,4 +1,4 @@ -//! Access to a Git index based registry. See [`RemoteRegistry`] for details. +//! Access to a Git index based registry. See [`GitRegistry`] for details. use crate::sources::git; use crate::sources::git::fetch::RemoteKind; @@ -46,7 +46,7 @@ use tracing::{debug, trace}; /// supporting Git-based index for a pretty long while. /// /// [`HttpRegistry`]: super::http_remote::HttpRegistry -pub struct RemoteRegistry<'gctx> { +pub struct GitRegistry<'gctx> { /// The name of this source, a unique string (across all sources) used as /// the directory name where its cached content is stored. name: InternedString, @@ -76,24 +76,20 @@ pub struct RemoteRegistry<'gctx> { current_sha: Cell>, /// Whether this registry needs to update package information. /// - /// See [`RemoteRegistry::mark_updated`] on how to make sure a registry + /// See [`GitRegistry::mark_updated`] on how to make sure a registry /// index is updated only once per session. needs_update: Cell, /// Disables status messages. quiet: bool, } -impl<'gctx> RemoteRegistry<'gctx> { +impl<'gctx> GitRegistry<'gctx> { /// Creates a Git-rebased remote registry for `source_id`. /// /// * `name` --- Name of a path segment where `.crate` tarballs and the /// registry index are stored. Expect to be unique. - pub fn new( - source_id: SourceId, - gctx: &'gctx GlobalContext, - name: &str, - ) -> RemoteRegistry<'gctx> { - RemoteRegistry { + pub fn new(source_id: SourceId, gctx: &'gctx GlobalContext, name: &str) -> GitRegistry<'gctx> { + GitRegistry { name: name.into(), index_path: gctx.registry_index_path().join(name), cache_path: gctx.registry_cache_path().join(name), @@ -185,9 +181,9 @@ impl<'gctx> RemoteRegistry<'gctx> { // Note that we don't actually hand out the static lifetime, instead we // only return a scoped one from this function. Additionally the repo // we loaded from (above) lives as long as this object - // (`RemoteRegistry`) so we then just need to ensure that the tree is + // (`GitRegistry`) so we then just need to ensure that the tree is // destroyed first in the destructor, hence the destructor on - // `RemoteRegistry` below. + // `GitRegistry` below. let tree = unsafe { mem::transmute::, git2::Tree<'static>>(tree) }; *self.tree.borrow_mut() = Some(tree); Ok(Ref::map(self.tree.borrow(), |s| s.as_ref().unwrap())) @@ -281,7 +277,7 @@ impl<'gctx> RemoteRegistry<'gctx> { } #[async_trait::async_trait(?Send)] -impl<'gctx> RegistryData for RemoteRegistry<'gctx> { +impl<'gctx> RegistryData for GitRegistry<'gctx> { fn prepare(&self) -> CargoResult<()> { self.repo()?; self.gctx @@ -339,7 +335,7 @@ impl<'gctx> RegistryData for RemoteRegistry<'gctx> { // in the index, so we don't need to worry about an `update_index` // happening in a different process. fn load_helper( - registry: &RemoteRegistry<'_>, + registry: &GitRegistry<'_>, path: &Path, index_version: Option<&str>, ) -> CargoResult { @@ -410,7 +406,7 @@ impl<'gctx> RegistryData for RemoteRegistry<'gctx> { /// Read the general concept for `invalidate_cache()` on /// [`RegistryData::invalidate_cache`]. /// - /// To fully invalidate, undo [`RemoteRegistry::mark_updated`]'s work. + /// To fully invalidate, undo [`GitRegistry::mark_updated`]'s work. fn invalidate_cache(&self) { self.needs_update.set(true); } @@ -458,8 +454,8 @@ impl<'gctx> RegistryData for RemoteRegistry<'gctx> { } /// Implemented to just be sure to drop `tree` field before our other fields. -/// See SAFETY inside [`RemoteRegistry::tree()`] for more. -impl<'gctx> Drop for RemoteRegistry<'gctx> { +/// See SAFETY inside [`GitRegistry::tree()`] for more. +impl<'gctx> Drop for GitRegistry<'gctx> { fn drop(&mut self) { self.tree.borrow_mut().take(); } diff --git a/src/sources/registry/index/cache.rs b/src/sources/registry/index/cache.rs index ef0cbe0a74b..dcadd4be9c5 100644 --- a/src/sources/registry/index/cache.rs +++ b/src/sources/registry/index/cache.rs @@ -26,7 +26,7 @@ //! of trying to parse as little as possible. //! //! > Note that as a small aside even *loading* the JSON from the registry is -//! > actually pretty slow. For crates.io and [`RemoteRegistry`] we don't +//! > actually pretty slow. For crates.io and [`GitRegistry`] we don't //! > actually check out the git index on disk because that takes quite some //! > time and is quite large. Instead we use `libgit2` to read the JSON from //! > the raw git objects. This in turn can be slow (aka show up high in @@ -63,7 +63,7 @@ //! [`Dependency`]: crate::workspace::Dependency //! [`IndexPackage`]: super::IndexPackage //! [`IndexSummary::parse`]: super::IndexSummary::parse -//! [`RemoteRegistry`]: crate::sources::registry::remote::RemoteRegistry +//! [`GitRegistry`]: crate::sources::registry::git_remote::GitRegistry use std::cell::RefCell; use std::fs; diff --git a/src/sources/registry/index/mod.rs b/src/sources/registry/index/mod.rs index d875e129f1a..9a6b8ab616f 100644 --- a/src/sources/registry/index/mod.rs +++ b/src/sources/registry/index/mod.rs @@ -55,7 +55,7 @@ const INDEX_V_MAX: u32 = 2; /// Different kinds of registries store the index differently: /// /// * [`LocalRegistry`] is a simple on-disk tree of files of the raw index. -/// * [`RemoteRegistry`] is stored as a raw git repository. +/// * [`GitRegistry`] is stored as a raw git repository. /// * [`HttpRegistry`] fills the on-disk index cache directly without keeping /// any raw index. /// @@ -63,7 +63,7 @@ const INDEX_V_MAX: u32 = 2; /// This transparently handles caching of the index in a more efficient format. /// /// [`LocalRegistry`]: super::local::LocalRegistry -/// [`RemoteRegistry`]: super::remote::RemoteRegistry +/// [`GitRegistry`]: super::git_remote::GitRegistry /// [`HttpRegistry`]: super::http_remote::HttpRegistry pub struct RegistryIndex<'gctx> { source_id: SourceId, @@ -349,12 +349,12 @@ impl<'gctx> RegistryIndex<'gctx> { /// /// The actual kind index file being parsed depends on which kind of /// [`RegistryData`] the `load` argument is given. For example, a - /// Git-based [`RemoteRegistry`] will first try a on-disk index cache + /// Git-based [`GitRegistry`] will first try a on-disk index cache /// file, and then try parsing registry raw index from Git repository. /// /// In effect, this is intended to be a quite cheap operation. /// - /// [`RemoteRegistry`]: super::remote::RemoteRegistry + /// [`GitRegistry`]: super::git_remote::GitRegistry async fn load_summaries( &self, name: InternedString, diff --git a/src/sources/registry/mod.rs b/src/sources/registry/mod.rs index e43e61b8d98..4394af7ea94 100644 --- a/src/sources/registry/mod.rs +++ b/src/sources/registry/mod.rs @@ -32,7 +32,7 @@ //! //! * [`LocalRegistry`] --- Serves the index and package contents entirely on //! a local filesystem. -//! * [`RemoteRegistry`] --- Serves the index ahead of time from a Git +//! * [`GitRegistry`] --- Serves the index ahead of time from a Git //! repository, and package contents are downloaded as needed. //! * [`HttpRegistry`] --- Serves both the index and package contents on demand //! over a HTTP-based registry API. This is the default starting from 1.70.0. @@ -41,7 +41,7 @@ //! created from either [`RegistrySource::local`] or [`RegistrySource::remote`]. //! //! [`LocalRegistry`]: local::LocalRegistry -//! [`RemoteRegistry`]: remote::RemoteRegistry +//! [`GitRegistry`]: git_remote::GitRegistry //! [`HttpRegistry`]: http_remote::HttpRegistry //! //! # The Index of a Registry @@ -401,8 +401,8 @@ mod download; mod http_remote; pub(crate) mod index; pub use index::IndexSummary; +mod git_remote; mod local; -mod remote; /// Generates a unique name for [`SourceId`] to have a unique path to put their /// index files. @@ -424,7 +424,7 @@ fn short_name(id: SourceId, is_shallow: bool) -> String { impl<'gctx> RegistrySource<'gctx> { /// Creates a [`Source`] of a "remote" registry. /// It could be either an HTTP-based [`http_remote::HttpRegistry`] or - /// a Git-based [`remote::RemoteRegistry`]. + /// a Git-based [`git_remote::GitRegistry`]. pub fn remote( source_id: SourceId, gctx: &'gctx GlobalContext, @@ -440,7 +440,7 @@ impl<'gctx> RegistrySource<'gctx> { let ops = if source_id.is_sparse() { Box::new(http_remote::HttpRegistry::new(source_id, gctx, &name)?) as Box<_> } else { - Box::new(remote::RemoteRegistry::new(source_id, gctx, &name)) as Box<_> + Box::new(git_remote::GitRegistry::new(source_id, gctx, &name)) as Box<_> }; Ok(RegistrySource::new(source_id, gctx, &name, ops)) diff --git a/src/util/errors.rs b/src/util/errors.rs index 69c2ba6c25c..3f15392f64d 100644 --- a/src/util/errors.rs +++ b/src/util/errors.rs @@ -375,11 +375,26 @@ pub type GitCliResult = Result<(), GitCliError>; pub struct GitCliError { inner: Error, is_spurious: bool, + workaround: Option<&'static str>, } impl GitCliError { - pub fn new(inner: Error, is_spurious: bool) -> GitCliError { - GitCliError { inner, is_spurious } + pub fn new(inner: Error) -> GitCliError { + GitCliError { + inner, + is_spurious: false, + workaround: None, + } + } + + pub fn workaround(mut self, workaround: &'static str) -> Self { + self.workaround = Some(workaround); + self + } + + pub fn spurious(mut self, yes: bool) -> Self { + self.is_spurious = yes; + self } pub fn is_spurious(&self) -> bool { @@ -395,7 +410,13 @@ impl std::error::Error for GitCliError { impl fmt::Display for GitCliError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.inner.fmt(f) + self.inner.fmt(f)?; + if let Some(workaround) = self.workaround { + writeln!(f)?; + writeln!(f)?; + write!(f, "{workaround}")?; + } + Ok(()) } } diff --git a/src/util/network/retry.rs b/src/util/network/retry.rs index 2dfe117ecb9..839bbfa4dfd 100644 --- a/src/util/network/retry.rs +++ b/src/util/network/retry.rs @@ -422,9 +422,9 @@ fn retry_after_parsing() { #[test] fn git_cli_error_spurious() { - let error = GitCliError::new(Error::msg("test-git-cli-error"), false); + let error = GitCliError::new(Error::msg("test-git-cli-error")).spurious(false); assert!(!maybe_spurious(&error.into())); - let error = GitCliError::new(Error::msg("test-git-cli-error"), true); + let error = GitCliError::new(Error::msg("test-git-cli-error")).spurious(true); assert!(maybe_spurious(&error.into())); } diff --git a/src/util/progress.rs b/src/util/progress.rs index e938e5b7116..64464450adc 100644 --- a/src/util/progress.rs +++ b/src/util/progress.rs @@ -6,9 +6,7 @@ use std::time::{Duration, Instant}; use crate::context::ProgressWhen; use crate::util::{CargoResult, GlobalContext}; use anstyle_progress::TermProgress; -use cargo_util::is_ci; use cargo_util_terminal::Shell; -use cargo_util_terminal::Verbosity; use unicode_width::UnicodeWidthChar; /// CLI progress bar. @@ -56,23 +54,17 @@ impl<'gctx> Progress<'gctx> { style: ProgressStyle, gctx: &'gctx GlobalContext, ) -> Progress<'gctx> { - // report no progress when -q (for quiet) or TERM=dumb are set - // or if running on Continuous Integration service like Travis where the - // output logs get mangled. - let dumb = match gctx.get_env("TERM") { - Ok(term) => term == "dumb", - Err(_) => false, - }; let progress_config = gctx.progress_config(); - match progress_config.when { - ProgressWhen::Always => return Progress::new_priv(name, style, gctx), - ProgressWhen::Never => return Progress { gctx, state: None }, - ProgressWhen::Auto => {} - } - if gctx.shell().verbosity() == Verbosity::Quiet || dumb || is_ci() { + let progress = match progress_config.when { + ProgressWhen::Always => true, + ProgressWhen::Never => false, + ProgressWhen::Auto => gctx.shell().progress_supported(), + }; + if progress { + Progress::new_priv(name, style, gctx) + } else { return Progress { gctx, state: None }; } - Progress::new_priv(name, style, gctx) } fn new_priv(name: &str, style: ProgressStyle, gctx: &'gctx GlobalContext) -> Progress<'gctx> { diff --git a/src/workspace/features.rs b/src/workspace/features.rs index 6a270d9af9e..e8c83fd3d2b 100644 --- a/src/workspace/features.rs +++ b/src/workspace/features.rs @@ -408,18 +408,6 @@ enum Status { Removed, } -/// Config for the `-Zembed-metadata` option. -#[derive(Debug, Default, Deserialize)] -pub enum EmbedMetadata { - /// Embed metadata in .rlib files, the original rustc behavior. - Embed, - /// Do not embed metadata in .rlib files. - DoNotEmbed, - /// The `-Zembed-metadata` flag wasn't set. - #[default] - Unset, -} - /// A listing of stable and unstable new syntax in Cargo.toml. /// /// This generates definitions and impls for [`Features`] and [`Feature`] @@ -826,7 +814,7 @@ macro_rules! unstable_cli_options { /// Cargo, like `rustc`, accepts a suite of `-Z` flags which are intended for /// gating unstable functionality to Cargo. These flags are only available on /// the nightly channel of Cargo. - #[derive(Default, Debug, Deserialize)] + #[derive(Debug, Deserialize)] #[serde(default, rename_all = "kebab-case")] pub struct CliUnstable { $( @@ -842,6 +830,23 @@ macro_rules! unstable_cli_options { fields } } + impl Default for CliUnstable { + fn default() -> Self { + let mut unstable = Self { + $( + $element: Default::default() + ),* + }; + + // Defaults to enabled on nightly unless explicitly opted out. + if !is_new_build_dir_layout_opt_out() { + unstable.build_dir_new_layout = + matches!(crate::version().release_channel.as_deref(), Some("nightly" | "dev")); + } + + return unstable; + } + } #[cfg(test)] mod test { @@ -868,7 +873,7 @@ macro_rules! unstable_cli_options { unstable_cli_options!( // Permanently unstable features: allow_features: Option = ("Allow *only* the listed unstable features"), - embed_metadata: EmbedMetadata = ("Avoid embedding metadata in library artifacts"), + embed_metadata: Option = ("Avoid embedding metadata in library artifacts"), print_im_a_teapot: bool, // All other unstable features. @@ -912,6 +917,7 @@ unstable_cli_options!( panic_immediate_abort: bool = ("Enable setting `panic = \"immediate-abort\"` in profiles"), profile_hint_mostly_unused: bool = ("Enable the `hint-mostly-unused` setting in profiles to mark a crate as mostly unused."), profile_rustflags: bool = ("Enable the `rustflags` option in profiles in .cargo/config.toml file"), + pubgrub_resolver: bool = ("Use the PubGrub dependency resolver instead of the default resolver"), public_dependency: bool = ("Respect a dependency's `public` field in Cargo.toml to control public/private dependencies"), publish_timeout: bool = ("Enable the `publish.timeout` key in .cargo/config.toml file"), root_dir: Option = ("Set the root directory relative to which paths are printed (defaults to workspace root)"), @@ -1283,12 +1289,6 @@ impl CliUnstable { self.gitoxide = GitoxideFeatures::safe().into(); } - // NOTE: We set this before `implicitly_enable_features_if_needed` as `-Zfine-grain-locking` - // must use the new layout so that takes priority. - if is_new_build_dir_layout_opt_out() { - self.build_dir_new_layout = false; - } - self.implicitly_enable_features_if_needed(); Ok(warnings) @@ -1307,11 +1307,11 @@ impl CliUnstable { } } - fn parse_embed_metadata(key: &str, value: Option<&str>) -> CargoResult { + fn parse_option_bool(key: &str, value: Option<&str>) -> CargoResult> { match value { - None => Ok(EmbedMetadata::Unset), - Some("yes") => Ok(EmbedMetadata::Embed), - Some("no") => Ok(EmbedMetadata::DoNotEmbed), + None => Ok(None), + Some("yes") => Ok(Some(true)), + Some("no") => Ok(Some(false)), Some(s) => bail!("flag -Z{key} expected `no` or `yes`, found: `{s}`"), } } @@ -1367,7 +1367,7 @@ impl CliUnstable { // Permanently unstable features // Sorted alphabetically: "allow-features" => self.allow_features = Some(parse_list(v).into_iter().collect()), - "embed-metadata" => self.embed_metadata = parse_embed_metadata(k, v)?, + "embed-metadata" => self.embed_metadata = parse_option_bool(k, v)?, "print-im-a-teapot" => self.print_im_a_teapot = parse_bool(k, v)?, // Stabilized features @@ -1472,6 +1472,7 @@ impl CliUnstable { "no-index-update" => self.no_index_update = parse_empty(k, v)?, "panic-abort-tests" => self.panic_abort_tests = parse_empty(k, v)?, "public-dependency" => self.public_dependency = parse_empty(k, v)?, + "pubgrub-resolver" => self.pubgrub_resolver = parse_empty(k, v)?, "profile-hint-mostly-unused" => self.profile_hint_mostly_unused = parse_empty(k, v)?, "profile-rustflags" => self.profile_rustflags = parse_empty(k, v)?, "trim-paths" => self.trim_paths = parse_empty(k, v)?, diff --git a/src/workspace/parser/mod.rs b/src/workspace/parser/mod.rs index 4098ef06dd0..6e57423d0b6 100644 --- a/src/workspace/parser/mod.rs +++ b/src/workspace/parser/mod.rs @@ -2698,7 +2698,23 @@ supported tools: {}", if tool == "cargo" && !gctx.cli_unstable().cargo_lints { warn_for_cargo_lint_feature(gctx, warnings); } + let mut seen_normalized: HashMap = HashMap::default(); for (name, config) in lints { + let normalized = name.replace('-', "_"); + if name.contains('-') { + warnings.push(format!( + "`lints.{tool}.{name}` is deprecated in favor of \ + `lints.{tool}.{normalized}` and will not work in a \ + future edition" + )); + } + if let Some(existing) = seen_normalized.get(&normalized) { + warnings.push(format!( + "duplicate lint `{existing}` in `[lints.{tool}]`, \ + conflicts with `{name}` and will not work in a future edition" + )); + } + seen_normalized.insert(normalized.clone(), name.to_string()); if let Some((prefix, suffix)) = name.split_once("::") { if tool == prefix { anyhow::bail!( diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index e7fe9c4a954..06eae295266 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -1308,6 +1308,7 @@ fn cargo_compile_with_dep_name_mismatch() { [ERROR] no matching package named `notquitebar` found location searched: [ROOT]/foo/bar required by package `foo v0.0.1 ([ROOT]/foo)` +[HELP] package `bar` exists at `[ROOT]/foo/bar` "#]]) .run(); @@ -6736,3 +6737,109 @@ qux (cdylib) on search path: true "#]]) .run(); } + +#[cargo_test( + nightly, + reason = "Depends on https://github.com/rust-lang/rust/pull/155439/changes/61f3e086acc1c187bb262ab43cac71f44018c397" +)] +fn should_not_include_proc_macro_deps_paths_in_rustc_args() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.0" + edition = "2021" + authors = [] + resolver = "2" + + [dependencies] + my-proc-macro = { path = "my-proc-macro" } + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "my-dylib/Cargo.toml", + r#" + [package] + name = "my-dylib" + version = "0.1.0" + edition = "2021" + authors = [] + + [lib] + crate-type = ["dylib"] + "#, + ) + .file( + "my-dylib/src/lib.rs", + r#"pub fn value_from_dylib() -> i32 { 100 }"#, + ) + .file( + "my-rlib/Cargo.toml", + r#" + [package] + name = "my-rlib" + version = "0.1.0" + edition = "2021" + authors = [] + "#, + ) + .file( + "my-rlib/src/lib.rs", + r#"pub fn value_from_rlib() -> i32 { 200 }"#, + ) + .file( + "my-proc-macro/Cargo.toml", + r#" + [package] + name = "my-proc-macro" + version = "0.1.0" + edition = "2021" + authors = [] + + [lib] + proc-macro = true + + [dependencies] + my-dylib = { path = "../my-dylib" } + my-rlib = { path = "../my-rlib" } + "#, + ) + .file( + "my-proc-macro/src/lib.rs", + r#" + use proc_macro::TokenStream; + use my_dylib::value_from_dylib; + use my_rlib::value_from_rlib; + + #[proc_macro] + pub fn make_bar(_item: TokenStream) -> TokenStream { + let val = value_from_dylib() + value_from_rlib(); + format!("fn bar() -> u32 {{ {val} }}").parse().unwrap() + } + "#, + ) + .build(); + + p.cargo("-Zbuild-dir-new-layout -v build") + .masquerade_as_nightly_cargo(&["new build-dir layout"]) + .enable_mac_dsym() + // Verify that the proc-macro dependencies (my-rlib and my-dylib) are not added to the rustc + // invocation for the `foo` crate as `-L` args. + .with_stderr_data(str![[r#" +[LOCKING] 3 packages to latest compatible versions +[COMPILING] my-dylib v0.1.0 ([ROOT]/foo/my-dylib) +[RUNNING] `rustc --crate-name my_dylib [..]` +[COMPILING] my-rlib v0.1.0 ([ROOT]/foo/my-rlib) +[RUNNING] `rustc --crate-name my_rlib [..]` +[COMPILING] my-proc-macro v0.1.0 ([ROOT]/foo/my-proc-macro) +[RUNNING] `rustc --crate-name my_proc_macro [..]` +[COMPILING] foo v0.0.0 ([ROOT]/foo) +[RUNNING] `rustc --crate-name foo [..] --out-dir [ROOT]/foo/target/debug/build/foo/[HASH]/out -L dependency=[ROOT]/foo/target/debug/build/my-proc-macro/[HASH]/out --extern my_proc_macro=[ROOT]/foo/target/debug/build/my-proc-macro/[HASH]/out/[..] --verbose` +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]].unordered()) + .run(); +} diff --git a/tests/testsuite/build_dir.rs b/tests/testsuite/build_dir.rs index deb5bcb57cc..eceec5e3666 100644 --- a/tests/testsuite/build_dir.rs +++ b/tests/testsuite/build_dir.rs @@ -1430,9 +1430,9 @@ fn new_layout_opt_out_nightly() { ) .build(); - p.cargo("-Zbuild-dir-new-layout build") + p.cargo("build") .env("__CARGO_TEMPORARY_BUILD_DIR_NEW_LAYOUT_OPT_OUT", "1") - .masquerade_as_nightly_cargo(&["new build-dir layout"]) + .masquerade_as_nightly_cargo(&["new build-dir layout enabled by default on nightly"]) .enable_mac_dsym() .run(); diff --git a/tests/testsuite/cargo/z_help/stdout.term.svg b/tests/testsuite/cargo/z_help/stdout.term.svg index f8f602766b3..402af4d6a5e 100644 --- a/tests/testsuite/cargo/z_help/stdout.term.svg +++ b/tests/testsuite/cargo/z_help/stdout.term.svg @@ -1,4 +1,4 @@ - +