Use stable order for dependencies

.github/workflows/ci.yml

@@ -19,6 +19,19 @@ jobs:
       - run: cargo build --workspace
       - run: cargo test --all-features --workspace
 
+  test-i686:
+    name: Tests (32-bit)
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5
+      - uses: dtolnay/rust-toolchain@stable
+      - run: | 
+          sudo apt-get update
+          sudo apt-get install -y gcc-multilib
+      - run: rustup target add i686-unknown-linux-gnu
+      - run: cargo build --workspace --target i686-unknown-linux-gnu
+      - run: cargo test --all-features --workspace --target i686-unknown-linux-gnu
+
   clippy:
     name: Clippy
     runs-on: ubuntu-latest

CHANGELOG.md

@@ -2,6 +2,14 @@
 
 All notable changes to this project will be documented in this file.
 
+## 0.4.0
+
+`DependencyConstraints` is now an opaque type that retains the ordering of the dependencies across platforms. This fixes
+unstable resolutions across platform (https://github.com/pubgrub-rs/pubgrub/issues/373). `DependencyConstraints` can be
+constructed from an iterator. It is currently backed by a `Vec` internally.
+
+`SelectedDependencies` is now an opaque type that support iteration and `get()`.
+
 ## [0.3.0] - 2025-02-12 - [(diff with 0.2.1)][0.2.1-diff]
 
 PubGrub 0.3 has a more flexible interface and speeds resolution significantly. The public API is very different now, we

Cargo.toml

@@ -5,7 +5,7 @@ members = ["version-ranges"]
 
 [package]
 name = "pubgrub"
-version = "0.3.0"
+version = "0.4.0"
 authors = [
     "Matthieu Pizenberg <[email protected]>",
     "Alex Tokarev <[email protected]>",

src/lib.rs

@@ -229,9 +229,12 @@ pub use report::{
     DefaultStringReportFormatter, DefaultStringReporter, DerivationTree, Derived, External,
     ReportFormatter, Reporter,
 };
-pub use solver::{Dependencies, DependencyProvider, PackageResolutionStatistics, resolve};
+pub use solver::{
+    Dependencies, DependencyConstraints, DependencyProvider, PackageResolutionStatistics,
+    SelectedDependencies, resolve,
+};
 pub use term::Term;
-pub use type_aliases::{DependencyConstraints, Map, SelectedDependencies, Set};
+pub use type_aliases::{Map, Set};
 pub use version::{SemanticVersion, VersionParseError};
 pub use version_ranges::Ranges;
 #[deprecated(note = "Use `Ranges` instead")]

src/solver.rs

@@ -4,12 +4,9 @@ use std::collections::BTreeSet as Set;
 use std::error::Error;
 use std::fmt::{Debug, Display};
 
-use log::{debug, info};
-
 use crate::internal::{Id, Incompatibility, State};
-use crate::{
-    DependencyConstraints, Map, Package, PubGrubError, SelectedDependencies, Term, VersionSet,
-};
+use crate::{Map, Package, PubGrubError, Term, VersionSet};
+use log::{debug, info};
 
 /// Statistics on how often a package conflicted with other packages.
 #[derive(Debug, Default, Clone)]
@@ -48,6 +45,36 @@ impl PackageResolutionStatistics {
     }
 }
 
+/// The resolved dependencies and their versions.
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub struct SelectedDependencies<P: Package, V>(Map<P, V>);
+
+impl<P: Package, V> SelectedDependencies<P, V> {
+    /// Iterate over the resolved dependencies and their versions.
+    pub fn iter(&self) -> impl Iterator<Item = (&P, &V)> {
+        self.0.iter()
+    }
+
+    /// Get the version of a specific dependencies.
+    pub fn get(&self, package: &P) -> Option<&V> {
+        self.0.get(package)
+    }
+}
+
+impl<P: Package, V> FromIterator<(P, V)> for SelectedDependencies<P, V> {
+    fn from_iter<I: IntoIterator<Item = (P, V)>>(iter: I) -> Self {
+        Self(Map::from_iter(iter))
+    }
+}
+
+impl<P: Package, V> IntoIterator for SelectedDependencies<P, V> {
+    type Item = (P, V);
+    type IntoIter = <Map<P, V> as IntoIterator>::IntoIter;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.0.into_iter()
+    }
+}
 /// Finds a set of packages satisfying dependency bounds for a given package + version pair.
 ///
 /// It consists in efficiently finding a set of packages and versions
