threads / discuss / 46660

Should rerere auto-update a merge resolution?

Subject: Should rerere auto-update a merge resolution?

## tl;dr

8 messages between Aug 23, 2017 and Aug 25, 2017.

replies: 7people: 3as markdown or json

Martin Langhoff· Aug 23, 2017, 19:39 UTC · lore
Hi List!
Let's say...
 - git v2.9.4
 - rerere is enabled.
 - I merge maint into master, resolve erroneously, commit
 - I publish my merge in a temp branch, a reviewer points out my mistake
 - I reset hard, retry the merge, using --no-commit, rerere  applies
what it knows
 - I fix things up, then commit
So far so good.
Oops! One of the branches has moved forward in the meantime, so
 - git fetch
 - git reset --hard master
 - git merge maint
... rerere applies the first (incorrect) resolution...
Am I doing it wrong? {C,Sh}ould rerere have done better?
cheers,
m
-- 
 martin.langhoff@gmail.com
 - ask interesting questions  ~  http://linkedin.com/in/martinlanghoff
 - don't be distracted        ~  http://github.com/martin-langhoff
   by shiny stuff
Junio C Hamano· Aug 23, 2017, 20:34 UTC · re: Martin Langhoff · lore

Re: Should rerere auto-update a merge resolution?

Martin Langhoff <martin.langhoff@gmail.com> writes:
Show 21 quoted lines
> Hi List!
>
> Let's say...
>  - git v2.9.4
>  - rerere is enabled.
>  - I merge maint into master, resolve erroneously, commit
>  - I publish my merge in a temp branch, a reviewer points out my mistake
>  - I reset hard, retry the merge, using --no-commit, rerere  applies
> what it knows
>  - I fix things up, then commit
>
> So far so good.
>
> Oops! One of the branches has moved forward in the meantime, so
>
>  - git fetch
>  - git reset --hard master
>  - git merge maint
> ... rerere applies the first (incorrect) resolution...
>
> Am I doing it wrong? {C,Sh}ould rerere have done better?
Between these two steps:
>  - I reset hard, retry the merge, using --no-commit, rerere applies what it knows
>  - I fix things up, then commit

You'd tell rerere to forget what it knows because it is wrong. Then after these two (eh, now "three" because there is the "forget" step), "rerere" notices that an updated resolution needs to be recorded, so it remembers it. Later re-resolution will replay the corrected one, simply because the old incorrect one is forgotten and replaced by the updated correct one.

Martin Langhoff· Aug 23, 2017, 21:12 UTC · re: Junio C Hamano · lore

Re: Should rerere auto-update a merge resolution?

On Wed, Aug 23, 2017 at 4:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 6 quoted lines
> Between these two steps:
>
>>  - I reset hard, retry the merge, using --no-commit, rerere applies what it knows
>>  - I fix things up, then commit
>
> You'd tell rerere to forget what it knows because it is wrong.
Hi Junio!
thanks for the quick response.
Questions
 - when I tell it to forget, won't it forget the pre-resolution state?
my read of the rerere docs imply that it gets called during the merge
to record the conflicted state.
 - would it be a feature if it updated its resolution db
automagically? rerere is plenty automagic already...
cheers,
m
-- 
 martin.langhoff@gmail.com
 - ask interesting questions  ~  http://linkedin.com/in/martinlanghoff
 - don't be distracted        ~  http://github.com/martin-langhoff
   by shiny stuff
Johannes Schindelin· Aug 25, 2017, 10:19 UTC · re: Martin Langhoff · lore

Re: Should rerere auto-update a merge resolution?

Hi Martin,
On Wed, 23 Aug 2017, Martin Langhoff wrote:
Show 13 quoted lines
> On Wed, Aug 23, 2017 at 4:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Between these two steps:
> >
> >>  - I reset hard, retry the merge, using --no-commit, rerere applies what it knows
> >>  - I fix things up, then commit
> >
> > You'd tell rerere to forget what it knows because it is wrong.
> 
> Questions
> 
>  - when I tell it to forget, won't it forget the pre-resolution state?
> my read of the rerere docs imply that it gets called during the merge
> to record the conflicted state.

In my hands, I need to tell rerere to forget, *and then recreate the merge conflict* before I can resolve it again and let rerere learn the new resolution.

>  - would it be a feature if it updated its resolution db
> automagically? rerere is plenty automagic already...
That would most likely be a very welcome feature here.

Thanks, Johannes

Junio C Hamano· Aug 25, 2017, 16:13 UTC · re: Johannes Schindelin · lore

Re: Should rerere auto-update a merge resolution?

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> In my hands, I need to tell rerere to forget, *and then recreate the merge
> conflict* before I can resolve it again and let rerere learn the new
> resolution.

I can believe that---that is how I originally desiged "forget" to behave.

Even though My quick tests to prepare a reply to Martin seemed to show that unmerging was not strictly needed in today's Git in order to forget and learn new, I wouldn't be surprised if an explicit unmerging in addition to "rerere forget" was necessary, especially if the original preimage were very different from the state before the botched resolution was applied.

Junio C Hamano· Aug 25, 2017, 15:16 UTC · re: Martin Langhoff · lore

Re: Should rerere auto-update a merge resolution?

