[PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Tao Klerks via GitGitGadget]

From: Tao Klerks <tao@klerks.biz>

When "git pull" is called without a conflict-handling instruction or
configuration, it displays a hint proposing "pull.rebase" and "pull.ff"
config options for future handling.

The hint offers three permanent settings, "merge", rebase", and "ff". The
proposed command for "rebase" is "git config pull.rebase true".

Unfortunately, this rebase configuration can easily lead to non-expert users
accidentally rebasing not their own commits, instead others' commits, if the
new commits they have locally before the "pull" include a merge of another
branch, eg "main".

Since 2018 in git version "2.18", it has supported a new rebase flag
"--rebase-merges", with corresponding pull.rebase config option "merges".
This new option is ideal for rebasing local work on "pull", as it will
not "mangle"/flatten any local merge commits but rather recreate them.

Change the pull conflict hint text to propose "pull.rebase merges" instead
of "pull.rebase true", and "git pull --rebase=merges" instead of
"git pull --rebase".

Signed-off-by: Tao Klerks <tao@klerks.biz>
---
    pull: conflict hint pull.rebase suggestion should offer "merges" vs
    "true"
    
    Hint change as proposed in
    https://lore.kernel.org/git/xmqqa61uo3q0.fsf@gitster.g/

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1474%2FTaoK%2Ftao-fetch-rebase-hint-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1474/TaoK/tao-fetch-rebase-hint-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1474

 builtin/pull.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 1ab4de0005d..535364fbb07 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -967,13 +967,13 @@ static void show_advice_pull_non_ff(void)
 		 "your next pull:\n"
 		 "\n"
 		 "  git config pull.rebase false  # merge\n"
-		 "  git config pull.rebase true   # rebase\n"
+		 "  git config pull.rebase merges # rebase\n"
 		 "  git config pull.ff only       # fast-forward only\n"
 		 "\n"
 		 "You can replace \"git config\" with \"git config --global\" to set a default\n"
-		 "preference for all repositories. You can also pass --rebase, --no-rebase,\n"
-		 "or --ff-only on the command line to override the configured default per\n"
-		 "invocation.\n"));
+		 "preference for all repositories. You can also pass --rebase=merges,\n"
+		 "--no-rebase, or --ff-only on the command line to override the configured\n"
+		 "default per invocation.\n"));
 }
 
 int cmd_pull(int argc, const char **argv, const char *prefix)

base-commit: a6a323b31e2bcbac2518bddec71ea7ad558870eb
-- 
gitgitgadget

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Alex Henrie]

On Sun, Feb 5, 2023 at 9:41 AM Tao Klerks via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Tao Klerks <tao@klerks.biz>
>
> When "git pull" is called without a conflict-handling instruction or
> configuration, it displays a hint proposing "pull.rebase" and "pull.ff"
> config options for future handling.
>
> The hint offers three permanent settings, "merge", rebase", and "ff". The
> proposed command for "rebase" is "git config pull.rebase true".
>
> Unfortunately, this rebase configuration can easily lead to non-expert users
> accidentally rebasing not their own commits, instead others' commits, if the
> new commits they have locally before the "pull" include a merge of another
> branch, eg "main".
>
> Since 2018 in git version "2.18", it has supported a new rebase flag
> "--rebase-merges", with corresponding pull.rebase config option "merges".
> This new option is ideal for rebasing local work on "pull", as it will
> not "mangle"/flatten any local merge commits but rather recreate them.
>
> Change the pull conflict hint text to propose "pull.rebase merges" instead
> of "pull.rebase true", and "git pull --rebase=merges" instead of
> "git pull --rebase".
>
> Signed-off-by: Tao Klerks <tao@klerks.biz>
> ---
>     pull: conflict hint pull.rebase suggestion should offer "merges" vs
>     "true"
>
>     Hint change as proposed in
>     https://lore.kernel.org/git/xmqqa61uo3q0.fsf@gitster.g/
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1474%2FTaoK%2Ftao-fetch-rebase-hint-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1474/TaoK/tao-fetch-rebase-hint-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1474
>
>  builtin/pull.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 1ab4de0005d..535364fbb07 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -967,13 +967,13 @@ static void show_advice_pull_non_ff(void)
>                  "your next pull:\n"
>                  "\n"
>                  "  git config pull.rebase false  # merge\n"
> -                "  git config pull.rebase true   # rebase\n"
> +                "  git config pull.rebase merges # rebase\n"
>                  "  git config pull.ff only       # fast-forward only\n"
>                  "\n"
>                  "You can replace \"git config\" with \"git config --global\" to set a default\n"
> -                "preference for all repositories. You can also pass --rebase, --no-rebase,\n"
> -                "or --ff-only on the command line to override the configured default per\n"
> -                "invocation.\n"));
> +                "preference for all repositories. You can also pass --rebase=merges,\n"
> +                "--no-rebase, or --ff-only on the command line to override the configured\n"
> +                "default per invocation.\n"));

Hi Tao, thank you for sharing your experiences with non-experts using
`git pull`. I am always curious to see how people who are learning Git
react to it, and I am very interested in making Git as straightforward
as possible.

I'm afraid I have several objections to this patch...

- The proposed wording is likely to further confuse novices. It's
asking the user to choose between the reconciliation strategies of
merging and rebasing, but then says to use the unintuitive combination
"rebase=merges" which sounds like it's going to make a merge commit at
the end of the branch anyway.

- The proposed wording makes it sound like there's something wrong
with doing a regular rebase, but that's not usually the case because
in practice a regular rebase is almost always equivalent to
rebase=merges. A regular rebase may even be what the user really
wants: For example, the user might choose to merge when pulling and
then change their mind and decide that they really wanted to rebase.
Repeating the pull with the regular -r or --rebase flag fixes the
mistake.

- `git pull -ri` (or its longer form `git pull --rebase=interactive`)
is generally more useful than `git pull --rebase=merges`, but once
rebase=merges has been specified, there's no way to specify
rebase=interactive also. Recommending rebase=merges steers people away
from rebase=interactive, hiding useful functionality from the user.

Now, this is not to say that there's no room for improvement. I like
the rebase=merges option and I wish everyone knew about it because
there are situations where it really is the best option. I suggest
leaving the existing text alone, but adding an additional paragraph,
something like:

Note that --rebase or pull.rebase=true will drop existing merge
commits and rebase all of the commits from all of the merged branches.
If you want to rebase but preserve existing merge commits, use
--rebase=merges or pull.rebase=merges instead.

-Alex

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Tao Klerks]

On Thu, Feb 16, 2023 at 4:22 AM Alex Henrie <alexhenrie24@gmail.com> wrote:
>
> - The proposed wording is likely to further confuse novices. It's
> asking the user to choose between the reconciliation strategies of
> merging and rebasing, but then says to use the unintuitive combination
> "rebase=merges"

My thesis, which you clearly disagree with, is that for this type of
situation, "rebase=merges" is not an "unintuitive combination", but
rather is "a plain and simple rebase". It is truly unfortunate that
git's history has led us to a place where this command is so awkwardly
named, I agree with that at least.

If there's an appetite for it, I would love to contribute to a
multi-year adventure to change git's behavior, little by little, until
the behavior of "rebase=merges" is the default, and the old behavior
becomes a different option like
"rebase=copy-merged-commits-to-flatten"

> which sounds like it's going to make a merge commit at
> the end of the branch anyway.

I can't quite tell whether you're referring to the naming of the
option (which I agree, sucks), or saying that it sounds *to you* like
it will make a merge commit. It will not make a merge commit unless
*you* previously made a merge commit. It will rebase your merge
commits, only if there are any that should be rebased.

If your concern is that we shouldn't be showing anyone the
"consistently reasonable rebase option" because it's confusingly named
wrt the "rebase option that experts understand and has a shorter
name", then let's figure out how to rename it. In the meantime, let's
help avoid people shooting themselves in the foot. A hint pointing at
the cool loaded gun lying on the mantelpiece is *not* helping users
avoid shooting themselves in the foot.

>
> - The proposed wording makes it sound like there's something wrong
> with doing a regular rebase, but that's not usually the case because
> in practice a regular rebase is almost always equivalent to
> rebase=merges.

The new proposed option will do the right thing in (almost?) all
cases. The previous option will make a horrible mess of things in some
(or, depending on the workflow, many!) cases.

In all the cases where the behavior is equivalent, that's great. In
almost all the cases where the behavior is different, the proposed new
behavior is superior (to anyone who needs that hint).

There is only one case that I know of in which the proposed new
behavior could be considered marginally worse:

