From: Sergey Organov <sorganov@gmail•com>
To: Tao Klerks <tao@klerks•biz>
Cc: phillip.wood@dunelm•org.uk, Elijah Newren <newren@gmail•com>,
Alex Henrie <alexhenrie24@gmail•com>,
Tao Klerks via GitGitGadget <gitgitgadget@gmail•com>,
git@vger•kernel.org,
Johannes Schindelin <Johannes.Schindelin@gmx•de>
Subject: Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"
Date: Wed, 22 Feb 2023 17:27:15 +0300 [thread overview]
Message-ID: <87a615vkqk.fsf@osv.gnss.ru> (raw)
In-Reply-To: CAPMMpogi_QoGKD824JW+85v_Sgaf5d3TAd_P55YyT5NF6AUJ=w@mail.gmail.com
Tao Klerks <tao@klerks•biz> writes:
> On Sat, Feb 18, 2023 at 5:39 PM Phillip Wood <phillip.wood123@gmail•com> wrote:
>>
>> On 18/02/2023 03:17, Elijah Newren wrote:
[...]
>> > My personal opinion would be adding such a capability should be step
>> > 2.5 in your list, though I suspect that would make Tao unhappy (it's a
>> > non-trivial amount of work, unlike the other steps in your list).
>>
>> I've got a couple of patches[1] that cherry-pick the merge if only one
>> of the parents has changed. I've never tried upstreaming them as it is
>> only a partial solution to the problem of rebasing merges but that
>> approach should work well with "git pull --rebase=merges" as only the
>> upstream side will have changed (when rebasing my git integration branch
>> with that patch the merges are cherry-picked). They might make a useful
>> starting point if anyone wants to try and improve the rebasing of merges.
>>
>
> This is awesome!
>
> It feels like the first step towards the general strategy that was (I
> believe) best described by Buga at
> https://public-inbox.org/git/a0cc88d2-bfed-ce7b-1b3f-3c447d2b32da@gmail.com/
> !
Being the provoker of all the fuss then, as well as the author of basic
original method, I agree Buga has summarized and described all the ideas
in existence at that time extremely well.
>
> (unless I'm missing something, the result of this is exactly the same
> as the result of that strategy, in these "simple" cases where it kicks
> in)
>
> The one concern I have with this is that, *if I understand correctly*,
> it sometimes throws away the existing merge information, and sometimes
> doesn't, and there's no easy way to know which it is at runtime.
As far as I'm aware, it's not the case. The originally described method
indeed misbehaved, but this simple mistake has been quickly fixed, and
the description by Buga you've referenced already discusses updated
version.
> Would adding a warning on stderr when a both-parents merge is
> encountered (and any merge resolutions or related changes are still
> discarded) be enough to make this shippable?
Even if there are in fact such corner cases, we could make ourselves
very cautious and stop even after non-conflicting rebase, if we detect
that U1' and U2' don't match, and let user decide if the result is
acceptable (similar to what rerere does on successful application of
replayed resolutions).
I also agree (in particular with Buga) that from the POV of user
experience the method suggested by Phillip should be superior, as it
emphasizes the natural dominance of the "current branch", as opposed to
originally described symmetric method that is more suitable for formal
analysis than for actual convenient implementation. Yet creating U1' and
U2' from the original method could be useful for the purpose of checking
for possible problems with automatic rebase that the user may need to be
aware of.
The biggest problem here, as I see it, is designing UI that'd make sense
in the case of conflicts in multiple stages of the suggested algorithms,
but I think we can simplify it for now by stopping and suggesting blind
re-merge in case of any conflict but that on rebasing of changes to the
first parent. Even this would be a huge step forward compared to silent
drop of merge commits and blindly re-merging of updated parents.
>
> Are there *any* circumstances where the new cherry-picking behavior
> introduced here wouldn't be the right thing to have happen?
None that I'm aware off, but I admit I'm not familiar with later Elijah
work on the subject, so I could be mistaken. I only got a sketchy look
at what Elijah did, and it looks like advanced material to me. I'd
incline to rather get solid implementation of basics first, probably
using Phillip method, then consider advanced methods if practice reveals
demands for further improvements.
I'm afraid that there is no ideal general solution for the problem of
rebasing merge commits, so we need to limit ourselves and get a
practical one that has already been described.
Overall, I'd love to finally have reliable Git behavior when rebasing
merge commits, even though I've already got a habit to perform all the
merges in 2 steps: auto-merge resolving textual conflicts only (if any),
followed by a fixup for semantics conflicts (if any).
Thanks,
-- Sergey Organov
next prev parent reply other threads:[~2023-02-22 14:27 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-05 16:24 [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true" Tao Klerks via GitGitGadget
2023-02-16 3:22 ` Alex Henrie
2023-02-16 12:31 ` Tao Klerks
2023-02-17 3:15 ` Alex Henrie
2023-02-17 11:15 ` Tao Klerks
2023-02-17 18:56 ` Alex Henrie
2023-02-17 17:39 ` Junio C Hamano
2023-02-18 3:17 ` Elijah Newren
2023-02-18 16:39 ` Phillip Wood
2023-02-20 8:03 ` Tao Klerks
2023-02-20 16:45 ` Phillip Wood
2023-02-20 16:56 ` Elijah Newren
2023-02-21 14:04 ` Tao Klerks
2023-02-22 14:27 ` Sergey Organov [this message]
2023-02-24 7:06 ` Elijah Newren
2023-02-24 22:06 ` Sergey Organov
2023-02-24 23:59 ` Elijah Newren
2023-02-25 15:15 ` Sergey Organov
2023-02-25 16:28 ` Elijah Newren
2023-02-26 9:29 ` Sergey Organov
2023-02-27 15:20 ` Elijah Newren
2023-02-27 17:17 ` Sergey Organov
2023-02-28 2:35 ` Elijah Newren
2023-02-20 16:46 ` Elijah Newren
2023-02-20 6:01 ` Tao Klerks
2023-02-20 17:20 ` Elijah Newren
2023-02-20 18:33 ` Alex Henrie
2023-02-21 15:40 ` Tao Klerks
2023-02-21 17:45 ` Alex Henrie
2023-02-21 15:01 ` Tao Klerks
2023-02-24 7:06 ` Elijah Newren
2023-02-28 14:13 ` Felipe Contreras
2023-02-28 20:04 ` Alex Henrie
2023-03-01 12:46 ` Felipe Contreras
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=87a615vkqk.fsf@osv.gnss.ru \
--to=sorganov@gmail$(echo .)com \
--cc=Johannes.Schindelin@gmx$(echo .)de \
--cc=alexhenrie24@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitgitgadget@gmail$(echo .)com \
--cc=newren@gmail$(echo .)com \
--cc=phillip.wood@dunelm$(echo .)org.uk \
--cc=tao@klerks$(echo .)biz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox