Configure request promotion patterns separate from build key

Refactor BuildKeyPromotionPatterns to make it more generic so it can be
used for more than just "build key" ordering.

Create a new static / env var for configuring the patterns that will be
used reorder requests, so this can be configured independently of the
build key ordering configuration.

Start conservatively by only promoting "*platform*" requests, and revert
the build key configuration back to its original value.

Signed-off-by: J Robert Ray <jrray@imageworks.com>

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

@@ -4,6 +4,7 @@
 
 use std::collections::hash_map::{DefaultHasher, Entry};
 use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet, VecDeque};
+use std::ffi::OsString;
 use std::hash::{Hash, Hasher};
 use std::iter::FromIterator;
 use std::sync::atomic::{AtomicU64, Ordering};
@@ -30,7 +31,7 @@ use spk_schema::{
     Spec,
     SpecRecipe,
 };
-use spk_solve_package_iterator::{PackageIterator, BUILD_KEY_NAME_ORDER};
+use spk_solve_package_iterator::{PackageIterator, PromotionPatterns};
 use spk_solve_solution::{PackageSource, Solution};
 use thiserror::Error;
 
@@ -42,6 +43,18 @@ pub static DEAD_STATE: Lazy<Arc<State>> = Lazy::new(State::default_state);
 
 const BRANCH_ALREADY_ATTEMPTED: &str = "Branch already attempted";
 
