df78783 storage: Cross-platform implementation of locking — heartwood

heartwood

Radicle Heartwood Protocol & Stack

storage: Cross-platform implementation of locking

✗ CI failure Lorenz Leutgeb committed 7 months ago

commit df7878365f6806f951e1ba7afd7bc9995f210b95
parent ee49e28766ce7c703b95e22d177cce046072f03d

1 passed 1 failed 1 pending (3 total) View logs

3 files changed +65 -18

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

@@ -13,6 +13,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,

@@ -28,7 +29,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>,

@@ -173,16 +174,16 @@ 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");
         return Err(Error::new(
             ErrorKind::AlreadyExists,
             format!("repository already exists {to:?}"),

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

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

 use std::{fs, io};
 use crypto::Verified;
 use tempfile::TempDir;
 use crate::git::canonical::Quorum;
 use crate::identity::crefs::GetCanonicalRefs as _;

@@ -198,6 +197,33 @@ impl WriteStorage for Storage {

     }
 }
 pub struct TemporaryPath {
     inner: std::path::PathBuf,
 }
 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) {
             if err.kind() != io::ErrorKind::NotFound {
                 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.
     pub fn open<P: AsRef<Path>>(path: P, info: UserInfo) -> Result<Self, Error> {

@@ -213,23 +239,43 @@ 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.
     ///
     /// 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,
     ) -> 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());
         }
         let tmp = tempfile::Builder::new()
             .prefix(&rid.canonical())
             .suffix(".lock")
             .tempdir_in(self.path())
             .map_err(Error::from)?;
         Ok((Repository::create(tmp.path(), rid, &self.info)?, 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)?,
             TemporaryPath::new(tmp),
         ))
     }
     pub fn path(&self) -> &Path {