git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Yuri <yuri@rawbw.com>, Git Mailing List <git@vger.kernel.org>
Subject: Re: 'git pull' complains that a locally resurrected directory would be overwritten by merge when no pulled changes are affecting that directory
Date: Wed, 25 Aug 2021 08:42:09 -0700	[thread overview]
Message-ID: <CABPp-BEMXW3EOdT4jt1g63uPyZ5YuKUPfBE9BL=E66QcT5uXXA@mail.gmail.com> (raw)
In-Reply-To: <YSWXSWiDWNU93lhC@coredump.intra.peff.net>

On Tue, Aug 24, 2021 at 6:09 PM Jeff King <peff@peff.net> wrote:
>
> On Tue, Aug 24, 2021 at 09:41:49AM -0700, Yuri wrote:
>
> > In the FreeBSD ports repository I resurrected the directory (that was
> > removed a long time ago) with the command:
> >
> > > $ git checkout {hash}~1 -- math/polymake
> >
> >
> > I made local changes to this directory and called 'git add math/polymake'.
> >
> > Then 'git pull' complained:
> >
> > > $ git pull
> > > error: Your local changes to the following files would be overwritten by
> > merge:
> > >   math/polymake/Makefile math/polymake/distinfo
> > math/polymake/files/patch-Makefile
> > math/polymake/files/patch-support_install.pl math/polymake/pkg-descr
> > math/polymake/pkg-plist
> >
> >
> > No incoming changes affect math/polymake. Nobody has created this directory
> > simultaneously with me. There is no intersection with incoming changes.
> >
> >
> > Why does 'git pull' complain then?
>
> It's hard to say without seeing a full example. The merge machinery is
> supposed to handle this situation (and indeed, in a simple reproduction
> example it does). So presumably it thinks the other side may have
> touched those files for some reason. Perhaps it's confused by rename
> detection?
>
> Is it possible to give us a more complete example, including:
>
>   - a url for the repository
>   - the commit at HEAD when you ran "git checkout"
>   - the {hash} commit from which you rescued the files
>   - the state of the remote branch (i.e., what we attempted to merge
>     with "git pull")
>
> ?

The `git checkout {hash}~1 -- math/polymake` is enough to highlight
that Yuri doesn't just have local changes (which the merge machinery
should allow if the incoming changes don't touch the same files), but
local *staged* changes.  As per the merge manpage:

"""
To avoid recording unrelated changes in the merge commit, git pull and
git merge will also abort if there are any changes registered in the
index relative to the HEAD commit.
"""

While this particular example could theoretically be handled by the
merge machinery without requiring the index match HEAD, at least with
the new ort backend (it'd be way too complex with the recursive
backend, see below), in practice I'm so sick of index != HEAD bugs
that I have zero motivation to even consider it and I might even NAK
patches from others who attempted it.  For why, let me quote from the
commit message of 9822175d2b ("Ensure index matches head before
invoking merge machinery, round N", 2019-08-17), which also references
another commit that I'm tempted to also quote:

"""
Ensure index matches head before invoking merge machinery, round N

This is the bug that just won't die; there always seems to be another
form of it somewhere.  See the commit message of 55f39cf7551b ("merge:
fix misleading pre-merge check documentation", 2018-06-30) for a more
detailed explanation), but in short:

<quick summary>

builtin/merge.c contains this important requirement for merge
strategies:

    ...the index must be in sync with the head commit.  The strategies are
    responsible to ensure this.

This condition is important to enforce because there are two likely
failure cases when the index isn't in sync with the head commit:

  * we silently throw away changes the user had staged before the merge

  * we accidentally (and silently) include changes in the merge that
    were not part of either of the branches/trees being merged

Discarding users' work and mis-merging are both bad outcomes, especially
when done silently, so naturally this rule was stated sternly -- but,
unfortunately totally ignored in practice unless and until actual bugs
were found.  But, fear not: the bugs from this were fixed in commit
  ee6566e8d70d ("[PATCH] Rewrite read-tree", 2005-09-05)
through a rewrite of read-tree (again, commit 55f39cf7551b has a more
detailed explanation of how this affected merge).  And it was fixed
again in commit
  160252f81626 ("git-merge-ours: make sure our index matches HEAD", 2005-11-03)
...and it was fixed again in commit
  3ec62ad9ffba ("merge-octopus: abort if index does not match HEAD", 2016-04-09)
...and again in commit
  65170c07d466 ("merge-recursive: avoid incorporating uncommitted
changes in a merge", 2017-12-21)
...and again in commit
  eddd1a411d93 ("merge-recursive: enforce rule that index matches head
before merging", 2018-06-30)

...with multiple testcases added to the testsuite that could be
enumerated in even more commits.

Then, finally, in a patch in the same series as the last fix above, the
documentation about this requirement was fixed in commit 55f39cf7551b
("merge: fix misleading pre-merge check documentation", 2018-06-30), and
we all lived happily ever after...

</quick summary>

Unfortunately, "ever after" apparently denotes a limited time and it
expired today.
"""

and then the commit message goes on to explain multiple other bugs
found and fixed due to not strictly enforcing that the index needs to
match HEAD before starting a merge.

  reply	other threads:[~2021-08-25 15:43 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24 16:41 'git pull' complains that a locally resurrected directory would be overwritten by merge when no pulled changes are affecting that directory Yuri
2021-08-25  1:05 ` Jeff King
2021-08-25 15:42   ` Elijah Newren [this message]
2021-08-25 19:05     ` Jeff King
2021-08-25 21:19     ` Junio C Hamano
2021-08-27  1:05       ` Jeff King
2021-08-28  5:21         ` Elijah Newren
2021-08-25  5:43 ` Bagas Sanjaya
2021-08-25  5:47   ` Yuri

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABPp-BEMXW3EOdT4jt1g63uPyZ5YuKUPfBE9BL=E66QcT5uXXA@mail.gmail.com' \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=yuri@rawbw.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).