Impl Clone for Filesystem

This surprisingly small addition required some bigger changes. Filesystems are now lifetime-unbound, and rely on internal `Arc`s instead of references. Technically this means higher heap pressure and slower code due to increased use of atomics, but `Filesystem`s are by nature not part of hot paths, so this is fine.

Signed-off-by: Marek Kaput <marek.kaput@swmansion.com>

commit-id:f7f37d16

scarb/src/compiler/compilation_unit.rs

@@ -77,7 +77,7 @@ impl CompilationUnit {
         &self.main_component().target
     }
 
-    pub fn target_dir<'c>(&self, ws: &'c Workspace<'_>) -> Filesystem<'c> {
+    pub fn target_dir(&self, ws: &Workspace<'_>) -> Filesystem {
         ws.target_dir().child(self.profile.as_str())
     }
 

scarb/src/compiler/helpers.rs

@@ -46,7 +46,7 @@ pub fn collect_all_crate_ids(unit: &CompilationUnit, db: &RootDatabase) -> Vec<C
 pub fn write_json(
     file_name: &str,
     description: &str,
-    target_dir: &Filesystem<'_>,
+    target_dir: &Filesystem,
     ws: &Workspace<'_>,
     value: impl Serialize,
 ) -> Result<()> {
@@ -60,7 +60,7 @@ pub fn write_json(
 pub fn write_string(
     file_name: &str,
     description: &str,
-    target_dir: &Filesystem<'_>,
+    target_dir: &Filesystem,
     ws: &Workspace<'_>,
     value: impl ToString,
 ) -> Result<()> {

scarb/src/core/dirs.rs

@@ -7,13 +7,13 @@ use anyhow::{anyhow, Result};
 use camino::Utf8PathBuf;
 use directories::ProjectDirs;
 
-use crate::flock::{Filesystem, RootFilesystem};
+use crate::flock::Filesystem;
 use crate::internal::fsx::PathUtf8Ext;
 
 #[derive(Debug)]
 pub struct AppDirs {
-    pub cache_dir: RootFilesystem,
-    pub config_dir: RootFilesystem,
+    pub cache_dir: Filesystem,
+    pub config_dir: Filesystem,
     pub path_dirs: Vec<PathBuf>,
 }
 
@@ -57,8 +57,8 @@ impl AppDirs {
         };
 
         Ok(Self {
-            cache_dir: RootFilesystem::new_output_dir(cache_dir),
-            config_dir: RootFilesystem::new(config_dir),
+            cache_dir: Filesystem::new_output_dir(cache_dir),
+            config_dir: Filesystem::new(config_dir),
             path_dirs,
         })
     }
@@ -67,7 +67,7 @@ impl AppDirs {
         env::join_paths(self.path_dirs.iter()).unwrap()
     }
 
-    pub fn registry_dir(&self) -> Filesystem<'_> {
+    pub fn registry_dir(&self) -> Filesystem {
         self.cache_dir.child("registry")
     }
 }

scarb/src/core/registry/client/http.rs

@@ -29,7 +29,7 @@ pub struct HttpRegistryClient<'c> {
     source_id: SourceId,
     config: &'c Config,
     cached_index_config: OnceCell<IndexConfig>,
-    dl_fs: Filesystem<'c>,
+    dl_fs: Filesystem,
 }
 
 enum HttpCacheKey {

scarb/src/core/registry/package_source_store.rs

@@ -11,7 +11,7 @@ use crate::internal::fsx::PathUtf8Ext;
 use crate::internal::restricted_names::is_windows_restricted_path;
 
 pub struct PackageSourceStore<'a> {
-    fs: Filesystem<'a>,
+    fs: Filesystem,
     config: &'a Config,
 }
 

scarb/src/core/workspace.rs

@@ -9,7 +9,7 @@ use crate::compiler::Profile;
 use crate::core::config::Config;
 use crate::core::package::Package;
 use crate::core::PackageId;
-use crate::flock::RootFilesystem;
+use crate::flock::Filesystem;
 use crate::{DEFAULT_TARGET_DIR_NAME, LOCK_FILE_NAME, MANIFEST_FILE_NAME};
 
 /// The core abstraction for working with a workspace of packages.
@@ -22,7 +22,7 @@ pub struct Workspace<'c> {
     manifest_path: Utf8PathBuf,
     profiles: Vec<Profile>,
     root_package: Option<PackageId>,
-    target_dir: RootFilesystem,
+    target_dir: Filesystem,
 }
 
 impl<'c> Workspace<'c> {
@@ -43,7 +43,7 @@ impl<'c> Workspace<'c> {
                 .expect("parent of manifest path must always exist")
                 .join(DEFAULT_TARGET_DIR_NAME)
         });
