diff --git a/radicle-cli/examples/rad-merge-after-update.md b/radicle-cli/examples/rad-merge-after-update.md new file mode 100644 index 00000000..13f9d1e0 --- /dev/null +++ b/radicle-cli/examples/rad-merge-after-update.md @@ -0,0 +1,33 @@ +Let's start by creating a patch. + +``` (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 0ec956c94256fa101db4c32956ce195a1aa0edf2 opened +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + * [new reference] HEAD -> refs/patches +``` + +Then let's update the code and merge the updated code without updating the patch: + +``` (stderr) RAD_SOCKET=/dev/null +$ git commit --amend --allow-empty -q -m "Amended change" +$ git checkout master -q +$ git merge feature/1 -q +$ git push rad master +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + f2de534..954bcdb master -> master +``` + +As we can see, no patch is merged. Now if we go back to our patch and try to +update it, we expect it to be updated and merged: + +``` (stderr) RAD_SOCKET=/dev/null +$ git checkout feature/1 -q +$ git push -f +✓ Patch 0ec956c updated to 8175b00f4d75059976930cfcb75ef08454c87055 +✓ Patch 0ec956c94256fa101db4c32956ce195a1aa0edf2 merged +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + + 20aa5dd...954bcdb feature/1 -> patches/0ec956c94256fa101db4c32956ce195a1aa0edf2 (forced update) +``` diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs index 1d701b03..a8a2692f 100644 --- a/radicle-cli/tests/commands.rs +++ b/radicle-cli/tests/commands.rs @@ -966,6 +966,33 @@ fn rad_merge_via_push() { .unwrap(); } +#[test] +fn rad_merge_after_update() { + let mut environment = Environment::new(); + let alice = environment.node(Config::new(Alias::new("alice"))); + let working = environment.tmp().join("working"); + + fixtures::repository(working.join("alice")); + + test( + "examples/rad-init.md", + working.join("alice"), + Some(&alice.home), + [], + ) + .unwrap(); + + let alice = alice.spawn(); + + test( + "examples/rad-merge-after-update.md", + working.join("alice"), + Some(&alice.home), + [], + ) + .unwrap(); +} + #[test] fn git_push_and_pull() { logger::init(log::Level::Debug); diff --git a/radicle-remote-helper/src/push.rs b/radicle-remote-helper/src/push.rs index d195b216..fca88855 100644 --- a/radicle-remote-helper/src/push.rs +++ b/radicle-remote-helper/src/push.rs @@ -386,9 +386,6 @@ fn patch_update( } else { stored.merge_base(&target, &head)? }; - if base == head { - return Err(Error::EmptyPatch); - } let revision = patch.update(message, base, head, signer)?; eprintln!( @@ -398,6 +395,16 @@ fn patch_update( cli::format::tertiary(revision) ); + // In this case, the patch was already merged via git, and pushed to storage. + // To handle this situation, we simply update the patch state to "merged". + // + // This can happen if for eg. a patch commit is amended, the patch branch is merged + // and pushed, but the patch hasn't yet been updated. On push to the patch branch, + // it'll seem like the patch is "empty", because the changes are already in the base branch. + if base == head && patch.is_open() { + patch_merge(patch, revision, head, working, stored, signer)?; + } + Ok(()) } @@ -428,15 +435,15 @@ fn push( let old = old.peel_to_commit()?.id(); // Only delegates should publish the merge result to the COB. if stored.delegates()?.contains(&nid.into()) { - patch_merge(old.into(), head.into(), working, stored, signer)?; + patch_merge_all(old.into(), head.into(), working, stored, signer)?; } } } Ok(()) } -/// Merge a patch. -fn patch_merge( +/// 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, @@ -457,37 +464,50 @@ fn patch_merge( if patch.is_open() && commits.contains(&revision.head()) { let revision_id = *revision_id; - let mut patch = patch::PatchMut::new(id, patch, &mut patches); + let patch = patch::PatchMut::new(id, patch, &mut patches); - patch.merge(revision_id, new, signer)?; - - eprintln!( - "{} Patch {} merged", - 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(); + patch_merge(patch, revision_id, new, working, stored, signer)?; } } Ok(()) } +fn patch_merge( + mut patch: patch::PatchMut, + revision: patch::RevisionId, + commit: git::Oid, + working: &git::raw::Repository, + stored: &storage::git::Repository, + signer: &G, +) -> Result<(), Error> { + patch.merge(revision, commit, signer)?; + + eprintln!( + "{} Patch {} merged", + cli::format::positive("✓"), + cli::format::tertiary(patch.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(()) +} + /// Push a single reference to storage. fn push_ref( src: &git::RefStr,