From 3898fece082e0088c8f8fcb385617265f196125a Mon Sep 17 00:00:00 2001 From: cloudhead Date: Thu, 3 Aug 2023 13:15:04 +0200 Subject: [PATCH] cli: `git pull` to update patches in working copy We enable users to pull updates to a patch by calling `git pull` on the patch branch. --- radicle-cli/examples/rad-patch-pull-update.md | 117 ++++++++++++++++++ radicle-cli/examples/rad-patch-via-push.md | 2 +- .../workflow/6-pulling-contributor.md | 1 + radicle-cli/src/commands/patch/checkout.rs | 23 +--- radicle-cli/tests/commands.rs | 33 +++++ radicle-cob/src/object.rs | 11 +- radicle-remote-helper/src/fetch.rs | 16 ++- radicle-remote-helper/src/lib.rs | 17 ++- radicle-remote-helper/src/list.rs | 28 ++++- radicle-remote-helper/src/push.rs | 4 +- radicle/src/git.rs | 5 +- radicle/src/node.rs | 15 ++- 12 files changed, 232 insertions(+), 40 deletions(-) create mode 100644 radicle-cli/examples/rad-patch-pull-update.md diff --git a/radicle-cli/examples/rad-patch-pull-update.md b/radicle-cli/examples/rad-patch-pull-update.md new file mode 100644 index 00000000..dcdc81a1 --- /dev/null +++ b/radicle-cli/examples/rad-patch-pull-update.md @@ -0,0 +1,117 @@ +Let's look at how patch updates work. + +Alice creates a project and Bob clones it. + +``` ~alice +$ rad init --name heartwood --description "radicle heartwood protocol & stack" --no-confirm --announce + +Initializing radicle 👾 project in . + +✓ Project heartwood created +✓ Syncing inventory.. +✓ Announcing inventory.. + +Your project's Repository ID (RID) is rad:zhbMU4DUXrzB8xT6qAJh6yZ7bFMK. +You can show it any time by running `rad .` +``` + +``` ~bob +$ rad clone rad:zhbMU4DUXrzB8xT6qAJh6yZ7bFMK +✓ Tracking relationship established for rad:zhbMU4DUXrzB8xT6qAJh6yZ7bFMK with scope 'all' +✓ Fetching rad:zhbMU4DUXrzB8xT6qAJh6yZ7bFMK from z6MknSL…StBU8Vi.. +✓ Forking under z6Mkt67…v4N1tRk.. +✓ Creating checkout in ./heartwood.. +✓ Remote alice@z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi added +✓ Remote-tracking branch alice@z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/master created for z6MknSL…StBU8Vi +✓ Repository successfully cloned under [..]/heartwood/ +``` + +We wait for Alice to sync our fork. + +``` ~bob +$ rad node events -n 1 --timeout 1 +{"type":"refsSynced","remote":"z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi","rid":"rad:zhbMU4DUXrzB8xT6qAJh6yZ7bFMK"} +``` + +Bob then opens a patch. + +``` ~bob (stderr) +$ cd heartwood +$ git checkout -b bob/feature -q +$ git commit --allow-empty -m "Bob's commit #1" -q +$ git push rad -o sync -o patch.message="Bob's patch" HEAD:refs/patches +✓ Patch 627477fdb46b9aaf3f0677c415b569cd21227b76 opened +✓ Synced with 1 node(s) +To rad://zhbMU4DUXrzB8xT6qAJh6yZ7bFMK/z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk + * [new reference] HEAD -> refs/patches +``` +``` ~bob +$ git status --short --branch +## bob/feature...rad/patches/627477fdb46b9aaf3f0677c415b569cd21227b76 +``` + +Alice checks it out. + +``` ~alice +$ rad patch checkout 627477f +✓ Switched to branch patch/627477f +✓ Branch patch/627477f setup to track rad/patches/627477fdb46b9aaf3f0677c415b569cd21227b76 +$ git show +commit bdcdb30b3c0f513620dd0f1c24ff8f4f71de956b +Author: radicle +Date: Thu Dec 15 17:28:04 2022 +0000 + + Bob's commit #1 +``` + +Bob then updates the patch. + +``` ~bob (stderr) +$ git commit --allow-empty -m "Bob's commit #2" -q +$ git push rad -o sync -o patch.message="Updated." +✓ Patch 627477f updated to c4114446af35501300c68571cfb07a6f5c7e1eef +✓ Synced with 1 node(s) +To rad://zhbMU4DUXrzB8xT6qAJh6yZ7bFMK/z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk + bdcdb30..cad2666 bob/feature -> patches/627477fdb46b9aaf3f0677c415b569cd21227b76 +``` + +Alice pulls the update. + +``` ~alice +$ rad patch show 627477f +╭──────────────────────────────────────────────────────────────────────────────╮ +│ Title Bob's patch │ +│ Patch 627477fdb46b9aaf3f0677c415b569cd21227b76 │ +│ Author did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk │ +│ Head cad2666a8a2250e4dee175ed5044be2c251ff08b │ +│ Commits ahead 2, behind 0 │ +│ Status open │ +├──────────────────────────────────────────────────────────────────────────────┤ +│ cad2666 Bob's commit #2 │ +│ bdcdb30 Bob's commit #1 │ +├──────────────────────────────────────────────────────────────────────────────┤ +│ ● opened by bob (z6Mkt67…v4N1tRk) [ ... ] │ +│ ↑ updated to c4114446af35501300c68571cfb07a6f5c7e1eef (cad2666) [ ... ] │ +╰──────────────────────────────────────────────────────────────────────────────╯ +$ git ls-remote rad +f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354 refs/heads/master +cad2666a8a2250e4dee175ed5044be2c251ff08b refs/heads/patches/627477fdb46b9aaf3f0677c415b569cd21227b76 +``` +``` ~alice +$ git fetch rad +$ git status --short --branch +## patch/627477f...rad/patches/627477fdb46b9aaf3f0677c415b569cd21227b76 [behind 1] +``` +``` ~alice +$ git pull +Updating bdcdb30..cad2666 +Fast-forward +``` +``` ~alice +$ git show +commit cad2666a8a2250e4dee175ed5044be2c251ff08b +Author: radicle +Date: Thu Dec 15 17:28:04 2022 +0000 + + Bob's commit #2 +``` diff --git a/radicle-cli/examples/rad-patch-via-push.md b/radicle-cli/examples/rad-patch-via-push.md index f36519c6..241686eb 100644 --- a/radicle-cli/examples/rad-patch-via-push.md +++ b/radicle-cli/examples/rad-patch-via-push.md @@ -62,7 +62,7 @@ f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354 refs/remotes/rad/master 42d894a83c9c356552a57af09ccdbd5587a99045 refs/remotes/rad/patches/2647168c23e7c2b2c1936d695443944e143bc3f7 ``` ``` -$ git ls-remote rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 'refs/heads/patches/*' +$ git ls-remote rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji 'refs/heads/patches/*' 42d894a83c9c356552a57af09ccdbd5587a99045 refs/heads/patches/2647168c23e7c2b2c1936d695443944e143bc3f7 $ git ls-remote rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 'refs/cobs/*' 2647168c23e7c2b2c1936d695443944e143bc3f7 refs/cobs/xyz.radicle.patch/2647168c23e7c2b2c1936d695443944e143bc3f7 diff --git a/radicle-cli/examples/workflow/6-pulling-contributor.md b/radicle-cli/examples/workflow/6-pulling-contributor.md index 486e2e20..e60b07a9 100644 --- a/radicle-cli/examples/workflow/6-pulling-contributor.md +++ b/radicle-cli/examples/workflow/6-pulling-contributor.md @@ -23,6 +23,7 @@ Your branch is up to date with 'rad/master'. $ git pull --all --ff From rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji f2de534..f567f69 master -> rad/master + 27857ec..f567f69 patches/50e29a111972f3b7d2123c5057de5bdf09bc7b1c -> rad/patches/50e29a111972f3b7d2123c5057de5bdf09bc7b1c From rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi f2de534..f567f69 master -> alice@z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/master ``` diff --git a/radicle-cli/src/commands/patch/checkout.rs b/radicle-cli/src/commands/patch/checkout.rs index 68ac2ecc..14042b02 100644 --- a/radicle-cli/src/commands/patch/checkout.rs +++ b/radicle-cli/src/commands/patch/checkout.rs @@ -23,7 +23,7 @@ pub fn run( let patch_branch = // SAFETY: Patch IDs are valid refstrings. git::refname!("patch").join(RefString::try_from(term::format::cob(patch_id)).unwrap()); - let commit = find_patch_commit(&patch, &patch_branch, stored, working)?; + let commit = find_patch_commit(&patch, stored, working)?; // Create patch branch and switch to it. working.branch(patch_branch.as_str(), &commit, true)?; @@ -53,7 +53,6 @@ pub fn run( /// fetch it from storage first. fn find_patch_commit<'a>( patch: &Patch, - patch_branch: &RefString, stored: &Repository, working: &'a git::raw::Repository, ) -> anyhow::Result> { @@ -62,26 +61,10 @@ fn find_patch_commit<'a>( match working.find_commit(patch_head.into()) { Ok(commit) => Ok(commit), Err(e) if git::ext::is_not_found_err(&e) => { - let (_, rev) = patch.latest(); - let author = *rev.author().id(); - let remote = stored.remote(&author)?; + let url = git::url::File::new(stored.path()); - // Find a ref in storage that points to our patch, so that we can fetch the patch - // objects into our working copy. - let (refstr, _) = remote - .refs - .iter() - .find(|(_, o)| **o == patch_head) - .ok_or(anyhow!("patch ref for {patch_head} not found in storage"))?; - let remote_branch = git::refs::workdir::remote_branch( - &RefString::try_from(author.as_key().to_human())?, - patch_branch, - ); - let url = git::Url::from(stored.id).with_namespace(*author); - - // Fetch only the ref pointing to the patch revision. working.remote_anonymous(url.to_string().as_str())?.fetch( - &[&format!("{refstr}:{remote_branch}")], + &[patch_head.to_string()], None, None, )?; diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs index 8882ba0e..540e74b3 100644 --- a/radicle-cli/tests/commands.rs +++ b/radicle-cli/tests/commands.rs @@ -1000,6 +1000,39 @@ fn rad_merge_after_update() { .unwrap(); } +#[test] +fn rad_patch_pull_update() { + logger::init(log::Level::Debug); + + let mut environment = Environment::new(); + let alice = environment.node(Config::new(Alias::new("alice"))); + let bob = environment.node(Config::new(Alias::new("bob"))); + let working = environment.tmp().join("working"); + + fixtures::repository(working.join("alice")); + + let alice = alice.spawn(); + let mut bob = bob.spawn(); + + bob.connect(&alice).converge([&alice]); + + formula(&environment.tmp(), "examples/rad-patch-pull-update.md") + .unwrap() + .home( + "alice", + working.join("alice"), + [("RAD_HOME", alice.home.path().display())], + ) + .home( + "bob", + bob.home.path(), + [("RAD_HOME", bob.home.path().display())], + ) + .run() + .unwrap(); +} + +#[test] fn framework_home() { logger::init(log::Level::Debug); diff --git a/radicle-cob/src/object.rs b/radicle-cob/src/object.rs index da03f038..3220b8ae 100644 --- a/radicle-cob/src/object.rs +++ b/radicle-cob/src/object.rs @@ -95,9 +95,16 @@ impl<'de> Deserialize<'de> for ObjectId { impl From<&ObjectId> for Component<'_> { fn from(id: &ObjectId) -> Self { - let refstr = RefString::try_from(id.0.to_string()) - .expect("collaborative object id's are valid ref strings"); + let refstr = RefString::from(*id); + Component::from_refstr(refstr) .expect("collaborative object id's are valid refname components") } } + +impl From for RefString { + fn from(id: ObjectId) -> Self { + RefString::try_from(id.0.to_string()) + .expect("collaborative object id's are valid ref strings") + } +} diff --git a/radicle-remote-helper/src/fetch.rs b/radicle-remote-helper/src/fetch.rs index 72c31292..92493961 100644 --- a/radicle-remote-helper/src/fetch.rs +++ b/radicle-remote-helper/src/fetch.rs @@ -1,5 +1,6 @@ use std::io; use std::path::Path; +use std::str::FromStr; use thiserror::Error; @@ -27,7 +28,7 @@ pub enum Error { /// Run a git fetch command. pub fn run( - mut refs: Vec, + mut refs: Vec<(git::Oid, git::RefString)>, working: &Path, url: Url, stored: R, @@ -38,8 +39,11 @@ pub fn run( loop { let tokens = read_line(stdin, &mut line)?; match tokens.as_slice() { - ["fetch", _oid, refstr] => { - refs.push(refstr.to_string()); + ["fetch", oid, refstr] => { + let oid = git::Oid::from_str(oid)?; + let refstr = git::RefString::try_from(*refstr)?; + + refs.push((oid, refstr)); } // An empty line means end of input. [] => break, @@ -50,12 +54,12 @@ pub fn run( // Verify them and prepare the final refspecs. let mut refspecs = Vec::new(); - for refstr in refs { - let refstr = git::RefString::try_from(refstr)?; + for (oid, refstr) in refs { if let Some(nid) = url.namespace { refspecs.push(nid.to_namespace().join(refstr).to_string()); } else { - refspecs.push(refstr.to_string()); + // Just fetch the object directly in this case, it's simpler and faster. + refspecs.push(oid.to_string()); }; } diff --git a/radicle-remote-helper/src/lib.rs b/radicle-remote-helper/src/lib.rs index 9e1b0ff6..a797a4f9 100644 --- a/radicle-remote-helper/src/lib.rs +++ b/radicle-remote-helper/src/lib.rs @@ -8,6 +8,7 @@ mod list; mod push; use std::path::PathBuf; +use std::str::FromStr; use std::{env, io}; use thiserror::Error; @@ -40,6 +41,9 @@ pub enum Error { /// Git error. #[error("git: {0}")] Git(#[from] git::raw::Error), + /// Invalid reference name. + #[error("invalid ref: {0}")] + InvalidRef(#[from] radicle::git::fmt::Error), /// Storage error. #[error(transparent)] Storage(#[from] radicle::storage::Error), @@ -90,6 +94,8 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { let working = env::var("GIT_DIR") .map(PathBuf::from) .map_err(|_| Error::NoGitDir)?; + // Whether we should output debug logs. + let debug = env::var("RAD_DEBUG").is_ok(); let stdin = io::stdin(); let mut line = String::new(); @@ -98,6 +104,10 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { loop { let tokens = read_line(&stdin, &mut line)?; + if debug { + eprintln!("git-remote-rad: {:?}", &tokens); + } + match tokens.as_slice() { ["capabilities"] => { println!("option"); @@ -144,8 +154,11 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { ["option", ..] => { println!("unsupported"); } - ["fetch", _oid, refstr] => { - return fetch::run(vec![refstr.to_string()], &working, url, stored, &stdin) + ["fetch", oid, refstr] => { + let oid = git::Oid::from_str(oid)?; + let refstr = git::RefString::try_from(*refstr)?; + + return fetch::run(vec![(oid, refstr)], &working, url, stored, &stdin) .map_err(Error::from); } ["push", refspec] => { diff --git a/radicle-remote-helper/src/list.rs b/radicle-remote-helper/src/list.rs index b578013d..52c3bfbd 100644 --- a/radicle-remote-helper/src/list.rs +++ b/radicle-remote-helper/src/list.rs @@ -1,5 +1,6 @@ use thiserror::Error; +use radicle::cob; use radicle::git; use radicle::storage::git::transport::local::Url; use radicle::storage::ReadRepository; @@ -16,10 +17,13 @@ pub enum Error { /// Git error. #[error(transparent)] Git(#[from] radicle::git::ext::Error), + /// COB store error. + #[error(transparent)] + CobStore(#[from] cob::store::Error), } /// List refs for fetching (`git fetch` and `git ls-remote`). -pub fn for_fetch(url: &Url, stored: &R) -> Result<(), Error> { +pub fn for_fetch(url: &Url, stored: &R) -> Result<(), Error> { if let Some(namespace) = url.namespace { // Listing namespaced refs. for (name, oid) in stored.references_of(&namespace)? { @@ -36,6 +40,11 @@ pub fn for_fetch(url: &Url, stored: &R) -> Result<(), Error> println!("{oid} {name}"); } } + // List the patch refs, but don't abort if there's an error, as this would break + // all fetch behavior. Instead, just output an error to the user. + if let Err(e) = patch_refs(stored) { + eprintln!("remote: error listing patch refs: {e}"); + } } println!(); @@ -46,9 +55,24 @@ pub fn for_fetch(url: &Url, stored: &R) -> Result<(), Error> pub fn for_push(profile: &Profile, stored: &R) -> Result<(), Error> { // Only our own refs can be pushed to. for (name, oid) in stored.references_of(profile.id())? { - println!("{oid} {name}"); + // Only branches and tags can be pushed to. + if name.starts_with(git::refname!("refs/heads").as_str()) + || name.starts_with(git::refname!("refs/tags").as_str()) + { + println!("{oid} {name}"); + } } println!(); Ok(()) } + +/// List canonical patch references. These are magic refs that can be used to pull patch updates. +fn patch_refs(stored: &R) -> Result<(), Error> { + let patches = radicle::cob::patch::Patches::open(stored)?; + for patch in patches.all()? { + let (id, patch) = patch?; + println!("{} {}", patch.head(), git::refs::storage::patch(&id)); + } + Ok(()) +} diff --git a/radicle-remote-helper/src/push.rs b/radicle-remote-helper/src/push.rs index 30875bf4..192d291e 100644 --- a/radicle-remote-helper/src/push.rs +++ b/radicle-remote-helper/src/push.rs @@ -318,7 +318,7 @@ fn patch_open( // // refs/namespaces//refs/heads/patches/ // - let refname = git::refs::storage::patch(nid, &patch); + let refname = git::refs::storage::patch(&patch).with_namespace(nid.into()); let _ = stored.raw().reference( refname.as_str(), commit.id(), @@ -493,7 +493,7 @@ fn patch_merge( // Note that we don't return an error if we can't delete the refs, since it's // not critical. let nid = signer.public_key(); - let stored_ref = git::refs::storage::patch(nid, &patch.id); + let stored_ref = git::refs::storage::patch(&patch.id).with_namespace(nid.into()); let working_ref = git::refs::workdir::patch_upstream(&patch.id); stored diff --git a/radicle/src/git.rs b/radicle/src/git.rs index 010862ae..5cfdc0b8 100644 --- a/radicle/src/git.rs +++ b/radicle/src/git.rs @@ -230,15 +230,14 @@ pub mod refs { /// A patch reference. /// - /// `refs/namespaces//refs/heads/patches/` + /// `refs/heads/patches/` /// - pub fn patch<'a>(remote: &RemoteId, object_id: &cob::ObjectId) -> Namespaced<'a> { + pub fn patch<'a>(object_id: &cob::ObjectId) -> Qualified<'a> { Qualified::from_components( component!("heads"), component!("patches"), Some(object_id.into()), ) - .with_namespace(remote.into()) } /// Draft references. diff --git a/radicle/src/node.rs b/radicle/src/node.rs index 5ccc0414..e20fa331 100644 --- a/radicle/src/node.rs +++ b/radicle/src/node.rs @@ -408,6 +408,7 @@ impl Seeds { } /// Announcement result returned by [`Node::announce`]. +#[derive(Debug)] pub struct AnnounceResult { /// Nodes that timed out. pub timeout: Vec, @@ -416,6 +417,7 @@ pub struct AnnounceResult { } /// A sync event, emitted by [`Node::announce`]. +#[derive(Debug)] pub enum AnnounceEvent { /// Refs were synced with the given node. RefsSynced { remote: NodeId }, @@ -640,7 +642,16 @@ impl Node { stream.set_read_timeout(Some(timeout))?; Ok(BufReader::new(stream).lines().map(move |l| { - let l = l?; + let l = l.map_err(|e| { + if e.kind() == io::ErrorKind::WouldBlock { + io::Error::new( + io::ErrorKind::TimedOut, + "timed out reading from control socket", + ) + } else { + e + } + })?; let v = json::from_str(&l).map_err(|e| CallError::InvalidJson { response: l, error: e, @@ -680,7 +691,7 @@ impl Node { } Ok(_) => {} - Err(e) if e.kind() == io::ErrorKind::WouldBlock => { + Err(e) if e.kind() == io::ErrorKind::TimedOut => { timeout.extend(seeds.iter()); break; }