RFC: Add way for solver to iterate over versions oldest first

crates/spk-schema/crates/ident/Cargo.toml

@@ -17,14 +17,15 @@ format_serde_error = { version = "0.3", default_features = false, features = [
     "colored",
 ] }
 itertools = { workspace = true }
+miette = { workspace = true }
 nom = { workspace = true }
 nom-supreme = { workspace = true }
 relative-path = { workspace = true }
 serde = { workspace = true, features = ["derive"] }
 serde_yaml = { workspace = true }
 spk-schema-foundation = { workspace = true }
 thiserror = { workspace = true }
-miette = { workspace = true }
+variantly = { workspace = true }
 
 [dev-dependencies]
 data-encoding = "2.3"

crates/spk-schema/crates/ident/src/lib.rs

@@ -32,6 +32,7 @@ pub use request::{
     Request,
     RequestedBy,
     VarRequest,
+    VersionIterationOrder,
 };
 pub use satisfy::Satisfy;
 

crates/spk-schema/crates/ident/src/request.rs

@@ -28,6 +28,7 @@
     RestrictMode,
     VersionFilter,
 };
+use variantly::Variantly;
 
 use super::AnyIdent;
 use crate::{BuildIdent, Error, RangeIdent, Result, Satisfy, VersionIdent};
@@ -248,6 +249,7 @@
             pkg: Option<RangeIdent>,
             prerelease_policy: Option<PreReleasePolicy>,
             inclusion_policy: Option<InclusionPolicy>,
+            version_iteration_order: Option<VersionIterationOrder>,
 
             // VarRequest
             var: Option<OptNameBuf>,
@@ -291,6 +293,10 @@
                         }
                         "value" => self.value = Some(map.next_value::<String>()?),
                         "description" => self.description = Some(map.next_value::<String>()?),
+                        "versioniterationorder" => {
+                            self.version_iteration_order =
+                                Some(map.next_value::<VersionIterationOrder>()?)
+                        }
                         _ => {
                             // unrecognized fields are explicitly ignored in case
                             // they were added in a newer version of spk. We assume
@@ -313,6 +319,7 @@
                         inclusion_policy: self.inclusion_policy.unwrap_or_default(),
                         pin_policy: self.pin_policy.unwrap_or_default(),
                         pin: self.pin.unwrap_or_default().into_pkg_pin(),
+                        version_iteration_order: self.version_iteration_order,
                         required_compat: None,
                         requested_by: Default::default(),
                     })),
@@ -637,6 +644,27 @@
     }
 }
 
+/// Specify what order versions should be iterated over when resolving.
+#[derive(
+    Clone,
+    Copy,
+    Debug,
+    Default,
+    Deserialize,
+    Eq,
+    Hash,
+    Ord,
+    PartialEq,
+    PartialOrd,
+    Serialize,
+    Variantly,
+)]
+pub enum VersionIterationOrder {
+    #[default]
+    NewestFirst,
+    OldestFirst,
+}
+
 /// A desired package and set of restrictions on how it's selected.
 #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd, Serialize)]
 pub struct PkgRequest {
@@ -665,6 +693,12 @@
         skip_serializing_if = "PinPolicy::is_default"
     )]
     pub pin_policy: PinPolicy,
+    #[serde(
+        rename = "versionIterationOrder",
+        default,
+        skip_serializing_if = "Option::is_none"
+    )]
+    pub version_iteration_order: Option<VersionIterationOrder>,
     #[serde(skip)]
     pub required_compat: Option<CompatRule>,
     // The 'requested_by' field is a BTreeMap to keep all the
@@ -735,6 +769,7 @@
             inclusion_policy: Default::default(),
             pin_policy: Default::default(),
             pin: Default::default(),
+            version_iteration_order: Default::default(),
             required_compat: Some(CompatRule::Binary),
             requested_by: BTreeMap::from([(key, vec![requester])]),
         }

crates/spk-solve/crates/graph/src/graph.rs

@@ -20,7 +20,14 @@ use spk_schema::foundation::name::{OptNameBuf, PkgName, PkgNameBuf};
 use spk_schema::foundation::option_map;
 use spk_schema::foundation::option_map::OptionMap;
 use spk_schema::foundation::version::Compatibility;
-use spk_schema::ident::{InclusionPolicy, PkgRequest, Request, RequestedBy, VarRequest};
+use spk_schema::ident::{
+    InclusionPolicy,
+    PkgRequest,
+    Request,
+    RequestedBy,
+    VarRequest,
+    VersionIterationOrder,
+};
 use spk_schema::prelude::*;
 use spk_schema::{
     AnyIdent,
@@ -533,12 +540,13 @@ impl Graph {
                         _ => &old_node,
                     };
 
-                    for (name, iterator) in
-                        node_to_copy_iterators_from.read().await.iterators.iter()
+                    for (name, sub_map) in node_to_copy_iterators_from.read().await.iterators.iter()
                     {
-                        Arc::make_mut(&mut new_node_lock)
-                            .set_iterator(name.clone(), iterator)
-                            .await
+                        for (version_iteration_order, iterator) in sub_map.iter() {
+                            Arc::make_mut(&mut new_node_lock)
+                                .set_iterator(name.clone(), *version_iteration_order, iterator)
+                                .await
+                        }
                     }
                 }
                 Some(node) => {
@@ -680,6 +688,9 @@ impl<'graph> GraphIter<'graph> {
     }
 }
 
