All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/2] ll_binary_merge(): handle XDL_MERGE_FAVOR_UNION
Date: Fri, 11 Jun 2021 12:36:10 +0900	[thread overview]
Message-ID: <xmqq4ke55az9.fsf@gitster.g> (raw)
In-Reply-To: <YMIMIdnDB1Jq+lzp@coredump.intra.peff.net> (Jeff King's message of "Thu, 10 Jun 2021 08:57:05 -0400")

Jeff King <peff@peff.net> writes:

> Prior to commit a944af1d86 (merge: teach -Xours/-Xtheirs to binary
> ll-merge driver, 2012-09-08), we always reported a conflict from
> ll_binary_merge() by returning "1" (in the xdl_merge and ll_merge code,
> this value is the number of conflict hunks). After that commit, we
> report zero conflicts if the "variant" flag is set, under the assumption
> that it is one of XDL_MERGE_FAVOR_OURS or XDL_MERGE_FAVOR_THEIRS.
>
> But this gets confused by XDL_MERGE_FAVOR_UNION. We do not know how to
> do a binary union merge, but erroneously report no conflicts anyway (and
> just blindly use the "ours" content as the result).
>
> Let's tighten our check to just the cases that a944af1d86 meant to
> cover. This fixes the union case (which existed already back when that
> commit was made), as well as future-proofing us against any other
> variants that get added later.

Makes sense.

> Note that you can't trigger this from "git merge-file --union", as that
> bails on binary files before even calling into the ll-merge machinery.
> The test here uses the "union" merge attribute, which does erroneously
> report a successful merge.

Nice discovery.

Thanks.

  reply	other threads:[~2021-06-11  3:36 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-10 12:51 [PATCH 0/2] fix union merge with binary files Jeff King
2021-06-10 12:57 ` [PATCH 1/2] ll_binary_merge(): handle XDL_MERGE_FAVOR_UNION Jeff King
2021-06-11  3:36   ` Junio C Hamano [this message]
2021-06-10 12:58 ` [PATCH 2/2] ll_union_merge(): pass name labels to ll_xdl_merge() Jeff King
2021-06-10 15:19   ` Elijah Newren
2021-06-10 16:14     ` [PATCH 3/2] ll_union_merge(): rename path_unused parameter Jeff King
2021-06-10 15:19 ` [PATCH 0/2] fix union merge with binary files Elijah Newren

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=xmqq4ke55az9.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.