[PATCH v2 00/13] pull: pull mode part 2

[PATCH v2 00/13] pull: pull mode part 2

[Felipe Contreras]

This patch series depends on fc/fc/pull-merge-rebase.

It's basically a collection of all the pull.mode patches rebased on top
of what Junio merged to jch.

Except that while re-reading the old threads I noticed a suggestion by 
Theodore Ts'o that went unnoticed [1]. It would be very useful to make
the pull.mode configuration per-repository, instead of per-branch.

This way we can have two different configurations for:

  git pull origin *
  git pull john *

This also could allow us in the future to have a "merge-inverted" mode
so users could finally merge correctly an update from the upstream
branch: master to origin/master, not origin/master to master.

This in addition allows us to consider yet another possible future
default:

  git pull                # default mode: merge-inverted
  git pull origin master  # default mode: merge

Also, based on Junio's feedback [2] regarding the logic of opt_ff I
decided to remove it completely, since it's clear they don't
implicitely mean a merge.

And finally; I noticed some quirkiness with semantics of --rebase and
pull.rebase, which I handled in the last patch.

Changes since v1:

 * Added more cleanups
 * Added --merge option
 * Added REBASE_DEFAULT
 * Added warning with --ff options
 * Added pull.mode
 * Added pull.mode=fast-forward (previously ff-only)
 * Improved --rebase and pull.rebase interaction

Changes since previous patch series:

 * Add remote.<name>.pullmode
 * Remove branch.<name>.pullmode
 * pull.mode now overrides pull.rebase

[1] https://lore.kernel.org/git/20130312212027.GE14792@thunk.org
[2] https://lore.kernel.org/git/20201214202647.3340193-1-gitster@pobox.com

Felipe Contreras (13):
  doc: pull: explain what is a fast-forward
  pull: improve default warning
  pull: cleanup autostash check
  pull: trivial cleanup
  pull: trivial whitespace style fix
  pull: introduce --merge option
  rebase: add REBASE_DEFAULT
  pull: move configurations fetches
  pull: show warning with --ff options
  pull: add pull.mode
  pull: add pull.mode=fast-forward
  pull: reorganize mode conditionals
  pull: improve --rebase and pull.rebase interaction

 Documentation/config/pull.txt   |   6 ++
 Documentation/config/remote.txt |   6 ++
 Documentation/git-pull.txt      |  49 ++++++++--
 builtin/pull.c                  | 165 +++++++++++++++++++++++---------
 rebase.c                        |  12 +++
 rebase.h                        |  13 ++-
 t/t5520-pull.sh                 | 108 +++++++++++++++++++++
 t/t7601-merge-pull-config.sh    |  34 +++++--
 8 files changed, 330 insertions(+), 63 deletions(-)

Range-diff:
 1:  0925821483 !  1:  eb72fa24fa doc: pull: explain what is a fast-forward
    @@ Documentation/git-pull.txt: Assume the following history exists and the current
     +synchronize the local, and remote brances.
     +
     +In these situations `git pull` will warn you about your possible
    -+options, which are either merge, or rebase. However, by default it will
    -+continue doing a merge.
    ++options, which are either merge (`--no-rebase`), or rebase (`--rebase`).
    ++However, by default it will continue doing a merge.
     +
     +A merge will create a new commit with two parent commits (`G` and `C`)
     +and a log message describing the changes, which you can edit.
    @@ Documentation/git-pull.txt: and a log message from the user describing the chang
      
      In Git 1.7.0 or later, to cancel a conflicting merge, use
      `git reset --merge`.  *Warning*: In older versions of Git, running 'git pull'
    +@@ Documentation/git-pull.txt: version.
    + 
    + SEE ALSO
    + --------
    +-linkgit:git-fetch[1], linkgit:git-merge[1], linkgit:git-config[1]
    ++linkgit:git-fetch[1], linkgit:git-merge[1], linkgit:git-rebase[1],
    ++linkgit:git-config[1]
    + 
    + GIT
    + ---
 2:  46c14cf851 =  2:  37d1dcecfd pull: improve default warning
 3:  aeb17014f5 =  3:  e3d29270ac pull: cleanup autostash check
 -:  ---------- >  4:  214bbbfcff pull: trivial cleanup
 -:  ---------- >  5:  69c1073064 pull: trivial whitespace style fix
 -:  ---------- >  6:  7561b4e7a8 pull: introduce --merge option
 -:  ---------- >  7:  17bf94fb5d rebase: add REBASE_DEFAULT
 -:  ---------- >  8:  8bd30852dd pull: move configurations fetches
 -:  ---------- >  9:  107fd4e0db pull: show warning with --ff options
 -:  ---------- > 10:  18d4caec8f pull: add pull.mode
 -:  ---------- > 11:  38feb507ce pull: add pull.mode=fast-forward
 -:  ---------- > 12:  80cd35372a pull: reorganize mode conditionals
 -:  ---------- > 13:  dc2819720a pull: improve --rebase and pull.rebase interaction
-- 
2.30.0.rc0

[PATCH v2 01/13] doc: pull: explain what is a fast-forward

[Felipe Contreras]

We want users to know what is a fast-forward in order to understand the
default warning.

Let's expand the explanation in order to cover both the simple, and the
complex cases with as much detail as possible.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-pull.txt | 41 ++++++++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 5c3fb67c01..142df1c4a1 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -41,16 +41,41 @@ Assume the following history exists and the current branch is
 ------------
 	  A---B---C master on origin
 	 /
-    D---E---F---G master
+    D---E master
 	^
 	origin/master in your repository
 ------------
 
 Then "`git pull`" will fetch and replay the changes from the remote
 `master` branch since it diverged from the local `master` (i.e., `E`)