+pub type VersionIteratorsByIterationOrder =
+    HashMap<VersionIterationOrder, Arc<tokio::sync::Mutex<Box<dyn PackageIterator + Send>>>>;
+
 #[derive(Clone, Debug)]
 pub struct Node {
     // Preserve order of inputs/outputs for iterating
@@ -689,7 +700,7 @@ pub struct Node {
     outputs: HashSet<u64>,
     outputs_decisions: Vec<Arc<Decision>>,
     pub state: Arc<State>,
-    iterators: HashMap<PkgNameBuf, Arc<tokio::sync::Mutex<Box<dyn PackageIterator + Send>>>>,
+    iterators: HashMap<PkgNameBuf, VersionIteratorsByIterationOrder>,
 }
 
 impl Node {
@@ -710,8 +721,12 @@ impl Node {
     pub fn get_iterator(
         &self,
         package_name: &PkgName,
+        version_iteration_order: VersionIterationOrder,
     ) -> Option<Arc<tokio::sync::Mutex<Box<dyn PackageIterator + Send>>>> {
-        self.iterators.get(package_name).cloned()
+        self.iterators
+            .get(package_name)
+            .and_then(|h| h.get(&version_iteration_order))
+            .cloned()
     }
 
     pub fn new(state: Arc<State>) -> Self {
@@ -733,14 +748,21 @@ impl Node {
     pub async fn set_iterator(
         &mut self,
         package_name: PkgNameBuf,
+        version_iteration_order: VersionIterationOrder,
         iterator: &Arc<tokio::sync::Mutex<Box<dyn PackageIterator + Send>>>,
     ) {
-        if self.iterators.contains_key(&package_name) {
+        if self
+            .iterators
+            .get(&package_name)
+            .and_then(|h| h.get(&version_iteration_order))
+            .is_some()
+        {
             tracing::error!("iterator already exists [INTERNAL ERROR]");
             debug_assert!(false, "iterator already exists [INTERNAL ERROR]");
         }
-        self.iterators.insert(
-            package_name,
+
+        self.iterators.entry(package_name).or_default().insert(
+            version_iteration_order,
             Arc::new(tokio::sync::Mutex::new(
                 iterator.lock().await.async_clone().await,
             )),

crates/spk-solve/crates/package-iterator/Cargo.toml

@@ -20,15 +20,15 @@ async-trait = { workspace = true }
 dyn-clone = { workspace = true }
 futures = { workspace = true }
 glob = { workspace = true }
+miette = { workspace = true }
 once_cell = { workspace = true }
 spk-config = { workspace = true }
-spk-solve-solution = { workspace = true }
 spk-schema = { workspace = true }
+spk-solve-solution = { workspace = true }
 spk-storage = { workspace = true }
 thiserror = { workspace = true }
 tokio = { workspace = true, features = ["rt"] }
 tracing = { workspace = true }
-miette = { workspace = true }
 
 [dev-dependencies]
 itertools = { workspace = true }

crates/spk-solve/crates/package-iterator/src/package_iterator.rs

@@ -12,7 +12,7 @@ use once_cell::sync::Lazy;
 use spk_schema::foundation::name::{OptNameBuf, PkgNameBuf, RepositoryNameBuf};
 use spk_schema::foundation::option_map::OptionMap;
 use spk_schema::foundation::version::Version;
-use spk_schema::ident::VersionIdent;
+use spk_schema::ident::{VersionIdent, VersionIterationOrder};
 use spk_schema::version::Compatibility;
 use spk_schema::{AnyIdent, BuildIdent, Package, Spec};
 use spk_solve_solution::PackageSource;
@@ -114,6 +114,7 @@ pub struct RepositoryPackageIterator {
     builds_map: HashMap<Version, Arc<tokio::sync::Mutex<dyn BuildIterator + Send>>>,
     active_version: Option<Arc<Version>>,
     embedded_stubs: bool,
+    version_iteration_order: VersionIterationOrder,
 }
 
 #[async_trait::async_trait]
@@ -126,6 +127,7 @@ impl PackageIterator for RepositoryPackageIterator {
                 Err(Error::SpkStorageError(spk_storage::Error::PackageNotFound(_))) => {
                     return Box::new(RepositoryPackageIterator::new(
                         self.package_name.clone(),
+                        self.version_iteration_order,
                         self.repos.clone(),
                     ))
                 }
@@ -152,6 +154,7 @@ impl PackageIterator for RepositoryPackageIterator {
             builds_map: HashMap::default(),
             active_version: None,
             embedded_stubs: self.embedded_stubs,
+            version_iteration_order: self.version_iteration_order,
         })
     }
 
@@ -237,7 +240,11 @@ impl PackageIterator for RepositoryPackageIterator {
 }
 
 impl RepositoryPackageIterator {
-    pub fn new(package_name: PkgNameBuf, repos: Vec<Arc<RepositoryHandle>>) -> Self {
+    pub fn new(
+        package_name: PkgNameBuf,
+        version_iteration_order: VersionIterationOrder,
+        repos: Vec<Arc<RepositoryHandle>>,
+    ) -> Self {
         RepositoryPackageIterator {
             package_name,
             repos,
@@ -246,6 +253,7 @@ impl RepositoryPackageIterator {
             builds_map: HashMap::default(),
             active_version: None,
             embedded_stubs: false,
+            version_iteration_order,
         }
     }
 
@@ -290,7 +298,9 @@ impl RepositoryPackageIterator {
     async fn restart_version_iterator(&mut self) -> Result<()> {
         let mut versions: Vec<Arc<Version>> = self.version_map.keys().cloned().collect();
         versions.sort();
-        versions.reverse();
+        if self.version_iteration_order.is_newest_first() {
+            versions.reverse();
+        }
         self.versions = Some(VersionIterator::new(versions.into()));
         Ok(())
     }

crates/spk-solve/crates/package-iterator/src/package_iterator_test.rs

@@ -14,6 +14,7 @@ use spk_schema::{recipe, spec, BuildIdent, Package, Spec};
 use spk_solve_macros::{make_build, make_repo};
 
 use super::{BuildIterator, PackageIterator, RepositoryPackageIterator, SortedBuildIterator};
+use crate::package_iterator::VersionIterationOrder;
 
 #[rstest]
 #[tokio::test]
@@ -180,7 +181,11 @@ async fn test_solver_sorted_build_iterator_sort_by_option_values() {
     let arc_repo = Arc::new(repo);
     let repos = vec![Arc::clone(&arc_repo)];
 
-    let mut rp_iterator = RepositoryPackageIterator::new(pkg_name.to_owned(), repos.clone());
+    let mut rp_iterator = RepositoryPackageIterator::new(
+        pkg_name.to_owned(),
+        VersionIterationOrder::NewestFirst,
+        repos.clone(),
+    );
     while let Some((_pkg, builds)) = rp_iterator.next().await.unwrap() {
         // This runs the test, by sorting the builds
         let mut iterator = SortedBuildIterator::new(

crates/spk-solve/src/solver.rs

@@ -17,7 +17,14 @@ use spk_schema::foundation::ident_build::Build;
 use spk_schema::foundation::ident_component::Component;
 use spk_schema::foundation::name::{PkgName, PkgNameBuf};
 use spk_schema::foundation::version::Compatibility;
-use spk_schema::ident::{PkgRequest, Request, RequestedBy, Satisfy, VarRequest};
+use spk_schema::ident::{
+    PkgRequest,
+    Request,
+    RequestedBy,
+    Satisfy,
+    VarRequest,
+    VersionIterationOrder,
+};
 use spk_schema::ident_build::EmbeddedSource;
 use spk_schema::version::IncompatibleReason;
 use spk_schema::{try_recipe, BuildIdent, Deprecate, Package, Recipe, Spec, SpecRecipe};
@@ -319,24 +326,30 @@ impl Solver {
         &self,
         node: &mut Arc<Node>,
         package_name: &PkgName,
+        version_iteration_order: VersionIterationOrder,
     ) -> Arc<tokio::sync::Mutex<Box<dyn PackageIterator + Send>>> {
-        if let Some(iterator) = node.get_iterator(package_name) {
+        if let Some(iterator) = node.get_iterator(package_name, version_iteration_order) {
             return iterator;
         }
-        let iterator = self.make_iterator(package_name.to_owned());
+        let iterator = self.make_iterator(package_name.to_owned(), version_iteration_order);
         Arc::make_mut(node)
-            .set_iterator(package_name.to_owned(), &iterator)
+            .set_iterator(package_name.to_owned(), version_iteration_order, &iterator)
             .await;
         iterator
     }
 
     fn make_iterator(
         &self,
         package_name: PkgNameBuf,
+        version_iteration_order: VersionIterationOrder,
     ) -> Arc<tokio::sync::Mutex<Box<dyn PackageIterator + Send>>> {
         debug_assert!(!self.repos.is_empty());
         Arc::new(tokio::sync::Mutex::new(Box::new(
-            RepositoryPackageIterator::new(package_name, self.repos.clone()),
+            RepositoryPackageIterator::new(
+                package_name,
+                version_iteration_order,
+                self.repos.clone(),
+            ),
         )))
     }
 
@@ -511,7 +524,13 @@ impl Solver {
         // This is a step forward in the solve
         self.number_of_steps += 1;
 
-        let iterator = self.get_iterator(node, &request.pkg.name).await;
+        let iterator = self
+            .get_iterator(
+                node,
+                &request.pkg.name,
+                request.version_iteration_order.unwrap_or_default(),
+            )
+            .await;
         let mut iterator_lock = iterator.lock().await;
         loop {
             let (pkg, builds) = match iterator_lock.next().await {