From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
"Johannes Sixt" <j6t@kdbg.org>,
"John Keeping" <john@keeping.me.uk>,
"Pratik Karki" <predatoramigo@gmail.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 2/2] rebase: don't rebase linear topology with --fork-point
Date: Thu, 21 Feb 2019 22:40:59 +0100 [thread overview]
Message-ID: <20190221214059.9195-3-avarab@gmail.com> (raw)
In-Reply-To: <20190221151009.GA24309@sigill.intra.peff.net>
Fix a regression introduced in 4f21454b55 ("merge-base: handle
--fork-point without reflog", 2016-10-12).
Before that change having a linear history on top of an upstream
master would with --fork-point (aka argument-less rebase) tell us
there was nothing to be done:
$ git rebase
Current branch master is up to date.
After that change "rebase" will always redundantly find that it has
work to do (it doesn't):
$ git rebase
First, rewinding head to replay your work on top of it...
Applying: [...]
Whereas equivalently running:
$ git rebase @{upstream}
$ git rebase $(git merge-base --fork-point @{u})
Gives us the old behavior of doing nothing.
Now, why did we have this regression? Fully digging into it yields an
interesting combination of causes:
Way back in 1308c17b3e ("Allow rebase to run if upstream is completely
merged", 2007-07-04) "rebase" learned to not do this redundant work
when asked to rebase on a commit that was already an ancestor of the
current commit.
Then in 1e0dacdbdb ("rebase: omit patch-identical commits with
--fork-point", 2014-07-16) a rebase bug was fixed for a case where the
history to be rebased was divergent by entirely skipping the 2007-era
logic if --fork-point was provided.
But here's the critical thing, *only* if the --fork-point was
divergent. At that time "git merge-base --fork-point A B" would return
nothing if the two commits weren't divergent.
Then in 4f21454b55 ("merge-base: handle --fork-point without reflog",
2016-10-12) which introduced the regression being fixed here, a bug
fix for "git merge-base --fork-point" being run stand-alone by proxy
broke this use-case git-rebase.sh was relying on, since it was still
assuming that if we didn't have divergent history we'd have no output.
Finally, when "rebase" was rewritten in C a combination of
9a48a615b4 ("builtin rebase: try to fast forward when possible",
2018-09-04), 103148aad8 ("merge-base --fork-point: extract libified
function", 2018-09-04) and 92d0d74e8d ("builtin rebase: support
`fork-point` option", 2018-09-04) faithfully re-implemented the
then-buggy behavior.
So let's fix this. It's easy enough, we just stop explicitly excluding
--fork-point from the can_fast_forward(...) test we're doing, which as
discussed above is faithfully ported over from buggy shellscript-era
logic.
I'm not bothering to fix this in the legacy rebase mode. As discussed
in 9aea5e9286 ("rebase: fix regression in rebase.useBuiltin=false test
mode", 2019-02-13) it'll be going away shortly after 2.21.0 lands.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/rebase.c | 6 ++++--
t/t3421-rebase-topology-linear.sh | 15 +++++++++++++++
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 7c7bc13e91..7a16b8051c 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1664,9 +1664,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
* and if this is not an interactive rebase.
*/
if (can_fast_forward(options.onto, &options.orig_head, &merge_base) &&
- !is_interactive(&options) && !options.restrict_revision &&
+ !is_interactive(&options) &&
options.upstream &&
- !oidcmp(&options.upstream->object.oid, &options.onto->object.oid)) {
+ (options.restrict_revision
+ ? !oidcmp(&options.upstream->object.oid, &options.restrict_revision->object.oid)
+ : !oidcmp(&options.upstream->object.oid, &options.onto->object.oid))) {
int flag;
if (!(options.flags & REBASE_FORCE)) {
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index b847064f91..1754537789 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -55,6 +55,21 @@ test_run_rebase success -m
test_run_rebase success -i
test_have_prereq !REBASE_P || test_run_rebase success -p
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result "rebase $* is no-op if remote upstream is an ancestor" "
+ reset_rebase &&
+ GIT_TEST_REBASE_USE_BUILTIN=true git rebase $* branch-b branch-e &&
+ test_cmp_rev e HEAD
+ "
+}
+test_run_rebase success ''
+test_run_rebase success --fork-point
+test_run_rebase success -m
+test_run_rebase success -i
+test_have_prereq !REBASE_P || test_run_rebase success -p
+
test_run_rebase () {
result=$1
shift
--
2.21.0.rc0.258.g878e2cd30e
next prev parent reply other threads:[~2019-02-21 21:41 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-14 13:23 BUG: 2.11-era rebase regression when @{upstream} is implicitly used Ævar Arnfjörð Bjarmason
2019-02-21 14:10 ` Jeff King
2019-02-21 14:50 ` Ævar Arnfjörð Bjarmason
2019-02-21 15:10 ` Jeff King
2019-02-21 21:40 ` [PATCH 0/2] rebase: fix 2.11.0-era --fork-point regression Ævar Arnfjörð Bjarmason
2019-02-21 21:40 ` [PATCH 1/2] rebase tests: test linear branch topology Ævar Arnfjörð Bjarmason
2019-02-22 14:53 ` Jeff King
2019-02-22 18:46 ` Junio C Hamano
2019-02-21 21:40 ` Ævar Arnfjörð Bjarmason [this message]
2019-02-22 15:08 ` [PATCH 2/2] rebase: don't rebase linear topology with --fork-point Jeff King
2019-02-22 16:49 ` Ævar Arnfjörð Bjarmason
2019-02-24 10:10 ` Jeff King
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=20190221214059.9195-3-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=john@keeping.me.uk \
--cc=peff@peff.net \
--cc=predatoramigo@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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).