diff --git a/crates/spk-build/src/build/binary_test.rs b/crates/spk-build/src/build/binary_test.rs index f79761d6a..a9749af56 100644 --- a/crates/spk-build/src/build/binary_test.rs +++ b/crates/spk-build/src/build/binary_test.rs @@ -11,6 +11,7 @@ use spfstest::spfstest; use spk_schema::foundation::env::data_path; use spk_schema::foundation::fixtures::*; use spk_schema::foundation::ident_component::Component; +use spk_schema::foundation::spec_ops::HasVersion; use spk_schema::foundation::{opt_name, option_map, version_ident}; use spk_schema::ident::{PkgRequest, PkgRequestWithOptions, RangeIdent, RequestWithOptions}; use spk_schema::{ @@ -24,8 +25,8 @@ use spk_schema::{ SpecRecipe, recipe, }; -use spk_solve::{Solution, SolverImpl}; -use spk_solve_macros::make_repo; +use spk_solve::{Solution, SolverExt, SolverImpl, SolverMut}; +use spk_solve_macros::{make_repo, pinned_request}; use spk_storage::fixtures::*; use spk_storage::{self as storage, Repository}; @@ -274,6 +275,122 @@ async fn test_build_package_pinning( } } +#[spfstest] +#[rstest] +#[case::step(step_solver())] +#[case::resolvo(resolvo_solver())] +#[tokio::test] +async fn test_build_package_with_v1_platform_pins_frombuildenv_and_solves_binary_compatible_runtime( + #[case] solver: SolverImpl, +) { + let rt = spfs_runtime().await; + let dep_1_0_0 = recipe!( + { + "pkg": "dep/1.0.0", + "build": {"script": "touch /spfs/dep-file"}, + } + ); + let dep_1_0_1 = recipe!( + { + "pkg": "dep/1.0.1", + "build": {"script": "touch /spfs/dep-file"}, + } + ); + let platform = SpecRecipe::from_yaml( + r#"{ + api: "v1/platform", + platform: "dep-platform/1.0.0", + requirements: [ + { + pkg: "dep", + atBuild: "=1.0.0", + atRuntime: "Binary:1.0.0", + } + ], + }"#, + ) + .unwrap(); + let downstream = recipe!( + { + "pkg": "consumer/1.0.0", + "build": { + "script": ["touch /spfs/consumer-file"], + "options": [ + {"pkg": "dep"}, + {"pkg": "dep-platform/1.0.0"}, + ], + }, + "install": { + "requirements": [ + {"pkg": "dep", "fromBuildEnv": true}, + ] + }, + } + ); + + for recipe in [&dep_1_0_0, &dep_1_0_1] { + rt.tmprepo.publish_recipe(recipe).await.unwrap(); + + BinaryPackageBuilder::from_recipe_with_solver(recipe.clone(), solver.clone()) + .with_source(BuildSource::LocalPath(".".into())) + .with_repository(rt.tmprepo.clone()) + .build_and_publish(option_map! {}, &*rt.tmprepo) + .await + .unwrap(); + } + + rt.tmprepo.publish_recipe(&platform).await.unwrap(); + let (_platform, _) = BinaryPackageBuilder::from_recipe_with_solver(platform, solver.clone()) + .with_source(BuildSource::LocalPath(".".into())) + .with_repository(rt.tmprepo.clone()) + .build_and_publish(option_map! {}, &*rt.tmprepo) + .await + .unwrap(); + + rt.tmprepo.publish_recipe(&downstream).await.unwrap(); + let (downstream, _) = + BinaryPackageBuilder::from_recipe_with_solver(downstream.clone(), solver.clone()) + .with_source(BuildSource::LocalPath(".".into())) + .with_repository(rt.tmprepo.clone()) + .build_and_publish(option_map! {}, &*rt.tmprepo) + .await + .unwrap(); + + let downstream = rt.tmprepo.read_package(downstream.ident()).await.unwrap(); + let req = downstream.runtime_requirements().first().unwrap().clone(); + match req { + RequestWithOptions::Pkg(req) => { + assert_eq!(req.pkg.to_string(), "dep/Binary:1.0.0"); + } + _ => panic!("expected a package request"), + } + + let mut solver = solver; + solver.add_repository(rt.tmprepo.clone()); + solver.add_request(pinned_request!("dep/=1.0.1")); + solver.add_request(pinned_request!("dep-platform")); + solver.add_request(pinned_request!("consumer")); + + let solution = solver.solve().await.unwrap(); + assert_eq!( + solution.get("dep").unwrap().spec.version().to_string(), + "1.0.1" + ); + assert_eq!( + solution + .get("dep-platform") + .unwrap() + .spec + .version() + .to_string(), + "1.0.0" + ); + assert_eq!( + solution.get("consumer").unwrap().spec.version().to_string(), + "1.0.0" + ); +} + #[spfstest] #[rstest] #[case::step(step_solver())] diff --git a/crates/spk-cli/cmd-make-binary/src/cmd_make_binary_test.rs b/crates/spk-cli/cmd-make-binary/src/cmd_make_binary_test.rs index 04a9ac372..0ffa8a0ad 100644 --- a/crates/spk-cli/cmd-make-binary/src/cmd_make_binary_test.rs +++ b/crates/spk-cli/cmd-make-binary/src/cmd_make_binary_test.rs @@ -83,3 +83,52 @@ build: .await .expect("With override, build script should succeed."); } + +#[spfstest] +#[rstest] +#[case::v0("v0/platform", true)] +#[case::v1("v1/platform", true)] +#[tokio::test] +async fn build_a_platform( + tmpdir: tempfile::TempDir, + #[case] api: &str, + #[case] should_succeed: bool, +) { + let _rt = spfs_runtime().await; + + let filename = tmpdir.path().join("simple.spk.yaml"); + { + let mut file = File::create(&filename).unwrap(); + file.write_all( + format!( + r#" +platform: demo-platform/1.0.0 +api: {api} +requirements: [] +"# + ) + .as_bytes(), + ) + .unwrap(); + } + + let filename_str = filename.as_os_str().to_str().unwrap(); + + let mut opt = Opt::try_parse_from([ + "make-binary", + // Don't exec a new process to move into a new runtime, this confuses + // coverage testing. + "--no-runtime", + "--disable-repo=origin", + "--here", + filename_str, + ]) + .unwrap(); + let res = opt.mkb.run().await; + + if should_succeed { + res.expect("Build should succeed."); + } else { + assert!(res.is_err(), "Build should fail, got {res:?}."); + } +} diff --git a/crates/spk-schema/src/spec.rs b/crates/spk-schema/src/spec.rs index e84c445f1..5eb3c53a8 100644 --- a/crates/spk-schema/src/spec.rs +++ b/crates/spk-schema/src/spec.rs @@ -481,6 +481,11 @@ impl FromYaml for SpecRecipe { .map_err(|err| SerdeError::new(yaml, SerdeYamlError(err)))?; Ok(Self::V0Platform(inner)) } + ApiVersion::V1Platform => { + let inner = serde_yaml::from_str(&yaml) + .map_err(|err| SerdeError::new(yaml, SerdeYamlError(err)))?; + Ok(Self::V1Platform(inner)) + } ApiVersion::V0Requirements => { // Reading a list of requests/requirements file is not // supported here. But it might be in future. @@ -567,6 +572,11 @@ impl SpecFileData { .map_err(|err| SerdeError::new(yaml, SerdeYamlError(err)))?; SpecFileData::Recipe(Arc::new(SpecRecipe::V0Platform(inner))) } + ApiVersion::V1Platform => { + let inner = serde_yaml::from_value(value) + .map_err(|err| SerdeError::new(yaml, SerdeYamlError(err)))?; + SpecFileData::Recipe(Arc::new(SpecRecipe::V1Platform(inner))) + } ApiVersion::V0Requirements => { let requests: v0::Requirements = serde_yaml::from_value(value) .map_err(|err| SerdeError::new(yaml, SerdeYamlError(err)))?; @@ -885,6 +895,11 @@ impl FromYaml for Spec { .map_err(|err| SerdeError::new(yaml, SerdeYamlError(err)))?; Ok(Self::V0Package(inner)) } + ApiVersion::V1Platform => { + let inner = serde_yaml::from_str(&yaml) + .map_err(|err| SerdeError::new(yaml, SerdeYamlError(err)))?; + Ok(Self::V0Package(inner)) + } ApiVersion::V0Requirements => { // Reading a list of requests/requirement file is not // supported here. But it might be in future. @@ -925,6 +940,8 @@ pub enum ApiVersion { V0Package, #[serde(rename = "v0/platform")] V0Platform, + #[serde(rename = "v1/platform")] + V1Platform, #[serde(rename = "v0/requirements")] V0Requirements, } diff --git a/crates/spk-schema/src/spec_test.rs b/crates/spk-schema/src/spec_test.rs index 3d34cf868..f30fb9cf0 100644 --- a/crates/spk-schema/src/spec_test.rs +++ b/crates/spk-schema/src/spec_test.rs @@ -358,3 +358,27 @@ fn test_template_namespace_options() { let recipe = rendered_data.into_recipe().unwrap(); assert_eq!(recipe.version().to_string(), "1.0.0"); } + +#[rstest] +fn test_template_render_supports_v1_platform_recipes() { + static SPEC: &str = r#" +api: v1/platform +platform: my-platform/{{ opt.version }} +requirements: [] +"#; + + let tpl = SpecTemplate { + name: Some(PkgName::new("my-platform").unwrap().to_owned()), + file_path: "my-platform.spk.yaml".into(), + versions: Default::default(), + template: SPEC.into(), + }; + let options = option_map! {"version" => "1.0.0"}; + let rendered_data = tpl + .render(&options) + .expect("template should render a v1/platform recipe"); + let recipe = rendered_data.into_recipe().unwrap(); + + assert!(matches!(recipe.as_ref(), crate::SpecRecipe::V1Platform(_))); + assert_eq!(recipe.ident().to_string(), "my-platform/1.0.0"); +} diff --git a/crates/spk-solve/crates/graph/src/graph.rs b/crates/spk-solve/crates/graph/src/graph.rs index 55c4c8d87..f5b80054d 100644 --- a/crates/spk-solve/crates/graph/src/graph.rs +++ b/crates/spk-solve/crates/graph/src/graph.rs @@ -471,9 +471,24 @@ impl<'state> DecisionBuilder<'state, '_> { // is buggy now continue; } - changes.extend( - self.requirements_to_changes(component.requirements_with_options(), &requested_by), - ); + changes.extend(component.requirements_with_options().iter().flat_map( + |req| match req { + RequestWithOptions::Pkg(req) => { + let mut req = req.clone(); + req.add_requester(requested_by.clone()); + if component.name == Component::Build + && req.pkg.components.is_empty() + && !req.pkg.is_source() + { + req.pkg.components.insert(Component::default_for_build()); + } + self.pkg_request_to_changes(&req) + } + RequestWithOptions::Var(req) => { + vec![Change::RequestVar(RequestVar::new(req.clone()))] + } + }, + )); } changes } diff --git a/crates/spk-solve/src/solvers/resolvo/spk_provider.rs b/crates/spk-solve/src/solvers/resolvo/spk_provider.rs index 550dd7325..4c324368b 100644 --- a/crates/spk-solve/src/solvers/resolvo/spk_provider.rs +++ b/crates/spk-solve/src/solvers/resolvo/spk_provider.rs @@ -719,19 +719,6 @@ impl SpkProvider { .collect() } - /// Return a list of requirements for all the package requests found in the - /// given requests. - fn dep_pkg_requirements(&self, requests: &[RequestWithOptions]) -> Vec { - requests - .iter() - .filter_map(|req| match req { - RequestWithOptions::Pkg(pkg) => Some(pkg), - _ => None, - }) - .flat_map(|req| self.pkg_request_to_known_dependencies(req).requirements) - .collect() - } - pub fn is_canceled(&self) -> bool { self.cancel_solving.borrow().is_some() } @@ -821,6 +808,27 @@ impl SpkProvider { known_deps } + /// Return known dependencies for a requirement as interpreted from the + /// given component context. + fn request_to_known_dependencies_in_component_context( + &self, + requirement: &RequestWithOptions, + component: &Component, + ) -> KnownDependencies { + let mut requirement = requirement.clone(); + if *component == Component::Build + && let RequestWithOptions::Pkg(pkg_request) = &mut requirement + && pkg_request.pkg.components.is_empty() + && !pkg_request.pkg.is_source() + { + pkg_request + .pkg + .components + .insert(Component::default_for_build()); + } + self.request_to_known_dependencies(&requirement) + } + /// Return a new provider to restart the solve, preserving what was learned /// about global variables. pub fn reset(&self) -> Self { @@ -1437,9 +1445,14 @@ impl DependencyProvider for SpkProvider { .into(), ); }); - known_deps.requirements.extend( - self.dep_pkg_requirements(component_spec.requirements_with_options()), - ); + for requirement in component_spec.requirements_with_options().iter() { + let kd = self.request_to_known_dependencies_in_component_context( + requirement, + actual_component, + ); + known_deps.requirements.extend(kd.requirements); + known_deps.constrains.extend(kd.constrains); + } } } // Also add dependencies on any packages embedded in this @@ -1557,8 +1570,10 @@ impl DependencyProvider for SpkProvider { embedded_component.requirements_with_options().iter() }) { - let kd = - self.request_to_known_dependencies(embedded_component_requirement); + let kd = self.request_to_known_dependencies_in_component_context( + embedded_component_requirement, + actual_component, + ); known_deps.requirements.extend(kd.requirements); known_deps.constrains.extend(kd.constrains); } diff --git a/crates/spk-solve/src/solvers/solver_test.rs b/crates/spk-solve/src/solvers/solver_test.rs index 0be69a1ba..f57043550 100644 --- a/crates/spk-solve/src/solvers/solver_test.rs +++ b/crates/spk-solve/src/solvers/solver_test.rs @@ -292,9 +292,9 @@ async fn test_solver_package_with_no_recipe_from_cmd_line( .into_iter() .collect(); repo.publish_package(&spec, &components).await.unwrap(); - let repo = wrap_repo_for_test(repo, use_index).await; + let repo = Arc::new(wrap_repo_for_test(repo, use_index).await); - solver.add_repository(Arc::new(repo)); + solver.add_repository(repo.clone()); // Create this one as requested by the command line, rather than the tests let req = PinnedRequest::Pkg(PkgRequest::new( parse_ident_range("my-pkg").unwrap(), @@ -463,9 +463,9 @@ async fn test_solver_dependency_incompatible( }, ] ); - let repo = wrap_repo_for_test(repo, use_index).await; + let repo = Arc::new(wrap_repo_for_test(repo, use_index).await); - solver.add_repository(Arc::new(repo)); + solver.add_repository(repo.clone()); solver.add_request(pinned_request!("my-plugin/1")); // this one is incompatible with requirements of my-plugin but the solver doesn't know it yet solver.add_request(pinned_request!("maya/2019")); @@ -501,9 +501,9 @@ async fn test_solver_dependency_incompatible_stepback( }, ] ); - let repo = wrap_repo_for_test(repo, use_index).await; + let repo = Arc::new(wrap_repo_for_test(repo, use_index).await); - solver.add_repository(Arc::new(repo)); + solver.add_repository(repo.clone()); solver.add_request(pinned_request!("my-plugin/1")); // this one is incompatible with requirements of my-plugin/1.1.0 but not my-plugin/1.0 solver.add_request(pinned_request!("maya/2019")); @@ -2793,6 +2793,142 @@ async fn test_solver_component_availability( assert_resolved!(solution, "python", components = ["bin", "lib"]); } +#[rstest] +#[case::step(step_solver())] +#[case::resolvo(resolvo_solver())] +#[tokio::test] +async fn test_solver_build_component_ifalreadypresent_uses_build_context( + #[case] mut solver: SolverImpl, + #[values(true, false)] use_index: bool, +) { + let repo = make_repo!( + [ + {"pkg": "dep/1.0.0"}, + {"pkg": "dep/1.0.1"}, + { + "pkg": "dep-carrier/1.0.0", + "install": { + "components": [ + { + "name": "build", + "requirements": [ + {"pkg": "dep/=1.0.0", "include": "IfAlreadyPresent"} + ] + }, + { + "name": "run", + "requirements": [ + {"pkg": "dep/Binary:1.0.0", "include": "IfAlreadyPresent"} + ] + } + ] + } + } + ] + ); + let repo = Arc::new(wrap_repo_for_test(repo, use_index).await); + + solver.add_repository(repo.clone()); + solver.add_request(pinned_request!("dep-carrier:build")); + solver.add_request(pinned_request!("dep:build")); + + let solution = run_and_print_resolve_for_tests(&mut solver).await.unwrap(); + assert_resolved!(solution, "dep", "1.0.0"); + + solver.reset(); + solver.add_repository(repo); + solver.add_request(pinned_request!("dep-carrier")); + solver.add_request(pinned_request!("dep/=1.0.1")); + + let solution = run_and_print_resolve_for_tests(&mut solver).await.unwrap(); + + assert_resolved!(solution, "dep", "1.0.1"); +} + +#[rstest] +#[case::step(step_solver())] +#[case::resolvo(resolvo_solver())] +#[tokio::test] +async fn test_solver_build_component_ifalreadypresent_reconsiders_existing_build_request( + #[case] mut solver: SolverImpl, + #[values(true, false)] use_index: bool, +) { + let repo = make_repo!( + [ + {"pkg": "dep/1.0.0"}, + {"pkg": "dep/1.0.1"}, + { + "pkg": "dep-carrier/1.0.0", + "install": { + "components": [ + { + "name": "build", + "requirements": [ + {"pkg": "dep/=1.0.0", "include": "IfAlreadyPresent"} + ] + } + ] + } + } + ] + ); + let repo = Arc::new(wrap_repo_for_test(repo, use_index).await); + + solver.add_repository(repo); + solver.add_request(pinned_request!("dep:build")); + solver.add_request(pinned_request!("dep-carrier:build")); + + let solution = run_and_print_resolve_for_tests(&mut solver).await.unwrap(); + assert_resolved!(solution, "dep", "1.0.0"); +} + +#[rstest] +#[case::step(step_solver())] +#[case::resolvo(resolvo_solver())] +#[tokio::test] +async fn test_solver_build_component_ifalreadypresent_reconsiders_dependent_package( + #[case] mut solver: SolverImpl, + #[values(true, false)] use_index: bool, +) { + let repo = make_repo!( + [ + {"pkg": "dep/1.0.0"}, + {"pkg": "dep/1.0.1"}, + { + "pkg": "consumer/1.0.0", + "install": {"requirements": [{"pkg": "dep/=1.0.0"}]}, + }, + { + "pkg": "consumer/1.0.1", + "install": {"requirements": [{"pkg": "dep/=1.0.1"}]}, + }, + { + "pkg": "dep-carrier/1.0.0", + "install": { + "components": [ + { + "name": "build", + "requirements": [ + {"pkg": "dep/=1.0.0", "include": "IfAlreadyPresent"} + ] + } + ] + } + } + ] + ); + let repo = Arc::new(wrap_repo_for_test(repo, use_index).await); + + solver.add_repository(repo); + solver.add_request(pinned_request!("consumer")); + solver.add_request(pinned_request!("dep:build")); + solver.add_request(pinned_request!("dep-carrier:build")); + + let solution = run_and_print_resolve_for_tests(&mut solver).await.unwrap(); + assert_resolved!(solution, "consumer", "1.0.0"); + assert_resolved!(solution, "dep", "1.0.0"); +} + #[rstest] #[case::step(step_solver())] #[case::resolvo(resolvo_solver())] diff --git a/crates/spk-solve/src/solvers/step/solver.rs b/crates/spk-solve/src/solvers/step/solver.rs index b84b56e7d..33b69b79d 100644 --- a/crates/spk-solve/src/solvers/step/solver.rs +++ b/crates/spk-solve/src/solvers/step/solver.rs @@ -63,7 +63,13 @@ use spk_solve_package_iterator::{ SortedBuildIterator, }; use spk_solve_solution::{PackageSource, Solution}; -use spk_solve_validation::validators::BinaryOnlyValidator; +use spk_solve_validation::validators::{ + BinaryOnlyValidator, + OptionsValidator, + PkgRequestValidator, + PkgRequirementsValidator, + VarRequirementsValidator, +}; use spk_solve_validation::{ IMPOSSIBLE_CHECKS_TARGET, ImpossibleRequestsChecker, @@ -475,6 +481,7 @@ impl Solver { node: &mut Arc, ) -> Result> { let mut notes = Vec::::new(); + self.ensure_resolved_packages_are_valid(&node.state)?; let request = if let Some(request) = node.state.get_next_request()? { request } else { @@ -946,6 +953,98 @@ impl Solver { }))) } + fn ensure_resolved_packages_are_valid(&self, state: &State) -> Result<()> { + for package in state.get_ordered_resolved_packages().iter() { + let Some((spec, source, _)) = state.get_resolved_packages().get(package.name()) else { + continue; + }; + if spec.ident().build().is_embedded() { + continue; + } + + let request = match state.get_merged_request(spec.name()) { + Ok(request) => request, + Err(err) => { + return Err(Error::OutOfOptions(Box::new(OutOfOptions { + request: Self::request_for_resolved_package(state, spec.name())?, + notes: vec![Note::Other(format!( + "resolved package {} has an invalid request stack: {err}", + spec.ident() + ))], + }))); + } + }; + + for compat in [ + (PkgRequestValidator {}).validate_package(state, spec.as_ref(), source), + OptionsValidator::default().validate_package(state, spec.as_ref(), source), + VarRequirementsValidator::default().validate_package(state, spec.as_ref(), source), + ] { + let compat = compat.map_err(Error::ValidationError)?; + if !compat.is_ok() { + return Err(Error::OutOfOptions(Box::new(OutOfOptions { + request: request.pkg_request.clone(), + notes: vec![Note::Other(format!( + "resolved package {} no longer satisfies the current state: {compat}", + spec.ident() + ))], + }))); + } + } + + let compat = match (PkgRequirementsValidator {}).validate_package( + state, + spec.as_ref(), + source, + ) { + Ok(compat) => compat, + Err(spk_solve_validation::Error::SpkSolverGraphGetMergedRequestError(err)) => { + return Err(Error::OutOfOptions(Box::new(OutOfOptions { + request: Self::request_for_resolved_package(state, spec.name())?, + notes: vec![Note::Other(format!( + "resolved package {} relies on an invalid request stack: {err}", + spec.ident() + ))], + }))); + } + Err(err) => return Err(Error::ValidationError(err)), + }; + if !compat.is_ok() { + return Err(Error::OutOfOptions(Box::new(OutOfOptions { + request: request.pkg_request, + notes: vec![Note::Other(format!( + "resolved package {} no longer satisfies the current state: {compat}", + spec.ident() + ))], + }))); + } + } + + Ok(()) + } + + fn request_for_resolved_package(state: &State, name: &PkgName) -> Result { + let mut requests = state + .get_pkg_requests() + .iter() + .filter(|request| request.pkg.name == *name) + .map(|request| &request.pkg_request); + + let Some(mut request) = requests.next().cloned() else { + return Err(Error::String(format!( + "resolved package '{name}' has no package requests [INTERNAL ERROR]" + ))); + }; + + for extra_request in requests { + for requester in extra_request.get_requesters() { + request.add_requester(requester); + } + } + + Ok(request) + } + fn validate_recipe(&self, state: &State, recipe: &R) -> Result { for validator in self.validators.as_ref() { let compat = validator.validate_recipe(state, recipe)?;