[PATCH] merge: add automatic tag merge algorithm

[Mads Kiilerich]

On 02/23/2014 03:19 AM, Angel Ezquerra wrote:
> On Sat, Feb 22, 2014 at 5:59 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
> Sorting the tags when writing them is not strictly necessary.

Then I suggest leaving it out ... or saving it for later where it can be 
discussed by its own merits.

> I have reworked this patch into a patch series in which I first move
> sortdict into its own file, then I use this sortdict in tags._readtags
> and finally I introduce the automated merge, with the key difference
> that there is no need to sort the tags after a successful merge
> anymore. I will send this V2 patch series soon (unless you give me
> some new comments before I do).

It is not clear to me how using a sortdict would solve the problem ... 
and whether it would be better than alternative simpler solutions. 
Patches might explain it to me.

Another comment would be that I think it would be better to have this in 
filemerge, perhaps as 'internal:tagsmerge' where it in first milestone 
could be enabled with [merge-patterns] .hgignore=internal:tagsmerge .

> A nice side benefit of this change would be that we could also change
> (and improve!) the way the tag command reads and writes tags. We could
> reuse the tags._readtags function and create a a new writetags
> function as well. Currently the tag command is "append only" in the
> sense that it just adds new entries at the end of the .hgtags file.
> This has several undesirable consequences:
>
> - It makes merge conflicts very common (this is what this patch we are
> discussing addresses) so in a sense this is moot.
> - It creates duplicate entries in the .hgtags file which make manually
> merging .hgtags files very confusing. For example, if you first add a
> tag A, then add a tag B and finally move the tag A the final .hgtags
> file will look something like this:
>
> FIRST_A_REVID A
> FIRST_B_REVID B
> FIRST_A_REVID A
> SECOND_A_REVID A
>
> This is quite weird and hard to understand. Instead it would be best
> (IMHO) if the result were:
>
> FIRST_A_REVID A
> SECOND_A_REVID A
> FIRST_B_REVID B
>
> This would be easier to understand, and easier to merge.

You are aware that it was a deliberate design decision to do it that 
way? That ensures that you do get a conflict in some cases that 
otherwise would merge cleanly but incorrectly.

> I think this would be an easy (and desirable) fix. I also think that
> it would be backwards compatible. The only drawback that I can think
> of is that we would need to write the whole .hgtags file when tagging.
> I doubt this would be a problem even with a huge number of tags
> (specially since tagging is not such a common operation anyway (maybe
> at Facebook's scale this would be a problem though?). I will write a
> patch for this and add it to my V2 patch series of this patch, unless
> you think it is a bad idea.

Tagging could have been done differently with different trade-offs. I 
think the changes you are proposing are too invasive and not 
sufficiently backward compatible for this user-facing file, especially 
considering that they don't add significant value. Automatic tag merging 
could however still be nice and valuable.

If we were to make allmost backward compatible changes to .hgtags then I 
would suggest considering taking the full step and leaving history out 
of the sorted file.

/Mads