@@ -109,7 +136,7 @@ pub fn resolve<DP: DependencyProvider>(
     dependency_provider: &DP,
     package: DP::P,
     version: impl Into<DP::V>,
-) -> Result<SelectedDependencies<DP>, PubGrubError<DP>> {
+) -> Result<SelectedDependencies<DP::P, DP::V>, PubGrubError<DP>> {
     let mut state: State<DP> = State::init(package.clone(), version.into());
     let mut conflict_tracker: Map<Id<DP::P>, PackageResolutionStatistics> = Map::default();
     let mut added_dependencies: Map<Id<DP::P>, Set<DP::V>> = Map::default();
@@ -154,11 +181,13 @@ pub fn resolve<DP: DependencyProvider>(
                 )
             })
         else {
-            return Ok(state
-                .partial_solution
-                .extract_solution()
-                .map(|(p, v)| (state.package_store[p].clone(), v))
-                .collect());
+            return Ok(SelectedDependencies(
+                state
+                    .partial_solution
+                    .extract_solution()
+                    .map(|(p, v)| (state.package_store[p].clone(), v))
+                    .collect(),
+            ));
         };
         next = highest_priority_pkg;
 
@@ -247,6 +276,69 @@ pub fn resolve<DP: DependencyProvider>(
     }
 }
 
+/// The dependencies of a package with their version ranges.
+///
+/// There is a difference in semantics between an empty [DependencyConstraints] and
+/// [Dependencies::Unavailable]:
+/// The former means the package has no dependency and it is a known fact,
+/// while the latter means they could not be fetched by the [DependencyProvider].
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct DependencyConstraints<P, VS>(Vec<(P, VS)>);
+
+/// Backwards compatibility: Serialize as map.
+#[cfg(feature = "serde")]
+impl<P: Package + serde::Serialize, VS: serde::Serialize> serde::Serialize
+    for DependencyConstraints<P, VS>
+{
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        Map::from_iter(self.0.iter().map(|(p, v)| (p, v))).serialize(serializer)
+    }
+}
+
+/// Backwards compatibility: Deserialize as map.
+#[cfg(feature = "serde")]
+impl<'de, P: Package + serde::Deserialize<'de>, VS: serde::Deserialize<'de>> serde::Deserialize<'de>
+    for DependencyConstraints<P, VS>
+{
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        Ok(Self::from_iter(Map::deserialize(deserializer)?))
+    }
+}
+
+impl<P, VS> DependencyConstraints<P, VS> {
+    /// Iterate over each dependency in order.
+    pub fn iter(&self) -> impl Iterator<Item = &(P, VS)> {
+        self.0.iter()
+    }
+}
+
+impl<P, VS> Default for DependencyConstraints<P, VS> {
+    fn default() -> Self {
+        Self(Vec::new())
+    }
+}
+
+impl<P, VS> FromIterator<(P, VS)> for DependencyConstraints<P, VS> {
+    fn from_iter<T: IntoIterator<Item = (P, VS)>>(iter: T) -> Self {
+        Self(iter.into_iter().collect())
+    }
+}
+
+impl<P, VS> IntoIterator for DependencyConstraints<P, VS> {
+    type Item = (P, VS);
+    type IntoIter = <Vec<(P, VS)> as IntoIterator>::IntoIter;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.0.into_iter()
+    }
+}
+
 /// An enum used by [DependencyProvider] that holds information about package dependencies.
 /// For each [Package] there is a set of versions allowed as a dependency.
 #[derive(Clone)]

src/type_aliases.rs

@@ -2,22 +2,8 @@
 
 //! Publicly exported type aliases.
 
-use crate::DependencyProvider;
-
 /// Map implementation used by the library.
 pub type Map<K, V> = rustc_hash::FxHashMap<K, V>;
 
 /// Set implementation used by the library.
 pub type Set<V> = rustc_hash::FxHashSet<V>;
-
-/// Concrete dependencies picked by the library during [resolve](crate::solver::resolve)
-/// from [DependencyConstraints].
-pub type SelectedDependencies<DP> =
-    Map<<DP as DependencyProvider>::P, <DP as DependencyProvider>::V>;
-
-/// Holds information about all possible versions a given package can accept.
-/// There is a difference in semantics between an empty map
-/// inside [DependencyConstraints] and [Dependencies::Unavailable](crate::solver::Dependencies::Unavailable):
-/// the former means the package has no dependency and it is a known fact,
-/// while the latter means they could not be fetched by the [DependencyProvider].
-pub type DependencyConstraints<P, VS> = Map<P, VS>;

tests/examples.rs

@@ -2,7 +2,7 @@
 
 use pubgrub::{
     DefaultStringReporter, Map, OfflineDependencyProvider, PubGrubError, Ranges, Reporter as _,
-    SemanticVersion, Set, resolve,
+    SelectedDependencies, SemanticVersion, Set, resolve,
 };
 
 type NumVS = Ranges<u32>;