* I have a local branch "feature-1", with some local work on it
* This is a shared branch, others are working on it also, so
"origin/feature-1" has a few other commits on it also (diverged)
* I create a derived branch, "feature-1-sub". I do some work on it
* I pull with rebase (--rebase or --rebase=merges, makes no
difference), so my original local feature-1 changes are rebased on top
of others' changes.
* I do some more work on (my local, rebased) "feature-1"
* I merge "feature-1-sub" into "feature-1"
** -> I now have duplicate commits in my history: the original local
"feature-1" work is in the history of "feature-1-sub", and it was/is
separately rebased in "feature-1"
* I "git pull --rebase" or  "git pull --rebase=merges":
** A "simple rebase" will automatically squash the duplicated commit(s)
** A "rebase with rebase-merges" will retain/recreate the merge
commit, and thereby retain the existence of a duplicated commit in the
commit graph.
(test script available upon request, I didn't want to spam the list with it)

While I agree "--rebase=merges" is not clearly superior, or could even
be worse in this one contrived case, I would argue that this is far
less harmful than the current "pathological case" of "--rebase", which
will happen far more easily and which I will outline again:

* There is a "main" branch with lots of commit activity from lots of
contributors
* There is a "feature-1" branch with a few contributors collaborating
on something that will be merged into "main" when ready
* These contributors are not experts - they don't coordinate on
rebasing "feature-1" every time they need to incorporate changes from
"main" - instead, they merge "main" into "feature-1" when that's
needed
* One of those contributors is tasked with doing the merge from main,
resolving conflicts, spot-checking, etc - the last merge from main was
10 days ago, 100 commits.
* They have a merge commit ready, tested
* They try to push "feature-1", but one of the other contributors has
added some work/commits, so the error tells them to "git pull" first
* They "git pull", get an error and follow the wrong advice, or they
followed the wrong advice in the preceding days/weeks - they end up
doing a "git pull --rebase" without knowing what that means for their
recent merge commit, and/or without even realizing that's how they
previously configured things
* Their "feature-1" branch now has 100 duplicated commits by arbitrary
"main" developers, but they haven't even necessarily noticed - they
may well be using a GUI that just congratulates them on a successful
pull
* They now push, successfully.
* Unless anyone looks at the commit graph of "feature-1" carefully,
no-one necessarily notices anything is wrong; the changes from "main"
are in "feature-1", as expected; it's just the lines connecting them
that are wrong
* Two weeks later, the team needs to merge in "main" again - and they
start to get all sorts of weird conflicts
** "Oh man, git sucks - I thought it was supposed to merge *better*
than that other stuff, but it's finding conflicts that have nothing to
do with us, all over the place!"
** "Hmm, this is weird - let's see what the git expert says"
** "Oh man, 'feature-1' needs to be rebuilt, and everyone working on
it needs to figure out how to rebase their work / their branch(es)
onto the new state"
** etc

> A regular rebase may even be what the user really
> wants: For example, the user might choose to merge when pulling and
> then change their mind and decide that they really wanted to rebase.
> Repeating the pull with the regular -r or --rebase flag fixes the
> mistake.

I don't understand the relevance of this example: No-one is suggesting
to forbid "merge-flattening rebases" - only avoiding the suggestion to
use them as the default for people who don't know what they are doing
(that's who the hints are *for*!)

The way you suggest this example, it feels like you think this might
be intuitive/predictable: "I chose the wrong thing, so I flip the
choice and I get the other outcome" - that's not true at all, because
if you flip the choice the other way (--rebase first, and then merge),
you get a completely different outcome! (especially if what you
accidentally rebased contained a merge of course - but even without
merges in the rebased history, doing a merge later does not yield
nearly the same outcome).

>
> - `git pull -ri` (or its longer form `git pull --rebase=interactive`)
> is generally more useful than `git pull --rebase=merges`, but once
> rebase=merges has been specified, there's no way to specify
> rebase=interactive also. Recommending rebase=merges steers people away
> from rebase=interactive, hiding useful functionality from the user.

I don't understand your argument here... Are you saying that users
reading "You can also pass --rebase" would have been more likely to
end up running "--rebase=interactive" than users reading "You can also
pass --rebase=merges"? I believe this to be a grave misreading of user
behavior, but I have no credentials to back up this belief.

People consistently, and unhesitantly, copy-paste the suggestions
offered to them. If you believe users should be running
"--rebase=interactive", then the new wording is no worse than the old.

Now, as to whether users should in fact typically be running
"--rebase=interactive" when doing a "git pull" - is there an option to
"preserve merges" in this interaction? For users who *do not ever
merge* your suggestion sounds... possibly-overbearing, but not wrong.
For users who *do* merge, it is plain wrong as far as I know.

>
> Now, this is not to say that there's no room for improvement. I like
> the rebase=merges option and I wish everyone knew about it because
> there are situations where it really is the best option. I suggest
> leaving the existing text alone, but adding an additional paragraph,
> something like:
>
> Note that --rebase or pull.rebase=true will drop existing merge
> commits and rebase all of the commits from all of the merged branches.
> If you want to rebase but preserve existing merge commits, use
> --rebase=merges or pull.rebase=merges instead.

My primary motivation with this pull request is to reduce the
incidences, out there in the world, of people copy-pasting "git config
pull.rebase true" into their command-line, and causing themselves
major headaches days or weeks later. The "--rebase=interactive" part
is secondary (to my concerns), because it's much less copy-pastable.

Your proposal does nothing for my concern, unfortunately - it leaves a
message that, overall, offers three copy-pastable options, two of
which are safe-enough, and one of which has substantial chances of
plunging you into a world of pain that you cannot comprehend. It is
plain wrong. We need to change it.

I am very happy to add the paragraph you proposed instead of changing
"--rebase" to "--rebase=interactive", but I would like to see a much
better suggestion as to how to address the harm of "git config
pull.rebase true".


Thanks for your feedback, and my apologies for the insistent response
- I'm having a hard time figuring out how to express just how *bad*
the existing copy-pastable suggestion in this hint is (in this day and
age), for users who merge - users who, I believe, make up the
significant majority of "corporate" developers at the very least, and
I suspect even the significant majority of git users out in the world.

I'm adding Johannes Schindelin to the thread in case he has the cycles
to weigh in - as the original author of what I would call "the better
way" (5 years ago now!), I'm sure he's more aware than most of its
limitations, and of any reasons why we *wouldn't* want to make the
change(s) I've suggested here.

Thanks,
Tao

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Alex Henrie]

On Thu, Feb 16, 2023 at 5:31 AM Tao Klerks <tao@klerks.biz> wrote:
>
> If there's an appetite for it, I would love to contribute to a
> multi-year adventure to change git's behavior, little by little, until
> the behavior of "rebase=merges" is the default, and the old behavior
> becomes a different option like
> "rebase=copy-merged-commits-to-flatten"

I know you had a lot to say in your last email, but I'd like to focus
on this point. I would be OK with the proposed patch if it were part
of a larger effort to make --rebase-merges the default behavior of
`git rebase`. That seems like an achievable goal, and I don't think it
would take multiple years, maybe one year at the most. The process
would look something like this:

1. Add a --no-rebase-merges option to `git rebase`.

2. Add a rebase.merges config option.

3. Add a warning to `git rebase` that appears if rebase.merges is
unset and neither --rebase-merges nor --no-rebase-merges is given. The
warning would advise the user that the default behavior of `git
rebase` will change in a future release and suggest setting
rebase.merges=no-rebase-cousins to get the new behavior now.

4. Change the `git pull` advice to recommend --rebase=merges and
pull.rebase=merges.

5. Wait a couple of releases.

6. Change the default behavior of `git rebase` to `git rebase
--rebase-merges` and the default behavior of `git pull --rebase` to
`git pull --rebase=merges`. At the same time, remove the warning from
`git rebase`. The old `git pull` behavior would still be available as
`git pull --rebase=true`.

7. Change the `git pull` advice to recommend the short and simple
--rebase option again (leaving the recommendation of
pull.rebase=merges for the config option).

Does that sound reasonable? I think I could lend a hand with steps 1-3.

-Alex

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Tao Klerks]

On Fri, Feb 17, 2023 at 4:15 AM Alex Henrie <alexhenrie24@gmail.com> wrote:
>
> I would be OK with the proposed patch if it were part
> of a larger effort to make --rebase-merges the default behavior of
> `git rebase`.

Heh, what would it take to convince you there is such an effort? :) -
sparse and minor as my contributions are, I certainly believe that is
a "natural" effort that I will do what I can to support.

> That seems like an achievable goal, and I don't think it
> would take multiple years, maybe one year at the most.

My estimate is based on the observation that there are still, several
years after --rebase-merges was introduced, git GUIs that don't handle
it right - eg Jetbrains IDEA:
https://youtrack.jetbrains.com/issue/IDEA-232160/Rebase-merges-is-not-properly-supported

This kind of functionality change should be slow, not because it's a
huge amount of work, but more because it takes time for the entire
ecosystem to adapt. Git releases basically-monthly, but many of the
systems that users use git with release far less often; similarly,
it's helpful to users who use a mix of current and older systems (I'm
looking at you, CentOS 7) for the introduction and recommendation of a
behavior change to come *long* before its defaulting.

> The process
> would look something like this:
>
> 1. Add a --no-rebase-merges option to `git rebase`.
>
> 2. Add a rebase.merges config option.

Yes and yes! I alluded to this in
https://lore.kernel.org/git/CAPMMpoj6E-85a59EaHD2aR_oKA=_u78qRV+wp8mqXkR39KctmA@mail.gmail.com/
but didn't feel I'd likely to make a solid change along these lines.

>
> 3. Add a warning to `git rebase` that appears if rebase.merges is
> unset and neither --rebase-merges nor --no-rebase-merges is given. The
> warning would advise the user that the default behavior of `git
> rebase` will change in a future release and suggest setting
> rebase.merges=no-rebase-cousins to get the new behavior now.
>

Makes sense to me!

> 4. Change the `git pull` advice to recommend --rebase=merges and
> pull.rebase=merges.
>

I'm not sure why this would be step 4 - I would (and did try to) make
it step 1 :)

> 5. Wait a couple of releases.
>

As I noted above, I believe it should be far more than a couple.

> 6. Change the default behavior of `git rebase` to `git rebase
> --rebase-merges` and the default behavior of `git pull --rebase` to
> `git pull --rebase=merges`. At the same time, remove the warning from
> `git rebase`. The old `git pull` behavior would still be available as
> `git pull --rebase=true`.
>

Makes sense to me!

> 7. Change the `git pull` advice to recommend the short and simple
> --rebase option again (leaving the recommendation of
> pull.rebase=merges for the config option).
>
> Does that sound reasonable? I think I could lend a hand with steps 1-3.
>

I'm sold, except insofar as I think the right approach is to move step
4 to be the first :)

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Alex Henrie]

On Fri, Feb 17, 2023 at 4:15 AM Tao Klerks <tao@klerks.biz> wrote:
>
> On Fri, Feb 17, 2023 at 4:15 AM Alex Henrie <alexhenrie24@gmail.com> wrote:
> >
> > I would be OK with the proposed patch if it were part
> > of a larger effort to make --rebase-merges the default behavior of
> > `git rebase`.
>
> Heh, what would it take to convince you there is such an effort? :)

Doing steps 1-3 :)

> > 4. Change the `git pull` advice to recommend --rebase=merges and
> > pull.rebase=merges.
>
> I'm not sure why this would be step 4 - I would (and did try to) make
> it step 1 :)

The unintuitive syntax --rebase=merges makes a little more sense if
there is a warning in `git rebase` about it being a temporary
necessity to support a planned behavior change, and we're explicitly
committing to not expect users to use that syntax forever. It might be
a good idea to add a similar note to the `git pull` warning too.

-Alex

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Junio C Hamano]

Alex Henrie <alexhenrie24@gmail.com> writes:

> 1. Add a --no-rebase-merges option to `git rebase`.
>
> 2. Add a rebase.merges config option.
>
> 3. Add a warning to `git rebase` that appears if rebase.merges is
> unset and neither --rebase-merges nor --no-rebase-merges is given. The
> warning would advise the user that the default behavior of `git
> rebase` will change in a future release and suggest setting
> rebase.merges=no-rebase-cousins to get the new behavior now.
>
> 4. Change the `git pull` advice to recommend --rebase=merges and
> pull.rebase=merges.
>
> 5. Wait a couple of releases.

The above sounds like a standard "flip the default" dance executed
in the usual order.  I am not sure about the remainder but that is
not because I find anything wrong in it, but because I haven't
thought things through that far into the future ;-).

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Elijah Newren]

On Thu, Feb 16, 2023 at 8:02 PM Alex Henrie <alexhenrie24@gmail.com> wrote:
>
> On Thu, Feb 16, 2023 at 5:31 AM Tao Klerks <tao@klerks.biz> wrote:
> >
> > If there's an appetite for it, I would love to contribute to a
> > multi-year adventure to change git's behavior, little by little, until
> > the behavior of "rebase=merges" is the default, and the old behavior
> > becomes a different option like
> > "rebase=copy-merged-commits-to-flatten"
>
> I know you had a lot to say in your last email, but I'd like to focus
> on this point. I would be OK with the proposed patch if it were part
> of a larger effort to make --rebase-merges the default behavior of
> `git rebase`. That seems like an achievable goal, and I don't think it
> would take multiple years, maybe one year at the most. The process
> would look something like this:
>
> 1. Add a --no-rebase-merges option to `git rebase`.
>
> 2. Add a rebase.merges config option.
>
> 3. Add a warning to `git rebase` that appears if rebase.merges is
> unset and neither --rebase-merges nor --no-rebase-merges is given. The
> warning would advise the user that the default behavior of `git
> rebase` will change in a future release and suggest setting
> rebase.merges=no-rebase-cousins to get the new behavior now.
>
> 4. Change the `git pull` advice to recommend --rebase=merges and
> pull.rebase=merges.
>
> 5. Wait a couple of releases.
>
> 6. Change the default behavior of `git rebase` to `git rebase
> --rebase-merges` and the default behavior of `git pull --rebase` to
> `git pull --rebase=merges`. At the same time, remove the warning from
> `git rebase`. The old `git pull` behavior would still be available as
> `git pull --rebase=true`.
>
> 7. Change the `git pull` advice to recommend the short and simple
> --rebase option again (leaving the recommendation of
> pull.rebase=merges for the config option).
>
> Does that sound reasonable? I think I could lend a hand with steps 1-3.