-until its current commit (`C`) on top of `master` and record the
-result in a new commit along with the names of the two parent commits
-and a log message from the user describing the changes.
+until its current commit (`C`) on top of `master`.
+
+After the remote changes have been synchronized, the local `master` will
+be fast-forwarded to the same commit as the remote one, therefore
+creating a linear history.
+
+------------
+    D---E---A---B---C master, origin/master
+------------
+
+However, a non-fast-forward case looks very different:
+
+------------
+	  A---B---C origin/master
+	 /
+    D---E---F---G master
+------------
+
+If there are additional changes in the local `master`, it's
+not possible to fast-forward, so a decision must be made how to
+synchronize the local, and remote brances.
+
+In these situations `git pull` will warn you about your possible
+options, which are either merge (`--no-rebase`), or rebase (`--rebase`).
+However, by default it will continue doing a merge.
+
+A merge will create a new commit with two parent commits (`G` and `C`)
+and a log message describing the changes, which you can edit.
 
 ------------
 	  A---B---C origin/master
@@ -58,8 +83,11 @@ and a log message from the user describing the changes.
     D---E---F---G---H master
 ------------
 
+Once the merge commit is created (`H`), your local `master` branch has
+incorporated the changes of the remote `master` branch.
+
 See linkgit:git-merge[1] for details, including how conflicts
-are presented and handled.
+are presented and handled, and also linkgit:git-rebase[1].
 
 In Git 1.7.0 or later, to cancel a conflicting merge, use
 `git reset --merge`.  *Warning*: In older versions of Git, running 'git pull'
@@ -248,7 +276,8 @@ version.
 
 SEE ALSO
 --------
-linkgit:git-fetch[1], linkgit:git-merge[1], linkgit:git-config[1]
+linkgit:git-fetch[1], linkgit:git-merge[1], linkgit:git-rebase[1],
+linkgit:git-config[1]
 
 GIT
 ---
-- 
2.30.0.rc0

[PATCH v2 02/13] pull: improve default warning

[Felipe Contreras]

According to feedback from GitHub trainers [1], most newcomers don't
understand what a rebase is. So in the default warning we want to
provide our users with a command that does the most sensible thing,
fixes the divergence, gets rid of the warning, with the minimum mental
effort, and happens to be the default:

  git pull --no-rebase (later --merge)

In addition, we don't want to start by recommending a permanent
configuration, but a temporary solution so they start training their
fingers and maybe learn how to do a rebase. So we start with the commands.

Also, we need to be clear about what we mean by "specifying"; merge, or
rebase.

Moreover, thanks to the previous patch now "git pull --help" explains
what a fast-forward is, let's mention that reference.

And finally, use --global in the configuration commands like we did with
push.default.

[1] https://lore.kernel.org/git/20130909201751.GA14437@sigill.intra.peff.net/

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/pull.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index e8927fc2ff..a766d9762c 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -927,18 +927,20 @@ static int get_can_ff(struct object_id *orig_head, struct object_id *orig_merge_
 
 static void show_advice_pull_non_ff(void)
 {
-	advise(_("Pulling without specifying how to reconcile divergent branches is\n"
-		 "discouraged. You can squelch this message by running one of the following\n"
-		 "commands sometime before your next pull:\n"
+	advise(_("Pulling without specifying how to reconcile divergent branches is discouraged;\n"
+		 "you need to specify if you want a merge, or a rebase.\n"
 		 "\n"
-		 "  git config pull.rebase false  # merge (the default strategy)\n"
-		 "  git config pull.rebase true   # rebase\n"
-		 "  git config pull.ff only       # fast-forward only\n"
+		 "  git pull --no-rebase # the default (merge)\n"
+		 "  git pull --rebase\n"
 		 "\n"
-		 "You can replace \"git config\" with \"git config --global\" to set a default\n"
-		 "preference for all repositories. You can also pass --rebase, --no-rebase,\n"
-		 "or --ff-only on the command line to override the configured default per\n"
-		 "invocation.\n"));
+		 "You can squelch this message by running one of the following commands:\n"
+		 "\n"
+		 "  git config --global pull.rebase false  # merge\n"
+		 "  git config --global pull.rebase true   # rebase\n"
+		 "  git config --global pull.ff only       # fast-forward only\n"
+		 "\n"
+		 "If unsure, run \"git pull --no-rebase\".\n"
+		 "Read \"git pull --help\" for more information."));
 }
 
 int cmd_pull(int argc, const char **argv, const char *prefix)
-- 
2.30.0.rc0

[PATCH v2 03/13] pull: cleanup autostash check

[Felipe Contreras]

Currently "git pull --rebase" takes a shortcut in the case a
fast-forward merge is possible; run_merge() is called with --ff-only.

However, "git merge" didn't have an --autostash option, so, when "git
pull --rebase --autostash" was called *and* the fast-forward merge
shortcut was taken, then the pull failed.

This was fixed in commit f15e7cf5cc (pull: ff --rebase --autostash
works in dirty repo, 2017-06-01) by simply skipping the fast-forward
merge shortcut.

Later on "git merge" learned the --autostash option [a03b55530a
(merge: teach --autostash option, 2020-04-07)], and so did "git pull"
[d9f15d37f1 (pull: pass --autostash to merge, 2020-04-07)].

Therefore it's not necessary to skip the fast-forward merge shortcut
anymore when called with --rebase --autostash.

Let's always take the fast-forward merge shortcut by essentially
reverting f15e7cf5cc.

Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/pull.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index a766d9762c..42cd6c38d8 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -949,7 +949,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	struct oid_array merge_heads = OID_ARRAY_INIT;
 	struct object_id orig_head, curr_head;
 	struct object_id rebase_fork_point;
-	int autostash;
 	int rebase_unspecified = 0;
 	int can_ff;
 
@@ -984,8 +983,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	if (get_oid("HEAD", &orig_head))
 		oidclr(&orig_head);
 
-	autostash = config_autostash;
 	if (opt_rebase) {
+		int autostash = config_autostash;
 		if (opt_autostash != -1)
 			autostash = opt_autostash;
 
@@ -1067,13 +1066,12 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 		     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) &&
 		    submodule_touches_in_range(the_repository, &upstream, &curr_head))
 			die(_("cannot rebase with locally recorded submodule modifications"));
