Cleanup `clone` code

Remove a bunch of different instances of it.

radicle-cli/src/commands/checkout.rs

@@ -158,7 +158,7 @@ pub fn setup_remotes(setup: project::SetupRemote, remotes: &[NodeId]) -> anyhow:
             term::success!("Remote {} created", term::format::tertiary(remote));
             term::success!(
                 "Remote-tracking branch {} created for {}",
-                term::format::highlight(branch),
+                term::format::tertiary(branch),
                 term::format::tertiary(term::format::node(remote_id))
             );
         }

radicle-cli/src/commands/clone.rs

@@ -4,11 +4,17 @@ use std::path::Path;
 use std::str::FromStr;
 
 use anyhow::anyhow;
-use anyhow::Context as _;
+use thiserror::Error;
 
-use radicle::node::Handle;
+use radicle::git::raw;
+use radicle::identity::doc;
+use radicle::identity::doc::{DocError, Id};
+use radicle::node;
+use radicle::node::{FetchLookup, Handle};
 use radicle::prelude::*;
 use radicle::rad;
+use radicle::storage;
+use radicle::storage::git::{ProjectError, Storage};
 use radicle::storage::WriteStorage;
 
 use crate::commands::rad_checkout as checkout;
@@ -37,6 +43,7 @@ Options
 #[derive(Debug)]
 pub struct Options {
     id: Id,
+    #[allow(dead_code)]
     interactive: Interactive,
 }
 
@@ -75,30 +82,10 @@ impl Args for Options {
 }
 
 pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
