Re: A rebase regression in Git 2.18.0

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 31, 2018, 10:11 UTC
Message-ID: <nycvar.QRO.7.76.6.1808311158540.71@tvgsbejvaqbjf.bet>
In-Reply-To: <CABPp-BH8-1YQ49aaM-dXkT9ukKO92MF-FZ2kLwd=KHe4TVbjqQ@mail.gmail.com>

Hi Elijah,

On Thu, 30 Aug 2018, Elijah Newren wrote:

Show 15 quoted lines

> On Tue, Aug 28, 2018 at 9:58 AM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Elijah Newren <newren@gmail.com> writes:
> >
> > >   - Add a flag to turn off directory rename detection, and set the
> > > flag for every call from am.c in order to avoid problems like this.
> >
> > I'd say this is the only practical solution, before you deprecate
> > the "pipe format-patch output to am -3" style of "git rebase" (and
> > optionally replace with something else).
> 
> I posted a patch a while back to add an --am flag to "git rebase",
> make "--am" be implied by options which are still am-specific
> (--whitespace, --committer-date-is-author-date, and -C), and change
> --merge to be the default.

Didn't you also post a patch to fold --merge into the --interactive backend? What's your current state of thinking about this?

As to switching from --am as the default: I still think that --am has serious speed advantages over --merge (or for that matter, --interactive). I have no numbers to back that up, though, and I am currently really busy with working on the CI, so I won't be able to measure these numbers, either...

Also please note: I converted the `am` backend to pure C (it is waiting at https://github.com/gitgitgadget/git/pull/24, to be submitted after the v2.19.0 RC period). Switching to `--merge` as the default would force me to convert that backend, too ;-)

> I'll post it as an RFC again after the various rebase-rewrite series
> have settled and merged down...along with my other rebase cleanups
> that I was waiting on to avoid conflicts with GSoC stuff.

Thanks for waiting! Please note that I am interested, yet I will be on vacation for a couple of weeks in September. Don't let that stop you, though!

Show 8 quoted lines

> > The whole point of "am -3" is to do _better_ than just "patch" with
> > minimum amount of information available on the pre- and post- image
> > blobs, without knowing the remainder of the tree that the patch did
> > not touch.  It is not surprising that the heuristics that look at
> > the unchanging part of the tree to infer renames that may or may not
> > exist guesses incorrectly, either with false positive or negative.
> > In the context of "rebase", we always have all the trees that are
> > involved.  We should be able to do better than "am -3".

Right. I think that Elijah's right, and --merge is that "do better" solution.

Ciao, Dscho