-		if (!autostash) {
-			if (can_ff) {
-				/* we can fast-forward this without invoking rebase */
-				opt_ff = "--ff-only";
-				ran_ff = 1;
-				ret = run_merge();
-			}
+
+		if (can_ff) {
+			/* we can fast-forward this without invoking rebase */
+			opt_ff = "--ff-only";
+			ran_ff = 1;
+			ret = run_merge();
 		}
 		if (!ran_ff)
 			ret = run_rebase(&newbase, &upstream);
-- 
2.30.0.rc0

[PATCH v2 04/13] pull: trivial cleanup

[Felipe Contreras]

There's no need to store ran_ff. Now it's obvious from the conditionals.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/pull.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 42cd6c38d8..21089e5a29 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1055,7 +1055,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 	if (opt_rebase) {
 		int ret = 0;
-		int ran_ff = 0;
 
 		struct object_id newbase;
 		struct object_id upstream;
@@ -1070,11 +1069,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 		if (can_ff) {
 			/* we can fast-forward this without invoking rebase */
 			opt_ff = "--ff-only";
-			ran_ff = 1;
 			ret = run_merge();
-		}
-		if (!ran_ff)
+		} else {
 			ret = run_rebase(&newbase, &upstream);
+		}
 
 		if (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON ||
 			     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))
-- 
2.30.0.rc0

[PATCH v2 05/13] pull: trivial whitespace style fix

[Felipe Contreras]

Two spaces unaligned to anything is not part of the coding-style. A
single tab is.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/pull.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 21089e5a29..48e25a5061 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -126,9 +126,9 @@ static struct option pull_options[] = {
 	/* Options passed to git-merge or git-rebase */
 	OPT_GROUP(N_("Options related to merging")),
 	OPT_CALLBACK_F('r', "rebase", &opt_rebase,
-	  "(false|true|merges|preserve|interactive)",
-	  N_("incorporate changes by rebasing rather than merging"),
-	  PARSE_OPT_OPTARG, parse_opt_rebase),
+		"(false|true|merges|preserve|interactive)",
+		N_("incorporate changes by rebasing rather than merging"),
+		PARSE_OPT_OPTARG, parse_opt_rebase),
 	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),
-- 
2.30.0.rc0

[PATCH v2 06/13] pull: introduce --merge option

[Felipe Contreras]

Previously --no-rebase (which still works for backwards compatibility).

Now we can update the default warning, and the git-pull(1) man page to
use --merge instead of the non-intuitive --no-rebase.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-pull.txt   | 11 +++++++----
 builtin/pull.c               |  6 ++++--
 t/t7601-merge-pull-config.sh |  8 ++++----
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 142df1c4a1..195496e63d 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -71,8 +71,8 @@ not possible to fast-forward, so a decision must be made how to
 synchronize the local, and remote brances.
 
 In these situations `git pull` will warn you about your possible
-options, which are either merge (`--no-rebase`), or rebase (`--rebase`).
-However, by default it will continue doing a merge.
+options, which are either `--merge`, or `--rebase`. However, by default
+it will continue doing a merge.
 
 A merge will create a new commit with two parent commits (`G` and `C`)
 and a log message describing the changes, which you can edit.
@@ -159,8 +159,11 @@ It rewrites history, which does not bode well when you
 published that history already.  Do *not* use this option
 unless you have read linkgit:git-rebase[1] carefully.
 
---no-rebase::
-	Override earlier --rebase.
+-m::
+--merge::
+	Force a merge.
++
+Previously this was --no-rebase, but that usage has been deprecated.
 
 Options related to fetching
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/builtin/pull.c b/builtin/pull.c
index 48e25a5061..1336b59b21 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -129,6 +129,8 @@ 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_PASSTHRU('n', NULL, &opt_diffstat, NULL,
 		N_("do not show a diffstat at the end of the merge"),
 		PARSE_OPT_NOARG | PARSE_OPT_NONEG),
@@ -930,7 +932,7 @@ static void show_advice_pull_non_ff(void)
 	advise(_("Pulling without specifying how to reconcile divergent branches is discouraged;\n"
 		 "you need to specify if you want a merge, or a rebase.\n"
 		 "\n"
-		 "  git pull --no-rebase # the default (merge)\n"
+		 "  git pull --merge # the default\n"
 		 "  git pull --rebase\n"
 		 "\n"
 		 "You can squelch this message by running one of the following commands:\n"
@@ -939,7 +941,7 @@ static void show_advice_pull_non_ff(void)
 		 "  git config --global pull.rebase true   # rebase\n"
 		 "  git config --global pull.ff only       # fast-forward only\n"
 		 "\n"
-		 "If unsure, run \"git pull --no-rebase\".\n"
+		 "If unsure, run \"git pull --merge\".\n"
 		 "Read \"git pull --help\" for more information."));
 }
 
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 52e8ccc933..6d03e0b9fe 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -60,9 +60,9 @@ test_expect_success 'pull.rebase not set and --rebase given' '
 	test_i18ngrep ! "Pulling without specifying how to reconcile" err
 '
 
-test_expect_success 'pull.rebase not set and --no-rebase given' '
+test_expect_success 'pull.rebase not set and --merge given' '
 	git reset --hard c0 &&
-	git pull --no-rebase . c1 2>err &&
+	git pull --merge . c1 2>err &&
 	test_i18ngrep ! "Pulling without specifying how to reconcile" err
 '
 
@@ -119,9 +119,9 @@ test_expect_success 'pull.rebase not set and --rebase given (not-fast-forward)'
 	test_i18ngrep ! "Pulling without specifying how to reconcile" err
 '
 
-test_expect_success 'pull.rebase not set and --no-rebase given (not-fast-forward)' '
+test_expect_success 'pull.rebase not set and --merge given (not-fast-forward)' '
 	git reset --hard c2 &&
-	git pull --no-rebase . c1 2>err &&
+	git pull --merge . c1 2>err &&
 	test_i18ngrep ! "Pulling without specifying how to reconcile" err
 '
 
-- 
2.30.0.rc0

[PATCH v2 07/13] rebase: add REBASE_DEFAULT

[Felipe Contreras]

By introducing a default we can distinguish when the user has forced an
option.

Therefore there's no need pass around an extra variable variable (it's
the same as opt_rebase == REBASE_DEFAULT), nor is there any need to
initialize opt_rebase to an invalid value.

