pull: abort if --ff-only is given and fast-forwarding is impossible

alexhenrie · gitster · commit 3d5fc24daefb · 2021-07-20T21:43:12.000-07:00
The warning about pulling without specifying how to reconcile divergent
branches says that after setting pull.rebase to true, --ff-only can
still be passed on the command line to require a fast-forward. Make that
actually work.

Signed-off-by: Alex Henrie &lt;alexhenrie24@gmail.com&gt;
[en: updated tests; note 3 fixes and 1 new failure]
Signed-off-by: Elijah Newren &lt;newren@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/advice.c b/advice.c
@@ -286,6 +286,11 @@ void NORETURN die_conclude_merge(void)
 	die(_("Exiting because of unfinished merge."));
 }
 
+void NORETURN die_ff_impossible(void)
+{
+	die(_("Not possible to fast-forward, aborting."));
+}
+
 void advise_on_updating_sparse_paths(struct string_list *pathspec_list)
 {
 	struct string_list_item *item;
diff --git a/advice.h b/advice.h
@@ -95,6 +95,7 @@ void advise_if_enabled(enum advice_type type, const char *advice, ...);
 int error_resolve_conflict(const char *me);
 void NORETURN die_resolve_conflict(const char *me);
 void NORETURN die_conclude_merge(void);
+void NORETURN die_ff_impossible(void);
 void advise_on_updating_sparse_paths(struct string_list *pathspec_list);
 void detach_advice(const char *new_name);
 
diff --git a/builtin/merge.c b/builtin/merge.c
@@ -1620,7 +1620,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	}
 
 	if (fast_forward == FF_ONLY)
-		die(_("Not possible to fast-forward, aborting."));
+		die_ff_impossible();
 
 	if (autostash)
 		create_autostash(the_repository,
diff --git a/builtin/pull.c b/builtin/pull.c
@@ -1046,9 +1046,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 	can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);
 
