Merge pull request #1258 from spkenv/push-rvwyootwrpsk

Fix resolvo treating options as var requirements

Author: jrray

crates/spk-solve/src/solvers/resolvo/mod.rs

@@ -145,7 +145,6 @@ impl Solver {
     pub async fn solve(&self) -> Result<Solution> {
         let repos = self.repos.clone();
         let requests = self.requests.clone();
-        let options = self.options.clone();
         let binary_only = self.binary_only;
         let build_from_source_trail = self.build_from_source_trail.clone();
         // Use a blocking thread so resolvo can call `block_on` on the runtime.
@@ -160,11 +159,7 @@ impl Solver {
                 loop_counter += 1;
                 let mut this_iter_provider = provider.take().expect("provider is always Some");
                 let pkg_requirements = this_iter_provider.root_pkg_requirements(&requests);
-                let mut var_requirements = this_iter_provider.var_requirements(&requests);
-                // XXX: Not sure if this will result in the desired precedence
-                // when options and var requests for the same thing exist.
-                var_requirements
-                    .extend(this_iter_provider.var_requirements_from_options(options.clone()));
+                let var_requirements = this_iter_provider.var_requirements(&requests);
                 let mut solver = resolvo::Solver::new(this_iter_provider)
                     .with_runtime(tokio::runtime::Handle::current());
                 let problem = resolvo::Problem::new()

crates/spk-solve/src/solvers/resolvo/spk_provider.rs

@@ -39,17 +39,7 @@ use spk_schema::ident_component::Component;
 use spk_schema::name::{OptNameBuf, PkgNameBuf};
 use spk_schema::prelude::{HasVersion, Named};
 use spk_schema::version_range::{DoubleEqualsVersion, Ranged, VersionFilter, parse_version_range};
-use spk_schema::{
-    BuildIdent,
-    Deprecate,
-    Opt,
-    OptionMap,
-    Package,
-    Recipe,
-    Request,
-    Spec,
-    VersionIdent,
-};
+use spk_schema::{BuildIdent, Deprecate, Opt, Package, Recipe, Request, Spec, VersionIdent};
 use spk_solve_package_iterator::{BuildKey, BuildToSortedOptName, SortedBuildIterator};
 use spk_storage::RepositoryHandle;
 use tracing::{Instrument, debug_span};
@@ -863,45 +853,6 @@ impl SpkProvider {
             })
             .collect()
     }
-
-    pub fn var_requirements_from_options(&mut self, options: OptionMap) -> Vec<VersionSetId> {
-        self.global_var_requests.reserve(options.len());
-        options
-            .into_iter()
-            .filter_map(|(var, value)| {
-                let var_value = VarValue::Owned(value.clone());
-                let req = VarRequest::new_with_value(var, value);
-                match req.var.namespace() {
-                    Some(pkg_name) => {
-                        // A global request applicable to a specific package.
-                        let dep_name = self.pool.intern_package_name(
-                            ResolvoPackageName::PkgNameBufWithComponent(PkgNameBufWithComponent {
-                                name: pkg_name.to_owned(),
-                                component: SyntheticComponent::Base,
-                            }),
-                        );
-                        Some(
-                            self.pool.intern_version_set(
-                                dep_name,
-                                RequestVS::SpkRequest(Request::Var(req)),
-                            ),
-                        )
-                    }
-                    None => {
-                        // A global request affecting all packages.
-                        self.global_var_requests
-                            .insert(req.var.without_namespace().to_owned(), req.clone());
-                        self.known_global_var_values
-                            .borrow_mut()
-                            .entry(req.var.without_namespace().to_owned())
-                            .or_default()
-                            .insert(var_value);
-                        None
-                    }
-                }
-            })
-            .collect()
-    }
 }
 
 impl DependencyProvider for SpkProvider {

crates/spk-solve/src/solvers/solver_test.rs

@@ -3086,3 +3086,69 @@ async fn request_for_all_component_picks_correct_version(
     let solution = run_and_print_resolve_for_tests(&mut solver).await.unwrap();
     assert_resolved!(solution, "mypkg", version = version);
 }
+
+/// Verify that when solving the dependencies of a package, the build options of
+/// the candidates do not factor into the selection of the candidate.
+#[rstest]
+#[case::step(step_solver())]
+#[case::resolvo(resolvo_solver())]
+#[tokio::test]
+async fn build_options_not_checked_on_dependencies(#[case] mut solver: SolverImpl) {
+    // Suppose a platform exists
+    let spi_platform_2024_1_1_1 =
+        make_build!({"pkg": "spi-platform/2024.1.1.1", "compat": "x.x.a.b"});
+    // And a library package targets that platform version
+    let openimageio_1_2_3 = make_build!(
+            {
+                "pkg": "openimageio/1.2.3",
+                "build": {
+                    "options": [
+                        { "pkg": "spi-platform/~2024.1.1.1" },
+                    ],
+                },
+            },
+            [spi_platform_2024_1_1_1]
+    );
+    // And an application package depends on that library package
+    let my_app_4_5_6 = make_build!(
+            {
+                "pkg": "my-app/4.5.6",
+                "build": {
+                    "options": [
+                        { "pkg": "openimageio" },
+                    ],
+                },
+                "install": {
+                    "requirements": [
+                        { "pkg": "openimageio", "fromBuildEnv": true },
+                    ],
+                },
+            },
+            [openimageio_1_2_3]
+    );
+    // And a new version of the platform is published
+    let spi_platform_2025_1_1_1 =
+        make_build!({"pkg": "spi-platform/2025.1.1.1", "compat": "x.x.a.b"});
+    let repo = make_repo!([
+        spi_platform_2024_1_1_1,
+        openimageio_1_2_3,
+        my_app_4_5_6,
+        spi_platform_2025_1_1_1,
+    ]);
+
+    // Then we want to solve for my-app using the new platform version.
+    // Although the only build of openimageio was built using the old platform,
+    // it is expected to be resolvable when using the new platform.
+    let repo = Arc::new(repo);
+
+    solver.add_repository(repo);
+    // The platform version is specified as a build option rather than a request
+    solver.update_options(option_map! {
+        "spi-platform" => "~2025.1.1.1"
+    });
+    solver.add_request(request!("my-app"));
+
+    let solution = run_and_print_resolve_for_tests(&mut solver).await.unwrap();
+    assert_resolved!(solution, "my-app", "4.5.6");
+    assert_resolved!(solution, "openimageio", "1.2.3");
+}