"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: got patch; got patch -R
To:
Christian Weisgerber <naddy@mips.inka.de>
Cc:
gameoftrees@openbsd.org
Date:
Wed, 20 Jul 2022 23:51:56 +0200

Download raw body.

Thread
Christian Weisgerber <naddy@mips.inka.de> wrote:
> Hmm, wasn't there talk that re-applying the same patch, but reversed,
> would be handled as a revert?  Maybe I misremember.
> 
> This here seems... unexpected:
> 
> $ got patch ~/ng.diff3    # one of Mark's tog nG diffs
> G  tog/tog.1
> G  tog/tog.c
> $ got patch -R ~/ng.diff3
> #  tog/tog.1
> @@ -93,10 +93,6 @@ hunk failed to apply
> #  tog/tog.c
> @@ -516,7 +516,6 @@ hunk failed to apply
> got: patch failed to apply

this is yet another regression of the diff3 merge... sigh.

`got patch' recognizes the got log (and git diff) format and extracts
the some information for the diff3 merge.  The issue is that this is
done even when working in -R mode: apply_patch attems to apply the
reverse of the patch to the original blob, which of course fails.

A quick solution may be to disable the diff3 merge in -R mode:

diff /home/op/w/got
commit - fced5a66069199024aaf413a06bcf544b959f6a8
path + /home/op/w/got
blob - 703c22b84de200f2a0ec9fc0f7bdc70dc725ab20
file + lib/patch.c
--- lib/patch.c
+++ lib/patch.c
@@ -916,6 +916,10 @@ reverse_patch(struct got_patch *p)
 	size_t i;
 	int tmp;
 
+	/* can't diff3 merge a reverse */
+	memset(&p->cid, 0, sizeof(p->cid));
+	memset(&p->blob, 0, sizeof(p->blob));
+
 	STAILQ_FOREACH(h, &p->head, entries) {
 		tmp = h->old_from;
 		h->old_from = h->new_from;


this allows to apply the patch and the reversal somehow:

	% got patch < patch
	G  tog/tog.1
	G  tog/tog.c
	% got patch -R < patch
	M  tog/tog.1
	M  tog/tog.c
	@@ -516,7 +516,6 @@ applied with offset 1
	@@ -1235,7 +1234,6 @@ applied with offset 1
	@@ -1257,12 +1255,6 @@ applied with offset 1
	...

However, I think we can do better.  i'm a bit in a hurry so i haven't
had the time to think more about it, but i suspect we could detect the
operation and do a revert with merge anyway.  (i'm unsure if ignoring
the patch and just doing a merge with the old blob makes sense and has
bad outcomes)