Re: A rebase regression in Git 2.18.0

Hi Dscho,

On Fri, Aug 31, 2018 at 3:12 AM Johannes Schindelin
[off-list ref] wrote:

On Thu, 30 Aug 2018, Elijah Newren wrote:

quoted

On Tue, Aug 28, 2018 at 9:58 AM Junio C Hamano [off-list ref] wrote:

quoted

Elijah Newren [off-list ref] writes:

...

quoted

quoted

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?

Yes.  I updated it once or twice, but it had conflicts with the GSoC
projects each time, so I decided to just hold off on it a bit longer.
I'm still planning to resubmit this once the GSoC projects merge down.

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

Yep, we talked about this before and you mentioned that the rewrite in
C should bring some performance improvements, and we agreed that
merge-recursive is probably the next issue performance-wise.  I think
it's at least worth measuring what the approximate performance
differences are with the rewrite of rebase in C, and posting an RFC
with that info.  If the answer comes back that we need to do more
optimization before we switch the default, that's fine.

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

Not if git-rebase--merge is deleted and --merge is implemented on top
of the interactive backend as an implicitly_interactive case.  In
fact, that's probably the simplest way to "convert" that backend to C.
Anyway, since I plan to submit that change first, we should be good.

quoted

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!

Enjoy your vacation!

quoted

quoted

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.

Cool, good to see others seem to agree on the direction I'd like to
see things move.