[PATCH 3 of 3 V2] merge: add automatic tag merge algorithm

[Angel Ezquerra]

On Mon, Mar 24, 2014 at 1:28 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 03/17/2014 07:36 AM, Angel Ezquerra wrote:
>>
>> # HG changeset patch
>> # User Angel Ezquerra <angel.ezquerra at gmail.com>
>> # Date 1395006485 -3600
>> #      Sun Mar 16 22:48:05 2014 +0100
>> # Node ID ee742da53c1e43b33d84555662da9ed3ba938f6f
>> # Parent  d2b518fe1a6c7d113cd0d8df5ce1f687e6ec0219
>> merge: add automatic tag merge algorithm
>>
>> Try to automatically merge conflicting .hgtags files using the following
>> algorithm:
>>
>> - keep all tags that are identical on both parents (including their
>> history)
>> - keep all the tags that are on only one of the merge parents
>>
>> If there are any tags that are different in any way on both parents, the
>> automatic merge fails and we revert to the original merge behavior (which
>> is to
>> do a regular .hgtags text merge).
>>
>> This is a very simple algorithm, but it solves what is possibly the most
>> common
>> tag merge conflict: the one that occurs when new, different tags are added
>> on
>> each merge parent. Without this algorithm that scenario always results on
>> an
>> .hgtags merge conflict which requires user intervention even though there
>> is a
>> simple recipe to fix it (the one that this revision implements).
>>
>> We could probably come up with ways to solve other less common merge
>> conflicts
>> but those would get us a much smaller gain with an increased risk. We can
>> explore those in the future.
>>
>> # Notes:
>> - The algorithm assumes that tags are never removed from the .hgtags file.
>> This
>> should be true in most cases (particularly if the user does not normally
>> modify
>> the .hgtags file on its own). We could improve this merge algorithm to
>> handle
>> that case as well.
>> - All existing and new tests pass without modification.
>
>
> This is a custom merge tool for a specific file type. Shouldn't it be in
> filemerge like other merge patterns?
>
> It hardcodes an algorithm for merging .hgtags. It might be good enough to
> use by default, but I think we should avoid making it mandatory. There might
> be cases where it not is what the user want (for instance if they have their
> own .hgtags merge tool). It should be possible to disable / overrule the use
> of this tool - perhaps by configuring a merge pattern for .hgtags.

When I first started writing this reply I was going to agree with you.
However, after thinking a bit about it I am not so sure anymore. Let
me explain my reasoning:

I think we should definitely be able to configure a _fallback_ tag
merge tool which is run when this merge algorithm fails to merge
(because of conflicts). However, if we guarantee that the default the
algorithm never does the wrong thing (i.e. that it only successfully
merges the .hgtags file when there are no conflicts and there is only
one valid way the merge should be done), shouldn't we always accept
that correct merge result?

Imagine that we turned this algorithm into an actual merge tool (lets
call it hgtagmerge, for example). This would have several drawbacks:

1. There would be no way to configure mercurial to use a fallback
merge tool for .hgtags. You could either use the default merge
algorithm or your own tool, but not both (unless I misunderstood the
way merge-patterns work). This means that if I wanted to use WinMerge
(for example) to resolve .hgtags conflicts (instead of the default
KDiff3 tool that TortoiseHg uses for most file types), I would not be
able to do so while still using the default merge algorithm.

2. When the hgtagmerge algorithm fails we want to revert to a regular,
old-style text merge, so that the user can use his merge tool of
choice to do the merge. I don't think (but I may be wrong) that it is
possible to configure mercurial to call a second merge tool if the
first one that you specified via a merge pattern fails.

3. The user could choose to use the hgtagmerge to merge any filetype
of his choice. Does it ever make sense to use this merge algorithm
(which is quite specific to the .hgtags format) for any other sort of
file?

So IMHO the merging of .hgtags should be a bit special, always go
through this merge algorithm first, and only if that fails it should
try to execute the merge tool of your choice (the fallback tag merge
tool, usually the default text merge tool).

> The merge algorithm do not use the ancestor - and can thus not see if
> anything has been removed. It do not even detect when it is reintroducing
> removed tags - it will just silently merge it incorrectly. I think it should
> handle that correctly before it is enabled by default.

I agree (actually the commit message mentions this). As I said above
the algorithm should never do the wrong thing.

> The merge process will sort the .hgtags entries by tag names when writing
> back. If the .hgtags not already thad the entries grouped by tag name
> (because the tag had been reused), it will rewrite the tag file completely.
> That will make other merges that not are using this merge tool much harder.
> I don't think it is ok it regroups the entries - especially not when it not
> is necessary. It would be significantly less intrusive if it started out by
> writing out all the (undeleted) lines from one of the ancestor and then
> appended the missing entries from the other parent.

That is a good idea. Since the merge algorithm can only add new
entries to the .hgtags file I think we could do as you suggest. I'll
have a look and send an updated version that also handles the removal
of tags from the .hgtags file. As I said I'd rather not turn this into
a full blown merge tool unless you really think I should.

Thanks for the review and the thoughtful comments! Please let me know
what you think about my arguments against turning this into a full
blown hgtagmerge tool.

Angel