Additionally this will allow us to override the default with a
configuration, and subsequently the configuration with arguments.

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/pull.c | 27 ++++++++++++---------------
 rebase.h       |  3 ++-
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 1336b59b21..c0a90fa741 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -74,7 +74,7 @@ static char *opt_progress;
 static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 
 /* Options passed to git-merge or git-rebase */
-static enum rebase_type opt_rebase = -1;
+static enum rebase_type opt_rebase;
 static char *opt_diffstat;
 static char *opt_log;
 static char *opt_signoff;
@@ -326,7 +326,7 @@ static const char *config_get_ff(void)
  * looks for the value of "pull.rebase". If both configuration keys do not
  * exist, returns REBASE_FALSE.
  */
-static enum rebase_type config_get_rebase(int *rebase_unspecified)
+static enum rebase_type config_get_rebase(void)
 {
 	struct branch *curr_branch = branch_get("HEAD");
 	const char *value;
@@ -346,9 +346,7 @@ static enum rebase_type config_get_rebase(int *rebase_unspecified)
 	if (!git_config_get_value("pull.rebase", &value))
 		return parse_config_rebase("pull.rebase", value, 1);
 
-	*rebase_unspecified = 1;
-
-	return REBASE_FALSE;
+	return REBASE_DEFAULT;
 }
 
 /**
@@ -443,7 +441,7 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs
 	const char *remote = curr_branch ? curr_branch->remote_name : NULL;
 
 	if (*refspecs) {
-		if (opt_rebase)
+		if (opt_rebase >= REBASE_TRUE)
 			fprintf_ln(stderr, _("There is no candidate for rebasing against among the refs that you just fetched."));
 		else
 			fprintf_ln(stderr, _("There are no candidates for merging among the refs that you just fetched."));
@@ -456,7 +454,7 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs
 			repo);
 	} else if (!curr_branch) {
 		fprintf_ln(stderr, _("You are not currently on a branch."));
-		if (opt_rebase)
+		if (opt_rebase >= REBASE_TRUE)
 			fprintf_ln(stderr, _("Please specify which branch you want to rebase against."));
 		else
 			fprintf_ln(stderr, _("Please specify which branch you want to merge with."));
@@ -471,7 +469,7 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs
 			remote_name = _("<remote>");
 
 		fprintf_ln(stderr, _("There is no tracking information for the current branch."));
-		if (opt_rebase)
+		if (opt_rebase >= REBASE_TRUE)
 			fprintf_ln(stderr, _("Please specify which branch you want to rebase against."));
 		else
 			fprintf_ln(stderr, _("Please specify which branch you want to merge with."));
@@ -951,7 +949,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	struct oid_array merge_heads = OID_ARRAY_INIT;
 	struct object_id orig_head, curr_head;
 	struct object_id rebase_fork_point;
-	int rebase_unspecified = 0;
 	int can_ff;
 
 	if (!getenv("GIT_REFLOG_ACTION"))
@@ -973,8 +970,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	if (!opt_ff)
 		opt_ff = xstrdup_or_null(config_get_ff());
 
-	if (opt_rebase < 0)
-		opt_rebase = config_get_rebase(&rebase_unspecified);
+	if (!opt_rebase)
+		opt_rebase = config_get_rebase();
 
 	if (read_cache_unmerged())
 		die_resolve_conflict("pull");
@@ -985,7 +982,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	if (get_oid("HEAD", &orig_head))
 		oidclr(&orig_head);
 
-	if (opt_rebase) {
+	if (opt_rebase >= REBASE_TRUE) {
 		int autostash = config_autostash;
 		if (opt_autostash != -1)
 			autostash = opt_autostash;
@@ -1045,17 +1042,17 @@ 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)
+	if (opt_rebase >= REBASE_TRUE && merge_heads.nr > 1)
 		die(_("Cannot rebase onto multiple branches."));
 
 	can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);
 
-	if (rebase_unspecified && !opt_ff && !can_ff) {
+	if (!opt_rebase && !opt_ff && !can_ff) {
 		if (opt_verbosity >= 0)
 			show_advice_pull_non_ff();
 	}
 
-	if (opt_rebase) {
+	if (opt_rebase >= REBASE_TRUE) {
 		int ret = 0;
 
 		struct object_id newbase;
diff --git a/rebase.h b/rebase.h
index cc723d4748..34d4acfd74 100644
--- a/rebase.h
+++ b/rebase.h
@@ -3,7 +3,8 @@
 
 enum rebase_type {
 	REBASE_INVALID = -1,
-	REBASE_FALSE = 0,
+	REBASE_DEFAULT = 0,
+	REBASE_FALSE,
 	REBASE_TRUE,
 	REBASE_PRESERVE,
 	REBASE_MERGES,
-- 
2.30.0.rc0

[PATCH v2 08/13] pull: move configurations fetches

[Felipe Contreras]

Now that we have FETCH_DEFAULT we can fetch the configuration before
parsing the argument options.

The options will override the configuration, and if they don't;
opt_rebase will remain being FETCH_DEFAULT.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/pull.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index c0a90fa741..2fd5e44e03 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -951,6 +951,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	struct object_id rebase_fork_point;
 	int can_ff;
 
+	opt_ff = xstrdup_or_null(config_get_ff());
+	opt_rebase = config_get_rebase();
+
 	if (!getenv("GIT_REFLOG_ACTION"))
 		set_reflog_message(argc, argv);
 
@@ -967,12 +970,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)
-		opt_rebase = config_get_rebase();
-
 	if (read_cache_unmerged())
 		die_resolve_conflict("pull");
 
-- 
2.30.0.rc0

[PATCH v2 09/13] pull: show warning with --ff options

[Felipe Contreras]

We want the user to specify either --merge or --rebase, if she doesn't
we throw a warning.

Using --ff, --no-ff, or --ff-only does not make the merge explicit.

For example, if the user has the following configuration:

  git config pull.rebase true
  git pull --no-ff

A merge is not implied.

We should be consistent and either imply a merge; in which case a
previous "pull.rebase=true" configuration is overridden, or don't; in
which case the warning should be thrown.

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/pull.c               |  2 +-
 t/t7601-merge-pull-config.sh | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 2fd5e44e03..f1a03ccd14 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1044,7 +1044,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 	can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);
 
-	if (!opt_rebase && !opt_ff && !can_ff) {
+	if (!opt_rebase && !can_ff) {
 		if (opt_verbosity >= 0)
 			show_advice_pull_non_ff();
 	}
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 6d03e0b9fe..7c4607191a 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -96,21 +96,21 @@ test_expect_success 'pull.rebase not set and pull.ff=true (not-fast-forward)' '
 	git reset --hard c2 &&
 	test_config pull.ff true &&
 	git pull . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep "Pulling without specifying how to reconcile" err
 '
 
 test_expect_success 'pull.rebase not set and pull.ff=false (not-fast-forward)' '
 	git reset --hard c2 &&
 	test_config pull.ff false &&
 	git pull . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep "Pulling without specifying how to reconcile" err
 '
 
 test_expect_success 'pull.rebase not set and pull.ff=only (not-fast-forward)' '
 	git reset --hard c2 &&
 	test_config pull.ff only &&
 	test_must_fail git pull . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep "Pulling without specifying how to reconcile" err
 '
 
 test_expect_success 'pull.rebase not set and --rebase given (not-fast-forward)' '
@@ -128,19 +128,19 @@ test_expect_success 'pull.rebase not set and --merge given (not-fast-forward)' '
 test_expect_success 'pull.rebase not set and --ff given (not-fast-forward)' '
 	git reset --hard c2 &&
 	git pull --ff . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep "Pulling without specifying how to reconcile" err
 '
 
 test_expect_success 'pull.rebase not set and --no-ff given (not-fast-forward)' '
 	git reset --hard c2 &&
 	git pull --no-ff . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep "Pulling without specifying how to reconcile" err
 '
 
 test_expect_success 'pull.rebase not set and --ff-only given (not-fast-forward)' '
 	git reset --hard c2 &&
 	test_must_fail git pull --ff-only . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep "Pulling without specifying how to reconcile" err
 '
 
 test_expect_success 'merge c1 with c2' '
-- 
2.30.0.rc0

[PATCH v2 10/13] pull: add pull.mode

[Felipe Contreras]

The evolution of pull options has somewhat served most users, however,
they have been found lacking for a very needed trio: merge / rebase /
fast-forward-only.

Another thing that is missing is the possibility to specify pull options
on a per-repository basis, as Theodore Ts'o suggested long time ago
[1].

This patch adds a pull.mode option with two possible values (for now);
merge and rebase. If set, it overrides what the user has specified in
pull.rebase, and it's updated with either --merge, or --rebase.

In addition to pull.mode, a 'remote.<name>.pullmode' configuration is
introduced, so the user can specify the desired options based on the
source of the pull, rather than the destination.

And of course; new modes like fast-forward-only can be introduced later
on.

[1] https://lore.kernel.org/git/20130312212027.GE14792@thunk.org

Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/config/pull.txt   |  6 +++
 Documentation/config/remote.txt |  6 +++
 Documentation/git-pull.txt      |  3 +-
 builtin/pull.c                  | 81 +++++++++++++++++++++++++++++++--
 rebase.c                        | 10 ++++
 rebase.h                        |  9 ++++
 t/t5520-pull.sh                 | 54 ++++++++++++++++++++++
 t/t7601-merge-pull-config.sh    | 14 ++++++
 8 files changed, 178 insertions(+), 5 deletions(-)

diff --git a/Documentation/config/pull.txt b/Documentation/config/pull.txt
index 5404830609..f4385cde33 100644
--- a/Documentation/config/pull.txt
+++ b/Documentation/config/pull.txt
@@ -29,6 +29,12 @@ mode.
 it unless you understand the implications (see linkgit:git-rebase[1]
 for details).
 
+pull.mode::
+	When "git pull" is run, this determines if it would either merge or
+	rebase the fetched branch. The possible values are 'merge',
+	and 'rebase'. See "branch.<name>.pullmode" for setting this on a
+	per-branch basis.
+
 pull.octopus::
 	The default merge strategy to use when pulling multiple branches
 	at once.
diff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt
index a8e6437a90..a732c92cf5 100644
--- a/Documentation/config/remote.txt
+++ b/Documentation/config/remote.txt
@@ -84,3 +84,9 @@ remote.<name>.promisor::
 remote.<name>.partialclonefilter::
 	The filter that will be applied when fetching from this
 	promisor remote.
+
+remote.<name>.pullmode::
+	When "git pull" is run, this determines if it would either merge or
+	rebase the branches from this remote. The possible values are 'merge', and
+	'rebase'. See "pull.mode" for doing this in a non
+	repository-specific manner.
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 195496e63d..65f8b16a7d 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -72,7 +72,8 @@ synchronize the local, and remote brances.
 
 In these situations `git pull` will warn you about your possible
 options, which are either `--merge`, or `--rebase`. However, by default
-it will continue doing a merge.
+it will continue doing a merge (you can change that with the `pull.mode`
+configuration).
 
 A merge will create a new commit with two parent commits (`G` and `C`)
 and a log message describing the changes, which you can edit.
diff --git a/builtin/pull.c b/builtin/pull.c
index f1a03ccd14..bfadd585c7 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -27,6 +27,8 @@
 #include "commit-reach.h"
 #include "sequencer.h"
 
+static enum pull_mode_type mode;
+
 /**
  * Parses the value of --rebase. If value is a false value, returns
  * REBASE_FALSE. If value is a true value, returns REBASE_TRUE. If value is
@@ -49,6 +51,14 @@ static enum rebase_type parse_config_rebase(const char *key, const char *value,
 	return REBASE_INVALID;
 }
 
+static enum pull_mode_type parse_config_pull_mode(const char *key, const char *value)
+{
+	enum pull_mode_type v = pull_mode_parse_value(value);
+	if (v == PULL_MODE_INVALID)
+		die(_("Invalid value for %s: %s"), key, value);
+	return v;
+}
+
 /**
  * Callback for --rebase, which parses arg with parse_config_rebase().
  */
