1fe1e73 storage: Repository locking for Windows — heartwood

heartwood

Radicle Heartwood Protocol & Stack

storage: Repository locking for Windows

◌ CI pending Lorenz Leutgeb committed 9 months ago

commit 1fe1e738294ca807f9a4ba304b1183f024500bc4
parent 19a262d3d67ea92a37ceaf18e79a783d8e65ccdd

2 pending (2 total) View logs

2 files changed +75 -16

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

@@ -10,6 +10,7 @@ use radicle::identity::crefs::GetCanonicalRefs as _;

 use radicle::prelude::NodeId;
 use radicle::prelude::RepoId;
 use radicle::storage::git::Repository;
 use radicle::storage::git::TemporaryPath;
 use radicle::storage::refs::RefsAt;
 use radicle::storage::{
     ReadRepository, ReadStorage as _, RefUpdate, RemoteRepository, RepositoryError,

@@ -25,7 +26,7 @@ use super::channels::ChannelsFlush;

 pub enum Handle {
     Clone {
         handle: radicle_fetch::Handle<ChannelsFlush>,
         tmp: tempfile::TempDir,
         tmp: TemporaryPath,
     },
     Pull {
         handle: radicle_fetch::Handle<ChannelsFlush>,

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

         let (result, clone, notifs) = match self {
             Self::Clone { mut handle, tmp } => {
                 log::debug!(target: "worker", "{} cloning from {remote}", handle.local());
                 let result = radicle_fetch::clone(&mut handle, limit, remote)?;
                 mv(tmp, storage, &rid)?;
                 (result, true, None)
                 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)
                     }
                 }
             }
             Self::Pull {
                 mut handle,

@@ -168,16 +177,19 @@ impl Handle {

 ///
 /// # Errors
 ///   - Will fail if `storage` contains `rid` already.
 fn mv(tmp: tempfile::TempDir, storage: &Storage, rid: &RepoId) -> Result<(), error::Fetch> {
 fn mv(tmp: TemporaryPath, storage: &Storage, rid: &RepoId) -> Result<(), error::Fetch> {
     use std::io::{Error, ErrorKind};
     let from = tmp.path();
     let to = storage.path_of(rid);
     if !to.exists() {
         std::fs::rename(from, 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:?}"),

@@ -188,6 +200,14 @@ fn mv(tmp: tempfile::TempDir, storage: &Storage, rid: &RepoId) -> Result<(), err

     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/src/storage/git.rs

@@ -9,7 +9,6 @@ use std::sync::LazyLock;

 use std::{fs, io};
 use crypto::Verified;
 use tempfile::TempDir;
 use crate::identity::crefs::GetCanonicalRefs as _;
 use crate::identity::doc::DocError;

@@ -197,6 +196,12 @@ impl WriteStorage for Storage {

     }
 }
 #[cfg(unix)]
 pub use tempfile::TempDir as TemporaryPath;
 #[cfg(windows)]
 pub use std::path::PathBuf as TemporaryPath;
 impl Storage {
     /// Open a new storage instance and load its inventory.
     pub fn open<P: AsRef<Path>>(path: P, info: UserInfo) -> Result<Self, Error> {

@@ -212,23 +217,57 @@ impl Storage {

     /// Create a [`Repository`] in a temporary directory.
     ///
     /// N.b. it is important to keep the [`TempDir`] in scope while
     /// using the [`Repository`]. If it is dropped, any action on the
     /// `Repository` will fail.
     pub fn lock_repository(&self, rid: RepoId) -> Result<(Repository, TempDir), RepositoryError> {
     /// This is used to prevent other processes accessing it during
     /// initialization. Usually, callers will want to move the repository
     /// 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).
     pub fn lock_repository(
         &self,
         rid: RepoId,
     ) -> Result<(Repository, TemporaryPath), RepositoryError> {
         const RANDOMNESS_LENGTH: usize = 6;
         const SUFFIX: &str = ".lock";
         let prefix = rid.canonical() + ".";
         if self.contains(&rid)? {
             return Err(Error::Io(io::Error::new(
                 io::ErrorKind::AlreadyExists,
                 format!("refusing to create '{rid}.lock'"),
                 format!("refusing to create '{prefix}*{SUFFIX}'"),
             ))
             .into());
         }
         #[cfg(unix)]
         let tmp = tempfile::Builder::new()
             .prefix(&rid.canonical())
             .suffix(".lock")
             .prefix(&prefix)
             .suffix(SUFFIX)
             .rand_bytes(RANDOMNESS_LENGTH)
             .tempdir_in(self.path())
             .map_err(Error::from)?;
         Ok((Repository::create(tmp.path(), rid, &self.info)?, tmp))
         #[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
         };
         Ok((Repository::create(&tmp, rid, &self.info)?, tmp))
     }
     pub fn path(&self) -> &Path {