From 359ecebc346eeb3c74af82f25d8fce94d3ce35a4 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Sun, 25 Aug 2019 05:11:57 -0400 Subject: [PATCH 1/9] t3431: add rebase --fork-point tests Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3431-rebase-fork-point.sh | 53 ++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100755 t/t3431-rebase-fork-point.sh diff --git a/t/t3431-rebase-fork-point.sh b/t/t3431-rebase-fork-point.sh new file mode 100755 index 0000000000..2d5c6e641e --- /dev/null +++ b/t/t3431-rebase-fork-point.sh @@ -0,0 +1,53 @@ +#!/bin/sh +# +# Copyright (c) 2019 Denton Liu +# + +test_description='git rebase --fork-point test' + +. ./test-lib.sh + +# A---B---D---E (master) +# \ +# C*---F---G (side) +# +# C was formerly part of master but master was rewound to remove C +# +test_expect_success setup ' + test_commit A && + test_commit B && + test_commit C && + git branch -t side && + git reset --hard HEAD^ && + test_commit D && + test_commit E && + git checkout side && + test_commit F && + test_commit G +' + +test_rebase () { + expected="$1" && + shift && + test_expect_success "git rebase $*" " + git checkout master && + git reset --hard E && + git checkout side && + git reset --hard G && + git rebase $* && + test_write_lines $expected >expect && + git log --pretty=%s >actual && + test_cmp expect actual + " +} + +test_rebase 'G F E D B A' +test_rebase 'G F D B A' --onto D +test_rebase 'G F C E D B A' --no-fork-point +test_rebase 'G F C D B A' --no-fork-point --onto D +test_rebase 'G F E D B A' --fork-point refs/heads/master +test_rebase 'G F D B A' --fork-point --onto D refs/heads/master +test_rebase 'G F C E D B A' refs/heads/master +test_rebase 'G F C D B A' --onto D refs/heads/master + +test_done From 793ac7e309437e2225825370a9c7a0f1ccb39480 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Sun, 25 Aug 2019 05:12:00 -0400 Subject: [PATCH 2/9] t3432: test rebase fast-forward behavior When rebase is run on a branch that can be fast-forwarded, this should automatically be done. Create test to ensure this behavior happens. There are some cases that currently don't pass. The first case is where a feature and master have diverged, running "git rebase master... master" causes a full rebase to happen even though a fast-forward should happen. The second case is when we are doing "git rebase --fork-point" and a fork-point commit is found. Once again, a full rebase happens even though a fast-forward should happen. Mark these cases as failure so we can fix it later. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3432-rebase-fast-forward.sh | 72 ++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100755 t/t3432-rebase-fast-forward.sh diff --git a/t/t3432-rebase-fast-forward.sh b/t/t3432-rebase-fast-forward.sh new file mode 100755 index 0000000000..f49af274e0 --- /dev/null +++ b/t/t3432-rebase-fast-forward.sh @@ -0,0 +1,72 @@ +#!/bin/sh +# +# Copyright (c) 2019 Denton Liu +# + +test_description='ensure rebase fast-forwards commits when possible' + +. ./test-lib.sh + +test_expect_success setup ' + test_commit A && + test_commit B && + test_commit C && + test_commit D && + git checkout -t -b side +' + +test_rebase_same_head () { + status="$1" && + shift && + test_expect_$status "git rebase $* with $changes is no-op" " + oldhead=\$(git rev-parse HEAD) && + test_when_finished 'git reset --hard \$oldhead' && + git rebase $* && + newhead=\$(git rev-parse HEAD) && + test_cmp_rev \$oldhead \$newhead + " +} + +changes='no changes' +test_rebase_same_head success +test_rebase_same_head success master +test_rebase_same_head success --onto B B +test_rebase_same_head success --onto B... B +test_rebase_same_head success --onto master... master +test_rebase_same_head success --no-fork-point +test_rebase_same_head success --fork-point master +test_rebase_same_head failure --fork-point --onto B B +test_rebase_same_head failure --fork-point --onto B... B +test_rebase_same_head success --fork-point --onto master... master + +test_expect_success 'add work to side' ' + test_commit E +' + +changes='our changes' +test_rebase_same_head success +test_rebase_same_head success master +test_rebase_same_head success --onto B B +test_rebase_same_head success --onto B... B +test_rebase_same_head success --onto master... master +test_rebase_same_head success --no-fork-point +test_rebase_same_head success --fork-point master +test_rebase_same_head failure --fork-point --onto B B +test_rebase_same_head failure --fork-point --onto B... B +test_rebase_same_head success --fork-point --onto master... master + +test_expect_success 'add work to upstream' ' + git checkout master && + test_commit F && + git checkout side +' + +changes='our and their changes' +test_rebase_same_head success --onto B B +test_rebase_same_head success --onto B... B +test_rebase_same_head failure --onto master... master +test_rebase_same_head failure --fork-point --onto B B +test_rebase_same_head failure --fork-point --onto B... B +test_rebase_same_head failure --fork-point --onto master... master + +test_done From 4336d365123e45be0ed4a7286df54ccce560d55f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Sun, 25 Aug 2019 05:12:02 -0400 Subject: [PATCH 3/9] t3432: distinguish "noop-same" v.s. "work-same" in "same head" tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change "same head" introduced in the preceding commit to check whether the rebase.c code lands in the can_fast_forward() case in, and thus prints out an "is up to date" and aborts early. In some of these cases we make it past that and to "rewinding head", then do a rebase, only to find out there's nothing to change so HEAD stays at the same OID. These tests presumed these two cases were the same thing. In terms of where HEAD ends up they are, but we're not only interested in rebase semantics, but also whether or not we're needlessly doing work when we could avoid it entirely. I'm adding "same" and "diff" here because I'll follow-up and add --no-ff tests, where some of those will be "diff"-erent, so add the "diff" code already. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3432-rebase-fast-forward.sh | 79 +++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 31 deletions(-) diff --git a/t/t3432-rebase-fast-forward.sh b/t/t3432-rebase-fast-forward.sh index f49af274e0..d9f20fa07c 100755 --- a/t/t3432-rebase-fast-forward.sh +++ b/t/t3432-rebase-fast-forward.sh @@ -18,55 +18,72 @@ test_expect_success setup ' test_rebase_same_head () { status="$1" && shift && - test_expect_$status "git rebase $* with $changes is no-op" " + what="$1" && + shift && + cmp="$1" && + shift && + test_expect_$status "git rebase $* with $changes is $what" " oldhead=\$(git rev-parse HEAD) && test_when_finished 'git reset --hard \$oldhead' && - git rebase $* && + git rebase $* >stdout && + if test $what = work + then + test_i18ngrep 'rewinding head' stdout + elif test $what = noop + then + test_i18ngrep 'is up to date' stdout + fi && newhead=\$(git rev-parse HEAD) && - test_cmp_rev \$oldhead \$newhead + if test $cmp = same + then + test_cmp_rev \$oldhead \$newhead + elif test $cmp = diff + then + ! test_cmp_rev \$oldhead \$newhead + fi " } changes='no changes' -test_rebase_same_head success -test_rebase_same_head success master -test_rebase_same_head success --onto B B -test_rebase_same_head success --onto B... B -test_rebase_same_head success --onto master... master -test_rebase_same_head success --no-fork-point -test_rebase_same_head success --fork-point master -test_rebase_same_head failure --fork-point --onto B B -test_rebase_same_head failure --fork-point --onto B... B -test_rebase_same_head success --fork-point --onto master... master +test_rebase_same_head success work same +test_rebase_same_head success noop same master +test_rebase_same_head success noop same --onto B B +test_rebase_same_head success noop same --onto B... B +test_rebase_same_head success noop same --onto master... master +test_rebase_same_head success noop same --no-fork-point +test_rebase_same_head success work same --fork-point master +test_rebase_same_head failure noop same --fork-point --onto B B +test_rebase_same_head failure work same --fork-point --onto B... B +test_rebase_same_head success work same --fork-point --onto master... master -test_expect_success 'add work to side' ' +test_expect_success 'add work same to side' ' test_commit E ' changes='our changes' -test_rebase_same_head success -test_rebase_same_head success master -test_rebase_same_head success --onto B B -test_rebase_same_head success --onto B... B -test_rebase_same_head success --onto master... master -test_rebase_same_head success --no-fork-point -test_rebase_same_head success --fork-point master -test_rebase_same_head failure --fork-point --onto B B -test_rebase_same_head failure --fork-point --onto B... B -test_rebase_same_head success --fork-point --onto master... master +test_rebase_same_head success work same +test_rebase_same_head success noop same master +test_rebase_same_head success noop same --onto B B +test_rebase_same_head success noop same --onto B... B +test_rebase_same_head success noop same --onto master... master +test_rebase_same_head success noop same --no-fork-point +test_rebase_same_head success work same --fork-point master +test_rebase_same_head failure work same --fork-point --onto B B +test_rebase_same_head failure work same --fork-point --onto B... B +test_rebase_same_head success work same --fork-point --onto master... master -test_expect_success 'add work to upstream' ' +test_expect_success 'add work same to upstream' ' git checkout master && test_commit F && git checkout side ' changes='our and their changes' -test_rebase_same_head success --onto B B -test_rebase_same_head success --onto B... B -test_rebase_same_head failure --onto master... master -test_rebase_same_head failure --fork-point --onto B B -test_rebase_same_head failure --fork-point --onto B... B -test_rebase_same_head failure --fork-point --onto master... master +test_rebase_same_head success noop same --onto B B +test_rebase_same_head success noop same --onto B... B +test_rebase_same_head failure work same --onto master... master +test_rebase_same_head failure work same --fork-point --onto B B +test_rebase_same_head failure work same --fork-point --onto B... B +test_rebase_same_head failure work same --fork-point --onto master... master test_done From c9efc216830fa2fb71e7d917294e4530a33a4da1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 27 Aug 2019 01:37:53 -0400 Subject: [PATCH 4/9] t3432: test for --no-ff's interaction with fast-forward MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add more stress tests for the can_fast_forward() case in rebase.c. These tests are getting rather verbose, but now we can see under --ff and --no-ff whether we skip work, or whether we're forced to run the rebase. These tests aren't supposed to endorse the status quo, just test for what we're currently doing. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3432-rebase-fast-forward.sh | 83 ++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 29 deletions(-) diff --git a/t/t3432-rebase-fast-forward.sh b/t/t3432-rebase-fast-forward.sh index d9f20fa07c..58ecb33e08 100755 --- a/t/t3432-rebase-fast-forward.sh +++ b/t/t3432-rebase-fast-forward.sh @@ -16,22 +16,47 @@ test_expect_success setup ' ' test_rebase_same_head () { + status_n="$1" && + shift && + what_n="$1" && + shift && + cmp_n="$1" && + shift && + status_f="$1" && + shift && + what_f="$1" && + shift && + cmp_f="$1" && + shift && + test_rebase_same_head_ $status_n $what_n $cmp_n "" "$*" && + test_rebase_same_head_ $status_f $what_f $cmp_f " --no-ff" "$*" +} + +test_rebase_same_head_ () { status="$1" && shift && what="$1" && shift && cmp="$1" && shift && - test_expect_$status "git rebase $* with $changes is $what" " + flag="$1" + shift && + test_expect_$status "git rebase$flag $* with $changes is $what with $cmp HEAD" " oldhead=\$(git rev-parse HEAD) && test_when_finished 'git reset --hard \$oldhead' && - git rebase $* >stdout && + git rebase$flag $* >stdout && if test $what = work then + # Must check this case first, for 'is up to + # date, rebase forced[...]rewinding head' cases test_i18ngrep 'rewinding head' stdout elif test $what = noop then - test_i18ngrep 'is up to date' stdout + test_i18ngrep 'is up to date' stdout && + test_i18ngrep ! 'rebase forced' stdout + elif test $what = noop-force + then + test_i18ngrep 'is up to date, rebase forced' stdout fi && newhead=\$(git rev-parse HEAD) && if test $cmp = same @@ -45,32 +70,32 @@ test_rebase_same_head () { } changes='no changes' -test_rebase_same_head success work same -test_rebase_same_head success noop same master -test_rebase_same_head success noop same --onto B B -test_rebase_same_head success noop same --onto B... B -test_rebase_same_head success noop same --onto master... master -test_rebase_same_head success noop same --no-fork-point -test_rebase_same_head success work same --fork-point master -test_rebase_same_head failure noop same --fork-point --onto B B -test_rebase_same_head failure work same --fork-point --onto B... B -test_rebase_same_head success work same --fork-point --onto master... master +test_rebase_same_head success work same success work same +test_rebase_same_head success noop same success noop-force same master +test_rebase_same_head success noop same success noop-force diff --onto B B +test_rebase_same_head success noop same success noop-force diff --onto B... B +test_rebase_same_head success noop same success noop-force same --onto master... master +test_rebase_same_head success noop same success noop-force same --no-fork-point +test_rebase_same_head success work same success work same --fork-point master +test_rebase_same_head failure noop same success work diff --fork-point --onto B B +test_rebase_same_head failure work same success work diff --fork-point --onto B... B +test_rebase_same_head success work same success work same --fork-point --onto master... master test_expect_success 'add work same to side' ' test_commit E ' changes='our changes' -test_rebase_same_head success work same -test_rebase_same_head success noop same master -test_rebase_same_head success noop same --onto B B -test_rebase_same_head success noop same --onto B... B -test_rebase_same_head success noop same --onto master... master -test_rebase_same_head success noop same --no-fork-point -test_rebase_same_head success work same --fork-point master -test_rebase_same_head failure work same --fork-point --onto B B -test_rebase_same_head failure work same --fork-point --onto B... B -test_rebase_same_head success work same --fork-point --onto master... master +test_rebase_same_head success work same success work same +test_rebase_same_head success noop same success noop-force same master +test_rebase_same_head success noop same success noop-force diff --onto B B +test_rebase_same_head success noop same success noop-force diff --onto B... B +test_rebase_same_head success noop same success noop-force same --onto master... master +test_rebase_same_head success noop same success noop-force same --no-fork-point +test_rebase_same_head success work same success work same --fork-point master +test_rebase_same_head failure work same success work diff --fork-point --onto B B +test_rebase_same_head failure work same success work diff --fork-point --onto B... B +test_rebase_same_head success work same success work same --fork-point --onto master... master test_expect_success 'add work same to upstream' ' git checkout master && @@ -79,11 +104,11 @@ test_expect_success 'add work same to upstream' ' ' changes='our and their changes' -test_rebase_same_head success noop same --onto B B -test_rebase_same_head success noop same --onto B... B -test_rebase_same_head failure work same --onto master... master -test_rebase_same_head failure work same --fork-point --onto B B -test_rebase_same_head failure work same --fork-point --onto B... B -test_rebase_same_head failure work same --fork-point --onto master... master +test_rebase_same_head success noop same success noop-force diff --onto B B +test_rebase_same_head success noop same success noop-force diff --onto B... B +test_rebase_same_head failure work same success work diff --onto master... master +test_rebase_same_head failure work same success work diff --fork-point --onto B B +test_rebase_same_head failure work same success work diff --fork-point --onto B... B +test_rebase_same_head failure work same success work diff --fork-point --onto master... master test_done From 2b318aa6c3566ad65d9547f42828f277955e6519 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 27 Aug 2019 01:37:56 -0400 Subject: [PATCH 5/9] rebase: refactor can_fast_forward into goto tower Before, can_fast_forward was written with an if-else statement. However, in the future, we may be adding more termination cases which would lead to deeply nested if statements. Refactor to use a goto tower so that future cases can be easily inserted. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- builtin/rebase.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 670096c065..1ddad46126 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1264,20 +1264,25 @@ static int can_fast_forward(struct commit *onto, struct object_id *head_oid, struct object_id *merge_base) { struct commit *head = lookup_commit(the_repository, head_oid); - struct commit_list *merge_bases; - int res; + struct commit_list *merge_bases = NULL; + int res = 0; if (!head) - return 0; + goto done; merge_bases = get_merge_bases(onto, head); - if (merge_bases && !merge_bases->next) { - oidcpy(merge_base, &merge_bases->item->object.oid); - res = oideq(merge_base, &onto->object.oid); - } else { + if (!merge_bases || merge_bases->next) { oidcpy(merge_base, &null_oid); - res = 0; + goto done; } + + oidcpy(merge_base, &merge_bases->item->object.oid); + if (!oideq(merge_base, &onto->object.oid)) + goto done; + + res = 1; + +done: free_commit_list(merge_bases); return res && is_linear_history(onto, head); } From c0efb4c1ddccf91dedd5775f2f449574e90b051a Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 27 Aug 2019 01:37:59 -0400 Subject: [PATCH 6/9] rebase: fast-forward --onto in more cases Before, when we had the following graph, A---B---C (master) \ D (side) running 'git rebase --onto master... master side' would result in D being always rebased, no matter what. However, the desired behavior is that rebase should notice that this is fast-forwardable and do that instead. Add detection to `can_fast_forward` so that this case can be detected and a fast-forward will be performed. First of all, rewrite the function to use gotos which simplifies the logic. Next, since the options.upstream && !oidcmp(&options.upstream->object.oid, &options.onto->object.oid) conditions were removed in `cmd_rebase`, we reintroduce a substitute in `can_fast_forward`. In particular, checking the merge bases of `upstream` and `head` fixes a failing case in t3416. The abbreviated graph for t3416 is as follows: F---G topic / A---B---C---D---E master and the failing command was git rebase --onto master...topic F topic Before, Git would see that there was one merge base (C), and the merge and onto were the same so it would incorrectly return 1, indicating that we could fast-forward. This would cause the rebased graph to be 'ABCFG' when we were expecting 'ABCG'. With the additional logic, we detect that upstream and head's merge base is F. Since onto isn't F, it means we're not rebasing the full set of commits from master..topic. Since we're excluding some commits, a fast-forward cannot be performed and so we correctly return 0. Add '-f' to test cases that failed as a result of this change because they were not expecting a fast-forward so that a rebase is forced. Helped-by: Phillip Wood Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- builtin/rebase.c | 27 +++++++++++++++++++-------- t/t3400-rebase.sh | 2 +- t/t3404-rebase-interactive.sh | 2 +- t/t3432-rebase-fast-forward.sh | 4 ++-- 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 1ddad46126..1e1406c8ba 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1260,8 +1260,8 @@ static int is_linear_history(struct commit *from, struct commit *to) return 1; } -static int can_fast_forward(struct commit *onto, struct object_id *head_oid, - struct object_id *merge_base) +static int can_fast_forward(struct commit *onto, struct commit *upstream, + struct object_id *head_oid, struct object_id *merge_base) { struct commit *head = lookup_commit(the_repository, head_oid); struct commit_list *merge_bases = NULL; @@ -1280,6 +1280,17 @@ static int can_fast_forward(struct commit *onto, struct object_id *head_oid, if (!oideq(merge_base, &onto->object.oid)) goto done; + if (!upstream) + goto done; + + free_commit_list(merge_bases); + merge_bases = get_merge_bases(upstream, head); + if (!merge_bases || merge_bases->next) + goto done; + + if (!oideq(&onto->object.oid, &merge_bases->item->object.oid)) + goto done; + res = 1; done: @@ -2027,13 +2038,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) /* * Check if we are already based on onto with linear history, - * but this should be done only when upstream and onto are the same - * and if this is not an interactive rebase. + * in which case we could fast-forward without replacing the commits + * with new commits recreated by replaying their changes. This + * optimization must not be done if this is an interactive rebase. */ - if (can_fast_forward(options.onto, &options.orig_head, &merge_base) && - !is_interactive(&options) && !options.restrict_revision && - options.upstream && - !oidcmp(&options.upstream->object.oid, &options.onto->object.oid)) { + if (can_fast_forward(options.onto, options.upstream, &options.orig_head, + &merge_base) && + !is_interactive(&options) && !options.restrict_revision) { int flag; if (!(options.flags & REBASE_FORCE)) { diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index 80b23fd326..d7c724bea3 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -295,7 +295,7 @@ test_expect_success 'rebase --am and --show-current-patch' ' echo two >>init.t && git commit -a -m two && git tag two && - test_must_fail git rebase --onto init HEAD^ && + test_must_fail git rebase -f --onto init HEAD^ && GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr && grep "show.*$(git rev-parse two)" stderr ) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 461dd539ff..3cc9052f10 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1058,7 +1058,7 @@ test_expect_success C_LOCALE_OUTPUT 'rebase --edit-todo does not work on non-int git reset --hard && git checkout conflict-branch && set_fake_editor && - test_must_fail git rebase --onto HEAD~2 HEAD~ && + test_must_fail git rebase -f --onto HEAD~2 HEAD~ && test_must_fail git rebase --edit-todo && git rebase --abort ' diff --git a/t/t3432-rebase-fast-forward.sh b/t/t3432-rebase-fast-forward.sh index 58ecb33e08..35bc94142d 100755 --- a/t/t3432-rebase-fast-forward.sh +++ b/t/t3432-rebase-fast-forward.sh @@ -106,9 +106,9 @@ test_expect_success 'add work same to upstream' ' changes='our and their changes' test_rebase_same_head success noop same success noop-force diff --onto B B test_rebase_same_head success noop same success noop-force diff --onto B... B -test_rebase_same_head failure work same success work diff --onto master... master +test_rebase_same_head success noop same success work diff --onto master... master test_rebase_same_head failure work same success work diff --fork-point --onto B B test_rebase_same_head failure work same success work diff --fork-point --onto B... B -test_rebase_same_head failure work same success work diff --fork-point --onto master... master +test_rebase_same_head success noop same success work diff --fork-point --onto master... master test_done From 4effc5bc96ab8cd057e034c11d6f4b93b49cb0a3 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 27 Aug 2019 01:38:01 -0400 Subject: [PATCH 7/9] rebase: fast-forward --fork-point in more cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before, when we rebased with a --fork-point invocation where the fork-point wasn't empty, we would be setting options.restrict_revision. The fast-forward logic would automatically declare that the rebase was not fast-forwardable if it was set. However, this was painting with a very broad brush. Refine the logic so that we can fast-forward in the case where the restricted revision is equal to the merge base, since we stop rebasing at the merge base anyway. Helped-by: Ævar Arnfjörð Bjarmason Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- builtin/rebase.c | 10 +++++++--- t/t3432-rebase-fast-forward.sh | 20 ++++++++++---------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 1e1406c8ba..7ef9095e7c 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1261,6 +1261,7 @@ static int is_linear_history(struct commit *from, struct commit *to) } static int can_fast_forward(struct commit *onto, struct commit *upstream, + struct commit *restrict_revision, struct object_id *head_oid, struct object_id *merge_base) { struct commit *head = lookup_commit(the_repository, head_oid); @@ -1280,6 +1281,9 @@ static int can_fast_forward(struct commit *onto, struct commit *upstream, if (!oideq(merge_base, &onto->object.oid)) goto done; + if (restrict_revision && !oideq(&restrict_revision->object.oid, merge_base)) + goto done; + if (!upstream) goto done; @@ -2042,9 +2046,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) * with new commits recreated by replaying their changes. This * optimization must not be done if this is an interactive rebase. */ - if (can_fast_forward(options.onto, options.upstream, &options.orig_head, - &merge_base) && - !is_interactive(&options) && !options.restrict_revision) { + if (can_fast_forward(options.onto, options.upstream, options.restrict_revision, + &options.orig_head, &merge_base) && + !is_interactive(&options)) { int flag; if (!(options.flags & REBASE_FORCE)) { diff --git a/t/t3432-rebase-fast-forward.sh b/t/t3432-rebase-fast-forward.sh index 35bc94142d..9ce0750b43 100755 --- a/t/t3432-rebase-fast-forward.sh +++ b/t/t3432-rebase-fast-forward.sh @@ -70,32 +70,32 @@ test_rebase_same_head_ () { } changes='no changes' -test_rebase_same_head success work same success work same +test_rebase_same_head success noop same success work same test_rebase_same_head success noop same success noop-force same master test_rebase_same_head success noop same success noop-force diff --onto B B test_rebase_same_head success noop same success noop-force diff --onto B... B test_rebase_same_head success noop same success noop-force same --onto master... master test_rebase_same_head success noop same success noop-force same --no-fork-point -test_rebase_same_head success work same success work same --fork-point master -test_rebase_same_head failure noop same success work diff --fork-point --onto B B -test_rebase_same_head failure work same success work diff --fork-point --onto B... B -test_rebase_same_head success work same success work same --fork-point --onto master... master +test_rebase_same_head success noop same success work same --fork-point master +test_rebase_same_head success noop same success work diff --fork-point --onto B B +test_rebase_same_head success noop same success work diff --fork-point --onto B... B +test_rebase_same_head success noop same success work same --fork-point --onto master... master test_expect_success 'add work same to side' ' test_commit E ' changes='our changes' -test_rebase_same_head success work same success work same +test_rebase_same_head success noop same success work same test_rebase_same_head success noop same success noop-force same master test_rebase_same_head success noop same success noop-force diff --onto B B test_rebase_same_head success noop same success noop-force diff --onto B... B test_rebase_same_head success noop same success noop-force same --onto master... master test_rebase_same_head success noop same success noop-force same --no-fork-point -test_rebase_same_head success work same success work same --fork-point master -test_rebase_same_head failure work same success work diff --fork-point --onto B B -test_rebase_same_head failure work same success work diff --fork-point --onto B... B -test_rebase_same_head success work same success work same --fork-point --onto master... master +test_rebase_same_head success noop same success work same --fork-point master +test_rebase_same_head success noop same success work diff --fork-point --onto B B +test_rebase_same_head success noop same success work diff --fork-point --onto B... B +test_rebase_same_head success noop same success work same --fork-point --onto master... master test_expect_success 'add work same to upstream' ' git checkout master && From 6330209d7d34f72e2e1511f7e42e267b926ff64d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 27 Aug 2019 01:38:04 -0400 Subject: [PATCH 8/9] rebase tests: test linear branch topology MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add tests rebasing a linear branch topology to linear rebase tests added in 2aad7cace2 ("add simple tests of consistency across rebase types", 2013-06-06). These tests are duplicates of two surrounding tests that do the same with tags pointing to the same objects. Right now there's no change in behavior being introduced, but as we'll see in a subsequent change rebase can have different behaviors when working implicitly with remote tracking branches. While I'm at it add a --fork-point test, strictly speaking this is redundant to the existing '' test, as no argument to rebase implies --fork-point. But now it's easier to grep for tests that explicitly stress --fork-point. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3421-rebase-topology-linear.sh | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh index 7274dca40b..b847064f91 100755 --- a/t/t3421-rebase-topology-linear.sh +++ b/t/t3421-rebase-topology-linear.sh @@ -31,6 +31,16 @@ test_run_rebase success -m test_run_rebase success -i test_have_prereq !REBASE_P || test_run_rebase success -p +test_expect_success 'setup branches and remote tracking' ' + git tag -l >tags && + for tag in $(cat tags) + do + git branch branch-$tag $tag || return 1 + done && + git remote add origin "file://$PWD" && + git fetch origin +' + test_run_rebase () { result=$1 shift @@ -57,10 +67,28 @@ test_run_rebase () { " } 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 failure -p +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* -f rewrites even if remote upstream is an ancestor" " + reset_rebase && + git rebase $* -f branch-b branch-e && + ! test_cmp_rev branch-e origin/branch-e && + test_cmp_rev branch-b HEAD~2 && + test_linear_range 'd e' branch-b.. + " +} +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 @@ -71,6 +99,7 @@ test_run_rebase () { " } 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 From 414d924beb41b9f39744b5574231856d5ffbcba8 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 27 Aug 2019 01:38:06 -0400 Subject: [PATCH 9/9] rebase: teach rebase --keep-base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A common scenario is if a user is working on a topic branch and they wish to make some changes to intermediate commits or autosquash, they would run something such as git rebase -i --onto master... master in order to preserve the merge base. This is useful when contributing a patch series to the Git mailing list, one often starts on top of the current 'master'. While developing the patches, 'master' is also developed further and it is sometimes not the best idea to keep rebasing on top of 'master', but to keep the base commit as-is. In addition to this, a user wishing to test individual commits in a topic branch without changing anything may run git rebase -x ./test.sh master... master Since rebasing onto the merge base of the branch and the upstream is such a common case, introduce the --keep-base option as a shortcut. This allows us to rewrite the above as git rebase -i --keep-base master and git rebase -x ./test.sh --keep-base master respectively. Add tests to ensure --keep-base works correctly in the normal case and fails when there are multiple merge bases, both in regular and interactive mode. Also, test to make sure conflicting options cause rebase to fail. While we're adding test cases, add a missing set_fake_editor call to 'rebase -i --onto master...side'. While we're documenting the --keep-base option, change an instance of "merge-base" to "merge base", which is the consistent spelling. Helped-by: Eric Sunshine Helped-by: Junio C Hamano Helped-by: Ævar Arnfjörð Bjarmason Helped-by: Johannes Schindelin Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- Documentation/git-rebase.txt | 30 ++++++++++++-- builtin/rebase.c | 32 ++++++++++++--- contrib/completion/git-completion.bash | 2 +- t/t3416-rebase-onto-threedots.sh | 57 ++++++++++++++++++++++++++ t/t3431-rebase-fork-point.sh | 4 ++ t/t3432-rebase-fast-forward.sh | 11 +++++ 6 files changed, 126 insertions(+), 10 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 6156609cf7..3146c1592d 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -8,8 +8,8 @@ git-rebase - Reapply commits on top of another base tip SYNOPSIS -------- [verse] -'git rebase' [-i | --interactive] [] [--exec ] [--onto ] - [ []] +'git rebase' [-i | --interactive] [] [--exec ] + [--onto | --keep-base] [ []] 'git rebase' [-i | --interactive] [] [--exec ] [--onto ] --root [] 'git rebase' (--continue | --skip | --abort | --quit | --edit-todo | --show-current-patch) @@ -217,6 +217,24 @@ As a special case, you may use "A\...B" as a shortcut for the merge base of A and B if there is exactly one merge base. You can leave out at most one of A and B, in which case it defaults to HEAD. +--keep-base:: + Set the starting point at which to create the new commits to the + merge base of . Running + 'git rebase --keep-base ' is equivalent to + running 'git rebase --onto ... '. ++ +This option is useful in the case where one is developing a feature on +top of an upstream branch. While the feature is being worked on, the +upstream branch may advance and it may not be the best idea to keep +rebasing on top of the upstream but to keep the base commit as-is. ++ +Although both this option and --fork-point find the merge base between + and , this option uses the merge base as the _starting +point_ on which new commits will be created, whereas --fork-point uses +the merge base to determine the _set of commits_ which will be rebased. ++ +See also INCOMPATIBLE OPTIONS below. + :: Upstream branch to compare against. May be any valid commit, not just an existing branch name. Defaults to the configured @@ -369,6 +387,10 @@ ends up being empty, the will be used as a fallback. + If either or --root is given on the command line, then the default is `--no-fork-point`, otherwise the default is `--fork-point`. ++ +If your branch was based on but was rewound and +your branch contains commits which were dropped, this option can be used +with `--keep-base` in order to drop those commits from your branch. --ignore-whitespace:: --whitespace=