@@ -60,9 +70,21 @@ 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 > 0)
+		mode = *value >= REBASE_TRUE ? PULL_MODE_REBASE : PULL_MODE_MERGE;
+
 	return *value == REBASE_INVALID ? -1 : 0;
 }
 
+static int parse_opt_merge(const struct option *opt, const char *arg, int unset)
+{
+	enum rebase_type *value = opt->value;
+	mode = PULL_MODE_MERGE;
+	*value = REBASE_FALSE;
+	return 0;
+}
+
 static const char * const pull_usage[] = {
 	N_("git pull [<options>] [<repository> [<refspec>...]]"),
 	NULL
@@ -129,8 +151,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),
@@ -349,6 +372,31 @@ static enum rebase_type config_get_rebase(void)
 	return REBASE_DEFAULT;
 }
 
+static enum pull_mode_type config_get_pull_mode(const char *repo)
+{
+	const char *value;
+	struct remote *remote;
+
+	remote = remote_get(repo);
+
+	if (remote) {
+		char *key = xstrfmt("remote.%s.pullmode", remote->name);
+
+		if (!git_config_get_value(key, &value)) {
+			enum pull_mode_type ret = parse_config_pull_mode(key, value);
+			free(key);
+			return ret;
+		}
+
+		free(key);
+	}
+
+	if (!git_config_get_value("pull.mode", &value))
+		return parse_config_pull_mode("pull.mode", value);
+
+	return PULL_MODE_DEFAULT;
+}
+
 /**
  * Read config variables.
  */
