From 5b1cdbaa4ada3565a019eb743785b2fa916e31da Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Tue, 28 Nov 2023 13:23:22 +0000 Subject: [PATCH] radicle: remove repo iff there's no local rad/sigrefs If the local peer does not have any `rad/sigrefs` then there is no risk of any data corruption. In this case the repository can be removed entirely. Signed-off-by: Fintan Halpenny X-Clacks-Overhead: GNU Terry Pratchett --- radicle-cli/examples/rad-clean.md | 29 +++++++++++++++++---------- radicle-cli/src/commands/clean.rs | 2 +- radicle-cli/tests/commands.rs | 33 ++++++++++++++++++++++++------- radicle/src/storage.rs | 9 +++++++++ radicle/src/storage/git.rs | 17 +++++++++++++--- 5 files changed, 69 insertions(+), 21 deletions(-) diff --git a/radicle-cli/examples/rad-clean.md b/radicle-cli/examples/rad-clean.md index 63d0cfe3..2d09bffe 100644 --- a/radicle-cli/examples/rad-clean.md +++ b/radicle-cli/examples/rad-clean.md @@ -5,7 +5,7 @@ clean` command. First let's look at what we have locally: -``` +``` ~alice $ rad ls ╭───────────────────────────────────────────────────────────────────────────────────────────────────────────╮ │ Name RID Visibility Head Description │ @@ -16,7 +16,7 @@ $ rad ls Let's also inspect what remotes are in the repository: -``` +``` ~alice $ rad inspect --sigrefs z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi f209c9f68aa689af24220a20462e13ee9dfb2a95 z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk 161b775a3509c8098de67f57f750972bba015b31 @@ -24,7 +24,7 @@ z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk 161b775a3509c8098de67f57f750972 Now let's clean the `heartwood` project: -``` +``` ~alice $ rad clean rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --no-confirm Removed z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk ✓ Successfully cleaned rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji @@ -32,7 +32,7 @@ Removed z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk Inspecting the remotes again, we see that Bob is now gone: -``` +``` ~alice $ rad inspect --sigrefs z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi f209c9f68aa689af24220a20462e13ee9dfb2a95 ``` @@ -44,15 +44,24 @@ possible to stop fetching Bob for this particular repository. Cleaning a repository again will remove no remotes, since we're already at the minimal set of remotes: -``` +``` ~alice $ rad clean rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --no-confirm ✓ Successfully cleaned rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji ``` -And attempting to clean a non-existent repository has no effect on the -storage at all: +Since Eve did not fork the repository, and has no refs of their own, +when they run `rad clean` it will remove the project entirely: -``` (fail) -$ rad clean rad:z42hL2jL4XNk6K8oHQaSWdeadbeef --no-confirm -✗ Error: repository rad:z42hL2jL4XNk6K8oHQaSWdeadbeef was not found +``` ~eve +$ rad clean rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --no-confirm +Removed z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi +✓ Successfully cleaned rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji +``` + +And attempting to clean the repository again, or any non-existent +repository, has no effect on the storage at all: + +``` ~eve (fail) +$ rad clean rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --no-confirm +✗ Error: repository rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji was not found ``` diff --git a/radicle-cli/src/commands/clean.rs b/radicle-cli/src/commands/clean.rs index 1e087423..19e099c9 100644 --- a/radicle-cli/src/commands/clean.rs +++ b/radicle-cli/src/commands/clean.rs @@ -80,7 +80,7 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { anyhow::bail!("repository {rid} was not found"); } - if !options.confirm || term::confirm(format!("Remove {rid}?")) { + if !options.confirm || term::confirm(format!("Clean {rid}?")) { let cleaned = storage.clean(rid)?; for remote in cleaned { term::info!("Removed {remote}"); diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs index f68989a3..829dfab9 100644 --- a/radicle-cli/tests/commands.rs +++ b/radicle-cli/tests/commands.rs @@ -642,6 +642,7 @@ fn rad_clean() { let mut environment = Environment::new(); let alice = environment.node(Config::test(Alias::new("alice"))); let bob = environment.node(Config::test(Alias::new("bob"))); + let eve = environment.node(Config::test(Alias::new("eve"))); let working = environment.tmp().join("working"); // Setup a test project. @@ -657,9 +658,12 @@ fn rad_clean() { let mut alice = alice.spawn(); let mut bob = bob.spawn(); + let mut eve = eve.spawn(); alice.handle.track_repo(acme, Scope::All).unwrap(); + eve.handle.track_repo(acme, Scope::Trusted).unwrap(); bob.connect(&alice).converge([&alice]); + eve.connect(&alice).converge([&alice]); test( "examples/rad-clone.md", @@ -669,16 +673,31 @@ fn rad_clean() { ) .unwrap(); + eve.handle.fetch(acme, alice.id, DEFAULT_TIMEOUT).unwrap(); + bob.has_inventory_of(&acme, &alice.id); alice.has_inventory_of(&acme, &bob.id); + eve.has_inventory_of(&acme, &alice.id); - test( - "examples/rad-clean.md", - working.join("acme"), - Some(&alice.home), - [], - ) - .unwrap(); + formula(&environment.tmp(), "examples/rad-clean.md") + .unwrap() + .home( + "alice", + working.join("acme"), + [("RAD_HOME", alice.home.path().display())], + ) + .home( + "bob", + working.join("bob"), + [("RAD_HOME", bob.home.path().display())], + ) + .home( + "eve", + working.join("eve"), + [("RAD_HOME", eve.home.path().display())], + ) + .run() + .unwrap(); } #[test] diff --git a/radicle/src/storage.rs b/radicle/src/storage.rs index afa36451..cb7e754f 100644 --- a/radicle/src/storage.rs +++ b/radicle/src/storage.rs @@ -349,6 +349,15 @@ pub trait WriteStorage: ReadStorage { /// Delete all remote namespaces apart from the local node's and /// delegates' namespace. + + /// Clean the repository found at `rid`. + /// + /// If the local peer has initialised `rad/sigrefs` by forking or + /// creating any COBs, then this will delete all remote namespaces + /// that are neither the local's or a delegate's. + /// + /// If the local peer has no initialised `rad/sigrefs`, then the + /// repository will be entirely removed from storage. fn clean(&self, rid: Id) -> Result, RepositoryError>; } diff --git a/radicle/src/storage/git.rs b/radicle/src/storage/git.rs index a6bea04c..f70d15c0 100644 --- a/radicle/src/storage/git.rs +++ b/radicle/src/storage/git.rs @@ -17,7 +17,7 @@ use crate::identity::doc::DocError; use crate::identity::{doc::DocAt, Doc, Id}; use crate::identity::{Identity, Project}; use crate::storage::refs; -use crate::storage::refs::{Refs, SignedRefs}; +use crate::storage::refs::{Refs, SignedRefs, SignedRefsAt}; use crate::storage::{ Inventory, ReadRepository, ReadStorage, Remote, Remotes, RepositoryError, SignRepository, WriteRepository, WriteStorage, @@ -141,7 +141,16 @@ impl WriteStorage for Storage { fn clean(&self, rid: Id) -> Result, RepositoryError> { let repo = self.repository(rid)?; - repo.clean(&self.info.key) + // N.b. we remove the repository if the `local` peer has no + // `rad/sigrefs`. There's no risk of them corrupting data. + let has_sigrefs = SignedRefsAt::load(self.info.key, &repo)?.is_some(); + if has_sigrefs { + repo.clean(&self.info.key) + } else { + let remotes = repo.remote_ids()?.collect::>()?; + repo.remove()?; + Ok(remotes) + } } } @@ -359,7 +368,7 @@ impl Repository { } /// Remove all the remotes of a repository that are not the - /// `local` remote or a delegate of the repository. + /// delegates of the repository or the local peer. /// /// N.b. failure to delete remotes or references will not result /// in an early exit. Instead, this method continues to delete the @@ -379,10 +388,12 @@ impl Repository { continue; } }; + // N.b. it is fatal to delete local or delegates if *local == id || delegates.contains(&id) { continue; } + let glob = git::refname!("refs/namespaces") .join(git::Component::from(&id)) .with_pattern(git::refspec::STAR);