Skip to content

Commit 8d8f699

Browse files
jrrayjrray
committed
Refactor FsRepository's renders
The goal is to make it so it is possible to open repo without creating the renders directory for the current user, in situations where that renders directory will not be used. Examples include when using fuse or when running `spfs clean`. This introduces different flavors of a "render store" besides `RenderStore` that either represent a render store that hasn't been created yet (but can be) or the lack of a render store. Operations that don't require a render store can access the local repository without the renders directory being created. Signed-off-by: J Robert Ray <[email protected]>
1 parent 7ec8e8b commit 8d8f699

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+1049
-414
lines changed

‎crates/spfs-cli/cmd-render/src/cmd_render.rs

+9-4
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44

55
use clap::builder::TypedValueParser;
66
use clap::Parser;
7-
use miette::{Context, Result};
7+
use miette::{Context, IntoDiagnostic, Result};
88
use spfs::prelude::*;
99
use spfs::storage::fallback::FallbackProxy;
10+
use spfs::storage::fs::{MaybeRenderStore, RenderStore};
1011
use spfs::{graph, Error, RenderResult};
1112
use spfs_cli_common as cli;
1213
use spfs_cli_common::CommandName;
@@ -74,7 +75,11 @@ impl CmdRender {
7475

7576
let rendered = match &self.target {
7677
Some(target) => self.render_to_dir(fallback, env_spec, target).await?,
77-
None => self.render_to_repo(fallback, env_spec).await?,
78+
None => {
79+
// This path requires a repository that supports renders.
80+
let fallback: FallbackProxy<RenderStore> = fallback.try_into().into_diagnostic()?;
81+
self.render_to_repo(fallback, env_spec).await?
82+
}
7883
};
7984

8085
tracing::debug!("render(s) completed successfully");
@@ -85,7 +90,7 @@ impl CmdRender {
8590

8691
async fn render_to_dir(
8792
&self,
88-
repo: FallbackProxy,
93+
repo: FallbackProxy<MaybeRenderStore>,
8994
env_spec: spfs::tracking::EnvSpec,
9095
target: &std::path::Path,
9196
) -> Result<RenderResult> {
@@ -134,7 +139,7 @@ impl CmdRender {
134139

135140
async fn render_to_repo(
136141
&self,
137-
repo: FallbackProxy,
142+
repo: FallbackProxy<RenderStore>,
138143
env_spec: spfs::tracking::EnvSpec,
139144
) -> Result<RenderResult> {
140145
let mut stack = graph::Stack::default();

‎crates/spfs-cli/common/src/args.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use miette::{Error, IntoDiagnostic, Result, WrapErr};
1111
#[cfg(feature = "sentry")]
1212
use once_cell::sync::OnceCell;
1313
use spfs::io::Pluralize;
14-
use spfs::storage::LocalRepository;
14+
use spfs::storage::LocalPayloads;
1515
use tracing_subscriber::prelude::*;
1616

1717
const SPFS_LOG: &str = "SPFS_LOG";
@@ -138,7 +138,7 @@ impl Render {
138138
reporter: Reporter,
139139
) -> spfs::storage::fs::Renderer<'repo, Repo, Reporter>
140140
where
141-
Repo: spfs::storage::Repository + LocalRepository,
141+
Repo: spfs::storage::Repository + LocalPayloads,
142142
Reporter: spfs::storage::fs::RenderReporter,
143143
{
144144
spfs::storage::fs::Renderer::new(repo)

‎crates/spfs-cli/main/src/cmd_init.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use std::path::PathBuf;
66

77
use clap::{Args, Subcommand};
88
use miette::Result;
9+
use spfs::storage::fs::NoRenderStore;
910

1011
/// Create an empty filesystem repository
1112
#[derive(Debug, Args)]
@@ -36,7 +37,7 @@ impl InitSubcommand {
3637
pub async fn run(&self, _config: &spfs::Config) -> Result<i32> {
3738
match self {
3839
Self::Repo { path } => {
39-
spfs::storage::fs::MaybeOpenFsRepository::create(&path).await?;
40+
spfs::storage::fs::MaybeOpenFsRepository::<NoRenderStore>::create(&path).await?;
4041
Ok(0)
4142
}
4243
}

‎crates/spfs-cli/main/src/cmd_search.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use clap::Args;
66
use miette::Result;
77
use spfs::prelude::*;
8+
use spfs::storage::fs::NoRenderStore;
89
use tokio_stream::StreamExt;
910

1011
/// Search for available tags by substring
@@ -29,7 +30,10 @@ impl CmdSearch {
2930
};
3031
repos.push(remote);
3132
}
32-
repos.insert(0, config.get_local_repository().await?.into());
33+
repos.insert(
34+
0,
35+
config.get_local_repository::<NoRenderStore>().await?.into(),
36+
);
3337
for repo in repos.into_iter() {
3438
let mut tag_streams = repo.iter_tags();
3539
while let Some(tag) = tag_streams.next().await {

‎crates/spfs-vfs/src/fuse.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use fuser::{
2929
Request,
3030
};
3131
use spfs::prelude::*;
32-
use spfs::storage::LocalRepository;
32+
use spfs::storage::LocalPayloads;
3333
#[cfg(feature = "fuse-backend-abi-7-31")]
3434
use spfs::tracking::BlobRead;
3535
use spfs::tracking::{Entry, EntryKind, EnvSpec, Manifest};
@@ -377,7 +377,7 @@ impl Filesystem {
377377
let mut flags = FOPEN_KEEP_CACHE;
378378
for repo in self.repos.iter() {
379379
match &**repo {
380-
spfs::storage::RepositoryHandle::FS(fs_repo) => {
380+
spfs::storage::RepositoryHandle::FSWithRenders(fs_repo) => {
381381
let Ok(fs_repo) = fs_repo.opened().await else {
382382
reply.error(libc::ENOENT);
383383
return;

‎crates/spfs/src/bootstrap_test.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use super::build_shell_initialized_command;
1111
use crate::fixtures::*;
1212
use crate::resolve::which;
1313
use crate::runtime;
14+
use crate::storage::fs::RenderStore;
1415

1516
#[rstest(
1617
shell,
@@ -37,7 +38,7 @@ async fn test_shell_initialization_startup_scripts(
3738
};
3839
let root = tmpdir.path().to_string_lossy().to_string();
3940
let repo = crate::storage::RepositoryHandle::from(
40-
crate::storage::fs::MaybeOpenFsRepository::create(&root)
41+
crate::storage::fs::MaybeOpenFsRepository::<RenderStore>::create(&root)
4142
.await
4243
.unwrap(),
4344
);
@@ -113,7 +114,7 @@ async fn test_shell_initialization_no_startup_scripts(shell: &str, tmpdir: tempf
113114
};
114115
let root = tmpdir.path().to_string_lossy().to_string();
115116
let repo = crate::storage::RepositoryHandle::from(
116-
crate::storage::fs::MaybeOpenFsRepository::create(&root)
117+
crate::storage::fs::MaybeOpenFsRepository::<RenderStore>::create(&root)
117118
.await
118119
.unwrap(),
119120
);

‎crates/spfs/src/clean.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ where
570570
/// remove data that is still being used
571571
async unsafe fn remove_unvisited_renders_and_proxies(&self) -> Result<CleanResult> {
572572
let mut result = CleanResult::default();
573-
let storage::RepositoryHandle::FS(repo) = self.repo else {
573+
let storage::RepositoryHandle::FSWithRenders(repo) = self.repo else {
574574
return Ok(result);
575575
};
576576
let repo = repo.opened().await?;
@@ -715,8 +715,10 @@ where
715715
let future = async move {
716716
if !self.dry_run {
717717
tracing::trace!(?path, "removing proxy render");
718-
storage::fs::OpenFsRepository::remove_dir_atomically(&path, &workdir)
719-
.await?;
718+
storage::fs::OpenFsRepository::<storage::fs::RenderStore>::remove_dir_atomically(
719+
&path, &workdir,
720+
)
721+
.await?;
720722
}
721723
Ok(digest)
722724
};

‎crates/spfs/src/clean_test.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use tokio::time::sleep;
1313
use super::{Cleaner, TracingCleanReporter};
1414
use crate::encoding::prelude::*;
1515
use crate::fixtures::*;
16+
use crate::storage::fs::RenderStore;
1617
use crate::{storage, tracking, Error};
1718

1819
#[rstest]
@@ -225,7 +226,7 @@ async fn test_clean_untagged_objects_layers_platforms(#[future] tmprepo: TempRep
225226
async fn test_clean_manifest_renders(tmpdir: tempfile::TempDir) {
226227
init_logging();
227228
let tmprepo = Arc::new(
228-
storage::fs::MaybeOpenFsRepository::create(tmpdir.path())
229+
storage::fs::MaybeOpenFsRepository::<RenderStore>::create(tmpdir.path())
229230
.await
230231
.unwrap()
231232
.into(),
@@ -249,7 +250,7 @@ async fn test_clean_manifest_renders(tmpdir: tempfile::TempDir) {
249250
.unwrap();
250251

251252
let fs_repo = match &*tmprepo {
252-
RepositoryHandle::FS(fs) => fs,
253+
RepositoryHandle::FSWithRenders(fs) => fs,
253254
_ => panic!("Unexpected tmprepo type!"),
254255
};
255256
let fs_repo = fs_repo.opened().await.unwrap();
@@ -269,7 +270,7 @@ async fn test_clean_manifest_renders(tmpdir: tempfile::TempDir) {
269270
.expect("failed to clean repo");
270271
println!("{result:#?}");
271272

272-
let files = list_files(fs_repo.fs_impl.renders.as_ref().unwrap().renders.root());
273+
let files = list_files(fs_repo.fs_impl.rs_impl.renders.root());
273274
assert_eq!(
274275
files,
275276
Vec::<String>::new(),

‎crates/spfs/src/commit_test.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,21 @@ use rstest::rstest;
66

77
use super::Committer;
88
use crate::fixtures::*;
9+
use crate::storage::fs::RenderStore;
910
use crate::Error;
1011

1112
#[rstest]
1213
#[tokio::test]
1314
async fn test_commit_empty(tmpdir: tempfile::TempDir) {
1415
let root = tmpdir.path().to_string_lossy().to_string();
1516
let repo = crate::storage::RepositoryHandle::from(
16-
crate::storage::fs::MaybeOpenFsRepository::create(&root)
17+
crate::storage::fs::MaybeOpenFsRepository::<RenderStore>::create(&root)
1718
.await
1819
.unwrap(),
1920
);
2021
let storage = crate::runtime::Storage::new(repo).unwrap();
2122
let repo = crate::storage::RepositoryHandle::from(
22-
crate::storage::fs::MaybeOpenFsRepository::create(root)
23+
crate::storage::fs::MaybeOpenFsRepository::<RenderStore>::create(root)
2324
.await
2425
.unwrap(),
2526
);

‎crates/spfs/src/config.rs

+31-10
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,13 @@ impl RemoteConfig {
311311
inner,
312312
} = self;
313313
let mut handle: storage::RepositoryHandle = match inner.clone() {
314-
RepositoryConfig::Fs(config) => storage::fs::MaybeOpenFsRepository::from_config(config)
315-
.await?
316-
.into(),
314+
// Remote repositories never have a render store (from our
315+
// perspective).
316+
RepositoryConfig::Fs(config) => <storage::fs::MaybeOpenFsRepository<
317+
storage::fs::NoRenderStore,
318+
> as Into<storage::RepositoryHandle>>::into(
319+
storage::fs::MaybeOpenFsRepository::from_config(config).await?,
320+
),
317321
RepositoryConfig::Tar(config) => storage::tar::TarRepository::from_config(config)
318322
.await?
319323
.into(),
@@ -532,7 +536,12 @@ impl Config {
532536
}
533537

534538
/// Get the local repository instance as configured, creating it if needed.
535-
pub async fn get_opened_local_repository(&self) -> Result<storage::fs::OpenFsRepository> {
539+
pub async fn get_opened_local_repository<RS>(&self) -> Result<storage::fs::OpenFsRepository<RS>>
540+
where
541+
RS: storage::DefaultRenderStoreCreationPolicy
542+
+ storage::RenderStoreForUser<RenderStore = RS>
543+
+ Clone,
544+
{
536545
// Possibly use a different path for the local repository, depending
537546
// on enabled features.
538547
#[allow(unused_mut)]
@@ -544,7 +553,7 @@ impl Config {
544553
Some(self.storage.root.join("ci").join(format!("pipeline_{id}")));
545554
}
546555

547-
let mut local_repo = storage::fs::OpenFsRepository::create(
556+
let mut local_repo = storage::fs::OpenFsRepository::<RS>::create(
548557
use_ci_isolated_storage_path
549558
.as_ref()
550559
.unwrap_or(&self.storage.root),
@@ -565,16 +574,24 @@ impl Config {
565574
///
566575
/// The returned repo is guaranteed to be created, valid and open already. Ie
567576
/// the local repository is not allowed to be lazily opened.
568-
pub async fn get_local_repository(&self) -> Result<storage::fs::OpenFsRepository> {
577+
pub async fn get_local_repository<RS>(&self) -> Result<storage::fs::OpenFsRepository<RS>>
578+
where
579+
RS: storage::DefaultRenderStoreCreationPolicy
580+
+ storage::RenderStoreForUser<RenderStore = RS>
581+
+ Clone,
582+
{
569583
self.get_opened_local_repository().await.map(Into::into)
570584
}
571585

572-
/// Get the local repository handle as configured, creating it if needed.
586+
/// Get the local repository handle as configured, creating it if needed.
573587
///
574588
/// The returned repo is guaranteed to be created, valid and open already. Ie
575589
/// the local repository is not allowed to be lazily opened.
576590
pub async fn get_local_repository_handle(&self) -> Result<storage::RepositoryHandle> {
577-
Ok(self.get_local_repository().await?.into())
591+
Ok(self
592+
.get_local_repository::<storage::fs::MaybeRenderStore>()
593+
.await?
594+
.into())
578595
}
579596

580597
/// Get a remote repository by name, or the local repository.
@@ -590,14 +607,18 @@ impl Config {
590607
{
591608
match name {
592609
Some(name) => self.get_remote(name).await,
593-
None => Ok(self.get_local_repository().await?.into()),
610+
None => Ok(self
611+
.get_local_repository::<storage::fs::MaybeRenderStore>()
612+
.await?
613+
.into()),
594614
}
595615
}
596616

597617
/// Get the local runtime storage, as configured.
598618
pub async fn get_runtime_storage(&self) -> Result<runtime::Storage> {
599619
runtime::Storage::new(storage::RepositoryHandle::from(
600-
self.get_local_repository().await?,
620+
self.get_local_repository::<storage::fs::MaybeRenderStore>()
621+
.await?,
601622
))
602623
}
603624

‎crates/spfs/src/config_test.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use rstest::rstest;
66

77
use super::{Config, Remote, RemoteConfig, RepositoryConfig};
8+
use crate::storage::fs::NoRenderStore;
89
use crate::storage::prelude::*;
910
use crate::storage::RepositoryHandle;
1011
use crate::{get_config, load_config};
@@ -41,7 +42,7 @@ async fn test_config_get_remote() {
4142
.tempdir()
4243
.unwrap();
4344
let remote = tmpdir.path().join("remote");
44-
let _ = crate::storage::fs::MaybeOpenFsRepository::create(&remote)
45+
let _ = crate::storage::fs::MaybeOpenFsRepository::<NoRenderStore>::create(&remote)
4546
.await
4647
.unwrap();
4748

‎crates/spfs/src/fixtures.rs

+20-14
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use rstest::fixture;
1010
use tempfile::TempDir;
1111

1212
use crate as spfs;
13+
use crate::storage::fs::RenderStore;
1314

1415
pub enum TempRepo {
1516
FS(Arc<spfs::storage::RepositoryHandle>, Arc<TempDir>),
@@ -39,13 +40,14 @@ impl TempRepo {
3940
{
4041
match self {
4142
TempRepo::FS(_, tempdir) => {
42-
let repo = spfs::storage::fs::MaybeOpenFsRepository {
43+
let repo = spfs::storage::fs::MaybeOpenFsRepository::<RenderStore> {
4344
fs_impl: {
44-
let mut fs_impl = spfs::storage::fs::MaybeOpenFsRepositoryImpl::open(
45-
tempdir.path().join("repo"),
46-
)
47-
.await
48-
.unwrap();
45+
let mut fs_impl =
46+
spfs::storage::fs::MaybeOpenFsRepositoryImpl::<RenderStore>::open(
47+
tempdir.path().join("repo"),
48+
)
49+
.await
50+
.unwrap();
4951
fs_impl.set_tag_namespace(Some(spfs::storage::TagNamespaceBuf::new(
5052
namespace.as_ref(),
5153
)));
@@ -143,10 +145,12 @@ pub async fn tmprepo(kind: &str) -> TempRepo {
143145
let tmpdir = tmpdir();
144146
match kind {
145147
"fs" => {
146-
let repo = spfs::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("repo"))
147-
.await
148-
.unwrap()
149-
.into();
148+
let repo = spfs::storage::fs::MaybeOpenFsRepository::<RenderStore>::create(
149+
tmpdir.path().join("repo"),
150+
)
151+
.await
152+
.unwrap()
153+
.into();
150154
TempRepo::FS(Arc::new(repo), Arc::new(tmpdir))
151155
}
152156
"tar" => {
@@ -159,10 +163,12 @@ pub async fn tmprepo(kind: &str) -> TempRepo {
159163
#[cfg(feature = "server")]
160164
"rpc" => {
161165
use crate::storage::prelude::*;
162-
let repo = std::sync::Arc::new(spfs::storage::RepositoryHandle::FS(
163-
spfs::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("repo"))
164-
.await
165-
.unwrap(),
166+
let repo = std::sync::Arc::new(spfs::storage::RepositoryHandle::FSWithRenders(
167+
spfs::storage::fs::MaybeOpenFsRepository::<RenderStore>::create(
168+
tmpdir.path().join("repo"),
169+
)
170+
.await
171+
.unwrap(),
166172
));
167173
let listen: std::net::SocketAddr = "127.0.0.1:0".parse().unwrap();
168174
let http_listener = std::net::TcpListener::bind(listen).unwrap();

0 commit comments

Comments
 (0)