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: Junio C Hamano <gitster@pobox.com>, 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: Fri, 27 Aug 2021 22:21:25 -0700	[thread overview]
Message-ID: <CABPp-BEt=wBMZHP1P+sy1+GPhEgynu-RoUs2ZC7ZuWspfCsTVQ@mail.gmail.com> (raw)
In-Reply-To: <YSg6dshmB5JA7KZO@coredump.intra.peff.net>

On Thu, Aug 26, 2021 at 6:05 PM Jeff King <peff@peff.net> wrote:
>
> On Wed, Aug 25, 2021 at 02:19:15PM -0700, Junio C Hamano wrote:
>
> > Elijah Newren <newren@gmail.com> writes:
> >
> > > ... 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,...
> >
> > While I do not mind seeing a patch that loosens the condition ONLY
> > when the merge will cleanly auto-resolve without end-user
> > interaction, when any paths conflict and require editing by the
> > end-user, it is pretty much essential to require that the index
> > matches HEAD to keep "git merge" usable.
> >
> > This is because the final step to conclude such an "automated
> > procedure cannot cleanly resolve, so the end user helps resolving
> > with the editor and mark the resolved paths with 'git add' or 'git
> > rm'" session will become very error prone if we did not have the
> > requirement.  Not just the user MUST remember not to use "commit -a"
> > or "git add" a path that was already dirty in the working tree
> > before the merge started (which is the consequence of the current
> > requirement, which allows local changes to the unrelated working
> > tree files), they must MUST remember to somehow EXCLUDE the changes
> > already registered for unrelted paths from the concluded merge.
>
> Good point.
>
> In theory we could reject the merge only after finding that there were
> conflicts. That lets the happy path continue even with unrelated changes
> (just like a fast-forward does). I suspect we'd need to change the
> interface to the merge-backends, though (to say "do the merge, and bail
> on conflicts, but _don't_ write out conflict markers or touch any other
> state in that case").

We actually wouldn't need to change the interface.  One of the reasons
there were so many annoying bugs with index != HEAD, was that
builtin/merge.c stated that the merge backends were responsible for
enforcing that condition.  So every single merge backend had to be
individually fixed -- ours, octopus, resolve, recursive.  And then
some of those, like recursive, had multiple affected code paths, and
each one had to be fixed, and I didn't catch them all the first time.

That doesn't mean I'm in favor of the change, just pointing out that
one backend could decide to do this on its own even if the other
backends didn't support it.

> (I am just thinking out loud, though. My personal opinion is that if you
> have a bunch of staged changes and want to do any non-trivial merging,
> you should considering committing or stashing those changes).

Agreed.

Though perhaps I should have pointed out to Yuri that he wouldn't have
run into these problems if he would have used
    git restore --source={hash}~1 -- math/polymake
instead of
    git checkout {hash}~1 -- math/polymake

(since the former defaults to updating just the working directory,
while the later updates both the index and the working directory.)

  reply	other threads:[~2021-08-28  5:22 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
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 [this message]
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-BEt=wBMZHP1P+sy1+GPhEgynu-RoUs2ZC7ZuWspfCsTVQ@mail.gmail.com' \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --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).