Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Alban Gruin <alban.gruin@gmail.com>
Cc: "SZEDER Gábor" <szeder.dev@gmail.com>,
	git@vger.kernel.org, "Elijah Newren" <newren@gmail.com>,
	"Ævar Arnfjörð" <avarab@gmail.com>,
	"Eugeniu Rosca" <erosca@de.adit-jv.com>,
	"Jeff King" <peff@peff.net>,
	"Eugeniu Rosca" <roscaeugeniu@gmail.com>,
	"Phillip Wood" <phillip.wood@dunelm.org.uk>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v2] rebase -i: stop checking out the tip of the branch to rebase
Date: Fri, 24 Jan 2020 10:12:30 -0800
Message-ID: <xmqqv9p0206p.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <9221098c-cba4-5db5-5870-bf6af721c448@gmail.com>

Alban Gruin <alban.gruin@gmail.com> writes:

> Forget this patch, I forgot to clearly say that the `am' backend is not
> affected.

The phrase "is not affected" makes it sound like "Do not worry, I
made sure it is not broken by this patch", but I do not think that
is the more important part ;-).  

The shared codepath for all types of rebase before dispatching
already knew what commit at the tip of the branch being rebased is,
but the sequencer-based backend was doing unnecessary work to figure
it out again by checking it out.  And this patch is about fixing
that, isn't it?

So I do not think singling out 'am' is a good use of readers' time.
The first paragraph can be further tweaked why the extra checkout is
unneeded.

    Before dispatching the control to one of the individual rebase
    backends, the shared codepath in "rebase" figures out what
    branch is being rebased, because it is necessary to compute the
    range of commits to replay to run any backend.  The rebase
    backend based on the sequencer machinery (used for '-i', '-r'
    and '-m') however computed this commit range by actually
    checking out the branch and reading HEAD, which was unnecessary,
    as the working tree is then immediately gets reset to that of
    the commit on which rebased history is built (aka "onto"
    commit).

or something along the line, perhaps?

With this patch applied, the wasteful prepare_branch_to_be_rebased()
has no caller, and the patch removes it from sequencer.[ch] as well,
which is very good.



  reply index

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-08 21:43 Unreliable 'git rebase --onto' Eugeniu Rosca
2020-01-08 22:35 ` SZEDER Gábor
2020-01-09  0:55   ` Elijah Newren
2020-01-09 15:03     ` SZEDER Gábor
2020-01-09 17:53       ` Elijah Newren
2020-01-21 19:18       ` [PATCH v1] rebase -i: stop checking out the tip of the branch to rebase Alban Gruin
2020-01-21 20:07         ` Elijah Newren
2020-01-22 20:24         ` Junio C Hamano
2020-01-22 20:47         ` Junio C Hamano
2020-01-24 14:45           ` Alban Gruin
2020-01-24 14:45         ` [PATCH v2] " Alban Gruin
2020-01-24 14:55           ` Alban Gruin
2020-01-24 18:12             ` Junio C Hamano [this message]
2020-01-24 15:05           ` [PATCH v3] " Alban Gruin
2020-01-24 18:30             ` Junio C Hamano
2020-02-05 14:31             ` Johannes Schindelin
2020-01-24 17:11           ` [PATCH v2] " Andrei Rybak
2020-01-09 11:13   ` Unreliable 'git rebase --onto' Eugeniu Rosca
     [not found] ` <CABPp-BHsyMOz+hi7EYoAnAWfzms7FRfwqCoarnu8H+vyDoN6SQ@mail.gmail.com>
2020-01-09 10:53   ` Eugeniu Rosca
2020-01-09 18:05     ` Elijah Newren
2020-01-10  0:06       ` Eugeniu Rosca
2020-01-10  2:35         ` Elijah Newren

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

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

  git send-email \
    --in-reply-to=xmqqv9p0206p.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=alban.gruin@gmail.com \
    --cc=avarab@gmail.com \
    --cc=erosca@de.adit-jv.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=roscaeugeniu@gmail.com \
    --cc=szeder.dev@gmail.com \
    /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 Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

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

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


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