@@ -935,8 +983,8 @@ static void show_advice_pull_non_ff(void)
 		 "\n"
 		 "You can squelch this message by running one of the following commands:\n"
 		 "\n"
-		 "  git config --global pull.rebase false  # merge\n"
-		 "  git config --global pull.rebase true   # rebase\n"
+		 "  git config --global pull.mode merge\n"
+		 "  git config --global pull.mode rebase\n"
 		 "  git config --global pull.ff only       # fast-forward only\n"
 		 "\n"
 		 "If unsure, run \"git pull --merge\".\n"
@@ -970,6 +1018,31 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 	parse_repo_refspecs(argc, argv, &repo, &refspecs);
 
+	/*
+	 * If the user has not specified --merge or --rebase, fetch pull.mode to override
+	 * pull.rename.
+	 */
+	if (!mode) {
+		mode = config_get_pull_mode(repo);
+
+		switch (mode) {
+		case PULL_MODE_MERGE:
+			opt_rebase = REBASE_FALSE;
+			break;
+		case PULL_MODE_REBASE:
+			/* Do not oeverride other rebase modes */
+			if (opt_rebase < REBASE_TRUE)
+				opt_rebase = REBASE_TRUE;
+			break;
+		case PULL_MODE_DEFAULT:
+			if (opt_rebase > 0)
+				mode = opt_rebase >= REBASE_TRUE ? PULL_MODE_REBASE : PULL_MODE_MERGE;
+			break;
+		default:
+			break;
+		}
+	}
+
 	if (read_cache_unmerged())
 		die_resolve_conflict("pull");
 
diff --git a/rebase.c b/rebase.c
index f8137d859b..bdfca49886 100644
--- a/rebase.c
+++ b/rebase.c
@@ -33,3 +33,13 @@ enum rebase_type rebase_parse_value(const char *value)
 
 	return REBASE_INVALID;
 }
