diff --git a/radicle-cli/examples/rad-merge-via-push.md b/radicle-cli/examples/rad-merge-via-push.md index 1ec7d0db..7e2c1683 100644 --- a/radicle-cli/examples/rad-merge-via-push.md +++ b/radicle-cli/examples/rad-merge-via-push.md @@ -17,6 +17,36 @@ To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkE * [new reference] HEAD -> refs/patches ``` +This creates some remote tracking branches for us: + +``` +$ git branch -r + rad/master + rad/patches/dce2ff0b2baf6da67fae5143b828ebfab65d41e4 + rad/patches/f4e9dcffb21bee746e0eee965933c7e237aa207a +``` + +And some remote refs: + +``` +$ rad inspect --refs +. +`-- z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + `-- refs + |-- cobs + | `-- xyz.radicle.patch + | |-- dce2ff0b2baf6da67fae5143b828ebfab65d41e4 + | `-- f4e9dcffb21bee746e0eee965933c7e237aa207a + |-- heads + | |-- master + | `-- patches + | |-- dce2ff0b2baf6da67fae5143b828ebfab65d41e4 + | `-- f4e9dcffb21bee746e0eee965933c7e237aa207a + `-- rad + |-- id + `-- sigrefs +``` + Then let's merge the changes into `master`. ``` (stderr) RAD_SOCKET=/dev/null @@ -44,3 +74,28 @@ $ rad patch --merged │ ✔ f4e9dcf First change z6MknSL…StBU8Vi (you) 20aa5dd +0 -0 [ ... ] │ ╰──────────────────────────────────────────────────────────────────────────────────╯ ``` + +We can verify that the remote tracking branches were also deleted: + +``` +$ git branch -r + rad/master +``` + +And so were the remote branches: + +``` +$ rad inspect --refs +. +`-- z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + `-- refs + |-- cobs + | `-- xyz.radicle.patch + | |-- dce2ff0b2baf6da67fae5143b828ebfab65d41e4 + | `-- f4e9dcffb21bee746e0eee965933c7e237aa207a + |-- heads + | `-- master + `-- rad + |-- id + `-- sigrefs +``` diff --git a/radicle-remote-helper/src/push.rs b/radicle-remote-helper/src/push.rs index ea1f6814..427cbf79 100644 --- a/radicle-remote-helper/src/push.rs +++ b/radicle-remote-helper/src/push.rs @@ -44,6 +44,9 @@ pub enum Error { /// Invalid reference name. #[error("invalid ref: {0}")] InvalidRef(#[from] radicle::git::fmt::Error), + /// Invalid reference name. + #[error("invalid qualified ref: {0}")] + InvalidQualifiedRef(git::RefString), /// Git error. #[error("git: {0}")] Git(#[from] git::raw::Error), @@ -168,14 +171,19 @@ pub fn run( Command::Push(git::Refspec { src, dst, force }) => { let working = git::raw::Repository::open(working)?; - if let Some(oid) = dst.strip_prefix(git::refname!("refs/heads/patches")) { - let oid = git::Oid::from_str(oid)?; - - patch_update(src, dst, *force, &oid, &nid, &working, stored, &signer) - } else if dst == &*rad::PATCHES_REFNAME { + if dst == &*rad::PATCHES_REFNAME { patch_open(src, &nid, &working, stored, &signer) } else { - push(src, dst, *force, &nid, &working, stored, &signer) + let dst = git::Qualified::from_refstr(dst) + .ok_or_else(|| Error::InvalidQualifiedRef(dst.clone()))?; + + if let Some(oid) = dst.strip_prefix(git::refname!("refs/heads/patches")) { + let oid = git::Oid::from_str(oid)?; + + patch_update(src, &dst, *force, &oid, &nid, &working, stored, &signer) + } else { + push(src, &dst, *force, &nid, &working, stored, &signer) + } } } }; @@ -224,7 +232,7 @@ fn patch_open( ) -> Result<(), Error> { let reference = working.find_reference(src.as_str())?; let commit = reference.peel_to_commit()?; - let dst = &*git::refs::storage::staging::patch(nid, commit.id()); + let dst = git::refs::storage::staging::patch(nid, commit.id()); // Before creating the patch, we must push the associated git objects to storage. // Unfortunately, we don't have an easy way to transfer the missing objects without @@ -233,7 +241,7 @@ fn patch_open( // // In case the reference is not properly deleted, the next attempt to open a patch should // not fail, since the reference will already exist with the correct OID. - push_ref(src, dst, false, nid, working, stored.raw())?; + push_ref(src, &dst, false, working, stored.raw())?; let mut patches = patch::Patches::open(stored)?; let message = commit.message().unwrap_or_default(); @@ -306,7 +314,7 @@ fn patch_open( // Delete short-lived patch head reference. stored .raw() - .find_reference(dst) + .find_reference(&dst) .map(|mut r| r.delete()) .ok(); @@ -317,7 +325,7 @@ fn patch_open( #[allow(clippy::too_many_arguments)] fn patch_update( src: &git::RefStr, - dst: &git::RefStr, + dst: &git::Qualified, force: bool, oid: &git::Oid, nid: &NodeId, @@ -328,8 +336,9 @@ fn patch_update( let reference = working.find_reference(src.as_str())?; let commit = reference.peel_to_commit()?; let patch_id = radicle::cob::ObjectId::from(oid); + let dst = dst.with_namespace(nid.into()); - push_ref(src, dst, force, nid, working, stored.raw())?; + push_ref(src, &dst, force, working, stored.raw())?; let mut patches = patch::Patches::open(stored)?; let Ok(mut patch) = patches.get_mut(&patch_id) else { @@ -357,7 +366,7 @@ fn patch_update( fn push( src: &git::RefStr, - dst: &git::RefStr, + dst: &git::Qualified, force: bool, nid: &NodeId, working: &git::raw::Repository, @@ -366,13 +375,11 @@ fn push( ) -> Result<(), Error> { let head = working.find_reference(src.as_str())?; let head = head.peel_to_commit()?.id(); + let dst = dst.with_namespace(nid.into()); // It's ok for the destination reference to be unknown, eg. when pushing a new branch. - let old = stored - .backend - .find_reference(nid.to_namespace().join(dst).as_str()) - .ok(); + let old = stored.backend.find_reference(dst.as_str()).ok(); - push_ref(src, dst, force, nid, working, stored.raw())?; + push_ref(src, &dst, force, working, stored.raw())?; if let Some(old) = old { let proj = stored.project()?; @@ -380,7 +387,7 @@ fn push( // If we're pushing to the project's default branch, we want to see if any patches got // merged, and if so, update the patch COB. - if dst == master { + if &*dst.strip_namespace() == master { let old = old.peel_to_commit()?.id(); // Only delegates should publish the merge result to the COB. if stored.delegates()?.contains(&nid.into()) { @@ -424,6 +431,23 @@ fn patch_merge( cli::format::positive("✓"), cli::format::tertiary(id) ); + + // Delete patch references that were created when the patch was opened. + // 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 working_ref = git::refs::workdir::patch_upstream(&patch.id); + + stored + .raw() + .find_reference(&stored_ref) + .and_then(|mut r| r.delete()) + .ok(); + working + .find_reference(&working_ref) + .and_then(|mut r| r.delete()) + .ok(); } } Ok(()) @@ -432,14 +456,12 @@ fn patch_merge( /// Push a single reference to storage. fn push_ref( src: &git::RefStr, - dst: &git::RefStr, + dst: &git::Namespaced, force: bool, - nid: &NodeId, working: &git::raw::Repository, stored: &git::raw::Repository, ) -> Result<(), Error> { let mut remote = working.remote_anonymous(&git::url::File::new(stored.path()).to_string())?; - let dst = nid.to_namespace().join(dst); let refspec = git::Refspec { src, dst, force }; // Nb. The *force* indicator (`+`) is processed by Git tooling before we even reach this code.