Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
storage: Rewrite temporary repositories for clones
◌ CI pending Lorenz Leutgeb committed 7 months ago
commit dbdef0d843205eaf5d271ebe8b604c82845ed8c2
parent 5cd016b587a2a90f2321af41122cc12b01b7f391
1 pending (1 total) View logs
4 files changed +58 -31
modified crates/radicle-cli/tests/commands.rs
@@ -1375,7 +1375,7 @@ fn rad_clone_partial_fail() {
    eve.connect(&bob);
    eve.routes_to(&[(acme, carol), (acme, bob.id), (acme, alice.id)]);
    bob.storage.repository(acme).unwrap().remove().unwrap(); // Cause the fetch from Bob to fail.
-
    bob.storage.lock_repository(acme).ok(); // Prevent repo from being re-fetched.
+
    bob.storage.temporary_repository(acme).ok(); // Prevent repo from being re-fetched.

    test(
        "examples/rad-clone-partial-fail.md",
modified crates/radicle-node/src/worker/fetch.rs
@@ -3,6 +3,7 @@ use radicle::identity::CanonicalRefs;
pub(crate) use radicle_protocol::worker::fetch::error;

use std::collections::BTreeSet;
+
use std::path::Path;
use std::str::FromStr;

use localtime::LocalTime;
@@ -28,7 +29,6 @@ use super::channels::ChannelsFlush;
pub enum Handle {
    Clone {
        handle: radicle_fetch::Handle<ChannelsFlush>,
-
        tmp: tempfile::TempDir,
    },
    Pull {
        handle: radicle_fetch::Handle<ChannelsFlush>,
@@ -55,9 +55,9 @@ impl Handle {
                notifications,
            })
        } else {
-
            let (repo, tmp) = storage.lock_repository(rid)?;
+
            let repo = storage.temporary_repository(rid)?;
            let handle = radicle_fetch::Handle::new(local, repo, follow, blocked, channels)?;
-
            Ok(Handle::Clone { handle, tmp })
+
            Ok(Handle::Clone { handle })
        }
    }

@@ -72,11 +72,21 @@ impl Handle {
        refs_at: Option<Vec<RefsAt>>,
    ) -> Result<FetchResult, error::Fetch> {
        let (result, clone, notifs) = match self {
-
            Self::Clone { mut handle, tmp } => {
+
            Self::Clone { mut handle } => {
                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)
+
                let path = handle.repository().path().to_path_buf();
+
                match radicle_fetch::clone(&mut handle, limit, remote) {
+
                    Err(err) => {
+
                        drop(handle);
+
                        cleanup(path);
+
                        return Err(err.into());
+
                    }
+
                    Ok(result) => {
+
                        drop(handle);
+
                        mv(path, storage, &rid)?;
+
                        (result, true, None)
+
                    }
+
                }
            }
            Self::Pull {
                mut handle,
@@ -173,24 +183,31 @@ impl Handle {
///
/// # Errors
///   - Will fail if `storage` contains `rid` already.
-
fn mv(tmp: tempfile::TempDir, storage: &Storage, rid: &RepoId) -> Result<(), error::Fetch> {
+
fn mv(path: impl AsRef<Path>, storage: &Storage, rid: &RepoId) -> std::io::Result<()> {
    use std::io::{Error, ErrorKind};
+
    let path = path.as_ref();

-
    let from = tmp.path();
    let to = storage.path_of(rid);
+
    if to.exists() {
+
        log::warn!(target: "worker", "Refusing to move from temporary directory '{}' because destination {rid} already exists. Removing the temporary directory.", path.display());
+

+
        cleanup(path);

-
    if !to.exists() {
-
        std::fs::rename(from, 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:?}"),
-
        )
-
        .into());
+
        ));
    }

-
    Ok(())
+
    std::fs::rename(path, to)
+
}
+

+
fn cleanup(path: impl AsRef<Path>) {
+
    let path = path.as_ref();
+
    if let Err(err) = std::fs::remove_dir_all(path) {
+
        let path = path.display();
+
        log::error!(target: "worker", "Failed to remove temporary directory '{path}': {err}");
+
    }
}

// Post notifications for the given refs.
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 _;
@@ -213,23 +212,34 @@ 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.
+
    pub fn temporary_repository(&self, rid: RepoId) -> Result<Repository, RepositoryError> {
+
        const SUFFIX: &str = ".tmp";
+
        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 tmp = {
+
            const RANDOMNESS_LENGTH: usize = 6;
+

+
            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)?)
    }

    pub fn path(&self) -> &Path {