Skip to content

Commit

Permalink
Search partial versions in workspace
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
rydrman committed Dec 18, 2024
1 parent c7437d0 commit 345ce0a
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 37 deletions.
2 changes: 1 addition & 1 deletion crates/spk-cli/common/src/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
10 changes: 8 additions & 2 deletions crates/spk-schema/crates/foundation/src/version_range/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
}

Expand Down
45 changes: 21 additions & 24 deletions crates/spk-schema/src/version_range_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<_>>()
.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::<Vec<_>>()
.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,
}))
})
},
)
Expand Down
29 changes: 21 additions & 8 deletions crates/spk-workspace/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand All @@ -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<R: Ranged>(
&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::<Vec<_>>()
}

Expand Down Expand Up @@ -228,7 +241,7 @@ impl<'a> FindPackageTemplateResult<'a> {
.collect::<Vec<_>>()
.join(", ");
if versions.is_empty() {
versions.push_str("?");
versions.push_str("<any>");
}
tracing::error!(" - {path} versions=[{versions}]",);
}
Expand Down
4 changes: 2 additions & 2 deletions crates/spk-workspace/src/workspace_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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:#?}");
};
Expand Down

0 comments on commit 345ce0a

Please sign in to comment.