git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: BUG?: xdl_merge surprisingly does not recognize content conflict
Date: Fri, 16 Aug 2019 12:26:06 -0700	[thread overview]
Message-ID: <CABPp-BF_PdzOvsgUQUtQRgiQe1qQ_O9Pn2rbpwJ095e7VA2xKA@mail.gmail.com> (raw)
In-Reply-To: <xmqqh86h12au.fsf@gitster-ct.c.googlers.com>

On Fri, Aug 16, 2019 at 9:51 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > Now, a manual merge of these files gives no conflicts, which surprises me:
> >
> >   $ git merge-file ours base theirs; echo $?
> >   0
>
> Indeed that is surprising.
>
> > -- 8< --
> > Subject: checkout: remove duplicate code
> >
> > Both commit a7256debd4b6 ("checkout.txt: note about losing staged
> > changes with --merge", 2019-03-19) from nd/checkout-m-doc-update and
> > commit 6eff409e8a76 ("checkout: prevent losing staged changes with
> > --merge", 2019-03-22) from nd/checkout-m were included in git.git
> > despite the fact that the latter was meant to be v2 of the former.
> > The merge of these two topics resulted in a redundant chunk of code;
> > remove it.
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
>
> The latter half that you remove cannot be reached, because the first
> half would have already died after checking the same condition, so
> we were doing the same repo-index-has-changes check twice when there
> is no change; in other words, we were lucky that this accidental dup
> did not cause any real damange.

Yes, but I am wondering if there are other cases we just don't know
about yet and for which we did not get lucky.

I dug a little further and found that even the first version of `git
merge-file` that Dscho added to git in 2006 also exhibited the same
behavior.  Since he was basically making a minimal replacement for RCS
merge, I tried out /usr/bin/merge from the rcs package and found it
has the same behavior.  I then downloaded the oldest tarball on the
GNU ftp site I could find for rcs (5.7 from June 1995) and found that
its' merge command had the same behavior.

I'm not sure if that alarms me (not only have all versions of git
potentially been mis-merging things, but all versions of rcs, cvs, and
probably any other version control system out there in current use),
or comforts me ("hey, it's been around for at least 24 years so it
must only mis-merge things in both rare and innocuous ways or else
someone would have noticed it decades ago")

I'm leaning towards "It can't be that bad; I'll look at it again later
if someone else who knows xdiff better hasn't already solved it"

  reply	other threads:[~2019-08-16 19:26 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-15 22:03 Elijah Newren
2019-08-15 22:06 ` Elijah Newren
2019-08-16 16:51 ` Junio C Hamano
2019-08-16 19:26   ` Elijah Newren [this message]
2019-08-16 18:40 ` 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=CABPp-BF_PdzOvsgUQUtQRgiQe1qQ_O9Pn2rbpwJ095e7VA2xKA@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --subject='Re: BUG?: xdl_merge surprisingly does not recognize content conflict' \
    /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

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).