@matheusbsbs / matheus-bagliano / commits / c0efb4c1dd

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 <phillip.wood@dunelm.org.uk> Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Denton Liu committed Aug 27, 2019 at 01:37 UTC c0efb4c1ddccf91dedd5775f2f449574e90b051a

4 files changed +23 -12

builtin/rebase.c

+19 -8

@@ -1260,8 +1260,8 @@ static int is_linear_history(struct commit *from, struct commit *to)

1260 return 1;

1261 }

1262

1263 -static int can_fast_forward(struct commit *onto, struct object_id *head_oid,

1264 - struct object_id *merge_base)

1263 +static int can_fast_forward(struct commit *onto, struct commit *upstream,

1264 + struct object_id *head_oid, struct object_id *merge_base)

1265 {

1266 struct commit *head = lookup_commit(the_repository, head_oid);

1267 struct commit_list *merge_bases = NULL;

@@ -1280,6 +1280,17 @@ static int can_fast_forward(struct commit *onto, struct object_id *head_oid,

1280 if (!oideq(merge_base, &onto->object.oid))

1281 goto done;

1282

1283 + if (!upstream)

1284 + goto done;

1285 +

1286 + free_commit_list(merge_bases);

1287 + merge_bases = get_merge_bases(upstream, head);

1288 + if (!merge_bases || merge_bases->next)

1289 + goto done;

1290 +

1291 + if (!oideq(&onto->object.oid, &merge_bases->item->object.oid))

1292 + goto done;

1293 +

1294 res = 1;

1295

1296 done:

@@ -2027,13 +2038,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)

2038

2039 /*

2040 * Check if we are already based on onto with linear history,

2030 - * but this should be done only when upstream and onto are the same

2031 - * and if this is not an interactive rebase.

2041 + * in which case we could fast-forward without replacing the commits

2042 + * with new commits recreated by replaying their changes. This

2043 + * optimization must not be done if this is an interactive rebase.

2044 */

2033 - if (can_fast_forward(options.onto, &options.orig_head, &merge_base) &&

2034 - !is_interactive(&options) && !options.restrict_revision &&

2035 - options.upstream &&

2036 - !oidcmp(&options.upstream->object.oid, &options.onto->object.oid)) {

2045 + if (can_fast_forward(options.onto, options.upstream, &options.orig_head,

2046 + &merge_base) &&

2047 + !is_interactive(&options) && !options.restrict_revision) {

2048 int flag;

2049

2050 if (!(options.flags & REBASE_FORCE)) {

t/t3400-rebase.sh

+1 -1

@@ -295,7 +295,7 @@ test_expect_success 'rebase --am and --show-current-patch' '

295 echo two >>init.t &&

296 git commit -a -m two &&

297 git tag two &&

298 - test_must_fail git rebase --onto init HEAD^ &&

298 + test_must_fail git rebase -f --onto init HEAD^ &&

299 GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&

300 grep "show.*$(git rev-parse two)" stderr

301 )

t/t3404-rebase-interactive.sh

+1 -1

@@ -1058,7 +1058,7 @@ test_expect_success C_LOCALE_OUTPUT 'rebase --edit-todo does not work on non-int

1058 git reset --hard &&

1059 git checkout conflict-branch &&

1060 set_fake_editor &&

1061 - test_must_fail git rebase --onto HEAD~2 HEAD~ &&

1061 + test_must_fail git rebase -f --onto HEAD~2 HEAD~ &&

1062 test_must_fail git rebase --edit-todo &&

1063 git rebase --abort

1064 '

t/t3432-rebase-fast-forward.sh

+2 -2

@@ -106,9 +106,9 @@ test_expect_success 'add work same to upstream' '

106 changes='our and their changes'

107 test_rebase_same_head success noop same success noop-force diff --onto B B

108 test_rebase_same_head success noop same success noop-force diff --onto B... B

109 -test_rebase_same_head failure work same success work diff --onto master... master

109 +test_rebase_same_head success noop same success work diff --onto master... master

110 test_rebase_same_head failure work same success work diff --fork-point --onto B B

111 test_rebase_same_head failure work same success work diff --fork-point --onto B... B

112 -test_rebase_same_head failure work same success work diff --fork-point --onto master... master

112 +test_rebase_same_head success noop same success work diff --fork-point --onto master... master

113

114 test_done