-        let target_dir = RootFilesystem::new_output_dir(target_dir);
+        let target_dir = Filesystem::new_output_dir(target_dir);
         Ok(Self {
             config,
             manifest_path,
@@ -90,7 +90,7 @@ impl<'c> Workspace<'c> {
         self.root().join(LOCK_FILE_NAME)
     }
 
-    pub fn target_dir(&self) -> &RootFilesystem {
+    pub fn target_dir(&self) -> &Filesystem {
         &self.target_dir
     }
 

scarb/src/flock.rs

@@ -144,7 +144,7 @@ pub struct AdvisoryLock<'f> {
         // (only guards do).
         Weak<FileLockGuard>,
     >,
-    filesystem: &'f Filesystem<'f>,
+    filesystem: &'f Filesystem,
     config: &'f Config,
 }
 
@@ -176,25 +176,23 @@ impl<'f> AdvisoryLock<'f> {
     }
 }
 
-/// A [`Filesystem`] that does not have a parent.
-pub type RootFilesystem = Filesystem<'static>;
-
 /// A [`Filesystem`] is intended to be a globally shared, hence locked, resource in Scarb.
 ///
 /// The [`Utf8Path`] of a file system cannot be learned unless it's done in a locked fashion,
 /// and otherwise functions on this structure are prepared to handle concurrent invocations across
 /// multiple instances of Scarb and its extensions.
 ///
 /// All paths within a [`Filesystem`] must be UTF-8 encoded.
-pub struct Filesystem<'a> {
-    root: LazyDirectoryCreator<'a>,
+#[derive(Clone)]
+pub struct Filesystem {
+    root: Arc<LazyDirectoryCreator>,
 }
 
-impl<'a> Filesystem<'a> {
+impl Filesystem {
     /// Creates a new [`Filesystem`] to be rooted at the given path.
     pub fn new(root: Utf8PathBuf) -> Self {
         Self {
-            root: LazyDirectoryCreator::new(root),
+            root: LazyDirectoryCreator::new(root, false),
         }
     }
 
@@ -204,12 +202,12 @@ impl<'a> Filesystem<'a> {
     /// directory.
     pub fn new_output_dir(root: Utf8PathBuf) -> Self {
         Self {
-            root: LazyDirectoryCreator::new_output_dir(root),
+            root: LazyDirectoryCreator::new(root, true),
         }
     }
 
     /// Like [`Utf8Path::join`], creates a new [`Filesystem`] rooted at a subdirectory of this one.
-    pub fn child(&self, path: impl AsRef<Utf8Path>) -> Filesystem<'_> {
+    pub fn child(&self, path: impl AsRef<Utf8Path>) -> Filesystem {
         Filesystem {
             root: self.root.child(path),
         }
@@ -218,7 +216,7 @@ impl<'a> Filesystem<'a> {
     /// Like [`Utf8Path::join`], creates a new [`Filesystem`] rooted at a subdirectory of this one.
     ///
     /// Unlike [`Filesystem::child`], this method consumes the current [`Filesystem`].
-    pub fn into_child(self, path: impl AsRef<Utf8Path>) -> Filesystem<'a> {
+    pub fn into_child(self, path: impl AsRef<Utf8Path>) -> Filesystem {
         Filesystem {
             root: self.root.into_child(path),
         }
@@ -332,7 +330,7 @@ impl<'a> Filesystem<'a> {
     }
 
     /// Construct an [`AdvisoryLock`] within this file system.
-    pub fn advisory_lock(
+    pub fn advisory_lock<'a>(
         &'a self,
         path: impl AsRef<Utf8Path>,
         description: impl ToString,
@@ -380,15 +378,17 @@ impl<'a> Filesystem<'a> {
     }
 }
 
-impl<'a> fmt::Display for Filesystem<'a> {
+impl fmt::Display for Filesystem {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{}", self.root)
     }
 }
 
-impl<'a> fmt::Debug for Filesystem<'a> {
+impl fmt::Debug for Filesystem {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.debug_tuple("Filesystem").field(&self.root).finish()
+        f.debug_tuple("Filesystem")
+            .field(self.root.deref())
+            .finish()
     }
 }
 
@@ -405,7 +405,7 @@ impl<'a> fmt::Debug for Filesystem<'a> {
 /// in examples tests, when the second condition was missing.
 macro_rules! protected_run_if_not_ok {
     ($fs:expr, $lock:expr, $body:block) => {{
-        let fs: &$crate::flock::Filesystem<'_> = $fs;
+        let fs: &$crate::flock::Filesystem = $fs;
         let lock: &$crate::flock::AdvisoryLock<'_> = $lock;
         if !fs.is_ok() {
             let _lock = lock.acquire_async().await?;

scarb/src/internal/lazy_directory_creator.rs

@@ -1,4 +1,4 @@
-use std::ops::Deref;
+use std::sync::Arc;
 use std::{fmt, path};
 
 use anyhow::Result;
@@ -8,46 +8,39 @@ use tracing::trace;
 
 use crate::internal::fsx;
 
-pub struct LazyDirectoryCreator<'p> {
+pub struct LazyDirectoryCreator {
     path: Utf8PathBuf,
     creation_lock: OnceCell<()>,
-    parent: Option<Calf<'p, LazyDirectoryCreator<'p>>>,
+    parent: Option<Arc<LazyDirectoryCreator>>,
     is_output_dir: bool,
 }
 
-impl<'p> LazyDirectoryCreator<'p> {
-    pub fn new(path: impl Into<Utf8PathBuf>) -> Self {
-        Self {
+impl LazyDirectoryCreator {
+    pub fn new(path: impl Into<Utf8PathBuf>, is_output_dir: bool) -> Arc<Self> {
+        Arc::new(Self {
             path: path.into(),
             creation_lock: OnceCell::new(),
             parent: None,
-            is_output_dir: false,
-        }
+            is_output_dir,
+        })
     }
 
-    pub fn new_output_dir(path: impl Into<Utf8PathBuf>) -> Self {
-        Self {
-            is_output_dir: true,
-            ..Self::new(path)
-        }
-    }
-
-    pub fn child(&'p self, path: impl AsRef<Utf8Path>) -> Self {
-        Self {
+    pub fn child(self: &Arc<Self>, path: impl AsRef<Utf8Path>) -> Arc<Self> {
+        Arc::new(Self {
             path: self.path.join(path),
             creation_lock: OnceCell::new(),
-            parent: Some(Calf::Borrowed(self)),
+            parent: Some(self.clone()),
             is_output_dir: false,
-        }
+        })
     }
 
-    pub fn into_child(self, path: impl AsRef<Utf8Path>) -> Self {
-        Self {
+    pub fn into_child(self: Arc<Self>, path: impl AsRef<Utf8Path>) -> Arc<Self> {
+        Arc::new(Self {
             path: self.path.join(path),
             creation_lock: OnceCell::new(),
-            parent: Some(Calf::Owned(Box::new(self))),
+            parent: Some(self),
             is_output_dir: false,
-        }
+        })
     }
 
     pub fn as_unchecked(&self) -> &Utf8Path {
@@ -86,13 +79,13 @@ impl<'p> LazyDirectoryCreator<'p> {
     }
 }
 
-impl<'p> fmt::Display for LazyDirectoryCreator<'p> {
+impl fmt::Display for LazyDirectoryCreator {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{}", self.as_unchecked())
     }
 }
 
-impl<'p> fmt::Debug for LazyDirectoryCreator<'p> {
+impl fmt::Debug for LazyDirectoryCreator {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let base = self
             .parent
@@ -119,19 +112,3 @@ impl<'p> fmt::Debug for LazyDirectoryCreator<'p> {
         Ok(())
     }
 }
-
-enum Calf<'a, T> {
-    Borrowed(&'a T),
-    Owned(Box<T>),
-}
-
-impl<'a, T: 'a> Deref for Calf<'a, T> {
-    type Target = T;
-
-    fn deref(&self) -> &Self::Target {
-        match self {
-            Calf::Borrowed(t) => t,
-            Calf::Owned(t) => t,
-        }
-    }
-}

scarb/src/sources/git/client.rs

@@ -110,7 +110,7 @@ impl GitRemote {
     #[tracing::instrument(level = "trace", skip(config))]
     pub fn checkout(
         &self,
-        fs: &Filesystem<'_>,
+        fs: &Filesystem,
         db: Option<GitDatabase>,
         reference: &GitReference,
         locked_rev: Option<Rev>,
@@ -155,7 +155,7 @@ impl GitRemote {
 
 impl GitDatabase {
     #[tracing::instrument(level = "trace")]
-    pub fn open(remote: &GitRemote, fs: &Filesystem<'_>) -> Result<Self> {
+    pub fn open(remote: &GitRemote, fs: &Filesystem) -> Result<Self> {
         let path = fs.path_existent()?;
         let opts = gix::open::Options::default().open_path_as_is(true);
         let repo = gix::open_opts(path, opts)?;
@@ -167,7 +167,7 @@ impl GitDatabase {
     }
 
     #[tracing::instrument(level = "trace")]
-    pub fn init_bare(remote: &GitRemote, fs: &Filesystem<'_>) -> Result<Self> {
+    pub fn init_bare(remote: &GitRemote, fs: &Filesystem) -> Result<Self> {
         let path = fs.path_existent()?;
         let repo = gix::init_bare(path)?;
         Ok(Self {
@@ -201,12 +201,7 @@ impl GitDatabase {
         exec(&mut cmd, config)
     }
 
-    pub fn copy_to(
-        &self,
-        fs: &Filesystem<'_>,
-        rev: Rev,
-        config: &Config,
-    ) -> Result<GitCheckout<'_>> {
+    pub fn copy_to(&self, fs: &Filesystem, rev: Rev, config: &Config) -> Result<GitCheckout<'_>> {
         let checkout = GitCheckout::clone(self, fs, rev, config)?;
         checkout.reset(config)?;
         Ok(checkout)
@@ -264,7 +259,7 @@ impl GitDatabase {
 
 impl<'d> GitCheckout<'d> {
     #[tracing::instrument(level = "trace", skip(config))]
-    fn clone(db: &'d GitDatabase, fs: &Filesystem<'_>, rev: Rev, config: &Config) -> Result<Self> {
+    fn clone(db: &'d GitDatabase, fs: &Filesystem, rev: Rev, config: &Config) -> Result<Self> {
         unsafe {
             fs.recreate()?;
         }

scarb/src/sources/git/mod.rs

@@ -99,9 +99,7 @@ impl<'c> GitSource<'c> {
 
             let git_fs = config.dirs().registry_dir().into_child("git");
 
-            let db_fs = git_fs
-                .child("db")
-                .into_child(&format!("{remote_ident}.git"));
+            let db_fs = git_fs.child("db").into_child(format!("{remote_ident}.git"));
 
             let db = GitDatabase::open(&remote, &db_fs).ok();
             let (db, actual_rev) = match (db, locked_rev) {