[PATCH] experiment: pull: change --ff-only and default mode

[Felipe Contreras <felipe.contreras@xxxxxxxxx>]

Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
---
 builtin/pull.c  | 40 ++++++++++++++++++++++++++--------------
 t/t5520-pull.sh | 23 +++++++++++++++++++++++
 2 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index e389ffcdc3..95ecbdaad5 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -62,6 +62,7 @@ static int parse_opt_rebase(const struct option *opt, const char *arg, int unset
 		*value = parse_config_rebase("--rebase", arg, 0);
 	else
 		*value = unset ? REBASE_FALSE : REBASE_TRUE;
+	if (*value != REBASE_INVALID) default_mode = 0;
 	return *value == REBASE_INVALID ? -1 : 0;
 }
 
@@ -114,6 +115,23 @@ 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_FALSE;
+	*value = "--ff-only";
+	return 0;
+}
+
+static int parse_opt_merge(const struct option *opt, const char *arg, int unset)
+{
+	char **value = opt->value;
+	opt_ff = NULL;
+	*value = REBASE_FALSE;
+	default_mode = 0;
+	return 0;
+}
+
 static struct option pull_options[] = {
 	/* Shared options */
 	OPT__VERBOSITY(&opt_verbosity),
@@ -131,8 +149,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"), 0),
+	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),
@@ -161,9 +180,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),
@@ -924,6 +943,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 	git_config(git_pull_config, NULL);
 
+	opt_ff = xstrdup_or_null(config_get_ff());
+	opt_rebase = config_get_rebase();
+
 	argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);
 
 	if (cleanup_arg)
@@ -935,12 +957,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 	parse_repo_refspecs(argc, argv, &repo, &refspecs);
 
-	if (!opt_ff)
-		opt_ff = xstrdup_or_null(config_get_ff());
-
-	if (opt_rebase < 0)
-		opt_rebase = config_get_rebase();
-
 	if (read_cache_unmerged())
 		die_resolve_conflict("pull");
 
@@ -1037,10 +1053,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	/* Disable --ff-only when --merge is specified */
-	if (!can_ff && !default_mode && !opt_rebase && opt_ff && !strcmp(opt_ff, "--ff-only"))
-		opt_ff = NULL;
-
 	if (opt_rebase) {
 		int ret = 0;
 		if ((recurse_submodules == RECURSE_SUBMODULES_ON ||
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index f48d0f8d50..c0cfde54e1 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -881,4 +881,27 @@ test_expect_success 'git pull non-fast-forward error message' '
 	grep -q "The pull was not fast-forward" error
 '
 
+test_expect_success 'git pull --merge overrides --ffonly' '
+	test_when_finished "git checkout master && git branch -D other test" &&
+	test_config pull.ff only &&
+	git checkout -b other master^ &&
+	>new &&
+	git add new &&
+	git commit -m new &&
+	git checkout -b test -t other &&
+	git reset --hard master &&
+	git pull --ff-only --merge
+'
+
+test_expect_success 'git pull --ff-only overrides --merge' '
+	test_when_finished "git checkout master && git branch -D other test" &&
+	git checkout -b other master^ &&
+	>new &&
+	git add new &&
+	git commit -m new &&
+	git checkout -b test -t other &&
+	git reset --hard master &&
+	test_must_fail git pull --merge --ff-only
+'
+
 test_done
-- 
2.29.2