+
+enum pull_mode_type pull_mode_parse_value(const char *value)
+{
+	if (!strcmp(value, "merge") || !strcmp(value, "m"))
+		return PULL_MODE_MERGE;
+	else if (!strcmp(value, "rebase") || !strcmp(value, "r"))
+		return PULL_MODE_REBASE;
+
+	return PULL_MODE_INVALID;
+}
diff --git a/rebase.h b/rebase.h
index 34d4acfd74..5ab8f4ddd5 100644
--- a/rebase.h
+++ b/rebase.h
@@ -13,4 +13,13 @@ enum rebase_type {
 
 enum rebase_type rebase_parse_value(const char *value);
 
+enum pull_mode_type {
+	PULL_MODE_INVALID = -1,
+	PULL_MODE_DEFAULT = 0,
+	PULL_MODE_MERGE,
+	PULL_MODE_REBASE
+};
+
+enum pull_mode_type pull_mode_parse_value(const char *value);
+
 #endif /* REBASE */
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 9fae07cdfa..59799ac4d5 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -449,6 +449,16 @@ test_expect_success 'pull.rebase' '
 	test_cmp expect actual
 '
 
+test_expect_success 'pull.mode rebase' '
+	git reset --hard before-rebase &&
+	test_config pull.mode rebase &&
+	git pull . copy &&
+	test_cmp_rev HEAD^ copy &&
+	echo new >expect &&
+	git show HEAD:file2 >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'pull --autostash & pull.rebase=true' '
 	test_config pull.rebase true &&
 	test_pull_autostash 1 --autostash
@@ -480,6 +490,18 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
 	test_cmp expect actual
 '
 
+test_expect_success 'remote..pullmode' '
+	git reset --hard before-rebase &&
+	git remote add test_remote . &&
+	test_when_finished "git remote remove test_remote" &&
+	test_config remote.test_remote.pullmode rebase &&
+	git pull test_remote copy &&
+	test_cmp_rev HEAD^ copy &&
+	echo new >expect &&
+	git show HEAD:file2 >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'pull --rebase warns on --verify-signatures' '
 	git reset --hard before-rebase &&
 	git pull --rebase --verify-signatures . copy 2>err &&
@@ -523,6 +545,17 @@ test_expect_success 'pull.rebase=false create a new merge commit' '
 	test_cmp expect actual
 '
 
+test_expect_success 'pull.mode=merge create a new merge commit' '
+	git reset --hard before-preserve-rebase &&
+	test_config pull.mode merge &&
+	git pull . copy &&
+	test_cmp_rev HEAD^1 before-preserve-rebase &&
+	test_cmp_rev HEAD^2 copy &&
+	echo file3 >expect &&
+	git show HEAD:file3.t >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'pull.rebase=true flattens keep-merge' '
 	git reset --hard before-preserve-rebase &&
 	test_config pull.rebase true &&
@@ -552,6 +585,16 @@ test_expect_success REBASE_P \
 	test_cmp_rev HEAD^2 keep-merge
 '
 
+test_expect_success REBASE_P \
+	'pull.rebase=preserve rebases and merges keep-merge with pull.mode' '
+	git reset --hard before-preserve-rebase &&
+	test_config pull.mode rebase &&
+	test_config pull.rebase preserve &&
+	git pull . copy &&
+	test_cmp_rev HEAD^^ copy &&
+	test_cmp_rev HEAD^2 keep-merge
+'
+
 test_expect_success 'pull.rebase=interactive' '
 	write_script "$TRASH_DIRECTORY/fake-editor" <<-\EOF &&
 	echo I was here >fake.out &&
@@ -593,6 +636,17 @@ test_expect_success '--rebase=false create a new merge commit' '
 	test_cmp expect actual
 '
 
+test_expect_success '--rebase=false create a new merge commit with pull.mode' '
+	git reset --hard before-preserve-rebase &&
+	test_config pull.mode rebase &&
+	git pull --rebase=false . copy &&
+	test_cmp_rev HEAD^1 before-preserve-rebase &&
+	test_cmp_rev HEAD^2 copy &&
+	echo file3 >expect &&
+	git show HEAD:file3.t >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success '--rebase=true rebases and flattens keep-merge' '
 	git reset --hard before-preserve-rebase &&
 	test_config pull.rebase preserve &&
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 7c4607191a..47fd2e2d05 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -33,6 +33,13 @@ test_expect_success 'pull.rebase not set' '
 	test_i18ngrep ! "Pulling without specifying how to reconcile" err
 '
 
+test_expect_success 'pull.mode set' '
+	git reset --hard c0 &&
+	test_config pull.mode merge &&
+	git pull . c1 2>err &&
+	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+'
+
 test_expect_success 'pull.rebase not set and pull.ff=true' '
 	git reset --hard c0 &&
 	test_config pull.ff true &&
@@ -92,6 +99,13 @@ test_expect_success 'pull.rebase not set (not-fast-forward)' '
 	test_i18ngrep "Pulling without specifying how to reconcile" decoded
 '
 
+test_expect_success 'pull.mode set' '
+	git reset --hard c2 &&
+	test_config pull.mode merge &&
+	git pull . c1 2>err &&
+	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+'
+
 test_expect_success 'pull.rebase not set and pull.ff=true (not-fast-forward)' '
 	git reset --hard c2 &&
 	test_config pull.ff true &&
-- 
2.30.0.rc0

[PATCH v2 11/13] pull: add pull.mode=fast-forward

[Felipe Contreras]

It is very typical for Git newcomers to inadvertently create merges and
worse; pushing them. This is one of the reasons many experienced users
prefer to avoid 'git pull', and recommend newcomers to avoid it as well.

To escape these problems--and keep 'git pull' useful--it has been
suggested that 'git pull' barfs by default if the merge is
non-fast-forward, which unfortunately would break backwards
compatibility.

This patch leaves everything in place to enable this new mode, but it
only gets enabled if the user specifically configures it:

  pull.mode = fast-forward

Later on this mode can be enabled by default.

For *some* of the long discussions you can read:

https://lore.kernel.org/git/20130522115042.GA20649@inner.h.apk.li
https://lore.kernel.org/git/1377988690-23460-1-git-send-email-felipe.contreras@gmail.com
https://lore.kernel.org/4ay6w9i74cygt6ii1b0db7wg.1398433713382@email.android.com

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/config/pull.txt   |  4 +--
 Documentation/config/remote.txt |  4 +--
 builtin/pull.c                  |  6 ++++-
 rebase.c                        |  2 ++
 rebase.h                        |  3 ++-
 t/t5520-pull.sh                 | 44 +++++++++++++++++++++++++++++++++
 6 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/Documentation/config/pull.txt b/Documentation/config/pull.txt
index f4385cde33..2b5e813259 100644
--- a/Documentation/config/pull.txt
+++ b/Documentation/config/pull.txt
@@ -32,8 +32,8 @@ for details).
 pull.mode::
 	When "git pull" is run, this determines if it would either merge or
 	rebase the fetched branch. The possible values are 'merge',
-	and 'rebase'. See "branch.<name>.pullmode" for setting this on a
-	per-branch basis.
+	'rebase', and 'fast-forward'. See "branch.<name>.pullmode" for setting
+	this on a per-branch basis.
 
 pull.octopus::
 	The default merge strategy to use when pulling multiple branches
diff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt
index a732c92cf5..e7db1d46c1 100644
--- a/Documentation/config/remote.txt
+++ b/Documentation/config/remote.txt
@@ -87,6 +87,6 @@ remote.<name>.partialclonefilter::
 
 remote.<name>.pullmode::
 	When "git pull" is run, this determines if it would either merge or
-	rebase the branches from this remote. The possible values are 'merge', and
-	'rebase'. See "pull.mode" for doing this in a non
+	rebase the branches from this remote. The possible values are 'merge',
+	'rebase', and 'fast-forward'. See "pull.mode" for doing this in a non
 	repository-specific manner.
diff --git a/builtin/pull.c b/builtin/pull.c
index bfadd585c7..ef23d8a52f 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -985,7 +985,7 @@ static void show_advice_pull_non_ff(void)
 		 "\n"
 		 "  git config --global pull.mode merge\n"
 		 "  git config --global pull.mode rebase\n"
-		 "  git config --global pull.ff only       # fast-forward only\n"
+		 "  git config --global pull.mode fast-forward\n"
 		 "\n"
 		 "If unsure, run \"git pull --merge\".\n"
 		 "Read \"git pull --help\" for more information."));
@@ -1027,6 +1027,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 		switch (mode) {
 		case PULL_MODE_MERGE:
+		case PULL_MODE_FAST_FORWARD:
 			opt_rebase = REBASE_FALSE;
 			break;
 		case PULL_MODE_REBASE:
@@ -1117,6 +1118,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 	can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);
 
