Thanks for the comments, Junio and Phillip. I've fixed the ASCII art graphs and also refactored the if-else into a goto tower. Hopefully, this will be the last reroll of this series ;) --- This patchset now depends "[PATCH 1/8] tests (rebase): spell out the `--keep-empty` option" which is the first patch of Johannes's "Do not use abbreviated options in tests" patchset[1]. (Thanks for catching that, Johannes!) Changes since v1: * Squashed old set into one patch * Fixed indentation style and dangling else * Added more documentation after discussion with Ævar Changes since v2: * Add testing for rebase --fork-point behaviour * Add testing for rebase fast-forward behaviour * Make rebase --onto fast-forward in more cases * Update documentation to include use-case Changes since v3: * Fix tests failing on bash 4.2 * Fix typo in t3431 comment Changes since v4: * Make rebase --fork-point fast-forward in more cases Changes since v5: * Fix graph illustrations so that the "branch off" is visually in the correct place * Refactor if-else in can_fast_forward into one-level-deep ifs to increase clarity [1]: https://public-inbox.org/git/a1b4b74b9167e279dae4cd8c58fb28d8a714a66a.1553537656.git.gitgitgadget@xxxxxxxxx/ Denton Liu (6): t3431: add rebase --fork-point tests t3432: test rebase fast-forward behavior rebase: refactor can_fast_forward into goto tower rebase: fast-forward --onto in more cases rebase: fast-forward --fork-point in more cases rebase: teach rebase --keep-base Documentation/git-rebase.txt | 30 +++++++++-- builtin/rebase.c | 86 +++++++++++++++++++++++--------- t/t3400-rebase.sh | 2 +- t/t3404-rebase-interactive.sh | 2 +- t/t3416-rebase-onto-threedots.sh | 57 +++++++++++++++++++++ t/t3431-rebase-fork-point.sh | 57 +++++++++++++++++++++ t/t3432-rebase-fast-forward.sh | 83 ++++++++++++++++++++++++++++++ 7 files changed, 289 insertions(+), 28 deletions(-) create mode 100755 t/t3431-rebase-fork-point.sh create mode 100755 t/t3432-rebase-fast-forward.sh Range-diff against v5: 1: 0f1e9ac5c8 ! 1: eb64f6c91d t3431: add rebase --fork-point tests @@ -18,9 +18,9 @@ + +. ./test-lib.sh + -+# A---B---D---E (master) -+# \ -+# C*---F---G (side) ++# A---B---D---E (master) ++# \ ++# C*---F---G (side) +# +# C was formerly part of master but master was rewound to remove C +# 2: 148027a14b = 2: 4c087ec041 t3432: test rebase fast-forward behavior -: ---------- > 3: 3d348d2c37 rebase: refactor can_fast_forward into goto tower 3: ec55da0719 ! 4: 2559ab54a2 rebase: fast-forward --onto in more cases @@ -5,8 +5,8 @@ Before, when we had the following graph, A---B---C (master) - \ - D (side) + \ + D (side) running 'git rebase --onto master... master side' would result in D being always rebased, no matter what. However, the desired behavior is @@ -49,6 +49,7 @@ While we're at it, remove a trailing whitespace from rebase.c. + Helped-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> Signed-off-by: Denton Liu <liu.denton@xxxxxxxxx> diff --git a/builtin/rebase.c b/builtin/rebase.c @@ -64,45 +65,26 @@ + 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; + struct commit_list *merge_bases = NULL; @@ - 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); -+ if (!oideq(merge_base, &onto->object.oid)) -+ goto done; - } else { - oidcpy(merge_base, &null_oid); -- res = 0; -+ goto done; - } -+ + if (!oideq(merge_base, &onto->object.oid)) + goto done; + + if (!upstream) + goto done; + - free_commit_list(merge_bases); ++ free_commit_list(merge_bases); + merge_bases = get_merge_bases(upstream, head); -+ if (merge_bases && !merge_bases->next) { -+ if (!oideq(&onto->object.oid, &merge_bases->item->object.oid)) -+ goto done; -+ } else ++ if (!merge_bases || merge_bases->next) { + goto done; ++ } + -+ res = 1; ++ if (!oideq(&onto->object.oid, &merge_bases->item->object.oid)) ++ goto done; + -+done: -+ if (merge_bases) -+ free_commit_list(merge_bases); - return res && is_linear_history(onto, head); - } + res = 1; + done: @@ /* @@ -115,7 +97,8 @@ - !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) && ++ if (can_fast_forward(options.onto, options.upstream, &options.orig_head, ++ &merge_base) && + !is_interactive(&options) && !options.restrict_revision) { int flag; 4: 2256a902c1 ! 5: 0a466e830f rebase: fast-forward --fork-point in more cases @@ -27,8 +27,8 @@ { struct commit *head = lookup_commit(the_repository, head_oid); @@ + if (!oideq(merge_base, &onto->object.oid)) goto done; - } + if (restrict_revision && !oideq(&restrict_revision->object.oid, merge_base)) + goto done; @@ -40,7 +40,8 @@ * Check if we are already based on onto with linear history, * but this should be done if this is not an interactive rebase. */ -- if (can_fast_forward(options.onto, options.upstream, &options.orig_head, &merge_base) && +- 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) && 5: d6e7e1ca4b = 6: c542c7afc1 rebase: teach rebase --keep-base -- 2.21.0.944.gce45564dfd
