From 30e9d6103c40a65b4b0ecc20b322c1e40484c956 Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Wed, 25 Jan 2023 14:26:41 +0100 Subject: [PATCH] Cleanup `clone` code Remove a bunch of different instances of it. --- radicle-cli/src/commands/checkout.rs | 2 +- radicle-cli/src/commands/clone.rs | 128 +++++++++++++++++++++------ radicle-node/src/tests/e2e.rs | 19 +++- radicle-tools/Cargo.toml | 4 - radicle-tools/src/rad-clone.rs | 25 ------ radicle/src/node.rs | 2 +- radicle/src/rad.rs | 60 ------------- 7 files changed, 120 insertions(+), 120 deletions(-) delete mode 100644 radicle-tools/src/rad-clone.rs diff --git a/radicle-cli/src/commands/checkout.rs b/radicle-cli/src/commands/checkout.rs index fb1ca25a..c59fac8f 100644 --- a/radicle-cli/src/commands/checkout.rs +++ b/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)) ); } diff --git a/radicle-cli/src/commands/clone.rs b/radicle-cli/src/commands/clone.rs index 6ec370db..6198fee9 100644 --- a/radicle-cli/src/commands/clone.rs +++ b/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()); - - // 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 (working, doc, proj) = clone(options.id, &signer, &profile.storage, &mut node)?; 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::>(); 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 { + #[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( + id: Id, + signer: &G, + storage: &Storage, + node: &mut H, +) -> Result<(raw::Repository, Doc, Project), CloneError> { + 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)) +} diff --git a/radicle-node/src/tests/e2e.rs b/radicle-node/src/tests/e2e.rs index b7e69aac..f5d4f655 100644 --- a/radicle-node/src/tests/e2e.rs +++ b/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 diff --git a/radicle-tools/Cargo.toml b/radicle-tools/Cargo.toml index a9f16221..c6b8fc61 100644 --- a/radicle-tools/Cargo.toml +++ b/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" diff --git a/radicle-tools/src/rad-clone.rs b/radicle-tools/src/rad-clone.rs deleted file mode 100644 index 428cfe72..00000000 --- a/radicle-tools/src/rad-clone.rs +++ /dev/null @@ -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(()) -} diff --git a/radicle/src/node.rs b/radicle/src/node.rs index 8a4c1f83..93398bdb 100644 --- a/radicle/src/node.rs +++ b/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; diff --git a/radicle/src/rad.rs b/radicle/src/rad.rs index efd16d64..3653c5b9 100644 --- a/radicle/src/rad.rs +++ b/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( Ok(()) } -#[derive(Error, Debug)] -pub enum CloneError { - #[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, G: Signer, H: node::Handle>( - proj: Id, - path: P, - signer: &G, - storage: &Storage, - handle: &mut H, -) -> Result> { - 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")]