diff --git a/radicle-cli/examples/rad-patch-revert-merge.md b/radicle-cli/examples/rad-patch-revert-merge.md new file mode 100644 index 00000000..efb33561 --- /dev/null +++ b/radicle-cli/examples/rad-patch-revert-merge.md @@ -0,0 +1,75 @@ +Let's create a patch, merge it and then revert it. + +``` (stderr) RAD_SOCKET=/dev/null +$ git checkout -b feature/1 -q +$ git commit --allow-empty -q -m "First change" +$ git push rad HEAD:refs/patches +✓ Patch 696ec5508494692899337afe6713fe1796d0315c opened +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + * [new reference] HEAD -> refs/patches +$ git checkout master +Switched to branch 'master' +$ git merge feature/1 +$ git push rad master +✓ Patch 696ec5508494692899337afe6713fe1796d0315c merged +✓ Canonical head updated to 20aa5dde6210796c3a2f04079b42316a31d02689 +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + f2de534..20aa5dd master -> master +``` + +First we see the patch as merged. + +``` +$ rad patch show 696ec5508494692899337afe6713fe1796d0315c +╭────────────────────────────────────────────────────────────────╮ +│ Title First change │ +│ Patch 696ec5508494692899337afe6713fe1796d0315c │ +│ Author alice (you) │ +│ Head 20aa5dde6210796c3a2f04079b42316a31d02689 │ +│ Branches feature/1, master │ +│ Commits up to date │ +│ Status merged │ +├────────────────────────────────────────────────────────────────┤ +│ 20aa5dd First change │ +├────────────────────────────────────────────────────────────────┤ +│ ● opened by alice (you) (20aa5dd) now │ +│ └─ ✓ merged by alice (you) at revision 696ec55 (20aa5dd) now │ +╰────────────────────────────────────────────────────────────────╯ +``` + +Now let's revert the patch by pushing a new `master` that doesn't include +the commit. + +``` +$ git reset --hard HEAD^ +HEAD is now at f2de534 Second commit +``` + +When pushing, notice that we're told our patch is reverted. + +``` (stderr) RAD_SOCKET=/dev/null +$ git push rad master --force +! Patch 696ec5508494692899337afe6713fe1796d0315c reverted at revision 696ec55 +✓ Canonical head updated to f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354 +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + + 20aa5dd...f2de534 master -> master (forced update) +``` + +The patch shows up as open again. + +``` +$ rad patch show 696ec5508494692899337afe6713fe1796d0315c +╭────────────────────────────────────────────────────╮ +│ Title First change │ +│ Patch 696ec5508494692899337afe6713fe1796d0315c │ +│ Author alice (you) │ +│ Head 20aa5dde6210796c3a2f04079b42316a31d02689 │ +│ Branches feature/1 │ +│ Commits ahead 1, behind 0 │ +│ Status open │ +├────────────────────────────────────────────────────┤ +│ 20aa5dd First change │ +├────────────────────────────────────────────────────┤ +│ ● opened by alice (you) (20aa5dd) now │ +╰────────────────────────────────────────────────────╯ +``` diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs index 71026e10..864106c5 100644 --- a/radicle-cli/tests/commands.rs +++ b/radicle-cli/tests/commands.rs @@ -1156,6 +1156,26 @@ fn rad_patch_merge_draft() { .unwrap(); } +#[test] +fn rad_patch_revert_merge() { + let mut environment = Environment::new(); + let profile = environment.profile(config::profile("alice")); + let working = tempfile::tempdir().unwrap(); + let home = &profile.home; + + // Setup a test repository. + fixtures::repository(working.path()); + + test("examples/rad-init.md", working.path(), Some(home), []).unwrap(); + test( + "examples/rad-patch-revert-merge.md", + working.path(), + Some(home), + [], + ) + .unwrap(); +} + #[test] #[cfg(not(target_os = "macos"))] fn rad_review_by_hunk() { diff --git a/radicle-remote-helper/src/push.rs b/radicle-remote-helper/src/push.rs index d2bb13f4..d479ce33 100644 --- a/radicle-remote-helper/src/push.rs +++ b/radicle-remote-helper/src/push.rs @@ -566,7 +566,10 @@ fn push( nid: &NodeId, working: &git::raw::Repository, stored: &storage::git::Repository, - patches: patch::Cache, cob::cache::StoreWriter>, + mut patches: patch::Cache< + patch::Patches<'_, storage::git::Repository>, + cob::cache::StoreWriter, + >, signer: &G, ) -> Result, Error> { let head = match working.find_reference(src.as_str()) { @@ -592,24 +595,93 @@ fn push( let master = &*git::Qualified::from(git::lit::refs_heads(proj.default_branch())); // 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. + // merged or reverted, and if so, update the patch COB. if &*dst.strip_namespace() == master { let old = old.peel_to_commit()?.id(); - // Only delegates should publish the merge result to the COB. + // Only delegates affect the merge state of the COB. if stored.delegates()?.contains(&nid.into()) { - patch_merge_all(old.into(), head.into(), working, patches, signer)?; + patch_revert_all( + old.into(), + head.into(), + &stored.backend, + &mut patches, + signer, + )?; + patch_merge_all(old.into(), head.into(), working, &mut patches, signer)?; } } } Ok(Some(ExplorerResource::Tree { oid: head.into() })) } +/// Revert all patches that are no longer included in the base branch. +fn patch_revert_all( + old: git::Oid, + new: git::Oid, + stored: &git::raw::Repository, + patches: &mut patch::Cache< + patch::Patches<'_, storage::git::Repository>, + cob::cache::StoreWriter, + >, + _signer: &G, +) -> Result<(), Error> { + // Find all commits reachable from the old OID but not from the new OID. + let mut revwalk = stored.revwalk()?; + revwalk.push(*old)?; + revwalk.hide(*new)?; + + // List of commits that have been dropped. + let dropped = revwalk + .map(|r| r.map(git::Oid::from)) + .collect::, _>>()?; + if dropped.is_empty() { + return Ok(()); + } + + // Get the list of merged patches. + let merged = patches + .merged()? + // Skip patches that failed to load. + .filter_map(|patch| patch.ok()) + .collect::>(); + + for (id, patch) in merged { + let revisions = patch + .revisions() + .map(|(id, r)| (id, r.head())) + .collect::>(); + + for commit in &dropped { + if let Some((revision_id, _)) = revisions.iter().find(|(_, head)| commit == head) { + // Simply refreshing the cache entry will pick up on the fact that this patch + // is no longer merged in the canonical branch. + match patches.write(&id) { + Ok(()) => { + eprintln!( + "{} Patch {} reverted at revision {}", + term::format::yellow("!"), + term::format::tertiary(&id), + term::format::dim(term::format::oid(*revision_id)), + ); + } + Err(e) => { + eprintln!("{} Error reverting patch {id}: {e}", term::ERROR_PREFIX); + } + } + break; + } + } + } + + Ok(()) +} + /// Merge all patches that have been included in the base branch. fn patch_merge_all( old: git::Oid, new: git::Oid, working: &git::raw::Repository, - mut patches: patch::Cache< + patches: &mut patch::Cache< patch::Patches<'_, storage::git::Repository>, cob::cache::StoreWriter, >, @@ -622,14 +694,17 @@ fn patch_merge_all( let commits = revwalk .map(|r| r.map(git::Oid::from)) .collect::, _>>()?; + if commits.is_empty() { + return Ok(()); + } - let all = patches + let open = patches .opened()? .chain(patches.drafted()?) // Skip patches that failed to load. .filter_map(|patch| patch.ok()) .collect::>(); - for (id, patch) in all { + for (id, patch) in open { // Later revisions are more likely to be merged, so we build the list backwards. let revisions = patch .revisions() @@ -642,7 +717,7 @@ fn patch_merge_all( // revision that is closest to the tip of the commit chain we're pushing. for commit in &commits { if let Some((revision_id, _)) = revisions.iter().find(|(_, head)| commit == head) { - let patch = patch::PatchMut::new(id, patch, &mut patches); + let patch = patch::PatchMut::new(id, patch, patches); patch_merge(patch, *revision_id, new, working, signer)?; break;