+	if (mode == PULL_MODE_FAST_FORWARD && !can_ff)
+		die(_("The pull was not fast-forward, please either merge or rebase.\n"));
+
 	if (!opt_rebase && !can_ff) {
 		if (opt_verbosity >= 0)
 			show_advice_pull_non_ff();
diff --git a/rebase.c b/rebase.c
index bdfca49886..9fe99b5b16 100644
--- a/rebase.c
+++ b/rebase.c
@@ -40,6 +40,8 @@ enum pull_mode_type pull_mode_parse_value(const char *value)
 		return PULL_MODE_MERGE;
 	else if (!strcmp(value, "rebase") || !strcmp(value, "r"))
 		return PULL_MODE_REBASE;
+	else if (!strcmp(value, "fast-forward") || !strcmp(value, "f"))
+		return PULL_MODE_FAST_FORWARD;
 
 	return PULL_MODE_INVALID;
 }
diff --git a/rebase.h b/rebase.h
index 5ab8f4ddd5..e66a73feb4 100644
--- a/rebase.h
+++ b/rebase.h
@@ -17,7 +17,8 @@ enum pull_mode_type {
 	PULL_MODE_INVALID = -1,
 	PULL_MODE_DEFAULT = 0,
 	PULL_MODE_MERGE,
-	PULL_MODE_REBASE
+	PULL_MODE_REBASE,
+	PULL_MODE_FAST_FORWARD
 };
 
 enum pull_mode_type pull_mode_parse_value(const char *value);
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 59799ac4d5..45d818065f 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -873,4 +873,48 @@ test_expect_success 'git pull --rebase against local branch' '
 	test_cmp expect file2
 '
 
+setup_other () {
+	test_when_finished "git checkout master && git branch -D other test" &&
+	git checkout -b other $1 &&
+	>new &&
+	git add new &&
+	git commit -m new &&
+	git checkout -b test -t other &&
+	git reset --hard master
+}
+
+setup_ff () {
+	setup_other master
+}
+
+setup_non_ff () {
+	setup_other master^
+}
+
+test_expect_success 'fast-forward (pull.mode=fast-forward)' '
+	setup_ff &&
+	git -c pull.mode=fast-forward pull
+'
+
+test_expect_success 'non-fast-forward (pull.mode=fast-forward)' '
+	setup_non_ff &&
+	test_must_fail git -c pull.mode=fast-forward pull
+'
+
+test_expect_success 'non-fast-forward with merge (pull.mode=fast-forward)' '
+	setup_non_ff &&
+	git -c pull.mode=fast-forward pull --merge
+'
+
+test_expect_success 'non-fast-forward with rebase (pull.mode=fast-forward)' '
+	setup_non_ff &&
+	git -c pull.mode=fast-forward pull --rebase
+'
+
+test_expect_success 'non-fast-forward error message (pull.mode=fast-forward)' '
+	setup_non_ff &&
+	test_must_fail git -c pull.mode=fast-forward pull 2> error &&
+	test_i18ngrep "The pull was not fast-forward" error
+'
+
 test_done
-- 
2.30.0.rc0

[PATCH v2 12/13] pull: reorganize mode conditionals

[Felipe Contreras]

Now that everything is in place we can shuffle around the conditionals
so it's clearer what we are trying to do.

No functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/pull.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index ef23d8a52f..ad8afabe9b 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1118,12 +1118,12 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 	can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);
 
-	if (mode == PULL_MODE_FAST_FORWARD && !can_ff)
-		die(_("The pull was not fast-forward, please either merge or rebase.\n"));
-
-	if (!opt_rebase && !can_ff) {
-		if (opt_verbosity >= 0)
+	if (!can_ff) {
+		if (!mode && opt_verbosity >= 0)
 			show_advice_pull_non_ff();
+
+		if (mode == PULL_MODE_FAST_FORWARD)
+			die(_("The pull was not fast-forward, please either merge or rebase.\n"));
 	}
 
 	if (opt_rebase >= REBASE_TRUE) {
-- 
2.30.0.rc0

[PATCH v2 13/13] pull: improve --rebase and pull.rebase interaction

[Felipe Contreras]

Currently --rebase without argument overrides pull.rebase:

  git config pull.rebase merges
  git pull --rebase

Up until now this hasn't been a big issue, since user has not been
forced to specify a merge, or a rebase. But with the introduction of
--merge and pull.mode, the user could in theory have the following
configuration:

  git config pull.mode merge
  git config pull.rebase merges

In such case, the user would expect:

  git pull --rebase

To be the same as:

  git pull --rebase=merges

If the user wants to override the configuration, she can do:

  git pull --rebase=true

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/pull.c  | 10 ++++++++--
 t/t5520-pull.sh | 10 ++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index ad8afabe9b..4e8e9d85a2 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -68,8 +68,14 @@ static int parse_opt_rebase(const struct option *opt, const char *arg, int unset
 
 	if (arg)
 		*value = parse_config_rebase("--rebase", arg, 0);
-	else
-		*value = unset ? REBASE_FALSE : REBASE_TRUE;
+	else {
+		if (!unset) {
+			/* --rebase shouldn't override pull.rebase=merges (and others) */
+			if (*value < REBASE_TRUE)
+				*value = REBASE_TRUE;
+		} else
+			*value = REBASE_FALSE;
+	}
 
 	if (*value > 0)
 		mode = *value >= REBASE_TRUE ? PULL_MODE_REBASE : PULL_MODE_MERGE;
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 45d818065f..d0f5722eab 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -595,6 +595,16 @@ test_expect_success REBASE_P \
 	test_cmp_rev HEAD^2 keep-merge
 '
 
+test_expect_success REBASE_P \
+	'pull.rebase=preserve interacts correctly with pull.mode and --rebase' '
+	git reset --hard before-preserve-rebase &&
+	test_config pull.mode merge &&
+	test_config pull.rebase preserve &&
+	git pull --rebase . copy &&
+	test_cmp_rev HEAD^^ copy &&
+	test_cmp_rev HEAD^2 keep-merge
+'
+
 test_expect_success 'pull.rebase=interactive' '
 	write_script "$TRASH_DIRECTORY/fake-editor" <<-\EOF &&
 	echo I was here >fake.out &&
-- 
2.30.0.rc0