Skip to content

Commit d08ddba

Browse files
committed
Combine FsRepository and OpenFsRepository
Use the type state pattern to combine these two types into a single type. This is some groundwork for creating new types to represent repos that have renders or not. This code doesn't make much use of the type state pattern yet. `FsRepository` used to represent a repo that may not be opened yet, this is now represented by the type alias: type MaybeOpenFsRepository = FsRepository<MaybeOpenFsRepositoryImpl> An already opened repository has the same name, but is now a type alias: type OpenFsRepository = FsRepository<OpenFsRepositoryImpl> The original types have been renamed to these *Impl names: FsRepository -> MaybeOpenFsRepositoryImpl OpenFsRepository -> OpenFsRepositoryImpl Signed-off-by: J Robert Ray <[email protected]>
1 parent 208b237 commit d08ddba

31 files changed

+467
-159
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ impl InitSubcommand {
3636
pub async fn run(&self, _config: &spfs::Config) -> Result<i32> {
3737
match self {
3838
Self::Repo { path } => {
39-
spfs::storage::fs::FsRepository::create(&path).await?;
39+
spfs::storage::fs::MaybeOpenFsRepository::create(&path).await?;
4040
Ok(0)
4141
}
4242
}

crates/spfs-vfs/src/fuse.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use fuser::{
2929
Request,
3030
};
3131
use spfs::prelude::*;
32+
use spfs::storage::LocalRepository;
3233
#[cfg(feature = "fuse-backend-abi-7-31")]
3334
use spfs::tracking::BlobRead;
3435
use spfs::tracking::{Entry, EntryKind, EnvSpec, Manifest};
@@ -381,7 +382,7 @@ impl Filesystem {
381382
reply.error(libc::ENOENT);
382383
return;
383384
};
384-
let payload_path = fs_repo.payloads.build_digest_path(digest);
385+
let payload_path = fs_repo.payloads().build_digest_path(digest);
385386
match std::fs::OpenOptions::new().read(true).open(payload_path) {
386387
Ok(file) => {
387388
handle = Some(Handle::BlobFile { entry, file });

crates/spfs-vfs/src/winfsp/mount.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use std::sync::Arc;
99
use dashmap::DashMap;
1010
use libc::c_void;
1111
use spfs::prelude::*;
12+
use spfs::storage::LocalRepository;
1213
use spfs::tracking::{Entry, EntryKind};
1314
use spfs::OsError;
1415
use tokio::io::AsyncReadExt;
@@ -287,7 +288,7 @@ impl winfsp::filesystem::FileSystemContext for Mount {
287288
send.send(Err(winfsp::FspError::IO(std::io::ErrorKind::NotFound)));
288289
return;
289290
};
290-
let payload_path = fs_repo.payloads.build_digest_path(&digest);
291+
let payload_path = fs_repo.payloads().build_digest_path(&digest);
291292
match std::fs::OpenOptions::new().read(true).open(payload_path) {
292293
Ok(file) => {
293294
let _ = send.send(Ok(Some(Handle::BlobFile { entry, file })));

crates/spfs/benches/spfs_bench.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ pub fn commit_benchmark(c: &mut Criterion) {
4444
.expect("create a temp directory for spfs repo");
4545
let repo: Arc<RepositoryHandle> = Arc::new(
4646
tokio_runtime
47-
.block_on(spfs::storage::fs::FsRepository::create(
47+
.block_on(spfs::storage::fs::MaybeOpenFsRepository::create(
4848
repo_path.path().join("repo"),
4949
))
5050
.expect("create spfs repo")

crates/spfs/src/bootstrap_test.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ async fn test_shell_initialization_startup_scripts(
3737
};
3838
let root = tmpdir.path().to_string_lossy().to_string();
3939
let repo = crate::storage::RepositoryHandle::from(
40-
crate::storage::fs::FsRepository::create(&root)
40+
crate::storage::fs::MaybeOpenFsRepository::create(&root)
4141
.await
4242
.unwrap(),
4343
);
@@ -113,7 +113,7 @@ async fn test_shell_initialization_no_startup_scripts(shell: &str, tmpdir: tempf
113113
};
114114
let root = tmpdir.path().to_string_lossy().to_string();
115115
let repo = crate::storage::RepositoryHandle::from(
116-
crate::storage::fs::FsRepository::create(&root)
116+
crate::storage::fs::MaybeOpenFsRepository::create(&root)
117117
.await
118118
.unwrap(),
119119
);

crates/spfs/src/clean.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use progress_bar_derive_macro::ProgressBar;
1818
use super::prune::PruneParameters;
1919
use crate::prelude::*;
2020
use crate::runtime::makedirs_with_perms;
21-
use crate::storage::fs::OpenFsRepository;
21+
use crate::storage::fs::FsRepositoryOps;
2222
use crate::{encoding, graph, storage, tracking, Error, Result};
2323

2424
#[cfg(test)]
@@ -609,7 +609,7 @@ where
609609
async fn remove_unvisited_renders_and_proxies_for_storage(
610610
&self,
611611
username: Option<String>,
612-
repo: &storage::fs::OpenFsRepository,
612+
repo: impl FsRepositoryOps,
613613
) -> Result<CleanResult> {
614614
let mut result = CleanResult::default();
615615
let mut stream = repo
@@ -715,7 +715,8 @@ where
715715
let future = async move {
716716
if !self.dry_run {
717717
tracing::trace!(?path, "removing proxy render");
718-
OpenFsRepository::remove_dir_atomically(&path, &workdir).await?;
718+
storage::fs::OpenFsRepository::remove_dir_atomically(&path, &workdir)
719+
.await?;
719720
}
720721
Ok(digest)
721722
};

crates/spfs/src/clean_test.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ async fn test_clean_untagged_objects_layers_platforms(#[future] tmprepo: TempRep
225225
async fn test_clean_manifest_renders(tmpdir: tempfile::TempDir) {
226226
init_logging();
227227
let tmprepo = Arc::new(
228-
storage::fs::FsRepository::create(tmpdir.path())
228+
storage::fs::MaybeOpenFsRepository::create(tmpdir.path())
229229
.await
230230
.unwrap()
231231
.into(),
@@ -254,12 +254,12 @@ async fn test_clean_manifest_renders(tmpdir: tempfile::TempDir) {
254254
};
255255
let fs_repo = fs_repo.opened().await.unwrap();
256256

257-
storage::fs::Renderer::new(&*fs_repo)
257+
storage::fs::Renderer::new(&fs_repo)
258258
.render_manifest(&manifest.to_graph_manifest(), None)
259259
.await
260260
.unwrap();
261261

262-
let files = list_files(fs_repo.objects.root());
262+
let files = list_files(fs_repo.fs_impl.objects.root());
263263
assert!(!files.is_empty(), "should have stored data");
264264

265265
let cleaner = Cleaner::new(&tmprepo).with_reporter(TracingCleanReporter);
@@ -269,7 +269,7 @@ async fn test_clean_manifest_renders(tmpdir: tempfile::TempDir) {
269269
.expect("failed to clean repo");
270270
println!("{result:#?}");
271271

272-
let files = list_files(fs_repo.renders.as_ref().unwrap().renders.root());
272+
let files = list_files(fs_repo.fs_impl.renders.as_ref().unwrap().renders.root());
273273
assert_eq!(
274274
files,
275275
Vec::<String>::new(),

crates/spfs/src/commit_test.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@ use crate::Error;
1313
async fn test_commit_empty(tmpdir: tempfile::TempDir) {
1414
let root = tmpdir.path().to_string_lossy().to_string();
1515
let repo = crate::storage::RepositoryHandle::from(
16-
crate::storage::fs::FsRepository::create(&root)
16+
crate::storage::fs::MaybeOpenFsRepository::create(&root)
1717
.await
1818
.unwrap(),
1919
);
2020
let storage = crate::runtime::Storage::new(repo).unwrap();
2121
let repo = crate::storage::RepositoryHandle::from(
22-
crate::storage::fs::FsRepository::create(root)
22+
crate::storage::fs::MaybeOpenFsRepository::create(root)
2323
.await
2424
.unwrap(),
2525
);

crates/spfs/src/config.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,9 @@ impl RemoteConfig {
311311
inner,
312312
} = self;
313313
let mut handle: storage::RepositoryHandle = match inner.clone() {
314-
RepositoryConfig::Fs(config) => {
315-
storage::fs::FsRepository::from_config(config).await?.into()
316-
}
314+
RepositoryConfig::Fs(config) => storage::fs::MaybeOpenFsRepository::from_config(config)
315+
.await?
316+
.into(),
317317
RepositoryConfig::Tar(config) => storage::tar::TarRepository::from_config(config)
318318
.await?
319319
.into(),
@@ -555,7 +555,8 @@ impl Config {
555555
source,
556556
})?;
557557

558-
local_repo.set_tag_namespace(self.storage.tag_namespace.clone());
558+
Arc::make_mut(&mut local_repo.fs_impl)
559+
.set_tag_namespace(self.storage.tag_namespace.clone());
559560

560561
Ok(local_repo)
561562
}
@@ -564,7 +565,7 @@ impl Config {
564565
///
565566
/// The returned repo is guaranteed to be created, valid and open already. Ie
566567
/// the local repository is not allowed to be lazily opened.
567-
pub async fn get_local_repository(&self) -> Result<storage::fs::FsRepository> {
568+
pub async fn get_local_repository(&self) -> Result<storage::fs::OpenFsRepository> {
568569
self.get_opened_local_repository().await.map(Into::into)
569570
}
570571

crates/spfs/src/config_test.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ async fn test_config_get_remote() {
4141
.tempdir()
4242
.unwrap();
4343
let remote = tmpdir.path().join("remote");
44-
let _ = crate::storage::fs::FsRepository::create(&remote)
44+
let _ = crate::storage::fs::MaybeOpenFsRepository::create(&remote)
4545
.await
4646
.unwrap();
4747

crates/spfs/src/error.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ impl Error {
223223

224224
/// Create an [`Error::FailedToOpenRepository`] instance for
225225
/// a repository using its address and root cause.
226-
pub fn failed_to_open_repository<R: storage::Repository>(
226+
pub fn failed_to_open_repository<R: storage::Address>(
227227
repo: &R,
228228
source: storage::OpenRepositoryError,
229229
) -> Self {

crates/spfs/src/fixtures.rs

+15-8
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,19 @@ impl TempRepo {
3939
{
4040
match self {
4141
TempRepo::FS(_, tempdir) => {
42-
let mut repo = spfs::storage::fs::FsRepository::open(tempdir.path().join("repo"))
43-
.await
44-
.unwrap();
45-
repo.set_tag_namespace(Some(spfs::storage::TagNamespaceBuf::new(
46-
namespace.as_ref(),
47-
)));
42+
let repo = spfs::storage::fs::MaybeOpenFsRepository {
43+
fs_impl: {
44+
let mut fs_impl = spfs::storage::fs::MaybeOpenFsRepositoryImpl::open(
45+
tempdir.path().join("repo"),
46+
)
47+
.await
48+
.unwrap();
49+
fs_impl.set_tag_namespace(Some(spfs::storage::TagNamespaceBuf::new(
50+
namespace.as_ref(),
51+
)));
52+
fs_impl.into()
53+
},
54+
};
4855
TempRepo::FS(Arc::new(repo.into()), Arc::clone(tempdir))
4956
}
5057
_ => panic!("only TempRepo::FS type supports setting tag namespaces"),
@@ -136,7 +143,7 @@ pub async fn tmprepo(kind: &str) -> TempRepo {
136143
let tmpdir = tmpdir();
137144
match kind {
138145
"fs" => {
139-
let repo = spfs::storage::fs::FsRepository::create(tmpdir.path().join("repo"))
146+
let repo = spfs::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("repo"))
140147
.await
141148
.unwrap()
142149
.into();
@@ -153,7 +160,7 @@ pub async fn tmprepo(kind: &str) -> TempRepo {
153160
"rpc" => {
154161
use crate::storage::prelude::*;
155162
let repo = std::sync::Arc::new(spfs::storage::RepositoryHandle::FS(
156-
spfs::storage::fs::FsRepository::create(tmpdir.path().join("repo"))
163+
spfs::storage::fs::MaybeOpenFsRepository::create(tmpdir.path().join("repo"))
157164
.await
158165
.unwrap(),
159166
));

crates/spfs/src/runtime/storage.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1178,7 +1178,7 @@ impl Storage {
11781178
pub async fn durable_path(&self, name: String) -> Result<PathBuf> {
11791179
match &*self.inner {
11801180
RepositoryHandle::FS(repo) => {
1181-
let mut upper_root_path = repo.root();
1181+
let mut upper_root_path = repo.fs_impl.root();
11821182
upper_root_path.push(DURABLE_EDITS_DIR);
11831183
upper_root_path.push(name);
11841184
Ok(upper_root_path)

crates/spfs/src/runtime/storage_test.rs

+10-10
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ fn test_config_serialization() {
3535
async fn test_storage_create_runtime(tmpdir: tempfile::TempDir) {
3636
let root = tmpdir.path().to_string_lossy().to_string();
3737
let repo = crate::storage::RepositoryHandle::from(
38-
crate::storage::fs::FsRepository::create(root)
38+
crate::storage::fs::MaybeOpenFsRepository::create(root)
3939
.await
4040
.unwrap(),
4141
);
@@ -73,7 +73,7 @@ async fn test_storage_runtime_with_annotation(
7373

7474
let root = tmpdir.path().to_string_lossy().to_string();
7575
let repo = crate::storage::RepositoryHandle::from(
76-
crate::storage::fs::FsRepository::create(root)
76+
crate::storage::fs::MaybeOpenFsRepository::create(root)
7777
.await
7878
.unwrap(),
7979
);
@@ -139,7 +139,7 @@ async fn test_storage_runtime_add_annotations_list(
139139

140140
let root = tmpdir.path().to_string_lossy().to_string();
141141
let repo = crate::storage::RepositoryHandle::from(
142-
crate::storage::fs::FsRepository::create(root)
142+
crate::storage::fs::MaybeOpenFsRepository::create(root)
143143
.await
144144
.unwrap(),
145145
);
@@ -212,7 +212,7 @@ async fn test_storage_runtime_with_nested_annotation(
212212
// Setup the objects needed for the runtime used in the test
213213
let root = tmpdir.path().to_string_lossy().to_string();
214214
let repo = crate::storage::RepositoryHandle::from(
215-
crate::storage::fs::FsRepository::create(root)
215+
crate::storage::fs::MaybeOpenFsRepository::create(root)
216216
.await
217217
.unwrap(),
218218
);
@@ -281,7 +281,7 @@ async fn test_storage_runtime_with_annotation_all(
281281

282282
let root = tmpdir.path().to_string_lossy().to_string();
283283
let repo = crate::storage::RepositoryHandle::from(
284-
crate::storage::fs::FsRepository::create(root)
284+
crate::storage::fs::MaybeOpenFsRepository::create(root)
285285
.await
286286
.unwrap(),
287287
);
@@ -355,7 +355,7 @@ async fn test_storage_runtime_with_nested_annotation_all(
355355
// setup the objects needed for the runtime used in the test
356356
let root = tmpdir.path().to_string_lossy().to_string();
357357
let repo = crate::storage::RepositoryHandle::from(
358-
crate::storage::fs::FsRepository::create(root)
358+
crate::storage::fs::MaybeOpenFsRepository::create(root)
359359
.await
360360
.unwrap(),
361361
);
@@ -431,7 +431,7 @@ async fn test_storage_runtime_with_nested_annotation_all(
431431
async fn test_storage_remove_runtime(tmpdir: tempfile::TempDir) {
432432
let root = tmpdir.path().to_string_lossy().to_string();
433433
let repo = crate::storage::RepositoryHandle::from(
434-
crate::storage::fs::FsRepository::create(root)
434+
crate::storage::fs::MaybeOpenFsRepository::create(root)
435435
.await
436436
.unwrap(),
437437
);
@@ -452,7 +452,7 @@ async fn test_storage_remove_runtime(tmpdir: tempfile::TempDir) {
452452
async fn test_storage_iter_runtimes(tmpdir: tempfile::TempDir) {
453453
let root = tmpdir.path().to_string_lossy().to_string();
454454
let repo = crate::storage::RepositoryHandle::from(
455-
crate::storage::fs::FsRepository::create(root)
455+
crate::storage::fs::MaybeOpenFsRepository::create(root)
456456
.await
457457
.unwrap(),
458458
);
@@ -504,7 +504,7 @@ async fn test_storage_iter_runtimes(tmpdir: tempfile::TempDir) {
504504
async fn test_runtime_reset(tmpdir: tempfile::TempDir) {
505505
let root = tmpdir.path().to_string_lossy().to_string();
506506
let repo = crate::storage::RepositoryHandle::from(
507-
crate::storage::fs::FsRepository::create(root)
507+
crate::storage::fs::MaybeOpenFsRepository::create(root)
508508
.await
509509
.unwrap(),
510510
);
@@ -551,7 +551,7 @@ async fn test_runtime_reset(tmpdir: tempfile::TempDir) {
551551
async fn test_runtime_ensure_extra_bind_mount_locations_exist(tmpdir: tempfile::TempDir) {
552552
let root = tmpdir.path().to_string_lossy().to_string();
553553
let repo = crate::storage::RepositoryHandle::from(
554-
crate::storage::fs::FsRepository::create(root)
554+
crate::storage::fs::MaybeOpenFsRepository::create(root)
555555
.await
556556
.unwrap(),
557557
);

crates/spfs/src/storage/fallback/repository.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,8 @@ impl TagStorageMut for FallbackProxy {
361361
&mut self,
362362
tag_namespace: Option<TagNamespaceBuf>,
363363
) -> Result<Option<TagNamespaceBuf>> {
364-
Ok(Arc::make_mut(&mut self.primary).set_tag_namespace(tag_namespace))
364+
Ok(Arc::make_mut(&mut Arc::make_mut(&mut self.primary).fs_impl)
365+
.set_tag_namespace(tag_namespace))
365366
}
366367
}
367368

@@ -386,12 +387,12 @@ impl Address for FallbackProxy {
386387
impl LocalRepository for FallbackProxy {
387388
#[inline]
388389
fn payloads(&self) -> &FsHashStore {
389-
self.primary.payloads()
390+
self.primary.fs_impl.payloads()
390391
}
391392

392393
#[inline]
393394
fn render_store(&self) -> Result<&RenderStore> {
394-
self.primary.render_store()
395+
self.primary.fs_impl.render_store()
395396
}
396397
}
397398

crates/spfs/src/storage/fallback/repository_test.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ async fn test_proxy_payload_repair(tmpdir: tempfile::TempDir) {
3636
.unwrap();
3737

3838
// Delete the payload file from the primary repo.
39-
let payload_path = primary.payloads.build_digest_path(&digest);
39+
let payload_path = primary.fs_impl.payloads.build_digest_path(&digest);
4040
tokio::fs::remove_file(payload_path).await.unwrap();
4141

4242
// Loading the payload from the primary should fail.

0 commit comments

Comments
 (0)