Re: [PATCH 9/9] rerere forget path: forget recorded resolution

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 11, 2010, 01:58 UTC
Message-ID: <7v4omte72j.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7vpr5krxoo.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 11 quoted lines

> Johannes Sixt <j6t@kdbg.org> writes:
>
>> Your implementation forgets to re-insert the forgotten resolutions into
>> MERGE_RR, therefore, the next 'git rerere' does not record the new
>> resolution.
>>
>> In my implementation of 'rerere forget', I had the following tests.
>
> Please filfre to roll a patch that adds the tests and code that inserts
> necessary MERGE_RR entries, if the feature is pressing; unfortunately I
> don't think I will have much git time during the coming weekend.

I ended up doing this myself. As we are dropping the postimage and adding a new MERGE_RR entry, I also think that it is safer to update the preimage with the conflict we got for this round, so I added that as well.

However, I think there is a room for improvement in preimage handling.

Currently, the rerere database is indexed with the conflict hash and for each conflict hash you can record a single preimage-postimage pair to replay. But you can have conflicts with the same conflict hash, but with slightly different contexts outside the conflicted region, and the right resolution can be different depending on the outside context.

In the traditional implementation, I punted this issue by noticing conflicts in the three-way merge between pre, post and this images. If preimage is too different from the conflicted contents we got during this merge, then the previous resolution should not apply.

But I think the right solution would be to have more than one preimage and postimage pairs (preimage.0 vs postimage.0,... etc.) and try to use each of them in handle_path() until it finds one that can be used to cleanly merge with the conflict we got in thisimage during this round.