+/// Allow the request order found as defined in package specs to be reordered,
+/// moving package names that match entries in this list of patterns to the
+/// front of the request list.
+static REQUESTS_PRIORITY_ORDER: Lazy<PromotionPatterns> = Lazy::new(|| {
+    PromotionPatterns::new(
+        std::env::var_os("SPK_REQUEST_PRIORITY_ORDER")
+            .unwrap_or_else(|| OsString::from("*platform*"))
+            .to_string_lossy()
+            .as_ref(),
+    )
+});
+
 #[derive(Debug, Error)]
 #[allow(clippy::large_enum_variant)]
 pub enum GraphError {
@@ -863,7 +876,7 @@ impl RequestPackage {
 
             // Apply the configured request priority ordering to the request
             // list.
-            BUILD_KEY_NAME_ORDER
+            REQUESTS_PRIORITY_ORDER
                 .promote_names(new_requests.as_mut_slice(), |req| req.pkg.name().as_str());
         }
 

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

@@ -8,6 +8,7 @@
 mod build_key;
 mod error;
 mod package_iterator;
+mod promotion_patterns;
 
 pub use error::{Error, Result};
 pub use package_iterator::{
@@ -16,6 +17,6 @@ pub use package_iterator::{
     PackageIterator,
     RepositoryPackageIterator,
     SortedBuildIterator,
-    BUILD_KEY_NAME_ORDER,
     BUILD_SORT_TARGET,
 };
+pub use promotion_patterns::PromotionPatterns;

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

@@ -9,7 +9,6 @@ use std::sync::Arc;
 use std::time::{Duration, Instant};
 
 use dyn_clone::DynClone;
-use glob::Pattern;
 use once_cell::sync::Lazy;
 use spk_schema::foundation::name::{OptNameBuf, PkgNameBuf, RepositoryNameBuf};
 use spk_schema::foundation::option_map::OptionMap;
@@ -21,47 +20,12 @@ use spk_solve_solution::PackageSource;
 use spk_storage::RepositoryHandle;
 
 use crate::build_key::BuildKey;
-use crate::{Error, Result};
+use crate::{Error, PromotionPatterns, Result};
 
 #[cfg(test)]
 #[path = "./package_iterator_test.rs"]
 mod package_iterator_test;
 
-pub struct BuildKeyPromotionPatterns(Vec<Pattern>);
-
-impl BuildKeyPromotionPatterns {
-    /// Parse a comma-separated string into a list of patterns.
-    fn new(comma_separated_patterns: &str) -> Self {
-        Self(
-            comma_separated_patterns
-                .split(',')
-                .filter_map(|p| Pattern::new(p).ok())
-                .collect(),
-        )
-    }
-
-    /// Sort the given list by moving any entries that match the list of
-    /// promoted names to the front, but otherwise preserving the original
-    /// order. The function `f` is used to extract the name to compare to for
-    /// each element of the list.
-    ///
-    /// Entries that match are ordered based on the order of the patterns,
-    /// where patterns at a lower index are prioritized.
-    pub fn promote_names<N, F>(&self, names: &mut [N], f: F)
-    where
-        F: Fn(&N) -> &str,
-    {
-        names.sort_by_cached_key(|name| {
-            self.0
-                .iter()
-                .enumerate()
-                .find(|(_, pattern)| pattern.matches(f(name)))
-                .map(|(index, _)| index)
-                .unwrap_or(usize::MAX)
-        })
-    }
-}
-
 /// Allows control of the order option names are using in build key
 /// generation. Names in this list will be put at the front of the
 /// list of option names used to generate keys for ordering builds
@@ -74,10 +38,10 @@ impl BuildKeyPromotionPatterns {
 /// the package name `"spi-platform"`.
 //
 // TODO: add the default value to a config file, once spk has one
-pub static BUILD_KEY_NAME_ORDER: Lazy<BuildKeyPromotionPatterns> = Lazy::new(|| {
-    BuildKeyPromotionPatterns::new(
+static BUILD_KEY_NAME_ORDER: Lazy<PromotionPatterns> = Lazy::new(|| {
+    PromotionPatterns::new(
         std::env::var_os("SPK_BUILD_OPTION_KEY_ORDER")
-            .unwrap_or_else(|| OsString::from("*platform*,gcc,python"))
+            .unwrap_or_else(|| OsString::from("gcc,python"))
             .to_string_lossy()
             .as_ref(),
     )

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

@@ -13,13 +13,7 @@ use spk_schema::foundation::version::Compatibility;
 use spk_schema::{recipe, spec, BuildIdent, Package, Spec};
 use spk_solve_macros::{make_build, make_repo};
 
-use super::{
-    BuildIterator,
-    BuildKeyPromotionPatterns,
-    PackageIterator,
-    RepositoryPackageIterator,
-    SortedBuildIterator,
-};
+use super::{BuildIterator, PackageIterator, RepositoryPackageIterator, SortedBuildIterator};
 
 #[rstest]
 #[tokio::test]
@@ -223,15 +217,3 @@ async fn test_solver_sorted_build_iterator_sort_by_option_values() {
         }
     }
 }
-
-#[rstest]
-#[case("gcc", &["a", "b", "gcc"], &["gcc", "a", "b"])]
-#[case::pattern_order_matters_1("gcc,python", &["a", "python", "b", "gcc"], &["gcc", "python", "a", "b"])]
-#[case::pattern_order_matters_2("python,gcc", &["a", "python", "b", "gcc"], &["python", "gcc", "a", "b"])]
-#[case::pattern_glob("*platform*,python,gcc", &["a", "python", "b", "gcc", "spi-platform"], &["spi-platform", "python", "gcc", "a", "b"])]
-fn test_promote_names(#[case] patterns: &str, #[case] input: &[&str], #[case] expected: &[&str]) {
-    let patterns = BuildKeyPromotionPatterns::new(patterns);
-    let mut subject = input.to_owned();
-    patterns.promote_names(subject.as_mut_slice(), |n| n);
-    assert_eq!(subject, expected)
-}

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

@@ -0,0 +1,45 @@
+// Copyright (c) Sony Pictures Imageworks, et al.
+// SPDX-License-Identifier: Apache-2.0
+// https://github.com/imageworks/spk
+
+use glob::Pattern;
+
+#[cfg(test)]
+#[path = "./promotion_patterns_test.rs"]
+mod promotion_patterns_test;
+
+/// A list of glob patterns that can be used to match and reorder other lists.
+pub struct PromotionPatterns(Vec<Pattern>);
+
+impl PromotionPatterns {
+    /// Parse a comma-separated string into a list of patterns.
+    pub fn new(comma_separated_patterns: &str) -> Self {
+        Self(
+            comma_separated_patterns
+                .split(',')
+                .filter_map(|p| Pattern::new(p).ok())
+                .collect(),
+        )
+    }
+
+    /// Sort the given list by moving any entries that match the list of
+    /// promoted names to the front, but otherwise preserving the original
+    /// order. The function `f` is used to extract the name to compare to for
+    /// each element of the list.
+    ///
+    /// Entries that match are ordered based on the order of the patterns,
+    /// where patterns at a lower index are prioritized.
+    pub fn promote_names<N, F>(&self, names: &mut [N], f: F)
+    where
+        F: Fn(&N) -> &str,
+    {
+        names.sort_by_cached_key(|name| {
+            self.0
+                .iter()
+                .enumerate()
+                .find(|(_, pattern)| pattern.matches(f(name)))
+                .map(|(index, _)| index)
+                .unwrap_or(usize::MAX)
+        })
+    }
+}

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

@@ -0,0 +1,19 @@
+// Copyright (c) Sony Pictures Imageworks, et al.
+// SPDX-License-Identifier: Apache-2.0
+// https://github.com/imageworks/spk
+
+use rstest::rstest;
+
+use crate::PromotionPatterns;
+
+#[rstest]
+#[case("gcc", &["a", "b", "gcc"], &["gcc", "a", "b"])]
+#[case::pattern_order_matters_1("gcc,python", &["a", "python", "b", "gcc"], &["gcc", "python", "a", "b"])]
+#[case::pattern_order_matters_2("python,gcc", &["a", "python", "b", "gcc"], &["python", "gcc", "a", "b"])]
+#[case::pattern_glob("*platform*,python,gcc", &["a", "python", "b", "gcc", "spi-platform"], &["spi-platform", "python", "gcc", "a", "b"])]
+fn test_promote_names(#[case] patterns: &str, #[case] input: &[&str], #[case] expected: &[&str]) {
+    let patterns = PromotionPatterns::new(patterns);
+    let mut subject = input.to_owned();
+    patterns.promote_names(subject.as_mut_slice(), |n| n);
+    assert_eq!(subject, expected)
+}