diff --git a/Cargo.lock b/Cargo.lock index 7937ce2704..6973e63b5c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3978,6 +3978,7 @@ dependencies = [ "unix_mode", "url", "uuid 1.18.1", + "variantly", "walkdir", "whoami", "windows 0.51.1", diff --git a/crates/spfs-cli/cmd-render/src/cmd_render.rs b/crates/spfs-cli/cmd-render/src/cmd_render.rs index c7508623d3..c10d218102 100644 --- a/crates/spfs-cli/cmd-render/src/cmd_render.rs +++ b/crates/spfs-cli/cmd-render/src/cmd_render.rs @@ -4,9 +4,10 @@ use clap::Parser; use clap::builder::TypedValueParser; -use miette::{Context, Result}; +use miette::{Context, IntoDiagnostic, Result}; use spfs::prelude::*; use spfs::storage::fallback::FallbackProxy; +use spfs::storage::fs::{MaybeRenderStore, RenderStore}; use spfs::{Error, RenderResult, graph}; use spfs_cli_common::{self as cli, CommandName, HasRepositoryArgs}; use strum::VariantNames; @@ -80,7 +81,11 @@ impl CmdRender { let rendered = match &self.target { Some(target) => self.render_to_dir(fallback, env_spec, target).await?, - None => self.render_to_repo(fallback, env_spec).await?, + None => { + // This path requires a repository that supports renders. + let fallback: FallbackProxy = fallback.try_into().into_diagnostic()?; + self.render_to_repo(fallback, env_spec).await? + } }; tracing::debug!("render(s) completed successfully"); @@ -91,7 +96,7 @@ impl CmdRender { async fn render_to_dir( &self, - repo: FallbackProxy, + repo: FallbackProxy, env_spec: spfs::tracking::EnvSpec, target: &std::path::Path, ) -> Result { @@ -140,7 +145,7 @@ impl CmdRender { async fn render_to_repo( &self, - repo: FallbackProxy, + repo: FallbackProxy, env_spec: spfs::tracking::EnvSpec, ) -> Result { let mut stack = graph::Stack::default(); diff --git a/crates/spfs-cli/common/src/args.rs b/crates/spfs-cli/common/src/args.rs index 382156a825..9f2c86f50d 100644 --- a/crates/spfs-cli/common/src/args.rs +++ b/crates/spfs-cli/common/src/args.rs @@ -13,7 +13,7 @@ use miette::{Error, IntoDiagnostic, Result, WrapErr}; #[cfg(feature = "sentry")] use once_cell::sync::OnceCell; use spfs::io::Pluralize; -use spfs::storage::LocalRepository; +use spfs::storage::LocalPayloads; use tracing_subscriber::prelude::*; const SPFS_LOG: &str = "SPFS_LOG"; @@ -140,7 +140,7 @@ impl Render { reporter: Reporter, ) -> spfs::storage::fs::Renderer<'repo, Repo, Reporter> where - Repo: spfs::storage::Repository + LocalRepository, + Repo: spfs::storage::Repository + LocalPayloads, Reporter: spfs::storage::fs::RenderReporter, { spfs::storage::fs::Renderer::new(repo) diff --git a/crates/spfs-cli/common/src/lib.rs b/crates/spfs-cli/common/src/lib.rs index b2b849d859..856a23ae65 100644 --- a/crates/spfs-cli/common/src/lib.rs +++ b/crates/spfs-cli/common/src/lib.rs @@ -8,7 +8,8 @@ mod args; pub mod __private { // Private re-exports for macros - pub use {libc, spfs}; + pub use libc; + pub use spfs; } pub use args::{ diff --git a/crates/spfs-cli/main/src/cmd_init.rs b/crates/spfs-cli/main/src/cmd_init.rs index fd274f71af..98b70d0216 100644 --- a/crates/spfs-cli/main/src/cmd_init.rs +++ b/crates/spfs-cli/main/src/cmd_init.rs @@ -6,6 +6,7 @@ use std::path::PathBuf; use clap::{Args, Subcommand}; use miette::Result; +use spfs::storage::fs::NoRenderStore; /// Create an empty filesystem repository #[derive(Debug, Args)] @@ -36,7 +37,7 @@ impl InitSubcommand { pub async fn run(&self, _config: &spfs::Config) -> Result { match self { Self::Repo { path } => { - spfs::storage::fs::MaybeOpenFsRepository::create(&path).await?; + spfs::storage::fs::MaybeOpenFsRepository::::create(&path).await?; Ok(0) } } diff --git a/crates/spfs-cli/main/src/cmd_search.rs b/crates/spfs-cli/main/src/cmd_search.rs index ad86dfe503..ca4f4d375d 100644 --- a/crates/spfs-cli/main/src/cmd_search.rs +++ b/crates/spfs-cli/main/src/cmd_search.rs @@ -5,6 +5,7 @@ use clap::Args; use miette::Result; use spfs::prelude::*; +use spfs::storage::fs::NoRenderStore; use spfs_cli_common as cli; use tokio_stream::StreamExt; @@ -33,7 +34,10 @@ impl CmdSearch { }; repos.push(remote); } - repos.insert(0, config.get_local_repository().await?.into()); + repos.insert( + 0, + config.get_local_repository::().await?.into(), + ); for repo in repos.into_iter() { let mut tag_streams = repo.iter_tags(); while let Some(tag) = tag_streams.next().await { diff --git a/crates/spfs-vfs/src/fuse.rs b/crates/spfs-vfs/src/fuse.rs index 5561099270..ff82fd6a42 100644 --- a/crates/spfs-vfs/src/fuse.rs +++ b/crates/spfs-vfs/src/fuse.rs @@ -30,7 +30,7 @@ use fuser::{ }; use spfs::OsError; use spfs::prelude::*; -use spfs::storage::LocalRepository; +use spfs::storage::LocalPayloads; #[cfg(feature = "fuse-backend-abi-7-31")] use spfs::tracking::BlobRead; use spfs::tracking::{Entry, EntryKind, EnvSpec, Manifest}; @@ -379,9 +379,13 @@ impl Filesystem { #[allow(unused_mut)] let mut flags = FOPEN_KEEP_CACHE; for repo in self.repos.iter() { - match &**repo { - spfs::storage::RepositoryHandle::FS(fs_repo) => { - let Ok(fs_repo) = fs_repo.opened().await else { + // XXX: Using a macro here for an easy fix but it would be nicer + // if there was a way to borrow the RepositoryHandle as a + // `&MaybeOpenFsRepository` since this code + // doesn't need to access renders. + macro_rules! read_fs { + ($fs_repo:ident) => { + let Ok(fs_repo) = $fs_repo.opened().await else { reply.error(libc::ENOENT); return; }; @@ -396,6 +400,17 @@ impl Filesystem { } Err(err) => err!(reply, err), } + }; + } + match &**repo { + spfs::storage::RepositoryHandle::FSWithMaybeRenders(fs_repo) => { + read_fs!(fs_repo); + } + spfs::storage::RepositoryHandle::FSWithRenders(fs_repo) => { + read_fs!(fs_repo); + } + spfs::storage::RepositoryHandle::FSWithoutRenders(fs_repo) => { + read_fs!(fs_repo); } #[cfg(feature = "fuse-backend-abi-7-31")] repo => match repo.open_payload(*digest).await { @@ -747,8 +762,7 @@ impl SessionInner { .map_err(|source| spfs::Error::FailedToOpenRepository { repository: "".into(), source, - })? - .into(); + })?; tracing::debug!("Computing environment manifest..."); let manifest = spfs::compute_environment_manifest(&self.reference, &repo).await?; diff --git a/crates/spfs-vfs/src/winfsp/mod.rs b/crates/spfs-vfs/src/winfsp/mod.rs index fd1b1352a9..f6c74cbd88 100644 --- a/crates/spfs-vfs/src/winfsp/mod.rs +++ b/crates/spfs-vfs/src/winfsp/mod.rs @@ -78,7 +78,13 @@ impl Service { repository: "".into(), source, })?; - let repos = repo.into_stack().into_iter().map(Arc::new).collect(); + let repos = match repo { + spfs::storage::RepositoryHandle::Proxy(proxy) => proxy.into_stack(), + repo => vec![repo], + } + .into_iter() + .map(Arc::new) + .collect(); // as of writing, the descriptor mode is the only one that works in // winfsp-rs without causing crashes diff --git a/crates/spfs-vfs/src/winfsp/mount.rs b/crates/spfs-vfs/src/winfsp/mount.rs index 652758d64d..1ee9c3a8e1 100644 --- a/crates/spfs-vfs/src/winfsp/mount.rs +++ b/crates/spfs-vfs/src/winfsp/mount.rs @@ -10,7 +10,7 @@ use dashmap::DashMap; use libc::c_void; use spfs::OsError; use spfs::prelude::*; -use spfs::storage::LocalRepository; +use spfs::storage::LocalPayloads; use spfs::tracking::{Entry, EntryKind}; use tokio::io::AsyncReadExt; use windows::Win32::Foundation::{ERROR_SEEK_ON_DEVICE, STATUS_NOT_A_DIRECTORY}; @@ -281,9 +281,13 @@ impl winfsp::filesystem::FileSystemContext for Mount { let digest = entry.object; self.rt.spawn(async move { for repo in repos.into_iter() { - match &*repo { - spfs::storage::RepositoryHandle::FS(fs_repo) => { - let Ok(fs_repo) = fs_repo.opened().await else { + // XXX: Using a macro here for an easy fix but it would be nicer + // if there was a way to borrow the RepositoryHandle as a + // `&MaybeOpenFsRepository` since this code + // doesn't need to access renders. + macro_rules! read_fs { + ($fs_repo:ident) => { + let Ok(fs_repo) = $fs_repo.opened().await else { let _ = send.send(Err(winfsp::FspError::IO(std::io::ErrorKind::NotFound))); return; @@ -299,6 +303,17 @@ impl winfsp::filesystem::FileSystemContext for Mount { } Err(err) => err!(send, err), } + }; + } + match &*repo { + spfs::storage::RepositoryHandle::FSWithMaybeRenders(fs_repo) => { + read_fs!(fs_repo); + } + spfs::storage::RepositoryHandle::FSWithRenders(fs_repo) => { + read_fs!(fs_repo); + } + spfs::storage::RepositoryHandle::FSWithoutRenders(fs_repo) => { + read_fs!(fs_repo); } repo => match repo.open_payload(digest).await { Ok((stream, _)) => { diff --git a/crates/spfs/Cargo.toml b/crates/spfs/Cargo.toml index f3850f6b9d..de868d51a9 100644 --- a/crates/spfs/Cargo.toml +++ b/crates/spfs/Cargo.toml @@ -101,6 +101,7 @@ ulid = { workspace = true } unix_mode = "0.1.3" url = { version = "2.2", features = ["serde"] } uuid = { version = "1.1", features = ["v4"] } +variantly = { workspace = true } walkdir = "2.3" whoami = { workspace = true } diff --git a/crates/spfs/benches/spfs_bench.rs b/crates/spfs/benches/spfs_bench.rs index ce1a266682..f68060ddc3 100644 --- a/crates/spfs/benches/spfs_bench.rs +++ b/crates/spfs/benches/spfs_bench.rs @@ -9,6 +9,7 @@ use std::time::Duration; use criterion::{Criterion, Throughput, criterion_group, criterion_main}; use spfs::prelude::*; +use spfs::storage::fs::NoRenderStore; pub fn commit_benchmark(c: &mut Criterion) { const NUM_FILES: usize = 1024; @@ -44,9 +45,11 @@ pub fn commit_benchmark(c: &mut Criterion) { .expect("create a temp directory for spfs repo"); let repo: Arc = Arc::new( tokio_runtime - .block_on(spfs::storage::fs::MaybeOpenFsRepository::create( - repo_path.path().join("repo"), - )) + .block_on( + spfs::storage::fs::MaybeOpenFsRepository::::create( + repo_path.path().join("repo"), + ), + ) .expect("create spfs repo") .into(), ); diff --git a/crates/spfs/src/bootstrap_test.rs b/crates/spfs/src/bootstrap_test.rs index e643499a1f..380d2781f5 100644 --- a/crates/spfs/src/bootstrap_test.rs +++ b/crates/spfs/src/bootstrap_test.rs @@ -11,6 +11,7 @@ use super::build_shell_initialized_command; use crate::fixtures::*; use crate::resolve::which; use crate::runtime; +use crate::storage::fs::RenderStore; #[rstest] #[case::bash("bash", "test.sh", "echo hi; export TEST_VALUE='spfs-test-value'")] @@ -33,7 +34,7 @@ async fn test_shell_initialization_startup_scripts( }; let root = tmpdir.path().to_string_lossy().to_string(); let repo = crate::storage::RepositoryHandle::from( - crate::storage::fs::MaybeOpenFsRepository::create(&root) + crate::storage::fs::MaybeOpenFsRepository::::create(&root) .await .unwrap(), ); @@ -118,7 +119,7 @@ async fn test_shell_initialization_no_startup_scripts( }; let root = tmpdir.path().to_string_lossy().to_string(); let repo = crate::storage::RepositoryHandle::from( - crate::storage::fs::MaybeOpenFsRepository::create(&root) + crate::storage::fs::MaybeOpenFsRepository::::create(&root) .await .unwrap(), ); diff --git a/crates/spfs/src/clean.rs b/crates/spfs/src/clean.rs index 8d6a0c3ca1..b9582c3ace 100644 --- a/crates/spfs/src/clean.rs +++ b/crates/spfs/src/clean.rs @@ -8,6 +8,7 @@ use std::future::ready; use std::num::NonZero; #[cfg(unix)] use std::os::linux::fs::MetadataExt; +use std::path::Path; use chrono::{DateTime, Duration, Local, Utc}; use colored::Colorize; @@ -20,7 +21,7 @@ use super::prune::PruneParameters; use crate::io::Pluralize; use crate::prelude::*; use crate::runtime::makedirs_with_perms; -use crate::storage::fs::FsRepositoryOps; +use crate::storage::fs::{FsRepositoryOps, MaybeOpenFsRepository}; use crate::storage::{TagNamespace, TagNamespaceBuf}; use crate::{Digest, Error, Result, encoding, graph, storage, tracking}; @@ -665,11 +666,19 @@ where /// This function should only be called once the discovery of all attached /// objects has completed successfully and with no errors. Otherwise, it may /// remove data that is still being used - async unsafe fn remove_unvisited_renders_and_proxies(&self) -> Result { + async unsafe fn remove_unvisited_renders_and_proxies_on_repo( + &self, + repo: &MaybeOpenFsRepository, + ) -> Result + where + RS: storage::DefaultRenderStoreCreationPolicy + + storage::RenderStoreForUser + + storage::TryRenderStore + + Send + + Sync + + 'static, + { let mut result = CleanResult::default(); - let storage::RepositoryHandle::FS(repo) = self.repo else { - return Ok(result); - }; let repo = repo.opened().await?; result += match self @@ -685,17 +694,6 @@ where let renders_for_all_users = repo.renders_for_all_users()?; for (username, sub_repo) in renders_for_all_users.iter() { - // Some users are missing a "renders//proxy" subdirectory, - // making `get_render_storage` return `Err(NoRenderStorage)`, - // therefore failing the whole clean attempt before any work is - // performed. The missing proxy directory is likely a symptom of - // some other problem elsewhere. - if !sub_repo.has_renders() { - #[cfg(feature = "sentry")] - tracing::error!(target: "sentry", %username, "Skipping clean of user's renders (NoRenderStorage)"); - continue; - } - result += self .remove_unvisited_renders_and_proxies_for_storage(Some(username.clone()), sub_repo) .await?; @@ -703,6 +701,28 @@ where Ok(result) } + /// # Safety + /// This function should only be called once the discovery of all attached + /// objects has completed successfully and with no errors. Otherwise, it may + /// remove data that is still being used + async unsafe fn remove_unvisited_renders_and_proxies(&self) -> Result { + match self.repo { + storage::RepositoryHandle::FSWithMaybeRenders(repo) => unsafe { + // Convert this repo into one that will not create renders + // on demand. We only want to clean existing renders. + let repo = repo.clone().without_render_creation(); + + self.remove_unvisited_renders_and_proxies_on_repo(&repo) + .await + }, + storage::RepositoryHandle::FSWithRenders(repo) => unsafe { + self.remove_unvisited_renders_and_proxies_on_repo(repo) + .await + }, + _ => Ok(CleanResult::default()), + } + } + async fn remove_unvisited_renders_and_proxies_for_storage( &self, username: Option, @@ -745,7 +765,7 @@ where drop(stream); if let Some(proxy_path) = repo.proxy_path() { - result += self.clean_proxies(username, proxy_path.to_owned()).await?; + result += self.clean_proxies(username, &proxy_path).await?; } Ok(result) } @@ -755,7 +775,7 @@ where async fn clean_proxies( &self, username: Option, - proxy_path: std::path::PathBuf, + proxy_path: &Path, ) -> Result { let mut result = CleanResult::default(); let removed = result.removed_proxies.entry(username).or_default(); @@ -812,8 +832,10 @@ where let future = async move { if !self.dry_run { tracing::trace!(?path, "removing proxy render"); - storage::fs::OpenFsRepository::remove_dir_atomically(&path, &workdir) - .await?; + storage::fs::OpenFsRepository::::remove_dir_atomically( + &path, &workdir, + ) + .await?; } Ok(digest) }; diff --git a/crates/spfs/src/clean_test.rs b/crates/spfs/src/clean_test.rs index 151a5e5f6b..ad1fbcb88c 100644 --- a/crates/spfs/src/clean_test.rs +++ b/crates/spfs/src/clean_test.rs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 // https://github.com/spkenv/spk -use std::sync::Arc; use std::time::Duration; use chrono::Utc; @@ -13,6 +12,13 @@ use tokio::time::sleep; use super::{Cleaner, TracingCleanReporter}; use crate::encoding::prelude::*; use crate::fixtures::*; +use crate::storage::fs::MaybeOpenFsRepository; +use crate::storage::{ + DefaultRenderStoreCreationPolicy, + LocalRenderStore, + RenderStoreForUser, + TryRenderStore, +}; use crate::{Error, storage, tracking}; #[rstest] @@ -82,8 +88,16 @@ async fn test_get_attached_unattached_objects_blob( } #[rstest] +#[case::fs_with_renders(tmprepo("fs-with-renders"))] +#[case::fs_with_maybe_renders(tmprepo("fs-with-maybe-renders"))] +#[case::fs_without_renders(tmprepo("fs-without-renders"))] #[tokio::test] -async fn test_clean_untagged_objects(#[future] tmprepo: TempRepo, tmpdir: tempfile::TempDir) { +async fn test_clean_untagged_objects( + #[case] + #[future] + tmprepo: TempRepo, + tmpdir: tempfile::TempDir, +) { init_logging(); let tmprepo = tmprepo.await; @@ -414,8 +428,15 @@ async fn test_clean_on_repo_with_tag_namespace_set( } #[rstest] +#[case::fs_with_renders(tmprepo("fs-with-renders"))] +#[case::fs_with_maybe_renders(tmprepo("fs-with-maybe-renders"))] +#[case::fs_without_renders(tmprepo("fs-without-renders"))] #[tokio::test] -async fn test_clean_untagged_objects_layers_platforms(#[future] tmprepo: TempRepo) { +async fn test_clean_untagged_objects_layers_platforms( + #[case] + #[future] + tmprepo: TempRepo, +) { init_logging(); let tmprepo = tmprepo.await; let manifest = tracking::Manifest::<()>::default(); @@ -449,21 +470,26 @@ async fn test_clean_untagged_objects_layers_platforms(#[future] tmprepo: TempRep } #[rstest] +#[case::fs_with_renders(tmprepo("fs-with-renders"))] +#[case::fs_with_maybe_renders(tmprepo("fs-with-maybe-renders"))] #[tokio::test] -async fn test_clean_manifest_renders(tmpdir: tempfile::TempDir) { +async fn test_clean_manifest_renders( + #[case] + #[future] + tmprepo: TempRepo, +) { + use crate::storage::fs::RenderStore; + init_logging(); - let tmprepo = Arc::new( - storage::fs::MaybeOpenFsRepository::create(tmpdir.path()) - .await - .unwrap() - .into(), - ); + let TempRepo::FS(tmprepo, tmpdir) = &tmprepo.await else { + panic!("unexpected tmprepo type"); + }; let data_dir = tmpdir.path().join("data"); ensure(data_dir.join("dir/dir/file.txt"), "hello"); ensure(data_dir.join("dir/name.txt"), "john doe"); - let manifest = crate::Committer::new(&tmprepo) + let manifest = crate::Committer::new(tmprepo) .commit_dir(data_dir.as_path()) .await .unwrap(); @@ -476,10 +502,38 @@ async fn test_clean_manifest_renders(tmpdir: tempfile::TempDir) { .await .unwrap(); - let fs_repo = match &*tmprepo { - RepositoryHandle::FS(fs) => fs, + match &**tmprepo { + RepositoryHandle::FSWithRenders(fs) => { + test_clean_manifest_renders_with_repo(tmprepo, fs, manifest).await + } + RepositoryHandle::FSWithMaybeRenders(fs) => { + let repo_with_render_store: MaybeOpenFsRepository = fs + .clone() + .try_into() + .expect("render store created successfully"); + let handle: RepositoryHandle = RepositoryHandle::FSWithRenders(repo_with_render_store); + let RepositoryHandle::FSWithRenders(borrow) = &handle else { + unreachable!() + }; + test_clean_manifest_renders_with_repo(&handle, borrow, manifest).await + } _ => panic!("Unexpected tmprepo type!"), }; +} + +async fn test_clean_manifest_renders_with_repo( + tmprepo: &RepositoryHandle, + fs_repo: &MaybeOpenFsRepository, + manifest: tracking::Manifest, +) where + RS: LocalRenderStore + + TryRenderStore + + DefaultRenderStoreCreationPolicy + + RenderStoreForUser + + Send + + Sync + + 'static, +{ let fs_repo = fs_repo.opened().await.unwrap(); storage::fs::Renderer::new(&fs_repo) @@ -490,14 +544,14 @@ async fn test_clean_manifest_renders(tmpdir: tempfile::TempDir) { let files = list_files(fs_repo.objects.root()); assert!(!files.is_empty(), "should have stored data"); - let cleaner = Cleaner::new(&tmprepo).with_reporter(TracingCleanReporter); + let cleaner = Cleaner::new(tmprepo).with_reporter(TracingCleanReporter); let result = cleaner .prune_all_tags_and_clean() .await .expect("failed to clean repo"); println!("{result:#?}"); - let files = list_files(fs_repo.renders.as_ref().unwrap().renders.root()); + let files = list_files(fs_repo.rs_impl.render_store().renders.root()); assert_eq!( files, Vec::::new(), diff --git a/crates/spfs/src/commit_test.rs b/crates/spfs/src/commit_test.rs index b6167246b6..b49329e618 100644 --- a/crates/spfs/src/commit_test.rs +++ b/crates/spfs/src/commit_test.rs @@ -7,19 +7,20 @@ use rstest::rstest; use super::Committer; use crate::Error; use crate::fixtures::*; +use crate::storage::fs::RenderStore; #[rstest] #[tokio::test] async fn test_commit_empty(tmpdir: tempfile::TempDir) { let root = tmpdir.path().to_string_lossy().to_string(); let repo = crate::storage::RepositoryHandle::from( - crate::storage::fs::MaybeOpenFsRepository::create(&root) + crate::storage::fs::MaybeOpenFsRepository::::create(&root) .await .unwrap(), ); let storage = crate::runtime::Storage::new(repo).unwrap(); let repo = crate::storage::RepositoryHandle::from( - crate::storage::fs::MaybeOpenFsRepository::create(root) + crate::storage::fs::MaybeOpenFsRepository::::create(root) .await .unwrap(), ); diff --git a/crates/spfs/src/config.rs b/crates/spfs/src/config.rs index 2d4e82d585..9e26883ede 100644 --- a/crates/spfs/src/config.rs +++ b/crates/spfs/src/config.rs @@ -328,22 +328,29 @@ impl RemoteConfig { inner, } = self; let mut handle: storage::RepositoryHandle = match inner.clone() { - RepositoryConfig::Fs(config) => storage::fs::MaybeOpenFsRepository::from_config(config) + // Remote repositories never have a render store (from our + // perspective). + RepositoryConfig::Fs(mut config) => { + config.params.create_renders = false; + storage::fs::MaybeOpenFsRepository::::from_config( + config, + ) .await? - .into(), - RepositoryConfig::Tar(config) => storage::tar::TarRepository::from_config(config) - .await? - .into(), - RepositoryConfig::Grpc(config) => storage::rpc::RpcRepository::from_config(config) - .await? - .into(), - RepositoryConfig::Proxy(config) => storage::proxy::ProxyRepository::from_config(config) - .await? - .into(), + } + RepositoryConfig::Tar(config) => { + storage::tar::TarRepository::from_config(config).await? + } + RepositoryConfig::Grpc(config) => { + storage::rpc::RpcRepository::from_config(config).await? + } + RepositoryConfig::Proxy(config) => { + storage::proxy::ProxyRepository::from_config(config).await? + } RepositoryConfig::Fallback(config) => { - storage::fallback::FallbackProxy::from_config(config) - .await? - .into() + storage::fallback::FallbackProxy::::from_config( + config, + ) + .await? } }; // Set tag namespace first before pinning, because it is not possible @@ -692,7 +699,12 @@ impl Config { } /// Get the local repository instance as configured, creating it if needed. - pub async fn get_opened_local_repository(&self) -> Result { + pub async fn get_opened_local_repository(&self) -> Result> + where + RS: storage::DefaultRenderStoreCreationPolicy + + storage::RenderStoreForUser + + Clone, + { // Possibly use a different path for the local repository, depending // on enabled features. #[allow(unused_mut)] @@ -704,7 +716,7 @@ impl Config { Some(self.storage.root.join("ci").join(format!("pipeline_{id}"))); } - let mut local_repo = storage::fs::OpenFsRepository::create( + let mut local_repo = storage::fs::OpenFsRepository::::create( use_ci_isolated_storage_path .as_ref() .unwrap_or(&self.storage.root), @@ -725,16 +737,24 @@ impl Config { /// /// The returned repo is guaranteed to be created, valid and open already. Ie /// the local repository is not allowed to be lazily opened. - pub async fn get_local_repository(&self) -> Result { + pub async fn get_local_repository(&self) -> Result> + where + RS: storage::DefaultRenderStoreCreationPolicy + + storage::RenderStoreForUser + + Clone, + { self.get_opened_local_repository().await } - /// Get the local repository handle as configured, creating it if needed. + /// Get the local repository handle as configured, creating it if needed. /// /// The returned repo is guaranteed to be created, valid and open already. Ie /// the local repository is not allowed to be lazily opened. pub async fn get_local_repository_handle(&self) -> Result { - Ok(self.get_local_repository().await?.into()) + Ok(self + .get_local_repository::() + .await? + .into()) } /// Get a remote repository by name, or the local repository. @@ -750,14 +770,18 @@ impl Config { { match name { Some(name) => self.get_remote(name).await, - None => Ok(self.get_local_repository().await?.into()), + None => Ok(self + .get_local_repository::() + .await? + .into()), } } /// Get the local runtime storage, as configured. pub async fn get_runtime_storage(&self) -> Result { runtime::Storage::new(storage::RepositoryHandle::from( - self.get_local_repository().await?, + self.get_local_repository::() + .await?, )) } diff --git a/crates/spfs/src/config_test.rs b/crates/spfs/src/config_test.rs index 13276ac562..ce22b50ebd 100644 --- a/crates/spfs/src/config_test.rs +++ b/crates/spfs/src/config_test.rs @@ -6,6 +6,7 @@ use rstest::rstest; use super::{Config, Remote, RemoteConfig, RepositoryConfig}; use crate::storage::RepositoryHandle; +use crate::storage::fs::NoRenderStore; use crate::storage::prelude::*; use crate::{get_config, load_config, reset_config}; @@ -41,7 +42,7 @@ async fn test_config_get_remote() { .tempdir() .unwrap(); let remote = tmpdir.path().join("remote"); - let _ = crate::storage::fs::MaybeOpenFsRepository::create(&remote) + let _ = crate::storage::fs::MaybeOpenFsRepository::::create(&remote) .await .unwrap(); diff --git a/crates/spfs/src/fixtures.rs b/crates/spfs/src/fixtures.rs index c537067066..989e307372 100644 --- a/crates/spfs/src/fixtures.rs +++ b/crates/spfs/src/fixtures.rs @@ -10,6 +10,7 @@ use rstest::fixture; use tempfile::TempDir; use crate as spfs; +use crate::storage::fs::{MaybeRenderStore, NoRenderStore, RenderStore}; pub enum TempRepo { FS(Arc, Arc), @@ -39,13 +40,14 @@ impl TempRepo { { match self { TempRepo::FS(_, tempdir) => { - let repo = spfs::storage::fs::MaybeOpenFsRepository { + let repo = spfs::storage::fs::MaybeOpenFsRepository:: { fs_impl: { - let mut fs_impl = spfs::storage::fs::MaybeOpenFsRepositoryImpl::open( - tempdir.path().join("repo"), - ) - .await - .unwrap(); + let mut fs_impl = + spfs::storage::fs::MaybeOpenFsRepositoryImpl::::open( + tempdir.path().join("repo"), + ) + .await + .unwrap(); fs_impl.set_tag_namespace(Some( spfs::storage::TagNamespaceBuf::new(namespace.as_ref()) .expect("tag namespaces used in tests must be valid"), @@ -132,16 +134,37 @@ pub fn tmpdir() -> TempDir { .expect("failed to create dir for test") } -#[fixture(kind = "fs")] +#[fixture(kind = "fs-with-renders")] pub async fn tmprepo(kind: &str) -> TempRepo { init_logging(); let tmpdir = tmpdir(); match kind { - "fs" => { - let repo = spfs::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("repo")) - .await - .unwrap() - .into(); + // "fs" was the old name for this kind + "fs" | "fs-with-renders" => { + let repo = spfs::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("repo"), + ) + .await + .unwrap() + .into(); + TempRepo::FS(Arc::new(repo), Arc::new(tmpdir)) + } + "fs-with-maybe-renders" => { + let repo = spfs::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("repo"), + ) + .await + .unwrap() + .into(); + TempRepo::FS(Arc::new(repo), Arc::new(tmpdir)) + } + "fs-without-renders" => { + let repo = spfs::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("repo"), + ) + .await + .unwrap() + .into(); TempRepo::FS(Arc::new(repo), Arc::new(tmpdir)) } "tar" => { @@ -154,10 +177,12 @@ pub async fn tmprepo(kind: &str) -> TempRepo { #[cfg(feature = "server")] "rpc" => { use crate::storage::prelude::*; - let repo = std::sync::Arc::new(spfs::storage::RepositoryHandle::FS( - spfs::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("repo")) - .await - .unwrap(), + let repo = std::sync::Arc::new(spfs::storage::RepositoryHandle::FSWithRenders( + spfs::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("repo"), + ) + .await + .unwrap(), )); let listen: std::net::SocketAddr = "127.0.0.1:0".parse().unwrap(); let http_listener = tokio::net::TcpListener::bind(listen).await.unwrap(); diff --git a/crates/spfs/src/resolve.rs b/crates/spfs/src/resolve.rs index 429d4af78a..778b583a61 100644 --- a/crates/spfs/src/resolve.rs +++ b/crates/spfs/src/resolve.rs @@ -5,6 +5,7 @@ #[cfg(unix)] use std::collections::HashSet; use std::path::{Path, PathBuf}; +use std::sync::Arc; use futures::{FutureExt, TryFutureExt, TryStreamExt}; use itertools::Itertools; @@ -97,8 +98,12 @@ async fn render_via_subcommand( /// Compute or load the spfs manifest representation for a saved reference. pub async fn compute_manifest>(reference: R) -> Result { let config = get_config()?; - let mut repos: Vec = - vec![config.get_local_repository().await?.into()]; + let mut repos: Vec = vec![ + config + .get_local_repository::() + .await? + .into(), + ]; for name in config.list_remote_names() { match config.get_remote(&name).await { Ok(repo) => repos.push(repo), @@ -365,13 +370,24 @@ where /// the `flattened_layers` property is modified. Only pass true here if the /// runtime is unconditionally saved shortly after calling this function. #[cfg(unix)] -pub(crate) async fn resolve_and_render_overlay_dirs( +pub(crate) async fn resolve_and_render_overlay_dirs( runtime: &mut runtime::Runtime, skip_runtime_save: bool, -) -> Result { +) -> Result +where + Arc>: Into + ManifestRenderPath, + RS: storage::DefaultRenderStoreCreationPolicy + + storage::LocalRenderStore + + Clone + + std::fmt::Debug + + Send + + Sync, +{ let config = get_config()?; - let (repo, remotes) = - tokio::try_join!(config.get_opened_local_repository(), config.list_remotes())?; + let (repo, remotes) = tokio::try_join!( + config.get_opened_local_repository::(), + config.list_remotes() + )?; let fallback_repo = FallbackProxy::new( repo, remotes, // preserve original behavior of not looking for tags in the secondary @@ -414,15 +430,33 @@ pub async fn resolve_stack_to_layers( Some(repo) => repo, None => { let config = get_config()?; - owned_handle = storage::RepositoryHandle::from(config.get_local_repository().await?); + owned_handle = storage::RepositoryHandle::from( + config + .get_local_repository::() + .await?, + ); &owned_handle } }; match repo { - storage::RepositoryHandle::FS(r) => resolve_stack_to_layers_with_repo(stack, r).await, + storage::RepositoryHandle::FSWithMaybeRenders(r) => { + resolve_stack_to_layers_with_repo(stack, r).await + } + storage::RepositoryHandle::FSWithRenders(r) => { + resolve_stack_to_layers_with_repo(stack, r).await + } + storage::RepositoryHandle::FSWithoutRenders(r) => { + resolve_stack_to_layers_with_repo(stack, r).await + } storage::RepositoryHandle::Tar(r) => resolve_stack_to_layers_with_repo(stack, r).await, storage::RepositoryHandle::Rpc(r) => resolve_stack_to_layers_with_repo(stack, r).await, - storage::RepositoryHandle::FallbackProxy(r) => { + storage::RepositoryHandle::FallbackProxyWithMaybeRenders(r) => { + resolve_stack_to_layers_with_repo(stack, &**r).await + } + storage::RepositoryHandle::FallbackProxyWithRenders(r) => { + resolve_stack_to_layers_with_repo(stack, &**r).await + } + storage::RepositoryHandle::FallbackProxyWithoutRenders(r) => { resolve_stack_to_layers_with_repo(stack, &**r).await } storage::RepositoryHandle::Proxy(r) => resolve_stack_to_layers_with_repo(stack, &**r).await, diff --git a/crates/spfs/src/resolve_test.rs b/crates/spfs/src/resolve_test.rs index e3fa4088a9..7c098dcd7d 100644 --- a/crates/spfs/src/resolve_test.rs +++ b/crates/spfs/src/resolve_test.rs @@ -10,11 +10,12 @@ use rstest::rstest; use super::resolve_stack_to_layers; use crate::fixtures::*; #[cfg(unix)] -use crate::io; -#[cfg(unix)] use crate::io::DigestFormat; use crate::prelude::*; -use crate::{encoding, graph}; +use crate::storage::fallback::FallbackProxy; +use crate::storage::fs::RenderStore; +use crate::storage::{RepositoryHandle, fs}; +use crate::{encoding, graph, io}; #[rstest] #[tokio::test] @@ -30,6 +31,88 @@ async fn test_stack_to_layers_dedupe(#[future] tmprepo: TempRepo) { assert_eq!(resolved.len(), 1, "should deduplicate layers in resolve"); } +#[rstest] +#[tokio::test] +async fn test_stack_to_layers_dedupe_all_repo_handle_variants(tmpdir: tempfile::TempDir) { + fn make_test_stack() -> (graph::Layer, graph::Platform, graph::Stack) { + let layer = graph::Layer::new(encoding::EMPTY_DIGEST.into()); + let platform = graph::Platform::from_digestible([&layer, &layer]).unwrap(); + let mut stack = graph::Stack::from_digestible([&layer]).unwrap(); + stack.push(platform.digest().unwrap()); + (layer, platform, stack) + } + + async fn assert_dedupe(handle: &RepositoryHandle) { + let (layer, platform, stack) = make_test_stack(); + handle.write_object(&layer).await.unwrap(); + handle.write_object(&platform).await.unwrap(); + let resolved = resolve_stack_to_layers(&stack, Some(handle)).await.unwrap(); + assert_eq!(resolved.len(), 1, "should deduplicate layers in resolve"); + } + + let repo_root = |name: &str| tmpdir.path().join(name); + + let fs_with_maybe: RepositoryHandle = + fs::MaybeOpenFsRepository::::create(repo_root("fs-with-maybe")) + .await + .unwrap() + .into(); + assert_dedupe(&fs_with_maybe).await; + + let fs_with_renders: RepositoryHandle = + fs::MaybeOpenFsRepository::::create(repo_root("fs-with-renders")) + .await + .unwrap() + .into(); + assert_dedupe(&fs_with_renders).await; + + let fs_without_renders: RepositoryHandle = + fs::MaybeOpenFsRepository::::create(repo_root("fs-without-renders")) + .await + .unwrap() + .into(); + assert_dedupe(&fs_without_renders).await; + + let fallback_with_maybe: RepositoryHandle = FallbackProxy::::new( + fs::MaybeOpenFsRepository::::create(repo_root("fallback-maybe")) + .await + .unwrap() + .opened() + .await + .unwrap(), + vec![], + false, + ) + .into(); + assert_dedupe(&fallback_with_maybe).await; + + let fallback_with_renders: RepositoryHandle = FallbackProxy::::new( + fs::MaybeOpenFsRepository::::create(repo_root("fallback-render")) + .await + .unwrap() + .opened() + .await + .unwrap(), + vec![], + false, + ) + .into(); + assert_dedupe(&fallback_with_renders).await; + + let fallback_without_renders: RepositoryHandle = FallbackProxy::::new( + fs::MaybeOpenFsRepository::::create(repo_root("fallback-none")) + .await + .unwrap() + .opened() + .await + .unwrap(), + vec![], + false, + ) + .into(); + assert_dedupe(&fallback_without_renders).await; +} + // `resolve_overlay_dirs` only exists on unix #[cfg(unix)] /// Test that if there are too many layers to fit on a single mount @@ -42,7 +125,7 @@ async fn test_auto_merge_layers(tmpdir: tempfile::TempDir) { // This test must use the "local" repository for spfs-render to succeed. let config = crate::get_config().expect("get config"); let fs_repo = config - .get_opened_local_repository() + .get_opened_local_repository::() .await .expect("open local repository"); let repo = Arc::new(fs_repo.clone().into()); @@ -100,7 +183,7 @@ async fn test_auto_merge_layers_with_edit(tmpdir: tempfile::TempDir) { // This test must use the "local" repository for spfs-render to succeed. let config = crate::get_config().expect("get config"); let fs_repo = config - .get_opened_local_repository() + .get_opened_local_repository::() .await .expect("open local repository"); let repo = Arc::new(fs_repo.clone().into()); diff --git a/crates/spfs/src/runtime/storage.rs b/crates/spfs/src/runtime/storage.rs index d036ccee8a..21b724d9c5 100644 --- a/crates/spfs/src/runtime/storage.rs +++ b/crates/spfs/src/runtime/storage.rs @@ -1187,12 +1187,23 @@ impl Storage { } pub async fn durable_path(&self, name: String) -> Result { - match &*self.inner { - RepositoryHandle::FS(repo) => { - let mut upper_root_path = repo.root(); + macro_rules! fs_upper_path { + ($repo:expr, $name:expr) => {{ + let mut upper_root_path = $repo.root(); upper_root_path.push(DURABLE_EDITS_DIR); - upper_root_path.push(name); + upper_root_path.push($name); Ok(upper_root_path) + }}; + } + match &*self.inner { + RepositoryHandle::FSWithMaybeRenders(repo) => { + fs_upper_path!(repo, name) + } + RepositoryHandle::FSWithRenders(repo) => { + fs_upper_path!(repo, name) + } + RepositoryHandle::FSWithoutRenders(repo) => { + fs_upper_path!(repo, name) } _ => Err(Error::DoesNotSupportDurableRuntimePath), } diff --git a/crates/spfs/src/runtime/storage_test.rs b/crates/spfs/src/runtime/storage_test.rs index add11be58f..850b55edda 100644 --- a/crates/spfs/src/runtime/storage_test.rs +++ b/crates/spfs/src/runtime/storage_test.rs @@ -16,6 +16,7 @@ use crate::fixtures::*; use crate::graph::object::{DigestStrategy, EncodingFormat}; use crate::graph::{AnnotationValue, Layer, Platform}; use crate::runtime::{BindMount, KeyValuePair, LiveLayer, LiveLayerContents, SpecApiVersion}; +use crate::storage::fs::RenderStore; use crate::storage::prelude::DatabaseExt; use crate::{Config, encoding, reset_config_async}; @@ -35,7 +36,7 @@ fn test_config_serialization() { async fn test_storage_create_runtime(tmpdir: tempfile::TempDir) { let root = tmpdir.path().to_string_lossy().to_string(); let repo = crate::storage::RepositoryHandle::from( - crate::storage::fs::MaybeOpenFsRepository::create(root) + crate::storage::fs::MaybeOpenFsRepository::::create(root) .await .unwrap(), ); @@ -76,7 +77,7 @@ async fn test_storage_runtime_with_annotation( let root = tmpdir.path().to_string_lossy().to_string(); let repo = crate::storage::RepositoryHandle::from( - crate::storage::fs::MaybeOpenFsRepository::create(root) + crate::storage::fs::MaybeOpenFsRepository::::create(root) .await .unwrap(), ); @@ -144,7 +145,7 @@ async fn test_storage_runtime_add_annotations_list( let root = tmpdir.path().to_string_lossy().to_string(); let repo = crate::storage::RepositoryHandle::from( - crate::storage::fs::MaybeOpenFsRepository::create(root) + crate::storage::fs::MaybeOpenFsRepository::::create(root) .await .unwrap(), ); @@ -219,7 +220,7 @@ async fn test_storage_runtime_with_nested_annotation( // Setup the objects needed for the runtime used in the test let root = tmpdir.path().to_string_lossy().to_string(); let repo = crate::storage::RepositoryHandle::from( - crate::storage::fs::MaybeOpenFsRepository::create(root) + crate::storage::fs::MaybeOpenFsRepository::::create(root) .await .unwrap(), ); @@ -290,7 +291,7 @@ async fn test_storage_runtime_with_annotation_all( let root = tmpdir.path().to_string_lossy().to_string(); let repo = crate::storage::RepositoryHandle::from( - crate::storage::fs::MaybeOpenFsRepository::create(root) + crate::storage::fs::MaybeOpenFsRepository::::create(root) .await .unwrap(), ); @@ -366,7 +367,7 @@ async fn test_storage_runtime_with_nested_annotation_all( // setup the objects needed for the runtime used in the test let root = tmpdir.path().to_string_lossy().to_string(); let repo = crate::storage::RepositoryHandle::from( - crate::storage::fs::MaybeOpenFsRepository::create(root) + crate::storage::fs::MaybeOpenFsRepository::::create(root) .await .unwrap(), ); @@ -443,7 +444,7 @@ async fn test_storage_runtime_with_nested_annotation_all( async fn test_storage_remove_runtime(tmpdir: tempfile::TempDir) { let root = tmpdir.path().to_string_lossy().to_string(); let repo = crate::storage::RepositoryHandle::from( - crate::storage::fs::MaybeOpenFsRepository::create(root) + crate::storage::fs::MaybeOpenFsRepository::::create(root) .await .unwrap(), ); @@ -464,7 +465,7 @@ async fn test_storage_remove_runtime(tmpdir: tempfile::TempDir) { async fn test_storage_iter_runtimes(tmpdir: tempfile::TempDir) { let root = tmpdir.path().to_string_lossy().to_string(); let repo = crate::storage::RepositoryHandle::from( - crate::storage::fs::MaybeOpenFsRepository::create(root) + crate::storage::fs::MaybeOpenFsRepository::::create(root) .await .unwrap(), ); @@ -516,7 +517,7 @@ async fn test_storage_iter_runtimes(tmpdir: tempfile::TempDir) { async fn test_runtime_reset(tmpdir: tempfile::TempDir) { let root = tmpdir.path().to_string_lossy().to_string(); let repo = crate::storage::RepositoryHandle::from( - crate::storage::fs::MaybeOpenFsRepository::create(root) + crate::storage::fs::MaybeOpenFsRepository::::create(root) .await .unwrap(), ); @@ -563,7 +564,7 @@ async fn test_runtime_reset(tmpdir: tempfile::TempDir) { async fn test_runtime_ensure_extra_bind_mount_locations_exist(tmpdir: tempfile::TempDir) { let root = tmpdir.path().to_string_lossy().to_string(); let repo = crate::storage::RepositoryHandle::from( - crate::storage::fs::MaybeOpenFsRepository::create(root) + crate::storage::fs::MaybeOpenFsRepository::::create(root) .await .unwrap(), ); diff --git a/crates/spfs/src/status.rs b/crates/spfs/src/status.rs index e590154d87..7e51059ff0 100644 --- a/crates/spfs/src/status.rs +++ b/crates/spfs/src/status.rs @@ -62,7 +62,6 @@ pub async fn get_runtime_backing_repo( repository: RUNTIME_REPO_NAME.into(), source, }) - .map(Into::into) } } diff --git a/crates/spfs/src/status_unix.rs b/crates/spfs/src/status_unix.rs index 026ca8f010..040500c81c 100644 --- a/crates/spfs/src/status_unix.rs +++ b/crates/spfs/src/status_unix.rs @@ -4,7 +4,7 @@ use crate::config::OverlayFsOptions; use crate::resolve::{RenderResult, resolve_and_render_overlay_dirs}; -use crate::storage::fs::RenderSummary; +use crate::storage::fs::{RenderStore, RenderSummary}; use crate::{Error, Result, bootstrap, env, runtime}; /// Remount the given runtime as configured. @@ -112,7 +112,7 @@ pub async unsafe fn change_to_durable_runtime( // remount the overlayfs only, using its new durable path settings let render_result = match rt.config.mount_backend { runtime::MountBackend::OverlayFsWithRenders => { - resolve_and_render_overlay_dirs(rt, false).await? + resolve_and_render_overlay_dirs::(rt, false).await? } runtime::MountBackend::OverlayFsWithFuse | runtime::MountBackend::FuseOnly @@ -146,7 +146,7 @@ pub async unsafe fn reinitialize_runtime( ) -> Result { let render_result = match rt.config.mount_backend { runtime::MountBackend::OverlayFsWithRenders => { - resolve_and_render_overlay_dirs(rt, false).await? + resolve_and_render_overlay_dirs::(rt, false).await? } runtime::MountBackend::OverlayFsWithFuse | runtime::MountBackend::FuseOnly @@ -219,7 +219,7 @@ pub async fn initialize_runtime( let render_result = match rt.config.mount_backend { runtime::MountBackend::OverlayFsWithRenders => { - resolve_and_render_overlay_dirs( + resolve_and_render_overlay_dirs::( rt, // skip saving the runtime in this step because we will save it after // learning the mount namespace below diff --git a/crates/spfs/src/storage/config.rs b/crates/spfs/src/storage/config.rs index 11a905fc3a..96980a61c2 100644 --- a/crates/spfs/src/storage/config.rs +++ b/crates/spfs/src/storage/config.rs @@ -4,7 +4,7 @@ use async_trait::async_trait; -use super::OpenRepositoryError; +use super::{OpenRepositoryError, RepositoryHandle}; pub type OpenRepositoryResult = std::result::Result; @@ -20,16 +20,5 @@ pub trait FromUrl: Sized { pub trait FromConfig: Sized { type Config: FromUrl + Send; - async fn from_config(config: Self::Config) -> OpenRepositoryResult; -} - -#[async_trait] -impl FromUrl for T -where - T: FromConfig + Send + Sync + Sized, -{ - async fn from_url(url: &url::Url) -> OpenRepositoryResult { - let config = T::Config::from_url(url).await?; - Self::from_config(config).await - } + async fn from_config(config: Self::Config) -> OpenRepositoryResult; } diff --git a/crates/spfs/src/storage/error.rs b/crates/spfs/src/storage/error.rs index 5fddde2020..8027e8d3a1 100644 --- a/crates/spfs/src/storage/error.rs +++ b/crates/spfs/src/storage/error.rs @@ -21,6 +21,10 @@ pub enum OpenRepositoryError { source: std::io::Error, }, + #[error("Render store not available")] + #[diagnostic(code("spfs::storage::fs::no_render_store"))] + RenderStorageUnavailable, + #[error("Could not validate repository version")] FsMigration(#[from] super::fs::migrations::MigrationError), @@ -105,6 +109,9 @@ pub enum OpenRepositoryError { #[error("Unsupported repository type: {0}")] UnsupportedRepositoryType(String), + + #[error("Invalid render store creation policy: {0}")] + InvalidRenderStoreCreationPolicy(String), } impl OpenRepositoryError { diff --git a/crates/spfs/src/storage/fallback/repository.rs b/crates/spfs/src/storage/fallback/repository.rs index b18564ce46..dcc3e1de65 100644 --- a/crates/spfs/src/storage/fallback/repository.rs +++ b/crates/spfs/src/storage/fallback/repository.rs @@ -13,17 +13,28 @@ use relative_path::RelativePath; use crate::config::{ToAddress, default_fallback_repo_include_secondary_tags}; use crate::graph::ObjectProto; use crate::prelude::*; -use crate::storage::fs::{FsHashStore, ManifestRenderPath, OpenFsRepository, RenderStore}; +use crate::storage::fs::{ + FsHashStore, + ManifestRenderPath, + MaybeRenderStore, + NoRenderStore, + OpenFsRepository, + RenderStore, + RenderStoreCreationPolicy, +}; use crate::storage::proxy::ProxyRepositoryExt; use crate::storage::tag::TagSpecAndTagStream; use crate::storage::{ EntryType, - LocalRepository, + LocalPayloads, + LocalRenderStore, OpenRepositoryError, OpenRepositoryResult, + RenderStoreForUser, TagNamespace, TagNamespaceBuf, TagStorageMut, + TryRenderStore, }; use crate::sync::reporter::SyncReporters; use crate::tracking::BlobRead; @@ -76,25 +87,19 @@ impl storage::FromUrl for Config { /// payloads are copied into the primary repository. Missing blobs are also /// repaired in the same way. #[derive(Debug)] -pub struct FallbackProxy { +pub struct FallbackProxy { // Why isn't this a RepositoryHandle? // - // It needs to be something that implements LocalRepository so this + // It needs to be something that implements LocalPayloads so this // struct can implement it too. RepositoryHandle can't implement that // trait. - primary: Arc, + primary: Arc>, secondary: Vec, include_secondary_tags: bool, } -impl FallbackProxy { - pub fn into_stack(self) -> Vec { - let mut stack = vec![self.primary.into()]; - stack.extend(self.secondary); - stack - } - - pub fn new>>( +impl FallbackProxy { + pub fn new>>>( primary: P, secondary: Vec, include_secondary_tags: bool, @@ -107,38 +112,72 @@ impl FallbackProxy { } } +impl FallbackProxy +where + Arc>: Into, +{ + pub fn into_stack(self) -> Vec { + let mut stack = vec![self.primary.into()]; + stack.extend(self.secondary); + stack + } +} + #[async_trait::async_trait] -impl storage::FromConfig for FallbackProxy { +impl storage::FromConfig for FallbackProxy { type Config = Config; - async fn from_config(config: Self::Config) -> OpenRepositoryResult { + async fn from_config( + config: Self::Config, + ) -> OpenRepositoryResult { + enum PrimaryFsRepository { + Maybe(OpenFsRepository), + Render(OpenFsRepository), + None(OpenFsRepository), + } + let spfs_config = crate::Config::current().map_err(|source| OpenRepositoryError::FailedToLoadConfig { source: Box::new(source), })?; + let primary = async { - let primary = + let primary_handle = crate::config::open_repository_from_string(&spfs_config, Some(&config.primary)) .await .map_err(|source| OpenRepositoryError::FailedToOpenPartial { source: Box::new(source), })?; - let primary = match primary { - RepositoryHandle::FS(fs) => fs, - _ => { - return Err(OpenRepositoryError::UnsupportedRepositoryType( - "The primary repository of a FallbackProxy must be a filesystem repository" - .into(), - )); - } - }; - primary - .opened() - .await - .map_err(|source| OpenRepositoryError::FailedToOpenPartial { - source: Box::new(source), - }) + + match primary_handle { + RepositoryHandle::FSWithMaybeRenders(fs) => fs + .opened() + .await + .map(PrimaryFsRepository::Maybe) + .map_err(|source| OpenRepositoryError::FailedToOpenPartial { + source: Box::new(source), + }), + RepositoryHandle::FSWithRenders(fs) => fs + .opened() + .await + .map(PrimaryFsRepository::Render) + .map_err(|source| OpenRepositoryError::FailedToOpenPartial { + source: Box::new(source), + }), + RepositoryHandle::FSWithoutRenders(fs) => fs + .opened() + .await + .map(PrimaryFsRepository::None) + .map_err(|source| OpenRepositoryError::FailedToOpenPartial { + source: Box::new(source), + }), + _ => Err(OpenRepositoryError::UnsupportedRepositoryType( + "The primary repository of a FallbackProxy must be a filesystem repository" + .into(), + )), + } }; + let secondary = async { let mut secondary = Vec::with_capacity(config.secondary.len()); for name in config.secondary.iter() { @@ -147,7 +186,13 @@ impl storage::FromConfig for FallbackProxy { .map_err(|source| OpenRepositoryError::FailedToOpenPartial { source: Box::new(source), })? { - RepositoryHandle::FallbackProxy(proxy) => { + RepositoryHandle::FallbackProxyWithMaybeRenders(proxy) => { + secondary.extend(proxy.into_stack()); + } + RepositoryHandle::FallbackProxyWithRenders(proxy) => { + secondary.extend(proxy.into_stack()); + } + RepositoryHandle::FallbackProxyWithoutRenders(proxy) => { // Instead of nesting proxy repos, flatten them into // a single proxy repo with multiple secondaries. // This helps spfs-fuse handle the case where @@ -159,19 +204,39 @@ impl storage::FromConfig for FallbackProxy { repo => secondary.push(repo), }; } - Ok(secondary) + Ok::<_, OpenRepositoryError>(secondary) }; + let (primary, secondary) = tokio::try_join!(primary, secondary)?; - Ok(Self { - primary: primary.into(), - secondary, - include_secondary_tags: config.include_secondary_tags, + + Ok(match primary { + PrimaryFsRepository::Maybe(primary) => Self { + primary: primary.into(), + secondary, + include_secondary_tags: config.include_secondary_tags, + } + .into(), + PrimaryFsRepository::Render(primary) => FallbackProxy:: { + primary: primary.into(), + secondary, + include_secondary_tags: config.include_secondary_tags, + } + .into(), + PrimaryFsRepository::None(primary) => FallbackProxy:: { + primary: primary.into(), + secondary, + include_secondary_tags: config.include_secondary_tags, + } + .into(), }) } } #[async_trait::async_trait] -impl graph::DatabaseView for FallbackProxy { +impl graph::DatabaseView for FallbackProxy +where + RS: Send + Sync, +{ async fn has_object(&self, digest: encoding::Digest) -> bool { if self.primary.has_object(digest).await { return true; @@ -240,7 +305,10 @@ impl graph::DatabaseView for FallbackProxy { } #[async_trait::async_trait] -impl graph::Database for FallbackProxy { +impl graph::Database for FallbackProxy +where + RS: Send + Sync, +{ async fn remove_object(&self, digest: encoding::Digest) -> Result<()> { self.primary.remove_object(digest).await?; Ok(()) @@ -259,7 +327,10 @@ impl graph::Database for FallbackProxy { } #[async_trait::async_trait] -impl graph::DatabaseExt for FallbackProxy { +impl graph::DatabaseExt for FallbackProxy +where + RS: Send + Sync, +{ async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { self.primary.write_object(obj).await?; Ok(()) @@ -267,7 +338,11 @@ impl graph::DatabaseExt for FallbackProxy { } #[async_trait::async_trait] -impl PayloadStorage for FallbackProxy { +impl PayloadStorage for FallbackProxy +where + Arc>: Into, + RS: Send + Sync, +{ async fn has_payload(&self, digest: encoding::Digest) -> bool { if self.primary.has_payload(digest).await { return true; @@ -372,7 +447,10 @@ impl PayloadStorage for FallbackProxy { } } -impl ProxyRepositoryExt for FallbackProxy { +impl ProxyRepositoryExt for FallbackProxy +where + RS: Send + Sync, +{ #[inline] fn include_secondary_tags(&self) -> bool { self.include_secondary_tags @@ -390,7 +468,10 @@ impl ProxyRepositoryExt for FallbackProxy { } #[async_trait::async_trait] -impl TagStorage for FallbackProxy { +impl TagStorage for FallbackProxy +where + RS: Send + Sync, +{ #[inline] fn get_tag_namespace(&self) -> Option> { self.primary.get_tag_namespace() @@ -457,7 +538,10 @@ impl TagStorage for FallbackProxy { } } -impl TagStorageMut for FallbackProxy { +impl TagStorageMut for FallbackProxy +where + RS: Clone, +{ fn try_set_tag_namespace( &mut self, tag_namespace: Option, @@ -467,7 +551,7 @@ impl TagStorageMut for FallbackProxy { } } -impl Address for FallbackProxy { +impl Address for FallbackProxy { fn address(&self) -> Cow<'_, url::Url> { let config = Config { primary: self.primary.address().to_string(), @@ -486,20 +570,69 @@ impl Address for FallbackProxy { } } -impl LocalRepository for FallbackProxy { +impl LocalPayloads for FallbackProxy { #[inline] fn payloads(&self) -> &FsHashStore { self.primary.payloads() } +} +impl LocalRenderStore for FallbackProxy +where + RS: LocalRenderStore + RenderStoreForUser, +{ #[inline] - fn render_store(&self) -> Result<&RenderStore> { - self.primary.render_store() + fn render_store(&self) -> &RenderStore { + self.primary.rs_impl.render_store() } } -impl ManifestRenderPath for FallbackProxy { +impl ManifestRenderPath for FallbackProxy +where + Arc>: ManifestRenderPath, +{ fn manifest_render_path(&self, manifest: &graph::Manifest) -> Result { self.primary.manifest_render_path(manifest) } } + +impl RenderStoreForUser for FallbackProxy +where + RS: RenderStoreForUser, +{ + type RenderStore = RS; + + fn render_store_for_user( + creation_policy: RenderStoreCreationPolicy, + url: url::Url, + root: &std::path::Path, + username: &std::path::Path, + ) -> OpenRepositoryResult { + RS::render_store_for_user(creation_policy, url, root, username) + } +} + +impl TryRenderStore for FallbackProxy +where + RS: TryRenderStore, +{ + fn try_render_store(&self) -> OpenRepositoryResult> { + self.primary.fs_impl.rs_impl.try_render_store() + } + + fn proxy_path(&self) -> Option> { + self.primary.fs_impl.rs_impl.proxy_path() + } +} + +impl TryFrom> for FallbackProxy { + type Error = OpenRepositoryError; + + fn try_from(value: FallbackProxy) -> OpenRepositoryResult { + Ok(Self { + primary: Arc::new(Arc::unwrap_or_clone(value.primary).try_into()?), + secondary: value.secondary, + include_secondary_tags: value.include_secondary_tags, + }) + } +} diff --git a/crates/spfs/src/storage/fallback/repository_test.rs b/crates/spfs/src/storage/fallback/repository_test.rs index cd21305809..bbe1521e1a 100644 --- a/crates/spfs/src/storage/fallback/repository_test.rs +++ b/crates/spfs/src/storage/fallback/repository_test.rs @@ -8,6 +8,8 @@ use rstest::rstest; use crate::fixtures::*; use crate::prelude::*; +use crate::storage::TryRenderStore; +use crate::storage::fs::{MaybeOpenFsRepository, MaybeRenderStore, RenderStore}; #[rstest] #[tokio::test] @@ -15,14 +17,16 @@ async fn test_proxy_payload_repair(tmpdir: tempfile::TempDir) { init_logging(); let primary = Arc::new( - crate::storage::fs::OpenFsRepository::create(tmpdir.path().join("primary")) + crate::storage::fs::OpenFsRepository::::create(tmpdir.path().join("primary")) .await .unwrap(), ); let secondary = Arc::new( - crate::storage::fs::OpenFsRepository::create(tmpdir.path().join("secondary")) - .await - .unwrap(), + crate::storage::fs::OpenFsRepository::::create( + tmpdir.path().join("secondary"), + ) + .await + .unwrap(), ); let digest = primary @@ -44,9 +48,62 @@ async fn test_proxy_payload_repair(tmpdir: tempfile::TempDir) { assert!(err.is_err()); // Loading the payload through the fallback should succeed. - let proxy = super::FallbackProxy::new(primary, vec![secondary.into()], false); + let proxy = super::FallbackProxy::::new(primary, vec![secondary.into()], false); proxy .open_payload(digest) .await .expect("payload should be loadable via the secondary"); } + +#[rstest] +#[tokio::test] +async fn test_try_from_fallback_maybe_to_render_fails_without_render_creation( + tmpdir: tempfile::TempDir, +) { + init_logging(); + + let primary = MaybeOpenFsRepository::::create(tmpdir.path().join("primary")) + .await + .unwrap() + .without_render_creation() + .opened() + .await + .unwrap(); + let proxy = super::FallbackProxy::::new(primary, vec![], false); + + let err = super::FallbackProxy::::try_from(proxy) + .expect_err("conversion should fail when render creation is disabled"); + assert!( + matches!( + err, + crate::storage::OpenRepositoryError::PathNotInitialized { .. } + ), + "conversion should fail with PathNotInitialized when renders are unavailable" + ); +} + +#[rstest] +#[tokio::test] +async fn test_try_from_fallback_maybe_to_render_succeeds_after_render_store_exists( + tmpdir: tempfile::TempDir, +) { + init_logging(); + + let primary = MaybeOpenFsRepository::::create(tmpdir.path().join("primary")) + .await + .unwrap() + .opened() + .await + .unwrap(); + primary + .fs_impl + .try_render_store() + .expect("create render store before conversion"); + + let proxy = super::FallbackProxy::::new(primary, vec![], false); + let converted = super::FallbackProxy::::try_from(proxy); + assert!( + converted.is_ok(), + "conversion should succeed after render store exists" + ); +} diff --git a/crates/spfs/src/storage/fs/database.rs b/crates/spfs/src/storage/fs/database.rs index 74bb83e3d7..5bd1532c1b 100644 --- a/crates/spfs/src/storage/fs/database.rs +++ b/crates/spfs/src/storage/fs/database.rs @@ -12,11 +12,20 @@ use encoding::prelude::*; use futures::{Stream, StreamExt, TryFutureExt}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use super::DefaultRenderStoreCreationPolicy; use crate::graph::{DatabaseView, Object, ObjectProto}; +use crate::storage::RenderStoreForUser; use crate::{Error, Result, encoding, graph}; #[async_trait::async_trait] -impl DatabaseView for super::MaybeOpenFsRepository { +impl DatabaseView for super::MaybeOpenFsRepository +where + RS: DefaultRenderStoreCreationPolicy + + RenderStoreForUser + + Send + + Sync + + 'static, +{ async fn has_object(&self, digest: encoding::Digest) -> bool { let Ok(opened) = self.opened().await else { return false; @@ -55,7 +64,14 @@ impl DatabaseView for super::MaybeOpenFsRepository { } #[async_trait::async_trait] -impl graph::Database for super::MaybeOpenFsRepository { +impl graph::Database for super::MaybeOpenFsRepository +where + RS: DefaultRenderStoreCreationPolicy + + RenderStoreForUser + + Send + + Sync + + 'static, +{ async fn remove_object(&self, digest: encoding::Digest) -> crate::Result<()> { self.opened().await?.remove_object(digest).await } @@ -73,14 +89,24 @@ impl graph::Database for super::MaybeOpenFsRepository { } #[async_trait::async_trait] -impl graph::DatabaseExt for super::MaybeOpenFsRepository { +impl graph::DatabaseExt for super::MaybeOpenFsRepository +where + RS: DefaultRenderStoreCreationPolicy + + RenderStoreForUser + + Send + + Sync + + 'static, +{ async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { self.opened().await?.write_object(obj).await } } #[async_trait::async_trait] -impl DatabaseView for super::OpenFsRepository { +impl DatabaseView for super::OpenFsRepository +where + RS: Send + Sync, +{ async fn has_object(&self, digest: encoding::Digest) -> bool { let filepath = self.objects.build_digest_path(&digest); tokio::fs::symlink_metadata(filepath).await.is_ok() @@ -129,7 +155,10 @@ impl DatabaseView for super::OpenFsRepository { } #[async_trait::async_trait] -impl graph::Database for super::OpenFsRepository { +impl graph::Database for super::OpenFsRepository +where + RS: Send + Sync, +{ async fn remove_object(&self, digest: encoding::Digest) -> crate::Result<()> { let filepath = self.objects.build_digest_path(&digest); @@ -200,7 +229,10 @@ impl graph::Database for super::OpenFsRepository { } #[async_trait::async_trait] -impl graph::DatabaseExt for super::OpenFsRepository { +impl graph::DatabaseExt for super::OpenFsRepository +where + RS: Send + Sync, +{ async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { let digest = obj.digest()?; let filepath = self.objects.build_digest_path(&digest); diff --git a/crates/spfs/src/storage/fs/hash_store.rs b/crates/spfs/src/storage/fs/hash_store.rs index bb68b5ed90..42ca2f083d 100644 --- a/crates/spfs/src/storage/fs/hash_store.rs +++ b/crates/spfs/src/storage/fs/hash_store.rs @@ -36,6 +36,7 @@ pub(crate) enum PersistableObject { }, } +#[derive(Debug)] pub struct FsHashStore { root: PathBuf, /// permissions used when creating new directories diff --git a/crates/spfs/src/storage/fs/manifest_render_path.rs b/crates/spfs/src/storage/fs/manifest_render_path.rs index 0f95c06e1d..4f590ff042 100644 --- a/crates/spfs/src/storage/fs/manifest_render_path.rs +++ b/crates/spfs/src/storage/fs/manifest_render_path.rs @@ -3,6 +3,7 @@ // https://github.com/spkenv/spk use std::path::PathBuf; +use std::sync::Arc; use crate::{Result, graph}; @@ -21,3 +22,13 @@ where T::manifest_render_path(self, manifest) } } + +impl ManifestRenderPath for Arc +where + T: ManifestRenderPath, +{ + #[inline] + fn manifest_render_path(&self, manifest: &graph::Manifest) -> Result { + T::manifest_render_path(self, manifest) + } +} diff --git a/crates/spfs/src/storage/fs/mod.rs b/crates/spfs/src/storage/fs/mod.rs index 8712436a44..7d84974a2b 100644 --- a/crates/spfs/src/storage/fs/mod.rs +++ b/crates/spfs/src/storage/fs/mod.rs @@ -38,10 +38,14 @@ pub use repository::MaybeOpenFsRepositoryImpl; pub use repository::{ Config, DURABLE_EDITS_DIR, + DefaultRenderStoreCreationPolicy, FsRepositoryOps, MaybeOpenFsRepository, + MaybeRenderStore, + NoRenderStore, OpenFsRepository, Params, RenderStore, + RenderStoreCreationPolicy, read_last_migration_version, }; diff --git a/crates/spfs/src/storage/fs/payloads.rs b/crates/spfs/src/storage/fs/payloads.rs index d1205ceabe..a046e4d5c7 100644 --- a/crates/spfs/src/storage/fs/payloads.rs +++ b/crates/spfs/src/storage/fs/payloads.rs @@ -8,13 +8,21 @@ use std::pin::Pin; use futures::future::ready; use futures::{Stream, StreamExt, TryFutureExt}; -use super::{MaybeOpenFsRepository, OpenFsRepository}; +use super::{DefaultRenderStoreCreationPolicy, MaybeOpenFsRepository, OpenFsRepository}; +use crate::storage::RenderStoreForUser; use crate::storage::prelude::*; use crate::tracking::BlobRead; use crate::{Error, Result, encoding, graph}; #[async_trait::async_trait] -impl crate::storage::PayloadStorage for MaybeOpenFsRepository { +impl crate::storage::PayloadStorage for MaybeOpenFsRepository +where + RS: DefaultRenderStoreCreationPolicy + + RenderStoreForUser + + Send + + Sync + + 'static, +{ async fn has_payload(&self, digest: encoding::Digest) -> bool { let Ok(opened) = self.opened().await else { return false; @@ -52,7 +60,10 @@ impl crate::storage::PayloadStorage for MaybeOpenFsRepository { } #[async_trait::async_trait] -impl crate::storage::PayloadStorage for OpenFsRepository { +impl crate::storage::PayloadStorage for OpenFsRepository +where + RS: Send + Sync, +{ async fn has_payload(&self, digest: encoding::Digest) -> bool { let path = self.payloads.build_digest_path(&digest); tokio::fs::symlink_metadata(path).await.is_ok() diff --git a/crates/spfs/src/storage/fs/renderer.rs b/crates/spfs/src/storage/fs/renderer.rs index 47ee2b462f..cfd0fed42d 100644 --- a/crates/spfs/src/storage/fs/renderer.rs +++ b/crates/spfs/src/storage/fs/renderer.rs @@ -19,13 +19,13 @@ use tokio::sync::Semaphore; use crate::prelude::*; use crate::runtime::makedirs_with_perms; -use crate::storage::LocalRepository; use crate::storage::fs::{ ManifestRenderPath, OpenFsRepository, RenderReporter, SilentRenderReporter, }; +use crate::storage::{LocalPayloads, LocalRenderStore, TryRenderStore}; use crate::{Error, OsError, Result, encoding, graph, tracking}; #[cfg(test)] @@ -73,19 +73,16 @@ impl From for RenderType { } } -impl OpenFsRepository { +impl OpenFsRepository +where + RS: LocalRenderStore + Send + Sync, +{ fn get_render_storage(&self) -> Result<&crate::storage::fs::FsHashStore> { - match &self.renders { - Some(render_store) => Ok(&render_store.renders), - None => Err(Error::NoRenderStorage(self.address().into_owned())), - } + Ok(&self.rs_impl.render_store().renders) } pub async fn has_rendered_manifest(&self, digest: encoding::Digest) -> bool { - let renders = match &self.renders { - Some(render_store) => &render_store.renders, - None => return false, - }; + let renders = &self.rs_impl.render_store().renders; let rendered_dir = renders.build_digest_path(&digest); was_render_completed(rendered_dir).await } @@ -102,18 +99,12 @@ impl OpenFsRepository { } pub fn proxy_path(&self) -> Option<&std::path::Path> { - self.fs_impl - .renders - .as_ref() - .map(|render_store| render_store.proxy.root()) + Some(self.fs_impl.rs_impl.render_store().proxy.root()) } /// Remove the identified render from this storage. pub async fn remove_rendered_manifest(&self, digest: crate::encoding::Digest) -> Result<()> { - let renders = match &self.renders { - Some(render_store) => &render_store.renders, - None => return Ok(()), - }; + let renders = &self.rs_impl.render_store().renders; let rendered_dirpath = renders.build_digest_path(&digest); let workdir = renders.workdir(); if let Err(err) = makedirs_with_perms(&workdir, renders.directory_permissions) { @@ -126,33 +117,13 @@ impl OpenFsRepository { Self::remove_dir_atomically(&rendered_dirpath, &workdir).await } - pub(crate) async fn remove_dir_atomically(dirpath: &Path, workdir: &Path) -> Result<()> { - let uuid = uuid::Uuid::new_v4().to_string(); - let working_dirpath = workdir.join(uuid); - if let Err(err) = tokio::fs::rename(&dirpath, &working_dirpath).await { - return match err.kind() { - std::io::ErrorKind::NotFound => Ok(()), - _ => Err(crate::Error::StorageWriteError( - "rename on render before removal", - working_dirpath, - err, - )), - }; - } - - open_perms_and_remove_all(&working_dirpath).await - } - /// Returns true if the render was actually removed pub async fn remove_rendered_manifest_if_older_than( &self, older_than: DateTime, digest: encoding::Digest, ) -> Result { - let renders = match &self.renders { - Some(render_store) => &render_store.renders, - None => return Ok(false), - }; + let renders = &self.rs_impl.render_store().renders; let rendered_dirpath = renders.build_digest_path(&digest); let metadata = match tokio::fs::symlink_metadata(&rendered_dirpath).await { @@ -184,7 +155,29 @@ impl OpenFsRepository { } } -impl ManifestRenderPath for OpenFsRepository { +impl OpenFsRepository { + pub(crate) async fn remove_dir_atomically(dirpath: &Path, workdir: &Path) -> Result<()> { + let uuid = uuid::Uuid::new_v4().to_string(); + let working_dirpath = workdir.join(uuid); + if let Err(err) = tokio::fs::rename(&dirpath, &working_dirpath).await { + return match err.kind() { + std::io::ErrorKind::NotFound => Ok(()), + _ => Err(crate::Error::StorageWriteError( + "rename on render before removal", + working_dirpath, + err, + )), + }; + } + + open_perms_and_remove_all(&working_dirpath).await + } +} + +impl ManifestRenderPath for OpenFsRepository +where + RS: LocalRenderStore + Send + Sync, +{ fn manifest_render_path(&self, manifest: &graph::Manifest) -> Result { Ok(self .get_render_storage()? @@ -240,7 +233,7 @@ impl<'repo, Repo> Renderer<'repo, Repo, SilentRenderReporter> { impl<'repo, Repo, Reporter> Renderer<'repo, Repo, Reporter> where - Repo: Repository + LocalRepository, + Repo: Repository + LocalPayloads, Reporter: RenderReporter, { /// Report progress to the given instance, replacing any existing one @@ -276,36 +269,13 @@ where self.max_concurrent_branches = max_concurrent_branches; self } +} - /// Render all layers in the given env to the render storage of the underlying - /// repository, returning the paths to all relevant layers in the appropriate order. - pub async fn render( - &self, - stack: &graph::Stack, - render_type: Option, - ) -> Result> { - let layers = crate::resolve::resolve_stack_to_layers_with_repo(stack, self.repo) - .await - .map_err(|err| err.wrap("resolve stack to layers"))?; - let mut futures = futures::stream::FuturesOrdered::new(); - for layer in layers { - if let Some(manifest_digest) = layer.manifest() { - let digest = *manifest_digest; - let fut = self - .repo - .read_manifest(digest) - .map_err(move |err| err.wrap(format!("read manifest {digest}"))) - .and_then(move |manifest| async move { - self.render_manifest(&manifest, render_type) - .await - .map_err(move |err| err.wrap(format!("render manifest {digest}"))) - }); - futures.push_back(fut); - } - } - futures.try_collect().await - } - +impl Renderer<'_, Repo, Reporter> +where + Repo: Repository + LocalPayloads + TryRenderStore, + Reporter: RenderReporter, +{ /// Recreate the full structure of a stored environment on disk pub async fn render_into_directory, P: AsRef>( &self, @@ -335,6 +305,41 @@ where self.render_manifest_into_dir(&manifest, target_dir, render_type) .await } +} + +impl Renderer<'_, Repo, Reporter> +where + Repo: Repository + LocalPayloads + LocalRenderStore + TryRenderStore, + Reporter: RenderReporter, +{ + /// Render all layers in the given env to the render storage of the underlying + /// repository, returning the paths to all relevant layers in the appropriate order. + pub async fn render( + &self, + stack: &graph::Stack, + render_type: Option, + ) -> Result> { + let layers = crate::resolve::resolve_stack_to_layers_with_repo(stack, self.repo) + .await + .map_err(|err| err.wrap("resolve stack to layers"))?; + let mut futures = futures::stream::FuturesOrdered::new(); + for layer in layers { + if let Some(manifest_digest) = layer.manifest() { + let digest = *manifest_digest; + let fut = self + .repo + .read_manifest(digest) + .map_err(move |err| err.wrap(format!("read manifest {digest}"))) + .and_then(move |manifest| async move { + self.render_manifest(&manifest, render_type) + .await + .map_err(move |err| err.wrap(format!("render manifest {digest}"))) + }); + futures.push_back(fut); + } + } + futures.try_collect().await + } /// Render a manifest into the renders area of the underlying repository, /// returning the absolute local path of the directory. @@ -343,7 +348,7 @@ where manifest: &graph::Manifest, render_type: Option, ) -> Result { - let render_store = self.repo.render_store()?; + let render_store = self.repo.render_store(); let rendered_dirpath = render_store.renders.build_digest_path(&manifest.digest()?); if was_render_completed(&rendered_dirpath).await { tracing::trace!(path = ?rendered_dirpath, "render already completed"); diff --git a/crates/spfs/src/storage/fs/renderer_test.rs b/crates/spfs/src/storage/fs/renderer_test.rs index 30e967b449..9eec19433c 100644 --- a/crates/spfs/src/storage/fs/renderer_test.rs +++ b/crates/spfs/src/storage/fs/renderer_test.rs @@ -10,8 +10,8 @@ use super::was_render_completed; use crate::encoding::prelude::*; use crate::fixtures::*; use crate::graph::object::{DigestStrategy, EncodingFormat}; -use crate::storage::fs::{MaybeOpenFsRepository, OpenFsRepository}; -use crate::storage::{RepositoryExt, RepositoryHandle}; +use crate::storage::fs::{MaybeOpenFsRepository, NoRenderStore, OpenFsRepository, RenderStore}; +use crate::storage::{LayerStorageExt, RepositoryExt, RepositoryHandle, TagStorage}; use crate::{Config, reset_config_async, tracking}; #[rstest( @@ -31,7 +31,7 @@ async fn test_render_manifest( config.storage.digest_strategy = write_digest_strategy; config.make_current().unwrap(); - let storage = OpenFsRepository::create(tmpdir.path().join("storage")) + let storage = OpenFsRepository::::create(tmpdir.path().join("storage")) .await .unwrap(); @@ -86,7 +86,7 @@ async fn test_render_manifest_with_repo( config.make_current().unwrap(); let tmprepo = Arc::new( - MaybeOpenFsRepository::create(tmpdir.path().join("repo")) + MaybeOpenFsRepository::::create(tmpdir.path().join("repo")) .await .unwrap() .into(), @@ -105,15 +105,13 @@ async fn test_render_manifest_with_repo( // Safety: tmprepo was created as an FsRepository let tmprepo = match &*tmprepo { - RepositoryHandle::FS(fs) => fs.opened().await.unwrap(), + RepositoryHandle::FSWithRenders(fs) => fs.opened().await.unwrap(), _ => panic!("Unexpected tmprepo type!"), }; let render = tmprepo .fs_impl - .renders - .as_ref() - .unwrap() + .rs_impl .renders .build_digest_path(&manifest.digest().unwrap()); assert!(!render.exists(), "render should NOT be seen as existing"); @@ -133,3 +131,58 @@ async fn test_render_manifest_with_repo( ); } } + +#[tokio::test] +async fn test_render_into_directory_without_render_store_does_not_create_renders_dir() { + let tmpdir = tempfile::Builder::new() + .prefix("spfs-test-") + .tempdir() + .unwrap(); + let repo_root = tmpdir.path().join("repo"); + let repo = MaybeOpenFsRepository::::create(&repo_root) + .await + .unwrap(); + let repo_handle = Arc::new(RepositoryHandle::from(repo.clone())); + + let src_dir = tmpdir.path().join("source"); + ensure(src_dir.join("dir/file.txt"), "hello"); + ensure(src_dir.join("file.txt"), "world"); + let expected_manifest = crate::Committer::new(&repo_handle) + .commit_dir(&src_dir) + .await + .unwrap(); + let layer = repo_handle + .create_layer(&expected_manifest.to_graph_manifest()) + .await + .unwrap(); + let tag = tracking::TagSpec::parse("renderer/no-render-store").unwrap(); + repo_handle + .push_tag(&tag, &layer.digest().unwrap()) + .await + .unwrap(); + + let opened = repo.opened().await.unwrap(); + let target_dir = tmpdir.path().join("rendered"); + let env_spec = tracking::EnvSpec::parse(tag.to_string()).unwrap(); + super::Renderer::new(&opened) + .render_into_directory( + env_spec, + &target_dir, + super::RenderType::HardLink(super::HardLinkRenderType::WithoutProxy), + ) + .await + .unwrap(); + + let root_contents = tokio::fs::read_to_string(target_dir.join("file.txt")) + .await + .unwrap(); + let nested_contents = tokio::fs::read_to_string(target_dir.join("dir/file.txt")) + .await + .unwrap(); + assert_eq!(root_contents, "world"); + assert_eq!(nested_contents, "hello"); + assert!( + !repo_root.join("renders").exists(), + "render_into_directory should not create repository renders dir" + ); +} diff --git a/crates/spfs/src/storage/fs/renderer_unix.rs b/crates/spfs/src/storage/fs/renderer_unix.rs index 0b89fd7764..4980944f58 100644 --- a/crates/spfs/src/storage/fs/renderer_unix.rs +++ b/crates/spfs/src/storage/fs/renderer_unix.rs @@ -17,14 +17,14 @@ use tokio::io::AsyncReadExt; use super::{BlobSemaphorePermit, HardLinkRenderType, RenderType, Renderer}; use crate::prelude::*; -use crate::storage::LocalRepository; use crate::storage::fs::RenderReporter; use crate::storage::fs::render_reporter::RenderBlobResult; +use crate::storage::{LocalPayloads, TryRenderStore}; use crate::{Error, OsError, Result, get_config, graph, tracking}; impl Renderer<'_, Repo, Reporter> where - Repo: Repository + LocalRepository, + Repo: Repository + LocalPayloads + TryRenderStore, Reporter: RenderReporter, { /// Recreate the full structure of a stored manifest on disk. @@ -227,7 +227,7 @@ where let render_blob_result = if matches!(render_type, HardLinkRenderType::WithoutProxy) { // explicitly skip proxy generation RenderBlobResult::PayloadCopiedByRequest - } else if let Ok(render_store) = self.repo.render_store() { + } else if let Ok(render_store) = self.repo.try_render_store() { let proxy_path = render_store .proxy .build_digest_path(entry.object()) @@ -418,9 +418,7 @@ where render_blob_result } else { - return Err( - "Cannot render blob as hard link to repository with no render store".into(), - ); + return Err(Error::NoRenderStorage(self.repo.address().into_owned())); }; break if let Err(err) = nix::unistd::linkat( diff --git a/crates/spfs/src/storage/fs/renderer_win.rs b/crates/spfs/src/storage/fs/renderer_win.rs index 1d49d29c50..34d1b1e563 100644 --- a/crates/spfs/src/storage/fs/renderer_win.rs +++ b/crates/spfs/src/storage/fs/renderer_win.rs @@ -6,13 +6,13 @@ use std::path::Path; use super::{RenderType, Renderer}; use crate::prelude::*; -use crate::storage::LocalRepository; use crate::storage::fs::RenderReporter; +use crate::storage::{LocalPayloads, TryRenderStore}; use crate::{Result, graph}; impl<'repo, Repo, Reporter> Renderer<'repo, Repo, Reporter> where - Repo: Repository + LocalRepository, + Repo: Repository + LocalPayloads + TryRenderStore, Reporter: RenderReporter, { /// Recreate the full structure of a stored manifest on disk. diff --git a/crates/spfs/src/storage/fs/repository.rs b/crates/spfs/src/storage/fs/repository.rs index e4f510e0d5..5f433ed516 100644 --- a/crates/spfs/src/storage/fs/repository.rs +++ b/crates/spfs/src/storage/fs/repository.rs @@ -16,6 +16,7 @@ use arc_swap::ArcSwap; use async_stream::try_stream; use chrono::{DateTime, Utc}; use futures::Stream; +use variantly::Variantly; use super::FsHashStore; use super::hash_store::PROXY_DIRNAME; @@ -24,14 +25,21 @@ use crate::config::{ToAddress, pathbuf_deserialize_with_tilde_expansion}; use crate::runtime::makedirs_with_perms; use crate::storage::prelude::*; use crate::storage::{ - LocalRepository, + LocalPayloads, + LocalRenderStore, OpenRepositoryError, OpenRepositoryResult, + RenderStoreForUser, TagNamespace, TagNamespaceBuf, + TryRenderStore, }; use crate::{Error, Result}; +#[cfg(test)] +#[path = "./repository_test.rs"] +mod repository_test; + /// The directory name within the repo where durable runtimes keep /// their upper path roots and upper/work directories. pub const DURABLE_EDITS_DIR: &str = "durable_edits"; @@ -67,6 +75,15 @@ pub struct Params { #[serde(default)] pub lazy: bool, pub tag_namespace: Option, + #[serde(default = "Params::default_create_renders")] + pub create_renders: bool, +} + +impl Params { + /// Whether to create the render store if it doesn't already exist. Defaults to true. + fn default_create_renders() -> bool { + true + } } #[async_trait::async_trait] @@ -89,46 +106,267 @@ impl FromUrl for Config { } /// Renders need a place for proxy files and the rendered hard links. +/// +/// An instance of `RenderStore` represents a valid render store that has +/// already been created. +#[derive(Debug)] pub struct RenderStore { + url: url::Url, pub proxy: FsHashStore, pub renders: FsHashStore, } -impl RenderStore { - pub fn for_user>(root: &Path, username: P) -> Result { - let username = username.as_ref(); +impl DefaultRenderStoreCreationPolicy for RenderStore { + fn default_creation_policy() -> RenderStoreCreationPolicy { + RenderStoreCreationPolicy::CreateIfMissing + } +} + +impl LocalRenderStore for RenderStore { + fn render_store(&self) -> &RenderStore { + self + } +} + +impl RenderStoreForUser for RenderStore { + type RenderStore = Self; + + fn render_store_for_user( + creation_policy: RenderStoreCreationPolicy, + url: url::Url, + root: &Path, + username: &Path, + ) -> OpenRepositoryResult + where + Self: Sized, + { let renders_dir = root.join("renders").join(username); - FsHashStore::open(renders_dir.join(PROXY_DIRNAME)) - .and_then(|proxy| { - FsHashStore::open(&renders_dir).map(|renders| RenderStore { proxy, renders }) - }) - .map_err(|source| Error::FailedToOpenRepository { - repository: format!("", username.display()), - source, + let proxy_dir = renders_dir.join(PROXY_DIRNAME); + + // Verify the renders directory exists. + let stat = std::fs::symlink_metadata(&proxy_dir); + + match creation_policy { + RenderStoreCreationPolicy::CreateIfMissing => match stat { + Ok(_) => {} + Err(source) if source.kind() == std::io::ErrorKind::NotFound => { + makedirs_with_perms(&proxy_dir, 0o777).map_err(|source| { + OpenRepositoryError::PathNotInitialized { + path: proxy_dir.clone(), + source, + } + })?; + } + Err(source) => { + return Err(OpenRepositoryError::PathNotInitialized { + path: proxy_dir, + source, + }); + } + }, + RenderStoreCreationPolicy::DoNotCreate => { + if let Err(source) = stat { + return Err(OpenRepositoryError::PathNotInitialized { + path: proxy_dir, + source, + }); + } + } + }; + + FsHashStore::open(proxy_dir).and_then(|proxy| { + FsHashStore::open(&renders_dir).map(|renders| RenderStore { + url, + proxy, + renders, }) + }) + } +} + +impl TryRenderStore for RenderStore { + fn try_render_store(&self) -> OpenRepositoryResult> { + Ok(Cow::Borrowed(self)) + } + + fn proxy_path(&self) -> Option> { + Some(Cow::Borrowed(self.proxy.root())) } } impl Clone for RenderStore { fn clone(&self) -> Self { Self { + url: self.url.clone(), proxy: FsHashStore::open_unchecked(self.proxy.root()), renders: FsHashStore::open_unchecked(self.renders.root()), } } } +#[derive(Clone, Debug)] +enum InnerMaybeRenderStore { + /// The render store has not been created or validated yet. + StatusUnknown { + url: url::Url, + root: PathBuf, + username: PathBuf, + }, + /// The render store is known to exist and is valid. + Valid { renders: RenderStore }, +} + +pub trait DefaultRenderStoreCreationPolicy { + fn default_creation_policy() -> RenderStoreCreationPolicy; +} + +#[derive(Clone, Copy, Debug, Variantly)] +pub enum RenderStoreCreationPolicy { + CreateIfMissing, + DoNotCreate, +} + +/// A render store flavor for repositories that may support renders, but the +/// storage may not have been created or validated. +#[derive(Clone, Debug)] +pub struct MaybeRenderStore { + /// If the store should be created if necessary. + creation_policy: RenderStoreCreationPolicy, + inner: Arc>, +} + +impl MaybeRenderStore { + /// Return a new instance of this render store with creation disabled. The render + /// store will only be accessible if it already exists, but will not be created on demand. + fn without_render_creation(self) -> Self { + Self { + creation_policy: RenderStoreCreationPolicy::DoNotCreate, + inner: self.inner, + } + } +} + +impl TryFrom for RenderStore { + type Error = OpenRepositoryError; + + fn try_from(value: MaybeRenderStore) -> OpenRepositoryResult { + Ok(value.try_render_store()?.into_owned()) + } +} + +impl DefaultRenderStoreCreationPolicy for MaybeRenderStore { + fn default_creation_policy() -> RenderStoreCreationPolicy { + RenderStoreCreationPolicy::CreateIfMissing + } +} + +impl TryRenderStore for MaybeRenderStore { + fn try_render_store(&self) -> OpenRepositoryResult> { + match &**self.inner.load() { + InnerMaybeRenderStore::StatusUnknown { + url, + root, + username, + } => { + // Create the render store if it doesn't exist (if requested), or + // return an error if it doesn't already exist. + match RenderStore::render_store_for_user( + self.creation_policy, + url.clone(), + root, + username, + ) { + Ok(store) => { + self.inner.rcu(|_| InnerMaybeRenderStore::Valid { + renders: store.clone(), + }); + Ok(Cow::Owned(store)) + } + Err(err) => Err(err), + } + } + InnerMaybeRenderStore::Valid { renders } => { + // Can't borrow from the temporary returned by ArcSwap::load(). + Ok(Cow::Owned(renders.clone())) + } + } + } + + fn proxy_path(&self) -> Option> { + self.try_render_store() + .ok() + .map(|store| Cow::Owned(store.proxy.root().to_owned())) + } +} + +impl RenderStoreForUser for MaybeRenderStore { + type RenderStore = Self; + + fn render_store_for_user( + creation_policy: RenderStoreCreationPolicy, + url: url::Url, + root: &Path, + username: &Path, + ) -> OpenRepositoryResult { + Ok(Self { + creation_policy, + inner: Arc::new(ArcSwap::new(Arc::new( + InnerMaybeRenderStore::StatusUnknown { + url, + root: root.to_owned(), + username: username.to_owned(), + }, + ))), + }) + } +} + +/// Represents a render store flavor for repositories that don't have renders +/// and/or don't support renders, like tar repositories, or when accessing a +/// repository in a way that doesn't require renders. +#[derive(Clone, Debug)] +pub struct NoRenderStore; + +impl DefaultRenderStoreCreationPolicy for NoRenderStore { + fn default_creation_policy() -> RenderStoreCreationPolicy { + RenderStoreCreationPolicy::DoNotCreate + } +} + +impl RenderStoreForUser for NoRenderStore { + type RenderStore = Self; + + fn render_store_for_user( + _creation_policy: RenderStoreCreationPolicy, + _url: url::Url, + _root: &Path, + _username: &Path, + ) -> OpenRepositoryResult { + Ok(Self) + } +} + +impl TryRenderStore for NoRenderStore { + fn try_render_store(&self) -> OpenRepositoryResult> { + Err(OpenRepositoryError::RenderStorageUnavailable) + } + + fn proxy_path(&self) -> Option> { + None + } +} + /// Operations on a FsRepository. #[async_trait::async_trait] pub trait FsRepositoryOps: Send + Sync { - /// True if this repo is setup to generate local manifest renders. - fn has_renders(&self) -> bool; - fn iter_rendered_manifests( &self, ) -> Pin> + Send + Sync + '_>>; - fn proxy_path(&self) -> Option<&std::path::Path>; + /// Return the path to the proxy directory for this repository, if it + /// exists. Some render store types may create this directory on demand, + /// if it didn't already exist. + fn proxy_path(&self) -> Option>; /// Remove the identified render from this storage. async fn remove_rendered_manifest(&self, digest: crate::encoding::Digest) -> Result<()>; @@ -152,17 +390,13 @@ impl FsRepositoryOps for &T where T: FsRepositoryOps, { - fn has_renders(&self) -> bool { - T::has_renders(*self) - } - fn iter_rendered_manifests( &self, ) -> Pin> + Send + Sync + '_>> { T::iter_rendered_manifests(*self) } - fn proxy_path(&self) -> Option<&std::path::Path> { + fn proxy_path(&self) -> Option> { T::proxy_path(*self) } @@ -198,22 +432,58 @@ impl std::ops::Deref for FsRepository { } } +impl TryFrom>> + for FsRepository> +{ + type Error = OpenRepositoryError; + + fn try_from( + value: FsRepository>, + ) -> OpenRepositoryResult { + Ok(Self { + fs_impl: Arc::new(Arc::unwrap_or_clone(value.fs_impl).try_into()?), + }) + } +} + +impl TryFrom>> + for FsRepository> +{ + type Error = OpenRepositoryError; + + fn try_from( + value: FsRepository>, + ) -> OpenRepositoryResult { + Ok(Self { + fs_impl: Arc::new(Arc::unwrap_or_clone(value.fs_impl).try_into()?), + }) + } +} + +impl FsRepository> { + /// Return a new instance of this repository with render store creation + /// disabled. The render store will only be accessible if it already exists, + /// but will not be created on demand. + pub fn without_render_creation(self) -> Self { + let new_impl = Arc::unwrap_or_clone(self.fs_impl); + Self { + fs_impl: Arc::new(new_impl.without_render_creation()), + } + } +} + #[async_trait::async_trait] impl FsRepositoryOps for FsRepository where FS: FsRepositoryOps, { - fn has_renders(&self) -> bool { - self.fs_impl.has_renders() - } - fn iter_rendered_manifests( &self, ) -> Pin> + Send + Sync + '_>> { self.fs_impl.iter_rendered_manifests() } - fn proxy_path(&self) -> Option<&std::path::Path> { + fn proxy_path(&self) -> Option> { self.fs_impl.proxy_path() } @@ -245,38 +515,86 @@ where } } -impl LocalRepository for FsRepository +impl LocalPayloads for FsRepository where - FS: LocalRepository, + FS: LocalPayloads, { fn payloads(&self) -> &FsHashStore { self.fs_impl.payloads() } +} - fn render_store(&self) -> Result<&RenderStore> { +impl LocalRenderStore for FsRepository +where + FS: LocalRenderStore, +{ + fn render_store(&self) -> &RenderStore { self.fs_impl.render_store() } } -pub type MaybeOpenFsRepository = FsRepository; -pub type OpenFsRepository = FsRepository; +impl RenderStoreForUser for FsRepository +where + FS: RenderStoreForUser, +{ + type RenderStore = RS; -impl MaybeOpenFsRepository { + fn render_store_for_user( + creation_policy: RenderStoreCreationPolicy, + url: url::Url, + root: &Path, + username: &Path, + ) -> OpenRepositoryResult { + FS::render_store_for_user(creation_policy, url, root, username) + } +} + +impl TryRenderStore for FsRepository +where + FS: TryRenderStore, +{ + fn try_render_store(&self) -> OpenRepositoryResult> { + self.fs_impl.try_render_store() + } + + fn proxy_path(&self) -> Option> { + self.fs_impl.proxy_path() + } +} + +pub type MaybeOpenFsRepository = FsRepository>; +pub type OpenFsRepository = FsRepository>; + +impl MaybeOpenFsRepository +where + RS: DefaultRenderStoreCreationPolicy + + RenderStoreForUser + + Send + + Sync + + 'static, +{ /// Get the opened version of this repository, performing /// any required opening and validation as needed - pub fn opened(&self) -> impl futures::Future> + 'static { + pub fn opened(&self) -> impl futures::Future>> + 'static { let fs_impl = Arc::clone(&self.fs_impl); async move { let fs_impl = fs_impl .opened_and_map_err(Error::failed_to_open_repository) .await?; - Ok(OpenFsRepository { fs_impl }) + Ok(OpenFsRepository:: { fs_impl }) } } /// Open a filesystem repository, creating it if necessary - pub async fn create>(root: P) -> OpenRepositoryResult { - MaybeOpenFsRepositoryImpl::create(root) + pub async fn create(root: impl AsRef) -> OpenRepositoryResult { + MaybeOpenFsRepositoryImpl::::create(root) + .await + .map(Into::into) + .map(|fs_impl| FsRepository { fs_impl }) + } + + async fn from_fs_config(config: Config) -> crate::storage::OpenRepositoryResult { + MaybeOpenFsRepositoryImpl::::from_config(config) .await .map(Into::into) .map(|fs_impl| FsRepository { fs_impl }) @@ -284,43 +602,69 @@ impl MaybeOpenFsRepository { } #[async_trait::async_trait] -impl FromConfig for MaybeOpenFsRepository { +impl FromConfig for MaybeOpenFsRepository { type Config = Config; - async fn from_config(config: Self::Config) -> crate::storage::OpenRepositoryResult { - MaybeOpenFsRepositoryImpl::from_config(config) - .await - .map(Into::into) - .map(|fs_impl| FsRepository { fs_impl }) + async fn from_config( + config: Self::Config, + ) -> crate::storage::OpenRepositoryResult { + if config.params.create_renders { + MaybeOpenFsRepository::::from_fs_config(config) + .await + .map(Into::into) + } else { + MaybeOpenFsRepository::::from_fs_config(config) + .await + .map(Into::into) + } } } -impl OpenFsRepository { +impl OpenFsRepository +where + RS: DefaultRenderStoreCreationPolicy + RenderStoreForUser, +{ /// Establish a new filesystem repository - pub async fn create>(root: P) -> OpenRepositoryResult { - OpenFsRepositoryImpl::create(root) + pub async fn create(root: impl AsRef) -> OpenRepositoryResult { + OpenFsRepositoryImpl::::create(root) .await .map(Into::into) .map(|fs_impl| FsRepository { fs_impl }) } } -impl From for MaybeOpenFsRepository { - fn from(value: OpenFsRepository) -> Self { - MaybeOpenFsRepository { - fs_impl: Arc::new(MaybeOpenFsRepositoryImpl(Arc::new(ArcSwap::new(Arc::new( - InnerFsRepository::Open(value.fs_impl), - ))))), +impl From> for MaybeOpenFsRepository { + fn from(value: OpenFsRepository) -> Self { + MaybeOpenFsRepository:: { + fs_impl: Arc::new(MaybeOpenFsRepositoryImpl::(Arc::new(ArcSwap::new( + Arc::new(InnerFsRepository::Open(value.fs_impl)), + )))), } } } -impl From> for MaybeOpenFsRepository { - fn from(value: Arc) -> Self { +impl From>> for MaybeOpenFsRepository { + fn from(value: Arc>) -> Self { MaybeOpenFsRepository { - fs_impl: Arc::new(MaybeOpenFsRepositoryImpl(Arc::new(ArcSwap::new(Arc::new( - InnerFsRepository::Open(Arc::clone(&value.fs_impl)), - ))))), + fs_impl: Arc::new(MaybeOpenFsRepositoryImpl::(Arc::new(ArcSwap::new( + Arc::new(InnerFsRepository::Open(Arc::clone(&value.fs_impl))), + )))), + } + } +} + +impl From> for MaybeOpenFsRepository { + fn from(value: MaybeOpenFsRepository) -> Self { + Self { + fs_impl: Arc::new(Arc::unwrap_or_clone(value.fs_impl).into()), + } + } +} + +impl From> for MaybeOpenFsRepository { + fn from(value: MaybeOpenFsRepository) -> Self { + Self { + fs_impl: Arc::new(Arc::unwrap_or_clone(value.fs_impl).into()), } } } @@ -333,56 +677,116 @@ impl From> for MaybeOpenFsRepository { /// An [`OpenFsRepository`] is more useful than this one, but /// can also be easily retrieved via the [`Self::opened`]. #[derive(Clone)] -pub struct MaybeOpenFsRepositoryImpl(Arc>); +pub struct MaybeOpenFsRepositoryImpl(Arc>>); -enum InnerFsRepository { +enum InnerFsRepository { Closed(Config), - Open(Arc), + Open(Arc>), } -impl From for MaybeOpenFsRepositoryImpl { - fn from(value: OpenFsRepositoryImpl) -> Self { +impl From> for MaybeOpenFsRepositoryImpl { + fn from(value: OpenFsRepositoryImpl) -> Self { Arc::new(value).into() } } -impl From> for MaybeOpenFsRepositoryImpl { - fn from(value: Arc) -> Self { +impl From>> for MaybeOpenFsRepositoryImpl { + fn from(value: Arc>) -> Self { Self(Arc::new(ArcSwap::new(Arc::new(InnerFsRepository::Open( value, ))))) } } -#[async_trait::async_trait] -impl FromConfig for MaybeOpenFsRepositoryImpl { - type Config = Config; +impl From> + for MaybeOpenFsRepositoryImpl +{ + fn from(value: MaybeOpenFsRepositoryImpl) -> Self { + match &**value.0.load() { + InnerFsRepository::Closed(config) => Self(Arc::new(ArcSwap::new(Arc::new( + InnerFsRepository::Closed(config.clone()), + )))), + InnerFsRepository::Open(repo) => Self(Arc::new(ArcSwap::new(Arc::new( + InnerFsRepository::Open(Arc::new((**repo).clone().into())), + )))), + } + } +} + +impl From> for MaybeOpenFsRepositoryImpl { + fn from(value: MaybeOpenFsRepositoryImpl) -> Self { + match &**value.0.load() { + InnerFsRepository::Closed(config) => Self(Arc::new(ArcSwap::new(Arc::new( + InnerFsRepository::Closed(config.clone()), + )))), + InnerFsRepository::Open(repo) => Self(Arc::new(ArcSwap::new(Arc::new( + InnerFsRepository::Open(Arc::new((**repo).clone().into())), + )))), + } + } +} + +impl TryFrom> + for MaybeOpenFsRepositoryImpl +{ + type Error = OpenRepositoryError; + + fn try_from(value: MaybeOpenFsRepositoryImpl) -> OpenRepositoryResult { + Ok(Self(Arc::new(ArcSwap::new(Arc::new({ + match &**value.0.load() { + InnerFsRepository::Closed(config) => InnerFsRepository::Closed(config.clone()), + InnerFsRepository::Open(o) => { + InnerFsRepository::Open(Arc::new((**o).clone().try_into()?)) + } + } + }))))) + } +} - async fn from_config(config: Self::Config) -> crate::storage::OpenRepositoryResult { +impl MaybeOpenFsRepositoryImpl +where + RS: DefaultRenderStoreCreationPolicy + + RenderStoreForUser + + Send + + Sync + + 'static, +{ + async fn from_config(config: Config) -> crate::storage::OpenRepositoryResult { if config.params.lazy { - Ok(Self(Arc::new(ArcSwap::new(Arc::new( - InnerFsRepository::Closed(config), - ))))) + Ok(Self(Arc::new(ArcSwap::new(Arc::new(InnerFsRepository::< + RS, + >::Closed( + config + )))))) } else { - Ok(OpenFsRepositoryImpl::from_config(config).await?.into()) + Ok(OpenFsRepositoryImpl::::from_fs_config(config) + .await? + .into()) } } } -impl MaybeOpenFsRepositoryImpl { +impl MaybeOpenFsRepositoryImpl +where + RS: DefaultRenderStoreCreationPolicy + + RenderStoreForUser + + Send + + Sync + + 'static, +{ /// Open a filesystem repository, creating it if necessary - pub async fn create>(root: P) -> OpenRepositoryResult { - Ok(Self(Arc::new(ArcSwap::new(Arc::new( - InnerFsRepository::Open(Arc::new(OpenFsRepositoryImpl::create(root).await?)), + pub async fn create(root: impl AsRef) -> OpenRepositoryResult { + Ok(MaybeOpenFsRepositoryImpl(Arc::new(ArcSwap::new(Arc::new( + InnerFsRepository::Open(Arc::new(OpenFsRepositoryImpl::::create(root).await?)), ))))) } // Open a repository over the given directory, which must already // exist and be properly setup as a repository - pub async fn open>(root: P) -> OpenRepositoryResult { + pub async fn open(root: impl AsRef) -> OpenRepositoryResult { let root = root.as_ref(); - Ok(Self(Arc::new(ArcSwap::new(Arc::new( - InnerFsRepository::Open(Arc::new(OpenFsRepositoryImpl::open(&root).await?)), + Ok(MaybeOpenFsRepositoryImpl(Arc::new(ArcSwap::new(Arc::new( + InnerFsRepository::Open(Arc::new(OpenFsRepositoryImpl::::open(&root).await?)), ))))) } @@ -390,7 +794,7 @@ impl MaybeOpenFsRepositoryImpl { /// any required opening and validation as needed pub fn opened( &self, - ) -> impl futures::Future>> + 'static { + ) -> impl futures::Future>>> + 'static { self.opened_and_map_err(Error::failed_to_open_repository) } @@ -398,7 +802,7 @@ impl MaybeOpenFsRepositoryImpl { /// any required opening and validation as needed pub fn try_open( &self, - ) -> impl futures::Future>> + 'static + ) -> impl futures::Future>>> + 'static { self.opened_and_map_err(|_, e| e) } @@ -406,7 +810,7 @@ impl MaybeOpenFsRepositoryImpl { fn opened_and_map_err( &self, map: F, - ) -> impl futures::Future, E>> + 'static + ) -> impl futures::Future>, E>> + 'static where F: FnOnce(&Self, OpenRepositoryError) -> E + 'static, { @@ -415,26 +819,20 @@ impl MaybeOpenFsRepositoryImpl { match &**inner.load() { InnerFsRepository::Closed(config) => { let config = config.clone(); - let opened = match OpenFsRepositoryImpl::from_config(config).await { + let opened = match OpenFsRepositoryImpl::::from_fs_config(config).await { Ok(o) => Arc::new(o), Err(err) => return Err(map(&Self(inner), err)), }; inner.rcu(|_| InnerFsRepository::Open(Arc::clone(&opened))); Ok(opened) } - InnerFsRepository::Open(o) => Ok(Arc::clone(o)), + InnerFsRepository::::Open(o) => Ok(Arc::clone(o)), } } } +} - /// The filesystem root path of this repository - pub fn root(&self) -> PathBuf { - match &**self.0.load() { - InnerFsRepository::Closed(config) => config.path.clone(), - InnerFsRepository::Open(o) => o.root(), - } - } - +impl MaybeOpenFsRepositoryImpl { pub fn get_tag_namespace(&self) -> Option> { match &**self.0.load() { InnerFsRepository::Open(repo) => repo @@ -450,6 +848,19 @@ impl MaybeOpenFsRepositoryImpl { } } + /// The filesystem root path of this repository + pub fn root(&self) -> PathBuf { + match &**self.0.load() { + InnerFsRepository::Closed(config) => config.path.clone(), + InnerFsRepository::Open(o) => o.root(), + } + } +} + +impl MaybeOpenFsRepositoryImpl +where + RS: Clone, +{ pub fn set_tag_namespace( &mut self, tag_namespace: Option, @@ -472,20 +883,39 @@ impl MaybeOpenFsRepositoryImpl { } } -impl Address for MaybeOpenFsRepositoryImpl { +impl MaybeOpenFsRepositoryImpl { + /// Return a new version of this repository with render store creation + /// disabled. The render store will only be accessible if it already exists, + /// but will not be created on demand. + pub fn without_render_creation(self) -> Self { + self.0.rcu(|inner| match &**inner { + InnerFsRepository::Open(repo) => { + InnerFsRepository::Open(Arc::new((**repo).clone().without_render_creation())) + } + InnerFsRepository::Closed(config) => InnerFsRepository::Closed({ + let mut config = config.clone(); + config.params.create_renders = false; + config + }), + }); + self + } +} + +impl Address for MaybeOpenFsRepositoryImpl { fn address(&self) -> Cow<'_, url::Url> { Cow::Owned(url::Url::from_directory_path(self.root()).unwrap()) } } -impl std::fmt::Debug for MaybeOpenFsRepositoryImpl { +impl std::fmt::Debug for MaybeOpenFsRepositoryImpl { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_fmt(format_args!("FsRepository @ {:?}", self.root())) } } /// A validated and opened fs repository. -pub struct OpenFsRepositoryImpl { +pub struct OpenFsRepositoryImpl { root: PathBuf, /// the namespace to use for tag resolution. If set, then this is treated /// as "chroot" of the real tag root. @@ -495,18 +925,62 @@ pub struct OpenFsRepositoryImpl { /// stores all digraph object data for this repo pub objects: FsHashStore, /// stores rendered file system layers for use in overlayfs - pub renders: Option, + pub rs_impl: RS, } -#[async_trait::async_trait] -impl FromConfig for OpenFsRepositoryImpl { - type Config = Config; +impl TryFrom> for OpenFsRepositoryImpl { + type Error = OpenRepositoryError; + + fn try_from(value: OpenFsRepositoryImpl) -> OpenRepositoryResult { + Ok(Self { + root: value.root, + tag_namespace: value.tag_namespace, + payloads: value.payloads, + objects: value.objects, + rs_impl: value.rs_impl.try_into()?, + }) + } +} + +impl From> for OpenFsRepositoryImpl { + fn from(value: OpenFsRepositoryImpl) -> Self { + Self { + root: value.root, + tag_namespace: value.tag_namespace, + payloads: value.payloads, + objects: value.objects, + rs_impl: NoRenderStore, + } + } +} + +impl From> for OpenFsRepositoryImpl { + fn from(value: OpenFsRepositoryImpl) -> Self { + Self { + root: value.root, + tag_namespace: value.tag_namespace, + payloads: value.payloads, + objects: value.objects, + rs_impl: NoRenderStore, + } + } +} + +impl OpenFsRepositoryImpl +where + RS: DefaultRenderStoreCreationPolicy + RenderStoreForUser + Send + Sync, +{ + async fn from_fs_config(config: Config) -> crate::storage::OpenRepositoryResult { + let creation_policy = if config.params.create_renders { + RenderStoreCreationPolicy::CreateIfMissing + } else { + RenderStoreCreationPolicy::DoNotCreate + }; - async fn from_config(config: Self::Config) -> crate::storage::OpenRepositoryResult { let repo = if config.params.create { - Self::create(&config.path).await + Self::create_with_policy(&config.path, creation_policy).await } else { - Self::open(&config.path).await + Self::open_with_policy(&config.path, creation_policy).await }; repo.map(|mut repo| { repo.set_tag_namespace(config.params.tag_namespace); @@ -515,34 +989,68 @@ impl FromConfig for OpenFsRepositoryImpl { } } -impl Clone for OpenFsRepositoryImpl { +impl Clone for OpenFsRepositoryImpl { fn clone(&self) -> Self { let root = self.root.clone(); Self { objects: FsHashStore::open_unchecked(root.join("objects")), payloads: FsHashStore::open_unchecked(root.join("payloads")), - renders: self.renders.clone(), + rs_impl: self.rs_impl.clone(), root, tag_namespace: self.tag_namespace.clone(), } } } -impl LocalRepository for OpenFsRepositoryImpl { +impl LocalPayloads for OpenFsRepositoryImpl { #[inline] fn payloads(&self) -> &FsHashStore { &self.payloads } +} - #[inline] - fn render_store(&self) -> Result<&RenderStore> { - self.renders - .as_ref() - .ok_or_else(|| Error::NoRenderStorage(self.address())) +impl LocalRenderStore for OpenFsRepositoryImpl +where + RS: LocalRenderStore, +{ + fn render_store(&self) -> &RenderStore { + self.rs_impl.render_store() + } +} + +impl RenderStoreForUser for OpenFsRepositoryImpl +where + RS: RenderStoreForUser, +{ + type RenderStore = RS; + + fn render_store_for_user( + creation_policy: RenderStoreCreationPolicy, + url: url::Url, + root: &Path, + username: &Path, + ) -> OpenRepositoryResult { + RS::render_store_for_user(creation_policy, url, root, username) + } +} + +impl TryRenderStore for OpenFsRepositoryImpl +where + RS: TryRenderStore, +{ + fn try_render_store(&self) -> OpenRepositoryResult> { + self.rs_impl.try_render_store() + } + + fn proxy_path(&self) -> Option> { + self.rs_impl.proxy_path() } } -impl OpenFsRepositoryImpl { +impl OpenFsRepositoryImpl +where + RS: DefaultRenderStoreCreationPolicy, +{ /// The address of this repository that can be used to re-open it pub fn address(&self) -> url::Url { Config { @@ -551,14 +1059,34 @@ impl OpenFsRepositoryImpl { create: false, lazy: false, tag_namespace: self.tag_namespace.clone(), + create_renders: RS::default_creation_policy().is_create_if_missing(), }, } .to_address() .expect("repository address is valid") } +} - /// Establish a new filesystem repository - pub async fn create>(root: P) -> OpenRepositoryResult { +impl OpenFsRepositoryImpl { + /// The latest repository version that this was migrated to. + pub async fn last_migration(&self) -> MigrationResult { + Ok(read_last_migration_version(self.root()) + .await? + .unwrap_or_else(|| { + semver::Version::parse(crate::VERSION) + .expect("crate::VERSION is a valid semver value") + })) + } +} + +impl OpenFsRepositoryImpl +where + RS: DefaultRenderStoreCreationPolicy + RenderStoreForUser, +{ + async fn create_with_policy( + root: impl AsRef, + creation_policy: RenderStoreCreationPolicy, + ) -> OpenRepositoryResult { let root = root.as_ref(); // avoid creating any blocking tasks so as to not spawn // threads for the case where this repo is being opened as @@ -575,12 +1103,11 @@ impl OpenFsRepositoryImpl { source, } })?; - let username = whoami::username(); + // let username = whoami::username(); for path in [ root.join("tags"), root.join("objects"), root.join("payloads"), - root.join("renders").join(username).join(PROXY_DIRNAME), root.join(DURABLE_EDITS_DIR), ] { makedirs_with_perms(&path, 0o777) @@ -592,35 +1119,13 @@ impl OpenFsRepositoryImpl { // `VERSION` to our version, so it is compatible. // FIXME: No attempt to check if the repo already existed and is // actually incompatible. - unsafe { Self::open_unchecked(root) } + unsafe { Self::open_unchecked_with_policy(root, creation_policy) } } - pub(crate) fn get_render_storage(&self) -> Result<&crate::storage::fs::FsHashStore> { - match &self.renders { - Some(render_store) => Ok(&render_store.renders), - None => Err(Error::NoRenderStorage(self.address())), - } - } - - /// Return the configured tag namespace, if any. - #[inline] - pub fn get_tag_namespace(&self) -> Option> { - self.tag_namespace.as_deref().map(Cow::Borrowed) - } - - /// The latest repository version that this was migrated to. - pub async fn last_migration(&self) -> MigrationResult { - Ok(read_last_migration_version(self.root()) - .await? - .unwrap_or_else(|| { - semver::Version::parse(crate::VERSION) - .expect("crate::VERSION is a valid semver value") - })) - } - - // Open a repository over the given directory, which must already - // exist and be a repository - pub async fn open>(root: P) -> OpenRepositoryResult { + async fn open_with_policy( + root: impl AsRef, + creation_policy: RenderStoreCreationPolicy, + ) -> OpenRepositoryResult { // although this is an async function, we avoid spawning a blocking task // here for the cases where a local fs repo is opened to spawn a runtime // and the program cannot spawn another thread without angering the kernel @@ -636,7 +1141,7 @@ impl OpenFsRepositoryImpl { // Safety: we canonicalized `root` and check the version compatibility // in the next step. - let repo = unsafe { Self::open_unchecked(&root)? }; + let repo = unsafe { Self::open_unchecked_with_policy(&root, creation_policy)? }; let current_version = semver::Version::parse(crate::VERSION).unwrap(); let repo_version = repo.last_migration().await?; @@ -650,8 +1155,20 @@ impl OpenFsRepositoryImpl { Ok(repo) } + /// Establish a new filesystem repository + pub async fn create(root: impl AsRef) -> OpenRepositoryResult { + Self::create_with_policy(root, RS::default_creation_policy()).await + } + + // Open a repository over the given directory, which must already + // exist and be a repository + pub async fn open(root: impl AsRef) -> OpenRepositoryResult { + Self::open_with_policy(root, RS::default_creation_policy()).await + } + /// Open a repository at the given directory, without reading or verifying - /// the migration version of the repository. + /// the migration version of the repository, with explicit render-store + /// creation policy. /// /// # Safety /// @@ -659,17 +1176,37 @@ impl OpenFsRepositoryImpl { /// /// The caller must ensure that the repository version is compatible with /// this version of spfs before using the repository. - unsafe fn open_unchecked>(root: P) -> OpenRepositoryResult { + unsafe fn open_unchecked_with_policy( + root: impl AsRef, + creation_policy: RenderStoreCreationPolicy, + ) -> OpenRepositoryResult { let root = root.as_ref(); - let username = whoami::username(); - Ok(Self { + let username = PathBuf::from(whoami::username()); + let url = url::Url::from_directory_path(root).map_err(|()| { + OpenRepositoryError::PathNotInitialized { + path: root.to_owned(), + source: std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "repository path is not a valid directory URL", + ), + } + })?; + Ok(OpenFsRepositoryImpl:: { objects: FsHashStore::open(root.join("objects"))?, payloads: FsHashStore::open(root.join("payloads"))?, - renders: RenderStore::for_user(root, username).ok(), + rs_impl: RS::render_store_for_user(creation_policy, url, root, &username)?, root: root.to_owned(), tag_namespace: None, }) } +} + +impl OpenFsRepositoryImpl { + /// Return the configured tag namespace, if any. + #[inline] + pub fn get_tag_namespace(&self) -> Option> { + self.tag_namespace.as_deref().map(Cow::Borrowed) + } /// The filesystem root path of this repository pub fn root(&self) -> PathBuf { @@ -693,41 +1230,53 @@ impl OpenFsRepositoryImpl { } } -#[async_trait::async_trait] -impl FsRepositoryOps for OpenFsRepositoryImpl { - /// True if this repo is setup to generate local manifest renders. - fn has_renders(&self) -> bool { - self.renders.is_some() +impl OpenFsRepositoryImpl { + /// Return a new version of this repository with render store creation + /// disabled. The render store will only be accessible if it already exists, + /// but will not be created on demand. + pub fn without_render_creation(self) -> Self { + Self { + root: self.root, + tag_namespace: self.tag_namespace, + payloads: self.payloads, + objects: self.objects, + rs_impl: self.rs_impl.without_render_creation(), + } } +} +#[async_trait::async_trait] +impl FsRepositoryOps for OpenFsRepositoryImpl +where + RS: TryRenderStore + RenderStoreForUser + Send + Sync, +{ fn iter_rendered_manifests( &self, ) -> Pin> + Send + Sync + '_>> { Box::pin(try_stream! { - let renders = self.get_render_storage()?; - for await digest in renders.iter() { - yield digest?; + if let Ok(store) = self.rs_impl.try_render_store() { + for await digest in store.renders.iter() { + yield digest?; + } } }) } - fn proxy_path(&self) -> Option<&std::path::Path> { - self.renders - .as_ref() - .map(|render_store| render_store.proxy.root()) + fn proxy_path(&self) -> Option> { + self.rs_impl.proxy_path() } async fn remove_rendered_manifest(&self, digest: crate::encoding::Digest) -> Result<()> { - let renders = match &self.renders { - Some(render_store) => &render_store.renders, - None => return Ok(()), - }; + let renders = &self + .try_render_store() + .map_err(|source| Error::failed_to_open_repository(self, source))? + .renders; let rendered_dirpath = renders.build_digest_path(&digest); let workdir = renders.workdir(); makedirs_with_perms(&workdir, renders.directory_permissions).map_err(|source| { Error::StorageWriteError("remove render create workdir", workdir.clone(), source) })?; - OpenFsRepository::remove_dir_atomically(&rendered_dirpath, &workdir).await + OpenFsRepository::::remove_dir_atomically(&rendered_dirpath, &workdir).await } async fn remove_rendered_manifest_if_older_than( @@ -735,10 +1284,10 @@ impl FsRepositoryOps for OpenFsRepositoryImpl { older_than: DateTime, digest: crate::encoding::Digest, ) -> Result { - let renders = match &self.renders { - Some(render_store) => &render_store.renders, - None => return Ok(false), - }; + let renders = &self + .try_render_store() + .map_err(|source| Error::failed_to_open_repository(self, source))? + .renders; let rendered_dirpath = renders.build_digest_path(&digest); let metadata = match tokio::fs::symlink_metadata(&rendered_dirpath).await { @@ -774,16 +1323,29 @@ impl FsRepositoryOps for OpenFsRepositoryImpl { /// /// Returns tuples of (username, `ManifestViewer`). fn renders_for_all_users(&self) -> Result> { - if !self.has_renders() { - return Ok(Vec::new()); - } - let mut render_dirs = Vec::new(); let renders_dir = self.root.join("renders"); - for entry in std::fs::read_dir(&renders_dir).map_err(|err| { - Error::StorageReadError("read_dir on renders dir", renders_dir.clone(), err) - })? { + for entry in { + match std::fs::read_dir(&renders_dir) { + Ok(entries) => entries, + Err(err) => { + match err.kind() { + std::io::ErrorKind::NotFound => { + // no renders dir means no renders, so just return empty + return Ok(Vec::new()); + } + _ => { + return Err(Error::StorageReadError( + "read_dir on renders dir", + renders_dir.clone(), + err, + )); + } + } + } + } + } { let entry = entry.map_err(|err| { Error::StorageReadError("entry in renders dir", renders_dir.clone(), err) })?; @@ -792,44 +1354,91 @@ impl FsRepositoryOps for OpenFsRepositoryImpl { if !dir.is_dir() { continue; } - render_dirs.push(( + render_dirs.push( entry .file_name() .to_str() .expect("filename is valid utf8") .to_string(), - dir, - )); + ); } - Ok(render_dirs + render_dirs .into_iter() - .map(|(username, dir)| -> (String, Self) { - ( - username, - Self { - objects: FsHashStore::open_unchecked(self.root.join("objects")), - payloads: FsHashStore::open_unchecked(self.root.join("payloads")), - renders: self - .renders - .as_ref() - .and_then(|_| RenderStore::for_user(self.root.as_ref(), dir).ok()), - root: self.root.clone(), - tag_namespace: self.tag_namespace.clone(), - }, - ) + .filter_map(|username| { + // Pre-validate the proxy directory on disk before calling + // render_store_for_user. Some RS types (e.g. MaybeRenderStore) + // defer validation, so render_store_for_user alone may succeed + // even when the on-disk proxy directory is missing. + let proxy_dir = self + .root + .join("renders") + .join(&username) + .join(PROXY_DIRNAME); + match std::fs::symlink_metadata(&proxy_dir) { + Ok(_) => {} + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + tracing::warn!( + %username, + "Skipping per-user render store (proxy directory not found)" + ); + return None; + } + Err(err) => { + tracing::warn!( + %username, + ?err, + "Skipping per-user render store (unable to read proxy directory)" + ); + return None; + } + } + + let rs_impl = match RS::render_store_for_user( + RenderStoreCreationPolicy::DoNotCreate, + self.address().into_owned(), + &self.root, + Path::new(&username), + ) { + Ok(rs) => rs, + Err( + OpenRepositoryError::PathNotInitialized { .. } + | OpenRepositoryError::RenderStorageUnavailable, + ) => { + tracing::warn!( + %username, + "Skipping per-user render store (not initialized or unavailable)" + ); + return None; + } + Err(source) => { + return Some(Err(Error::FailedToOpenRepository { + repository: format!(""), + source, + })); + } + }; + + let fs_impl = Self { + objects: FsHashStore::open_unchecked(self.root.join("objects")), + payloads: FsHashStore::open_unchecked(self.root.join("payloads")), + rs_impl, + root: self.root.clone(), + tag_namespace: self.tag_namespace.clone(), + }; + Some(Ok((username, fs_impl))) }) - .collect()) + .collect::>>() } } -impl Address for OpenFsRepositoryImpl { +impl Address for OpenFsRepositoryImpl { fn address(&self) -> Cow<'_, url::Url> { Cow::Owned(url::Url::from_directory_path(self.root()).unwrap()) } } -impl std::fmt::Debug for OpenFsRepositoryImpl { +impl std::fmt::Debug for OpenFsRepositoryImpl { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_fmt(format_args!("OpenFsRepositoryImpl @ {:?}", self.root())) } diff --git a/crates/spfs/src/storage/fs/repository_test.rs b/crates/spfs/src/storage/fs/repository_test.rs new file mode 100644 index 0000000000..9818bf559f --- /dev/null +++ b/crates/spfs/src/storage/fs/repository_test.rs @@ -0,0 +1,411 @@ +// Copyright (c) Contributors to the SPK project. +// SPDX-License-Identifier: Apache-2.0 +// https://github.com/spkenv/spk + +use std::borrow::Cow; +use std::path::{Path, PathBuf}; +use std::sync::{Mutex, OnceLock}; + +use super::{ + DefaultRenderStoreCreationPolicy, + FsHashStore, + FsRepositoryOps, + MaybeOpenFsRepository, + MaybeRenderStore, + OpenFsRepositoryImpl, + RenderStore, + RenderStoreCreationPolicy, +}; +use crate::storage::{ + OpenRepositoryError, + OpenRepositoryResult, + RenderStoreForUser, + TryRenderStore, +}; + +#[derive(Clone, Debug)] +struct RecordingRenderStore; + +fn recorded_user_paths() -> &'static Mutex> { + static RECORDED_USER_PATHS: OnceLock>> = OnceLock::new(); + RECORDED_USER_PATHS.get_or_init(|| Mutex::new(Vec::new())) +} + +impl DefaultRenderStoreCreationPolicy for RecordingRenderStore { + fn default_creation_policy() -> RenderStoreCreationPolicy { + RenderStoreCreationPolicy::DoNotCreate + } +} + +impl RenderStoreForUser for RecordingRenderStore { + type RenderStore = Self; + + fn render_store_for_user( + _creation_policy: RenderStoreCreationPolicy, + _url: url::Url, + _root: &Path, + username: &Path, + ) -> OpenRepositoryResult { + recorded_user_paths() + .lock() + .expect("recorded user paths mutex should not be poisoned") + .push(username.to_path_buf()); + Ok(Self) + } +} + +impl TryRenderStore for RecordingRenderStore { + fn try_render_store(&self) -> OpenRepositoryResult> { + Err(OpenRepositoryError::RenderStorageUnavailable) + } + + fn proxy_path(&self) -> Option> { + None + } +} + +#[tokio::test] +async fn test_render_store_for_user_create_if_missing_creates_proxy_dir() { + let tmpdir = tempfile::Builder::new() + .prefix("spfs-test-") + .tempdir() + .unwrap(); + let root = tmpdir.path().join("repo"); + std::fs::create_dir_all(&root).unwrap(); + + let username = PathBuf::from("test-user-create"); + let url = url::Url::from_directory_path(&root).unwrap(); + + let store = RenderStore::render_store_for_user( + RenderStoreCreationPolicy::CreateIfMissing, + url, + &root, + &username, + ) + .unwrap(); + + assert!( + store.proxy.root().is_dir(), + "proxy dir should exist when create-if-missing is used" + ); + assert!( + store.renders.root().is_dir(), + "renders dir should exist when create-if-missing is used" + ); +} + +#[tokio::test] +async fn test_render_store_for_user_do_not_create_returns_error() { + let tmpdir = tempfile::Builder::new() + .prefix("spfs-test-") + .tempdir() + .unwrap(); + let root = tmpdir.path().join("repo"); + std::fs::create_dir_all(&root).unwrap(); + + let username = PathBuf::from("test-user-no-create"); + let url = url::Url::from_directory_path(&root).unwrap(); + + let err = RenderStore::render_store_for_user( + RenderStoreCreationPolicy::DoNotCreate, + url, + &root, + &username, + ) + .expect_err("do-not-create should fail when proxy dir does not exist"); + + assert!( + matches!( + err, + crate::storage::OpenRepositoryError::PathNotInitialized { .. } + ), + "do-not-create should fail with PathNotInitialized" + ); + + let proxy_dir = root.join("renders").join(&username).join("proxy"); + assert!( + !proxy_dir.exists(), + "do-not-create should not create the proxy path" + ); +} + +#[tokio::test] +async fn test_render_store_for_user_create_if_missing_returns_non_not_found_errors() { + let tmpdir = tempfile::Builder::new() + .prefix("spfs-test-") + .tempdir() + .unwrap(); + let root = tmpdir.path().join("repo"); + std::fs::create_dir_all(&root).unwrap(); + + let username = PathBuf::from("test-user-enotdir"); + let renders_dir = root.join("renders").join(&username); + std::fs::create_dir_all(renders_dir.parent().unwrap()).unwrap(); + std::fs::write(&renders_dir, b"not a directory").unwrap(); + + let url = url::Url::from_directory_path(&root).unwrap(); + let err = RenderStore::render_store_for_user( + RenderStoreCreationPolicy::CreateIfMissing, + url, + &root, + &username, + ) + .expect_err("create-if-missing should return non-NotFound metadata errors"); + + assert!( + matches!( + err, + crate::storage::OpenRepositoryError::PathNotInitialized { .. } + ), + "create-if-missing should preserve metadata errors that are not NotFound" + ); +} + +#[tokio::test] +async fn test_without_render_creation_disables_lazy_render_store_creation() { + let tmpdir = tempfile::Builder::new() + .prefix("spfs-test-") + .tempdir() + .unwrap(); + let root = tmpdir.path().join("repo"); + + let repo = MaybeOpenFsRepository::::create(&root) + .await + .unwrap(); + let repo = repo.without_render_creation(); + let opened = repo.opened().await.unwrap(); + + let err = opened + .fs_impl + .try_render_store() + .expect_err("without_render_creation should not create missing render store"); + assert!( + matches!( + err, + crate::storage::OpenRepositoryError::PathNotInitialized { .. } + ), + "missing render store should still be reported as not initialized" + ); +} + +#[tokio::test] +async fn test_try_from_maybe_open_repo_to_render_repo_fails_with_creation_disabled() { + let tmpdir = tempfile::Builder::new() + .prefix("spfs-test-") + .tempdir() + .unwrap(); + let root = tmpdir.path().join("repo"); + + let repo = MaybeOpenFsRepository::::create(&root) + .await + .unwrap() + .without_render_creation(); + let err = >::try_from(repo) + .expect_err("conversion should fail when renders have not been created"); + assert!( + matches!( + err, + crate::storage::OpenRepositoryError::PathNotInitialized { .. } + ), + "missing render store should fail with PathNotInitialized" + ); +} + +#[tokio::test] +async fn test_try_from_maybe_open_repo_to_render_repo_succeeds_after_render_store_exists() { + let tmpdir = tempfile::Builder::new() + .prefix("spfs-test-") + .tempdir() + .unwrap(); + let root = tmpdir.path().join("repo"); + + let repo = MaybeOpenFsRepository::::create(&root) + .await + .unwrap(); + let opened = repo.opened().await.unwrap(); + opened + .fs_impl + .try_render_store() + .expect("create the render store before conversion"); + + let converted = >::try_from(repo); + assert!( + converted.is_ok(), + "conversion should succeed once the render store exists" + ); +} + +#[tokio::test] +async fn test_renders_for_all_users_passes_username_segment_to_render_store_for_user() { + let tmpdir = tempfile::Builder::new() + .prefix("spfs-test-") + .tempdir() + .unwrap(); + let root = tmpdir.path().join("repo"); + std::fs::create_dir_all(root.join("objects")).unwrap(); + std::fs::create_dir_all(root.join("payloads")).unwrap(); + + let username = "test-user-segment"; + std::fs::create_dir_all(root.join("renders").join(username).join("proxy")).unwrap(); + + recorded_user_paths() + .lock() + .expect("recorded user paths mutex should not be poisoned") + .clear(); + + let repo = OpenFsRepositoryImpl:: { + objects: FsHashStore::open_unchecked(root.join("objects")), + payloads: FsHashStore::open_unchecked(root.join("payloads")), + rs_impl: RecordingRenderStore, + root: root.clone(), + tag_namespace: None, + }; + + let renders = repo.renders_for_all_users().unwrap(); + assert_eq!( + renders.len(), + 1, + "one user render directory should produce one repository entry" + ); + assert_eq!( + renders[0].0, username, + "returned username should match the renders directory name" + ); + + let recorded = recorded_user_paths() + .lock() + .expect("recorded user paths mutex should not be poisoned") + .clone(); + assert_eq!( + recorded, + vec![PathBuf::from(username)], + "render_store_for_user should receive a username segment, not renders/" + ); + assert!( + !recorded[0].to_string_lossy().contains("renders"), + "forwarded path should not include the renders directory prefix" + ); +} + +/// A render store that fails with [`OpenRepositoryError::PathNotInitialized`] +/// for usernames containing "bad-". +#[derive(Clone, Debug)] +struct FailingRenderStore; + +impl DefaultRenderStoreCreationPolicy for FailingRenderStore { + fn default_creation_policy() -> RenderStoreCreationPolicy { + RenderStoreCreationPolicy::DoNotCreate + } +} + +impl RenderStoreForUser for FailingRenderStore { + type RenderStore = Self; + + fn render_store_for_user( + _creation_policy: RenderStoreCreationPolicy, + _url: url::Url, + _root: &Path, + username: &Path, + ) -> OpenRepositoryResult { + if username.to_string_lossy().contains("bad-") { + return Err(OpenRepositoryError::PathNotInitialized { + path: username.to_path_buf(), + source: std::io::Error::new(std::io::ErrorKind::NotFound, "simulated"), + }); + } + Ok(Self) + } +} + +impl TryRenderStore for FailingRenderStore { + fn try_render_store(&self) -> OpenRepositoryResult> { + Err(OpenRepositoryError::RenderStorageUnavailable) + } + + fn proxy_path(&self) -> Option> { + None + } +} + +#[tokio::test] +async fn test_renders_for_all_users_skips_unavailable_per_user_stores() { + let tmpdir = tempfile::Builder::new() + .prefix("spfs-test-") + .tempdir() + .unwrap(); + let root = tmpdir.path().join("repo"); + std::fs::create_dir_all(root.join("objects")).unwrap(); + std::fs::create_dir_all(root.join("payloads")).unwrap(); + + // Create two user directories with proxy dirs: one good, one that will + // fail in render_store_for_user due to the "bad-" prefix. + std::fs::create_dir_all(root.join("renders").join("good-user").join("proxy")).unwrap(); + std::fs::create_dir_all(root.join("renders").join("bad-user").join("proxy")).unwrap(); + + let repo = OpenFsRepositoryImpl:: { + objects: FsHashStore::open_unchecked(root.join("objects")), + payloads: FsHashStore::open_unchecked(root.join("payloads")), + rs_impl: FailingRenderStore, + root: root.clone(), + tag_namespace: None, + }; + + let renders = repo + .renders_for_all_users() + .expect("should not fail even though one user store is unavailable"); + assert_eq!( + renders.len(), + 1, + "only the good user should be returned, the bad user should be skipped" + ); + assert_eq!( + renders[0].0, "good-user", + "the returned user should be the one whose render store succeeded" + ); +} + +#[tokio::test] +async fn test_renders_for_all_users_skips_deferred_render_store_with_missing_proxy() { + // MaybeRenderStore defers on-disk validation, so render_store_for_user() + // always succeeds. This test verifies that renders_for_all_users still + // skips users whose on-disk proxy directory is missing. + let tmpdir = tempfile::Builder::new() + .prefix("spfs-test-") + .tempdir() + .unwrap(); + let root = tmpdir.path().join("repo"); + std::fs::create_dir_all(root.join("objects")).unwrap(); + std::fs::create_dir_all(root.join("payloads")).unwrap(); + + // Create a user render directory but do NOT create the proxy subdirectory + // inside it, so try_render_store() will fail with PathNotInitialized. + std::fs::create_dir_all(root.join("renders").join("missing-proxy-user")).unwrap(); + + // Also create a valid user with proxy dir so we can verify it is returned. + let valid_user = "valid-user"; + std::fs::create_dir_all(root.join("renders").join(valid_user).join("proxy")).unwrap(); + + let url = url::Url::from_directory_path(&root).unwrap(); + let repo = OpenFsRepositoryImpl:: { + objects: FsHashStore::open_unchecked(root.join("objects")), + payloads: FsHashStore::open_unchecked(root.join("payloads")), + rs_impl: MaybeRenderStore::render_store_for_user( + RenderStoreCreationPolicy::DoNotCreate, + url, + &root, + Path::new("_unused"), + ) + .unwrap(), + root: root.clone(), + tag_namespace: None, + }; + + let renders = repo + .renders_for_all_users() + .expect("should not fail even though one user has no proxy dir"); + assert_eq!(renders.len(), 1, "only the valid user should be returned"); + assert_eq!( + renders[0].0, valid_user, + "the returned user should be the one with a valid proxy directory" + ); +} diff --git a/crates/spfs/src/storage/fs/tag.rs b/crates/spfs/src/storage/fs/tag.rs index 818aed641f..e92a122c4f 100644 --- a/crates/spfs/src/storage/fs/tag.rs +++ b/crates/spfs/src/storage/fs/tag.rs @@ -20,9 +20,10 @@ use futures::{Future, Stream, StreamExt, TryFutureExt}; use relative_path::RelativePath; use tokio::io::{AsyncRead, AsyncSeek, AsyncWriteExt, ReadBuf}; -use super::{MaybeOpenFsRepository, OpenFsRepository}; +use super::{DefaultRenderStoreCreationPolicy, MaybeOpenFsRepository, OpenFsRepository}; use crate::storage::tag::{EntryType, TagSpecAndTagStream, TagStream}; use crate::storage::{ + RenderStoreForUser, TAG_NAMESPACE_MARKER, TagNamespace, TagNamespaceBuf, @@ -34,7 +35,14 @@ use crate::{Error, OsError, OsErrorExt, Result, encoding, tracking}; const TAG_EXT: &str = "tag"; #[async_trait::async_trait] -impl TagStorage for MaybeOpenFsRepository { +impl TagStorage for MaybeOpenFsRepository +where + RS: DefaultRenderStoreCreationPolicy + + RenderStoreForUser + + Send + + Sync + + 'static, +{ #[inline] fn get_tag_namespace(&self) -> Option> { self.fs_impl.get_tag_namespace() @@ -131,7 +139,14 @@ impl TagStorage for MaybeOpenFsRepository { } } -impl MaybeOpenFsRepository { +impl MaybeOpenFsRepository +where + RS: DefaultRenderStoreCreationPolicy + + RenderStoreForUser + + Send + + Sync + + 'static, +{ /// Forcefully remove any lock file for the identified tag. /// /// # Safety @@ -154,7 +169,7 @@ impl MaybeOpenFsRepository { } } -impl OpenFsRepository { +impl OpenFsRepository { fn tags_root_in_namespace(&self, namespace: Option<&TagNamespace>) -> PathBuf { let mut tags_root = self.root().join("tags"); if let Some(tag_namespace) = namespace { @@ -192,7 +207,10 @@ impl OpenFsRepository { } #[async_trait::async_trait] -impl TagStorage for OpenFsRepository { +impl TagStorage for OpenFsRepository +where + RS: Send + Sync, +{ #[inline] fn get_tag_namespace(&self) -> Option> { self.fs_impl.get_tag_namespace() @@ -448,7 +466,10 @@ impl TagStorage for OpenFsRepository { } } -impl TagStorageMut for MaybeOpenFsRepository { +impl TagStorageMut for MaybeOpenFsRepository +where + RS: Clone, +{ fn try_set_tag_namespace( &mut self, tag_namespace: Option, diff --git a/crates/spfs/src/storage/handle.rs b/crates/spfs/src/storage/handle.rs index 846b763078..c26189cb16 100644 --- a/crates/spfs/src/storage/handle.rs +++ b/crates/spfs/src/storage/handle.rs @@ -11,6 +11,7 @@ use futures::Stream; use relative_path::RelativePath; use spfs_encoding as encoding; +use super::fs::{MaybeRenderStore, NoRenderStore, RenderStore}; use super::prelude::*; use super::tag::TagSpecAndTagStream; use super::{TagNamespace, TagNamespaceBuf, TagStorageMut}; @@ -21,10 +22,14 @@ use crate::{Error, Result, graph}; #[derive(Debug)] #[allow(clippy::large_enum_variant)] pub enum RepositoryHandle { - FS(super::fs::MaybeOpenFsRepository), + FSWithMaybeRenders(super::fs::MaybeOpenFsRepository), + FSWithRenders(super::fs::MaybeOpenFsRepository), + FSWithoutRenders(super::fs::MaybeOpenFsRepository), Tar(super::tar::TarRepository), Rpc(super::rpc::RpcRepository), - FallbackProxy(Box), + FallbackProxyWithMaybeRenders(Box>), + FallbackProxyWithRenders(Box>), + FallbackProxyWithoutRenders(Box>), Proxy(Box), Pinned(Box>), } @@ -70,31 +75,71 @@ impl RepositoryHandle { pub fn try_as_tag_mut(&mut self) -> Result<&mut dyn TagStorageMut> { match self { - RepositoryHandle::FS(repo) => Ok(repo), + RepositoryHandle::FSWithMaybeRenders(repo) => Ok(repo), + RepositoryHandle::FSWithRenders(repo) => Ok(repo), + RepositoryHandle::FSWithoutRenders(repo) => Ok(repo), RepositoryHandle::Tar(repo) => Ok(repo), RepositoryHandle::Rpc(repo) => Ok(repo), - RepositoryHandle::FallbackProxy(repo) => Ok(&mut **repo), + RepositoryHandle::FallbackProxyWithMaybeRenders(repo) => Ok(&mut **repo), + RepositoryHandle::FallbackProxyWithRenders(repo) => Ok(&mut **repo), + RepositoryHandle::FallbackProxyWithoutRenders(repo) => Ok(&mut **repo), RepositoryHandle::Proxy(repo) => Ok(&mut **repo), RepositoryHandle::Pinned(_) => Err(Error::RepositoryIsPinned), } } } -impl From for RepositoryHandle { - fn from(repo: super::fs::MaybeOpenFsRepository) -> Self { - RepositoryHandle::FS(repo) +impl From> for RepositoryHandle { + fn from(repo: super::fs::MaybeOpenFsRepository) -> Self { + RepositoryHandle::FSWithRenders(repo) } } -impl From for RepositoryHandle { - fn from(repo: super::fs::OpenFsRepository) -> Self { - RepositoryHandle::FS(repo.into()) +impl From> for RepositoryHandle { + fn from(repo: super::fs::OpenFsRepository) -> Self { + RepositoryHandle::FSWithRenders(repo.into()) } } -impl From> for RepositoryHandle { - fn from(repo: Arc) -> Self { - RepositoryHandle::FS(repo.into()) +impl From>> for RepositoryHandle { + fn from(repo: Arc>) -> Self { + RepositoryHandle::FSWithRenders(repo.into()) + } +} + +impl From> for RepositoryHandle { + fn from(repo: super::fs::MaybeOpenFsRepository) -> Self { + RepositoryHandle::FSWithoutRenders(repo) + } +} + +impl From> for RepositoryHandle { + fn from(repo: super::fs::OpenFsRepository) -> Self { + RepositoryHandle::FSWithoutRenders(repo.into()) + } +} + +impl From>> for RepositoryHandle { + fn from(repo: Arc>) -> Self { + RepositoryHandle::FSWithoutRenders(repo.into()) + } +} + +impl From> for RepositoryHandle { + fn from(repo: super::fs::MaybeOpenFsRepository) -> Self { + RepositoryHandle::FSWithMaybeRenders(repo) + } +} + +impl From> for RepositoryHandle { + fn from(repo: super::fs::OpenFsRepository) -> Self { + RepositoryHandle::FSWithMaybeRenders(repo.into()) + } +} + +impl From>> for RepositoryHandle { + fn from(repo: Arc>) -> Self { + RepositoryHandle::FSWithMaybeRenders(repo.into()) } } @@ -110,9 +155,21 @@ impl From for RepositoryHandle { } } -impl From for RepositoryHandle { - fn from(repo: super::fallback::FallbackProxy) -> Self { - RepositoryHandle::FallbackProxy(Box::new(repo)) +impl From> for RepositoryHandle { + fn from(repo: super::fallback::FallbackProxy) -> Self { + RepositoryHandle::FallbackProxyWithRenders(Box::new(repo)) + } +} + +impl From> for RepositoryHandle { + fn from(repo: super::fallback::FallbackProxy) -> Self { + RepositoryHandle::FallbackProxyWithMaybeRenders(Box::new(repo)) + } +} + +impl From> for RepositoryHandle { + fn from(repo: super::fallback::FallbackProxy) -> Self { + RepositoryHandle::FallbackProxyWithoutRenders(Box::new(repo)) } } @@ -134,10 +191,14 @@ impl From>> for Repository macro_rules! each_variant { ($repo:expr, $inner:ident, $ops:tt) => { match $repo { - RepositoryHandle::FS($inner) => $ops, + RepositoryHandle::FSWithMaybeRenders($inner) => $ops, + RepositoryHandle::FSWithRenders($inner) => $ops, + RepositoryHandle::FSWithoutRenders($inner) => $ops, RepositoryHandle::Tar($inner) => $ops, RepositoryHandle::Rpc($inner) => $ops, - RepositoryHandle::FallbackProxy($inner) => $ops, + RepositoryHandle::FallbackProxyWithMaybeRenders($inner) => $ops, + RepositoryHandle::FallbackProxyWithRenders($inner) => $ops, + RepositoryHandle::FallbackProxyWithoutRenders($inner) => $ops, RepositoryHandle::Proxy($inner) => $ops, RepositoryHandle::Pinned($inner) => $ops, } @@ -241,10 +302,20 @@ impl TagStorageMut for RepositoryHandle { tag_namespace: Option, ) -> Result> { match self { - RepositoryHandle::FS(repo) => repo.try_set_tag_namespace(tag_namespace), + RepositoryHandle::FSWithMaybeRenders(repo) => repo.try_set_tag_namespace(tag_namespace), + RepositoryHandle::FSWithRenders(repo) => repo.try_set_tag_namespace(tag_namespace), + RepositoryHandle::FSWithoutRenders(repo) => repo.try_set_tag_namespace(tag_namespace), RepositoryHandle::Tar(repo) => repo.try_set_tag_namespace(tag_namespace), RepositoryHandle::Rpc(repo) => repo.try_set_tag_namespace(tag_namespace), - RepositoryHandle::FallbackProxy(repo) => repo.try_set_tag_namespace(tag_namespace), + RepositoryHandle::FallbackProxyWithMaybeRenders(repo) => { + repo.try_set_tag_namespace(tag_namespace) + } + RepositoryHandle::FallbackProxyWithRenders(repo) => { + repo.try_set_tag_namespace(tag_namespace) + } + RepositoryHandle::FallbackProxyWithoutRenders(repo) => { + repo.try_set_tag_namespace(tag_namespace) + } RepositoryHandle::Proxy(repo) => repo.try_set_tag_namespace(tag_namespace), RepositoryHandle::Pinned(_) => Err(Error::RepositoryIsPinned), } diff --git a/crates/spfs/src/storage/mod.rs b/crates/spfs/src/storage/mod.rs index b719f2c2b5..e827069cb8 100644 --- a/crates/spfs/src/storage/mod.rs +++ b/crates/spfs/src/storage/mod.rs @@ -26,13 +26,21 @@ pub mod tar; pub use address::Address; pub use blob::{BlobStorage, BlobStorageExt}; pub use error::OpenRepositoryError; +pub use fs::DefaultRenderStoreCreationPolicy; pub use handle::RepositoryHandle; pub use layer::{LayerStorage, LayerStorageExt}; pub use manifest::ManifestStorage; pub use payload::PayloadStorage; pub use platform::{PlatformStorage, PlatformStorageExt}; pub use proxy::{Config, ProxyRepository}; -pub use repository::{LocalRepository, Repository, RepositoryExt}; +pub use repository::{ + LocalPayloads, + LocalRenderStore, + RenderStoreForUser, + Repository, + RepositoryExt, + TryRenderStore, +}; pub use tag::{EntryType, TagStorage, TagStorageMut}; pub use tag_namespace::{TAG_NAMESPACE_MARKER, TagNamespace, TagNamespaceBuf}; diff --git a/crates/spfs/src/storage/proxy/repository.rs b/crates/spfs/src/storage/proxy/repository.rs index 216676e77b..635bd361d6 100644 --- a/crates/spfs/src/storage/proxy/repository.rs +++ b/crates/spfs/src/storage/proxy/repository.rs @@ -93,7 +93,9 @@ impl ProxyRepository { impl storage::FromConfig for ProxyRepository { type Config = Config; - async fn from_config(config: Self::Config) -> OpenRepositoryResult { + async fn from_config( + config: Self::Config, + ) -> OpenRepositoryResult { let spfs_config = crate::Config::current().map_err(|source| OpenRepositoryError::FailedToLoadConfig { source: Box::new(source), @@ -124,7 +126,8 @@ impl storage::FromConfig for ProxyRepository { primary, secondary, include_secondary_tags: config.include_secondary_tags, - }) + } + .into()) } } diff --git a/crates/spfs/src/storage/proxy/repository_test.rs b/crates/spfs/src/storage/proxy/repository_test.rs index d0b17a05e4..45ab27302b 100644 --- a/crates/spfs/src/storage/proxy/repository_test.rs +++ b/crates/spfs/src/storage/proxy/repository_test.rs @@ -10,6 +10,7 @@ use rstest::rstest; use crate::config::default_proxy_repo_include_secondary_tags; use crate::fixtures::*; use crate::prelude::*; +use crate::storage::fs::RenderStore; use crate::storage::proxy::repository::RelativePath; #[rstest] @@ -17,13 +18,16 @@ use crate::storage::proxy::repository::RelativePath; async fn test_proxy_payload_read_through(tmpdir: tempfile::TempDir) { init_logging(); - let primary = crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("primary")) - .await - .unwrap(); - let secondary = - crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("secondary")) - .await - .unwrap(); + let primary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("primary"), + ) + .await + .unwrap(); + let secondary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("secondary"), + ) + .await + .unwrap(); let digest = secondary .commit_blob(Box::pin(b"some data".as_slice())) @@ -47,13 +51,16 @@ async fn test_proxy_payload_read_through(tmpdir: tempfile::TempDir) { async fn test_proxy_object_read_through(tmpdir: tempfile::TempDir) { init_logging(); - let primary = crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("primary")) - .await - .unwrap(); - let secondary = - crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("secondary")) - .await - .unwrap(); + let primary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("primary"), + ) + .await + .unwrap(); + let secondary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("secondary"), + ) + .await + .unwrap(); let payload = secondary .commit_blob(Box::pin(b"some data".as_slice())) @@ -77,13 +84,16 @@ async fn test_proxy_object_read_through(tmpdir: tempfile::TempDir) { async fn test_proxy_tag_read_through(tmpdir: tempfile::TempDir) { init_logging(); - let primary = crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("primary")) - .await - .unwrap(); - let secondary = - crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("secondary")) - .await - .unwrap(); + let primary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("primary"), + ) + .await + .unwrap(); + let secondary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("secondary"), + ) + .await + .unwrap(); let payload = secondary .commit_blob(Box::pin(b"some data".as_slice())) @@ -109,9 +119,11 @@ async fn test_proxy_tag_read_through(tmpdir: tempfile::TempDir) { async fn test_proxy_tag_ls(tmpdir: tempfile::TempDir) { init_logging(); - let primary = crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("primary")) - .await - .unwrap(); + let primary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("primary"), + ) + .await + .unwrap(); let payload1 = primary .commit_blob(Box::pin(b"some data".as_slice())) @@ -120,10 +132,11 @@ async fn test_proxy_tag_ls(tmpdir: tempfile::TempDir) { let tag_spec = crate::tracking::TagSpec::parse("spfs-test/proxy-read-through").unwrap(); primary.push_tag(&tag_spec, &payload1).await.unwrap(); - let secondary = - crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("secondary")) - .await - .unwrap(); + let secondary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("secondary"), + ) + .await + .unwrap(); let payload2 = secondary .commit_blob(Box::pin(b"some data".as_slice())) @@ -158,9 +171,11 @@ async fn test_proxy_tag_ls(tmpdir: tempfile::TempDir) { async fn test_proxy_tag_ls_config_for_primary_only(tmpdir: tempfile::TempDir) { init_logging(); - let primary = crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("primary")) - .await - .unwrap(); + let primary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("primary"), + ) + .await + .unwrap(); let payload1 = primary .commit_blob(Box::pin(b"some data".as_slice())) @@ -169,10 +184,11 @@ async fn test_proxy_tag_ls_config_for_primary_only(tmpdir: tempfile::TempDir) { let tag_spec = crate::tracking::TagSpec::parse("spfs-test/proxy-read-through").unwrap(); primary.push_tag(&tag_spec, &payload1).await.unwrap(); - let secondary = - crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("secondary")) - .await - .unwrap(); + let secondary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("secondary"), + ) + .await + .unwrap(); let payload2 = secondary .commit_blob(Box::pin(b"some data".as_slice())) @@ -218,9 +234,11 @@ async fn test_proxy_tag_ls_config_for_primary_only(tmpdir: tempfile::TempDir) { async fn test_proxy_tag_find(tmpdir: tempfile::TempDir) { init_logging(); - let primary = crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("primary")) - .await - .unwrap(); + let primary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("primary"), + ) + .await + .unwrap(); let payload1 = primary .commit_blob(Box::pin(b"some data".as_slice())) @@ -229,10 +247,11 @@ async fn test_proxy_tag_find(tmpdir: tempfile::TempDir) { let tag_spec = crate::tracking::TagSpec::parse("spfs-test/proxy-read-through").unwrap(); primary.push_tag(&tag_spec, &payload1).await.unwrap(); - let secondary = - crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("secondary")) - .await - .unwrap(); + let secondary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("secondary"), + ) + .await + .unwrap(); let payload2 = secondary .commit_blob(Box::pin(b"some data".as_slice())) @@ -266,9 +285,11 @@ async fn test_proxy_tag_find(tmpdir: tempfile::TempDir) { async fn test_proxy_tag_find_for_primary_only(tmpdir: tempfile::TempDir) { init_logging(); - let primary = crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("primary")) - .await - .unwrap(); + let primary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("primary"), + ) + .await + .unwrap(); let payload1 = primary .commit_blob(Box::pin(b"some data".as_slice())) @@ -277,10 +298,11 @@ async fn test_proxy_tag_find_for_primary_only(tmpdir: tempfile::TempDir) { let tag_spec = crate::tracking::TagSpec::parse("spfs-test/proxy-read-through").unwrap(); primary.push_tag(&tag_spec, &payload1).await.unwrap(); - let secondary = - crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("secondary")) - .await - .unwrap(); + let secondary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("secondary"), + ) + .await + .unwrap(); let payload2 = secondary .commit_blob(Box::pin(b"some data".as_slice())) @@ -317,9 +339,11 @@ async fn test_proxy_tag_find_for_primary_only(tmpdir: tempfile::TempDir) { async fn test_proxy_tag_iter_streams(tmpdir: tempfile::TempDir) { init_logging(); - let primary = crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("primary")) - .await - .unwrap(); + let primary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("primary"), + ) + .await + .unwrap(); let payload1 = primary .commit_blob(Box::pin(b"some data".as_slice())) @@ -328,10 +352,11 @@ async fn test_proxy_tag_iter_streams(tmpdir: tempfile::TempDir) { let tag_spec = crate::tracking::TagSpec::parse("spfs-test/proxy-read-through").unwrap(); primary.push_tag(&tag_spec, &payload1).await.unwrap(); - let secondary = - crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("secondary")) - .await - .unwrap(); + let secondary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("secondary"), + ) + .await + .unwrap(); let payload2 = secondary .commit_blob(Box::pin(b"some data".as_slice())) @@ -365,9 +390,11 @@ async fn test_proxy_tag_iter_streams(tmpdir: tempfile::TempDir) { async fn test_proxy_tag_iter_streams_for_primary_only(tmpdir: tempfile::TempDir) { init_logging(); - let primary = crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("primary")) - .await - .unwrap(); + let primary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("primary"), + ) + .await + .unwrap(); let payload1 = primary .commit_blob(Box::pin(b"some data".as_slice())) @@ -376,10 +403,11 @@ async fn test_proxy_tag_iter_streams_for_primary_only(tmpdir: tempfile::TempDir) let tag_spec = crate::tracking::TagSpec::parse("spfs-test/proxy-read-through").unwrap(); primary.push_tag(&tag_spec, &payload1).await.unwrap(); - let secondary = - crate::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("secondary")) - .await - .unwrap(); + let secondary = crate::storage::fs::MaybeOpenFsRepository::::create( + tmpdir.path().join("secondary"), + ) + .await + .unwrap(); let payload2 = secondary .commit_blob(Box::pin(b"some data".as_slice())) diff --git a/crates/spfs/src/storage/repository.rs b/crates/spfs/src/storage/repository.rs index aba782d1f4..0248f46360 100644 --- a/crates/spfs/src/storage/repository.rs +++ b/crates/spfs/src/storage/repository.rs @@ -2,14 +2,17 @@ // SPDX-License-Identifier: Apache-2.0 // https://github.com/spkenv/spk +use std::borrow::Cow; use std::collections::HashSet; +use std::path::Path; use std::pin::Pin; use async_trait::async_trait; use encoding::prelude::*; use tokio_stream::StreamExt; -use super::fs::{FsHashStore, RenderStore}; +use super::OpenRepositoryResult; +use super::fs::{FsHashStore, RenderStore, RenderStoreCreationPolicy}; use crate::tracking::{self, BlobRead}; use crate::{Error, Result, encoding, graph}; @@ -135,14 +138,51 @@ pub trait RepositoryExt: super::PayloadStorage + graph::DatabaseExt { impl RepositoryExt for T where T: super::PayloadStorage + graph::DatabaseExt {} /// Accessor methods for types only applicable to repositories that have -/// payloads and renders, e.g., local repositories. -pub trait LocalRepository { +/// payloads, e.g., local repositories. +pub trait LocalPayloads { /// Return the payload storage type fn payloads(&self) -> &FsHashStore; +} + +/// A trait for types that can support render stores, this provides a way to +/// instantiate those types. +pub trait RenderStoreForUser { + type RenderStore; + + /// Create an instance of the render store for the given user. + /// + /// This doesn't necessarily create the render store on disk; it depends on + /// the implementation. + /// + /// The `url` parameter is the URL of the repository the render store + /// belongs to. + fn render_store_for_user( + creation_policy: RenderStoreCreationPolicy, + url: url::Url, + root: &Path, + username: &Path, + ) -> OpenRepositoryResult; +} - /// If supported, returns the type responsible for locally rendered manifests +/// A trait for types that might have render store, but no guarantees are made +/// that it exists or is accessible. +pub trait TryRenderStore { + /// Return the render store for repositories that support it. /// - /// # Errors: - /// - [`Error::NoRenderStorage`] - if this repository does not support manifest rendering - fn render_store(&self) -> Result<&RenderStore>; + /// For some types this may create the render store for the user on demand + /// and may fail. + fn try_render_store(&self) -> OpenRepositoryResult>; + + /// Return the proxy path for the render store, creating it if the render + /// store is configured to. + /// + /// This only returns `Some` when the path exists. + fn proxy_path(&self) -> Option>; +} + +/// Accessor methods for types only applicable to repositories that have +/// renders, e.g., local repositories. +pub trait LocalRenderStore: RenderStoreForUser { + /// Returns the type responsible for locally rendered manifests + fn render_store(&self) -> &RenderStore; } diff --git a/crates/spfs/src/storage/repository_test.rs b/crates/spfs/src/storage/repository_test.rs index 9a78ec5ef2..5d495104e7 100644 --- a/crates/spfs/src/storage/repository_test.rs +++ b/crates/spfs/src/storage/repository_test.rs @@ -65,10 +65,12 @@ async fn test_find_aliases( #[rstest] #[tokio::test] async fn test_commit_mode_fs(tmpdir: tempfile::TempDir) { + use crate::storage::fs::RenderStore; + init_logging(); let dir = tmpdir.path(); let tmprepo = Arc::new( - fs::MaybeOpenFsRepository::create(dir.join("repo")) + fs::MaybeOpenFsRepository::::create(dir.join("repo")) .await .unwrap() .into(), @@ -90,7 +92,7 @@ async fn test_commit_mode_fs(tmpdir: tempfile::TempDir) { // Safety: tmprepo was created as an FsRepository let tmprepo = match &*tmprepo { - RepositoryHandle::FS(fs) => fs.opened().await.unwrap(), + RepositoryHandle::FSWithRenders(fs) => fs.opened().await.unwrap(), _ => panic!("Unexpected tmprepo type!"), }; diff --git a/crates/spfs/src/storage/rpc/repository.rs b/crates/spfs/src/storage/rpc/repository.rs index d22ca8cea9..a6c3928d88 100644 --- a/crates/spfs/src/storage/rpc/repository.rs +++ b/crates/spfs/src/storage/rpc/repository.rs @@ -92,7 +92,17 @@ pub struct RpcRepository { impl storage::FromConfig for RpcRepository { type Config = Config; - async fn from_config(config: Self::Config) -> OpenRepositoryResult { + async fn from_config( + config: Self::Config, + ) -> OpenRepositoryResult { + Self::new(config).await.map(Into::into) + } +} + +#[async_trait::async_trait] +impl storage::FromUrl for RpcRepository { + async fn from_url(url: &url::Url) -> OpenRepositoryResult { + let config = Config::from_url(url).await?; Self::new(config).await } } diff --git a/crates/spfs/src/storage/tag_test.rs b/crates/spfs/src/storage/tag_test.rs index b244fdec87..bd7e94672d 100644 --- a/crates/spfs/src/storage/tag_test.rs +++ b/crates/spfs/src/storage/tag_test.rs @@ -113,7 +113,9 @@ async fn test_tag_no_duplication( #[rstest] #[tokio::test] async fn test_tag_permissions(tmpdir: tempfile::TempDir) { - let storage = MaybeOpenFsRepository::create(tmpdir.path().join("repo")) + use crate::storage::fs::RenderStore; + + let storage = MaybeOpenFsRepository::::create(tmpdir.path().join("repo")) .await .unwrap(); let spec = tracking::TagSpec::parse("hello").unwrap(); diff --git a/crates/spfs/src/storage/tar/repository.rs b/crates/spfs/src/storage/tar/repository.rs index a17dbc7d49..4bd67294dc 100644 --- a/crates/spfs/src/storage/tar/repository.rs +++ b/crates/spfs/src/storage/tar/repository.rs @@ -16,7 +16,7 @@ use tar::{Archive, Builder}; use crate::config::{ToAddress, pathbuf_deserialize_with_tilde_expansion}; use crate::graph::ObjectProto; use crate::prelude::*; -use crate::storage::fs::DURABLE_EDITS_DIR; +use crate::storage::fs::{DURABLE_EDITS_DIR, NoRenderStore}; use crate::storage::tag::TagSpecAndTagStream; use crate::storage::{ EntryType, @@ -68,15 +68,17 @@ pub struct TarRepository { up_to_date: AtomicBool, archive: std::path::PathBuf, repo_dir: tempfile::TempDir, - repo: crate::storage::fs::MaybeOpenFsRepository, + repo: crate::storage::fs::MaybeOpenFsRepository, } #[async_trait::async_trait] impl storage::FromConfig for TarRepository { type Config = Config; - async fn from_config(config: Self::Config) -> OpenRepositoryResult { - Self::create(&config.path).await + async fn from_config( + config: Self::Config, + ) -> OpenRepositoryResult { + Self::create(&config.path).await.map(Into::into) } } @@ -158,7 +160,8 @@ impl TarRepository { up_to_date: AtomicBool::new(false), archive: path, repo_dir: tmpdir, - repo: crate::storage::fs::MaybeOpenFsRepository::create(&repo_path).await?, + repo: crate::storage::fs::MaybeOpenFsRepository::::create(&repo_path) + .await?, }) } diff --git a/crates/spfs/src/sync_test.rs b/crates/spfs/src/sync_test.rs index 000ab34bb3..ad0761ad2b 100644 --- a/crates/spfs/src/sync_test.rs +++ b/crates/spfs/src/sync_test.rs @@ -14,6 +14,7 @@ use crate::fixtures::*; use crate::graph::AnnotationValue; use crate::graph::object::EncodingFormat; use crate::prelude::*; +use crate::storage::fs::NoRenderStore; use crate::{Error, encoding, graph, reset_config_async, storage, tracking}; #[rstest] @@ -21,7 +22,11 @@ use crate::{Error, encoding, graph, reset_config_async, storage, tracking}; async fn test_sync_ref_unknown(#[future] config: (tempfile::TempDir, Config)) { init_logging(); let (_handle, config) = config.await; - let local = config.get_local_repository().await.unwrap().into(); + let local = config + .get_local_repository::() + .await + .unwrap() + .into(); let origin = config.get_remote("origin").await.unwrap(); let syncer = Syncer::new(&local, &origin); match syncer.sync_ref("--test-unknown--").await { @@ -50,7 +55,13 @@ async fn test_push_ref(#[future] config: (tempfile::TempDir, Config)) { ensure(src_dir.join("dir2/otherfile.txt"), "hello2"); ensure(src_dir.join("dir//dir/dir/file.txt"), "hello, world"); - let local = Arc::new(config.get_local_repository().await.unwrap().into()); + let local = Arc::new( + config + .get_local_repository::() + .await + .unwrap() + .into(), + ); let remote = config.get_remote("origin").await.unwrap(); let manifest = crate::Committer::new(&local) .commit_dir(src_dir.as_path()) @@ -369,11 +380,11 @@ async fn test_sync_through_tar( #[fixture] async fn config(tmpdir: tempfile::TempDir) -> (tempfile::TempDir, Config) { let repo_path = tmpdir.path().join("repo"); - crate::storage::fs::MaybeOpenFsRepository::create(&repo_path) + crate::storage::fs::MaybeOpenFsRepository::::create(&repo_path) .await .expect("failed to make repo for test"); let origin_path = tmpdir.path().join("origin"); - crate::storage::fs::MaybeOpenFsRepository::create(&origin_path) + crate::storage::fs::MaybeOpenFsRepository::::create(&origin_path) .await .expect("failed to make repo for test"); let mut conf = Config::default(); diff --git a/crates/spfs/tests/integration/unprivileged/test_fuse_does_not_create_renders.sh b/crates/spfs/tests/integration/unprivileged/test_fuse_does_not_create_renders.sh new file mode 100644 index 0000000000..7cc765f9d1 --- /dev/null +++ b/crates/spfs/tests/integration/unprivileged/test_fuse_does_not_create_renders.sh @@ -0,0 +1,43 @@ +#!/bin/bash + +# Copyright (c) Contributors to the SPK project. +# SPDX-License-Identifier: Apache-2.0 +# https://github.com/spkenv/spk + +set -o errexit + +# when using fuse, the local repo should not get a renders directory created for +# the current user + +temp_repo=$(mktemp -d) + +cleanup() { + rm -rf "$temp_repo" +} + +trap cleanup EXIT + +export SPFS_STORAGE_ROOT="$temp_repo" + +# create some content that would need to be rendered +SPFS_FILESYSTEM_BACKEND=OverlayFsWithFuse spfs run - -- bash -c "echo hello > /spfs/hello.txt && spfs commit layer -t some-content" + +# something that will open the local repo, using fuse +SPFS_FILESYSTEM_BACKEND=OverlayFsWithFuse spfs run some-content -- true + +# the renders directory should not have been created +if test -d "$temp_repo/renders"; then + echo "renders directory was not supposed to be created on step 1" + exit 1 +fi + +# something that will open the local repo, not using fuse +SPFS_FILESYSTEM_BACKEND=OverlayFsWithRenders spfs run some-content -- true + +# the renders directory should have been created, to prove the behavior is +# different when not using fuse +if test ! -d "$temp_repo/renders"; then + echo "renders directory was expected to be created on step 2" + exit 1 +fi + diff --git a/crates/spk-build/src/build/binary_test.rs b/crates/spk-build/src/build/binary_test.rs index 45c7210870..9542a59dfe 100644 --- a/crates/spk-build/src/build/binary_test.rs +++ b/crates/spk-build/src/build/binary_test.rs @@ -7,6 +7,7 @@ use std::path::PathBuf; use rstest::rstest; use spfs::encoding::EMPTY_DIGEST; use spfs::prelude::*; +use spfs::storage::fs::RenderStore; use spk_schema::foundation::env::data_path; use spk_schema::foundation::fixtures::*; use spk_schema::foundation::ident_component::Component; @@ -721,7 +722,7 @@ async fn test_build_package_source_cleanup(#[case] solver: SolverImpl) { .get(&Component::Run) .unwrap(); let config = spfs::get_config().unwrap(); - let repo = config.get_local_repository().await.unwrap(); + let repo = config.get_local_repository::().await.unwrap(); let layer = repo.read_layer(digest).await.unwrap(); let manifest_digest = match layer.manifest() { @@ -819,7 +820,7 @@ async fn test_build_filters_reset_files(#[case] solver: SolverImpl) { .get(&Component::Run) .unwrap(); let config = spfs::get_config().unwrap(); - let repo = config.get_local_repository().await.unwrap(); + let repo = config.get_local_repository::().await.unwrap(); let layer = repo.read_layer(digest).await.unwrap(); let manifest_digest = match layer.manifest() { diff --git a/crates/spk-cli/cmd-render/src/cmd_render.rs b/crates/spk-cli/cmd-render/src/cmd_render.rs index d1c6fde297..cc696024a5 100644 --- a/crates/spk-cli/cmd-render/src/cmd_render.rs +++ b/crates/spk-cli/cmd-render/src/cmd_render.rs @@ -7,6 +7,7 @@ use std::path::PathBuf; use clap::Args; use miette::{Context, IntoDiagnostic, Result, bail}; use spfs::storage::fallback::FallbackProxy; +use spfs::storage::fs::NoRenderStore; use spk_cli_common::{CommandArgs, Run, build_required_packages, flags}; use spk_exec::resolve_runtime_layers; use spk_solve::{Solver, SolverMut}; @@ -73,7 +74,7 @@ impl Run for Render { tracing::info!("Rendering into dir: {path:?}"); let config = spfs::get_config().wrap_err("Failed to load spfs config")?; let local = config - .get_opened_local_repository() + .get_opened_local_repository::() .await .wrap_err("Failed to open local spfs repo")?; diff --git a/crates/spk-cli/common/src/flags.rs b/crates/spk-cli/common/src/flags.rs index 37cf3d8abf..e0d9255d75 100644 --- a/crates/spk-cli/common/src/flags.rs +++ b/crates/spk-cli/common/src/flags.rs @@ -40,12 +40,13 @@ use spk_schema::ident::{ }; use spk_schema::option_map::HOST_OPTIONS; use spk_schema::{Recipe, SpecFileData, SpecRecipe, Template, TestStage, VariantExt}; +use spk_solve as solve; #[cfg(unix)] #[cfg(feature = "statsd")] use spk_solve::{SPK_RUN_TIME_METRIC, get_metrics_client}; +use spk_storage as storage; use spk_workspace::{FindOrLoadPackageTemplateError, FindPackageTemplateError}; pub use variant::{Variant, VariantBuildStatus, VariantLocation}; -use {spk_solve as solve, spk_storage as storage}; use crate::parsing::{VariantIndex, stage_specifier}; use crate::{CommandArgs, Error}; diff --git a/crates/spk-cli/group1/src/cmd_completion.rs b/crates/spk-cli/group1/src/cmd_completion.rs index 913ccc64bc..7abaf44079 100644 --- a/crates/spk-cli/group1/src/cmd_completion.rs +++ b/crates/spk-cli/group1/src/cmd_completion.rs @@ -7,8 +7,7 @@ use std::io::Write; use clap::{Command, Parser, value_parser}; -use clap_complete; -use clap_complete::Shell; +use clap_complete::{self, Shell}; use miette::Result; use spk_cli_common::CommandArgs; diff --git a/crates/spk-cli/group2/src/cmd_ls.rs b/crates/spk-cli/group2/src/cmd_ls.rs index d9064f37b8..3b2a13f0d6 100644 --- a/crates/spk-cli/group2/src/cmd_ls.rs +++ b/crates/spk-cli/group2/src/cmd_ls.rs @@ -12,15 +12,16 @@ use futures::TryStreamExt; use miette::Result; use spfs::Digest; use spk_cli_common::{CommandArgs, Run, flags}; +use spk_config; use spk_schema::foundation::format::{FormatComponents, FormatIdent, FormatOptionMap}; use spk_schema::foundation::ident_component::ComponentSet; use spk_schema::ident_component::Component; use spk_schema::name::OptNameBuf; use spk_schema::option_map::get_host_options_filters; use spk_schema::{Deprecate, OptionMap, OptionValues, Package, Spec, VersionIdent}; +use spk_storage as storage; use spk_storage::RepoWalker; use spk_storage::walker::{DeprecationState, RepoWalkerBuilder, RepoWalkerItem, WalkedBuild}; -use {spk_config, spk_storage as storage}; #[cfg(test)] #[path = "./cmd_ls_test.rs"] diff --git a/crates/spk-cli/group3/src/cmd_export_test.rs b/crates/spk-cli/group3/src/cmd_export_test.rs index c1abb116b6..8c93cb12a1 100644 --- a/crates/spk-cli/group3/src/cmd_export_test.rs +++ b/crates/spk-cli/group3/src/cmd_export_test.rs @@ -100,7 +100,6 @@ async fn test_export_works_with_missing_builds(#[case] solver: SolverImpl) { "VERSION".to_string(), "objects".to_string(), "payloads".to_string(), - "renders".to_string(), "tags".to_string(), "tags/spk".to_string(), "tags/spk/pkg".to_string(), diff --git a/crates/spk-cli/group3/src/cmd_import_test.rs b/crates/spk-cli/group3/src/cmd_import_test.rs index 613c99fca1..7029a7c5cb 100644 --- a/crates/spk-cli/group3/src/cmd_import_test.rs +++ b/crates/spk-cli/group3/src/cmd_import_test.rs @@ -66,7 +66,6 @@ async fn test_archive_io(#[case] solver: SolverImpl) { "VERSION".to_string(), "objects".to_string(), "payloads".to_string(), - "renders".to_string(), "tags".to_string(), "tags/spk".to_string(), "tags/spk/pkg".to_string(), diff --git a/crates/spk-cli/group4/src/cmd_view.rs b/crates/spk-cli/group4/src/cmd_view.rs index 2a45acb419..b51369d8fe 100644 --- a/crates/spk-cli/group4/src/cmd_view.rs +++ b/crates/spk-cli/group4/src/cmd_view.rs @@ -49,8 +49,7 @@ use spk_schema::{ }; use spk_solve::solution::{LayerPackageAndComponents, get_spfs_layers_to_packages}; use spk_solve::{PackageSource, Recipe, RequestedBy, Solution, Solver, SolverMut}; -use spk_storage; -use spk_storage::RepositoryHandle; +use spk_storage::{self, RepositoryHandle}; use strum::{Display, EnumString, IntoEnumIterator, VariantNames}; #[cfg(test)] diff --git a/crates/spk-launcher/src/main.rs b/crates/spk-launcher/src/main.rs index 877600399a..3fa119fa64 100644 --- a/crates/spk-launcher/src/main.rs +++ b/crates/spk-launcher/src/main.rs @@ -23,7 +23,7 @@ use spfs::encoding::Digest; use spfs::prelude::*; use spfs::storage::RepositoryHandle; use spfs::storage::fallback::FallbackProxy; -use spfs::storage::fs::OpenFsRepository; +use spfs::storage::fs::{NoRenderStore, OpenFsRepository}; use spfs::tracking::EnvSpec; const DEV_SHM: &str = "/dev/shm"; @@ -108,7 +108,7 @@ impl<'a> Dynamic<'a> { &self, tag: &str, platform_digest: &Digest, - local: Arc, + local: Arc>, remote: RepositoryHandle, ) -> Result { let digest_string = platform_digest.to_string(); diff --git a/crates/spk-solve/crates/macros/src/lib.rs b/crates/spk-solve/crates/macros/src/lib.rs index d226eb7c70..a85837d913 100644 --- a/crates/spk-solve/crates/macros/src/lib.rs +++ b/crates/spk-solve/crates/macros/src/lib.rs @@ -2,9 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 // https://github.com/spkenv/spk +pub use serde; +pub use serde_json; +pub use spfs; pub use spk_schema::recipe; pub use spk_solve_solution::{PackageSource, Solution}; -pub use {serde, serde_json, spfs}; /// Creates a repository containing a set of provided package specs. /// It will take care of publishing the spec, and creating a build for diff --git a/crates/spk-solve/src/lib.rs b/crates/spk-solve/src/lib.rs index e5e5a05c20..619b288ed2 100644 --- a/crates/spk-solve/src/lib.rs +++ b/crates/spk-solve/src/lib.rs @@ -32,10 +32,13 @@ pub use metrics::{ get_metrics_client, }; pub(crate) use search_space::show_search_space_stats; +pub use serde; +pub use serde_json; pub use solver::{Solver, SolverExt, SolverImpl, SolverMut}; // Publicly exported ResolvoSolver to stop dead code warnings pub use solvers::ResolvoSolver; pub use solvers::{StepSolver, StepSolverRuntime}; +pub use spfs; pub use spk_schema::foundation::ident_build::Build; pub use spk_schema::foundation::ident_component::Component; pub use spk_schema::foundation::option_map; @@ -49,15 +52,10 @@ pub use spk_schema::ident::{ parse_ident_range, }; pub use spk_schema::{Package, Recipe, Spec, SpecRecipe, recipe, spec, v0}; +pub use spk_solve_graph as graph; +pub use spk_solve_package_iterator as package_iterator; +pub use spk_solve_solution as solution; pub use spk_solve_solution::{PackageSource, Solution}; +pub use spk_solve_validation as validation; pub use spk_storage::RepositoryHandle; pub(crate) use status_line::StatusLine; -pub use { - serde, - serde_json, - spfs, - spk_solve_graph as graph, - spk_solve_package_iterator as package_iterator, - spk_solve_solution as solution, - spk_solve_validation as validation, -}; diff --git a/crates/spk-storage/src/fixtures.rs b/crates/spk-storage/src/fixtures.rs index 9ba97719db..45002719ef 100644 --- a/crates/spk-storage/src/fixtures.rs +++ b/crates/spk-storage/src/fixtures.rs @@ -10,6 +10,7 @@ use rstest::fixture; use spfs::Result; use spfs::config::Remote; use spfs::prelude::*; +use spfs::storage::fs::RenderStore; use spk_schema::foundation::fixtures::*; use tokio::sync::{Mutex, MutexGuard}; @@ -107,9 +108,10 @@ pub async fn make_repo(kind: RepoKind) -> TempRepo { let repo = match kind { RepoKind::Spfs => { let storage_root = tmpdir.path().join("repo"); - let spfs_repo = spfs::storage::fs::MaybeOpenFsRepository::create(&storage_root) - .await - .expect("failed to establish temporary local repo for test"); + let spfs_repo = + spfs::storage::fs::MaybeOpenFsRepository::::create(&storage_root) + .await + .expect("failed to establish temporary local repo for test"); let written = spfs_repo .commit_blob(Box::pin(std::io::Cursor::new(b""))) .await diff --git a/crates/spk-storage/src/storage/spfs.rs b/crates/spk-storage/src/storage/spfs.rs index de2a823f8e..338cb6d8cc 100644 --- a/crates/spk-storage/src/storage/spfs.rs +++ b/crates/spk-storage/src/storage/spfs.rs @@ -17,6 +17,7 @@ use relative_path::RelativePathBuf; use serde::{Deserialize, Serialize}; use spfs::prelude::{RepositoryExt as SpfsRepositoryExt, *}; use spfs::storage::EntryType; +use spfs::storage::fs::MaybeRenderStore; use spfs::tracking::{self, TagSpec}; use spk_schema::foundation::ident_build::{Build, parse_build}; use spk_schema::foundation::ident_component::Component; @@ -1170,7 +1171,7 @@ impl StoredPackage { /// Return the local packages repository used for development. pub async fn local_repository() -> Result { let config = spfs::get_config()?; - let repo = config.get_local_repository().await?; + let repo = config.get_local_repository::().await?; let inner: spfs::prelude::RepositoryHandle = repo.into(); let address = inner.address().into_owned(); Ok(SpfsRepository { diff --git a/crates/spk-storage/src/storage/spfs_test.rs b/crates/spk-storage/src/storage/spfs_test.rs index 57efa71f84..02cf6015f0 100644 --- a/crates/spk-storage/src/storage/spfs_test.rs +++ b/crates/spk-storage/src/storage/spfs_test.rs @@ -7,6 +7,7 @@ use std::str::FromStr; use rstest::rstest; use spfs::prelude::*; +use spfs::storage::fs::RenderStore; use spk_schema::BuildIdent; use spk_schema::foundation::fixtures::*; use spk_schema::foundation::version::Version; @@ -34,7 +35,7 @@ async fn test_metadata_io(tmpdir: tempfile::TempDir) { let repo_root = tmpdir.path(); let repo = SpfsRepository::try_from(NameAndRepository::new( "test-repo", - spfs::storage::fs::MaybeOpenFsRepository::create(repo_root) + spfs::storage::fs::MaybeOpenFsRepository::::create(repo_root) .await .unwrap(), )) @@ -54,7 +55,7 @@ async fn test_upgrade_sets_version(tmpdir: tempfile::TempDir) { let repo_root = tmpdir.path(); let repo = SpfsRepository::try_from(NameAndRepository::new( "test-repo", - spfs::storage::fs::MaybeOpenFsRepository::create(repo_root) + spfs::storage::fs::MaybeOpenFsRepository::::create(repo_root) .await .unwrap(), )) @@ -75,7 +76,7 @@ async fn test_upgrade_sets_version(tmpdir: tempfile::TempDir) { async fn test_upgrade_changes_tags(tmpdir: tempfile::TempDir) { init_logging(); let repo_root = tmpdir.path(); - let spfs_repo = spfs::storage::fs::MaybeOpenFsRepository::create(repo_root) + let spfs_repo = spfs::storage::fs::MaybeOpenFsRepository::::create(repo_root) .await .unwrap(); let repo = SpfsRepository::new("test-repo", &format!("file://{}", repo_root.display())) diff --git a/crates/spk/src/lib.rs b/crates/spk/src/lib.rs index e58380abdb..dd6df29ddc 100644 --- a/crates/spk/src/lib.rs +++ b/crates/spk/src/lib.rs @@ -2,12 +2,10 @@ // SPDX-License-Identifier: Apache-2.0 // https://github.com/spkenv/spk -pub use { - spk_build as build, - spk_exec as exec, - spk_schema as schema, - spk_solve as solve, - spk_storage as storage, -}; +pub use spk_build as build; +pub use spk_exec as exec; +pub use spk_schema as schema; +pub use spk_solve as solve; +pub use spk_storage as storage; pub const VERSION: &str = env!("CARGO_PKG_VERSION"); diff --git a/cspell.json b/cspell.json index 40fb9d887f..3fdd70c395 100644 --- a/cspell.json +++ b/cspell.json @@ -425,6 +425,7 @@ "mkrecipe", "mksource", "mksrc", + "mktemp", "modversions", "mountpoint", "mpfr",