From 345ce0a51dc4bdb476913c88978000b1c8b3e689 Mon Sep 17 00:00:00 2001 From: Ryan Bottriell Date: Wed, 18 Dec 2024 12:23:38 -0800 Subject: [PATCH] Search partial versions in workspace This allows a better user experience by only requiring enough of a version number to disambiguate spec files rather than an exact version to build. Signed-off-by: Ryan Bottriell --- crates/spk-cli/common/src/flags.rs | 2 +- .../foundation/src/version_range/mod.rs | 10 ++++- crates/spk-schema/src/version_range_test.rs | 45 +++++++++---------- crates/spk-workspace/src/workspace.rs | 29 ++++++++---- crates/spk-workspace/src/workspace_test.rs | 4 +- 5 files changed, 53 insertions(+), 37 deletions(-) diff --git a/crates/spk-cli/common/src/flags.rs b/crates/spk-cli/common/src/flags.rs index eee130f753..648d036d42 100644 --- a/crates/spk-cli/common/src/flags.rs +++ b/crates/spk-cli/common/src/flags.rs @@ -379,7 +379,7 @@ impl Requests { let path = std::path::Path::new(package); if path.is_file() { let workspace = self.workspace.load_or_default()?; - let configured = workspace.find_package_template(&package).must_be_found(); + let configured = workspace.find_package_template(package).must_be_found(); let rendered_data = configured.template.render(options)?; let recipe = rendered_data.into_recipe().wrap_err_with(|| { format!( diff --git a/crates/spk-schema/crates/foundation/src/version_range/mod.rs b/crates/spk-schema/crates/foundation/src/version_range/mod.rs index 83b36d8d02..e4bd6acdfb 100644 --- a/crates/spk-schema/crates/foundation/src/version_range/mod.rs +++ b/crates/spk-schema/crates/foundation/src/version_range/mod.rs @@ -576,8 +576,14 @@ pub struct LowestSpecifiedRange { impl LowestSpecifiedRange { pub const REQUIRED_NUMBER_OF_DIGITS: usize = 2; - pub fn new(specified: usize, base: Version) -> Self { - Self { specified, base } + pub fn new(mut base: Version) -> Self { + while base.parts.len() < Self::REQUIRED_NUMBER_OF_DIGITS { + base.parts.push(0); + } + Self { + specified: base.parts.len(), + base, + } } } diff --git a/crates/spk-schema/src/version_range_test.rs b/crates/spk-schema/src/version_range_test.rs index e291318569..d373f1a266 100644 --- a/crates/spk-schema/src/version_range_test.rs +++ b/crates/spk-schema/src/version_range_test.rs @@ -263,30 +263,27 @@ fn arb_lowest_specified_range_from_version( 0..=(*version.parts.get(parts_to_generate - 1).unwrap()), ) .prop_map(|(version, parts_to_generate, last_element_value)| { - VersionRange::LowestSpecified(LowestSpecifiedRange::new( - parts_to_generate, - Version { - parts: version - .parts - .iter() - .take(parts_to_generate) - .enumerate() - .map(|(index, num)| { - if index == parts_to_generate - 1 { - last_element_value - } else { - *num - } - }) - .collect::>() - .into(), - // Retain pre and post from original version because - // if the original has pre it might be smaller than - // the smallest value we generated without it. - pre: version.pre, - post: version.post, - }, - )) + VersionRange::LowestSpecified(LowestSpecifiedRange::new(Version { + parts: version + .parts + .iter() + .take(parts_to_generate) + .enumerate() + .map(|(index, num)| { + if index == parts_to_generate - 1 { + last_element_value + } else { + *num + } + }) + .collect::>() + .into(), + // Retain pre and post from original version because + // if the original has pre it might be smaller than + // the smallest value we generated without it. + pre: version.pre, + post: version.post, + })) }) }, ) diff --git a/crates/spk-workspace/src/workspace.rs b/crates/spk-workspace/src/workspace.rs index 51c6667164..e342617af1 100644 --- a/crates/spk-workspace/src/workspace.rs +++ b/crates/spk-workspace/src/workspace.rs @@ -6,7 +6,7 @@ use std::collections::HashMap; use std::str::FromStr; use spk_schema::name::{PkgName, PkgNameBuf}; -use spk_schema::version::Version; +use spk_schema::version_range::{LowestSpecifiedRange, Ranged}; use spk_schema::{SpecTemplate, Template, TemplateExt}; use crate::error; @@ -83,8 +83,15 @@ impl Workspace { tracing::debug!("Find package template by name: {name}"); self.find_package_templates(name) } else if let Ok(ident) = spk_schema::VersionIdent::from_str(package) { - tracing::debug!("Find package template for version: {ident}"); - self.find_package_template_for_version(ident.name(), ident.version()) + // The lowest specified range is preferred when valid, because it + // allows for the user to specify something like `python/3` to disambiguate + // between major versions without needing to select an exact/complete version. + let range = LowestSpecifiedRange::new(ident.version().clone()); + tracing::debug!( + "Find package template for version: {}/{range}", + ident.name() + ); + self.find_package_template_for_version(ident.name(), range) } else { tracing::debug!("Find package template by path: {package}"); self.find_package_template_by_file(std::path::Path::new(package)) @@ -96,18 +103,24 @@ impl Workspace { if found.len() > 1 { return FindPackageTemplateResult::MultipleTemplateFiles(found); } - FindPackageTemplateResult::Found(&found[0]) + FindPackageTemplateResult::Found(found[0]) } /// Like [`Self::find_package_template`], but further filters by package version. - pub fn find_package_template_for_version( + pub fn find_package_template_for_version( &self, package: &PkgName, - version: &Version, + range: R, ) -> Vec<&ConfiguredTemplate> { self.find_package_templates(package) .into_iter() - .filter(|t| t.config.versions.is_empty() || t.config.versions.contains(version)) + .filter(|t| { + t.config.versions.is_empty() + || t.config + .versions + .iter() + .any(|v| range.is_applicable(v).is_ok()) + }) .collect::>() } @@ -228,7 +241,7 @@ impl<'a> FindPackageTemplateResult<'a> { .collect::>() .join(", "); if versions.is_empty() { - versions.push_str("?"); + versions.push_str(""); } tracing::error!(" - {path} versions=[{versions}]",); } diff --git a/crates/spk-workspace/src/workspace_test.rs b/crates/spk-workspace/src/workspace_test.rs index 60c15f38d8..241adf5e20 100644 --- a/crates/spk-workspace/src/workspace_test.rs +++ b/crates/spk-workspace/src/workspace_test.rs @@ -133,7 +133,7 @@ fn test_workspace_find_template( let result = if request.is_empty() { workspace.default_package_template() } else { - workspace.find_package_template(&request) + workspace.find_package_template(request) }; let super::FindPackageTemplateResult::Found(result) = result else { @@ -200,7 +200,7 @@ fn test_workspace_find_by_version(tmpdir: tempfile::TempDir) { panic!("should fail to find template when there was no version given and multiple exist in the workspace, got {res:#?}"); }; - let res = workspace.find_package_template("my-package/1.0.0"); + let res = workspace.find_package_template("my-package/1"); let super::FindPackageTemplateResult::Found(found) = res else { panic!("should find template when multiple exist but an unambiguous version is given, got {res:#?}"); };