[2/2] tentative: pull: change the semantics of --ff-only

Commit Message

Felipe Contreras Dec. 5, 2020, 8:19 p.m. UTC

We want the last thing specified to override everything previously.

For example --merge should override a previous "pull.ff = only"
configuration.

And --ff-only should override a previous "pull.rebase = true"
configuration.

Currently "git pull --ff-only --merge" fails with:

  fatal: not possible to fast-forward, aborting.

But that's not what we want; we want --merge to override --ff-only and
do a --no-ff merge.

This is a backwards-incompatible change that achieves that.

Signed-off-by: Felipe Contreras <[e-mail]>
---
 builtin/pull.c  | 27 +++++++++++++++++++++++----
 t/t5520-pull.sh | 37 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 5 deletions(-)

Patch

diff --git a/builtin/pull.c b/builtin/pull.c
index 44ec6e7216..54c58618e9 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -112,6 +112,24 @@  static int opt_show_forced_updates = -1;
 static char *set_upstream;
 static struct strvec opt_fetch = STRVEC_INIT;
 
+static int parse_opt_ff_only(const struct option *opt, const char *arg, int unset)
+{
+	char **value = opt->value;
+	opt_rebase = REBASE_DEFAULT;
+	free(*value);
+	*value = xstrdup_or_null("--ff-only");
+	return 0;
+}
+
+static int parse_opt_merge(const struct option *opt, const char *arg, int unset)
+{
+	enum rebase_type *value = opt->value;
+	free(opt_ff);
+	opt_ff = NULL;
+	*value = REBASE_FALSE;
+	return 0;
+}
+
 static struct option pull_options[] = {
 	/* Shared options */
 	OPT__VERBOSITY(&opt_verbosity),
@@ -129,8 +147,9 @@  static struct option pull_options[] = {
 		"(false|true|merges|preserve|interactive)",
 		N_("incorporate changes by rebasing rather than merging"),
 		PARSE_OPT_OPTARG, parse_opt_rebase),
-	OPT_SET_INT('m', "merge", &opt_rebase,
-		N_("incorporate changes by merging"), REBASE_FALSE),
+	OPT_CALLBACK_F('m', "merge", &opt_rebase, NULL,
+		N_("incorporate changes by merging"),
+		PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_merge),
 	OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL,
 		N_("do not show a diffstat at the end of the merge"),
 		PARSE_OPT_NOARG | PARSE_OPT_NONEG),
@@ -159,9 +178,9 @@  static struct option pull_options[] = {
 	OPT_PASSTHRU(0, "ff", &opt_ff, NULL,
 		N_("allow fast-forward"),
 		PARSE_OPT_NOARG),
-	OPT_PASSTHRU(0, "ff-only", &opt_ff, NULL,
+	OPT_CALLBACK_F(0, "ff-only", &opt_ff, NULL,
 		N_("abort if fast-forward is not possible"),
-		PARSE_OPT_NOARG | PARSE_OPT_NONEG),
+		PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_ff_only),
 	OPT_PASSTHRU(0, "verify-signatures", &opt_verify_signatures, NULL,
 		N_("verify that the named commit has a valid GPG signature"),
 		PARSE_OPT_NOARG),
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 067780e658..0cdac4010b 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -849,7 +849,7 @@  test_expect_success 'non-fast-forward (ff-only)' '
 	test_must_fail git pull
 '
 
-test_expect_failure 'non-fast-forward with merge (ff-only)' '
+test_expect_success 'non-fast-forward with merge (ff-only)' '
 	test_config pull.ff only &&
 	setup_non_ff &&
 	git pull --merge
@@ -869,4 +869,39 @@  test_expect_success 'non-fast-forward error message (ff-only)' '
 	grep -q "The pull was not fast-forward" error
 '
 
+test_expect_success '--merge overrides --ff-only' '
+	setup_non_ff &&
+	git pull --ff-only --merge
+'
+
+test_expect_success '--rebase overrides --ff-only' '
+	setup_non_ff &&
+	git pull --ff-only --rebase
+'
+
+test_expect_success '--ff-only overrides --merge' '
+	setup_non_ff &&
+	test_must_fail git pull --merge --ff-only
+'
+
+test_expect_success '--ff-only overrides pull.rebase=false' '
+	test_config pull.rebase false &&
+	setup_non_ff &&
+	test_must_fail git pull --ff-only
+'
+
+test_expect_success 'pull.rebase=true overrides pull.ff=only' '
+	test_config pull.ff only &&
+	test_config pull.rebase true &&
+	setup_non_ff &&
+	git pull
+'
+
+test_expect_success 'pull.rebase=false overrides pull.ff=only' '
+	test_config pull.ff only &&
+	test_config pull.rebase false &&
+	setup_non_ff &&
+	test_must_fail git pull
+'
+
 test_done