From 2c712d5d46c0c852b219e69facd9a5ab2fe6f9b3 Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Sat, 25 Feb 2023 20:21:22 -0500 Subject: [PATCH] patch can conflict on not activated packages --- src/cargo/core/resolver/mod.rs | 40 +++++++------ tests/testsuite/patch.rs | 102 +++++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 17 deletions(-) diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index b0551891da1..831181a4634 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -811,10 +811,8 @@ impl RemainingCandidates { } } -/// Attempts to find a new conflict that allows a `find_candidate` feather then the input one. +/// Attempts to find a new conflict that allows a `find_candidate` better then the input one. /// It will add the new conflict to the cache if one is found. -/// -/// Panics if the input conflict is not all active in `cx`. fn generalize_conflicting( cx: &Context, registry: &mut RegistryQueryer<'_>, @@ -823,15 +821,12 @@ fn generalize_conflicting( dep: &Dependency, conflicting_activations: &ConflictMap, ) -> Option { - if conflicting_activations.is_empty() { - return None; - } // We need to determine the `ContextAge` that this `conflicting_activations` will jump to, and why. - let (backtrack_critical_age, backtrack_critical_id) = conflicting_activations - .keys() - .map(|&c| (cx.is_active(c).expect("not currently active!?"), c)) - .max() - .unwrap(); + let (backtrack_critical_age, backtrack_critical_id) = shortcircuit_max( + conflicting_activations + .keys() + .map(|&c| cx.is_active(c).map(|a| (a, c))), + )?; let backtrack_critical_reason: ConflictReason = conflicting_activations[&backtrack_critical_id].clone(); @@ -923,6 +918,19 @@ fn generalize_conflicting( None } +/// Returns Some of the largest item in the iterator. +/// Returns None if any of the items are None or the iterator is empty. +fn shortcircuit_max(iter: impl Iterator>) -> Option { + let mut out = None; + for i in iter { + if i.is_none() { + return None; + } + out = std::cmp::max(out, i); + } + out +} + /// Looks through the states in `backtrack_stack` for dependencies with /// remaining candidates. For each one, also checks if rolling back /// could change the outcome of the failed resolution that caused backtracking @@ -949,12 +957,10 @@ fn find_candidate( // the cause of that backtrack, so we do not update it. let age = if !backtracked { // we don't have abnormal situations. So we can ask `cx` for how far back we need to go. - let a = cx.is_conflicting(Some(parent.package_id()), conflicting_activations); - // If the `conflicting_activations` does not apply to `cx`, then something went very wrong - // in building it. But we will just fall back to laboriously trying all possibilities witch - // will give us the correct answer so only `assert` if there is a developer to debug it. - debug_assert!(a.is_some()); - a + // If the `conflicting_activations` does not apply to `cx`, + // we will just fall back to laboriously trying all possibilities witch + // will give us the correct answer. + cx.is_conflicting(Some(parent.package_id()), conflicting_activations) } else { None }; diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index dd8b84a9b13..681c02416bb 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -2541,3 +2541,105 @@ foo v0.1.0 [..] )) .run(); } + +// From https://github.com/rust-lang/cargo/issues/7463 +#[cargo_test] +fn patch_eq_conflict_panic() { + Package::new("bar", "0.1.0").publish(); + Package::new("bar", "0.1.1").publish(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + bar = "=0.1.0" + + [dev-dependencies] + bar = "=0.1.1" + + [patch.crates-io] + bar = {path="bar"} + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_manifest("bar", "0.1.1")) + .file("bar/src/lib.rs", "") + .build(); + + p.cargo("generate-lockfile") + .with_status(101) + .with_stderr( + r#"[UPDATING] `dummy-registry` index +[ERROR] failed to select a version for `bar`. + ... required by package `foo v0.1.0 ([..])` +versions that meet the requirements `=0.1.1` are: 0.1.1 + +all possible versions conflict with previously selected packages. + + previously selected package `bar v0.1.0` + ... which satisfies dependency `bar = "=0.1.0"` of package `foo v0.1.0 ([..])` + +failed to select a version for `bar` which could resolve this conflict +"#, + ) + .run(); +} + +// From https://github.com/rust-lang/cargo/issues/11336 +#[cargo_test] +fn mismatched_version2() { + Package::new("qux", "0.1.0-beta.1").publish(); + Package::new("qux", "0.1.0-beta.2").publish(); + Package::new("bar", "0.1.0") + .dep("qux", "=0.1.0-beta.1") + .publish(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + bar = "0.1.0" + qux = "0.1.0-beta.2" + + [patch.crates-io] + qux = { path = "qux" } + "#, + ) + .file("src/lib.rs", "") + .file( + "qux/Cargo.toml", + r#" + [package] + name = "qux" + version = "0.1.0-beta.1" + "#, + ) + .file("qux/src/lib.rs", "") + .build(); + + p.cargo("generate-lockfile") + .with_status(101) + .with_stderr( + r#"[UPDATING] `dummy-registry` index +[ERROR] failed to select a version for `qux`. + ... required by package `bar v0.1.0` + ... which satisfies dependency `bar = "^0.1.0"` of package `foo v0.1.0 ([..])` +versions that meet the requirements `=0.1.0-beta.1` are: 0.1.0-beta.1 + +all possible versions conflict with previously selected packages. + + previously selected package `qux v0.1.0-beta.2` + ... which satisfies dependency `qux = "^0.1.0-beta.2"` of package `foo v0.1.0 ([..])` + +failed to select a version for `qux` which could resolve this conflict"#, + ) + .run(); +}