Skip to content

Commit 73d6ecf

Browse files
authored
Allow multiple self-dependencies (#44)
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 a3b4db3 commit 73d6ecf

File tree

1 file changed

+73
-1
lines changed

1 file changed

+73
-1
lines changed

src/internal/incompatibility.rs

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

388398
use super::*;
389399
use crate::term::tests::strategy as term_strat;
390-
use crate::Ranges;
400+
use crate::{OfflineDependencyProvider, Ranges, State};
391401

392402
proptest! {
393403

@@ -424,4 +434,66 @@ pub(crate) mod tests {
424434
}
425435

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

0 commit comments

Comments
 (0)