From 53c0cbc6c9057e5628d824b12c812c1e6ece0e29 Mon Sep 17 00:00:00 2001 From: omskscream Date: Tue, 16 Dec 2025 21:59:00 +0200 Subject: [PATCH 01/12] test (patch): add test case for when patch is configured using CLI config options --- tests/testsuite/patch.rs | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index dc1b2083cca..1a6558b1e65 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -2315,6 +2315,45 @@ Caused by: .run(); } +#[cargo_test] +fn mismatched_version_from_cli_config() { + // A patch to a location that has an old version. + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + + [dependencies] + bar = "0.1.1" + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_manifest("bar", "0.1.0")) + .file("bar/src/lib.rs", "") + .build(); + + p.cargo("check") + .arg_line("--config 'patch.crates-io.bar.path=\"bar\"'") + .arg_line("--config 'patch.crates-io.bar.version=\"0.1.1\"'") + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to resolve patches for `https://github.com/rust-lang/crates.io-index` + +Caused by: + patch for `bar` in `https://github.com/rust-lang/crates.io-index` failed to resolve + +Caused by: + The patch location `[ROOT]/foo/bar` contains a `bar` package with version `0.1.0`, but the patch definition requires `^0.1.1`. + Check that the version in the patch location is what you expect, and update the patch definition to match. + +"#]]) + .run(); +} + #[cargo_test] fn patch_walks_backwards() { // Starting with a locked patch, change the patch so it points to an older version. From 8f792100023a44dc4b35f622a85d22eacaea106a Mon Sep 17 00:00:00 2001 From: omskscream Date: Wed, 17 Dec 2025 22:42:29 +0200 Subject: [PATCH 02/12] test (patch): add test case for patch that is defined via env variables and is ignored --- tests/testsuite/patch.rs | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index 1a6558b1e65..07a2a7736ed 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -2354,6 +2354,45 @@ Caused by: .run(); } +#[cargo_test] +fn patch_from_env_config_is_ignored() { + Package::new("bar", "1.0.0").publish(); // original dependency + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + + [dependencies] + bar = "1.0.0" + "#, + ) + .file("src/lib.rs", "") + .build(); + + // copy of the [mismatched_version_from_cli_config] cli options using conversion to env + // described in https://doc.rust-lang.org/cargo/reference/config.html#environment-variables + p.cargo("check") + .env("CARGO_PATCH_CRATES_IO_BAR_PATH", "bar") + .env("CARGO_PATCH_CRATES_IO_BAR_VERSION", "0.1.1") + .with_status(0) + .with_stderr_data(str![[r#" +[UPDATING] `dummy-registry` index +[LOCKING] 1 package to latest compatible version +[DOWNLOADING] crates ... +[DOWNLOADED] bar v1.0.0 (registry `dummy-registry`) +[CHECKING] bar v1.0.0 +[CHECKING] foo v0.1.0 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); +} + #[cargo_test] fn patch_walks_backwards() { // Starting with a locked patch, change the patch so it points to an older version. From e4151d5f06e47d408639d6cf4803d119c102885d Mon Sep 17 00:00:00 2001 From: omskscream Date: Wed, 17 Dec 2025 01:59:52 +0200 Subject: [PATCH 03/12] test (patch): add test case for when several patch entries defined in different places resolve to same version --- tests/testsuite/patch.rs | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index 07a2a7736ed..09458ce2cc0 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -1895,6 +1895,62 @@ fn patch_same_version() { .run(); } +#[cargo_test] +fn patch_same_version_different_patch_locations() { + let bar = git::repo(&paths::root().join("override")) + .file("Cargo.toml", &basic_manifest("bar", "0.1.0")) + .file("src/lib.rs", "") + .build(); + + registry::init(); + + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.0.1" + edition = "2015" + [dependencies] + bar = "0.1" + [patch.crates-io] + bar2 = {{ git = '{}', package = 'bar' }} + "#, + bar.url(), + ), + ) + .file( + ".cargo/config.toml", + r#" + [patch.crates-io] + bar = { path = "bar" } + "#, + ) + .file("src/lib.rs", "") + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version = "0.1.0" + edition = "2015" + "#, + ) + .file("bar/src/lib.rs", "") + .build(); + + p.cargo("check") + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] git repository `[ROOTURL]/override` +[ERROR] cannot have two `[patch]` entries which both resolve to `bar v0.1.0` + +"#]]) + .run(); +} + #[cargo_test] fn two_semver_compatible() { let bar = git::repo(&paths::root().join("override")) From be108bfe1dae557baa31a0531ee192b380c27558 Mon Sep 17 00:00:00 2001 From: omskscream Date: Wed, 17 Dec 2025 22:52:47 +0200 Subject: [PATCH 04/12] test (patch): add test case for patch that is defined in custom config file provided via cli --- tests/testsuite/patch.rs | 47 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index 09458ce2cc0..9b059d5d01d 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -2410,6 +2410,53 @@ Caused by: .run(); } +#[cargo_test] +fn mismatched_version_from_config_file_provided_via_cli() { + Package::new("bar", "0.1.1").publish(); // original dependency + + // A patch to a location that has an old version. + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + + [dependencies] + bar = "0.1.1" + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_manifest("bar", "0.1.0")) + .file("bar/src/lib.rs", "") + .file( + "tmp/my-config.toml", + r#" + [patch.crates-io] + bar = { path = 'bar', version = '0.1.1' } + "#, + ) + .build(); + + p.cargo("check") + .arg_line("--config tmp/my-config.toml") + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to resolve patches for `https://github.com/rust-lang/crates.io-index` + +Caused by: + patch for `bar` in `https://github.com/rust-lang/crates.io-index` failed to resolve + +Caused by: + The patch location `[ROOT]/foo/bar` contains a `bar` package with version `0.1.0`, but the patch definition requires `^0.1.1`. + Check that the version in the patch location is what you expect, and update the patch definition to match. + +"#]]) + .run(); +} + #[cargo_test] fn patch_from_env_config_is_ignored() { Package::new("bar", "1.0.0").publish(); // original dependency From ee4accbe209ebc3b804b987c1c72a82ecf9fd7b2 Mon Sep 17 00:00:00 2001 From: omskscream Date: Tue, 16 Dec 2025 21:25:00 +0200 Subject: [PATCH 05/12] refactor (patch): save manifest file path into manifest context instead of root dir --- src/cargo/util/toml/mod.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index bbe411b6227..17f1116c9f6 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1582,7 +1582,7 @@ pub fn to_real_manifest( gctx, warnings, platform: None, - root: package_root, + file: manifest_file, }; gather_dependencies( &mut manifest_ctx, @@ -1967,8 +1967,6 @@ fn to_virtual_manifest( warnings: &mut Vec, _errors: &mut Vec, ) -> CargoResult { - let root = manifest_file.parent().unwrap(); - let mut deps = Vec::new(); let (replace, patch) = { let mut manifest_ctx = ManifestContext { @@ -1977,7 +1975,7 @@ fn to_virtual_manifest( gctx, warnings, platform: None, - root, + file: manifest_file, }; ( replace(&normalized_toml, &mut manifest_ctx)?, @@ -2045,7 +2043,7 @@ struct ManifestContext<'a, 'b> { gctx: &'b GlobalContext, warnings: &'a mut Vec, platform: Option, - root: &'a Path, + file: &'a Path, } #[tracing::instrument(skip_all)] @@ -2163,7 +2161,7 @@ pub(crate) fn to_dependency( gctx: &GlobalContext, warnings: &mut Vec, platform: Option, - root: &Path, + file: &Path, kind: Option, ) -> CargoResult { dep_to_dependency( @@ -2175,7 +2173,7 @@ pub(crate) fn to_dependency( gctx, warnings, platform, - root, + file, }, kind, ) @@ -2415,7 +2413,7 @@ fn to_dependency_source_id( // always end up hashing to the same value no matter where it's // built from. if manifest_ctx.source_id.is_path() { - let path = manifest_ctx.root.join(path); + let path = manifest_ctx.file.parent().unwrap().join(path); let path = paths::normalize_path(&path); SourceId::for_path(&path) } else { From bae1bc790bd0c985d06128393f95bd964477ed6d Mon Sep 17 00:00:00 2001 From: omskscream Date: Tue, 16 Dec 2025 19:56:34 +0200 Subject: [PATCH 06/12] refactor (patch): introduce `Patch` to use instead of `Dependency` for resolving "patch" dependency overrides --- src/cargo/core/dependency.rs | 30 +++++++++++++++++++++++++++++- src/cargo/core/manifest.rs | 14 +++++++------- src/cargo/core/mod.rs | 2 +- src/cargo/core/registry.rs | 13 ++++++------- src/cargo/core/resolver/encode.rs | 4 ++-- src/cargo/core/workspace.rs | 21 +++++++++++++-------- src/cargo/ops/resolve.rs | 9 +++++---- src/cargo/util/toml/mod.rs | 13 +++++++++---- 8 files changed, 72 insertions(+), 34 deletions(-) diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index 216c6fba9f3..9b7cc2f61cc 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -4,13 +4,15 @@ use serde::Serialize; use serde::ser; use std::borrow::Cow; use std::fmt; -use std::path::PathBuf; +use std::fmt::{Display, Formatter}; +use std::path::{Path, PathBuf}; use std::sync::Arc; use tracing::trace; use crate::core::compiler::{CompileKind, CompileTarget}; use crate::core::{CliUnstable, Feature, Features, PackageId, SourceId, Summary}; use crate::util::OptVersionReq; +use crate::util::context::Definition; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; @@ -664,3 +666,29 @@ impl ArtifactKind { Ok(kinds) } } + +/// Patch is a dependency override that knows where it has been defined. +/// See [PatchLocation] for possible locations. +#[derive(Clone, Debug)] +pub struct Patch { + pub dep: Dependency, + pub loc: PatchLocation, +} + +/// Place where a patch has been defined. +#[derive(Clone, Debug)] +pub enum PatchLocation { + /// Defined in a manifest. + Manifest(PathBuf), + /// Defined in cargo configuration. + Config(Definition), +} + +impl Display for PatchLocation { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + PatchLocation::Manifest(p) => Path::display(p).fmt(f), + PatchLocation::Config(def) => def.fmt(f), + } + } +} diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index eb24127775e..652fddf1978 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -17,7 +17,7 @@ use url::Url; use crate::core::compiler::rustdoc::RustdocScrapeExamples; use crate::core::compiler::{CompileKind, CrateType}; use crate::core::resolver::ResolveBehavior; -use crate::core::{Dependency, PackageId, PackageIdSpec, SourceId, Summary}; +use crate::core::{Dependency, PackageId, PackageIdSpec, Patch, SourceId, Summary}; use crate::core::{Edition, Feature, Features, WorkspaceConfig}; use crate::util::errors::*; use crate::util::interning::InternedString; @@ -80,7 +80,7 @@ pub struct Manifest { custom_metadata: Option, publish: Option>, replace: Vec<(PackageIdSpec, Dependency)>, - patch: HashMap>, + patch: HashMap>, workspace: WorkspaceConfig, unstable_features: Features, edition: Edition, @@ -116,7 +116,7 @@ pub struct VirtualManifest { // this form of manifest: replace: Vec<(PackageIdSpec, Dependency)>, - patch: HashMap>, + patch: HashMap>, workspace: WorkspaceConfig, warnings: Warnings, features: Features, @@ -512,7 +512,7 @@ impl Manifest { custom_metadata: Option, publish: Option>, replace: Vec<(PackageIdSpec, Dependency)>, - patch: HashMap>, + patch: HashMap>, workspace: WorkspaceConfig, unstable_features: Features, edition: Edition, @@ -640,7 +640,7 @@ impl Manifest { pub fn replace(&self) -> &[(PackageIdSpec, Dependency)] { &self.replace } - pub fn patch(&self) -> &HashMap> { + pub fn patch(&self) -> &HashMap> { &self.patch } pub fn links(&self) -> Option<&str> { @@ -749,7 +749,7 @@ impl VirtualManifest { original_toml: Rc, normalized_toml: Rc, replace: Vec<(PackageIdSpec, Dependency)>, - patch: HashMap>, + patch: HashMap>, workspace: WorkspaceConfig, features: Features, resolve_behavior: Option, @@ -789,7 +789,7 @@ impl VirtualManifest { &self.replace } - pub fn patch(&self) -> &HashMap> { + pub fn patch(&self) -> &HashMap> { &self.patch } diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 786883f2dd3..fa2a0df73bd 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -1,4 +1,4 @@ -pub use self::dependency::{Dependency, SerializedDependency}; +pub use self::dependency::{Dependency, Patch, PatchLocation, SerializedDependency}; pub use self::features::{CliUnstable, Edition, Feature, Features}; pub use self::manifest::{EitherManifest, VirtualManifest}; pub use self::manifest::{Manifest, Target, TargetKind}; diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 38a5ed6ddfe..85f278f1110 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -12,8 +12,7 @@ use std::collections::{HashMap, HashSet}; use std::task::{Poll, ready}; -use crate::core::PackageSet; -use crate::core::{Dependency, PackageId, SourceId, Summary}; +use crate::core::{Dependency, PackageId, PackageSet, Patch, SourceId, Summary}; use crate::sources::IndexSummary; use crate::sources::config::SourceConfigMap; use crate::sources::source::QueryKind; @@ -177,7 +176,7 @@ enum Kind { /// It is the patch locked to a specific version found in Cargo.lock. /// This will be `None` if `Cargo.lock` doesn't exist, /// or the patch did not match any existing entries in `Cargo.lock`. -pub type PatchDependency<'a> = (&'a Dependency, Option); +pub type PatchDependency<'a> = (&'a Patch, Option); /// Argument to [`PackageRegistry::patch`] which is information about a `[patch]` /// directive that we found in a lockfile, if present. @@ -342,7 +341,7 @@ impl<'gctx> PackageRegistry<'gctx> { &mut self, url: &Url, patch_deps: &[PatchDependency<'_>], - ) -> CargoResult> { + ) -> CargoResult> { // NOTE: None of this code is aware of required features. If a patch // is missing a required feature, you end up with an "unused patch" // warning, which is very hard to understand. Ideally the warning @@ -372,7 +371,7 @@ impl<'gctx> PackageRegistry<'gctx> { // Use the locked patch if it exists, otherwise use the original. let dep = match locked { Some(lock) => &lock.dependency, - None => *orig_patch, + None => &orig_patch.dep, }; debug!( "registering a patch for `{}` with `{}`", @@ -430,7 +429,7 @@ impl<'gctx> PackageRegistry<'gctx> { let summaries = summaries.into_iter().map(|s| s.into_summary()).collect(); let (summary, should_unlock) = - match summary_for_patch(orig_patch, &locked, summaries, source) { + match summary_for_patch(&orig_patch.dep, &locked, summaries, source) { Poll::Ready(x) => x, Poll::Pending => { patch_deps_pending.push(patch_dep_remaining); @@ -440,7 +439,7 @@ impl<'gctx> PackageRegistry<'gctx> { .with_context(|| { format!( "patch for `{}` in `{}` failed to resolve", - orig_patch.package_name(), + orig_patch.dep.package_name(), url, ) }) diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs index d7c44de3da0..4f437123282 100644 --- a/src/cargo/core/resolver/encode.rs +++ b/src/cargo/core/resolver/encode.rs @@ -112,7 +112,7 @@ //! format. use super::{Resolve, ResolveVersion}; -use crate::core::{Dependency, GitReference, Package, PackageId, SourceId, Workspace}; +use crate::core::{Dependency, GitReference, Package, PackageId, Patch, SourceId, Workspace}; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; use crate::util::{Graph, internal}; @@ -454,7 +454,7 @@ fn build_path_deps( build_pkg(member, ws, &mut ret, &mut visited); } for deps in ws.root_patch()?.values() { - for dep in deps { + for Patch { dep, loc: _ } in deps { build_dep(dep, ws, &mut ret, &mut visited); } } diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 7c5517c71df..a9b325e9d94 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -17,7 +17,8 @@ use crate::core::registry::PackageRegistry; use crate::core::resolver::ResolveBehavior; use crate::core::resolver::features::CliFeatures; use crate::core::{ - Dependency, Edition, FeatureValue, PackageId, PackageIdSpec, PackageIdSpecQuery, + Dependency, Edition, FeatureValue, PackageId, PackageIdSpec, PackageIdSpecQuery, Patch, + PatchLocation, }; use crate::core::{EitherManifest, Package, SourceId, VirtualManifest}; use crate::lints::analyze_cargo_lints_table; @@ -26,7 +27,7 @@ use crate::lints::rules::check_im_a_teapot; use crate::lints::rules::implicit_minimum_version_req; use crate::ops; use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY, PathSource, SourceConfigMap}; -use crate::util::context::FeatureUnification; +use crate::util::context::{FeatureUnification, Value}; use crate::util::edit_distance; use crate::util::errors::{CargoResult, ManifestError}; use crate::util::interning::InternedString; @@ -483,9 +484,9 @@ impl<'gctx> Workspace<'gctx> { } } - fn config_patch(&self) -> CargoResult>> { + fn config_patch(&self) -> CargoResult>> { let config_patch: Option< - BTreeMap>>, + BTreeMap>>>, > = self.gctx.get("patch")?; let source = SourceId::for_manifest_path(self.root_manifest())?; @@ -507,9 +508,9 @@ impl<'gctx> Workspace<'gctx> { patch.insert( url, deps.iter() - .map(|(name, dep)| { + .map(|(name, dependency_cv)| { crate::util::toml::to_dependency( - dep, + &dependency_cv.val, name, source, self.gctx, @@ -520,6 +521,10 @@ impl<'gctx> Workspace<'gctx> { Path::new("unused-relative-path"), /* kind */ None, ) + .map(|dep| Patch { + dep, + loc: PatchLocation::Config(dependency_cv.definition.clone()), + }) }) .collect::>>()?, ); @@ -537,7 +542,7 @@ impl<'gctx> Workspace<'gctx> { /// Returns the root `[patch]` section of this workspace. /// /// This may be from a virtual crate or an actual crate. - pub fn root_patch(&self) -> CargoResult>> { + pub fn root_patch(&self) -> CargoResult>> { let from_manifest = match self.root_maybe() { MaybePackage::Package(p) => p.manifest().patch(), MaybePackage::Virtual(vm) => vm.patch(), @@ -562,7 +567,7 @@ impl<'gctx> Workspace<'gctx> { for dep_from_config in &mut *deps_from_config { if let Some(i) = from_manifest_pruned.iter().position(|dep_from_manifest| { // XXX: should this also take into account version numbers? - dep_from_config.name_in_toml() == dep_from_manifest.name_in_toml() + dep_from_config.dep.name_in_toml() == dep_from_manifest.dep.name_in_toml() }) { from_manifest_pruned.swap_remove(i); } diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index ecd95273f57..c21bcebc1de 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -903,7 +903,7 @@ fn register_patch_entries( let mut avoid_patch_ids = HashSet::new(); for (url, patches) in ws.root_patch()?.iter() { for patch in patches { - version_prefs.prefer_dependency(patch.clone()); + version_prefs.prefer_dependency(patch.dep.clone()); } let Some(previous) = previous else { let patches: Vec<_> = patches.iter().map(|p| (p, None)).collect(); @@ -921,7 +921,8 @@ fn register_patch_entries( // previous resolve graph, which is primarily what's done here to // build the `registrations` list. let mut registrations = Vec::new(); - for dep in patches { + for patch in patches { + let dep = &patch.dep; let candidates = || { previous .iter() @@ -986,7 +987,7 @@ fn register_patch_entries( } }; - registrations.push((dep, lock)); + registrations.push((patch, lock)); } let canonical = CanonicalUrl::new(url)?; @@ -995,7 +996,7 @@ fn register_patch_entries( avoid_patch_ids.insert(unlock_id); // Also avoid the thing it is patching. avoid_patch_ids.extend(previous.iter().filter(|id| { - orig_patch.matches_ignoring_source(*id) + orig_patch.dep.matches_ignoring_source(*id) && *id.source_id().canonical_url() == canonical })); } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 17f1116c9f6..441e3f184f1 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -25,7 +25,9 @@ use crate::core::compiler::{CompileKind, CompileTarget}; use crate::core::dependency::{Artifact, ArtifactTarget, DepKind}; use crate::core::manifest::{ManifestMetadata, TargetSourcePath}; use crate::core::resolver::ResolveBehavior; -use crate::core::{CliUnstable, FeatureValue, find_workspace_root, resolve_relative_path}; +use crate::core::{ + CliUnstable, FeatureValue, Patch, PatchLocation, find_workspace_root, resolve_relative_path, +}; use crate::core::{Dependency, Manifest, Package, PackageId, Summary, Target}; use crate::core::{Edition, EitherManifest, Feature, Features, VirtualManifest, Workspace}; use crate::core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, WorkspaceRootConfig}; @@ -2113,9 +2115,9 @@ fn replace( } fn patch( - me: &manifest::TomlManifest, + me: &TomlManifest, manifest_ctx: &mut ManifestContext<'_, '_>, -) -> CargoResult>> { +) -> CargoResult>> { let mut patch = HashMap::new(); for (toml_url, deps) in me.patch.iter().flatten() { let url = match &toml_url[..] { @@ -2146,7 +2148,10 @@ fn patch( dep.unused_keys(), &mut manifest_ctx.warnings, ); - dep_to_dependency(dep, name, manifest_ctx, None) + + let dep = dep_to_dependency(dep, name, manifest_ctx, None)?; + let loc = PatchLocation::Manifest(manifest_ctx.file.to_path_buf()); + Ok(Patch { dep, loc }) }) .collect::>>()?, ); From afe63af77fae6ee8fd5eeecf1ed1ff8b9b60efd6 Mon Sep 17 00:00:00 2001 From: omskscream Date: Wed, 17 Dec 2025 00:59:31 +0200 Subject: [PATCH 07/12] feat (patch): add patch location to the error message when too many matches found for this patch definition --- src/cargo/core/registry.rs | 15 +++++++++++---- tests/testsuite/patch.rs | 2 +- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 85f278f1110..842fdb77dd1 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -429,7 +429,7 @@ impl<'gctx> PackageRegistry<'gctx> { let summaries = summaries.into_iter().map(|s| s.into_summary()).collect(); let (summary, should_unlock) = - match summary_for_patch(&orig_patch.dep, &locked, summaries, source) { + match summary_for_patch(&orig_patch, &locked, summaries, source) { Poll::Ready(x) => x, Poll::Pending => { patch_deps_pending.push(patch_dep_remaining); @@ -931,11 +931,12 @@ fn lock( /// happens when a match cannot be found with the `locked` one, but found one /// via the original patch, so we need to inform the resolver to "unlock" it. fn summary_for_patch( - orig_patch: &Dependency, + original_patch: &Patch, locked: &Option, mut summaries: Vec, source: &mut dyn Source, ) -> Poll)>> { + let orig_patch = &original_patch.dep; if summaries.len() == 1 { return Poll::Ready(Ok((summaries.pop().unwrap(), None))); } @@ -953,12 +954,13 @@ fn summary_for_patch( return Poll::Ready(Err(anyhow::anyhow!( "patch for `{}` in `{}` resolved to more than one candidate\n\ Found versions: {}\n\ - Update the patch definition to select only one package.\n\ + Update the patch definition in `{}` to select only one package.\n\ For example, add an `=` version requirement to the patch definition, \ such as `version = \"={}\"`.", orig_patch.package_name(), orig_patch.source_id(), versions.join(", "), + original_patch.loc, versions.last().unwrap() ))); } @@ -978,7 +980,12 @@ fn summary_for_patch( let orig_matches = orig_matches.into_iter().map(|s| s.into_summary()).collect(); - let summary = ready!(summary_for_patch(orig_patch, &None, orig_matches, source))?; + let summary = ready!(summary_for_patch( + original_patch, + &None, + orig_matches, + source + ))?; return Poll::Ready(Ok((summary.0, Some(locked.package_id)))); } // Try checking if there are *any* packages that match this by name. diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index 9b059d5d01d..471ca3a761c 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -2285,7 +2285,7 @@ Caused by: Caused by: patch for `bar` in `registry `alternative`` resolved to more than one candidate Found versions: 0.1.0, 0.1.1 - Update the patch definition to select only one package. + Update the patch definition in `[ROOT]/foo/Cargo.toml` to select only one package. For example, add an `=` version requirement to the patch definition, such as `version = "=0.1.1"`. "#]]) From efd8d45491669bf6796e950d27908c20a703b76d Mon Sep 17 00:00:00 2001 From: omskscream Date: Wed, 17 Dec 2025 01:10:08 +0200 Subject: [PATCH 08/12] feat (patch): add patch location to the error message when there's a version mismatch --- src/cargo/core/registry.rs | 3 ++- tests/testsuite/patch.rs | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 842fdb77dd1..2acc94f3bd7 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -1023,12 +1023,13 @@ fn summary_for_patch( } else { anyhow::anyhow!( "The patch location `{}` contains a `{}` package with {}, but the patch \ - definition requires `{}`.\n\ + definition in `{}` requires `{}`.\n\ Check that the version in the patch location is what you expect, \ and update the patch definition to match.", orig_patch.source_id(), orig_patch.package_name(), found, + original_patch.loc, orig_patch.version_req() ) })) diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index 471ca3a761c..015e15653f7 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -2364,7 +2364,7 @@ Caused by: patch for `bar` in `https://github.com/rust-lang/crates.io-index` failed to resolve Caused by: - The patch location `[ROOT]/foo/bar` contains a `bar` package with version `0.1.0`, but the patch definition requires `^0.1.1`. + The patch location `[ROOT]/foo/bar` contains a `bar` package with version `0.1.0`, but the patch definition in `[ROOT]/foo/Cargo.toml` requires `^0.1.1`. Check that the version in the patch location is what you expect, and update the patch definition to match. "#]]) @@ -2403,7 +2403,7 @@ Caused by: patch for `bar` in `https://github.com/rust-lang/crates.io-index` failed to resolve Caused by: - The patch location `[ROOT]/foo/bar` contains a `bar` package with version `0.1.0`, but the patch definition requires `^0.1.1`. + The patch location `[ROOT]/foo/bar` contains a `bar` package with version `0.1.0`, but the patch definition in `--config cli option` requires `^0.1.1`. Check that the version in the patch location is what you expect, and update the patch definition to match. "#]]) @@ -2450,7 +2450,7 @@ Caused by: patch for `bar` in `https://github.com/rust-lang/crates.io-index` failed to resolve Caused by: - The patch location `[ROOT]/foo/bar` contains a `bar` package with version `0.1.0`, but the patch definition requires `^0.1.1`. + The patch location `[ROOT]/foo/bar` contains a `bar` package with version `0.1.0`, but the patch definition in `[ROOT]/foo/tmp/my-config.toml` requires `^0.1.1`. Check that the version in the patch location is what you expect, and update the patch definition to match. "#]]) @@ -2599,7 +2599,7 @@ Caused by: patch for `bar` in `https://github.com/rust-lang/crates.io-index` failed to resolve Caused by: - The patch location `[ROOT]/foo/bar` contains a `bar` package with version `0.1.0`, but the patch definition requires `^0.1.1`. + The patch location `[ROOT]/foo/bar` contains a `bar` package with version `0.1.0`, but the patch definition in `[ROOT]/foo/Cargo.toml` requires `^0.1.1`. Check that the version in the patch location is what you expect, and update the patch definition to match. "#]]) From 68046919e0ced07081ed628c3e04c1c215814602 Mon Sep 17 00:00:00 2001 From: omskscream Date: Wed, 17 Dec 2025 01:15:41 +0200 Subject: [PATCH 09/12] feat (patch): add patch location to the error message when there's no matches for a patch definition in the specified location --- src/cargo/core/registry.rs | 6 ++++-- tests/testsuite/patch.rs | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 2acc94f3bd7..ca5bce2e44b 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -1016,9 +1016,11 @@ fn summary_for_patch( Poll::Ready(Err(if found.is_empty() { anyhow::anyhow!( "The patch location `{}` does not appear to contain any packages \ - matching the name `{}`.", + matching the name `{}`.\n\ + Check the patch definition in `{}`.", orig_patch.source_id(), - orig_patch.package_name() + orig_patch.package_name(), + original_patch.loc ) } else { anyhow::anyhow!( diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index 015e15653f7..2136d0ec359 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -2326,6 +2326,7 @@ Caused by: Caused by: The patch location `[ROOT]/foo/bar` does not appear to contain any packages matching the name `bar`. + Check the patch definition in `[ROOT]/foo/Cargo.toml`. "#]]) .run(); From 373f626716e2ced9be152eafabb4e811a201bd21 Mon Sep 17 00:00:00 2001 From: omskscream Date: Wed, 17 Dec 2025 01:22:29 +0200 Subject: [PATCH 10/12] feat (patch): add patch location to the error message when patch source is the same as dependency source --- src/cargo/core/registry.rs | 6 ++++-- tests/testsuite/patch.rs | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index ca5bce2e44b..2e69a0457ba 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -456,9 +456,11 @@ impl<'gctx> PackageRegistry<'gctx> { if *summary.package_id().source_id().canonical_url() == canonical { return Err(anyhow::anyhow!( "patch for `{}` in `{}` points to the same source, but \ - patches must point to different sources", + patches must point to different sources.\n\ + Check the patch definition in `{}`.", dep.package_name(), - url + url, + orig_patch.loc ) .context(format!("failed to resolve patches for `{}`", url))); } diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index 2136d0ec359..cf61d95ff39 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -1513,7 +1513,8 @@ fn replace_with_crates_io() { [ERROR] failed to resolve patches for `https://github.com/rust-lang/crates.io-index` Caused by: - patch for `bar` in `https://github.com/rust-lang/crates.io-index` points to the same source, but patches must point to different sources + patch for `bar` in `https://github.com/rust-lang/crates.io-index` points to the same source, but patches must point to different sources. + Check the patch definition in `[ROOT]/foo/Cargo.toml`. "#]]) .run(); From 75bfdfd758374d3acf80a003df42ba1444d86b7a Mon Sep 17 00:00:00 2001 From: omskscream Date: Wed, 17 Dec 2025 02:01:24 +0200 Subject: [PATCH 11/12] feat (patch): add patch location to the error message when there are multiple patch definitions resolving to the same version --- src/cargo/core/registry.rs | 22 ++++++++++++++++------ tests/testsuite/patch.rs | 14 +++++++++++--- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 2e69a0457ba..3210e91da94 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -22,7 +22,8 @@ use crate::util::errors::CargoResult; use crate::util::interning::InternedString; use crate::util::{CanonicalUrl, GlobalContext}; use annotate_snippets::Level; -use anyhow::{Context as _, bail}; +use anyhow::Context as _; +use itertools::Itertools; use tracing::{debug, trace}; use url::Url; @@ -476,12 +477,21 @@ impl<'gctx> PackageRegistry<'gctx> { let name = summary.package_id().name(); let version = summary.package_id().version(); if !name_and_version.insert((name, version)) { - bail!( - "cannot have two `[patch]` entries which both resolve \ - to `{} v{}`", + let duplicate_locations: Vec<_> = patch_deps + .iter() + .filter(|&p| p.0.dep.package_name() == name) + .map(|p| p.0.loc.to_string()) + .unique() + .collect(); + return Err(anyhow::anyhow!( + "cannot have two `[patch]` entries which both resolve to `{} v{}`.\n\ + Check patch definitions for `{}` in `{}`", name, - version - ); + version, + name, + duplicate_locations.join(", ") + )) + .context(format!("failed to resolve patches for `{}`", url)); } } diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index cf61d95ff39..b7f64cf058f 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -1853,7 +1853,7 @@ fn patch_same_version() { .file("src/lib.rs", "") .build(); - cargo_test_support::registry::init(); + registry::init(); let p = project() .file( @@ -1890,7 +1890,11 @@ fn patch_same_version() { .with_status(101) .with_stderr_data(str![[r#" [UPDATING] git repository `[ROOTURL]/override` -[ERROR] cannot have two `[patch]` entries which both resolve to `bar v0.1.0` +[ERROR] failed to resolve patches for `https://github.com/rust-lang/crates.io-index` + +Caused by: + cannot have two `[patch]` entries which both resolve to `bar v0.1.0`. + Check patch definitions for `bar` in `[ROOT]/foo/Cargo.toml` "#]]) .run(); @@ -1946,7 +1950,11 @@ fn patch_same_version_different_patch_locations() { .with_status(101) .with_stderr_data(str![[r#" [UPDATING] git repository `[ROOTURL]/override` -[ERROR] cannot have two `[patch]` entries which both resolve to `bar v0.1.0` +[ERROR] failed to resolve patches for `https://github.com/rust-lang/crates.io-index` + +Caused by: + cannot have two `[patch]` entries which both resolve to `bar v0.1.0`. + Check patch definitions for `bar` in `[ROOT]/foo/.cargo/config.toml, [ROOT]/foo/Cargo.toml` "#]]) .run(); From 5ebec7d948c91e76ec97b36ef50e483715a971ee Mon Sep 17 00:00:00 2001 From: omskscream Date: Wed, 17 Dec 2025 02:10:51 +0200 Subject: [PATCH 12/12] refactor (patch): remove variable with confusing name --- src/cargo/core/registry.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 3210e91da94..dab7adc96c9 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -948,7 +948,6 @@ fn summary_for_patch( mut summaries: Vec, source: &mut dyn Source, ) -> Poll)>> { - let orig_patch = &original_patch.dep; if summaries.len() == 1 { return Poll::Ready(Ok((summaries.pop().unwrap(), None))); } @@ -969,8 +968,8 @@ fn summary_for_patch( Update the patch definition in `{}` to select only one package.\n\ For example, add an `=` version requirement to the patch definition, \ such as `version = \"={}\"`.", - orig_patch.package_name(), - orig_patch.source_id(), + &original_patch.dep.package_name(), + &original_patch.dep.source_id(), versions.join(", "), original_patch.loc, versions.last().unwrap() @@ -980,11 +979,11 @@ fn summary_for_patch( // No summaries found, try to help the user figure out what is wrong. if let Some(locked) = locked { // Since the locked patch did not match anything, try the unlocked one. - let orig_matches = - ready!(source.query_vec(orig_patch, QueryKind::Exact)).unwrap_or_else(|e| { + let orig_matches = ready!(source.query_vec(&original_patch.dep, QueryKind::Exact)) + .unwrap_or_else(|e| { tracing::warn!( "could not determine unlocked summaries for dep {:?}: {:?}", - orig_patch, + &original_patch.dep, e ); Vec::new() @@ -1001,7 +1000,10 @@ fn summary_for_patch( return Poll::Ready(Ok((summary.0, Some(locked.package_id)))); } // Try checking if there are *any* packages that match this by name. - let name_only_dep = Dependency::new_override(orig_patch.package_name(), orig_patch.source_id()); + let name_only_dep = Dependency::new_override( + original_patch.dep.package_name(), + original_patch.dep.source_id(), + ); let name_summaries = ready!(source.query_vec(&name_only_dep, QueryKind::Exact)).unwrap_or_else(|e| { @@ -1030,8 +1032,8 @@ fn summary_for_patch( "The patch location `{}` does not appear to contain any packages \ matching the name `{}`.\n\ Check the patch definition in `{}`.", - orig_patch.source_id(), - orig_patch.package_name(), + &original_patch.dep.source_id(), + &original_patch.dep.package_name(), original_patch.loc ) } else { @@ -1040,11 +1042,11 @@ fn summary_for_patch( definition in `{}` requires `{}`.\n\ Check that the version in the patch location is what you expect, \ and update the patch definition to match.", - orig_patch.source_id(), - orig_patch.package_name(), + &original_patch.dep.source_id(), + &original_patch.dep.package_name(), found, original_patch.loc, - orig_patch.version_req() + &original_patch.dep.version_req() ) })) }