[9/9] pull: fix handling of multiple heads

Commit Message

Elijah Newren July 17, 2021, 3:41 p.m. UTC

From: Elijah Newren <[e-mail]>

With multiple heads, we should not allow rebasing or fast-forwarding.
Also, it seems wrong to have our can_ff computation return true, so fix
that while we are at it too (we won't actually use the can_ff flag due
to setting opt_ff to "--no-ff", but it's confusing to leave it as
computed to be true).

Signed-off-by: Elijah Newren <[e-mail]>
---
 builtin/pull.c               | 20 ++++++++++++++++----
 t/t7601-merge-pull-config.sh |  4 ++--
 2 files changed, 18 insertions(+), 6 deletions(-)

Patch

diff --git a/builtin/pull.c b/builtin/pull.c
index 3a61b92f328..beaf6ee0653 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -913,12 +913,18 @@  static int run_rebase(const struct object_id *newbase,
 	return ret;
 }
 
-static int get_can_ff(struct object_id *orig_head, struct object_id *orig_merge_head)
+static int get_can_ff(struct object_id *orig_head,
+		      struct oid_array *merge_heads)
 {
 	int ret;
 	struct commit_list *list = NULL;
 	struct commit *merge_head, *head;
+	struct object_id *orig_merge_head;
 
+	if (merge_heads->nr > 1)
+		return 0;
+
+	orig_merge_head = &merge_heads->oid[0];
 	head = lookup_commit_reference(the_repository, orig_head);
 	commit_list_insert(head, &list);
 	merge_head = lookup_commit_reference(the_repository, orig_merge_head);
@@ -1046,10 +1052,16 @@  int cmd_pull(int argc, const char **argv, const char *prefix)
 			die(_("Cannot merge multiple branches into empty head."));
 		return pull_into_void(merge_heads.oid, &curr_head);
 	}
-	if (opt_rebase && merge_heads.nr > 1)
-		die(_("Cannot rebase onto multiple branches."));
+	if (merge_heads.nr > 1) {
+		if (opt_rebase)
+			die(_("Cannot rebase onto multiple branches."));
+		if (opt_ff && !strcmp(opt_ff, "--ff-only"))
+			die(_("Cannot fast-forward to multiple branches."));
+		if (!opt_ff)
+			opt_ff = "--no-ff";
+	}
 
-	can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);
+	can_ff = get_can_ff(&orig_head, &merge_heads);
 
 	/* ff-only takes precedence over rebase */
 	if (opt_ff && !strcmp(opt_ff, "--ff-only")) {
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 673b92afbab..29105b5b1ed 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -297,7 +297,7 @@  test_expect_success 'pull.rebase=true takes precedence over --ff' '
 
 # End of precedence rules
 
-test_expect_failure 'Multiple heads does not warn about fast forwarding' '
+test_expect_success 'Multiple heads does not warn about fast forwarding' '
 	git reset --hard c1 &&
 	git pull . c2 c3 2>err &&
 	test_i18ngrep ! "Pulling without specifying how to reconcile" err
@@ -307,7 +307,7 @@  test_expect_success 'Cannot fast-forward with multiple heads' '
 	git reset --hard c0 &&
 	test_must_fail git -c pull.ff=only pull . c1 c2 c3 2>err &&
 	test_i18ngrep ! "Pulling without specifying how to reconcile" err &&
-	test_i18ngrep "Not possible to fast-forward, aborting" err
+	test_i18ngrep "Cannot fast-forward to multiple branches" err
 '
 
 test_expect_success 'Cannot rebase with multiple heads' '