From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: David Krmpotic <david.krmpotic@gmail.com>, git@vger.kernel.org
Subject: Re: auto merge bug
Date: Tue, 5 Mar 2013 12:59:04 -0500 [thread overview]
Message-ID: <20130305175904.GC9379@sigill.intra.peff.net> (raw)
In-Reply-To: <7vtxopvoky.fsf@alter.siamese.dyndns.org>
On Tue, Mar 05, 2013 at 07:44:13AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I think the merge will produce the results you are looking for. This
> > would have to be configurable, though, as it is a regression for
> > existing users of "union", which would want the duplicate-line
> > suppression (or maybe not; it will only catch such duplicates at the
> > beginning and end of the conflict hunk, so maybe it is sane to always
> > ask "union" to keep all lines).
>
> The original use-case example of "union" was to merge two shopping
> lists (e.g. I add "bread" and "orange juice" to remind me that we
> need to buy these things, while my wife adds "bread" and "butter").
>
> We do not necessarily want to end up with a shopping list to buy two
> loaves of bread. When the user verifies and fixes up the result, we
> can keep the current behaviour and those who want to re-dup can add
> one back, or we can change the behaviour to leave the duplicates and
> those who do not want to see duplicates can remove them manually.
>
> Given that the caveat you quoted already tells the user to verify
> the result and not to use it without understanding its implications,
> I think it technically is fine either way (read: keeping duplicates
> is not a clearly superiour solution). So let's leave it as-is.
My problem with the current behavior is that it is not predictable
whether it will de-dup or not. If your shopping lists are:
bread
orange juice
bread
butter
it works; you get only one bread. If they are:
milk
bread
orange juice
beer
bread
butter
you get two. It depends on the exact behavior of the XDL_MERGE_ZEALOUS
flag. What I'd propose is two different drivers:
1. Find conflicts via 3-way merge, and include both sides of the
conflict verbatim. Do not use XDL_MERGE_ZEALOUS, as it is more
important to retain items from both sides (in their original order)
than it is to remove duplicates.
2. A true line-based union, which should act like "cat $ours $theirs |
sort | uniq". That is what you want for the shopping list example,
I think (you could also preserve existing ordering with a lookup
table, though I prefer clobbering the ordering; the ordering of
resolved conflicts will be arbitrary anyway, so it makes it clear
from the outset that you should not use this driver if your content
is not really a set (in the mathematical sense) of lines).
You could also have sets of other objects (e.g., blank-line
delimited paragraphs, changelog entries, etc). But you would need
some way to specify the parsing then[1].
I'm not sure which should be called "union". The first one would still
need careful examination of the result. The second one should always be
correct, but only because it is limited to a much more constrained
problem.
I'm also not sure how useful those really are in practice. I have not
used "union" myself ever. And in the example that started this thread, I
find the use of "union" slightly dubious. I do not even know how it
would react to a line _changing_, or other complicated edit. Short of a
specialized XML-aware merge driver, using XDL_MERGE_ZEALOUS and kicking
the result out to the user (i.e., what the default merge driver does)
seems like the only sane thing, even if it is more work at merge time.
-Peff
[1] Some of this is fairly easy to do with perl one-liners (e.g., "perl
-00 -ne 'print unless $h{$_}++" for paragraph mode), so maybe it is
just an education/documentation issue. I dunno. I have always been
happy enough with the stock merge.
next prev parent reply other threads:[~2013-03-05 17:59 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-04 16:46 auto merge bug David Krmpotic
2013-03-05 9:03 ` Jeff King
2013-03-05 9:12 ` Jeff King
2013-03-05 15:44 ` Junio C Hamano
2013-03-05 17:59 ` Jeff King [this message]
2013-03-05 18:47 ` Junio C Hamano
2013-03-05 20:56 ` Andreas Ericsson
[not found] ` <194F685F-9460-42C6-B5A5-59475F53D038@gmail.com>
2013-03-05 22:13 ` David Krmpotic
2013-03-06 9:15 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130305175904.GC9379@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=david.krmpotic@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).