@@ -48,7 +48,10 @@ fn no_conflict() {
     expected_solution.insert("bar", (1, 0, 0).into());
 
     // Comparing the true solution with the one computed by the algorithm.
-    assert_eq!(expected_solution, computed_solution);
+    assert_eq!(
+        SelectedDependencies::from_iter(expected_solution),
+        computed_solution
+    );
 }
 
 #[test]
@@ -84,7 +87,10 @@ fn avoiding_conflict_during_decision_making() {
     expected_solution.insert("bar", (1, 1, 0).into());
 
     // Comparing the true solution with the one computed by the algorithm.
-    assert_eq!(expected_solution, computed_solution);
+    assert_eq!(
+        SelectedDependencies::from_iter(expected_solution),
+        computed_solution
+    );
 }
 
 #[test]
@@ -118,7 +124,10 @@ fn conflict_resolution() {
     expected_solution.insert("foo", (1, 0, 0).into());
 
     // Comparing the true solution with the one computed by the algorithm.
-    assert_eq!(expected_solution, computed_solution);
+    assert_eq!(
+        SelectedDependencies::from_iter(expected_solution),
+        computed_solution
+    );
 }
 
 #[test]
@@ -177,7 +186,10 @@ fn conflict_with_partial_satisfier() {
     expected_solution.insert("target", (2, 0, 0).into());
 
     // Comparing the true solution with the one computed by the algorithm.
-    assert_eq!(expected_solution, computed_solution);
+    assert_eq!(
+        SelectedDependencies::from_iter(expected_solution),
+        computed_solution
+    );
 }
 
 #[test]
@@ -208,7 +220,10 @@ fn double_choices() {
 
     // Run the algorithm.
     let computed_solution = resolve(&dependency_provider, "a", 0u32).unwrap();
-    assert_eq!(expected_solution, computed_solution);
+    assert_eq!(
+        SelectedDependencies::from_iter(expected_solution),
+        computed_solution
+    );
 }
 
 #[test]

tests/proptest.rs

@@ -130,10 +130,7 @@ fn timeout_resolve<DP: DependencyProvider>(
     dependency_provider: DP,
     name: DP::P,
     version: impl Into<DP::V>,
-) -> Result<
-    SelectedDependencies<TimeoutDependencyProvider<DP>>,
-    PubGrubError<TimeoutDependencyProvider<DP>>,
-> {
+) -> Result<SelectedDependencies<DP::P, DP::V>, PubGrubError<TimeoutDependencyProvider<DP>>> {
     resolve(
         &TimeoutDependencyProvider::new(dependency_provider, 50_000),
         name,
@@ -292,7 +289,7 @@ fn meta_test_deep_trees_from_strategy() {
             let res = resolve(&dependency_provider, name, ver);
             dis[res
                 .as_ref()
-                .map(|x| std::cmp::min(x.len(), dis.len()) - 1)
+                .map(|x| std::cmp::min(x.iter().count(), dis.len()) - 1)
                 .unwrap_or(0)] += 1;
             if dis.iter().all(|&x| x > 0) {
                 return;
@@ -553,7 +550,7 @@ proptest! {
             .packages()
             .flat_map(|p| {
                 dependency_provider
-                    .versions(&p)
+                    .versions(p)
                     .unwrap()
                     .map(move |&v| (*p, v))
             })

tests/sat_dependency_provider.rs

@@ -68,10 +68,10 @@ impl<P: Package, VS: VersionSet> SatResolve<P, VS> {
                 Dependencies::Unavailable(_) => panic!(),
                 Dependencies::Available(d) => d,
             };
-            for (p1, range) in &deps {
+            for (p1, range) in deps {
                 let empty_vec = vec![];
                 let mut matches: Vec<varisat::Lit> = all_versions_by_p
-                    .get(p1)
+                    .get(&p1)
                     .unwrap_or(&empty_vec)
                     .iter()
                     .filter(|(v1, _)| range.contains(v1))
@@ -117,7 +117,7 @@ impl<P: Package, VS: VersionSet> SatResolve<P, VS> {
 
     pub fn is_valid_solution<DP: DependencyProvider<P = P, VS = VS, V = VS::V>>(
         &mut self,
-        pids: &SelectedDependencies<DP>,
+        pids: &SelectedDependencies<DP::P, DP::V>,
     ) -> bool {
         let mut assumption = vec![];
 
@@ -137,7 +137,7 @@ impl<P: Package, VS: VersionSet> SatResolve<P, VS> {
 
     pub fn check_resolve<DP: DependencyProvider<P = P, VS = VS, V = VS::V>>(
         &mut self,
-        res: &Result<SelectedDependencies<DP>, PubGrubError<DP>>,
+        res: &Result<SelectedDependencies<DP::P, DP::V>, PubGrubError<DP>>,
         p: &P,
         v: &VS::V,
     ) {