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

From: Elijah Newren <newren@gmail.com>
Date: Feb 20, 2023, 17:20 UTC
Message-ID: <CABPp-BEtXf9ja7Ec1fZ=BZwFDa+50zSAhtm3nN_=k+Nc2c=RXw@mail.gmail.com>
In-Reply-To: <CAPMMpojCYAwwu6_BE+myFaUy6fLqVSWAyiRWr_dGAmMqqUF12Q@mail.gmail.com>

On Sun, Feb 19, 2023 at 10:01 PM Tao Klerks <tao@klerks.biz> wrote:

Show 50 quoted lines

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