Martin Langhoff <martin.langhoff@gmail.com> writes:
>  - when I tell it to forget, won't it forget the pre-resolution state?

I do not recall the details of what I did ;-) so I played around a bit. Here is what I did:

	git checkout master^0
	git merge --no-ff --no-edit pb/trailers-from-command-line
	git merge --no-ff --no-edit jk/trailers-parse

I know that the last one I know gets conflict and triggers rerere to replay the recorded resolution. Imagine that I earlier botched resolution and the working tree contents is wrong or something at this point.

	make test ;# fails, perhaps
So I can do:
	git rerere forget <path>

After git rerere forget, I observe (check subdirectories in .git/rr-cache/ whose timestamps are recent) that postimage gets removed but preimage and thisimage stay.

I can then edit that file, and say "git rerere" again, which is greeted by "Recorded resolution for '<path>'".

I do not recall if I designed it that way or not, but it even seems to work correctly if you had "git add -u" after the botched auto application (i.e. between the "make test" step and "rerere forget" step in the above sequence). I never do "add -u" myself before testing so I didn't notice.

If you want to _see_ the conflicts again while fixing a botched resolution, then you'd need to do a bit more involved recovery. After seeing "make test" fail and realize your older resolution is botched, you'd probably do:

	git checkout -m <path>

to unmerge, "rerere forget <path>", fix the botched resolution and then finally "rerere" to record the correction.

Junio C Hamano· Aug 25, 2017, 16:07 UTC · re: Junio C Hamano · lore

Re: Should rerere auto-update a merge resolution?

Junio C Hamano <gitster@pobox.com> writes:
Show 10 quoted lines
> Martin Langhoff <martin.langhoff@gmail.com> writes:
>
>>  - when I tell it to forget, won't it forget the pre-resolution state?
>
> I do not recall the details of what I did ;-) so I played around a
> bit.  Here is what I did:
> ...
> After git rerere forget, I observe (check subdirectories in
> .git/rr-cache/ whose timestamps are recent) that postimage gets
> removed but preimage and thisimage stay.

Having said that, I suspect that it may be a bug if this procedure kept the original preimage. It should either remove it, or update it to record the state before the ealier resolution was applied (i.e. make the updated preimage identical to thisimage, so that a corrected resolution can be taken from the working tree to pair with it).

As you may be able to see, "rerere forget" is much less used, hence much less exposed to end-user needs to gain both feature and usability polish than other parts of "rerere". I'd expect there would be many cases where an extra cleverness can be used without making the result less robust, and it would be a good thing to see if we can improve them. We must avoid the cleverness leading us to a "works most of the time" machinery whose operation cannot be trusted, though.

The way the current machinery works is
 * "rerere" sees an unmerged index entry.  It computes the conflict id
   to look into its database, finds a <pre,post> image pair, and
   does a three-way merge to update the working tree file (with
   conflict markers) with pre->post change (i.e. previous resolution).

and that's it. We may be able to make "updating the resolution" easier, perhaps like so:

 * The end user edits working tree file further to correct the
   mistake in the earlier resolution that was applied.
 * The end user says "rerere update <path>", which internally does
   the equivalent of "checkout -m <path>" to unmerge and then
   computes the preimage for this round by redoing the original
   merge, write out the preimage and take the working tree file as
   the postimage

We might even be able to do a bit more in the "normal application" codepath to make it record the result of the three-way merge, perhaps in an index extension. Then "rerere update <path>" may not even be needed---at strategic places in the workflow (e.g. when the user adds the path to the index), we can check if the contents is different from what rerere wrote out as the resolution and if they differ, do the "rerere update <path>" automatically.

But we may be taking the extra cleverness a bit too close to a danger zone by trying to automate too much without thinking the ramification through, so I'd stop speculation at this point.

Junio C Hamano· Aug 25, 2017, 17:08 UTC · re: Junio C Hamano · lore

Re: Should rerere auto-update a merge resolution?

Junio C Hamano <gitster@pobox.com> writes:
Show 19 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Martin Langhoff <martin.langhoff@gmail.com> writes:
>>
>>>  - when I tell it to forget, won't it forget the pre-resolution state?
>>
>> I do not recall the details of what I did ;-) so I played around a
>> bit.  Here is what I did:
>> ...
>> After git rerere forget, I observe (check subdirectories in
>> .git/rr-cache/ whose timestamps are recent) that postimage gets
>> removed but preimage and thisimage stay.
>
> Having said that, I suspect that it may be a bug if this procedure
> kept the original preimage.  It should either remove it, or update
> it to record the state before the ealier resolution was applied
> (i.e. make the updated preimage identical to thisimage, so that a
> corrected resolution can be taken from the working tree to pair with
> it).

I just made a cursory scan of rerere.c again, and it seems we are doing the right thing. The details are in rerere_forget_one_path() where we unlink postimage, we recreate the conflicted state from the stages in the index and update preimage.

It seems that code gives up if you already declared that you'd take the previous resolution by adding the result to the index. It may probably be a good idea to unmerge such a merged index entry instead of giving up. #leftoverbits

So, yes, it will forget both preimage and postimage, and it should update the preimage with the conflict you got during _this_ merge, so that the resolution you make _this_ time will become usable as the corresponding postimage for the next time.

← back to recent threads