[darcs-devel] [patch1971] introduce some generic merge function helpers

[Ganesh Sittampalam]

Ganesh Sittampalam <ganesh at earth.li> added the comment:

On 22/02/2020 11:09, Ben Franksen wrote:
> 
> Ben Franksen <ben.franksen at online.de> added the comment:
> 
>> patch 80715460da0bdfa1ac95cd2361a920bb50fe6bb6
>> Author: Ganesh Sittampalam <ganesh at earth.li>
>> Date:   Tue Jan 28 06:36:39 GMT 2020
>>   * introduce some generic merge function helpers
> 
> +    -- TODO this should use mergerFLFL but that currently causes
> +    -- a break in convert-darcs2, probably because it's sensitive
> +    -- to the exact order of the merged patches
> 
> Indeed. More precisely, what differs now is the order of the conflicting
> Nons in the conflictor. The Eq2 instance for RepoPatchV2 uses eqSet
> (which compares lists as sets, IIUC), but the ShowPatch instance does
> not sort them. RepoPatchV3 does that, BTW, but it is easier there
> because we have a natural Ord instance via PatchIds.
> 
> So, should we sort the conflicting Nons in a V2 conflictor when we write
> them to disk (or for simplicity whenever we show them)?

Is this actually necessary? What I really had in mind with my TODO
comment was just to take a closer look at the new output, check it's
still correct and then update the test. But maybe having a canonical
output format is worth it.

> What ordering
> should we use? Prim.V1 comes with a suitable ordering but FileUUID does
> not. However, I guess testing FileUUID with RepoPatchV2 is sort of
> wasted anyway, so we should simply drop the unit tests for that combination.

Agreed.

> Even of we do that, we need to decide how to treat the context patches
> in a Non. I am inclined to be practical here and just compare the prims
> at the end. This is not perfect as theoretically we could have equal
> prims with different contexts (i.e. duplicate prims), except that I
> believe in this case RepoPatchV2 would have created a Duplicate, so this
> should be safe.

Two patches could be completely different while in the same context, but
identical textually in different contexts. e.g.

1: addfile foo/X
2: addfile bar/X

in context:
1: [] ; addfile foo/X
2: [mv foo old ; mv bar foo] ; addfile foo/X

Maybe that's not possible for Non as the context has to not commute with
the prim, but I'm not too convinced by the general statement.

Why not just sort including the contexts? If this is just about having a
canonical ordering we don't really care what it is, but having any
possibility of equality would mean there'll be edge cases.

> patch 0e80c5c65e9706de477cf6cdd2beefac0a0e7655
> Author: Ben Franksen <ben.franksen at online.de>
> Date:   Sat Feb 22 12:59:41 CET 2020
>   * sort Nons when showing a RepoPatchV2

You remove the tests for RepoPatchV2 FileUUID in this patch without
explanation, might be better in a separate patch.

PrimOrd would benefit from a bit more description of when it's
appropriate to use it.

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch1971>
__________________________________