One concern I have is that "--rebase-merges" itself has negative user
surprises in store.  In particular, "--rebase-merges", despite its
name, does not rebase merges.  It uses the existing author & commit
message info, but otherwise just discards the existing merge and
creates a new one.  Any information it contained about fixing
conflicts, or making adjustments to make the two branches work
together, is summarily and silently discarded.

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

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Phillip Wood]

On 18/02/2023 03:17, Elijah Newren wrote:
> On Thu, Feb 16, 2023 at 8:02 PM Alex Henrie <alexhenrie24@gmail.com> wrote:
>>
>> On Thu, Feb 16, 2023 at 5:31 AM Tao Klerks <tao@klerks.biz> wrote:
>>>
>>> If there's an appetite for it, I would love to contribute to a
>>> multi-year adventure to change git's behavior, little by little, until
>>> the behavior of "rebase=merges" is the default, and the old behavior
>>> becomes a different option like
>>> "rebase=copy-merged-commits-to-flatten"
>>
>> I know you had a lot to say in your last email, but I'd like to focus
>> on this point. I would be OK with the proposed patch if it were part
>> of a larger effort to make --rebase-merges the default behavior of
>> `git rebase`. That seems like an achievable goal, and I don't think it
>> would take multiple years, maybe one year at the most. The process
>> would look something like this:
>>
>> 1. Add a --no-rebase-merges option to `git rebase`.
>>
>> 2. Add a rebase.merges config option.
>>
>> 3. Add a warning to `git rebase` that appears if rebase.merges is
>> unset and neither --rebase-merges nor --no-rebase-merges is given. The
>> warning would advise the user that the default behavior of `git
>> rebase` will change in a future release and suggest setting
>> rebase.merges=no-rebase-cousins to get the new behavior now.
>>
>> 4. Change the `git pull` advice to recommend --rebase=merges and
>> pull.rebase=merges.
>>
>> 5. Wait a couple of releases.
>>
>> 6. Change the default behavior of `git rebase` to `git rebase
>> --rebase-merges` and the default behavior of `git pull --rebase` to
>> `git pull --rebase=merges`. At the same time, remove the warning from
>> `git rebase`. The old `git pull` behavior would still be available as
>> `git pull --rebase=true`.
>>
>> 7. Change the `git pull` advice to recommend the short and simple
>> --rebase option again (leaving the recommendation of
>> pull.rebase=merges for the config option).
>>
>> Does that sound reasonable? I think I could lend a hand with steps 1-3.
> 
> One concern I have is that "--rebase-merges" itself has negative user
> surprises in store.  In particular, "--rebase-merges", despite its
> name, does not rebase merges.  It uses the existing author & commit
> message info, but otherwise just discards the existing merge and
> creates a new one.  Any information it contained about fixing
> conflicts, or making adjustments to make the two branches work
> together, is summarily and silently discarded.

That's a good point. Another potentially surprising behavior is that 
when I'm rebasing an integration branch with -rno-rebase-cousins then if 
one of the topic branches merged into the integration branch happens to 
share the same base as the integration branch itself the topic branch 
gets rebased as well. -rno-rebase-cousins is also slower that it needs 
to be because it creates a todo list that contains all the commits on 
the topic branches merged into the integration branch rather than just 
the merges. The commits on the topic branches are fast-forwarded rather 
than rewritten so long as they don't share the same base as the 
integration branch but it noticeably slower than using a todo list with 
just the merge commands.

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

Best Wishes

Phillip

[1] https://github.com/phillipwood/git/commits/rebase-cherry-pick-merges

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Tao Klerks]

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:
> >
> > One concern I have is that "--rebase-merges" itself has negative user
> > surprises in store.  In particular, "--rebase-merges", despite its
> > name, does not rebase merges.  It uses the existing author & commit
> > message info, but otherwise just discards the existing merge and
> > creates a new one.  Any information it contained about fixing
> > conflicts, or making adjustments to make the two branches work
> > together, is summarily and silently discarded.
>
> That's a good point. Another potentially surprising behavior is that
> when I'm rebasing an integration branch with -rno-rebase-cousins then if
> one of the topic branches merged into the integration branch happens to
> share the same base as the integration branch itself the topic branch
> gets rebased as well.

I've been trying to understand how this behavior is (potentially)
surprising - I imagine it's been discussed elsewhere but I'm having a
hard time understanding, sorry.

The situation you described is a boundary condition between two others, right?
* The topic branch could be branched from the integration branch
(potentially *after* some other change were made to the integration
branch, but not in this case) - in which case rebasing is what you
would expect
* The topic branch could be branched from the main branch (potentially
*before* the integration branch branched, but not in this case) - in
which case not rebasing is what you would expect.

If topic branched from main (at around the same time as integration),
it might be surprising that it rebases; if it branched from
integration (before that had any changes), then it is expected.

> -rno-rebase-cousins is also slower that it needs
> to be because it creates a todo list that contains all the commits on
> the topic branches merged into the integration branch rather than just
> the merges. The commits on the topic branches are fast-forwarded rather
> than rewritten so long as they don't share the same base as the
> integration branch but it noticeably slower than using a todo list with
> just the merge commands.

This seems improvable, but no worse than a plain legacy rebase (as
Alex's new patch would have it, "rebase-merges=drop"), right? Insofar
as we're discussing why it might make sense to avoid promoting this
over a plain rebase, I don't understand the concern.


>
> > 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/
!

(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. 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?

Are there *any* circumstances where the new cherry-picking behavior
introduced here wouldn't be the right thing to have happen?

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Phillip Wood]

Hi Tao

On 20/02/2023 08:03, Tao Klerks wrote:
> 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:
>>>
>>> One concern I have is that "--rebase-merges" itself has negative user
>>> surprises in store.  In particular, "--rebase-merges", despite its
>>> name, does not rebase merges.  It uses the existing author & commit
>>> message info, but otherwise just discards the existing merge and
>>> creates a new one.  Any information it contained about fixing
>>> conflicts, or making adjustments to make the two branches work
>>> together, is summarily and silently discarded.
>>
>> That's a good point. Another potentially surprising behavior is that
>> when I'm rebasing an integration branch with -rno-rebase-cousins then if
>> one of the topic branches merged into the integration branch happens to
>> share the same base as the integration branch itself the topic branch
>> gets rebased as well.
> 
> I've been trying to understand how this behavior is (potentially)
> surprising - I imagine it's been discussed elsewhere but I'm having a
> hard time understanding, sorry.
> 
> The situation you described is a boundary condition between two others, right?
> * The topic branch could be branched from the integration branch
> (potentially *after* some other change were made to the integration
> branch, but not in this case) - in which case rebasing is what you
> would expect
> * The topic branch could be branched from the main branch (potentially
> *before* the integration branch branched, but not in this case) - in
> which case not rebasing is what you would expect.
> 
> If topic branched from main (at around the same time as integration),
> it might be surprising that it rebases;

Yes that's what I was referring to, on the one hand it isn't surprising 
at all because both the topic and integration branch have the same base 
but on the other hand using no-rebase-cousins is supposed to stop the 
topic branches being rebased.

> if it branched from
> integration (before that had any changes), then it is expected.

Yes

>> -rno-rebase-cousins is also slower that it needs
>> to be because it creates a todo list that contains all the commits on
>> the topic branches merged into the integration branch rather than just
>> the merges. The commits on the topic branches are fast-forwarded rather
>> than rewritten so long as they don't share the same base as the
>> integration branch but it noticeably slower than using a todo list with
>> just the merge commands.
> 
> This seems improvable, but no worse than a plain legacy rebase (as
> Alex's new patch would have it, "rebase-merges=drop"), right? Insofar
> as we're discussing why it might make sense to avoid promoting this
> over a plain rebase, I don't understand the concern.

My concern is to have a good understanding of the issues around 
--rebase-merges before we start promoting it over a plain rebase. It is 
not a reason not to make the change but it does show --rebase-merges 
would benefit from some additional polish.

>>> 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/
> !
> 
> (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)

Yes

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

Right, there are two ways the existing merge can be thrown away.

  (i) The existing merge has conflicts when being cherry picked
      and so we redo the merge (that is a choice, we could present
      the user with the conflicts from the cherry-pick). It is
      possible that the merge succeeds where the cherry-pick failed
      but most of the time we'd stop because if the cherry-pick has
      conflicts the merge will probably have conflicts as well.

(ii) More than one parent has changed and so we redo the merge

> 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?

I'm not sure. It works well enough for what I use it for (which is 
essentially "git pull --rebase") but sometimes cherry-picking and 
sometimes remerging does make it more complicated for users. If we 
printed a warning what is the user going to do? An experienced user can 
use the reflog to get back to the original state and redo the rebase 
with some break statements added in to let them fix up the merges. A 
less experienced user is going to think git lost their work.

> Are there *any* circumstances where the new cherry-picking behavior
> introduced here wouldn't be the right thing to have happen?

Not that I can think of

Best Wishes

Phillip

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Elijah Newren]

On Mon, Feb 20, 2023 at 12:03 AM Tao Klerks <tao@klerks.biz> wrote:
>
> 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/
> !

The strategies described by Buga and others in that mega-thread were
suboptimal solutions, in my opinion.  Johannes went and implemented
some and found them wanting; see the thread over at
https://lore.kernel.org/git/nycvar.QRO.7.76.6.1804130002090.65@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz/.
There were follow-ups with an improved strategy in the thread over at
https://lore.kernel.org/git/CABPp-BHWVO5VRhr1-Ou60F1wjKzJZ1e_dC01Mmzs+qB9kGayww@mail.gmail.com/
(Note that this route has also independently been discovered and
implemented in jj and found to work well, though it does handle
conflicts much differently).  And I've since improved the strategy
further at https://github.com/newren/git/blob/e84f5f3585fd770ed21f398d2ae5f96e90a51b1e/replay-design-notes.txt#L264-L341.
However, note that this isn't a case of merely performing the proper
series of merges, it needs some specialized logic and some new
capabilities at the xdiff level.

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Tao Klerks]

On Mon, Feb 20, 2023 at 5:56 PM Elijah Newren <newren@gmail.com> wrote:
>
> The strategies described by Buga and others in that mega-thread were
> suboptimal solutions, in my opinion.  Johannes went and implemented
> some and found them wanting; see the thread over at
> https://lore.kernel.org/git/nycvar.QRO.7.76.6.1804130002090.65@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz/.

Ah, thank you! I think you had mentioned this before, and I somehow
lost track of this.

At first I want to summarize this concern as "any strategy that treats
a merge rebase as a *pair* of cherry-picks risks encountering
nested/overlapping merge conflicts", but I must be understanding too
superficially, as you then mention arbitrary conflict nesting (and I
assume this is not about octopus merges).

