@matheusbsbs / matheus-bagliano / commits / 361cb52383

pull: --ff-only should make it a noop when already-up-to-date

Earlier, we made sure that "git pull --ff-only" (and "git -c pull.ff=only pull") errors out when our current HEAD is not an ancestor of the tip of the history we are merging, but the condition to trigger the error was implemented incorrectly. Imagine you forked from a remote branch, built your history on top of it, and then attempted to pull from them again. If they have not made any update in the meantime, our current HEAD is obviously not their ancestor, and this new error triggers. Without the --ff-only option, we just report that there is no need to pull; we did the same historically with --ff-only, too. Make sure we do not fail with the recently added check to restore the historical behaviour. Reported-by: Kenneth Arnold <ka37@calvin.edu> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Oct 20, 2021 at 10:28 UTC 361cb52383fb986f76a34506bdec9a1dd11133f0
2 files changed +43 -2
builtin/pull.c
+28 -1
@@ -933,6 +933,33 @@ static int get_can_ff(struct object_id *orig_head,
933 return ret;
934 }
935
936 +/*
937 + * Is orig_head a descendant of _all_ merge_heads?
938 + * Unfortunately is_descendant_of() cannot be used as it asks
939 + * if orig_head is a descendant of at least one of them.
940 + */
941 +static int already_up_to_date(struct object_id *orig_head,
942 + struct oid_array *merge_heads)
943 +{
944 + int i;
945 + struct commit *ours;
946 +
947 + ours = lookup_commit_reference(the_repository, orig_head);
948 + for (i = 0; i < merge_heads->nr; i++) {
949 + struct commit_list *list = NULL;
950 + struct commit *theirs;
951 + int ok;
952 +
953 + theirs = lookup_commit_reference(the_repository, &merge_heads->oid[i]);
954 + commit_list_insert(theirs, &list);
955 + ok = repo_is_descendant_of(the_repository, ours, list);
956 + free_commit_list(list);
957 + if (!ok)
958 + return 0;
959 + }
960 + return 1;
961 +}
962 +
963 static void show_advice_pull_non_ff(void)
964 {
965 advise(_("You have divergent branches and need to specify how to reconcile them.\n"
@@ -1074,7 +1101,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
1101
1102 /* ff-only takes precedence over rebase */
1103 if (opt_ff && !strcmp(opt_ff, "--ff-only")) {
1077 - if (!can_ff)
1104 + if (!can_ff && !already_up_to_date(&orig_head, &merge_heads))
1105 die_ff_impossible();
1106 opt_rebase = REBASE_FALSE;
1107 }
t/t7601-merge-pull-config.sh
+15 -1
@@ -2,7 +2,7 @@
2
3 test_description='git merge
4
5 -Testing pull.* configuration parsing.'
5 +Testing pull.* configuration parsing and other things.'
6
7 . ./test-lib.sh
8
@@ -387,6 +387,20 @@ test_expect_success 'pull prevents non-fast-forward with "only" in pull.ff' '
387 test_must_fail git pull . c3
388 '
389
390 +test_expect_success 'already-up-to-date pull succeeds with "only" in pull.ff' '
391 + git reset --hard c1 &&
392 + test_config pull.ff only &&
393 + git pull . c0 &&
394 + test "$(git rev-parse HEAD)" = "$(git rev-parse c1)"
395 +'
396 +
397 +test_expect_success 'already-up-to-date pull/rebase succeeds with "only" in pull.ff' '
398 + git reset --hard c1 &&
399 + test_config pull.ff only &&
400 + git -c pull.rebase=true pull . c0 &&
401 + test "$(git rev-parse HEAD)" = "$(git rev-parse c1)"
402 +'
403 +
404 test_expect_success 'merge c1 with c2 (ours in pull.twohead)' '
405 git reset --hard c1 &&
406 git config pull.twohead ours &&