rebase: factor out branch_base calculation

Separate out calculating the merge base between 'onto' and 'HEAD' from the check for whether we can fast-forward or not. This means we can skip the fast-forward checks when the rebase is forced and avoid calculating the merge-base between 'HEAD' and 'onto' when --keep-base is given. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Phillip Wood committed Oct 17, 2022 at 13:17 UTC d42c9ffa0fa169ea51c971d1143b3f20d5a32d83
1 file changed +20 -13
builtin/rebase.c
+20 -13
@@ -865,13 +865,9 @@ static int can_fast_forward(struct commit *onto, struct commit *upstream,
865 struct commit_list *merge_bases = NULL;
866 int res = 0;
867
868 - merge_bases = get_merge_bases(onto, head);
869 - if (!merge_bases || merge_bases->next) {
870 - oidcpy(branch_base, null_oid());
871 - goto done;
872 - }
868 + if (is_null_oid(branch_base))
869 + goto done; /* fill_branch_base() found multiple merge bases */
870
874 - oidcpy(branch_base, &merge_bases->item->object.oid);
871 if (!oideq(branch_base, &onto->object.oid))
872 goto done;
873
@@ -881,7 +877,6 @@ static int can_fast_forward(struct commit *onto, struct commit *upstream,
877 if (!upstream)
878 goto done;
879
884 - free_commit_list(merge_bases);
880 merge_bases = get_merge_bases(upstream, head);
881 if (!merge_bases || merge_bases->next)
882 goto done;
@@ -896,6 +891,20 @@ done:
891 return res && is_linear_history(onto, head);
892 }
893
894 +static void fill_branch_base(struct rebase_options *options,
895 + struct object_id *branch_base)
896 +{
897 + struct commit_list *merge_bases = NULL;
898 +
899 + merge_bases = get_merge_bases(options->onto, options->orig_head);
900 + if (!merge_bases || merge_bases->next)
901 + oidcpy(branch_base, null_oid());
902 + else
903 + oidcpy(branch_base, &merge_bases->item->object.oid);
904 +
905 + free_commit_list(merge_bases);
906 +}
907 +
908 static int parse_opt_am(const struct option *opt, const char *arg, int unset)
909 {
910 struct rebase_options *opts = opt->value;
@@ -1660,6 +1669,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
1669 if (!options.onto)
1670 die(_("Does not point to a valid commit '%s'"),
1671 options.onto_name);
1672 + fill_branch_base(&options, &branch_base);
1673 }
1674
1675 if (options.fork_point > 0)
@@ -1689,13 +1699,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
1699 * Check if we are already based on onto with linear history,
1700 * in which case we could fast-forward without replacing the commits
1701 * with new commits recreated by replaying their changes.
1692 - *
1693 - * Note that can_fast_forward() initializes branch_base, so we have to
1694 - * call it before checking allow_preemptive_ff.
1702 */
1696 - if (can_fast_forward(options.onto, options.upstream, options.restrict_revision,
1697 - options.orig_head, &branch_base) &&
1698 - allow_preemptive_ff) {
1703 + if (allow_preemptive_ff &&
1704 + can_fast_forward(options.onto, options.upstream, options.restrict_revision,
1705 + options.orig_head, &branch_base)) {
1706 int flag;
1707
1708 if (!(options.flags & REBASE_FORCE)) {