> There were follow-ups with an improved strategy in the thread over at
> https://lore.kernel.org/git/CABPp-BHWVO5VRhr1-Ou60F1wjKzJZ1e_dC01Mmzs+qB9kGayww@mail.gmail.com/
> (Note that this route has also independently been discovered and
> implemented in jj and found to work well, though it does handle
> conflicts much differently).  And I've since improved the strategy
> further at https://github.com/newren/git/blob/e84f5f3585fd770ed21f398d2ae5f96e90a51b1e/replay-design-notes.txt#L264-L341.
> However, note that this isn't a case of merely performing the proper
> series of merges, it needs some specialized logic and some new
> capabilities at the xdiff level.

Understood - thanks for the update, and of course for all your
continued work on this.

Is it fair to say that, for the simple situations that Phillip's
cherry-pick strategy *does* kick in for, the outcome should be exactly
the same as the outcome of the replay strategy?

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Sergey Organov]

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

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Elijah Newren]

On Wed, Feb 22, 2023 at 6:27 AM Sergey Organov <sorganov@gmail.com> wrote:
>
> 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:

[...]

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

I'm not so sure it's a huge step forward.  Or even a step forward.

Dscho actually implemented the old proposals and tried them out, as
mentioned in the threads I linked to.  The results on balance were
significantly worse to him than just throwing away the previous merge
resolution information and redoing the merge from scratch.  He really
wanted a better solution, but the previous proposals didn't provide
it.

This newer approximation, while more careful about only attempting to
run in specific cases and having some good ideas to improve the user
experience, still builds on the problematic foundations in those old
suggestions (namely, cherry-picking merges relative to either of their
parents).  I think it isn't careful enough about the subset of cases
where those problematic foundations can work right.

> > Are there *any* circumstances where the new cherry-picking behavior
> > introduced here wouldn't be the right thing to have happen?

Yes, I can think of one fairly readily: Do an interactive rebase and
drop one or more commits on the side-branch being merged.  This
cherry-picking of merges would reinstate those dropped changes via
silently squashing them into the merge commit itself, making for a
rather evil merge.

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

That'd be fine if there's another solution that can provide a "solid
implementation of the basics"; I've not seen another proposal that can
yet.

> I'm afraid that there is no ideal general solution for the problem of
> rebasing merge commits.

Sometimes problems aren't generically solvable.  However, sometimes
the problem is solvable, but folks so far have only provided solutions
built on a faulty basis, or that were only designed for special cases,
or that only look at a subset of the problem space.

I think rebasing merges falls into the latter category, and that the
prior proposals were just off the mark.  Granted, I haven't
implemented my proposal yet and I might discover more issues when I
do, but I'm optimistic.  It just really needs some good uninterrupted
time, and my Git time comes in highly interrupted occasional spurts
these days (and with new short-term priorities being inserted based on
other things that come up on the mailing list and from elsewhere to
boot).  But I'll get to it one way or another.

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Sergey Organov]

Elijah Newren <newren@gmail.com> writes:

> On Wed, Feb 22, 2023 at 6:27 AM Sergey Organov <sorganov@gmail.com> wrote:
>>
>> 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:
>
> [...]
>
>> 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.
>
> I'm not so sure it's a huge step forward.  Or even a step forward.

Git currently throws away my precious merges! Silently! How it's not a
step forward to stop doing this?! Sorry for getting that heated :)

Git is well-known for being extremely careful with user content, and
there is the only case where it fails miserably: rebasing merges. The
above method will simply fix this long-standing deficiency that is even
more dangerous as users do trust Git so much.

I can only tell that I, for example, will definitely benefit a lot once
it is implemented, as currently a rebase containing merge is at roughly
the same level of risk as "cvs update" was in the old days: run and keep
your fingers crossed. Well, with Git it's unless you are careful to do
2-step merge-fixup thingy every time you merge, that is basically just
poor man attempt at fighting long-standing Git weakness.

> Dscho actually implemented the old proposals and tried them out, as
> mentioned in the threads I linked to.  The results on balance were
> significantly worse to him than just throwing away the previous merge
> resolution information and redoing the merge from scratch.  He really
> wanted a better solution, but the previous proposals didn't provide
> it.

OTOH, Buga has sketched the proposals, confirmed problem with my
original one (that Dscho predicted), then sketched the update I came up
with, and showed it does work in common cases as expected.
 
That said, I'm almost sure that for any method of rebasing and/or
merging of whatever, one motivated enough will be able to find corner
cases where the method fails, yet we do have both merges and rebases in
Git, and rebasing of merges falls to the same category. We need them.
Merges need to be properly rebased, not silently replaced with
(different) merges, unless user asks for re-merge explicitly. To me
Dscho (or anybody else) finding rough cases is an expected outcome, and
is not a convincing argument against the feature.

As for Dscho results specifically, I've got an impression that he never
needed rebasing of merges in the first place, and re-merging always
suited him just fine, so it'd be rather a surprise if rebasing of merges
suddenly started to work better for his needs and workflows once he has
implemented it.

That said, when better method(s) of rebasing of merges will be found,
I'm sure they'll be adopted, but for now I do believe we need something
reliable that has been checked to actually work for common cases, as
blind re-merging simply does not, and I still suspect the best choice
for the time being is Phillip's incremental method.

Overall, I'm still in desperate need for my precious merge-the-commits
be rebased, and not replaced with Git idea of how merge commit would
look if [current version of] 'git-merge' algorithm merged my branch in
[using Git current default settings]. It's my dream that Git finally
stops silently substituting a result of 'git-merge'
(just-a-helper-operation intended to simplify creation of merge commits)
for actual merge-the-commit that is part of my content.

Best regards,
-- Sergey Organov

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Elijah Newren]

On Fri, Feb 24, 2023 at 2:06 PM Sergey Organov <sorganov@gmail.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > On Wed, Feb 22, 2023 at 6:27 AM Sergey Organov <sorganov@gmail.com> wrote:

> >> I also agree (in particular with Buga) that from the POV of user
> >> experience the method suggested by Phillip should be superior.
> >> [...]
> >> Even this would be a huge step forward compared to silent
> >> drop of merge commits and blindly re-merging of updated parents.
> >
> > I'm not so sure it's a huge step forward.  Or even a step forward.
>
> Git currently throws away my precious merges! Silently! How it's not a
> step forward to stop doing this?! Sorry for getting that heated :)

I totally agree with you that we have a big problem.  No need to
convince me on that.  :-)

But having a big problem does not imply we have to implement and ship
the first proposal that comes along to change things.  Or second, or
third.  Such proposals might actually make things even worse.  You
correctly point out that we do not need to require perfection, but we
can and should require that the proposed solutions not only make some
things better but that they make things better overall.

And in order to convincingly persuade others to adopt various
proposals, we should be aware of what the advantages and shortcomings
are...at least the ones that have already been discovered and
publicized, and be able to talk about those shortcomings candidly.

> As for Dscho results specifically, I've got an impression that he never
> needed rebasing of merges in the first place, and re-merging always
> suited him just fine, so it'd be rather a surprise if rebasing of merges
> suddenly started to work better for his needs and workflows once he has
> implemented it.

Are you serious?

You're claiming the author of --preserve-merges; and the author of
--rebase-merges; and someone who actually implemented the ideas you,
Buga, and Phillip were all discussing to improve rebasing of
merges[1]; and who maintains a project (Git for Windows) that has
countless branches with hundreds of commits and myriad merge points
and needs to rebase the whole lot as Git is updated...is someone who
doesn't actually care about rebasing of merges?

I thought you had tried to read up on this subject and were commenting
in good faith, but I'm starting to have my doubts.

Please, go read at least [1] to see Johannes comments about how the
prior proposals don't work beyond simple cases.  He didn't discard
those ideas because he didn't care about the useful information in
merge commits, he discarded them because in practice those ideas
resulted in behavior that was *even worse* than the current big
problems.

[1] https://lore.kernel.org/git/nycvar.QRO.7.76.6.1804130002090.65@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz/

[...]
> for now I do believe we need something
> reliable that has been checked to actually work for common cases, as
> blind re-merging simply does not.

I agree with you.  The word "reliable" is particularly key, and IMO
rules out any suggestion that involves applying the diff between a
merge commit and either of its parents.  Not only do I think it's the
wrong solution theoretically, I also think they have empirically been
shown to provide problems that many will consider to be as bad or
worse than our current poison.  I obviously don't have veto power or
anything close to it, but in my opinion any solution based on those
ideas do not meet the threshold bar for inclusion in Git and I'll
raise my voice against them.

Solutions based on other ideas are fair game.  Heck, I've proposed one
and I know of simpler variants to my proposal.  Other solutions may
exist too.  But can we stop pushing already discredited proposals and
instead reach for something that has a more solid foundation?

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Sergey Organov]

Elijah Newren <newren@gmail.com> writes:

> On Fri, Feb 24, 2023 at 2:06 PM Sergey Organov <sorganov@gmail.com> wrote:
>>
>> Elijah Newren <newren@gmail.com> writes:

[...]

> Please, go read at least [1] to see Johannes comments about how the
> prior proposals don't work beyond simple cases.

It's exactly handling of simple cases that we need most. We can get
fancy afterwards, if feasible.

Thanks,
-- Sergey Organov

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Elijah Newren]

On Sat, Feb 25, 2023 at 7:15 AM Sergey Organov <sorganov@gmail.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > On Fri, Feb 24, 2023 at 2:06 PM Sergey Organov <sorganov@gmail.com> wrote:
> >>
> >> Elijah Newren <newren@gmail.com> writes:
>
> [...]
>
> > Please, go read at least [1] to see Johannes comments about how the
> > prior proposals don't work beyond simple cases.
>
> It's exactly handling of simple cases that we need most. We can get
> fancy afterwards, if feasible.

If we can handle just the simple cases without making common cases
significantly worse, that'd be a potential path forward.  Any proposal
involving the diff between a merge commit and either of its parents
(or an equivalent such as a three-way merge involving the merge commit
and one of its parents) doesn't achieve that, IMO.

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Sergey Organov]

Elijah Newren <newren@gmail.com> writes:

> On Sat, Feb 25, 2023 at 7:15 AM Sergey Organov <sorganov@gmail.com> wrote:
>>
>> Elijah Newren <newren@gmail.com> writes:
>>
>> > On Fri, Feb 24, 2023 at 2:06 PM Sergey Organov <sorganov@gmail.com> wrote:
>> >>
>> >> Elijah Newren <newren@gmail.com> writes:
>>
>> [...]
>>
>> > Please, go read at least [1] to see Johannes comments about how the
>> > prior proposals don't work beyond simple cases.
>>
>> It's exactly handling of simple cases that we need most. We can get
>> fancy afterwards, if feasible.
>
> If we can handle just the simple cases without making common cases
> significantly worse, that'd be a potential path forward.  Any proposal
> involving the diff between a merge commit and either of its parents
> (or an equivalent such as a three-way merge involving the merge commit
> and one of its parents) doesn't achieve that, IMO.

Except the method discussed does achieve exactly that according to the
evidence gathered at the time of debates, and here is confirmation (from
Johannes himself) from the reference you provided:

"This strategy, while it performed well in my initial tests (and in
Buga's initial tests, too), *does* involve more than one 3-way merge,
and therefore it risks something very, very nasty: *nested* merge
conflicts."

So, overall, the method performs well in general, and we just need to
avoid driving ourselves into nested merge conflicts, as resolving them
is beyond capabilities of most human beings.

Setting this back into perspective, in comparison to blind re-merge,
that fails to keep user changes even when no conflicts at all exist, and
even when it's applied at the same place in the history, the discussed
method is a *huge* step forward, especially if re-merge is kept as a
fallback strategy.

