cli: improve rad clone
This change improves the `rad clone` command in a few different ways. The first improvement is to check if the repository already exists before fetching. If it does exist, then there is no need for the fetch to be performed, and can move straight to the checkout. If the repository does not exist, then the fetch is performed, as per usual. However, it uses a replication factor of 1, so that the fetch finishes as soon as it has the repository in local storage. The change also improves the output when failures occur while attempting to perform the checkout phase. It will print the failure that occurred and provide a hint message in each case to help debug the failure, or push the caller in the direction of performing the correct action. This required some changes in `test/commands.rs` setup. Some tests were using the `rad-clone.md` to setup a clone, which would have different output if the repository did not need to be fetched. In those cases, the `rad` method for performing the clone is used instead.
This commit is contained in:
Fintan Halpenny
Erik Kundt
parent
059c804588
commit
cc96b9ed7d
|
|
@ -5,7 +5,7 @@ automatically connect to the necessary seeds.
|
|||
$ rad clone rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji
|
||||
✓ Seeding policy updated for rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji with scope 'all'
|
||||
Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from the network, found 2 potential seed(s).
|
||||
✓ Target met: 2 seed(s)
|
||||
✓ Target met: 1 seed(s)
|
||||
✓ Creating checkout in ./heartwood..
|
||||
✓ Remote alice@z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi added
|
||||
✓ Remote-tracking branch alice@z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/master created for z6MknSL…StBU8Vi
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ and is not empty, will fail:
|
|||
|
||||
``` (fail)
|
||||
$ rad clone rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --scope followed Developer/Radicle
|
||||
Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from the network, found 1 potential seed(s).
|
||||
✓ Target met: 1 seed(s)
|
||||
✗ Error: the directory path "Developer/Radicle" already exists
|
||||
✗ Error: refusing to checkout repository to Developer/Radicle, since it already exists
|
||||
✗ Hint: try `rad checkout rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji` in a new directory
|
||||
✗ Error: failed to clone rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji
|
||||
```
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ still returns successfully.
|
|||
$ rad clone rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --timeout 3
|
||||
✓ Seeding policy updated for rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji with scope 'all'
|
||||
Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from the network, found 3 potential seed(s).
|
||||
✗ Target not met: required 2 more seed(s)
|
||||
✓ Target met: 1 seed(s)
|
||||
✓ Creating checkout in ./heartwood..
|
||||
✓ Remote alice@z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi added
|
||||
✓ Remote-tracking branch alice@z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/master created for z6MknSL…StBU8Vi
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ To create a local copy of a repository on the radicle network, we use the
|
|||
$ rad clone rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --scope followed
|
||||
✓ Seeding policy updated for rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji with scope 'followed'
|
||||
Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from the network, found [..] potential seed(s).
|
||||
[..]
|
||||
✓ Target met: [..] seed(s)
|
||||
✓ Creating checkout in ./heartwood..
|
||||
✓ Remote alice@z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi added
|
||||
✓ Remote-tracking branch alice@z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/master created for z6MknSL…StBU8Vi
|
||||
|
|
|
|||
|
|
@ -172,7 +172,7 @@ sync` and fetch his references:
|
|||
$ rad clone rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji
|
||||
✓ Seeding policy updated for rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji with scope 'all'
|
||||
Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from the network, found 2 potential seed(s).
|
||||
✓ Target met: 2 seed(s)
|
||||
✓ Target met: 1 seed(s)
|
||||
✓ Creating checkout in ./heartwood..
|
||||
✓ Remote alice@z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi added
|
||||
✓ Remote-tracking branch alice@z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/master created for z6MknSL…StBU8Vi
|
||||
|
|
|
|||
|
|
@ -9,7 +9,9 @@ $ rad clone rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu --seed z6MknSLrJoTcukLrE435hVNQT4J
|
|||
✓ Seeding policy updated for rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu with scope 'all'
|
||||
Fetching rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu from the network, found 1 potential seed(s).
|
||||
✗ Target not met: could not fetch from [z6MknSL…StBU8Vi], and required 1 more seed(s)
|
||||
✗ Error: repository rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu not found
|
||||
! Warning: Failed to fetch from 1 seed(s).
|
||||
! Warning: z6MknSL…StBU8Vi: failed to perform fetch handshake
|
||||
✗ Error: no seeds found for rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu
|
||||
```
|
||||
|
||||
She allows Bob to view the repository. And when she syncs, one node (Bob) gets
|
||||
|
|
|
|||
|
|
@ -11,14 +11,15 @@ use thiserror::Error;
|
|||
|
||||
use radicle::git::raw;
|
||||
use radicle::identity::doc;
|
||||
use radicle::identity::doc::{DocError, RepoId};
|
||||
use radicle::identity::doc::RepoId;
|
||||
use radicle::node;
|
||||
use radicle::node::policy::Scope;
|
||||
use radicle::node::{Handle as _, Node};
|
||||
use radicle::prelude::*;
|
||||
use radicle::rad;
|
||||
use radicle::storage;
|
||||
use radicle::storage::RepositoryError;
|
||||
use radicle::storage::RemoteId;
|
||||
use radicle::storage::{HasRepoId, RepositoryError};
|
||||
|
||||
use crate::commands::rad_checkout as checkout;
|
||||
use crate::commands::rad_sync as sync;
|
||||
|
|
@ -82,8 +83,6 @@ impl Args for Options {
|
|||
let value = term::args::nid(&value)?;
|
||||
|
||||
sync.seeds.insert(value);
|
||||
let n = sync.seeds.len();
|
||||
sync.replicas = node::sync::ReplicationFactor::must_reach(n);
|
||||
}
|
||||
Long("scope") => {
|
||||
let value = parser.value()?;
|
||||
|
|
@ -142,14 +141,21 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
|
|||
);
|
||||
}
|
||||
|
||||
let (working, repo, doc, proj) = clone(
|
||||
let Success {
|
||||
working_copy: working,
|
||||
repository: repo,
|
||||
doc,
|
||||
project: proj,
|
||||
} = clone(
|
||||
options.id,
|
||||
options.directory.clone(),
|
||||
options.scope,
|
||||
options.sync.with_profile(&profile),
|
||||
&mut node,
|
||||
&profile,
|
||||
)?;
|
||||
)?
|
||||
.print_or_success()
|
||||
.ok_or_else(|| anyhow::anyhow!("failed to clone {}", options.id))?;
|
||||
let delegates = doc
|
||||
.delegates()
|
||||
.iter()
|
||||
|
|
@ -205,39 +211,101 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
|
|||
}
|
||||
|
||||
#[derive(Error, Debug)]
|
||||
pub enum CloneError {
|
||||
#[error("the directory path {path:?} already exists")]
|
||||
Exists { path: PathBuf },
|
||||
enum CloneError {
|
||||
#[error("node: {0}")]
|
||||
Node(#[from] node::Error),
|
||||
#[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(transparent)]
|
||||
Repository(#[from] RepositoryError),
|
||||
#[error("repository {0} not found")]
|
||||
NotFound(RepoId),
|
||||
#[error("no seeds found for {0}")]
|
||||
NoSeeds(RepoId),
|
||||
#[error("fetch: {0}")]
|
||||
Fetch(#[from] sync::FetchError),
|
||||
}
|
||||
|
||||
pub fn clone(
|
||||
struct Checkout {
|
||||
id: RepoId,
|
||||
remote: RemoteId,
|
||||
path: PathBuf,
|
||||
repository: storage::git::Repository,
|
||||
doc: Doc,
|
||||
project: Project,
|
||||
}
|
||||
|
||||
impl Checkout {
|
||||
fn new(
|
||||
repository: storage::git::Repository,
|
||||
profile: &Profile,
|
||||
directory: Option<PathBuf>,
|
||||
) -> Result<Self, CheckoutFailure> {
|
||||
let rid = repository.rid();
|
||||
let doc = repository
|
||||
.identity_doc()
|
||||
.map_err(|err| CheckoutFailure::Identity { rid, err })?;
|
||||
let proj = doc
|
||||
.project()
|
||||
.map_err(|err| CheckoutFailure::Payload { rid, err })?;
|
||||
let path = directory.unwrap_or(Path::new(proj.name()).to_path_buf());
|
||||
// N.b. fail if the path exists and is not empty
|
||||
if path.exists() {
|
||||
if path.read_dir().map_or(true, |mut dir| dir.next().is_some()) {
|
||||
return Err(CheckoutFailure::Exists { rid, path });
|
||||
}
|
||||
}
|
||||
|
||||
Ok(Self {
|
||||
id: rid,
|
||||
remote: *profile.id(),
|
||||
path,
|
||||
repository,
|
||||
doc: doc.doc,
|
||||
project: proj,
|
||||
})
|
||||
}
|
||||
|
||||
fn destination(&self) -> &PathBuf {
|
||||
&self.path
|
||||
}
|
||||
|
||||
fn run<S>(self, storage: &S) -> Result<CloneResult, rad::CheckoutError>
|
||||
where
|
||||
S: storage::ReadStorage,
|
||||
{
|
||||
let destination = self.destination().to_path_buf();
|
||||
// Checkout.
|
||||
let mut spinner = term::spinner(format!(
|
||||
"Creating checkout in ./{}..",
|
||||
term::format::tertiary(destination.display())
|
||||
));
|
||||
match rad::checkout(self.id, &self.remote, self.path, storage) {
|
||||
Err(err) => {
|
||||
spinner.message(format!(
|
||||
"Failed to checkout in ./{}",
|
||||
term::format::tertiary(destination.display())
|
||||
));
|
||||
spinner.failed();
|
||||
Err(err)
|
||||
}
|
||||
Ok(working_copy) => {
|
||||
spinner.finish();
|
||||
Ok(CloneResult::Success(Success {
|
||||
working_copy,
|
||||
repository: self.repository,
|
||||
doc: self.doc,
|
||||
project: self.project,
|
||||
}))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn clone(
|
||||
id: RepoId,
|
||||
directory: Option<PathBuf>,
|
||||
scope: Scope,
|
||||
settings: SyncSettings,
|
||||
node: &mut Node,
|
||||
profile: &Profile,
|
||||
) -> Result<(raw::Repository, storage::git::Repository, Doc, Project), CloneError> {
|
||||
let me = profile.id();
|
||||
|
||||
) -> Result<CloneResult, CloneError> {
|
||||
// Seed repository.
|
||||
if node.seed(id, scope)? {
|
||||
term::success!(
|
||||
|
|
@ -246,49 +314,121 @@ pub fn clone(
|
|||
);
|
||||
}
|
||||
|
||||
let result = sync::fetch(id, settings, node, profile)?;
|
||||
// FIXME: handle the two cases
|
||||
let fetch_results = match &result {
|
||||
node::sync::FetcherResult::TargetReached(success) => success.fetch_results(),
|
||||
node::sync::FetcherResult::TargetError(failed) => failed.fetch_results(),
|
||||
};
|
||||
let Ok(repository) = profile.storage.repository(id) else {
|
||||
// If we don't have the repository locally, even after attempting to fetch,
|
||||
// there's nothing we can do.
|
||||
if fetch_results.is_empty() {
|
||||
return Err(CloneError::NoSeeds(id));
|
||||
} else {
|
||||
return Err(CloneError::NotFound(id));
|
||||
}
|
||||
};
|
||||
|
||||
let doc = repository.identity_doc()?;
|
||||
let proj = doc.project()?;
|
||||
let path = directory.unwrap_or(Path::new(proj.name()).to_path_buf());
|
||||
|
||||
// N.b. fail if the path exists and is not empty
|
||||
if path.exists() {
|
||||
if path.read_dir().map_or(true, |mut dir| dir.next().is_some()) {
|
||||
return Err(CloneError::Exists { path });
|
||||
match profile.storage.repository(id) {
|
||||
Err(_) => {
|
||||
// N.b. We only need to reach 1 replica in order for a clone to be
|
||||
// considered successful.
|
||||
let settings = settings.replicas(node::sync::ReplicationFactor::must_reach(1));
|
||||
let result = sync::fetch(id, settings, node, profile)?;
|
||||
match &result {
|
||||
node::sync::FetcherResult::TargetReached(_) => {
|
||||
profile.storage.repository(id).map_or_else(
|
||||
|err| Ok(CloneResult::RepositoryMissing { rid: id, err }),
|
||||
|repository| Ok(perform_checkout(repository, profile, directory)?),
|
||||
)
|
||||
}
|
||||
node::sync::FetcherResult::TargetError(failure) => {
|
||||
Err(handle_fetch_error(id, failure))
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(repository) => Ok(perform_checkout(repository, profile, directory)?),
|
||||
}
|
||||
}
|
||||
|
||||
fn perform_checkout(
|
||||
repository: storage::git::Repository,
|
||||
profile: &Profile,
|
||||
directory: Option<PathBuf>,
|
||||
) -> Result<CloneResult, rad::CheckoutError> {
|
||||
Checkout::new(repository, profile, directory).map_or_else(
|
||||
|failure| Ok(CloneResult::Failure(failure)),
|
||||
|checkout| checkout.run(&profile.storage),
|
||||
)
|
||||
}
|
||||
|
||||
fn handle_fetch_error(id: RepoId, failure: &node::sync::fetch::TargetMissed) -> CloneError {
|
||||
term::warning(format!(
|
||||
"Failed to fetch from {} seed(s).",
|
||||
failure.progress().failed()
|
||||
));
|
||||
for (node, reason) in failure.fetch_results().failed() {
|
||||
term::warning(format!(
|
||||
"{}: {}",
|
||||
term::format::node(node),
|
||||
term::format::yellow(reason),
|
||||
))
|
||||
}
|
||||
CloneError::NoSeeds(id)
|
||||
}
|
||||
|
||||
enum CloneResult {
|
||||
Success(Success),
|
||||
RepositoryMissing { rid: RepoId, err: RepositoryError },
|
||||
Failure(CheckoutFailure),
|
||||
}
|
||||
|
||||
struct Success {
|
||||
working_copy: raw::Repository,
|
||||
repository: storage::git::Repository,
|
||||
doc: Doc,
|
||||
project: Project,
|
||||
}
|
||||
|
||||
impl CloneResult {
|
||||
fn print_or_success(self) -> Option<Success> {
|
||||
match self {
|
||||
CloneResult::Success(success) => Some(success),
|
||||
CloneResult::RepositoryMissing { rid, err } => {
|
||||
term::error(format!(
|
||||
"failed to find repository in storage after fetching: {err}"
|
||||
));
|
||||
term::hint(format!(
|
||||
"try `rad inspect {rid}` to see if the repository exists"
|
||||
));
|
||||
None
|
||||
}
|
||||
CloneResult::Failure(failure) => {
|
||||
failure.print();
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub enum CheckoutFailure {
|
||||
Identity { rid: RepoId, err: RepositoryError },
|
||||
Payload { rid: RepoId, err: doc::PayloadError },
|
||||
Exists { rid: RepoId, path: PathBuf },
|
||||
}
|
||||
|
||||
impl CheckoutFailure {
|
||||
fn print(&self) {
|
||||
match self {
|
||||
CheckoutFailure::Identity { rid, err } => {
|
||||
term::error(format!(
|
||||
"failed to get the identity document of {rid} after fetching: {err}"
|
||||
));
|
||||
term::hint(format!(
|
||||
"try `rad inspect {rid} --identity`, if this works then try `rad checkout {rid}`"
|
||||
));
|
||||
}
|
||||
CheckoutFailure::Payload { rid, err } => {
|
||||
term::error(format!(
|
||||
"failed to get the project payload of {rid} after fetching: {err}"
|
||||
));
|
||||
term::hint(format!(
|
||||
"try `rad inspect {rid} --payload`, if this works then try `rad checkout {rid}`"
|
||||
));
|
||||
}
|
||||
CheckoutFailure::Exists { rid, path } => {
|
||||
term::error(format!(
|
||||
"refusing to checkout repository to {}, since it already exists",
|
||||
path.display()
|
||||
));
|
||||
term::hint(format!("try `rad checkout {rid}` in a new directory"))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if fetch_results.success().next().is_none() {
|
||||
if fetch_results.failed().next().is_some() {
|
||||
term::warning("Fetching failed, local copy is potentially stale");
|
||||
} else {
|
||||
term::warning("No seeds found, local copy is potentially stale");
|
||||
}
|
||||
}
|
||||
|
||||
// Checkout.
|
||||
let spinner = term::spinner(format!(
|
||||
"Creating checkout in ./{}..",
|
||||
term::format::tertiary(path.display())
|
||||
));
|
||||
let working = rad::checkout(id, me, path, &profile.storage)?;
|
||||
|
||||
spinner.finish();
|
||||
|
||||
Ok((working, repository, doc.into(), proj))
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1091,13 +1091,8 @@ fn rad_patch_checkout_force() {
|
|||
bob.handle.seed(acme, Scope::All).unwrap();
|
||||
alice.connect(&bob).converge([&bob]);
|
||||
|
||||
test(
|
||||
"examples/rad-clone.md",
|
||||
working.join("bob"),
|
||||
Some(&bob.home),
|
||||
[],
|
||||
)
|
||||
.unwrap();
|
||||
bob.rad("clone", &[acme.to_string().as_str()], working.join("bob"))
|
||||
.unwrap();
|
||||
|
||||
formula(&environment.tmp(), "examples/rad-patch-checkout-force.md")
|
||||
.unwrap()
|
||||
|
|
@ -1332,13 +1327,8 @@ fn rad_patch_delete() {
|
|||
bob.connect(&seed).converge([&seed]);
|
||||
bob.routes_to(&[(acme, seed.id)]);
|
||||
|
||||
test(
|
||||
"examples/rad-clone.md",
|
||||
working.join("bob"),
|
||||
Some(&bob.home),
|
||||
[],
|
||||
)
|
||||
.unwrap();
|
||||
bob.rad("clone", &[acme.to_string().as_str()], working.join("bob"))
|
||||
.unwrap();
|
||||
|
||||
formula(&environment.tmp(), "examples/rad-patch-delete.md")
|
||||
.unwrap()
|
||||
|
|
@ -1991,10 +1981,6 @@ fn rad_diff() {
|
|||
test("examples/rad-diff.md", working, None, []).unwrap();
|
||||
}
|
||||
|
||||
// FIXME(fintohaps): I plan on fixing this logic in the next patch – clone
|
||||
// should not need to fetch from the network if the repository is already
|
||||
// available locally.
|
||||
#[ignore]
|
||||
#[test]
|
||||
// User tries to clone; no seeds are available, but user has the repo locally.
|
||||
fn test_clone_without_seeds() {
|
||||
|
|
|
|||
Loading…
Reference in New Issue