git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Elijah Newren <newren@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>, Jeff King <peff@peff.net>,
	Phillip Wood <phillip.wood@dunelm.org.uk>,
	Linus Nilsson <Linus.Nilsson@trimma.se>
Subject: Re: [BUG] All files in folder are moved when cherry-picking commit that moves fewer files
Date: Thu, 07 Mar 2019 14:45:25 +0900	[thread overview]
Message-ID: <xmqqy35rme4q.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <CABPp-BF=GFgFRfgdG2rFK89x1tYrJO8TpECyv6BoTgz9q3Rb+g@mail.gmail.com> (Elijah Newren's message of "Wed, 6 Mar 2019 20:14:37 -0800")

Elijah Newren <newren@gmail.com> writes:

> I'm not sure I'm following well enough to answer any your question(s),
> so I'll instead ask a few of my own:
>   * Are you referring to stage #2 and stage #3 of the old path or the
> new path or one of each or both of each?

If they moved the sole neighbouring file X/A to Y/A, while we added
a new path to X/B, I would expect we'd have two blobs with the same
content we added, one at X/B at stage #2 (because that is what we
wanted to have) and the other at Y/B at stage #3 (that's their wish
the heuristic is guessing).  If it were the other way, the stages
would be different.

>   * Would this be consistent with how normal renames behave?
> and, as a partial answer to your question:

The use of the three-way merge logic at the tree structure level,
unlike the "because neighbours moved" heuristics, gives a more
definite answer.  We see that the contents in the original at path A
(in the common ancestor version) is now at path B (with or without
slight modification) in their tree, while we kept it at path A
(again, with or without modification).  Or vice versa.  Either way,
at the tree structure level, that is "one side changes, while the
other side does not modify" which is resolved cleanly to "take the
new path the side that modifies wants the file to have".  So we do
not need to use the stages to represent "these sides wants to place
it at different paths, which one is correct?"  Instead, we'd have
stages #1 (common ancestor), #2 (ours) and #3 (theirs) all at path B
(i.e. the cleanly resolved structural change conflict) to help the
user reconcile content level conflicts (iff needed).


> I would prefer to tell the user they can resolve the conflict with two
> other commands.  I'm currently thinking of something like these two:
>     git add $directory_renamed_path
>     git mv $directory_renamed_path $original_path [&& git add $original_path ?]
> This pair assumes we write the working tree and index entries for
> $directory_renamed_path and leave $original_path empty in both the
> working tree and index.  We could alternatively reverse that, with an
> equivalent but swapped set of commands for resolution.

Yeah, the final version may have difference in detail, but I think
it is sensible to leave a state, from which the user can go in
either direction, with a simple command or two, like the above one.

>> ...  With that
>> attitude, we may be able to get rid of choice (a) altogether, which
>> may make things a tad simpler.  IOW, we assume the heuristics would
>> suggest the right solution most of the time, just like we assume
>> rerere gives a reasonable resolution most of the time, with a knob
>> to accept the result blindly, together with hints to recover when
>> heuristics makes mistakes.
>
> I like that analogy.  I think that aligns well with my possible
> conflict-resolution commands, and further answers the question about
> whether to put the new path or the old path in the working tree and
> index.
>
> However, we do need some form of choice (a) for commands which can't
> provide accurate tree information, such as git-am.

Yeah, if we are not even running the heuristics for that case (a),
hence it is not sensible to expect to offer "if we were doing
directory renames, this path might have gone to that other place",
then I think I am fine with not giving any hint to change their mind
(after all, they can "git reset --hard" and retry the merge, this
time not using the non-default (a) mode).  And keeping around the
simplest "we do not even attempt to" (a) mode as another option is
fine by me.

Thanks.

  parent reply	other threads:[~2019-03-07  5:45 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-27 12:47 [BUG] All files in folder are moved when cherry-picking commit that moves fewer files Linus Nilsson
2019-02-27 14:30 ` Phillip Wood
2019-02-27 16:02   ` Elijah Newren
2019-02-27 16:40     ` Jeff King
2019-02-27 17:31       ` Elijah Newren
2019-02-28  8:16         ` Linus Nilsson
2019-03-01  2:52         ` Junio C Hamano
2019-03-02 23:48           ` Elijah Newren
2019-03-03  1:33             ` Junio C Hamano
2019-03-06  0:27               ` Elijah Newren
2019-03-06  4:43                 ` Junio C Hamano
2019-03-07  4:14                   ` Elijah Newren
2019-03-07  5:45                     ` Junio C Hamano
2019-03-07  5:45                     ` Junio C Hamano [this message]
2019-03-30  0:33                 ` [PATCH v2 00/15] Switch directory rename detection default Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 01/15] Use 'unsigned short' for mode, like diff_filespec does Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 02/15] merge-recursive: rename merge_options argument from 'o' to 'opt' Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 03/15] merge-recursive: rename diff_filespec 'one' to 'o' Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 04/15] merge-recursive: rename locals 'o' and 'a' to 'obuf' and 'abuf' Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 05/15] merge-recursive: use 'ci' for rename_conflict_info variable name Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 06/15] merge-recursive: move some struct declarations together Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 07/15] merge-recursive: shrink rename_conflict_info Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 08/15] merge-recursive: remove ren[12]_other fields from rename_conflict_info Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 09/15] merge-recursive: track branch where rename occurred in rename struct Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 10/15] merge-recursive: cleanup handle_rename_* function signatures Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 11/15] merge-recursive: switch from (oid,mode) pairs to a diff_filespec Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 12/15] t6043: fix copied test description to match its purpose Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 13/15] merge-recursive: track information associated with directory renames Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 14/15] merge-recursive: give callers of handle_content_merge() access to contents Elijah Newren
2019-03-30  0:33                   ` [PATCH v2 15/15] merge-recursive: switch directory rename detection default Elijah Newren
2019-03-30  9:12                     ` Ævar Arnfjörð Bjarmason
2019-04-01 15:41                       ` Elijah Newren
2019-04-05 15:00                   ` [PATCH v3 00/15] Switch " Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 01/15] Use 'unsigned short' for mode, like diff_filespec does Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 02/15] merge-recursive: rename merge_options argument from 'o' to 'opt' Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 03/15] merge-recursive: rename diff_filespec 'one' to 'o' Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 04/15] merge-recursive: rename locals 'o' and 'a' to 'obuf' and 'abuf' Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 05/15] merge-recursive: use 'ci' for rename_conflict_info variable name Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 06/15] merge-recursive: move some struct declarations together Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 07/15] merge-recursive: shrink rename_conflict_info Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 08/15] merge-recursive: remove ren[12]_other fields from rename_conflict_info Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 09/15] merge-recursive: track branch where rename occurred in rename struct Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 10/15] merge-recursive: cleanup handle_rename_* function signatures Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 11/15] merge-recursive: switch from (oid,mode) pairs to a diff_filespec Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 12/15] t6043: fix copied test description to match its purpose Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 13/15] merge-recursive: track information associated with directory renames Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 14/15] merge-recursive: give callers of handle_content_merge() access to contents Elijah Newren
2019-04-05 15:00                     ` [PATCH v3 15/15] merge-recursive: switch directory rename detection default Elijah Newren
2019-04-05 16:32                     ` [PATCH v3 00/15] Switch " Jacob Keller

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=xmqqy35rme4q.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=Linus.Nilsson@trimma.se \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=phillip.wood@dunelm.org.uk \
    /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).