diff --git a/crates/spk-cli/common/Cargo.toml b/crates/spk-cli/common/Cargo.toml index 6d909c92c..1fa600a98 100644 --- a/crates/spk-cli/common/Cargo.toml +++ b/crates/spk-cli/common/Cargo.toml @@ -8,10 +8,7 @@ version = { workspace = true } workspace = true [features] -legacy-spk-version-tags = [ - "spk-schema/legacy-spk-version-tags", - "spk-storage/legacy-spk-version-tags", -] +legacy-spk-version-tags = ["spk-storage/legacy-spk-version-tags"] migration-to-components = [ "spk-build/migration-to-components", "spk-config/migration-to-components", diff --git a/crates/spk-cli/common/src/flags.rs b/crates/spk-cli/common/src/flags.rs index 0ea1ac96c..d748f5f84 100644 --- a/crates/spk-cli/common/src/flags.rs +++ b/crates/spk-cli/common/src/flags.rs @@ -841,6 +841,16 @@ pub struct Repositories { /// that attempts to make changes to a repository, even if the time is in the future. #[clap(long)] pub when: Option, + + /// Enable support for legacy spk version tags in the repository. + /// + /// This causes extra file I/O but is required if the repository contains + /// any packages that were published with non-normalized version tags. + /// + /// This is enabled by default if spk is built with the legacy-spk-version-tags + /// feature flag enabled. + #[clap(long, hide = true)] + pub legacy_spk_version_tags: bool, } impl Repositories { @@ -872,6 +882,9 @@ impl Repositories { if let Some(ts) = self.when.as_ref() { repo.pin_at_time(ts); } + if self.legacy_spk_version_tags { + repo.set_legacy_spk_version_tags(true); + } repos.push(("local".into(), repo.into())); } for (name, ts) in enabled.iter() { @@ -893,6 +906,9 @@ impl Repositories { if let Some(ts) = ts.as_ref().or(self.when.as_ref()) { repo.pin_at_time(ts); } + if self.legacy_spk_version_tags { + repo.set_legacy_spk_version_tags(true); + } repos.push((name.to_string(), repo.into())); } Ok(repos.into_iter().collect()) @@ -930,6 +946,9 @@ impl Repositories { if let Some(ts) = self.when.as_ref() { repo.pin_at_time(ts); } + if self.legacy_spk_version_tags { + repo.set_legacy_spk_version_tags(true); + } repos.push(("local".into(), repo.into())); } if self.local_repo_only { @@ -953,6 +972,9 @@ impl Repositories { if let Some(ts) = ts.as_ref().or(self.when.as_ref()) { repo.pin_at_time(ts); } + if self.legacy_spk_version_tags { + repo.set_legacy_spk_version_tags(true); + } repos.push((name.into(), repo.into())); } Ok(repos) diff --git a/crates/spk-cli/group2/src/cmd_ls_test.rs b/crates/spk-cli/group2/src/cmd_ls_test.rs index 9bd1cc3bf..bd550d339 100644 --- a/crates/spk-cli/group2/src/cmd_ls_test.rs +++ b/crates/spk-cli/group2/src/cmd_ls_test.rs @@ -3,12 +3,17 @@ // https://github.com/imageworks/spk use clap::Parser; +use futures::prelude::*; +use relative_path::RelativePathBuf; use spfs::config::Remote; +use spfs::storage::EntryType; use spfs::RemoteAddress; use spk_schema::foundation::ident_component::Component; +use spk_schema::ident_ops::VerbatimTagStrategy; use spk_schema::recipe; use spk_solve::spec; use spk_storage::fixtures::*; +use spk_storage::RepositoryHandle; use super::{Ls, Output, Run}; @@ -501,15 +506,8 @@ async fn test_ls_shows_partially_deprecated_version() { /// When the legacy-spk-version-tags feature is enabled, and when a package /// is published with a non-normalized version tag, `spk ls` is expected to /// list the package. -#[cfg(feature = "legacy-spk-version-tags")] #[tokio::test] async fn test_ls_succeeds_for_package_saved_with_legacy_version_tag() { - use futures::prelude::*; - use relative_path::RelativePathBuf; - use spfs::storage::EntryType; - use spk_schema::ident_ops::VerbatimTagStrategy; - use spk_storage::RepositoryHandle; - let mut rt = spfs_runtime_with_tag_strategy::().await; let remote_repo = spfsrepo().await; @@ -551,7 +549,7 @@ async fn test_ls_succeeds_for_package_saved_with_legacy_version_tag() { _ => panic!("expected SPFSWithVerbatimTags"), } - let mut opt = Opt::try_parse_from(["ls", "my-local-pkg"]).unwrap(); + let mut opt = Opt::try_parse_from(["ls", "--legacy-spk-version-tags", "my-local-pkg"]).unwrap(); opt.ls.run().await.unwrap(); assert_eq!(opt.ls.output.vec.len(), 1); } diff --git a/crates/spk-schema/Cargo.toml b/crates/spk-schema/Cargo.toml index f4854cb26..2c23d968d 100644 --- a/crates/spk-schema/Cargo.toml +++ b/crates/spk-schema/Cargo.toml @@ -8,7 +8,6 @@ version = { workspace = true } workspace = true [features] -legacy-spk-version-tags = ["spk-schema-foundation/legacy-spk-version-tags"] migration-to-components = ["spk-schema-foundation/migration-to-components"] [dependencies] diff --git a/crates/spk-schema/crates/foundation/Cargo.toml b/crates/spk-schema/crates/foundation/Cargo.toml index 0f741a88f..8342b0a11 100644 --- a/crates/spk-schema/crates/foundation/Cargo.toml +++ b/crates/spk-schema/crates/foundation/Cargo.toml @@ -12,7 +12,6 @@ default = ["parsedbuf-serde"] # activates serde within the generated code from parsedbuf macros parsedbuf-serde = [] migration-to-components = [] -legacy-spk-version-tags = [] [dependencies] arc-swap = { workspace = true } diff --git a/crates/spk-schema/crates/foundation/src/ident_ops/mod.rs b/crates/spk-schema/crates/foundation/src/ident_ops/mod.rs index f12ca49e4..37c25d90a 100644 --- a/crates/spk-schema/crates/foundation/src/ident_ops/mod.rs +++ b/crates/spk-schema/crates/foundation/src/ident_ops/mod.rs @@ -7,6 +7,10 @@ pub mod parsing; mod tag_path; pub use metadata_path::MetadataPath; -#[cfg(feature = "legacy-spk-version-tags")] -pub use tag_path::VerbatimTagStrategy; -pub use tag_path::{NormalizedTagStrategy, TagPath, TagPathStrategy, TagPathStrategyType}; +pub use tag_path::{ + NormalizedTagStrategy, + TagPath, + TagPathStrategy, + TagPathStrategyType, + VerbatimTagStrategy, +}; diff --git a/crates/spk-schema/crates/foundation/src/ident_ops/tag_path.rs b/crates/spk-schema/crates/foundation/src/ident_ops/tag_path.rs index a3b155c3b..c9754c2a9 100644 --- a/crates/spk-schema/crates/foundation/src/ident_ops/tag_path.rs +++ b/crates/spk-schema/crates/foundation/src/ident_ops/tag_path.rs @@ -10,7 +10,6 @@ pub enum TagPathStrategyType { /// Normalize the version in tag path. Normalized, /// Use the version as specified in the tag path. - #[cfg(feature = "legacy-spk-version-tags")] Verbatim, } @@ -34,11 +33,9 @@ impl TagPathStrategy for NormalizedTagStrategy { /// When creating a tag path that contains a version, this strategy will /// render the version as specified in the version object, without any /// normalization. -#[cfg(feature = "legacy-spk-version-tags")] #[derive(Debug)] pub struct VerbatimTagStrategy {} -#[cfg(feature = "legacy-spk-version-tags")] impl TagPathStrategy for VerbatimTagStrategy { #[inline] fn strategy_type() -> TagPathStrategyType { diff --git a/crates/spk-storage/src/fixtures.rs b/crates/spk-storage/src/fixtures.rs index 77cf4fed6..f10cda2b8 100644 --- a/crates/spk-storage/src/fixtures.rs +++ b/crates/spk-storage/src/fixtures.rs @@ -11,9 +11,12 @@ use spfs::config::Remote; use spfs::prelude::*; use spfs::Result; use spk_schema::foundation::fixtures::*; -#[cfg(feature = "legacy-spk-version-tags")] -use spk_schema::ident_ops::VerbatimTagStrategy; -use spk_schema::ident_ops::{NormalizedTagStrategy, TagPathStrategy, TagPathStrategyType}; +use spk_schema::ident_ops::{ + NormalizedTagStrategy, + TagPathStrategy, + TagPathStrategyType, + VerbatimTagStrategy, +}; use tokio::sync::{Mutex, MutexGuard}; use crate as storage; @@ -147,7 +150,6 @@ where ) .unwrap(), ), - #[cfg(feature = "legacy-spk-version-tags")] TagPathStrategyType::Verbatim => storage::RepositoryHandle::SPFSWithVerbatimTags( storage::SpfsRepository::::try_from( NameAndRepositoryWithTagStrategy::<_, _, VerbatimTagStrategy>::new( diff --git a/crates/spk-storage/src/storage/handle.rs b/crates/spk-storage/src/storage/handle.rs index 2f208b7b8..4925b29d1 100644 --- a/crates/spk-storage/src/storage/handle.rs +++ b/crates/spk-storage/src/storage/handle.rs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 // https://github.com/imageworks/spk -#[cfg(feature = "legacy-spk-version-tags")] use spk_schema::ident_ops::VerbatimTagStrategy; use spk_schema::{Spec, SpecRecipe}; @@ -14,7 +13,6 @@ type Handle = dyn Repository; #[allow(clippy::large_enum_variant)] pub enum RepositoryHandle { SPFS(super::SpfsRepository), - #[cfg(feature = "legacy-spk-version-tags")] SPFSWithVerbatimTags(super::SpfsRepository), Mem(super::MemRepository), Runtime(super::RuntimeRepository), @@ -34,16 +32,10 @@ impl RepositoryHandle { Self::Runtime(Default::default()) } - #[cfg(feature = "legacy-spk-version-tags")] pub fn is_spfs(&self) -> bool { matches!(self, Self::SPFS(_) | Self::SPFSWithVerbatimTags(_)) } - #[cfg(not(feature = "legacy-spk-version-tags"))] - pub fn is_spfs(&self) -> bool { - matches!(self, Self::SPFS(_)) - } - pub fn is_mem(&self) -> bool { matches!(self, Self::Mem(_)) } @@ -55,7 +47,6 @@ impl RepositoryHandle { pub fn to_repo(self) -> Box { match self { Self::SPFS(repo) => Box::new(repo), - #[cfg(feature = "legacy-spk-version-tags")] Self::SPFSWithVerbatimTags(repo) => Box::new(repo), Self::Mem(repo) => Box::new(repo), Self::Runtime(repo) => Box::new(repo), @@ -69,7 +60,6 @@ impl std::ops::Deref for RepositoryHandle { fn deref(&self) -> &Self::Target { match self { RepositoryHandle::SPFS(repo) => repo, - #[cfg(feature = "legacy-spk-version-tags")] RepositoryHandle::SPFSWithVerbatimTags(repo) => repo, RepositoryHandle::Mem(repo) => repo, RepositoryHandle::Runtime(repo) => repo, @@ -81,7 +71,6 @@ impl std::ops::DerefMut for RepositoryHandle { fn deref_mut(&mut self) -> &mut Self::Target { match self { RepositoryHandle::SPFS(repo) => repo, - #[cfg(feature = "legacy-spk-version-tags")] RepositoryHandle::SPFSWithVerbatimTags(repo) => repo, RepositoryHandle::Mem(repo) => repo, RepositoryHandle::Runtime(repo) => repo, diff --git a/crates/spk-storage/src/storage/spfs.rs b/crates/spk-storage/src/storage/spfs.rs index 349f248c8..36bc642b2 100644 --- a/crates/spk-storage/src/storage/spfs.rs +++ b/crates/spk-storage/src/storage/spfs.rs @@ -24,9 +24,7 @@ use spk_schema::foundation::version::{parse_version, Version}; use spk_schema::ident::{ToAnyWithoutBuild, VersionIdent}; use spk_schema::ident_build::parsing::embedded_source_package; use spk_schema::ident_build::{EmbeddedSource, EmbeddedSourcePackage}; -#[cfg(feature = "legacy-spk-version-tags")] -use spk_schema::ident_ops::VerbatimTagStrategy; -use spk_schema::ident_ops::{NormalizedTagStrategy, TagPath, TagPathStrategy}; +use spk_schema::ident_ops::{NormalizedTagStrategy, TagPath, TagPathStrategy, VerbatimTagStrategy}; use spk_schema::spec_ops::{HasVersion, WithVersion}; use spk_schema::version::VersionParts; use spk_schema::{AnyIdent, BuildIdent, FromYaml, Package, Recipe, Spec, SpecRecipe}; @@ -64,12 +62,9 @@ macro_rules! verbatim_build_package_tag_if_enabled { macro_rules! verbatim_tag_if_enabled { ($self:expr, $tag:tt, $output:ty, $ident:expr) => {{ - #[cfg(feature = "legacy-spk-version-tags")] - { + if $self.legacy_spk_version_tags { $self.$tag::($ident) - } - #[cfg(not(feature = "legacy-spk-version-tags"))] - { + } else { $self.$tag::($ident) } }}; @@ -83,6 +78,7 @@ pub struct SpfsRepository { cache_policy: AtomicPtr, caches: CachesForAddress, tag_strategy: PhantomData, + legacy_spk_version_tags: bool, } impl std::hash::Hash for SpfsRepository { @@ -144,6 +140,7 @@ impl, T: Into> TryFrom<(S, T)> fo inner, cache_policy: AtomicPtr::new(Box::leak(Box::new(CachePolicy::CacheOk))), tag_strategy: PhantomData, + legacy_spk_version_tags: cfg!(feature = "legacy-spk-version-tags"), }) } } @@ -195,6 +192,7 @@ where inner, cache_policy: AtomicPtr::new(Box::leak(Box::new(CachePolicy::CacheOk))), tag_strategy: PhantomData, + legacy_spk_version_tags: cfg!(feature = "legacy-spk-version-tags"), }) } } @@ -210,6 +208,7 @@ impl SpfsRepository { inner, cache_policy: AtomicPtr::new(Box::leak(Box::new(CachePolicy::CacheOk))), tag_strategy: PhantomData, + legacy_spk_version_tags: cfg!(feature = "legacy-spk-version-tags"), }) } @@ -229,6 +228,11 @@ impl SpfsRepository { .query_pairs_mut() .append_pair("when", &ts.to_string()); } + + /// Enable or disable the use of legacy spk version tags + pub fn set_legacy_spk_version_tags(&mut self, enabled: bool) { + self.legacy_spk_version_tags = enabled; + } } impl std::ops::Drop for SpfsRepository { @@ -345,7 +349,7 @@ where let mut builds = HashSet::new(); - for pkg in Self::iter_possible_parts(pkg) { + for pkg in Self::iter_possible_parts(pkg, self.legacy_spk_version_tags) { let spec_base = verbatim_build_spec_tag_if_enabled!(self, &pkg); let package_base = verbatim_build_package_tag_if_enabled!(self, &pkg); @@ -386,7 +390,7 @@ where let mut builds = HashSet::new(); let pkg = pkg.to_any(Some(Build::Source)); - for pkg in Self::iter_possible_parts(&pkg) { + for pkg in Self::iter_possible_parts(&pkg, self.legacy_spk_version_tags) { let mut base = verbatim_build_spec_tag_if_enabled!(self, &pkg); // the package tag contains the name and build, but we need to // remove the trailing build in order to list the containing 'folder' @@ -1001,7 +1005,10 @@ where /// /// If spk is built without the `legacy-spk-version-tags` feature enabled, /// then only the one canonical normalized part will be returned. - fn iter_possible_parts(pkg: &I) -> impl Iterator + '_ + fn iter_possible_parts( + pkg: &I, + legacy_spk_version_tags: bool, + ) -> impl Iterator + '_ where I: HasVersion + WithVersion, { @@ -1011,9 +1018,7 @@ where // Handle all the part lengths that are bigger than the normalized // parts, except for the normalized parts length itself, which may // be larger than 5 and not hit by this range. - .filter(move |num_parts| { - cfg!(feature = "legacy-spk-version-tags") && *num_parts > normalized_parts_len - }) + .filter(move |num_parts| legacy_spk_version_tags && *num_parts > normalized_parts_len) // Then, handle the normalized parts length itself, which is // skipped by the filter above so it isn't processed twice, // and is handled even if the length is outside the above range. @@ -1077,7 +1082,7 @@ where Fut: Future>, { let mut first_resolve_err = None; - for pkg in Self::iter_possible_parts(pkg) { + for pkg in Self::iter_possible_parts(pkg, self.legacy_spk_version_tags) { let tag_path = tag_path(&pkg); let tag_spec = spfs::tracking::TagSpec::parse(tag_path.as_str())?; let tag = match self @@ -1192,7 +1197,7 @@ where /// (with or without package components) async fn lookup_package(&self, pkg: &BuildIdent) -> Result { let mut first_resolve_err = None; - for pkg in Self::iter_possible_parts(pkg) { + for pkg in Self::iter_possible_parts(pkg, self.legacy_spk_version_tags) { let tag_path = verbatim_build_package_tag_if_enabled!(self, &pkg); let tag_specs: HashMap = self .ls_tags(&tag_path) @@ -1311,6 +1316,7 @@ pub async fn local_repository() -> Result { inner, cache_policy: AtomicPtr::new(Box::leak(Box::new(CachePolicy::CacheOk))), tag_strategy: PhantomData, + legacy_spk_version_tags: cfg!(feature = "legacy-spk-version-tags"), }) } @@ -1328,5 +1334,6 @@ pub async fn remote_repository>(name: S) -> Result inner, cache_policy: AtomicPtr::new(Box::leak(Box::new(CachePolicy::CacheOk))), tag_strategy: PhantomData, + legacy_spk_version_tags: cfg!(feature = "legacy-spk-version-tags"), }) } diff --git a/crates/spk/Cargo.toml b/crates/spk/Cargo.toml index 527635329..b99a79b6b 100644 --- a/crates/spk/Cargo.toml +++ b/crates/spk/Cargo.toml @@ -15,10 +15,7 @@ required-features = ["cli"] [features] default = ["cli"] -legacy-spk-version-tags = [ - "spk-schema/legacy-spk-version-tags", - "spk-storage/legacy-spk-version-tags", -] +legacy-spk-version-tags = ["spk-storage/legacy-spk-version-tags"] migration-to-components = [ "spk-schema/migration-to-components", "spk-solve/migration-to-components",