Skip to content

Commit ee0596f

Browse files
authored
Allow multiple self-dependencies (#338)
In uv, we don't use the `DependencyConstraints` map, but pass in the dependencies through an iterator. This means there can be duplicate dependencies in the input. This would previously make `merge_dependents` panic if a package dependent on itself twice with the same range: ```toml [package] name = "foo" version = "0.1.0" dependencies = ["foo", "foo"] ``` The fix is to ignore self-dependencies when merging dependents, given that they are always trivially true or trivially false.
1 parent b90cd72 commit ee0596f

File tree

1 file changed

+74
-1
lines changed

1 file changed

+74
-1
lines changed

src/internal/incompatibility.rs

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,14 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Incompatibilit
180180
return None;
181181
}
182182
let (p1, p2) = self_pkgs;
183+
// We ignore self-dependencies. They are always either trivially true or trivially false,
184+
// as the package version implies whether the constraint will always be fulfilled or always
185+
// violated.
186+
// At time of writing, the public crate API only allowed a map of dependencies,
187+
// meaning it can't hit this branch, which requires two self-dependencies.
188+
if p1 == p2 {
189+
return None;
190+
}
183191
let dep_term = self.get(p2);
184192
// The dependency range for p2 must be the same in both case
185193
// to be able to merge multiple p1 ranges.
@@ -381,10 +389,13 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Incompatibilit
381389
#[cfg(test)]
382390
pub(crate) mod tests {
383391
use proptest::prelude::*;
392+
use std::cmp::Reverse;
393+
use std::collections::BTreeMap;
384394

385395
use super::*;
396+
use crate::internal::State;
386397
use crate::term::tests::strategy as term_strat;
387-
use crate::Ranges;
398+
use crate::{OfflineDependencyProvider, Ranges};
388399

389400
proptest! {
390401

@@ -421,4 +432,66 @@ pub(crate) mod tests {
421432
}
422433

423434
}
435+
436+
/// Check that multiple self-dependencies are supported.
437+
///
438+
/// The current public API deduplicates dependencies through a map, so we test them here
439+
/// manually.
440+
///
441+
/// https://github.com/astral-sh/uv/issues/13344
442+
#[test]
443+
fn package_depend_on_self() {
444+
let cases: &[Vec<(String, Ranges<usize>)>] = &[
445+
vec![("foo".to_string(), Ranges::full())],
446+
vec![
447+
("foo".to_string(), Ranges::full()),
448+
("foo".to_string(), Ranges::full()),
449+
],
450+
vec![
451+
("foo".to_string(), Ranges::full()),
452+
("foo".to_string(), Ranges::singleton(1usize)),
453+
],
454+
vec![
455+
("foo".to_string(), Ranges::singleton(1usize)),
456+
("foo".to_string(), Ranges::from_range_bounds(1usize..2)),
457+
("foo".to_string(), Ranges::from_range_bounds(1usize..3)),
458+
],
459+
];
460+
461+
for case in cases {
462+
let mut state: State<OfflineDependencyProvider<String, Ranges<usize>>> =
463+
State::init("root".to_string(), 0);
464+
state.unit_propagation(state.root_package).unwrap();
465+
466+
// Add the root package
467+
state.add_package_version_dependencies(
468+
state.root_package,
469+
0,
470+
[("foo".to_string(), Ranges::singleton(1usize))],
471+
);
472+
state.unit_propagation(state.root_package).unwrap();
473+
474+
// Add a package that depends on itself twice
475+
let (next, _) = state
476+
.partial_solution
477+
.pick_highest_priority_pkg(|_p, _r| (0, Reverse(0)))
478+
.unwrap();
479+
state.add_package_version_dependencies(next, 1, case.clone());
480+
state.unit_propagation(next).unwrap();
481+
482+
assert!(state
483+
.partial_solution
484+
.pick_highest_priority_pkg(|_p, _r| (0, Reverse(0)))
485+
.is_none());
486+
487+
let solution: BTreeMap<String, usize> = state
488+
.partial_solution
489+
.extract_solution()
490+
.map(|(p, v)| (state.package_store[p].clone(), v))
491+
.collect();
492+
let expected = BTreeMap::from([("root".to_string(), 0), ("foo".to_string(), 1)]);
493+
494+
assert_eq!(solution, expected, "{:?}", case);
495+
}
496+
}
424497
}

0 commit comments

Comments
 (0)