P.S. BTW, where this hate for using of diffs with respect to parents
come from, I wonder, provided we do use them all the time anyway?

Thanks,
-- Sergey Organov

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Elijah Newren]

On Sun, Feb 26, 2023 at 1:29 AM Sergey Organov <sorganov@gmail.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > On Sat, Feb 25, 2023 at 7:15 AM Sergey Organov <sorganov@gmail.com> wrote:
> >>
> >> Elijah Newren <newren@gmail.com> writes:
> >>
> >> > On Fri, Feb 24, 2023 at 2:06 PM Sergey Organov <sorganov@gmail.com> wrote:
> >> >>
> >> >> Elijah Newren <newren@gmail.com> writes:
> >>
> >> [...]
> >>
> >> > Please, go read at least [1] to see Johannes comments about how the
> >> > prior proposals don't work beyond simple cases.
> >>
> >> It's exactly handling of simple cases that we need most. We can get
> >> fancy afterwards, if feasible.
> >
> > If we can handle just the simple cases without making common cases
> > significantly worse, that'd be a potential path forward.  Any proposal
> > involving the diff between a merge commit and either of its parents
> > (or an equivalent such as a three-way merge involving the merge commit
> > and one of its parents) doesn't achieve that, IMO.
>
> Except the method discussed does achieve exactly that according to the
> evidence gathered at the time of debates, and here is confirmation (from
> Johannes himself) from the reference you provided:

I'm glad you read it.  :-)

> "This strategy, while it performed well in my initial tests (and in
> Buga's initial tests, too), *does* involve more than one 3-way merge,
> and therefore it risks something very, very nasty: *nested* merge
> conflicts."
>
> So, overall, the method performs well in general,

Jumping from "performed well on initial tests" to "performs well in
general" seems to me to be quite a large and unwarranted logical leap.

> and we just need to
> avoid driving ourselves into nested merge conflicts

I'm glad you're discussing a disadvantage and how to address it, but I
don't understand how you can jump to the implication that this is the
only one.

> Setting this back into perspective, in comparison to blind re-merge,
> that fails to keep user changes even when no conflicts at all exist, and
> even when it's applied at the same place in the history, the discussed
> method is a *huge* step forward, especially if re-merge is kept as a
> fallback strategy.

The use of superlatives and asterisks doesn't change my opinion; I'm
still skeptical that the given strategy is overall a step forward, let
alone a large one.

(I do agree we have a huge problem and thus that a huge step forward
theoretically could be taken, I just don't see this as it.)

> P.S. BTW, where this hate for using of diffs with respect to parents
> come from, I wonder, provided we do use them all the time anyway?

I have no hate for such diffs; I just firmly believe they are
inappropriate as a solution for the particular problem space being
discussed.

But I've stated that more than enough, and no one is producing patches
on this topic right now, so I'll drop out of this thread.  I still
believe in my proposed solution, and I'll implement it as I get time
for it.

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Sergey Organov]

Elijah Newren <newren@gmail.com> writes:

> On Sun, Feb 26, 2023 at 1:29 AM Sergey Organov <sorganov@gmail.com> wrote:
>>
>> Elijah Newren <newren@gmail.com> writes:
>>
>> > On Sat, Feb 25, 2023 at 7:15 AM Sergey Organov <sorganov@gmail.com> wrote:
>> >>
>> >> Elijah Newren <newren@gmail.com> writes:
>> >>
>> >> > On Fri, Feb 24, 2023 at 2:06 PM Sergey Organov <sorganov@gmail.com> wrote:
>> >> >>
>> >> >> Elijah Newren <newren@gmail.com> writes:
>> >>
>> >> [...]
>> >>
>> >> > Please, go read at least [1] to see Johannes comments about how the
>> >> > prior proposals don't work beyond simple cases.
>> >>
>> >> It's exactly handling of simple cases that we need most. We can get
>> >> fancy afterwards, if feasible.
>> >
>> > If we can handle just the simple cases without making common cases
>> > significantly worse, that'd be a potential path forward.  Any proposal
>> > involving the diff between a merge commit and either of its parents
>> > (or an equivalent such as a three-way merge involving the merge commit
>> > and one of its parents) doesn't achieve that, IMO.
>>
>> Except the method discussed does achieve exactly that according to the
>> evidence gathered at the time of debates, and here is confirmation (from
>> Johannes himself) from the reference you provided:
>
> I'm glad you read it.  :-)

In fact I didn't read it, I rather re-read it ;-)

(I'm in the CC list there, so it should not have been a surprise I did
read it then.)

>
>> "This strategy, while it performed well in my initial tests (and in
>> Buga's initial tests, too), *does* involve more than one 3-way merge,
>> and therefore it risks something very, very nasty: *nested* merge
>> conflicts."
>>
>> So, overall, the method performs well in general,
>
> Jumping from "performed well on initial tests" to "performs well in
> general" seems to me to be quite a large and unwarranted logical leap.

There were quite a few tests performed and methods were polished before
Johannes has been persuaded to give the feature a try, some of the tests
being complex enough, and both methods did perform rather well. That is
what he calls "initial tests" there I believe, and then he found the
case that is very important to him, but lead him to nested merge
conflicts, that is indeed quite bad.

>
>> and we just need to avoid driving ourselves into nested merge
>> conflicts
>
> I'm glad you're discussing a disadvantage and how to address it, but I
> don't understand how you can jump to the implication that this is the
> only one.

Well, it was you who gave me the reference to comment on, and that was
the only disadvantage I was able to find being discussed there. I also
don't recall any other objections back then when the problem has been
discussed a lot.

>> Setting this back into perspective, in comparison to blind re-merge,
>> that fails to keep user changes even when no conflicts at all exist, and
>> even when it's applied at the same place in the history, the discussed
>> method is a *huge* step forward, especially if re-merge is kept as a
>> fallback strategy.
>
> The use of superlatives and asterisks doesn't change my opinion; I'm
> still skeptical that the given strategy is overall a step forward, let
> alone a large one.

You just repeat saying the same thing, without any further arguments?
OK, thank you for your opinion anyway.

> (I do agree we have a huge problem and thus that a huge step forward
> theoretically could be taken, I just don't see this as it.)

It works. Really.

>
>> P.S. BTW, where this hate for using of diffs with respect to parents
>> come from, I wonder, provided we do use them all the time anyway?
>
> I have no hate for such diffs; I just firmly believe they are
> inappropriate as a solution for the particular problem space being
> discussed.

From my POV particular problem space (rebasing commits) already uses the
diffs, and only them, that's why I can't figure how you end up coming to
such conclusion.

>
> But I've stated that more than enough, and no one is producing patches
> on this topic right now, so I'll drop out of this thread.

OK, I participate only in hope that there will be somebody who actually
cares enough to implement it. Maybe it will be me, maybe not, and I
already got it that neither you nor the original author of git-rebase
are interested.

> I still believe in my proposed solution, and I'll implement it as I
> get time for it.

Sure it'd be nice. Fortunately there is nothing mutually exclusive here.

Thanks,
-- Sergey Organov

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Elijah Newren]

Note: I'm not talking about rebasing merges anymore in this thread,
but I thought there was a useful how-we're-communicating subthread
that's worth addressing to see if we can make that part work better...

On Mon, Feb 27, 2023 at 9:17 AM Sergey Organov <sorganov@gmail.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > On Sun, Feb 26, 2023 at 1:29 AM Sergey Organov <sorganov@gmail.com> wrote:
> >>
> >> Elijah Newren <newren@gmail.com> writes:
> >>
> >> > On Sat, Feb 25, 2023 at 7:15 AM Sergey Organov <sorganov@gmail.com> wrote:
> >> >>
> >> >> Elijah Newren <newren@gmail.com> writes:
> >> >>
> >> >> > On Fri, Feb 24, 2023 at 2:06 PM Sergey Organov <sorganov@gmail.com> wrote:
> >> >> >>
> >> >> >> Elijah Newren <newren@gmail.com> writes:
> >> >>
> >> >> [...]
> >> >>
> >> >> > Please, go read at least [1] to see Johannes comments about how the
> >> >> > prior proposals don't work beyond simple cases.
[...]
> >> Except the method discussed does achieve exactly that according to the
> >> evidence gathered at the time of debates, and here is confirmation (from
> >> Johannes himself) from the reference you provided:
> >
> > I'm glad you read it.  :-)
>
> In fact I didn't read it, I rather re-read it ;-)
>
> (I'm in the CC list there, so it should not have been a surprise I did
> read it then.)

I knew you were on the CC, I just didn't believe at the time that you
could have read that email and still claimed that "As for Dscho
results specifically, I've got an impression that he never
needed rebasing of merges in the first place", so I assumed you had
skipped that email or only lightly skimmed it.

I'm still quite surprised, but clearly my assumption that you read the
email was wrong.  Sorry about that.

> >> Setting this back into perspective, in comparison to blind re-merge,
> >> that fails to keep user changes even when no conflicts at all exist, and
> >> even when it's applied at the same place in the history, the discussed
> >> method is a *huge* step forward, especially if re-merge is kept as a
> >> fallback strategy.
> >
> > The use of superlatives and asterisks doesn't change my opinion; I'm
> > still skeptical that the given strategy is overall a step forward, let
> > alone a large one.
>
> You just repeat saying the same thing, without any further arguments?
> OK, thank you for your opinion anyway.

That exactly mirrors how I've felt about your emails in this thread.
You are right that I'm not going into detail either...but why would I?

  * I'm not trying to convince you to implement these ideas or change
your own implementation, especially since:
  * You've previously said you aren't even planning on working on this[1].

Of course, you can easily ask why I would think you might provide
details when I was not providing any.  Well, from my viewpoint:

  * You did say you were hoping someone else would work on this
problem[1,2,& end of this email], and I've expressed interest in
working on the problem space.
  * If you want someone to work on your ideas, using your particular
favored approach, for free, then you need to convince them that your
approach is worth investing in
  * I have read the old proposals, in detail, and stated I don't
believe in them.
  * You didn't try to address my concerns beyond simply reasserting
that the ideas in the original proposals were good, and seemed to be
more interested in discrediting or minimizing my concerns (e.g. asking
why I "hated diffs from merge to either parent") than in learning
about or addressing them.

I think your intentions are good (you're trying to solve a big problem
in Git), I'm just a little worried about the execution (e.g. brushing
aside my concerns so folks won't pay attention to them while  actively
recruiting eager contributors, with the plan to send them down what I
believe is a dead-end path).  If it was just you going down this path,
I wouldn't be so concerned.  Personally, I pursue a *lot* of my own
bad ideas, and learn in the process that they were bad.  Also, if you
showed some willingness to entertain that I might be right that the
old proposals are bad, and could communicate that to new contributors
and let them decide, I would have dropped out of the thread sooner and
just let you do your thing.

The way you've responded in this thread doesn't seem unique to our
interactions; it reminds me of an interaction you had with Junio at
[3].  He suggested there were some code issues.  You could have asked
what they were and maybe learned how to improve things.  Or maybe you
could have learned that he just had a specific misunderstanding which
could have been corrected if you asked some questions to find out what
he was thinking.  Instead, you simply asserted that things were fine
and dismissed his concerns.  I think it was a lost opportunity.

