From 1ed3555bf087067d19780cd00ee5a74a8e8a8b88 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Mon, 20 May 2024 13:22:50 -0400 Subject: [PATCH] uv-resolver: sort in `format_terms` This makes use of the newly added `Ord` impl on `PubGrubPackage` to make the output of `format_terms` independent of hashmap iteration order. This was already collecting the terms into an intermediate `Vec`, so sorting probably isn't going to add any significant overhead here. (Plus, this is only running when formatting an error message after a solution could not be found, so an extra sort doesn't seem like a big deal here.) Note that some tests are updated in this commit as a result of this change. As far as I can tell, the semantic meaning of the output remains the same. But the order of the listed packages does not. Specific thing motivating this change is, in a subsequent, I added `Option` to `PubGrubPackage::Package`, and this caused similar changes in test output. So I backtracked and isolated this change from the addition of `Option`. --- crates/uv-resolver/src/pubgrub/report.rs | 5 ++++- crates/uv/tests/pip_install_scenarios.rs | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/uv-resolver/src/pubgrub/report.rs b/crates/uv-resolver/src/pubgrub/report.rs index 1481a9469..dafd4d97c 100644 --- a/crates/uv-resolver/src/pubgrub/report.rs +++ b/crates/uv-resolver/src/pubgrub/report.rs @@ -152,7 +152,10 @@ impl ReportFormatter, UnavailableReason> /// Try to print terms of an incompatibility in a human-readable way. fn format_terms(&self, terms: &Map>>) -> String { - let terms_vec: Vec<_> = terms.iter().collect(); + let mut terms_vec: Vec<_> = terms.iter().collect(); + // We avoid relying on hashmap iteration order here by always sorting + // by package first. + terms_vec.sort_by(|&(pkg1, _), &(pkg2, _)| pkg1.cmp(pkg2)); match terms_vec.as_slice() { [] | [(PubGrubPackage::Root(_), _)] => "the requirements are unsatisfiable".into(), [(package @ PubGrubPackage::Package { .. }, Term::Positive(range))] => { diff --git a/crates/uv/tests/pip_install_scenarios.rs b/crates/uv/tests/pip_install_scenarios.rs index af35a31b6..edd36a1e0 100644 --- a/crates/uv/tests/pip_install_scenarios.rs +++ b/crates/uv/tests/pip_install_scenarios.rs @@ -470,7 +470,7 @@ fn dependency_excludes_range_of_compatible_versions() { package-a<2.0.0 package-a>=3.0.0 - And because we know from (1) that package-a<2.0.0 depends on package-b==1.0.0, we can conclude that package-a!=3.0.0, all versions of package-c, package-b!=1.0.0 are incompatible. + And because we know from (1) that package-a<2.0.0 depends on package-b==1.0.0, we can conclude that package-a!=3.0.0, package-b!=1.0.0, all versions of package-c are incompatible. And because package-a==3.0.0 depends on package-b==3.0.0, we can conclude that all versions of package-c depend on one of: package-b<=1.0.0 package-b>=3.0.0 @@ -595,7 +595,7 @@ fn dependency_excludes_non_contiguous_range_of_compatible_versions() { package-a<2.0.0 package-a>=3.0.0 - And because we know from (1) that package-a<2.0.0 depends on package-b==1.0.0, we can conclude that all versions of package-c, package-a!=3.0.0, package-b!=1.0.0 are incompatible. + And because we know from (1) that package-a<2.0.0 depends on package-b==1.0.0, we can conclude that package-a!=3.0.0, package-b!=1.0.0, all versions of package-c are incompatible. And because package-a==3.0.0 depends on package-b==3.0.0, we can conclude that all versions of package-c depend on one of: package-b<=1.0.0 package-b>=3.0.0 @@ -3378,7 +3378,7 @@ fn transitive_prerelease_and_stable_dependency_many_versions() { × No solution found when resolving dependencies: ╰─▶ Because only package-a==1.0.0 is available and package-a==1.0.0 depends on package-c>=2.0.0b1, we can conclude that all versions of package-a depend on package-c>=2.0.0b1. And because only package-c<2.0.0b1 is available, we can conclude that all versions of package-a depend on package-c>3.0.0. - And because package-b==1.0.0 depends on package-c and only package-b==1.0.0 is available, we can conclude that all versions of package-b and all versions of package-a are incompatible. + And because package-b==1.0.0 depends on package-c and only package-b==1.0.0 is available, we can conclude that all versions of package-a and all versions of package-b are incompatible. And because you require package-a and package-b, we can conclude that the requirements are unsatisfiable. hint: package-c was requested with a pre-release marker (e.g., package-c>=2.0.0b1), but pre-releases weren't enabled (try: `--prerelease=allow`)