From bd31c3f29faa631f328944458b6a0869d1545442 Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Thu, 13 Nov 2025 17:07:25 -0800 Subject: [PATCH 1/2] Stop skipping SpecFile items that are not live layers The SpecFile::EnvLayersFile flavor is likely not handled correctly. Signed-off-by: J Robert Ray --- crates/spfs/src/tracking/env.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/spfs/src/tracking/env.rs b/crates/spfs/src/tracking/env.rs index dd6e74c3c..0275b4147 100644 --- a/crates/spfs/src/tracking/env.rs +++ b/crates/spfs/src/tracking/env.rs @@ -273,6 +273,7 @@ impl EnvSpecItem { } /// Returns true if this item is a live layer file + #[inline] pub fn is_livelayer(&self) -> bool { matches!(self, Self::SpecFile(SpecFile::LiveLayer(_))) } @@ -416,7 +417,7 @@ impl EnvSpec { let mut new_items: Vec = Vec::with_capacity(self.items.len()); for item in &self.items { // Filter out the LiveLayers entirely because they do not have digests - if let EnvSpecItem::SpecFile(_) = item { + if item.is_livelayer() { continue; } new_items.push(self.resolve_tag_item_to_digest_item(item, repos).await?); From 47a38863f91144f54b4885043a8788e8b13ded9f Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Thu, 13 Nov 2025 16:40:59 -0800 Subject: [PATCH 2/2] Create a new `RefSpec` alternative to `EnvSpec` The `EnvSpec` type has become overloaded and used is contexts where it can represent things that don't make sense. For example, `"-"` is a legal (empty) `EnvSpec` which can be used to create an empty environment, but it doesn't make sense to `spfs push` an empty environment. A `RefSpec` is a similar type but instead of representing items that can be used to create an environment, it represents items that can be copied between repositories. The `EnvSpec` scope has narrowed to restrict that any digests within it are treated as object digests (not payloads), because it is non-sensical to create an environment from a payload digest. The `RefSpec` allows digests to be payload digests, but does not support live layer files. It is also required to be non-empty, since it doesn't make sense to copy "nothing" between repositories. Spec files are still supported although the files themselves cannot be copied, however it should be possible to provide the list of refs to sync to `spfs push` via a spec file. # Design Notes There are still several places that accept an `EnvSpec` that is intended to be used to create an environment, but it is first used to sync items locally, so it needs to be converted to a `RefSpec` first (and the sync result converted back to an `EnvSpec`). These conversions are either lossy or fallible, so there are things likely broken in this PR that just aren't covered by tests yet. In this design, it makes sense for the conversion from `EnvSpec` to `RefSpec` to be lossy (instead of fallible) in the context of syncing content locally. These changes were created to build on the ideas in PR #1289 and further leverage `PartialDigestType` introduced there. Signed-off-by: J Robert Ray --- crates/spfs-cli/cmd-render/src/cmd_render.rs | 5 +- crates/spfs-cli/main/src/cmd_pull.rs | 7 +- crates/spfs-cli/main/src/cmd_push.rs | 7 +- crates/spfs-cli/main/src/cmd_reset.rs | 5 +- crates/spfs-cli/main/src/cmd_run.rs | 6 +- crates/spfs/src/sync.rs | 51 ++- crates/spfs/src/sync/reporter.rs | 78 ++-- crates/spfs/src/sync_test.rs | 4 +- crates/spfs/src/tracking/env.rs | 48 ++- crates/spfs/src/tracking/mod.rs | 2 + crates/spfs/src/tracking/ref_spec.rs | 418 +++++++++++++++++++ crates/spfs/src/tracking/ref_spec_test.rs | 80 ++++ crates/spk-cli/common/src/publish.rs | 5 +- crates/spk-cli/group3/src/cmd_import.rs | 16 +- crates/spk-launcher/src/main.rs | 13 +- crates/spk-storage/src/storage/archive.rs | 5 +- 16 files changed, 650 insertions(+), 100 deletions(-) create mode 100644 crates/spfs/src/tracking/ref_spec.rs create mode 100644 crates/spfs/src/tracking/ref_spec_test.rs diff --git a/crates/spfs-cli/cmd-render/src/cmd_render.rs b/crates/spfs-cli/cmd-render/src/cmd_render.rs index c7508623d..7859204ec 100644 --- a/crates/spfs-cli/cmd-render/src/cmd_render.rs +++ b/crates/spfs-cli/cmd-render/src/cmd_render.rs @@ -64,9 +64,10 @@ impl CmdRender { env_spec = self .sync .get_syncer(&origin, &handle) - .sync_env(env_spec) + .sync_ref_spec(env_spec.try_into()?) .await? - .env; + .ref_spec + .into(); } // Use PayloadFallback to repair any missing payloads found in the diff --git a/crates/spfs-cli/main/src/cmd_pull.rs b/crates/spfs-cli/main/src/cmd_pull.rs index d8214f381..2918c48d6 100644 --- a/crates/spfs-cli/main/src/cmd_pull.rs +++ b/crates/spfs-cli/main/src/cmd_pull.rs @@ -5,6 +5,7 @@ use clap::Args; use miette::Result; use spfs::sync::reporter::Summary; +use spfs::tracking::RefSpec; use spfs_cli_common as cli; /// Pull one or more objects to the local repository @@ -27,7 +28,7 @@ pub struct CmdPull { /// These can be individual tags or digests, or they may also /// be a collection of items joined by a '+' #[clap(value_name = "REF", required = true)] - refs: Vec, + refs: Vec, } impl CmdPull { @@ -37,11 +38,11 @@ impl CmdPull { spfs::config::open_repository_from_string(config, self.repos.remote.as_ref()) )?; - let env_spec = self.refs.iter().cloned().collect(); + let ref_spec = RefSpec::combine(&self.refs)?; let summary = self .sync .get_syncer(&remote, &repo) - .sync_env(env_spec) + .sync_ref_spec(ref_spec) .await? .summary(); diff --git a/crates/spfs-cli/main/src/cmd_push.rs b/crates/spfs-cli/main/src/cmd_push.rs index 74d2419fb..41793b6dd 100644 --- a/crates/spfs-cli/main/src/cmd_push.rs +++ b/crates/spfs-cli/main/src/cmd_push.rs @@ -5,6 +5,7 @@ use clap::Args; use miette::Result; use spfs::sync::reporter::Summary; +use spfs::tracking::RefSpec; use spfs_cli_common as cli; /// Push one or more objects to a remote repository @@ -24,7 +25,7 @@ pub struct CmdPush { /// These can be individual tags or digests, or they may also /// be a collection of items joined by a '+' #[clap(value_name = "REF", required = true)] - refs: Vec, + refs: Vec, } impl CmdPush { @@ -40,13 +41,13 @@ impl CmdPush { spfs::config::open_repository_from_string(config, self.repos.remote.as_ref()), )?; - let env_spec = self.refs.iter().cloned().collect(); + let ref_spec = RefSpec::combine(&self.refs)?; // the latest tag is always synced when pushing self.sync.sync = true; let summary = self .sync .get_syncer(&repo, &remote) - .sync_env(env_spec) + .sync_ref_spec(ref_spec) .await? .summary(); tracing::info!("{}", spfs::io::format_sync_summary(&summary)); diff --git a/crates/spfs-cli/main/src/cmd_reset.rs b/crates/spfs-cli/main/src/cmd_reset.rs index 2790b208c..dd9369d71 100644 --- a/crates/spfs-cli/main/src/cmd_reset.rs +++ b/crates/spfs-cli/main/src/cmd_reset.rs @@ -47,9 +47,10 @@ impl CmdReset { env_spec = self .sync .get_syncer(&origin, &repo) - .sync_env(env_spec) + .sync_ref_spec(env_spec.try_into()?) .await? - .env; + .ref_spec + .into(); } for item in env_spec.iter() { let digest = item.resolve_digest(&repo).await?; diff --git a/crates/spfs-cli/main/src/cmd_run.rs b/crates/spfs-cli/main/src/cmd_run.rs index 87ad695c2..37671d86b 100644 --- a/crates/spfs-cli/main/src/cmd_run.rs +++ b/crates/spfs-cli/main/src/cmd_run.rs @@ -212,7 +212,7 @@ impl CmdRun { let _synced = self .sync .get_syncer(&origin, &repo) - .sync_env(references_to_sync) + .sync_ref_spec(references_to_sync.try_into()?) .await?; } tracing::debug!("synced and about to launch process with durable runtime"); @@ -300,9 +300,9 @@ impl CmdRun { let synced = self .sync .get_syncer(&origin, &repo) - .sync_env(references_to_sync) + .sync_ref_spec(references_to_sync.try_into()?) .await?; - for item in synced.env.iter() { + for item in synced.ref_spec.iter() { let digest = item.resolve_digest(&repo).await?; runtime.push_digest(digest); } diff --git a/crates/spfs/src/sync.rs b/crates/spfs/src/sync.rs index 4515472bd..dc1a975ad 100644 --- a/crates/spfs/src/sync.rs +++ b/crates/spfs/src/sync.rs @@ -11,13 +11,13 @@ use reporter::{ SyncAnnotationResult, SyncBlobResult, SyncEntryResult, - SyncEnvItemResult, - SyncEnvResult, SyncLayerResult, SyncManifestResult, SyncObjectResult, SyncPayloadResult, SyncPlatformResult, + SyncRefItemResult, + SyncRefResult, SyncReporter, SyncReporters, SyncTagResult, @@ -177,52 +177,57 @@ impl<'src, 'dst> Syncer<'src, 'dst> { /// Sync the object(s) referenced by the given string. /// /// Any valid [`crate::tracking::EnvSpec`] is accepted as a reference. - pub async fn sync_ref>(&self, reference: R) -> Result { + pub async fn sync_ref>(&self, reference: R) -> Result { let env_spec = reference.as_ref().parse()?; - self.sync_env(env_spec).await + self.sync_ref_spec(env_spec).await } - /// Sync all of the objects identified by the given env. - pub async fn sync_env(&self, env: tracking::EnvSpec) -> Result { - self.reporter.visit_env(&env); + /// Sync all of the objects identified by the given ref spec. + pub async fn sync_ref_spec(&self, ref_spec: tracking::RefSpec) -> Result { + self.reporter.visit_ref_spec(&ref_spec); let mut futures = FuturesUnordered::new(); - for item in env.iter().cloned() { - futures.push(self.sync_env_item(item)); + for item in ref_spec.iter().cloned() { + futures.push(self.sync_ref_item(item)); } - let mut results = Vec::with_capacity(env.len()); + let mut results = Vec::with_capacity(ref_spec.len()); while let Some(result) = futures.try_next().await? { results.push(result); } - let res = SyncEnvResult { env, results }; - self.reporter.synced_env(&res); + let res = SyncRefResult { ref_spec, results }; + self.reporter.synced_ref_spec(&res); Ok(res) } /// Sync one environment item and any associated data. - pub async fn sync_env_item(&self, item: tracking::EnvSpecItem) -> Result { + pub async fn sync_ref_item(&self, item: tracking::RefSpecItem) -> Result { tracing::debug!(?item, "Syncing item"); - self.reporter.visit_env_item(&item); + self.reporter.visit_ref_item(&item); let res = match item { - tracking::EnvSpecItem::Digest(digest) => match self.sync_object_digest(digest).await { - Ok(r) => SyncEnvItemResult::Object(r), + tracking::RefSpecItem::Digest(digest) => match self.sync_object_digest(digest).await { + Ok(r) => SyncRefItemResult::Object(r), Err(Error::UnknownObject(digest)) => self .sync_payload(digest) .await - .map(SyncEnvItemResult::Payload)?, + .map(SyncRefItemResult::Payload)?, Err(e) => return Err(e), }, - tracking::EnvSpecItem::PartialDigest(digest) => { + tracking::RefSpecItem::PartialDigest(digest) => { self.sync_partial_digest(digest).await.map(Into::into)? } - tracking::EnvSpecItem::TagSpec(tag_spec) => { - self.sync_tag(tag_spec).await.map(SyncEnvItemResult::Tag)? + tracking::RefSpecItem::TagSpec(tag_spec) => { + self.sync_tag(tag_spec).await.map(SyncRefItemResult::Tag)? } // These are not objects in spfs, so they are not syncable - tracking::EnvSpecItem::SpecFile(_) => { - return Ok(SyncEnvItemResult::Object(SyncObjectResult::Ignorable)); + // XXX but it can be a spec file the contains syncable things? Would + // those things become garbage instantly in the destination repo? + // XXX shouldn't this be an error to inform the user that it was + // not synced? Or should a RefSpecItem even be allowed to contain + // SpecFiles? + tracking::RefSpecItem::SpecFile(_) => { + return Ok(SyncRefItemResult::Object(SyncObjectResult::Ignorable)); } }; - self.reporter.synced_env_item(&res); + self.reporter.synced_ref_item(&res); Ok(res) } diff --git a/crates/spfs/src/sync/reporter.rs b/crates/spfs/src/sync/reporter.rs index c3e0dbdac..7d28678fb 100644 --- a/crates/spfs/src/sync/reporter.rs +++ b/crates/spfs/src/sync/reporter.rs @@ -43,17 +43,17 @@ impl SyncReporters { /// followed up by a call to the corresponding synced_*. #[enum_dispatch::enum_dispatch] pub trait SyncReporter: Send + Sync { - /// Called when an environment has been identified to sync - fn visit_env(&self, _env: &tracking::EnvSpec) {} + /// Called when an ref spec has been identified to sync + fn visit_ref_spec(&self, _ref_spec: &tracking::RefSpec) {} - /// Called when a environment has finished syncing - fn synced_env(&self, _result: &SyncEnvResult) {} + /// Called when a ref spec has finished syncing + fn synced_ref_spec(&self, _result: &SyncRefResult) {} - /// Called when an environment item has been identified to sync - fn visit_env_item(&self, _item: &tracking::EnvSpecItem) {} + /// Called when an ref item has been identified to sync + fn visit_ref_item(&self, _item: &tracking::RefSpecItem) {} - /// Called when a environment item has finished syncing - fn synced_env_item(&self, _result: &SyncEnvItemResult) {} + /// Called when a ref item has finished syncing + fn synced_ref_item(&self, _result: &SyncRefItemResult) {} /// Called when a tag has been identified to sync fn visit_tag(&self, _tag: &tracking::TagSpec) {} @@ -114,17 +114,17 @@ impl SyncReporter for Arc where T: SyncReporter, { - fn visit_env(&self, env: &tracking::EnvSpec) { - (**self).visit_env(env) + fn visit_ref_spec(&self, ref_spec: &tracking::RefSpec) { + (**self).visit_ref_spec(ref_spec) } - fn synced_env(&self, result: &SyncEnvResult) { - (**self).synced_env(result) + fn synced_ref_spec(&self, result: &SyncRefResult) { + (**self).synced_ref_spec(result) } - fn visit_env_item(&self, item: &tracking::EnvSpecItem) { - (**self).visit_env_item(item) + fn visit_ref_item(&self, item: &tracking::RefSpecItem) { + (**self).visit_ref_item(item) } - fn synced_env_item(&self, result: &SyncEnvItemResult) { - (**self).synced_env_item(result) + fn synced_ref_item(&self, result: &SyncRefItemResult) { + (**self).synced_ref_item(result) } fn visit_tag(&self, tag: &tracking::TagSpec) { (**self).visit_tag(tag) @@ -177,17 +177,17 @@ where } impl SyncReporter for Box { - fn visit_env(&self, env: &tracking::EnvSpec) { - (**self).visit_env(env) + fn visit_ref_spec(&self, ref_spec: &tracking::RefSpec) { + (**self).visit_ref_spec(ref_spec) } - fn synced_env(&self, result: &SyncEnvResult) { - (**self).synced_env(result) + fn synced_ref_spec(&self, result: &SyncRefResult) { + (**self).synced_ref_spec(result) } - fn visit_env_item(&self, item: &tracking::EnvSpecItem) { - (**self).visit_env_item(item) + fn visit_ref_item(&self, item: &tracking::RefSpecItem) { + (**self).visit_ref_item(item) } - fn synced_env_item(&self, result: &SyncEnvItemResult) { - (**self).synced_env_item(result) + fn synced_ref_item(&self, result: &SyncRefItemResult) { + (**self).synced_ref_item(result) } fn visit_tag(&self, tag: &tracking::TagSpec) { (**self).visit_tag(tag) @@ -243,17 +243,17 @@ impl SyncReporter for Box> where T: SyncReporter, { - fn visit_env(&self, env: &tracking::EnvSpec) { - (***self).visit_env(env) + fn visit_ref_spec(&self, ref_spec: &tracking::RefSpec) { + (***self).visit_ref_spec(ref_spec) } - fn synced_env(&self, result: &SyncEnvResult) { - (***self).synced_env(result) + fn synced_ref_spec(&self, result: &SyncRefResult) { + (***self).synced_ref_spec(result) } - fn visit_env_item(&self, item: &tracking::EnvSpecItem) { - (***self).visit_env_item(item) + fn visit_ref_item(&self, item: &tracking::RefSpecItem) { + (***self).visit_ref_item(item) } - fn synced_env_item(&self, result: &SyncEnvItemResult) { - (***self).synced_env_item(result) + fn synced_ref_item(&self, result: &SyncRefItemResult) { + (***self).synced_ref_item(result) } fn visit_tag(&self, tag: &tracking::TagSpec) { (***self).visit_tag(tag) @@ -342,7 +342,7 @@ impl SyncReporter for ConsoleSyncReporter { bars.bytes.inc(result.summary().synced_payload_bytes); } - fn synced_env(&self, _result: &SyncEnvResult) { + fn synced_ref_spec(&self, _result: &SyncRefResult) { // Don't cause the bars to be initialized here if they haven't already // been, calling abandon will briefly display some zero-progress bars. if let Some(bars) = self.bars.get() { @@ -453,12 +453,12 @@ where } #[derive(Debug)] -pub struct SyncEnvResult { - pub env: tracking::EnvSpec, - pub results: Vec, +pub struct SyncRefResult { + pub ref_spec: tracking::RefSpec, + pub results: Vec, } -impl Summary for SyncEnvResult { +impl Summary for SyncRefResult { fn summary(&self) -> SyncSummary { self.results.iter().map(|r| r.summary()).sum() } @@ -466,13 +466,13 @@ impl Summary for SyncEnvResult { #[derive(Debug)] #[enum_dispatch::enum_dispatch(Summary)] -pub enum SyncEnvItemResult { +pub enum SyncRefItemResult { Tag(SyncTagResult), Object(SyncObjectResult), Payload(SyncPayloadResult), } -impl From for SyncEnvItemResult { +impl From for SyncRefItemResult { fn from(value: SyncItemResult) -> Self { match value { SyncItemResult::Object(obj) => Self::Object(obj), diff --git a/crates/spfs/src/sync_test.rs b/crates/spfs/src/sync_test.rs index d14283512..420a58705 100644 --- a/crates/spfs/src/sync_test.rs +++ b/crates/spfs/src/sync_test.rs @@ -283,11 +283,11 @@ async fn test_sync_missing_from_source( .await .expect("Should not fail when object is already in destination"); syncer - .sync_env(tag.into()) + .sync_ref_spec(tag.into()) .await .expect("Should not fail when object is already in destination"); syncer - .sync_env(platform_digest.into()) + .sync_ref_spec(platform_digest.into()) .await .expect("Should not fail when object is already in destination"); } diff --git a/crates/spfs/src/tracking/env.rs b/crates/spfs/src/tracking/env.rs index 0275b4147..f69c34dea 100644 --- a/crates/spfs/src/tracking/env.rs +++ b/crates/spfs/src/tracking/env.rs @@ -14,6 +14,8 @@ use serde::Deserialize; use super::tag::TagSpec; use crate::runtime::{LiveLayer, SpecApiVersion}; +use crate::tracking::ref_spec::RefSpecFile; +use crate::tracking::{RefSpec, RefSpecItem}; use crate::{Error, Result, encoding, graph}; #[cfg(test)] @@ -173,6 +175,14 @@ impl SpecFile { } } +impl From for SpecFile { + fn from(ref_spec_file: RefSpecFile) -> Self { + match ref_spec_file { + RefSpecFile::EnvLayersFile(x) => SpecFile::EnvLayersFile(x), + } + } +} + impl Display for SpecFile { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -222,7 +232,9 @@ impl Display for EnvLayersFile { #[derive(Debug, Clone, Eq, PartialEq)] pub enum EnvSpecItem { TagSpec(TagSpec), + /// May refer to an object, but not a payload PartialDigest(encoding::PartialDigest), + /// May refer to an object, but not a payload Digest(encoding::Digest), SpecFile(SpecFile), } @@ -232,11 +244,10 @@ impl EnvSpecItem { /// /// Any necessary lookups are done using the provided repository. /// - /// It is possible for this to succeed for tags even when no object or - /// payload exists with the digest. + /// It is possible for this to succeed for tags even when no object exists + /// with the digest. /// - /// The returned digest may refer to an object, a payload, or a non-existent - /// item. + /// The returned digest may refer to an object, or a non-existent item. pub async fn resolve_digest(&self, repo: &R) -> Result where R: crate::storage::Repository + ?Sized, @@ -244,7 +255,7 @@ impl EnvSpecItem { match self { Self::TagSpec(spec) => repo.resolve_tag(spec).await.map(|t| t.target), Self::PartialDigest(part) => repo - .resolve_full_digest(part, graph::PartialDigestType::Unknown) + .resolve_full_digest(part, graph::PartialDigestType::Object) .await .map(|found_digest| found_digest.into_digest()), Self::Digest(digest) => Ok(*digest), @@ -332,8 +343,8 @@ impl From for EnvSpecItem { /// Specifies a complete runtime environment that /// can be made up of multiple layers. /// -/// The env spec contains an non-empty, ordered set of references -/// that make up this environment. +/// The env spec contains an ordered set of references that make up this +/// environment. /// /// It can be easily parsed from a string containing /// tags and/or digests: @@ -359,6 +370,12 @@ pub struct EnvSpec { } impl EnvSpec { + /// Consume this EnvSpec and return its items + #[inline] + pub fn into_items(self) -> Vec { + self.items + } + /// Parse the provided string into an environment spec. pub fn parse>(spec: S) -> Result { Self::from_str(spec.as_ref()) @@ -461,6 +478,23 @@ impl FromStr for EnvSpec { } } +impl From for EnvSpec { + fn from(ref_spec: RefSpec) -> Self { + EnvSpec { + items: ref_spec + .into_items() + .into_iter() + .map(|r| match r { + RefSpecItem::TagSpec(t) => EnvSpecItem::TagSpec(t), + RefSpecItem::PartialDigest(d) => EnvSpecItem::PartialDigest(d), + RefSpecItem::Digest(d) => EnvSpecItem::Digest(d), + RefSpecItem::SpecFile(s) => EnvSpecItem::SpecFile(s.into()), + }) + .collect(), + } + } +} + impl From for EnvSpec where I: Into, diff --git a/crates/spfs/src/tracking/mod.rs b/crates/spfs/src/tracking/mod.rs index f9b8ade3e..efe77dec9 100644 --- a/crates/spfs/src/tracking/mod.rs +++ b/crates/spfs/src/tracking/mod.rs @@ -10,6 +10,7 @@ mod entry; mod env; pub mod manifest; mod object; +mod ref_spec; mod tag; pub use blob_reader::{BlobRead, BlobReadExt}; @@ -38,4 +39,5 @@ pub use manifest::{ pub use object::Object; pub use tag::{Tag, TagSpec, build_tag_spec, split_tag_spec}; mod time_spec; +pub use ref_spec::{RefSpec, RefSpecItem}; pub use time_spec::{TimeSpec, parse_duration, parse_time}; diff --git a/crates/spfs/src/tracking/ref_spec.rs b/crates/spfs/src/tracking/ref_spec.rs new file mode 100644 index 000000000..d30dde615 --- /dev/null +++ b/crates/spfs/src/tracking/ref_spec.rs @@ -0,0 +1,418 @@ +// Copyright (c) Contributors to the SPK project. +// SPDX-License-Identifier: Apache-2.0 +// https://github.com/spkenv/spk + +use std::borrow::Cow; +use std::fmt::Display; +use std::str::FromStr; + +use nonempty::{NonEmpty, nonempty}; +use serde::Deserialize; + +use super::tag::TagSpec; +use crate::tracking::env::EnvLayersFile; +use crate::tracking::{EnvSpec, EnvSpecItem, SpecFile}; +use crate::{Error, Result, encoding, graph}; + +#[cfg(test)] +#[path = "./ref_spec_test.rs"] +mod ref_spec_test; + +/// The pattern used to split components of an ref spec string +pub const REF_SPEC_SEPARATOR: &str = "+"; + +/// Enum of all the spfs ref spec things that can be constructed from +/// filepaths given on the command line. +#[derive(Deserialize, Debug, Clone, Eq, PartialEq)] +pub enum RefSpecFile { + EnvLayersFile(EnvLayersFile), +} + +impl Display for RefSpecFile { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::EnvLayersFile(x) => x.fmt(f), + } + } +} + +/// Specifies an spfs item. +/// +/// This represents something that, e.g., can be `spfs push`ed from one +/// repository to another. +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum RefSpecItem { + TagSpec(TagSpec), + /// May refer to an object or a payload + PartialDigest(encoding::PartialDigest), + /// May refer to an object or a payload + Digest(encoding::Digest), + SpecFile(RefSpecFile), +} + +impl RefSpecItem { + /// Find the digest for this ref spec item. + /// + /// Any necessary lookups are done using the provided repository. + /// + /// It is possible for this to succeed for tags even when no object or + /// payload exists with the digest. + /// + /// The returned digest may refer to an object, a payload, or a non-existent + /// item. + pub async fn resolve_digest(&self, repo: &R) -> Result + where + R: crate::storage::Repository + ?Sized, + { + match self { + Self::TagSpec(spec) => repo.resolve_tag(spec).await.map(|t| t.target), + Self::PartialDigest(part) => repo + .resolve_full_digest(part, graph::PartialDigestType::Unknown) + .await + .map(|found_digest| found_digest.into_digest()), + Self::Digest(digest) => Ok(*digest), + Self::SpecFile(_) => Err(Error::String(String::from( + "impossible operation: spfs env files do not have digests", + ))), + } + } + + /// RefSpecItem::TagSpec item variants return a + /// RefSpecItem::Digest item variant built from the TagSpec's + /// tag's underlying digest. All other item variants return the + /// existing item unchanged. + /// + /// Any necessary lookups are done using the provided repository + pub async fn with_tag_resolved(&self, repo: &R) -> Result> + where + R: crate::storage::Repository + ?Sized, + { + match self { + Self::TagSpec(_spec) => Ok(Cow::Owned(RefSpecItem::Digest( + self.resolve_digest(repo).await?, + ))), + _ => Ok(Cow::Borrowed(self)), + } + } +} + +impl<'de> Deserialize<'de> for RefSpecItem { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + let value = String::deserialize(deserializer)?; + + RefSpecItem::from_str(&value).map_err(|err| { + serde::de::Error::custom(format!("deserializing RefSpecItem failed: {err}")) + }) + } +} + +impl std::fmt::Display for RefSpecItem { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::TagSpec(x) => x.fmt(f), + Self::PartialDigest(x) => x.fmt(f), + Self::Digest(x) => x.fmt(f), + Self::SpecFile(x) => x.fmt(f), + } + } +} + +impl FromStr for RefSpecItem { + type Err = Error; + + fn from_str(s: &str) -> Result { + parse_ref_spec_item(s) + } +} + +impl From for RefSpecItem { + fn from(item: TagSpec) -> Self { + Self::TagSpec(item) + } +} + +impl From for RefSpecItem { + fn from(item: encoding::PartialDigest) -> Self { + Self::PartialDigest(item) + } +} + +impl From for RefSpecItem { + fn from(item: encoding::Digest) -> Self { + Self::Digest(item) + } +} + +impl TryFrom for RefSpecItem { + type Error = Error; + + fn try_from(item: EnvSpecItem) -> Result { + match item { + EnvSpecItem::TagSpec(tag_spec) => Ok(RefSpecItem::TagSpec(tag_spec)), + EnvSpecItem::PartialDigest(partial_digest) => { + Ok(RefSpecItem::PartialDigest(partial_digest)) + } + EnvSpecItem::Digest(digest) => Ok(RefSpecItem::Digest(digest)), + EnvSpecItem::SpecFile(spec_file) => Ok(RefSpecItem::SpecFile(match spec_file { + SpecFile::EnvLayersFile(layers_file) => RefSpecFile::EnvLayersFile(layers_file), + SpecFile::LiveLayer(_) => { + return Err(Error::String( + "cannot convert LiveLayer SpecFile to RefSpecItem".into(), + )); + } + })), + } + } +} + +/// Specifies a non-empty collection of spfs references. +/// +/// It can be easily parsed from a string containing +/// tags and/or digests: +/// +/// ```rust +/// use spfs::tracking::RefSpec; +/// +/// let spec = RefSpec::parse("sometag~1+my-other-tag").unwrap(); +/// let items: Vec<_> = spec.iter().map(ToString::to_string).collect(); +/// assert_eq!(items, vec!["sometag~1", "my-other-tag"]); +/// +/// let spec = RefSpec::parse("3YDG35SUMJS67N2QPQ4NQCYJ6QGKMEB5H4MHC76VRGMRWBRBLFHA====+my-tag").unwrap(); +/// let items: Vec<_> = spec.iter().map(ToString::to_string).collect(); +/// assert_eq!(items, vec!["3YDG35SUMJS67N2QPQ4NQCYJ6QGKMEB5H4MHC76VRGMRWBRBLFHA====", "my-tag"]); +/// ``` +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct RefSpec { + items: NonEmpty, +} + +impl RefSpec { + /// Combine multiple RefSpecs into a single RefSpec. + pub fn combine(ref_specs: &[RefSpec]) -> Result { + let Some((head, tail)) = ref_specs.split_first() else { + return Err(Error::String( + "at least one RefSpec is required to combine".into(), + )); + }; + Ok(tail.iter().cloned().fold(head.clone(), |mut acc, spec| { + acc.items.extend(spec.into_items()); + acc + })) + } + + /// Consume this RefSpec and return its items. + #[inline] + pub fn into_items(self) -> NonEmpty { + self.items + } + + /// Parse the provided string into an ref spec. + pub fn parse>(spec: S) -> Result { + Self::from_str(spec.as_ref()) + } + + /// TagSpec items are turned into Digest items using the digest + /// resolved from the tag. All other items are returned as is. + /// This will error when trying to resolve a tag that is not in + /// any of the repos. The repos are searched in order for the tag, + /// and first repo with the tag is used. + pub async fn resolve_tag_item_to_digest_item( + &self, + item: &RefSpecItem, + repos: &Vec<&R>, + ) -> Result + where + R: crate::storage::Repository + ?Sized, + { + for repo in repos { + match item.with_tag_resolved(*repo).await { + Ok(resolved_item) => return Ok(resolved_item.into_owned()), + Err(err) => { + tracing::debug!("{err}") + } + } + } + + Err(Error::UnknownReference(item.to_string())) + } + + /// Create a RefSpec from an iterator of RefSpecItems. + pub fn try_from_iter(iter: I) -> Result + where + I: IntoIterator, + R: Into, + { + let items: Vec = iter.into_iter().map(Into::into).collect(); + Ok(RefSpec { + items: NonEmpty::from_vec(items) + .ok_or_else(|| Error::String("a ref spec may not be empty".into()))?, + }) + } + + /// Return a new RefSpec based on this one, with all the tag items + /// converted to digest items using the tags' underlying digests. + pub async fn with_tag_items_resolved_to_digest_items( + &self, + repos: &Vec<&R>, + ) -> Result + where + R: crate::storage::Repository + ?Sized, + { + let mut new_items: Vec = Vec::with_capacity(self.items.len()); + for item in &self.items { + // Filter out the LiveLayers entirely because they do not have digests + if let RefSpecItem::SpecFile(_) = item { + continue; + } + new_items.push(self.resolve_tag_item_to_digest_item(item, repos).await?); + } + + Ok(RefSpec { + items: NonEmpty::from_vec(new_items).ok_or_else(|| { + Error::String("impossible: empty RefSpec after tag resolution".into()) + })?, + }) + } +} + +impl std::ops::Deref for RefSpec { + type Target = NonEmpty; + + fn deref(&self) -> &Self::Target { + &self.items + } +} + +impl std::ops::DerefMut for RefSpec { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.items + } +} + +impl std::iter::IntoIterator for RefSpec { + type Item = RefSpecItem; + + type IntoIter = as IntoIterator>::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.items.into_iter() + } +} + +impl FromStr for RefSpec { + type Err = Error; + + fn from_str(s: &str) -> Result { + Ok(Self { + items: parse_ref_spec_items(s)?, + }) + } +} + +impl TryFrom for RefSpec { + type Error = Error; + + fn try_from(env_spec: EnvSpec) -> Result { + let env_spec_items = env_spec.into_items(); + let mut items: Vec = Vec::with_capacity(env_spec_items.len()); + for env_item in env_spec_items { + items.push(env_item.try_into()?); + } + Ok(RefSpec { + items: NonEmpty::from_vec(items) + .ok_or_else(|| Error::String("a ref spec may not be empty".into()))?, + }) + } +} + +impl From for RefSpec +where + I: Into, +{ + fn from(item: I) -> Self { + RefSpec { + items: nonempty![item.into()], + } + } +} + +impl std::iter::Extend for RefSpec +where + I: Into, +{ + fn extend>(&mut self, iter: T) { + self.items.extend(iter.into_iter().map(Into::into)) + } +} + +impl std::fmt::Display for RefSpec { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let items: Vec<_> = self.items.iter().map(|i| i.to_string()).collect(); + write!(f, "{}", items.join(REF_SPEC_SEPARATOR)) + } +} + +/// Return the items identified in an ref spec string. +fn parse_ref_spec_items>(spec: S) -> Result> { + let mut items = Vec::new(); + for layer in spec.as_ref().split(REF_SPEC_SEPARATOR) { + let item = parse_ref_spec_item(layer)?; + // Env list of layers files are immediately expanded into the + // RefSpec's items list. Other items are just added as is. + if let RefSpecItem::SpecFile(RefSpecFile::EnvLayersFile(layers)) = item { + items.extend( + layers + .flatten()? + .into_iter() + .map(TryInto::try_into) + .collect::>>()?, + ); + } else { + items.push(item); + } + } + NonEmpty::from_vec(items) + .ok_or_else(|| Error::String("RefSpec must contain at least one valid RefSpecItem".into())) +} + +/// Parse the given string as an single ref spec item. +fn parse_ref_spec_item>(spec: S) -> Result { + let spec = spec.as_ref(); + if spec.is_empty() || spec == crate::tracking::ENV_SPEC_EMPTY { + return Err(Error::String("ref spec item may not be empty".into())); + } + encoding::parse_digest(spec) + .map(RefSpecItem::Digest) + .or_else(|err| { + tracing::debug!("Unable to parse as a Digest: {err}"); + encoding::PartialDigest::parse(spec).map(RefSpecItem::PartialDigest) + }) + .or_else(|err| { + tracing::debug!("Unable to parse as a Partial Digest: {err}"); + SpecFile::parse(spec).and_then(|spec_file| { + Ok(RefSpecItem::SpecFile(match spec_file { + SpecFile::EnvLayersFile(layers_file) => RefSpecFile::EnvLayersFile(layers_file), + SpecFile::LiveLayer(_) => { + return Err(Error::String( + "cannot use LiveLayer spec files in RefSpec".into(), + )); + } + })) + }) + }) + .or_else(|err| { + tracing::debug!("Unable to parse as a RefSpecFile: {err}"); + // A duplicate spec file reference error while parsing a + // spfs spec file means its filepath had already been read + // in. Reading it in again would generate an infinite + // parsing loop, so this should error out now. + if let Error::DuplicateSpecFileReference(ref _filepath) = err { + return Err(err); + } + + TagSpec::parse(spec).map(RefSpecItem::TagSpec) + }) +} diff --git a/crates/spfs/src/tracking/ref_spec_test.rs b/crates/spfs/src/tracking/ref_spec_test.rs new file mode 100644 index 000000000..b5cc23389 --- /dev/null +++ b/crates/spfs/src/tracking/ref_spec_test.rs @@ -0,0 +1,80 @@ +// Copyright (c) Contributors to the SPK project. +// SPDX-License-Identifier: Apache-2.0 +// https://github.com/spkenv/spk + +use std::fs::File; +use std::io::Write; + +use rstest::rstest; + +use super::RefSpec; +use crate::fixtures::tmpdir; +use crate::tracking; + +#[rstest] +fn test_ref_spec_validation() { + let spec = RefSpec::parse("one+two").expect("failed to parse ref spec"); + assert_eq!(spec.items.len(), 2); +} + +#[rstest] +fn test_ref_spec_empty() { + let _ = RefSpec::parse("").expect_err("empty spec should be invalid"); + let _ = RefSpec::parse(tracking::ENV_SPEC_EMPTY).expect_err("dash spec should be invalid"); +} + +#[rstest] +fn test_ref_spec_with_live_layer_dir(tmpdir: tempfile::TempDir) { + let dir = tmpdir.path(); + let file_path = dir.join("layer.spfs.yaml"); + let mut tmp_file = File::create(file_path).unwrap(); + writeln!(tmp_file, "# test live layer").unwrap(); + + let ref_spec = RefSpec::parse(dir.display().to_string()) + .expect("absolute directory containing a layer.spfs.yaml should be valid"); + assert!(!ref_spec.is_empty()) +} + +#[rstest] +fn test_ref_spec_with_live_layer_file(tmpdir: tempfile::TempDir) { + let dir = tmpdir.path(); + let file_path = dir.join("livelayer.spfs.yaml"); + let mut tmp_file = File::create(file_path.clone()).unwrap(); + writeln!(tmp_file, "# test live layer").unwrap(); + + let ref_spec = RefSpec::parse(file_path.display().to_string()) + .expect("absolute path to livelayer.spfs.yaml should be valid"); + assert!(!ref_spec.is_empty()); +} + +#[rstest] +fn test_ref_spec_with_runspec_file(tmpdir: tempfile::TempDir) { + let dir = tmpdir.path(); + let file_path = dir.join("runspec.spfs.yaml"); + let mut tmp_file = File::create(file_path.clone()).unwrap(); + writeln!( + tmp_file, + "# test run spec\napi: spfs/v0/runspec\nlayers:\n + - A7USTIBXPXHMD5CYEIIOBMFLM3X77ESVR3WAUXQ7XQQGTHKH7DMQ====" + ) + .unwrap(); + + let ref_spec = RefSpec::parse(file_path.display().to_string()) + .expect("absolute path to runspec.spfs.yaml should be valid"); + assert!(!ref_spec.is_empty()); +} + +#[rstest] +fn test_ref_spec_with_empty_runspec_file(tmpdir: tempfile::TempDir) { + let dir = tmpdir.path(); + let file_path = dir.join("runspec.spfs.yaml"); + let mut tmp_file = File::create(file_path.clone()).unwrap(); + writeln!( + tmp_file, + "# test run spec\napi: spfs/v0/runspec\nlayers: []\n" + ) + .unwrap(); + + let _ = RefSpec::parse(file_path.display().to_string()) + .expect_err("empty runspec.spfs.yaml should be invalid"); +} diff --git a/crates/spk-cli/common/src/publish.rs b/crates/spk-cli/common/src/publish.rs index 59450b996..b554f0874 100644 --- a/crates/spk-cli/common/src/publish.rs +++ b/crates/spk-cli/common/src/publish.rs @@ -5,6 +5,7 @@ use std::str::FromStr; use std::sync::Arc; +use spfs::tracking::RefSpec; use spk_schema::foundation::format::{FormatComponents, FormatIdent}; use spk_schema::foundation::ident_component::ComponentSet; use spk_schema::ident::AsVersionIdent; @@ -216,7 +217,7 @@ impl Publisher { let spec = self.from.read_package(build).await?; let components = self.from.read_components(build).await?; tracing::info!("publishing package: {}", spec.ident().format_ident()); - let env_spec = components.values().cloned().collect(); + let ref_spec = RefSpec::try_from_iter(components.values().cloned())?; tracing::debug!( " syncing components: {}", ComponentSet::from(components.keys().cloned()).format_components() @@ -231,7 +232,7 @@ impl Publisher { }; syncer .with_reporter(spfs::sync::reporter::SyncReporters::console()) - .sync_env(env_spec) + .sync_ref_spec(ref_spec) .await?; self.to.publish_package(&spec, &components).await?; } diff --git a/crates/spk-cli/group3/src/cmd_import.rs b/crates/spk-cli/group3/src/cmd_import.rs index 730c83f60..3bfa6f36e 100644 --- a/crates/spk-cli/group3/src/cmd_import.rs +++ b/crates/spk-cli/group3/src/cmd_import.rs @@ -7,6 +7,7 @@ use futures::TryStreamExt; use miette::{Context, Result}; use spfs::storage::TagStorage; use spfs::sync::reporter::Summary; +use spfs::tracking::RefSpec; use spk_cli_common::{CommandArgs, Run}; #[cfg(test)] @@ -37,16 +38,17 @@ impl Run for Import { for filename in self.files.iter() { let tar_repo = spfs::storage::tar::TarRepository::open(&filename).await?; let tar_repo: spfs::storage::RepositoryHandle = tar_repo.into(); - let env_spec = tar_repo - .iter_tags() - .map_ok(|(spec, _)| spec) - .try_collect() - .await - .wrap_err("Failed to collect tags from archive")?; + let ref_spec = RefSpec::try_from_iter( + tar_repo + .iter_tags() + .map_ok(|(spec, _)| spec) + .try_collect::>() + .await?, + )?; tracing::info!(archive = ?filename, "importing"); summary += syncer .clone_with_source(&tar_repo) - .sync_env(env_spec) + .sync_ref_spec(ref_spec) .await .wrap_err("Failed to sync archived data")? .summary(); diff --git a/crates/spk-launcher/src/main.rs b/crates/spk-launcher/src/main.rs index 877600399..1c65c6214 100644 --- a/crates/spk-launcher/src/main.rs +++ b/crates/spk-launcher/src/main.rs @@ -24,7 +24,7 @@ use spfs::prelude::*; use spfs::storage::RepositoryHandle; use spfs::storage::fallback::FallbackProxy; use spfs::storage::fs::OpenFsRepository; -use spfs::tracking::EnvSpec; +use spfs::tracking::RefSpec; const DEV_SHM: &str = "/dev/shm"; const ORIGIN: &str = "origin"; @@ -126,7 +126,7 @@ impl<'a> Dynamic<'a> { .into_diagnostic() .wrap_err("create temp working directory")?; - let env_spec = EnvSpec::parse(tag).wrap_err("create env spec")?; + let ref_spec = RefSpec::parse(tag).wrap_err("create ref spec")?; // Ensure tag is sync'd local because `render_into_directory` operates // out of the local repo. @@ -134,8 +134,11 @@ impl<'a> Dynamic<'a> { let syncer = spfs::Syncer::new(&remote, &handle) .with_policy(spfs::sync::SyncPolicy::LatestTags) .with_reporter(spfs::sync::reporter::SyncReporters::console()); - let r = syncer.sync_env(env_spec).await.wrap_err("sync reference")?; - let env_spec = r.env; + let r = syncer + .sync_ref_spec(ref_spec) + .await + .wrap_err("sync reference")?; + let ref_spec = r.ref_spec; let fallback = FallbackProxy::new( local, @@ -148,7 +151,7 @@ impl<'a> Dynamic<'a> { spfs::storage::fs::Renderer::new(&fallback) .with_reporter(spfs::storage::fs::ConsoleRenderReporter::default()) .render_into_directory( - env_spec, + ref_spec, temp_dir.path(), spfs::storage::fs::RenderType::Copy, ) diff --git a/crates/spk-storage/src/storage/archive.rs b/crates/spk-storage/src/storage/archive.rs index 22b14c191..a43e81531 100644 --- a/crates/spk-storage/src/storage/archive.rs +++ b/crates/spk-storage/src/storage/archive.rs @@ -6,6 +6,7 @@ use std::convert::TryFrom; use std::path::Path; use itertools::{Itertools, Position}; +use spfs::tracking::RefSpec; use spk_schema::ident::AsVersionIdent; use spk_schema::{AnyIdent, BuildIdent, VersionIdent}; use variantly::Variantly; @@ -182,8 +183,8 @@ async fn copy_package( tracing::info!(%pkg, "exporting"); let syncer = spfs::Syncer::new(src_repo, dst_repo) .with_reporter(spfs::sync::reporter::SyncReporters::console()); - let desired = components.iter().map(|i| *i.1).collect(); - syncer.sync_env(desired).await?; + let desired = RefSpec::try_from_iter(components.iter().map(|i| *i.1))?; + syncer.sync_ref_spec(desired).await?; dst_repo.publish_package(&spec, &components).await?; Ok(()) }