-    clone(options.id, options.interactive, ctx)
-}
-
-pub fn clone(id: Id, _interactive: Interactive, ctx: impl term::Context) -> anyhow::Result<()> {
     let profile = ctx.profile()?;
     let signer = term::signer(&profile)?;
     let mut node = radicle::Node::new(profile.socket());
-
+    let (working, doc, proj) = clone(options.id, &signer, &profile.storage, &mut node)?;
-    // Track & fetch project.
-    node.track_repo(id).context("track")?;
-    node.fetch(id).context("fetch")?; // FIXME: Handle output
-
-    // Create a local fork of the project, under our own id.
-    rad::fork(id, &signer, &profile.storage).context("fork error")?;
-
-    let doc = profile
-        .storage
-        .repository(id)?
-        .identity_of(profile.id())
-        .map_err(|_e| anyhow!("couldn't load project {} from local state", id))?;
-    let proj = doc.project()?;
-
-    let path = Path::new(proj.name());
-    let repo = rad::checkout(id, profile.id(), path, &profile.storage)?;
     let delegates = doc
         .delegates
         .iter()
@@ -106,13 +93,14 @@ pub fn clone(id: Id, _interactive: Interactive, ctx: impl term::Context) -> anyh
         .filter(|id| id != profile.id())
         .collect::<Vec<_>>();
     let default_branch = proj.default_branch().clone();
+    let path = working.workdir().unwrap(); // SAFETY: The working copy is not bare.
 
     // Setup tracking for project delegates.
     checkout::setup_remotes(
         project::SetupRemote {
-            project: id,
+            project: options.id,
             default_branch,
-            repo: &repo,
+            repo: &working,
             fetch: true,
             tracking: true,
         },
@@ -126,3 +114,93 @@ pub fn clone(id: Id, _interactive: Interactive, ctx: impl term::Context) -> anyh
 
     Ok(())
 }
+
+#[derive(Error, Debug)]
+pub enum CloneError<H: node::Handle> {
+    #[error("node: {0}")]
+    Node(#[from] node::Error),
+    #[error("fetch: {0}")]
+    Fetch(#[from] node::FetchError),
+    #[error("fork: {0}")]
+    Fork(#[from] rad::ForkError),
+    #[error("storage: {0}")]
+    Storage(#[from] storage::Error),
+    #[error("checkout: {0}")]
+    Checkout(#[from] rad::CheckoutError),
+    #[error("identity document error: {0}")]
+    Doc(#[from] DocError),
+    #[error("payload: {0}")]
+    Payload(#[from] doc::PayloadError),
+    #[error("project error: {0}")]
+    Project(#[from] ProjectError),
+    #[error("handle error: {0}")]
+    Handle(H::Error),
+}
+
+pub fn clone<G: Signer, H: Handle>(
+    id: Id,
+    signer: &G,
+    storage: &Storage,
+    node: &mut H,
+) -> Result<(raw::Repository, Doc<Verified>, Project), CloneError<H>> {
+    let me = *signer.public_key();
+
+    // Track & fetch project.
+    if node.track_repo(id).map_err(CloneError::Handle)? {
+        term::success!(
+            "Tracking relationship restablished for {}",
+            term::format::tertiary(id)
+        );
+    }
+
+    let spinner = term::spinner(format!("Fetching {}..", term::format::tertiary(id)));
+    match node.fetch(id).map_err(CloneError::Handle)? {
+        FetchLookup::Found { seeds, results } => {
+            // TODO: If none of them succeeds, output an error. Otherwise tell the caller
+            // how many succeeded.
+            for result in results.iter().take(seeds.len()) {
+                match &*result {
+                    Ok(_updates) => {}
+                    Err(_err) => {}
+                }
+            }
+        }
+        FetchLookup::NotFound => {
+            // TODO: Return error.
+        }
+        FetchLookup::NotTracking => {
+            // SAFETY: Since we track it above, this shouldn't trigger unless there's a bug.
+            panic!("clone: Repository is not tracked");
+        }
+        FetchLookup::Error(err) => {
+            return Err(err.into());
+        }
+    }
+    spinner.finish();
+
+    // Create a local fork of the project, under our own id.
+    {
+        let spinner = term::spinner(format!(
+            "Forking under {}..",
+            term::format::tertiary(term::format::node(&me))
+        ));
+        rad::fork(id, signer, &storage)?;
+
+        spinner.finish();
+    }
+
+    let doc = storage.repository(id)?.identity_of(&me)?;
+    let proj = doc.project()?;
+    let path = Path::new(proj.name());
+
+    // Checkout.
+    let spinner = term::spinner(format!(
+        "Creating checkout in ./{}..",
+        term::format::tertiary(path.display())
+    ));
+    let repo = rad::checkout(id, &me, path, &storage)?;
+
+    spinner.finish();
+
+    Ok((repo, doc, proj))
+}

radicle-node/src/tests/e2e.rs

@@ -394,19 +394,30 @@ fn test_clone() {
 
     transport::local::register(alice.storage.clone());
 
-    let repo = rad::clone(
+    let _ = alice.handle.track_repo(acme).unwrap();
+    let lookup = alice.handle.fetch(acme).unwrap();
+
+    match lookup {
+        // Drain the channel.
+        FetchLookup::Found { seeds, results } => for _ in results.iter().take(seeds.len()) {},
+        other => {
+            panic!("Unexpected fetch lookup: {:?}", other);
+        }
+    }
+    rad::fork(acme, &alice.signer, &alice.storage).unwrap();
+
+    let working = rad::checkout(
         acme,
+        alice.signer.public_key(),
         tmp.path().join("clone"),
-        &alice.signer,
         &alice.storage,
-        &mut (*alice.handle),
     )
     .unwrap();
 
     // Makes test finish faster.
     drop(alice);
 
-    let head = repo.head().unwrap();
+    let head = working.head().unwrap();
     let oid = head.target().unwrap();
 
     let (_, canonical) = bob

radicle-tools/Cargo.toml

@@ -32,7 +32,3 @@ path = "src/rad-push.rs"
 [[bin]]
 name = "rad-agent"
 path = "src/rad-agent.rs"
-
-[[bin]]
-name = "rad-clone"
-path = "src/rad-clone.rs"

radicle-tools/src/rad-clone.rs

@@ -1,25 +0,0 @@
-use std::env;
-use std::path::Path;
-
-use radicle::identity::Id;
-
-fn main() -> anyhow::Result<()> {
-    let cwd = Path::new(".").canonicalize()?;
-    let profile = radicle::Profile::load()?;
-    let signer = profile.signer()?;
-
-    if let Some(id) = env::args().nth(1) {
-        let id = Id::from_urn(&id)?;
-        let mut node = radicle::Node::new(profile.socket());
-        let repo = radicle::rad::clone(id, &cwd, &signer, &profile.storage, &mut node)?;
-
-        println!(
-            "ok: project {id} cloned into `{}`",
-            repo.workdir().unwrap().display()
-        );
-    } else {
-        anyhow::bail!("Error: a project id must be specified");
-    }
-
-    Ok(())
-}

radicle/src/node.rs

@@ -120,7 +120,7 @@ pub trait Handle {
     /// The peer sessions type.
     type Sessions;
     /// The error returned by all methods.
-    type Error: std::error::Error;
+    type Error: std::error::Error + Send + Sync + 'static;
 
     /// Check if the node is running. to a peer.
     fn is_running(&self) -> bool;

radicle/src/rad.rs

@@ -11,7 +11,6 @@ use crate::git;
 use crate::identity::doc;
 use crate::identity::doc::{DocError, Id};
 use crate::identity::project::Project;
-use crate::node::{self, FetchLookup};
 use crate::storage::git::transport::{self, remote};
 use crate::storage::git::{ProjectError, Repository, Storage};
 use crate::storage::refs::SignedRefs;
@@ -186,65 +185,6 @@ pub fn fork<G: Signer, S: storage::WriteStorage>(
     Ok(())
 }
 
-#[derive(Error, Debug)]
-pub enum CloneError<H: node::Handle> {
-    #[error("node: {0}")]
-    Node(#[from] node::Error),
-    #[error("fetch: {0}")]
-    Fetch(#[from] node::FetchError),
-    #[error("fork: {0}")]
-    Fork(#[from] ForkError),
-    #[error("checkout: {0}")]
-    Checkout(#[from] CheckoutError),
-    #[error("identity document error: {0}")]
-    Doc(#[from] DocError),
-    #[error("handle error: {0}")]
-    Handle(H::Error),
-}
-
-pub fn clone<P: AsRef<Path>, G: Signer, H: node::Handle>(
-    proj: Id,
-    path: P,
-    signer: &G,
-    storage: &Storage,
-    handle: &mut H,
-) -> Result<git2::Repository, CloneError<H>> {
-    let _ = handle.track_repo(proj).map_err(CloneError::Handle)?;
-    let lookup = handle.fetch(proj).map_err(CloneError::Handle)?;
-
-    match lookup {
-        FetchLookup::Found { seeds, results } => {
-            // TODO: If none of them succeeds, output an error. Otherwise tell the caller
-            // how many succeeded.
-            for result in results.iter().take(seeds.len()) {
-                match &*result {
-                    Ok(_updates) => {}
-                    Err(_err) => {}
-                }
-            }
-        }
-        FetchLookup::NotFound => {
-            // TODO: Return error instead.
-            panic!("clone: Repository not found in routing table");
-        }
-        FetchLookup::NotTracking => {
-            // SAFETY: Since we track it above, this shouldn't trigger unless there's a bug.
-            panic!("clone: Repository is not tracked");
-        }
-        FetchLookup::Error(err) => {
-            return Err(err.into());
-        }
-    }
-
-    log::debug!("Creating fork in local storage..");
-    let _ = fork(proj, signer, storage)?;
-
-    log::debug!("Creating checkout at {}..", path.as_ref().display());
-    let working = checkout(proj, signer.public_key(), path, storage)?;
-
-    Ok(working)
-}
-
 #[derive(Error, Debug)]
 pub enum CloneUrlError {
     #[error("missing namespace in url")]