-	if (rebase_unspecified && !opt_ff && !can_ff) {
-		if (opt_verbosity >= 0)
-			show_advice_pull_non_ff();
+	if (!can_ff) {
+		if (opt_ff) {
+			if (!strcmp(opt_ff, "--ff-only"))
+				die_ff_impossible();
+		} else {
+			if (rebase_unspecified && opt_verbosity >= 0)
+				show_advice_pull_non_ff();
+		}
 	}
 
 	if (opt_rebase) {
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
@@ -209,19 +209,19 @@ test_attempts_fast_forward () {
 # Group 1: Interaction of --ff-only with --[no-]rebase
 # (And related interaction of pull.ff=only with pull.rebase)
 #
-test_expect_failure '--ff-only overrides --rebase' '
+test_expect_success '--ff-only overrides --rebase' '
 	test_attempts_fast_forward pull --rebase --ff-only
 '
 
-test_expect_failure '--ff-only overrides --rebase even if first' '
+test_expect_success '--ff-only overrides --rebase even if first' '
 	test_attempts_fast_forward pull --ff-only --rebase
 '
 
 test_expect_success '--ff-only overrides --no-rebase' '
 	test_attempts_fast_forward pull --ff-only --no-rebase
 '
 
-test_expect_failure 'pull.ff=only overrides pull.rebase=true' '
+test_expect_success 'pull.ff=only overrides pull.rebase=true' '
 	test_attempts_fast_forward -c pull.ff=only -c pull.rebase=true pull
 '
 
@@ -252,7 +252,7 @@ test_expect_success 'pull.rebase=true overrides pull.ff=true' '
 '
 
 # Group 3: command line flags take precedence over config
-test_expect_failure '--ff-only takes precedence over pull.rebase=true' '
+test_expect_success '--ff-only takes precedence over pull.rebase=true' '
 	test_attempts_fast_forward -c pull.rebase=true pull --ff-only
 '
 
@@ -264,7 +264,7 @@ test_expect_failure '--no-rebase takes precedence over pull.ff=only' '
 	test_falls_back_to_full_merge -c pull.ff=only pull --no-rebase
 '
 
-test_expect_success '--rebase takes precedence over pull.ff=only' '
+test_expect_failure '--rebase takes precedence over pull.ff=only' '
 	test_does_rebase -c pull.ff=only pull --rebase
 '
 

Original file line number	Diff line number	Diff line change
`@@ -286,6 +286,11 @@ void NORETURN die_conclude_merge(void)`
`286`	`286`	`die(_("Exiting because of unfinished merge."));`
`287`	`287`	`}`
`288`	`288`
	`289`	`+void NORETURN die_ff_impossible(void)`
	`290`	`+{`
	`291`	`+ die(_("Not possible to fast-forward, aborting."));`
	`292`	`+}`
	`293`	`+`
`289`	`294`	`void advise_on_updating_sparse_paths(struct string_list *pathspec_list)`
`290`	`295`	`{`
`291`	`296`	`struct string_list_item *item;`
Original file line number	Diff line number	Diff line change
`@@ -1620,7 +1620,7 @@ int cmd_merge(int argc, const char *argv, const char prefix)`
`1620`	`1620`	`}`
`1621`	`1621`
`1622`	`1622`	`if (fast_forward == FF_ONLY)`
`1623`		`- die(_("Not possible to fast-forward, aborting."));`
	`1623`	`+ die_ff_impossible();`
`1624`	`1624`
`1625`	`1625`	`if (autostash)`
`1626`	`1626`	`create_autostash(the_repository,`
Original file line number	Diff line number	Diff line change
`@@ -209,19 +209,19 @@ test_attempts_fast_forward () {`
`209`	`209`	`# Group 1: Interaction of --ff-only with --[no-]rebase`
`210`	`210`	`# (And related interaction of pull.ff=only with pull.rebase)`
`211`	`211`	`#`
`212`		`-test_expect_failure '--ff-only overrides --rebase' '`
	`212`	`+test_expect_success '--ff-only overrides --rebase' '`
`213`	`213`	`test_attempts_fast_forward pull --rebase --ff-only`
`214`	`214`	`'`
`215`	`215`
`216`		`-test_expect_failure '--ff-only overrides --rebase even if first' '`
	`216`	`+test_expect_success '--ff-only overrides --rebase even if first' '`
`217`	`217`	`test_attempts_fast_forward pull --ff-only --rebase`
`218`	`218`	`'`
`219`	`219`
`220`	`220`	`test_expect_success '--ff-only overrides --no-rebase' '`
`221`	`221`	`test_attempts_fast_forward pull --ff-only --no-rebase`
`222`	`222`	`'`
`223`	`223`
`224`		`-test_expect_failure 'pull.ff=only overrides pull.rebase=true' '`
	`224`	`+test_expect_success 'pull.ff=only overrides pull.rebase=true' '`
`225`	`225`	`test_attempts_fast_forward -c pull.ff=only -c pull.rebase=true pull`
`226`	`226`	`'`
`227`	`227`
`@@ -252,7 +252,7 @@ test_expect_success 'pull.rebase=true overrides pull.ff=true' '`
`252`	`252`	`'`
`253`	`253`
`254`	`254`	`# Group 3: command line flags take precedence over config`
`255`		`-test_expect_failure '--ff-only takes precedence over pull.rebase=true' '`
	`255`	`+test_expect_success '--ff-only takes precedence over pull.rebase=true' '`
`256`	`256`	`test_attempts_fast_forward -c pull.rebase=true pull --ff-only`
`257`	`257`	`'`
`258`	`258`
`@@ -264,7 +264,7 @@ test_expect_failure '--no-rebase takes precedence over pull.ff=only' '`
`264`	`264`	`test_falls_back_to_full_merge -c pull.ff=only pull --no-rebase`
`265`	`265`	`'`
`266`	`266`
`267`		`-test_expect_success '--rebase takes precedence over pull.ff=only' '`
	`267`	`+test_expect_failure '--rebase takes precedence over pull.ff=only' '`
`268`	`268`	`test_does_rebase -c pull.ff=only pull --rebase`
`269`	`269`	`'`
`270`	`270`