Now, it's fully possible here that I've misunderstood your purpose; if
so, I apologize.  The above was the understanding I was working off
of; maybe knowing that will help you understand my responses.

[1] stated in final paragraph of
https://lore.kernel.org/git/87zgkh9buq.fsf@osv.gnss.ru/
[2] hinted at in final paragraph of
https://lore.kernel.org/git/87bklilnvp.fsf@osv.gnss.ru/
[3] https://lore.kernel.org/git/87wna3jwx8.fsf@osv.gnss.ru/

> > (I do agree we have a huge problem and thus that a huge step forward
> > theoretically could be taken, I just don't see this as it.)
>
> It works. Really.
>

> > But I've stated that more than enough, and no one is producing patches
> > on this topic right now, so I'll drop out of this thread.
>
> OK, I participate only in hope that there will be somebody who actually
> cares enough to implement it. Maybe it will be me, maybe not, and I
> already got it that neither you nor the original author of git-rebase
> are interested.

Correct, I'm not interested in implementing it that particular way,
though I will be implementing it in what I feel is the right way.

Anyway, I hope something I said above helps in some way.  Even if not,
I wish you the best of luck on your efforts.

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Elijah Newren]

Hi Phillip,

On Sat, Feb 18, 2023 at 8:39 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 18/02/2023 03:17, Elijah Newren wrote:
> >
> > One concern I have is that "--rebase-merges" itself has negative user
> > surprises in store.  In particular, "--rebase-merges", despite its
> > name, does not rebase merges.  It uses the existing author & commit
> > message info, but otherwise just discards the existing merge and
> > creates a new one.  Any information it contained about fixing
> > conflicts, or making adjustments to make the two branches work
> > together, is summarily and silently discarded.
>
> That's a good point. Another potentially surprising behavior is that
> when I'm rebasing an integration branch with -rno-rebase-cousins then if
> one of the topic branches merged into the integration branch happens to
> share the same base as the integration branch itself the topic branch
> gets rebased as well. -rno-rebase-cousins is also slower that it needs
> to be because it creates a todo list that contains all the commits on
> the topic branches merged into the integration branch rather than just
> the merges. The commits on the topic branches are fast-forwarded rather
> than rewritten so long as they don't share the same base as the
> integration branch but it noticeably slower than using a todo list with
> just the merge commands.

Yeah, modifying rebase to accept a general range expression (instead
of assuming upstream..HEAD) would really help.  Then, to get just the
parts you are interested in, you could use a range with extra commit
exclusions and additional qualifiers like --ancestry-path=<commit> and
--first-parent.  In fact, you could also list multiple branches (none
of which necessarily fully contains any of the others) to replay
multiple branches at a time.  (See [2] for where I discuss this
more, though focusing on the --ancestry-path=<commit> part of it.).

But, it'd also fundamentally break existing workflows, so it might
have to be a new command, perhaps `git replay`.  However, there's
multiple other improvements needed in rebase (such as not wasting time
updating the working tree or index or reflog for every commit, or
wasting time writing N control files when we could move to 1 control
file, and allowing working on branches that aren't checked out) that I
think would likely also break compatibility, so maybe another command
is a good idea anyway[3].

[2] https://lore.kernel.org/git/CABPp-BHmj+QCBFDrH77iNfEU41V=UDu7nhBYkAbCsbXhshJzzw@mail.gmail.com/
[3] https://github.com/newren/git/blob/e84f5f3585fd770ed21f398d2ae5f96e90a51b1e/replay-design-notes.txt

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

I've actually put quite a bit of time into this problem.  I have
outlined what I think is a full solution to the rebasing of merges
problem space at [4], which expands on my earlier discussion with
Johannes on-list over at [5] (which in turn was a follow-up to
previous discussions that you, Johannes, and several others had years
ago).  If you're interested and have any thoughts on my plans for this
problem space, I'd love to hear it.  You tend to have very strong
insights on everything xdiff, sequencer, and rebasing related.  My
"replay" branch contains a partial implementation, but it's not really
usable for anything rebase-merges-related yet, so you'd mostly have to
go with my writeups.

A warning, though, that I won't be able to respond to feedback on this
topic very soon.  I will definitely get back to working on it, but
it's been much more challenging with more limited git time these days.
Unfortunately, the current economic environment reduces the number of
ways possible to extend the amount of time available for working on
Git, but one way or another I'll eventually get back to this problem
and implement my ideas, unless someone beats me to it.

[4] https://github.com/newren/git/blob/e84f5f3585fd770ed21f398d2ae5f96e90a51b1e/replay-design-notes.txt#L264-L341
[5] https://lore.kernel.org/git/CABPp-BHWVO5VRhr1-Ou60F1wjKzJZ1e_dC01Mmzs+qB9kGayww@mail.gmail.com/

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Tao Klerks]

On Sat, Feb 18, 2023 at 4:17 AM Elijah Newren <newren@gmail.com> wrote:
>
> On Thu, Feb 16, 2023 at 8:02 PM Alex Henrie <alexhenrie24@gmail.com> wrote:
> >
> > On Thu, Feb 16, 2023 at 5:31 AM Tao Klerks <tao@klerks.biz> wrote:
> > >
> > > If there's an appetite for it, I would love to contribute to a
> > > multi-year adventure to change git's behavior, little by little, until
> > > the behavior of "rebase=merges" is the default, and the old behavior
> > > becomes a different option like
> > > "rebase=copy-merged-commits-to-flatten"
> >
> > I know you had a lot to say in your last email, but I'd like to focus
> > on this point. I would be OK with the proposed patch if it were part
> > of a larger effort to make --rebase-merges the default behavior of
> > `git rebase`. That seems like an achievable goal, and I don't think it
> > would take multiple years, maybe one year at the most. The process
> > would look something like this:
> >
<SNIP>
> >
> > Does that sound reasonable? I think I could lend a hand with steps 1-3.
>
> One concern I have is that "--rebase-merges" itself has negative user
> surprises in store.  In particular, "--rebase-merges", despite its
> name, does not rebase merges.  It uses the existing author & commit
> message info, but otherwise just discards the existing merge and
> creates a new one.  Any information it contained about fixing
> conflicts, or making adjustments to make the two branches work
> together, is summarily and silently discarded.
>
> 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 apologize for my ignorance here, but I'm not sure how this "does not
rebase merges" concern overlaps with the "pull.rebase" context I'm
most specifically concerned about.

I would have assumed that when merge commits are "dropped", as results
from the current "pull.rebase=true" option in the pull conflict
advice, any merge resolution information is *also* dropped - so there
is no loss to the user here in advising the use of
"pull.rebase=merges" instead.

Is your concern about the "pull.rebase=merges" advice change, or more
about the broader "let's encourage users to more explicitly choose
between traditional merge-dropping rebase and rebase-merges" change
Alex is advocating for as a precondition to "my" change :) ?

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Elijah Newren]

On Sun, Feb 19, 2023 at 10:01 PM Tao Klerks <tao@klerks.biz> wrote:
>
> On Sat, Feb 18, 2023 at 4:17 AM Elijah Newren <newren@gmail.com> wrote:
> >
> > On Thu, Feb 16, 2023 at 8:02 PM Alex Henrie <alexhenrie24@gmail.com> wrote:
> > >
> > > On Thu, Feb 16, 2023 at 5:31 AM Tao Klerks <tao@klerks.biz> wrote:
> > > >
> > > > If there's an appetite for it, I would love to contribute to a
> > > > multi-year adventure to change git's behavior, little by little, until
> > > > the behavior of "rebase=merges" is the default, and the old behavior
> > > > becomes a different option like
> > > > "rebase=copy-merged-commits-to-flatten"
> > >
> > > I know you had a lot to say in your last email, but I'd like to focus
> > > on this point. I would be OK with the proposed patch if it were part
> > > of a larger effort to make --rebase-merges the default behavior of
> > > `git rebase`. That seems like an achievable goal, and I don't think it
> > > would take multiple years, maybe one year at the most. The process
> > > would look something like this:
> > >
> <SNIP>
> > >
> > > Does that sound reasonable? I think I could lend a hand with steps 1-3.
> >
> > One concern I have is that "--rebase-merges" itself has negative user
> > surprises in store.  In particular, "--rebase-merges", despite its
> > name, does not rebase merges.  It uses the existing author & commit
> > message info, but otherwise just discards the existing merge and
> > creates a new one.  Any information it contained about fixing
> > conflicts, or making adjustments to make the two branches work
> > together, is summarily and silently discarded.
> >
> > 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 apologize for my ignorance here, but I'm not sure how this "does not
> rebase merges" concern overlaps with the "pull.rebase" context I'm
> most specifically concerned about.
>
> I would have assumed that when merge commits are "dropped", as results
> from the current "pull.rebase=true" option in the pull conflict
> advice, any merge resolution information is *also* dropped - so there
> is no loss to the user here in advising the use of
> "pull.rebase=merges" instead.
>
> Is your concern about the "pull.rebase=merges" advice change, or more
> about the broader "let's encourage users to more explicitly choose
> between traditional merge-dropping rebase and rebase-merges" change
> Alex is advocating for as a precondition to "my" change :) ?

When we teach new folks about git, and get to rebasing, there is a
simple and easy rule to tell users: don't mix merges and rebases.
(There's a minor exception there in that merges with the upstream
branch are fine and rebasing can let you get rid of those otherwise
ugly-and-frequent back-merges that users sometimes make.)

Obviously, your users are ignoring that advice, and feeling pain.  To
be fair, the "RECOVERING FROM UPSTREAM REBASE" section of the rebase
manual isn't that prominent, and perhaps your users didn't have more
seasoned developers sharing this don't-mix-merges-and-rebases advice
with them.  (It seemed to me to be shared pretty widely and commonly,
but perhaps we are relying on education from others too much and
education is never uniform if not coming from the tool itself.)  I
understand you want to make it easier for users to avoid accidentally
getting into this state.  That's a valid concern and desire.  I think
we should improve the situation.

However, on what timetable and at what cost to others?

You're advocating we start advertising an alternate option, one which
has some caveats and gotchas that are not going to be so easy to
explain to users -- neither to new users, nor to folks who have been
using Git for years.  We could just bite the bullet and start
explaining, but these caveats and gotchas are completely incidental to
the implementation, and are in no-wise fundamental to the desired
operation.  I believe that switching to this new option is going to
generate an awful lot of questions and surprises by users.  It seems
to me to be a really sad state of affairs to be recommending an option
with known defects when (IMO) the solution is known.  Can't we fix it
first, then recommend it?

Granted, this is a trade-off.  You have users experiencing real pain.
You want a solution now.  I want to not recommend features with known
implementation shortcomings and known solutions, until those solutions
are implemented, and I know that will take a while.  What to do here
is a judgement call, and I was merely giving my opinion on the call to
make.  Other folks on the list might see things differently than I do.

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Alex Henrie]

On Sun, Feb 5, 2023 at 9:41 AM Tao Klerks via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Tao Klerks <tao@klerks.biz>
>
> Unfortunately, this rebase configuration can easily lead to non-expert users
> accidentally rebasing not their own commits, instead others' commits, if the
> new commits they have locally before the "pull" include a merge of another
> branch, eg "main".

