remote-helper: Handle patch update after merge

If you accidently merge updated patch code before updating the patch in
storage, you end up in this weird state where you can't update the patch
any longer. This fixes it.
This commit is contained in:
Alexis Sellier 2023-07-25 12:19:49 +02:00 committed by cloudhead
parent df5c8c7d4f
commit 2ea9a911af
No known key found for this signature in database
3 changed files with 111 additions and 31 deletions

View File

@ -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)
```

View File

@ -966,6 +966,33 @@ fn rad_merge_via_push() {
.unwrap(); .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] #[test]
fn git_push_and_pull() { fn git_push_and_pull() {
logger::init(log::Level::Debug); logger::init(log::Level::Debug);

View File

@ -386,9 +386,6 @@ fn patch_update<G: Signer>(
} else { } else {
stored.merge_base(&target, &head)? stored.merge_base(&target, &head)?
}; };
if base == head {
return Err(Error::EmptyPatch);
}
let revision = patch.update(message, base, head, signer)?; let revision = patch.update(message, base, head, signer)?;
eprintln!( eprintln!(
@ -398,6 +395,16 @@ fn patch_update<G: Signer>(
cli::format::tertiary(revision) 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(()) Ok(())
} }
@ -428,15 +435,15 @@ fn push<G: Signer>(
let old = old.peel_to_commit()?.id(); let old = old.peel_to_commit()?.id();
// Only delegates should publish the merge result to the COB. // Only delegates should publish the merge result to the COB.
if stored.delegates()?.contains(&nid.into()) { 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(()) Ok(())
} }
/// Merge a patch. /// Merge all patches that have been included in the base branch.
fn patch_merge<G: Signer>( fn patch_merge_all<G: Signer>(
old: git::Oid, old: git::Oid,
new: git::Oid, new: git::Oid,
working: &git::raw::Repository, working: &git::raw::Repository,
@ -457,37 +464,50 @@ fn patch_merge<G: Signer>(
if patch.is_open() && commits.contains(&revision.head()) { if patch.is_open() && commits.contains(&revision.head()) {
let revision_id = *revision_id; 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)?; patch_merge(patch, revision_id, new, working, stored, 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();
} }
} }
Ok(()) Ok(())
} }
fn patch_merge<G: Signer>(
mut patch: patch::PatchMut<storage::git::Repository>,
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. /// Push a single reference to storage.
fn push_ref( fn push_ref(
src: &git::RefStr, src: &git::RefStr,