git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð" <avarab@gmail.com>,
	corrmage@gmail.com, "Git Mailing List" <git@vger.kernel.org>,
	"Stefan Beller" <sbeller@google.com>
Subject: Re: A rebase regression in Git 2.18.0
Date: Fri, 31 Aug 2018 12:37:59 -0700	[thread overview]
Message-ID: <CABPp-BEJ-JPa8g8G1ELsjqf3EDxe+aCBdwERBi5hr5ukrDjJJg@mail.gmail.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1808311158540.71@tvgsbejvaqbjf.bet>

Hi Dscho,

On Fri, Aug 31, 2018 at 3:12 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Thu, 30 Aug 2018, Elijah Newren wrote:
> > On Tue, Aug 28, 2018 at 9:58 AM Junio C Hamano <gitster@pobox.com> wrote:
> > > Elijah Newren <newren@gmail.com> writes:
...
> > > 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.

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

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

      reply	other threads:[~2018-08-31 19:38 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-28 12:27 Nikolay Kasyanov
2018-08-28 13:17 ` Ævar Arnfjörð Bjarmason
2018-08-28 13:33   ` Johannes Schindelin
2018-08-28 13:46     ` Nikolay Kasyanov
2018-08-28 15:35     ` Elijah Newren
2018-08-28 16:58       ` Junio C Hamano
2018-08-29  7:06         ` [PATCH 0/3] Turn off directory rename detection in am -3 Elijah Newren
2018-08-29  7:06           ` [PATCH 1/3] t3401: add another directory rename testcase for rebase and am Elijah Newren
2018-08-29 22:12             ` Junio C Hamano
2018-08-29 23:47               ` Elijah Newren
2018-08-30 16:01                 ` Junio C Hamano
2018-08-30 16:26                   ` Elijah Newren
2018-08-29  7:06           ` [PATCH 2/3] merge-recursive: add ability to turn off directory rename detection Elijah Newren
2018-08-29 12:54             ` Johannes Schindelin
2018-08-29 23:00               ` Elijah Newren
2018-08-29  7:06           ` [PATCH 3/3] am: avoid directory rename detection when calling recursive merge machinery Elijah Newren
2018-08-29 12:51             ` Johannes Schindelin
2018-08-30 16:41         ` A rebase regression in Git 2.18.0 Elijah Newren
2018-08-31 10:11           ` Johannes Schindelin
2018-08-31 19:37             ` Elijah Newren [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABPp-BEJ-JPa8g8G1ELsjqf3EDxe+aCBdwERBi5hr5ukrDjJJg@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=corrmage@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sbeller@google.com \
    --subject='Re: A rebase regression in Git 2.18.0' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git