On Mon, Feb 20, 2023 at 10:21 AM Elijah Newren <newren@gmail.com> wrote:
>
> When we teach new folks about git, and get to rebasing, there is a
> simple and easy rule to tell users: don't mix merges and rebases.
> (There's a minor exception there in that merges with the upstream
> branch are fine and rebasing can let you get rid of those otherwise
> ugly-and-frequent back-merges that users sometimes make.)

The "minor exception" is merging a topic branch into main, right? And
the "ugly-and-frequent back-merges" are the merges from main into a
topic branch?

Tao, the primary motivation behind the `git pull` warning was to help
prevent users from merging main into a topic branch when that's not
what they really want to do. The fact that novices sometimes do that
has been a point of pain for many people, including Linus Torvalds:
See "Don't merge upstream code at random points" at [1] and "github
creates absolutely useless garbage merges" at [2].

If you're seeing users merge main into topic branches without a good
reason, that does sound like more of an education problem than a
bad-defaults problem. We might still want to change the default to
better support the more unusual cases, but if you're going for a quick
win, it would be faster to teach users the wisdom of not mixing rebase
and merge in the first place.

-Alex

[1] https://www.mail-archive.com/dri-devel@lists.sourceforge.net/msg39091.html
[2] https://lore.kernel.org/lkml/CAHk-=wjbtip559HcMG9VQLGPmkurh5Kc50y5BceL8Q8=aL0H3Q@mail.gmail.com/

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Tao Klerks]

On Mon, Feb 20, 2023 at 7:33 PM Alex Henrie <alexhenrie24@gmail.com> wrote:
>
> Tao, the primary motivation behind the `git pull` warning was to help
> prevent users from merging main into a topic branch when that's not
> what they really want to do. The fact that novices sometimes do that
> has been a point of pain for many people, including Linus Torvalds:
> See "Don't merge upstream code at random points" at [1] and "github
> creates absolutely useless garbage merges" at [2].
>
> If you're seeing users merge main into topic branches without a good
> reason, that does sound like more of an education problem than a
> bad-defaults problem.

I would disagree on two points:

1. The need for merging in the upstream varies project by project,
user by user, etc. If you are working on a part of a system where you
can reasonably assume the ground will not shift under your feet,
awesome, lucky you! Many users are not so fortunate, and need to
regularly ensure that their changes still make sense in the
ever-changing upstream context.

Regularly (whatever that means to you) merging in the upstream is the
simplest way of achieving that. If you're working on your own or as
part of a team that's happy to handle coordinated rebasing, then
rebasing is a potentially-more-satisfying way of achieving the same
end - either way, assuming that their changes will make sense in the
upstream context is simply not a luxury many users can afford over any
period of time.

Now, you note that Linus advocates for merging specific points,
because he doesn't respect you merging "random crap" from a branch
called "linus" - that's fine, but many projects strive to keep a
specific trunk branch "evergreen" in order to minimize late conflicts
are maximize coordination - there's a pretty cool site about it:
https://trunkbaseddevelopment.com/ - this is not really different to
Linus' advice except that the goal is to make there *never* be "random
crap" on the upstream.

2. The fact that the commit history of non-expert git users (those who
should not be using rebase, especially in teams) are so often...
spidery... is why the "Squash" option of pull requests / merge
requests is so popular in centralized workflows (GitHub, GitLab,
BitBucket, etc).

If your project follows a "merge down, squash up" strategy with a
well-CI-guarded evergreen trunk on a central server, there's simply no
reason to *require* your users to become rebasing experts - you can
let them use simple merge-based workflows, keep your trunk clean by
squashing away their complex commit graphs, let them merge down
whenever they need or want to, etc.


> We might still want to change the default to
> better support the more unusual cases, but

Do we have any analysis/understanding of how common workflows like
that of the git or linux projects are, vs github-style fork-based
projects, vs straight-up single central server projects?

I'm not sure what you mean by "unusual", but I don't think "avoid
rebase unless you really know what you're doing, merge down at will,
we will squash your contribution in the pull/merge request at the end
anyway" is an unusual flow at all nowadays.

> if you're going for a quick
> win, it would be faster to teach users the wisdom of not mixing rebase
> and merge in the first place.
>

"teach [...] wisdom" is a good one! No, seriously - of course I'm
going to do the best I can to prevent my users from falling into the
traps surrounding them - but my point here is that *we simply
shouldn't have pointless traps*. Offering a command that can cause
significant "harm" (time loss, frustration, etc), silently... just
doesn't seem like a good idea.


> [1] https://www.mail-archive.com/dri-devel@lists.sourceforge.net/msg39091.html
> [2] https://lore.kernel.org/lkml/CAHk-=wjbtip559HcMG9VQLGPmkurh5Kc50y5BceL8Q8=aL0H3Q@mail.gmail.com/

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Alex Henrie]

On Tue, Feb 21, 2023 at 8:40 AM Tao Klerks <tao@klerks.biz> wrote:
>
> 2. The fact that the commit history of non-expert git users (those who
> should not be using rebase, especially in teams) are so often...
> spidery... is why the "Squash" option of pull requests / merge
> requests is so popular in centralized workflows (GitHub, GitLab,
> BitBucket, etc).
>
> If your project follows a "merge down, squash up" strategy with a
> well-CI-guarded evergreen trunk on a central server, there's simply no
> reason to *require* your users to become rebasing experts - you can
> let them use simple merge-based workflows, keep your trunk clean by
> squashing away their complex commit graphs, let them merge down
> whenever they need or want to, etc.

The advantage to that workflow is that you don't have to teach users
how to rebase. (Whether the actual process of merging or rebasing is
easier, assuming that the user knows how to do both, is debatable and
likely depends a lot on the particular situation.) The disadvantage is
that even merge requests that seem like they only need one commit
often turn into multiple commits, and squashing all of those commits
together indiscriminately both makes it harder for the reviewer to
follow the progression of steps the developer took and decreases the
usefulness of tools like `git blame` and `git bisect`. For example,
the patch series that I sent to add a rebase.merges option will be 3
or 4 commits in the end, and other developers have good reasons to ask
me to keep those commits separate instead of squashing them all into a
single patch. On top of that, if your developers get the impression
that all projects on GitHub/GitLab/whatever use the same workflow,
they are likely to cause headaches when they present spidery merge
requests to other projects. If you are OK with those tradeoffs then
that's fine, Git will support you. My point is simply that every
workflow has its advantages and disadvantages, and there's no workflow
that solves every problem.

> Do we have any analysis/understanding of how common workflows like
> that of the git or linux projects are, vs github-style fork-based
> projects, vs straight-up single central server projects?

I don't have any statistics (although I would love to see them if they
exist), but I do know that all of these workflows are common enough
that `git pull` can't assume what the user wants. The warning exists
to try to prevent the user from shooting themself in the foot.

> I'm not sure what you mean by "unusual", but I don't think "avoid
> rebase unless you really know what you're doing, merge down at will,
> we will squash your contribution in the pull/merge request at the end
> anyway" is an unusual flow at all nowadays.

The unusual cases are the ones where you mix merge and rebase on your
own topic branch. Your developers did that accidentally (despite `git
pull` trying to warn them) and suffered because of it, because it
isn't well supported right now. I think we all agree that it should be
better supported, we just disagree on how to get there.

-Alex

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Tao Klerks]

On Mon, Feb 20, 2023 at 6:21 PM Elijah Newren <newren@gmail.com> wrote:
>
>
> When we teach new folks about git, and get to rebasing, there is a
> simple and easy rule to tell users: don't mix merges and rebases.
> (There's a minor exception there in that merges with the upstream
> branch are fine and rebasing can let you get rid of those otherwise
> ugly-and-frequent back-merges that users sometimes make.)

Who is "we" here? When I search for the exact text "don't mix merges
and rebases" in google, the only hit I get is this very email thread.

Without the quotes, I get a similar-looking page title, but I don't
understand whether the author's thesis is the same thing you're
getting at - I don't think so:
https://dev.to/jessekphillips/rebase-and-merge-don-t-mix-4aj

>
> Obviously, your users are ignoring that advice, and feeling pain.

"Ignoring" is a strong (and in my opinion, strange) term to use here.
They are *not seeing* that advice, and I think you can reasonably
assume that many, or most, users will not see almost any of the advice
you can possibly offer. As software designers I believe we all strive
to set things up so you need to learn as little as possible to use
something usefully, and safely.

> To
> be fair, the "RECOVERING FROM UPSTREAM REBASE" section of the rebase
> manual isn't that prominent, and perhaps your users didn't have more
> seasoned developers sharing this don't-mix-merges-and-rebases advice
> with them.  (It seemed to me to be shared pretty widely and commonly,
> but perhaps we are relying on education from others too much and
> education is never uniform if not coming from the tool itself.)

My equivalent of this is "never rebase a shared branch unless you and
your team know what you're doing". For most users I interact with,
that translates to "don't use rebase at all unless you have a git
veteran standing at your shoulder / on a screenshare with you".

This advice is, again, not necessarily something that users even *see*
before they start needing to use git. Maybe they should? Should we add
a child lock on the executable, "this is likely to do very surprising
things, please sign here that you have studied graph theory and really
understand how this stuff works before you get to use it"?

> I
> understand you want to make it easier for users to avoid accidentally
> getting into this state.  That's a valid concern and desire.  I think
> we should improve the situation.
>
> However, on what timetable and at what cost to others?
>
> You're advocating we start advertising an alternate option, one which
> has some caveats and gotchas that are not going to be so easy to
> explain to users -- neither to new users, nor to folks who have been
> using Git for years.

What I'm trying to understand is whether or how these caveats and
gotchas are *any worse than the status quo / than the current
"pull.rebase=true" behavior*. I haven't understood any clear concrete
ways in which this is true yet:
* "pull.rebase=merges" throws away your merge conflict resolutions -
so does "pull.rebase=merges", right??
* You might find it surprising that a same-merge-point branch gets
rebased with the default "-rno-rebase-cousins" behavior... but
"pull.rebase=true" will also do that!
* You might be disappointed at the fact that an interactive
--rebase-merges rebase fills your screen with stuff - but a
"flattening" rebase does that too.
* You might be disappointed at the fact that --rebase-merges takes a
long time when fast-forwarding over the merge of a large amount of
history - but a "flattening" rebase does that too.

I'm not advocating for experienced users being by any means required
to use this functionality in its less-than-perfect state - but I *am*
arguing that foisting that less-than-ideal state on people who
copy-paste a suggested command from a pull conflict hint is far better
than allowing them to accidentally "flatten the history" of
upstream-branch commits.

> We could just bite the bullet and start
> explaining, but these caveats and gotchas are completely incidental to
> the implementation, and are in no-wise fundamental to the desired
> operation.

We already *do* explain, right? We've already retired --preserve-merges!

> I believe that switching to this new option is going to
> generate an awful lot of questions and surprises by users.  It seems
> to me to be a really sad state of affairs to be recommending an option
> with known defects when (IMO) the solution is known.  Can't we fix it
> first, then recommend it?

I guess maybe I'm misunderstanding your concern:
* My main aim is to stop users shooting themselves in the foot
* If making --preserve-merges the overall default for "git rebase" is
the only or best way as Alex has proposed, great, let's do that. I
personally would like to have the "use --rebase-merges by default when
using "git rebase" option that Alex is proposing even if we don't do
this, but even that is secondary to me compared with the "stop
offering a rebase behavior that will cause users to duplicate upstream
history unknowingly, at a blocking prompt where you're forcing them to
pick *something*" objective that kicked this off.
* If there is another, better way of removing this foot-gun, I'm happy
to explore another direction

