Skip to content

Commit

Permalink
Rename AsVersion to AsVersionIdent
Browse files Browse the repository at this point in the history
Since there was also an existing `as_version` method on the Ident types,
change these to use the trait instead. If `as_version` could be confused
for something that returns a Version, then by extension the other
methods on Ident should be similarly renamed. This also keeps the naming
convention consistent.

Signed-off-by: J Robert Ray <jrray@jrray.org>
  • Loading branch information
jrray committed Sep 30, 2024
1 parent d56e61e commit cc404c3
Show file tree
Hide file tree
Showing 47 changed files with 259 additions and 201 deletions.
2 changes: 1 addition & 1 deletion crates/spk-build/src/archive_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ async fn test_archive_create_parents() {
panic!("only spfs repositories are supported")
}
};
export_package::<NormalizedTagStrategy>(&[repo], &spec.ident().to_any(), filename)
export_package::<NormalizedTagStrategy>(&[repo], &spec.ident().to_any_ident(), filename)
.await
.expect("export should create dirs as needed");
}
7 changes: 4 additions & 3 deletions crates/spk-build/src/build/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ where
{
/// Create a new builder that builds a binary package from the given recipe
pub fn from_recipe(recipe: Recipe) -> Self {
let source = BuildSource::SourcePackage(recipe.ident().to_build(Build::Source).into());
let source =
BuildSource::SourcePackage(recipe.ident().to_build_ident(Build::Source).into());
Self {
recipe,
source,
Expand Down Expand Up @@ -345,7 +346,7 @@ where

let environment_filesystem = resolved_layers
.get_environment_filesystem(
self.recipe.ident().to_build(Build::Source),
self.recipe.ident().to_build_ident(Build::Source),
&mut self.conflicting_packages,
)
.await?;
Expand Down Expand Up @@ -512,7 +513,7 @@ where

let source_ident =
VersionIdent::new(self.recipe.name().to_owned(), self.recipe.version().clone())
.into_any(Some(Build::Source));
.into_any_ident(Some(Build::Source));
let sources_dir = data_path(&source_ident);

let active_changes = spfs::runtime_active_changes()
Expand Down
4 changes: 2 additions & 2 deletions crates/spk-build/src/validation/inherit_requirements_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ async fn test_build_package_downstream_build_requests() {

let mut environment = Solution::default();
environment.add(
PkgRequest::from_ident(base_spec.ident().to_any(), RequestedBy::DoesNotMatter),
PkgRequest::from_ident(base_spec.ident().to_any_ident(), RequestedBy::DoesNotMatter),
base_spec.clone(),
spk_solve::PackageSource::SpkInternalTest,
);
Expand Down Expand Up @@ -107,7 +107,7 @@ async fn test_build_package_downstream_runtime_request() {

let mut environment = Solution::default();
environment.add(
PkgRequest::from_ident(base_spec.ident().to_any(), RequestedBy::DoesNotMatter),
PkgRequest::from_ident(base_spec.ident().to_any_ident(), RequestedBy::DoesNotMatter),
base_spec.clone(),
spk_solve::PackageSource::SpkInternalTest,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ async fn test_for_description_over_limit() {

let mut environment = Solution::default();
environment.add(
PkgRequest::from_ident(package.ident().to_any(), RequestedBy::DoesNotMatter),
PkgRequest::from_ident(package.ident().to_any_ident(), RequestedBy::DoesNotMatter),
package.clone(),
spk_solve::PackageSource::SpkInternalTest,
);
Expand Down
2 changes: 1 addition & 1 deletion crates/spk-build/src/validation/recursive_build_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ async fn test_build_with_circular_dependency() {

let mut environment = Solution::default();
environment.add(
PkgRequest::from_ident(old_build.ident().to_any(), RequestedBy::DoesNotMatter),
PkgRequest::from_ident(old_build.ident().to_any_ident(), RequestedBy::DoesNotMatter),
old_build.clone(),
spk_solve::PackageSource::SpkInternalTest,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ async fn test_strongly_inherited_vars_require_desc() {

let mut environment = Solution::default();
environment.add(
PkgRequest::from_ident(package.ident().to_any(), RequestedBy::DoesNotMatter),
PkgRequest::from_ident(package.ident().to_any_ident(), RequestedBy::DoesNotMatter),
package.clone(),
spk_solve::PackageSource::SpkInternalTest,
);
Expand Down
4 changes: 2 additions & 2 deletions crates/spk-cli/cmd-du/src/cmd_du.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use spfs::prelude::*;
use spfs::tracking::Entry;
use spfs::Digest;
use spk_cli_common::{flags, CommandArgs, Run};
use spk_schema::ident::parse_ident;
use spk_schema::ident::{parse_ident, AsVersionIdent};
use spk_schema::ident_build::Build;
use spk_schema::ident_component::Component;
use spk_schema::name::{PkgName, PkgNameBuf};
Expand Down Expand Up @@ -444,7 +444,7 @@ impl<T: Output> Du<T> {
let repos = self.repos.get_repos_for_non_destructive_operation().await?;
let (_, repo) = repos.get(repo_index).unwrap();

match repo.list_package_builds(pkg_ident.as_version()).await {
match repo.list_package_builds(pkg_ident.as_version_ident()).await {
Ok(builds) => {
for build in builds.iter().sorted_by_key(|k| *k) {
if build.is_embedded() {
Expand Down
6 changes: 4 additions & 2 deletions crates/spk-cli/cmd-make-binary/src/cmd_make_binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,10 @@ impl Run for MakeBinary {
);

if self.env {
let request =
PkgRequest::from_ident(out.ident().to_any(), RequestedBy::CommandLine);
let request = PkgRequest::from_ident(
out.ident().to_any_ident(),
RequestedBy::CommandLine,
);
let mut cmd = std::process::Command::new(spk_exe());
cmd.args(["env", "--enable-repo", "local"])
.arg(request.pkg.to_string());
Expand Down
2 changes: 1 addition & 1 deletion crates/spk-cli/cmd-test/src/cmd_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ impl Run for CmdTest {
BuildSource::SourcePackage(
recipe
.ident()
.to_any(Some(Build::Source))
.to_any_ident(Some(Build::Source))
.into(),
)
},
Expand Down
3 changes: 2 additions & 1 deletion crates/spk-cli/cmd-test/src/test/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ pub struct PackageBuildTester<'a> {

impl<'a> PackageBuildTester<'a> {
pub fn new(recipe: SpecRecipe, script: String) -> Self {
let source = BuildSource::SourcePackage(recipe.ident().to_any(Some(Build::Source)).into());
let source =
BuildSource::SourcePackage(recipe.ident().to_any_ident(Some(Build::Source)).into());
Self {
prefix: PathBuf::from("/spfs"),
recipe,
Expand Down
2 changes: 1 addition & 1 deletion crates/spk-cli/cmd-test/src/test/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ where
let build_to_test = self
.recipe
.ident()
.to_any(None)
.to_any_ident(None)
.with_build(Some(Build::BuildId(build_digest_for_variant)));

let pkg = RangeIdent::double_equals(&build_to_test, [Component::All]);
Expand Down
4 changes: 2 additions & 2 deletions crates/spk-cli/cmd-test/src/test/sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl<'a> PackageSourceTester<'a> {
if self.source.is_none() {
// we only require the source package to actually exist
// if a local directory has not been specified for the test
let source_pkg = self.recipe.ident().to_any(Some(Build::Source));
let source_pkg = self.recipe.ident().to_any_ident(Some(Build::Source));
let mut ident_range = RangeIdent::equals(&source_pkg, [Component::Source]);
ident_range.components.insert(Component::Source);
let request = PkgRequest::new(ident_range, RequestedBy::SourceTest(source_pkg))
Expand All @@ -128,7 +128,7 @@ impl<'a> PackageSourceTester<'a> {

let source_dir = match &self.source {
Some(source) => source.clone(),
None => source_package_path(&self.recipe.ident().to_build(Build::Source))
None => source_package_path(&self.recipe.ident().to_build_ident(Build::Source))
.to_path(&self.prefix),
};

Expand Down
3 changes: 2 additions & 1 deletion crates/spk-cli/common/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ pub async fn current_env() -> crate::Result<Solution> {
.into_iter()
.zip(components_in_runtime.into_iter())
{
let range_ident = RangeIdent::equals(&spec.ident().to_any(), components.keys().cloned());
let range_ident =
RangeIdent::equals(&spec.ident().to_any_ident(), components.keys().cloned());

let package_solve_data = if let Some(data) = solve_data.get(spec.ident()) {
data
Expand Down
20 changes: 14 additions & 6 deletions crates/spk-cli/common/src/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@ use spk_schema::foundation::name::OptName;
use spk_schema::foundation::option_map::OptionMap;
use spk_schema::foundation::spec_ops::Named;
use spk_schema::foundation::version::CompatRule;
use spk_schema::ident::{parse_ident, AnyIdent, PkgRequest, Request, RequestedBy, VarRequest};
use spk_schema::ident::{
parse_ident,
AnyIdent,
AsVersionIdent,
PkgRequest,
Request,
RequestedBy,
VarRequest,
};
use spk_schema::option_map::HOST_OPTIONS;
use spk_schema::{Recipe, SpecRecipe, SpecTemplate, Template, TemplateExt, TestStage, VariantExt};
#[cfg(feature = "statsd")]
Expand Down Expand Up @@ -366,7 +374,7 @@ impl Requests {

match stage {
TestStage::Sources => {
let ident = recipe.ident().to_any(Some(Build::Source));
let ident = recipe.ident().to_any_ident(Some(Build::Source));
idents.push(ident);
continue;
}
Expand All @@ -382,7 +390,7 @@ impl Requests {
if path.is_file() {
let (_, template) = find_package_template(Some(&package))?.must_be_found();
let recipe = template.render(options)?;
idents.push(recipe.ident().to_any(None));
idents.push(recipe.ident().to_any_ident(None));
} else {
idents.push(parse_ident(package)?)
}
Expand Down Expand Up @@ -431,7 +439,7 @@ impl Requests {
bail!("Source stage does not accept a build variant specifier")
}

let ident = recipe.ident().to_any(Some(Build::Source));
let ident = recipe.ident().to_any_ident(Some(Build::Source));
out.push(
PkgRequest::from_ident_exact(ident, RequestedBy::CommandLine).into(),
);
Expand Down Expand Up @@ -467,7 +475,7 @@ impl Requests {

out.push(
PkgRequest::from_ident_exact(
recipe.ident().to_any(None),
recipe.ident().to_any_ident(None),
RequestedBy::CommandLine,
)
.into(),
Expand Down Expand Up @@ -779,7 +787,7 @@ where
);

for repo in repos.iter() {
match repo.read_recipe(pkg.as_version()).await {
match repo.read_recipe(pkg.as_version_ident()).await {
Ok(recipe) => {
tracing::debug!(
"Using recipe found for {}",
Expand Down
5 changes: 3 additions & 2 deletions crates/spk-cli/common/src/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::sync::Arc;

use spk_schema::foundation::format::{FormatComponents, FormatIdent};
use spk_schema::foundation::ident_component::ComponentSet;
use spk_schema::ident::AsVersionIdent;
use spk_schema::{AnyIdent, BuildIdent, Package, Recipe, VersionIdent};
use spk_storage as storage;
use storage::{with_cache_policy, CachePolicy};
Expand Down Expand Up @@ -129,7 +130,7 @@ impl Publisher {
I: AsRef<AnyIdent>,
{
let pkg = pkg.as_ref();
let recipe_ident = pkg.as_version();
let recipe_ident = pkg.as_version_ident();
tracing::info!("loading recipe: {}", recipe_ident.format_ident());
match with_cache_policy!(self.from, CachePolicy::BypassCache, {
self.from.read_recipe(recipe_ident).await
Expand Down Expand Up @@ -192,7 +193,7 @@ impl Publisher {
})
.await?
}
Some(build) => vec![pkg.to_build(build.clone())],
Some(build) => vec![pkg.to_build_ident(build.clone())],
};

for build in builds.iter() {
Expand Down
19 changes: 14 additions & 5 deletions crates/spk-cli/common/src/publish_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use rstest::rstest;
use spk_schema::foundation::ident_component::Component;
use spk_schema::ident::AsVersionIdent;
use spk_schema::Package;
use spk_solve::{recipe, spec};
use spk_storage::fixtures::*;
Expand All @@ -30,7 +31,7 @@ async fn test_publish_no_version_spec() {
let destination = spfsrepo().await;
let publisher = Publisher::new(rt.tmprepo.clone(), destination.repo.clone());
publisher
.publish(spec.ident().base().to_any(None))
.publish(spec.ident().base().to_any_ident(None))
.await
.unwrap();
destination.read_components(spec.ident()).await.unwrap();
Expand Down Expand Up @@ -58,8 +59,13 @@ async fn test_publish_build_also_publishes_spec() {
let destination = spfsrepo().await;
let publisher = Publisher::new(rt.tmprepo.clone(), destination.repo.clone());
// Include build when publishing this spec.
publisher.publish(spec.ident().to_any()).await.unwrap();
let r = destination.read_recipe(spec.ident().as_version()).await;
publisher
.publish(spec.ident().to_any_ident())
.await
.unwrap();
let r = destination
.read_recipe(spec.ident().as_version_ident())
.await;
assert!(
r.is_ok(),
"Expected to be able to read spec, but got error: {}",
Expand Down Expand Up @@ -92,7 +98,10 @@ async fn test_publish_multiple_builds_individually() {
.unwrap();

// Include build when publishing this spec.
publisher.publish(spec.ident().to_any()).await.unwrap();
publisher
.publish(spec.ident().to_any_ident())
.await
.unwrap();
}

{
Expand All @@ -109,7 +118,7 @@ async fn test_publish_multiple_builds_individually() {
.unwrap();

// Include build when publishing this spec.
let r = publisher.publish(spec.ident().to_any()).await;
let r = publisher.publish(spec.ident().to_any_ident()).await;
assert!(
r.is_ok(),
"Expected second publish to succeed, but got error: {}",
Expand Down
2 changes: 1 addition & 1 deletion crates/spk-cli/common/src/with_version_and_build_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl WithVersionAndBuildSet for PkgRequest {
// This grabs a build for the version, from the first repo
// that has a build for the version in it.
let temp_ident: AnyIdent = new_request.pkg.clone().try_into()?;
let ident: VersionIdent = temp_ident.to_version();
let ident: VersionIdent = temp_ident.to_version_ident();

for repo in repos {
match select_build_by {
Expand Down
6 changes: 3 additions & 3 deletions crates/spk-cli/group1/src/cmd_deprecate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ pub(crate) async fn change_deprecation_state(
}
}
(ident, Some(build)) => {
let ident = ident.to_build(build);
let ident = ident.to_build_ident(build);
match repo.read_package(&ident).await {
Ok(package) => {
to_action.push((
Expand Down Expand Up @@ -373,8 +373,8 @@ impl DeprecateMut for DeprecationTarget {
impl DeprecationTarget {
fn ident(&self) -> AnyIdent {
match self {
DeprecationTarget::Recipe(r) => r.ident().to_any(None),
DeprecationTarget::Package(r) => r.ident().to_any(),
DeprecationTarget::Recipe(r) => r.ident().to_any_ident(None),
DeprecationTarget::Package(r) => r.ident().to_any_ident(),
}
}
}
10 changes: 5 additions & 5 deletions crates/spk-cli/group2/src/cmd_ls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use spk_cli_common::{flags, CommandArgs, Run};
use spk_schema::foundation::format::{FormatComponents, FormatIdent, FormatOptionMap};
use spk_schema::foundation::ident_component::ComponentSet;
use spk_schema::foundation::name::{PkgName, PkgNameBuf};
use spk_schema::ident::{parse_ident, AnyIdent};
use spk_schema::ident::{parse_ident, AnyIdent, AsVersionIdent};
use spk_schema::ident_ops::parsing::{ident_parts, IdentParts, KNOWN_REPOSITORY_NAMES};
use spk_schema::option_map::{get_host_options_filters, OptFilter};
use spk_schema::spec_ops::WithVersion;
Expand Down Expand Up @@ -173,7 +173,7 @@ impl<T: Output> Run for Ls<T> {
// inventory the builds of this version (do not depend on
// the existence of a "version spec").

let mut builds = repo.list_package_builds(ident.as_version()).await?;
let mut builds = repo.list_package_builds(ident.as_version_ident()).await?;
if builds.is_empty() {
// Does a version with no builds really exist?
continue;
Expand Down Expand Up @@ -243,7 +243,7 @@ impl<T: Output> Run for Ls<T> {
// Given a package version (or build), list all its builds
let pkg = parse_ident(package)?;
for (_, repo) in repos {
for build in repo.list_package_builds(pkg.as_version()).await? {
for build in repo.list_package_builds(pkg.as_version_ident()).await? {
// Doing this here slows the listing down, but
// the spec file is the only place that holds
// the deprecation status.
Expand Down Expand Up @@ -351,7 +351,7 @@ impl<T: Output> Ls<T> {
versions.sort();
versions.reverse();
for pkg in versions {
let mut builds = repo.list_package_builds(pkg.as_version()).await?;
let mut builds = repo.list_package_builds(pkg.as_version_ident()).await?;
builds.sort();
for build in builds {
if let Some(IdentParts {
Expand Down Expand Up @@ -431,7 +431,7 @@ impl<T: Output> Ls<T> {

for pkg in versions {
let mut found_a_match = false;
for build in repo.list_package_builds(pkg.as_version()).await? {
for build in repo.list_package_builds(pkg.as_version_ident()).await? {
let spec = match repo.read_package(&build).await {
Ok(spec) => spec,
Err(err) => {
Expand Down
2 changes: 1 addition & 1 deletion crates/spk-cli/group2/src/cmd_remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl Run for Remove {
remove_all(repo_name, repo, &version).await?;
}
(version, Some(build)) => {
remove_build(repo_name, repo, &version.into_build(build)).await?;
remove_build(repo_name, repo, &version.into_build_ident(build)).await?;
}
}
}
Expand Down
Loading

0 comments on commit cc404c3

Please sign in to comment.