6fbcbd6 storage: Cross-platform implementation of locking — heartwood

modified crates/radicle-node/src/worker/fetch.rs

@@ -72,17 +72,9 @@ impl Handle {

         let (result, clone, notifs) = match self {
             Self::Clone { mut handle, tmp } => {
                 log::debug!(target: "worker", "{} cloning from {remote}", handle.local());
                 match radicle_fetch::clone(&mut handle, limit, remote) {
                     Err(err) => {
                         #[cfg(windows)]
                         cleanup(tmp);
                         return Err(err.into());
                     }
                     Ok(result) => {
                         mv(tmp, storage, &rid)?;
                         (result, true, None)
                     }
                 }
                 let result = radicle_fetch::clone(&mut handle, limit, remote)?;
                 mv(tmp, storage, &rid)?;
                 (result, true, None)
             }
             Self::Pull {
                 mut handle,

@@ -183,13 +175,10 @@ fn mv(tmp: TemporaryPath, storage: &Storage, rid: &RepoId) -> Result<(), error::

     let to = storage.path_of(rid);
     if !to.exists() {
         std::fs::rename(&tmp, to)?;
         std::fs::rename(tmp, to)?;
     } else {
         log::warn!(target: "worker", "Refusing to move cloned repository {rid} already exists");
         #[cfg(windows)]
         cleanup(tmp);
         return Err(Error::new(
             ErrorKind::AlreadyExists,
             format!("repository already exists {to:?}"),

@@ -200,14 +189,6 @@ fn mv(tmp: TemporaryPath, storage: &Storage, rid: &RepoId) -> Result<(), error::

     Ok(())
 }
 #[cfg(windows)]
 fn cleanup(path: TemporaryPath) {
     if let Err(err) = std::fs::remove_dir_all(&path) {
         let display = path.display();
         log::error!(target: "worker", "Failed to remove temporary directory '{display}': {err}");
     }
 }
 // Post notifications for the given refs.
 fn notify(
     rid: &RepoId,

modified crates/radicle/Cargo.toml

@@ -11,7 +11,7 @@ rust-version.workspace = true

 [features]
 default = []
 test = ["qcheck", "radicle-crypto/test"]
 test = ["tempfile", "qcheck", "radicle-crypto/test"]
 logger = ["colored", "chrono"]
 [dependencies]

@@ -42,7 +42,7 @@ serde_json = { workspace = true, features = ["preserve_order"] }

 serde-untagged = "0.1.7"
 siphasher = "1.0.0"
 sqlite = { workspace = true, features = ["bundled"] }
 tempfile = { workspace = true }
 tempfile = { workspace = true, optional = true }
 thiserror = { workspace = true }
 unicode-normalization = { version = "0.1" }

modified crates/radicle/src/storage/git.rs

@@ -196,11 +196,30 @@ impl WriteStorage for Storage {

     }
 }
 #[cfg(unix)]
 pub use tempfile::TempDir as TemporaryPath;
 pub struct TemporaryPath {
     inner: std::path::PathBuf,
 }
 #[cfg(windows)]
 pub use std::path::PathBuf as TemporaryPath;
 impl TemporaryPath {
     fn new(inner: std::path::PathBuf) -> Self {
         Self { inner }
     }
 }
 impl AsRef<std::path::Path> for TemporaryPath {
     fn as_ref(&self) -> &std::path::Path {
         &self.inner
     }
 }
 impl Drop for TemporaryPath {
     fn drop(&mut self) {
         if let Err(err) = std::fs::remove_dir_all(&self.inner) {
             let display = self.inner.display();
             log::error!(target: "worker", "Failed to remove temporary directory '{display}': {err}");
         }
     }
 }
 impl Storage {
     /// Open a new storage instance and load its inventory.

@@ -222,15 +241,10 @@ impl Storage {

     /// to its destination after initialization in the temporary location
     /// returned by this function.
     ///
     /// On Unix-like system, upon success, returns a [`TempDir`].
     /// Make sure not to only drop it when the directory is not required
     /// anymore, as it removes the directory behind the file handle when dropped.
     /// Subsequent operations on the [`Repository`] will then fail.
     ///
     /// On Windows, we cannot keep a handle of the directory, because this
     /// prevents it from being moved. So, this returns a plain [`PathBuf`] and
     /// callers must take care to remove the directory when they do not need
     /// it anymore (probably also in error cases).
     /// The implementation of [`Drop`] of the returned [`TemporaryPath`]
     /// will remove the temporary directory. Thus, callers must ensure that
     /// this only happens *after* initialization has succeeded and the
     /// repository was moved to its destination in storage.
     pub fn lock_repository(
         &self,
         rid: RepoId,

@@ -248,26 +262,17 @@ impl Storage {

             .into());
         }
         #[cfg(unix)]
         let tmp = tempfile::Builder::new()
             .prefix(&prefix)
             .suffix(SUFFIX)
             .rand_bytes(RANDOMNESS_LENGTH)
             .tempdir_in(self.path())
             .map_err(Error::from)?;
         #[cfg(windows)]
         let tmp = {
             let randomness = std::iter::repeat_with(fastrand::alphanumeric)
                 .take(RANDOMNESS_LENGTH)
                 .collect::<String>();
             let mut tmp = self.path().to_path_buf();
             tmp.push(format!("{prefix}{randomness}{SUFFIX}"));
             tmp
         };
         let randomness = std::iter::repeat_with(fastrand::alphanumeric)
             .take(RANDOMNESS_LENGTH)
             .collect::<String>();
         let mut tmp = self.path().to_path_buf();
         tmp.push(format!("{prefix}{randomness}{SUFFIX}"));
         Ok((Repository::create(&tmp, rid, &self.info)?, tmp))
         Ok((
             Repository::create(&tmp, rid, &self.info)?,
             TemporaryPath::new(tmp),
         ))
     }
     pub fn path(&self) -> &Path {