Do you have a suggestion? Or would you advocate for *eventually*
replacing the foot-gun with a more childsafe tool, when that tool is
as sophisticated as we think it should eventually become?

>
> Granted, this is a trade-off.  You have users experiencing real pain.
> You want a solution now.  I want to not recommend features with known
> implementation shortcomings and known solutions, until those solutions
> are implemented, and I know that will take a while.  What to do here
> is a judgement call, and I was merely giving my opinion on the call to
> make.  Other folks on the list might see things differently than I do.

I think there are many options, and I'd love to understand which one
you advocate for in the immediate term, with respect to the specific
issue I noted:

* Replace the pull conflict hint only, as I initially proposed
* Engage on an "asap" replacement of default "git rebase" behavior to
"--rebase-merges" by default
* Change the pull conflict hint in some other way (that removes the
copy-paste footgun)
* Do nothing, accepting that we will revise all this in some future,
and it's been like this for so long, what's wrong with a few more
people hitting the classic issues?
* Some other proposal for short-term relief of this very specific problem?

I should note here, that for "my" users, setting the new config option
Alex proposes in "rebase: add a config option for --rebase-merges" by
default, in all their repos, is sufficient for me to ensure people
will stop hurting themselves, and that's something I can easily do
if/when the patch is accepted - but the main reason I hang out here is
to try to advocate for users *like* mine, people who use git because
it's the best or only game in town, rather than people like me who
think it's so friggin awesome and are fascinated to learn all its
arcane mysteries. In my environment, that's easily a 10:1 ratio. I
suspect that's a reasonable reflection on the universe of git users
generally.

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Elijah Newren]

On Tue, Feb 21, 2023 at 7:01 AM Tao Klerks <tao@klerks.biz> wrote:
>
> On Mon, Feb 20, 2023 at 6:21 PM Elijah Newren <newren@gmail.com> wrote:
> >
[...]
> > Obviously, your users are ignoring that advice, and feeling pain.
>
> "Ignoring" is a strong (and in my opinion, strange) term to use here.
> They are *not seeing* that advice, and I think you can reasonably
> assume that many, or most, users will not see almost any of the advice
> you can possibly offer. As software designers I believe we all strive
> to set things up so you need to learn as little as possible to use
> something usefully, and safely.

Yep, fair enough.

> > We could just bite the bullet and start
> > explaining, but these caveats and gotchas are completely incidental to
> > the implementation, and are in no-wise fundamental to the desired
> > operation.
>
> We already *do* explain, right? We've already retired --preserve-merges!

But we didn't suggest either `--preserve-merges` or `--rebase-merges`
to general users.  The only ones who used it went looking for it.
That's fundamentally different than recommending it to all new and
many existing Git users.

> > Granted, this is a trade-off.  You have users experiencing real pain.
> > You want a solution now.  I want to not recommend features with known
> > implementation shortcomings and known solutions, until those solutions
> > are implemented, and I know that will take a while.  What to do here
> > is a judgement call, and I was merely giving my opinion on the call to
> > make.  Other folks on the list might see things differently than I do.
>
> I think there are many options, and I'd love to understand which one
> you advocate for in the immediate term, with respect to the specific
> issue I noted:
>
> * Replace the pull conflict hint only, as I initially proposed
> * Engage on an "asap" replacement of default "git rebase" behavior to
> "--rebase-merges" by default
> * Change the pull conflict hint in some other way (that removes the
> copy-paste footgun)
> * Do nothing, accepting that we will revise all this in some future,
> and it's been like this for so long, what's wrong with a few more
> people hitting the classic issues?
> * Some other proposal for short-term relief of this very specific problem?

My personal opinion is that we should avoid long term problems for
users and maintainers of rebasing merges by pushing it too early to
too many folks.  For short term relief, I would suggest some mixture
of the following are worth looking into:
  * Attempt to improve the message shown to users, perhaps referring
them to somewhere in the docs that point out advantages and
disadvantages of each choice.
  * Possibly make the message shown to users be "smart" rather than
hardcoded.  For example, you could check the local-only portion of
history; if there are no merge commits, or if the upstream branch is
"origin/main" and the only merges within the local-only history are
'Merge branch "origin/main"...' then suggesting a regular rebase is
fine.  If there are other merges, then adapt the wording (and perhaps
in that special case, actually bringing up rebasing merges is okay,
though it'd still be nice if the docs with advantages/disadvantages
pointed out its shortcomings).  This might be an expensive check, but
if only users who haven't configured pull.<whatever> have to pay for
it, then perhaps it's a useful thing to spend cycles on.

> I should note here, that for "my" users, setting the new config option
> Alex proposes in "rebase: add a config option for --rebase-merges" by
> default, in all their repos, is sufficient for me to ensure people
> will stop hurting themselves, and that's something I can easily do
> if/when the patch is accepted - but the main reason I hang out here is
> to try to advocate for users *like* mine, people who use git because
> it's the best or only game in town, rather than people like me who
> think it's so friggin awesome and are fascinated to learn all its
> arcane mysteries. In my environment, that's easily a 10:1 ratio. I
> suspect that's a reasonable reflection on the universe of git users
> generally.

Yes, I understand.  It's frustrating when something you need isn't
there and we only have a suboptimal approximation.  I want to make
things better and have put a lot of time into it; some things that
already went into merge-ort were designed around this problem space
and I'm planning to do more here.

But, also, remember that I'm only one voice among many.  Others may
disagree with me and agree with you on pushing this earlier.

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Felipe Contreras]

On Thu, Feb 16, 2023 at 7:33 AM Tao Klerks <tao@klerks.biz> wrote:
> On Thu, Feb 16, 2023 at 4:22 AM Alex Henrie <alexhenrie24@gmail.com> wrote:

> > Now, this is not to say that there's no room for improvement. I like
> > the rebase=merges option and I wish everyone knew about it because
> > there are situations where it really is the best option. I suggest
> > leaving the existing text alone, but adding an additional paragraph,
> > something like:
> >
> > Note that --rebase or pull.rebase=true will drop existing merge
> > commits and rebase all of the commits from all of the merged branches.
> > If you want to rebase but preserve existing merge commits, use
> > --rebase=merges or pull.rebase=merges instead.
>
> My primary motivation with this pull request is to reduce the
> incidences, out there in the world, of people copy-pasting "git config
> pull.rebase true" into their command-line, and causing themselves
> major headaches days or weeks later. The "--rebase=interactive" part
> is secondary (to my concerns), because it's much less copy-pastable.

That's because the whole approach to the pull.rebase configuration is
wrong. I explained why multiple times in countless discussions and git
developers did not listen.

What we need is a pull.mode configuration that is *orthogonal* to
pull.rebase, then everything just works.

For example, you could have this configuration.

    git config pull.mode merge
    git config pull.rebase merges

Then doing `git pull --rebase` would do a merges rebase.

This is not possible with the current approach, which I objected to.

Then there's no problem with telling the users to do pull.mode=rebase
(or whatever), since that doesn't override pull.rebase=merges.

I programmed and explained this precise interaction with rebase=merges
more than two years ago [1], but nobody listened. For an example of
how such configuration would look like, see the patches I just sent
[2].

Cheers.

[1] https://lore.kernel.org/git/20201218211026.1937168-14-felipe.contreras@gmail.com/
[2] https://lore.kernel.org/git/20230228140236.4175835-1-felipe.contreras@gmail.com/T/#t

-- 
Felipe Contreras

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Alex Henrie]

On Tue, Feb 28, 2023 at 7:13 AM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> On Thu, Feb 16, 2023 at 7:33 AM Tao Klerks <tao@klerks.biz> wrote:
> > On Thu, Feb 16, 2023 at 4:22 AM Alex Henrie <alexhenrie24@gmail.com> wrote:
>
> > > Now, this is not to say that there's no room for improvement. I like
> > > the rebase=merges option and I wish everyone knew about it because
> > > there are situations where it really is the best option. I suggest
> > > leaving the existing text alone, but adding an additional paragraph,
> > > something like:
> > >
> > > Note that --rebase or pull.rebase=true will drop existing merge
> > > commits and rebase all of the commits from all of the merged branches.
> > > If you want to rebase but preserve existing merge commits, use
> > > --rebase=merges or pull.rebase=merges instead.
> >
> > My primary motivation with this pull request is to reduce the
> > incidences, out there in the world, of people copy-pasting "git config
> > pull.rebase true" into their command-line, and causing themselves
> > major headaches days or weeks later. The "--rebase=interactive" part
> > is secondary (to my concerns), because it's much less copy-pastable.
>
> That's because the whole approach to the pull.rebase configuration is
> wrong. I explained why multiple times in countless discussions and git
> developers did not listen.
>
> What we need is a pull.mode configuration that is *orthogonal* to
> pull.rebase, then everything just works.
>
> For example, you could have this configuration.
>
>     git config pull.mode merge
>     git config pull.rebase merges
>
> Then doing `git pull --rebase` would do a merges rebase.
>
> This is not possible with the current approach, which I objected to.
>
> Then there's no problem with telling the users to do pull.mode=rebase
> (or whatever), since that doesn't override pull.rebase=merges.
>
> I programmed and explained this precise interaction with rebase=merges
> more than two years ago [1], but nobody listened. For an example of
> how such configuration would look like, see the patches I just sent
> [2].
>
> Cheers.
>
> [1] https://lore.kernel.org/git/20201218211026.1937168-14-felipe.contreras@gmail.com/
> [2] https://lore.kernel.org/git/20230228140236.4175835-1-felipe.contreras@gmail.com/T/#t

I think everybody agrees that the current situation is kind of a mess.
For better or for worse, there was no consensus to get away from the
current set of config options and introduce something more
straightforward, so we keep making incremental evolutionary changes
within the current framework.

I didn't include it in my list of things to do, but your email made me
realize that the problem with `git pull` not being able to do an
interactive rebase that rebases merge commits is also something that
needs to be fixed before flipping on "rebase merges" by default. If we
add that to the list, I guess it would be step 2.25. It's going to be
confusing if `git pull -r` rebases merges but `git pull -ri` does not.
One way to solve that would be to make -ri/--rebase=interactive on the
command line work with pull.rebase=merges instead of overriding it,
and add a separate --(no-)rebase-merges command line option for if the
user really does want to override it.

On top of that, either before or at the same time as when the default
behavior of `git pull --rebase` changes to rebase merges, the behavior
of branch.autoSetupRebase needs to change to match.

-Alex

Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

[Felipe Contreras]

On Tue, Feb 28, 2023 at 2:04 PM Alex Henrie <alexhenrie24@gmail.com> wrote:

> I think everybody agrees that the current situation is kind of a mess.
> For better or for worse, there was no consensus to get away from the
> current set of config options and introduce something more
> straightforward, so we keep making incremental evolutionary changes
> within the current framework.

Yes, but my proposed incremental evolutionary changes would have
solved the problems by now if the maintainer club bothered to look at
them.

Instead, they implemented a solution from one of their club, which in
my opinion is an incremental change that made the situation *worse*,
not better.

This issue with pull.rebase=merges is one that I had *already*
explained to them, and they ignored it. And it's not the only one.

When you ignore the opinions of certain kinds of people, and only
listen to certain kinds of people, you end up with worse code.

Cheers.

-- 
Felipe Contreras