[RFC PATCH 00/35] git update: fix broken git pull

[RFC PATCH 00/35] git update: fix broken git pull

[Felipe Contreras]

Throughout the years many big threads have been started [1] [2] [3] [4]
[5] [6] because `git pull` is broken (for everyone that isn't a
maintainer), and nothing has been fixed.

The two main problems are:

  1. The order of the parents is the opposite of what it should be
  2. By default it should fail unless it's a fast-forward

However, every attempt to fix these issues gets entangled in an
inescapable web of interrelated problems.

That's because `git pull` is in fact *two* commands pretending to be
one.

I have spent several days re-reading old threads looking for anything
related to `git pull` and fast-forward throughout the entire history of
the git mailing list, going as far back as 2008.

This process has been illuminating.

The amount of people that have pondered about, and even suggested, a
`git update` command is striking:

 * Linus Torvalds: [7]
 * Junio C. Hamano: [8]
 * Richard Hansen: [9]
 * John Keeping: [10]

And of course me [11], who not only suggested it, but implemented it
as well [12].

It's not just the name, but the behavior too:

  `git fetch` + `git merge --ff-only`

This way `git pull` can be used by maintainers to integrate pull
requests, and `git update` for everyone else (normal developers) to
update their local branch to its upstream.

Plus it has a --merge mode that creates a merge commit, but with the
parents in the right order (master to upstream, not upstream to master).

Both problems are fixed by this approach.


Also, a thorny issue since a long time ago has been how to properly
advise users what to do in case a fast-forward is not possible.

I propose a new command: `git fast-forward`. When a fast-forward is not
possible `git fast-forward` fails, and shows an advice (that can be
easily turned off).

Therefore in fact `git update` is really `git fetch` +
`git fast-forward`, and it's the latter that throws the advice. The
advice also suggests `git help fast-forward` which has an explanation
about what a fast-forward is, what are the options to synchronize
with the remote branch (merge and rebase), and what do they look like.

In addition there's a ton of fixes for `git pull` which have been sent
before, including the `pull.mode` configuration, the `--merge` option,
the per-branch `pullMode` configuration, `pull.mode=fast-forward`, and
an improved advice message to prepare users for a future default change:

  The pull was not a fast-forward, in the future you will have to choose
  between a merge or a rebase.

  To quell this message you have two main options:

  1. Adopt the new behavior:

    git config --global pull.mode fast-forward

  2. Maintain the current behavior:

    git config --global pull.mode merge

  For now we will fall back to the traditional behavior: merge.
  For more information check "git help fast-forward".

It doesn't just improve the situation for normal developers, but it also
opens the possibility to fix a wart in `git pull --rebase`:

Right now:

 git pull --merge github john (merge "github/john" into "master)
 git pull --rebase github john (rebase "master" onto "github/john")

Since now `git update --rebase` can be used to rebase the current branch
onto its upstream, that leaves `git pull --rebase` open to rebase
github/john onto master, not the other way around like it currently is.


Junio has suggested there's no consensus over fast-forward by default,
but that's not true. Here is a non-exhaustive list of people that have
agreed the default should be fast-forward only.

 * Linus Torvalds
 * Junio C Hamano
 * Jeff King
 * Jonathan Nieder
 * Richard Hansen
 * Philip Oakley
 * Elijah Newren
 * John Keeping
 * Ramkumar Ramachandra
 * Alex Henrie
 * W. Trevor King
 * Greg Troxel
 * Peter Kjellerstedt
 * Konstantin Tokarev
 * Robert Dailey
 * Vít Ondruch
 * Raymond E. Pasco
 * Jacob Keller
 * Felipe Contreras

There's only one person against the change: Matthieu Moy. His argument
back then [13] was: why ask users to merge or rebase if newcomers will
not pick a rebase? To that my response is: for the same reason we ask
them to do `git commit --all`: so that they eventually learn about the
staging area. Just like they can do `git commit --all`, they can do
`git pull --merge`. They can figure out what that means later.

The people in favor of reordering the parents:

 * Andreas Krey
 * John Szakmeister
 * Jeremy Rosen
 * John Keeping

And the people in favor of my old `git update`:

 * Damien Robert
 * Ping Yin
 * Philippe Vaucher


This patch series doesn't implement all of the features my old
`git update` had, but it implements most of them, most
importantly--unlike `git pull`--it's not broken.

Right now only `git update` without arguments works, and only if the
upstream branch is configured. I have a pretty good idea of what the it
should do with different arguments (as I had already implemented that),
but for now that's open to discussion.

If you want to give it a try:

https://github.com/felipec/git/tree/fc/update

[1] https://lore.kernel.org/git/200910201947.50423.trast@student.ethz.ch/
[2] https://lore.kernel.org/git/20130522115042.GA20649@inner.h.apk.li/
[3] https://lore.kernel.org/git/1377988690-23460-1-git-send-email-felipe.contreras@gmail.com/
[4] https://lore.kernel.org/git/4ay6w9i74cygt6ii1b0db7wg.1398433713382@email.android.com/
[5] https://lore.kernel.org/git/5363BB9F.40102@xiplink.com/
[6] https://lore.kernel.org/git/20201204061623.1170745-1-felipe.contreras@gmail.com/
[7] https://lore.kernel.org/git/CA+55aFzxsNxgKD1uGZQCiib+=+wCMSa0=B+Ye3Zi-u6kpz8Vrg@mail.gmail.com/
[8] https://lore.kernel.org/git/xmqqppjyhnom.fsf@gitster.dls.corp.google.com/
[9] https://lore.kernel.org/git/5228A14B.3000804@bbn.com/
[10] https://lore.kernel.org/git/20130628174252.GF2232@serenity.lan/
[11] https://lore.kernel.org/git/5366db742d494_18f9e4b308aa@nysa.notmuch/
[12] https://github.com/felipec/git/commit/61e05c9798b3c74bab8b2d47ede4c12e8e305345
[13] https://lore.kernel.org/git/vpqbo3za8r9.fsf@anie.imag.fr/

Felipe Contreras (35):
  merge: improve fatal fast-forward message
  merge: split cmd_merge()
  fast-forward: add new builtin
  doc: fast-forward: explain what it is
  fast-forward: add advice for novices
  fast-forward: make the advise configurable
  fast-forward: add help about merge vs. rebase
  update: new built-in
  update: add options and usage skeleton
  update: add --ff option
  update: add --merge mode
  commit: support for multiple MERGE_MODE
  merge: add --reverse-parents option
  update: reverse the order of parents
  update: fake a reverse order of parents in message
  update: add --rebase mode
  update: add mode configuation
  update: add per-branch mode configuration
  pull: cleanup autostash check
  pull: trivial cleanup
  pull: trivial whitespace style fix
  pull: introduce --merge option
  rebase: add REBASE_DEFAULT
  pull: move configuration fetches
  pull: show warning with --ff options
  pull: add pull.mode
  pull: add per-branch mode configuration
  pull: add pull.mode=fast-forward
  pull: reorganize mode conditionals
  pull: add diverging advice on fast-forward mode
  pull: improve --rebase and pull.rebase interaction
  pull: improve default warning
  pull: advice of future changes
  FUTURE: pull: enable ff-only mode by default
  !fixup FUTURE: pull: enable ff-only mode by default

 .gitignore                             |   2 +
 Documentation/config.txt               |   4 +
 Documentation/config/advice.txt        |   2 +
 Documentation/config/branch.txt        |  10 ++
 Documentation/config/pull.txt          |   6 +
 Documentation/config/update.txt        |   5 +
 Documentation/git-fast-forward.txt     | 104 +++++++++++++++++
 Documentation/git-pull.txt             |  10 +-
 Documentation/git-update.txt           |  47 ++++++++
 Documentation/merge-options.txt        |   4 +
 Makefile                               |   2 +
 advice.c                               |  15 +++
 advice.h                               |   2 +
 builtin.h                              |   2 +
 builtin/commit.c                       |   7 +-
 builtin/merge.c                        |  59 +++++++---
 builtin/pull.c                         | 156 +++++++++++++++++--------
 builtin/update.c                       | 153 ++++++++++++++++++++++++
 contrib/completion/git-completion.bash |  22 ++++
 fmt-merge-msg.c                        |  21 +++-
 fmt-merge-msg.h                        |   3 +-
 git.c                                  |   2 +
 rebase.c                               |  12 ++
 rebase.h                               |  13 ++-
 t/t4013-diff-various.sh                |   2 +-
 t/t5520-pull.sh                        | 123 +++++++++++++++++--
 t/t5521-pull-options.sh                |   4 +-
 t/t5524-pull-msg.sh                    |   4 +-
 t/t5553-set-upstream.sh                |  14 +--
 t/t5563-update.sh                      |  87 ++++++++++++++
 t/t5604-clone-reference.sh             |   4 +-
 t/t6402-merge-rename.sh                |  16 +--
 t/t6409-merge-subtree.sh               |   6 +-
 t/t6417-merge-ours-theirs.sh           |  10 +-
 t/t7600-merge.sh                       |  47 ++++++++
 t/t7601-merge-pull-config.sh           | 116 ------------------
 t/t7603-merge-reduce-heads.sh          |   2 +-
 37 files changed, 870 insertions(+), 228 deletions(-)
 create mode 100644 Documentation/config/update.txt
 create mode 100644 Documentation/git-fast-forward.txt
 create mode 100644 Documentation/git-update.txt
 create mode 100644 builtin/update.c
 create mode 100755 t/t5563-update.sh

-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 01/35] merge: improve fatal fast-forward message

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/merge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index a8a843b1f5..05e631229d 100644
--- 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(_("unable to fast-forward"));
 
 	if (autostash)
 		create_autostash(the_repository,
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 02/35] merge: split cmd_merge()

[Felipe Contreras]

We want to re-use most of cmd_merge() for a new command.

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

diff --git a/builtin/merge.c b/builtin/merge.c
index 05e631229d..9c944f5f0f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1259,7 +1259,8 @@ static int merging_a_throwaway_tag(struct commit *commit)
 	return is_throwaway_tag;
 }
 
-int cmd_merge(int argc, const char **argv, const char *prefix)
+static int merge_common(int argc, const char **argv, const char *prefix,
+	const struct option *options, const char * const usage[])
 {
 	struct object_id result_tree, stash, head_oid;
 	struct commit *head_commit;
@@ -1273,7 +1274,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	int orig_argc = argc;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage_with_options(builtin_merge_usage, builtin_merge_options);
+		usage_with_options(usage, options);
 
 	/*
 	 * Check if we are _not_ on a detached HEAD, i.e. if there is a
@@ -1299,8 +1300,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 	if (branch_mergeoptions)
 		parse_branch_merge_options(branch_mergeoptions);
-	argc = parse_options(argc, argv, prefix, builtin_merge_options,
-			builtin_merge_usage, 0);
+	argc = parse_options(argc, argv, prefix, options, usage, 0);
 	if (shortlog_len < 0)
 		shortlog_len = (merge_log_config > 0) ? merge_log_config : 0;
 
@@ -1314,7 +1314,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 		if (orig_argc != 2)
 			usage_msg_opt(_("--abort expects no arguments"),
-			      builtin_merge_usage, builtin_merge_options);
+				usage, options);
 
 		if (!file_exists(git_path_merge_head(the_repository)))
 			die(_("There is no merge to abort (MERGE_HEAD missing)."));
@@ -1336,8 +1336,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	if (quit_current_merge) {
 		if (orig_argc != 2)
 			usage_msg_opt(_("--quit expects no arguments"),
-				      builtin_merge_usage,
-				      builtin_merge_options);
+				usage, options);
 
 		remove_merge_branch_state(the_repository);
 		goto done;
@@ -1349,7 +1348,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 		if (orig_argc != 2)
 			usage_msg_opt(_("--continue expects no arguments"),
-			      builtin_merge_usage, builtin_merge_options);
+				usage, options);
 
 		if (!file_exists(git_path_merge_head(the_repository)))
 			die(_("There is no merge in progress (MERGE_HEAD missing)."));
@@ -1416,8 +1415,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	}
 
 	if (!argc)
-		usage_with_options(builtin_merge_usage,
-			builtin_merge_options);
+		usage_with_options(usage, options);
 
 	if (!head_commit) {
 		/*
@@ -1458,8 +1456,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 				      argc, argv, &merge_msg);
 
 	if (!head_commit || !argc)
-		usage_with_options(builtin_merge_usage,
-			builtin_merge_options);
+		usage_with_options(usage, options);
 
 	if (verify_signatures) {
 		for (p = remoteheads; p; p = p->next) {
@@ -1738,3 +1735,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	free(branch_to_free);
 	return ret;
 }
+
+int cmd_merge(int argc, const char **argv, const char *prefix)
+{
+	return merge_common(argc, argv, prefix, builtin_merge_options, builtin_merge_usage);
+}
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 03/35] fast-forward: add new builtin

[Felipe Contreras]

This is one of the most common git operations, it makes sense it has its
own built-in.

This is basically the same as `git merge --ff-only` (for now).

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 .gitignore                             |  1 +
 Documentation/git-fast-forward.txt     | 26 ++++++++++++++++++++++++++
 Makefile                               |  1 +
 builtin.h                              |  1 +
 builtin/merge.c                        | 15 +++++++++++++++
 contrib/completion/git-completion.bash | 10 ++++++++++
 git.c                                  |  1 +
 t/t7600-merge.sh                       | 21 +++++++++++++++++++++
 8 files changed, 76 insertions(+)
 create mode 100644 Documentation/git-fast-forward.txt

diff --git a/.gitignore b/.gitignore
index 311841f9be..45703399b0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -62,6 +62,7 @@
 /git-describe
 /git-env--helper
 /git-fast-export
+/git-fast-forward
 /git-fast-import
 /git-fetch
 /git-fetch-pack
diff --git a/Documentation/git-fast-forward.txt b/Documentation/git-fast-forward.txt
new file mode 100644
index 0000000000..d457022629
--- /dev/null
+++ b/Documentation/git-fast-forward.txt
@@ -0,0 +1,26 @@
+git-fast-forward(1)
+===================
+
+NAME
+----
+git-fast-forward - Advance the branch pointer
+
+SYNOPSIS
+--------
+[verse]
+'git fast-forward' [<commit>]
+
+DESCRIPTION
+-----------
+Incorporates changes into the current branch. By default the upstream branch is
+used, but a different commit can be specified in the arguments.
+
+THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOUR MAY CHANGE.
+
+SEE ALSO
+--------
+linkgit:git-merge[1]
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index c3565fc0f8..cf42162a07 100644
--- a/Makefile
+++ b/Makefile
@@ -772,6 +772,7 @@ BUILT_INS += $(patsubst builtin/%.o,git-%$X,$(BUILTIN_OBJS))
 
 BUILT_INS += git-cherry$X
 BUILT_INS += git-cherry-pick$X
+BUILT_INS += git-fast-forward$X
 BUILT_INS += git-format-patch$X
 BUILT_INS += git-fsck-objects$X
 BUILT_INS += git-init$X
diff --git a/builtin.h b/builtin.h
index 16ecd5586f..601e438c9b 100644
--- a/builtin.h
+++ b/builtin.h
@@ -151,6 +151,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix);
 int cmd_difftool(int argc, const char **argv, const char *prefix);
 int cmd_env__helper(int argc, const char **argv, const char *prefix);
 int cmd_fast_export(int argc, const char **argv, const char *prefix);
+int cmd_fast_forward(int argc, const char **argv, const char *prefix);
 int cmd_fast_import(int argc, const char **argv, const char *prefix);
 int cmd_fetch(int argc, const char **argv, const char *prefix);
 int cmd_fetch_pack(int argc, const char **argv, const char *prefix);
diff --git a/builtin/merge.c b/builtin/merge.c
index 9c944f5f0f..e396943d37 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -61,6 +61,11 @@ static const char * const builtin_merge_usage[] = {
 	NULL
 };
 
+static const char * const builtin_ff_usage[] = {
+	N_("git fast-forward [<commit>]"),
+	NULL
+};
+
 static int show_diffstat = 1, shortlog_len = -1, squash;
 static int option_commit = -1;
 static int option_edit = -1;
@@ -304,6 +309,10 @@ static struct option builtin_merge_options[] = {
 	OPT_END()
 };
 
+static struct option builtin_ff_options[] = {
+	OPT_END()
+};
+
 static int save_state(struct object_id *stash)
 {
 	int len;
@@ -1740,3 +1749,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 {
 	return merge_common(argc, argv, prefix, builtin_merge_options, builtin_merge_usage);
 }
+
+int cmd_fast_forward(int argc, const char **argv, const char *prefix)
+{
+	fast_forward = FF_ONLY;
+	return merge_common(argc, argv, prefix, builtin_ff_options, builtin_ff_usage);
+}
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b50c5d0ea3..cfaee3aaeb 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1786,6 +1786,16 @@ _git_difftool ()
 	__git_complete_revlist_file
 }
 
+_git_fast_forward ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin fast-forward
+		return
+	esac
+	__git_complete_refs
+}
+
 __git_fetch_recurse_submodules="yes on-demand no"
 
 _git_fetch ()
diff --git a/git.c b/git.c
index 18bed9a996..6ab1fb9251 100644
--- a/git.c
+++ b/git.c
@@ -524,6 +524,7 @@ static struct cmd_struct commands[] = {
 	{ "difftool", cmd_difftool, RUN_SETUP_GENTLY },
 	{ "env--helper", cmd_env__helper },
 	{ "fast-export", cmd_fast_export, RUN_SETUP },
+	{ "fast-forward", cmd_fast_forward, RUN_SETUP | NEED_WORK_TREE },
 	{ "fast-import", cmd_fast_import, RUN_SETUP | NO_PARSEOPT },
 	{ "fetch", cmd_fetch, RUN_SETUP },
 	{ "fetch-pack", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 1cbc9715a8..10f8956665 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -205,6 +205,16 @@ test_expect_success 'merge c0 with c1 with --ff-only' '
 
 test_debug 'git log --graph --decorate --oneline --all'
 
+test_expect_success 'fast-forward c0 with c1' '
+	git reset --hard c0 &&
+	git fast-forward c1 &&
+	git fast-forward HEAD c0 c1 &&
+	verify_merge file result.1 &&
+	verify_head "$c1"
+'
+
+test_debug 'git log --graph --decorate --oneline --all'
+
 test_expect_success 'merge from unborn branch' '
 	git checkout -f main &&
 	test_might_fail git branch -D kid &&
@@ -322,6 +332,17 @@ test_expect_success 'merges with --ff-only' '
 	verify_head $c3
 '
 
+test_expect_success 'fast-forward' '
+	git reset --hard c1 &&
+	test_tick &&
+	test_must_fail git fast-forward c2 &&
+	test_must_fail git fast-forward c3 &&
+	test_must_fail git fast-forward c2 c3 &&
+	git reset --hard c0 &&
+	git merge c3 &&
+	verify_head $c3
+'
+
 test_expect_success 'merges with merge.ff=only' '
 	git reset --hard c1 &&
 	test_tick &&
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 04/35] doc: fast-forward: explain what it is

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-fast-forward.txt | 32 +++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-fast-forward.txt b/Documentation/git-fast-forward.txt
index d457022629..b7322c437e 100644
--- a/Documentation/git-fast-forward.txt
+++ b/Documentation/git-fast-forward.txt
@@ -15,11 +15,41 @@ DESCRIPTION
 Incorporates changes into the current branch. By default the upstream branch is
 used, but a different commit can be specified in the arguments.
 
+Assume the following history exists and the current branch is
+`master`:
+
+------------
+    D---C---B---A origin/master
+	^
+	|
+      master
+------------
+
+Then `git fast-forward` will advance the local `master` to `origin/master`:
+
+------------
+    D---C---B---A master, origin/master
+------------
+
+This operation is not always possible; if you made changes and the branches
+have diverged:
+
+------------
+    D---C---B---A origin/master
+	 \
+	  X---Y master
+------------
+
+then the fast-forward command will fail.
+
+In those cases you need to either `git merge`, or `git rebase` in order to
+synchronize the two branches.
+
 THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOUR MAY CHANGE.
 
 SEE ALSO
 --------
-linkgit:git-merge[1]
+linkgit:git-merge[1], linkgit:git-rebase[1]
 
 GIT
 ---
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 05/35] fast-forward: add advice for novices

[Felipe Contreras]

It doesn't hurt showing it on `git merge --ff-only` too.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 advice.c        | 11 +++++++++++
 advice.h        |  1 +
 builtin/merge.c |  4 +++-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/advice.c b/advice.c
index 0b9c89c48a..8f068c8be9 100644
--- a/advice.c
+++ b/advice.c
@@ -326,3 +326,14 @@ void detach_advice(const char *new_name)
 
 	fprintf(stderr, fmt, new_name);
 }
+
+void diverging_advice(void)
+{
+	advise(_("Diverging branches can't be fast-forwarded, you need to either:\n"
+		"\n"
+		"\tgit merge\n"
+		"\n"
+		"or:\n"
+		"\n"
+		"\tgit rebase\n"));
+}
diff --git a/advice.h b/advice.h
index bd26c385d0..6ce967c962 100644
--- a/advice.h
+++ b/advice.h
@@ -97,5 +97,6 @@ void NORETURN die_resolve_conflict(const char *me);
 void NORETURN die_conclude_merge(void);
 void advise_on_updating_sparse_paths(struct string_list *pathspec_list);
 void detach_advice(const char *new_name);
+void diverging_advice(void);
 
 #endif /* ADVICE_H */
diff --git a/builtin/merge.c b/builtin/merge.c
index e396943d37..1836f98f82 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1625,8 +1625,10 @@ static int merge_common(int argc, const char **argv, const char *prefix,
 		}
 	}
 
-	if (fast_forward == FF_ONLY)
+	if (fast_forward == FF_ONLY) {
+		diverging_advice();
 		die(_("unable to fast-forward"));
+	}
 
 	if (autostash)
 		create_autostash(the_repository,
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 06/35] fast-forward: make the advise configurable

[Felipe Contreras]

With advice.diverging.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/config/advice.txt | 2 ++
 advice.c                        | 4 +++-
 advice.h                        | 1 +
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index 8b2849ff7b..dae85310dc 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -123,4 +123,6 @@ advice.*::
 		Advice shown when either linkgit:git-add[1] or linkgit:git-rm[1]
 		is asked to update index entries outside the current sparse
 		checkout.
+	diverging::
+		Advice shown when a fast-forward is not possible.
 --
diff --git a/advice.c b/advice.c
index 8f068c8be9..60de7fbc4e 100644
--- a/advice.c
+++ b/advice.c
@@ -110,6 +110,7 @@ static struct {
 	[ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME] 	= { "checkoutAmbiguousRemoteBranchName", 1 },
 	[ADVICE_COMMIT_BEFORE_MERGE]			= { "commitBeforeMerge", 1 },
 	[ADVICE_DETACHED_HEAD]				= { "detachedHead", 1 },
+	[ADVICE_DIVERGING]				= { "diverging", 1 },
 	[ADVICE_FETCH_SHOW_FORCED_UPDATES]		= { "fetchShowForcedUpdates", 1 },
 	[ADVICE_GRAFT_FILE_DEPRECATED]			= { "graftFileDeprecated", 1 },
 	[ADVICE_IGNORED_HOOK]				= { "ignoredHook", 1 },
@@ -329,7 +330,8 @@ void detach_advice(const char *new_name)
 
 void diverging_advice(void)
 {
-	advise(_("Diverging branches can't be fast-forwarded, you need to either:\n"
+	advise_if_enabled(ADVICE_DIVERGING,
+		_("Diverging branches can't be fast-forwarded, you need to either:\n"
 		"\n"
 		"\tgit merge\n"
 		"\n"
diff --git a/advice.h b/advice.h
index 6ce967c962..695f5a62bb 100644
--- a/advice.h
+++ b/advice.h
@@ -49,6 +49,7 @@ extern int advice_add_empty_pathspec;
 	ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME,
 	ADVICE_COMMIT_BEFORE_MERGE,
 	ADVICE_DETACHED_HEAD,
+	ADVICE_DIVERGING,
 	ADVICE_FETCH_SHOW_FORCED_UPDATES,
 	ADVICE_GRAFT_FILE_DEPRECATED,
 	ADVICE_IGNORED_HOOK,
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 07/35] fast-forward: add help about merge vs. rebase

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-fast-forward.txt | 48 ++++++++++++++++++++++++++++++
 advice.c                           |  4 ++-
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-fast-forward.txt b/Documentation/git-fast-forward.txt
index b7322c437e..a73834e0f9 100644
--- a/Documentation/git-fast-forward.txt
+++ b/Documentation/git-fast-forward.txt
@@ -47,6 +47,54 @@ synchronize the two branches.
 
 THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOUR MAY CHANGE.
 
+MERGE OR REBASE
+---------------
+
+The decision to whether merge or rebase depends on the situation, and the
+project. Traditionally git has prefered merge over rebase, but that creates a
+new commit, and that's frowned up on some projects, so you can't just simply
+choose to merge blindly.
+
+------------
+    D---C---B---A origin/master
+	 \
+	  X---Y master
+------------
+
+The nature of distributed version control systems make this divergence
+unavoidable. You must decide how to synchronize this divergence.
+
+If you choose to merge, the two heads (master and origin/master) will be joined
+together in a new commit:
+
+------------
+	  origin/master
+		|
+		v
+    D---C---B---A---M master
+	 \	   /
+	  X---Y---+
+------------
+
+This new commit is called a merge commit and has two parents (A and Y).
+
+Rebasing on the other hand rewrites the history:
+
+------------
+	  origin/master
+		|
+		v
+    D---C---B---A---X'---Y' master
+------------
+
+The commits that diverged (X and Y) are rewritten as if they were created on top
+of the new base (A). This creates a linear history, which is cleaner, but some
+people prefer to preserve the original hsitory.
+
+In both cases it's likely you would have to resolve conflicts, the difference is
+that in a merge you would have to do it all at once in one commit, while with a
+rebase you would have to do it on every rewritten commit.
+
 SEE ALSO
 --------
 linkgit:git-merge[1], linkgit:git-rebase[1]
diff --git a/advice.c b/advice.c
index 60de7fbc4e..7f422b05d3 100644
--- a/advice.c
+++ b/advice.c
@@ -337,5 +337,7 @@ void diverging_advice(void)
 		"\n"
 		"or:\n"
 		"\n"
-		"\tgit rebase\n"));
+		"\tgit rebase\n"
+		"\n"
+		"For more information check \"git help fast-forward\".\n"));
 }
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 08/35] update: new built-in

[Felipe Contreras]

This is basically `git fetch` + `git fast-forward`.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 .gitignore                             |  1 +
 Documentation/config.txt               |  2 ++
 Documentation/git-update.txt           | 32 +++++++++++++++++
 Makefile                               |  1 +
 builtin.h                              |  1 +
 builtin/update.c                       | 48 ++++++++++++++++++++++++++
 contrib/completion/git-completion.bash | 12 +++++++
 git.c                                  |  1 +
 t/t5563-update.sh                      | 45 ++++++++++++++++++++++++
 9 files changed, 143 insertions(+)
 create mode 100644 Documentation/git-update.txt
 create mode 100644 builtin/update.c
 create mode 100755 t/t5563-update.sh

diff --git a/.gitignore b/.gitignore
index 45703399b0..2a3bc43ef2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -171,6 +171,7 @@
 /git-tag
 /git-unpack-file
 /git-unpack-objects
+/git-update
 /git-update-index
 /git-update-ref
 /git-update-server-info
diff --git a/Documentation/config.txt b/Documentation/config.txt
index bf82766a6a..fc4b49c0d4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -440,6 +440,8 @@ include::config/rerere.txt[]
 
 include::config/reset.txt[]
 
+include::config/update.txt[]
+
 include::config/sendemail.txt[]
 
 include::config/sequencer.txt[]
diff --git a/Documentation/git-update.txt b/Documentation/git-update.txt
new file mode 100644
index 0000000000..54c49c5d12
--- /dev/null
+++ b/Documentation/git-update.txt
@@ -0,0 +1,32 @@
+git-update(1)
+=============
+
+NAME
+----
+git-update - Update the current branch to the latest remote
+
+SYNOPSIS
+--------
+[verse]
+'git update'
+
+DESCRIPTION
+-----------
+
+Incorporates changes from a remote repository into the current branch.
+
+`git update` runs `git fetch` and then tries to advance the current branch to
+the remote branch with `git fast-forward`. If you don't have any extra changes
+the update operation is straight-forward, but if you do a further `git merge` or
+`git rebase` will be needed.
+
+THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOUR MAY CHANGE.
+
+SEE ALSO
+--------
+linkgit:git-fetch[1], linkgit:git-fast-forward[1],
+linkgit:git-merge[1], linkgit:git-rebase[1]
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index cf42162a07..6450574feb 100644
--- a/Makefile
+++ b/Makefile
@@ -1161,6 +1161,7 @@ BUILTIN_OBJS += builtin/symbolic-ref.o
 BUILTIN_OBJS += builtin/tag.o
 BUILTIN_OBJS += builtin/unpack-file.o
 BUILTIN_OBJS += builtin/unpack-objects.o
+BUILTIN_OBJS += builtin/update.o
 BUILTIN_OBJS += builtin/update-index.o
 BUILTIN_OBJS += builtin/update-ref.o
 BUILTIN_OBJS += builtin/update-server-info.o
diff --git a/builtin.h b/builtin.h
index 601e438c9b..7d18897682 100644
--- a/builtin.h
+++ b/builtin.h
@@ -229,6 +229,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix);
 int cmd_tar_tree(int argc, const char **argv, const char *prefix);
 int cmd_unpack_file(int argc, const char **argv, const char *prefix);
 int cmd_unpack_objects(int argc, const char **argv, const char *prefix);
+int cmd_update(int argc, const char **argv, const char *prefix);
 int cmd_update_index(int argc, const char **argv, const char *prefix);
 int cmd_update_ref(int argc, const char **argv, const char *prefix);
 int cmd_update_server_info(int argc, const char **argv, const char *prefix);
diff --git a/builtin/update.c b/builtin/update.c
new file mode 100644
index 0000000000..51e45b453d
--- /dev/null
+++ b/builtin/update.c
@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2021 Felipe Contreras
+ */
+
+#include "builtin.h"
+#include "run-command.h"
+#include "dir.h"
+
+static int run_fetch(void)
+{
+	struct strvec args = STRVEC_INIT;
+	int ret;
+
+	strvec_pushl(&args, "fetch", "--update-head-ok", NULL);
+
+	ret = run_command_v_opt(args.v, RUN_GIT_CMD);
+	strvec_clear(&args);
+	return ret;
+}
+
+static int run_fast_forward(void)
+{
+	struct strvec args = STRVEC_INIT;
+	int ret;
+
+	strvec_pushl(&args, "fast-forward", "FETCH_HEAD", NULL);
+
+	ret = run_command_v_opt(args.v, RUN_GIT_CMD);
+	strvec_clear(&args);
+	return ret;
+}
+
+int cmd_update(int argc, const char **argv, const char *prefix)
+{
+	if (!getenv("GIT_REFLOG_ACTION"))
+		setenv("GIT_REFLOG_ACTION", "update", 0);
+
+	if (repo_read_index_unmerged(the_repository))
+		die_resolve_conflict("update");
+
+	if (file_exists(git_path_merge_head(the_repository)))
+		die_conclude_merge();
+
+	if (run_fetch())
+		return 1;
+
+	return run_fast_forward();
+}
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index cfaee3aaeb..c5214d9856 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3257,6 +3257,18 @@ _git_tag ()
 	esac
 }
 
+_git_update ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin update
+
+		return
+		;;
+	esac
+	__git_complete_remote_or_refspec
+}
+
 _git_whatchanged ()
 {
 	_git_log
diff --git a/git.c b/git.c
index 6ab1fb9251..0156ea81a4 100644
--- a/git.c
+++ b/git.c
@@ -610,6 +610,7 @@ static struct cmd_struct commands[] = {
 	{ "tag", cmd_tag, RUN_SETUP | DELAY_PAGER_CONFIG },
 	{ "unpack-file", cmd_unpack_file, RUN_SETUP | NO_PARSEOPT },
 	{ "unpack-objects", cmd_unpack_objects, RUN_SETUP | NO_PARSEOPT },
+	{ "update", cmd_update, RUN_SETUP | NEED_WORK_TREE },
 	{ "update-index", cmd_update_index, RUN_SETUP },
 	{ "update-ref", cmd_update_ref, RUN_SETUP },
 	{ "update-server-info", cmd_update_server_info, RUN_SETUP },
diff --git a/t/t5563-update.sh b/t/t5563-update.sh
new file mode 100755
index 0000000000..951df41ac4
--- /dev/null
+++ b/t/t5563-update.sh
@@ -0,0 +1,45 @@
+#!/bin/sh
+
+test_description='update'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo one > file &&
+	git add file &&
+	git commit -a -m one &&
+	echo two > file &&
+	git commit -a -m two
+'
+
+test_expect_success 'basic update' '
+	test_when_finished "rm -rf test" &&
+	(
+	git clone . test &&
+	cd test &&
+	git reset --hard @^ &&
+	git update &&
+	test_cmp_rev master origin/master
+	)
+'
+
+test_expect_success 'non-fast-forward update' '
+	test_when_finished "rm -rf test" &&
+	(
+	git clone . test &&
+	cd 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 update &&
+	test_cmp_rev @ master
+	)
+'
+
+test_done
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 09/35] update: add options and usage skeleton

[Felipe Contreras]

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

diff --git a/builtin/update.c b/builtin/update.c
index 51e45b453d..1a69896aa8 100644
--- a/builtin/update.c
+++ b/builtin/update.c
@@ -3,9 +3,19 @@
  */
 
 #include "builtin.h"
+#include "parse-options.h"
 #include "run-command.h"
 #include "dir.h"
 
+static const char * const update_usage[] = {
+	N_("git update"),
+	NULL
+};
+
+static struct option update_options[] = {
+	OPT_END()
+};
+
 static int run_fetch(void)
 {
 	struct strvec args = STRVEC_INIT;
@@ -35,6 +45,8 @@ int cmd_update(int argc, const char **argv, const char *prefix)
 	if (!getenv("GIT_REFLOG_ACTION"))
 		setenv("GIT_REFLOG_ACTION", "update", 0);
 
+	argc = parse_options(argc, argv, prefix, update_options, update_usage, 0);
+
 	if (repo_read_index_unmerged(the_repository))
 		die_resolve_conflict("update");
 
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 10/35] update: add --ff option

[Felipe Contreras]

This is the default, it doesn't change anything.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-update.txt |  9 ++++++++-
 builtin/update.c             | 17 +++++++++++++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-update.txt b/Documentation/git-update.txt
index 54c49c5d12..df778b7ef4 100644
--- a/Documentation/git-update.txt
+++ b/Documentation/git-update.txt
@@ -8,7 +8,7 @@ git-update - Update the current branch to the latest remote
 SYNOPSIS
 --------
 [verse]
-'git update'
+'git update' [<options>]
 
 DESCRIPTION
 -----------
@@ -22,6 +22,13 @@ the update operation is straight-forward, but if you do a further `git merge` or
 
 THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOUR MAY CHANGE.
 
+OPTIONS
+-------
+
+-f::
+--ff::
+	Forces a fast-forward.
+
 SEE ALSO
 --------
 linkgit:git-fetch[1], linkgit:git-fast-forward[1],
diff --git a/builtin/update.c b/builtin/update.c
index 1a69896aa8..34681fe21a 100644
--- a/builtin/update.c
+++ b/builtin/update.c
@@ -7,12 +7,22 @@
 #include "run-command.h"
 #include "dir.h"
 
+enum update_mode_type {
+	UPDATE_MODE_FAST_FORWARD = 0
+};
+
+static enum update_mode_type mode = UPDATE_MODE_FAST_FORWARD;
+
 static const char * const update_usage[] = {
-	N_("git update"),
+	N_("git update [<options>]"),
 	NULL
 };
 
 static struct option update_options[] = {
+	OPT_SET_INT_F('f', "ff", &mode,
+		N_("incorporate changes by fast-forwarding"),
+		UPDATE_MODE_FAST_FORWARD, PARSE_OPT_NONEG),
+
 	OPT_END()
 };
 
@@ -56,5 +66,8 @@ int cmd_update(int argc, const char **argv, const char *prefix)
 	if (run_fetch())
 		return 1;
 
-	return run_fast_forward();
+	if (mode == UPDATE_MODE_FAST_FORWARD)
+		return run_fast_forward();
+
+	return 1;
 }
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 11/35] update: add --merge mode

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-update.txt |  4 ++++
 builtin/update.c             | 20 +++++++++++++++++++-
 t/t5563-update.sh            | 15 +++++++++++++++
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-update.txt b/Documentation/git-update.txt
index df778b7ef4..075653c6fd 100644
--- a/Documentation/git-update.txt
+++ b/Documentation/git-update.txt
@@ -29,6 +29,10 @@ OPTIONS
 --ff::
 	Forces a fast-forward.
 
+-m::
+--merge::
+	Forces a merge.
+
 SEE ALSO
 --------
 linkgit:git-fetch[1], linkgit:git-fast-forward[1],
diff --git a/builtin/update.c b/builtin/update.c
index 34681fe21a..5946b383f5 100644
--- a/builtin/update.c
+++ b/builtin/update.c
@@ -8,7 +8,8 @@
 #include "dir.h"
 
 enum update_mode_type {
-	UPDATE_MODE_FAST_FORWARD = 0
+	UPDATE_MODE_FAST_FORWARD = 0,
+	UPDATE_MODE_MERGE
 };
 
 static enum update_mode_type mode = UPDATE_MODE_FAST_FORWARD;
@@ -22,6 +23,9 @@ static struct option update_options[] = {
 	OPT_SET_INT_F('f', "ff", &mode,
 		N_("incorporate changes by fast-forwarding"),
 		UPDATE_MODE_FAST_FORWARD, PARSE_OPT_NONEG),
+	OPT_SET_INT_F('m', "merge", &mode,
+		N_("incorporate changes by merging"),
+		UPDATE_MODE_MERGE, PARSE_OPT_NONEG),
 
 	OPT_END()
 };
@@ -50,6 +54,18 @@ static int run_fast_forward(void)
 	return ret;
 }
 
+static int run_merge(void)
+{
+	int ret;
+	struct strvec args = STRVEC_INIT;
+
+	strvec_pushl(&args, "merge", "FETCH_HEAD", NULL);
+
+	ret = run_command_v_opt(args.v, RUN_GIT_CMD);
+	strvec_clear(&args);
+	return ret;
+}
+
 int cmd_update(int argc, const char **argv, const char *prefix)
 {
 	if (!getenv("GIT_REFLOG_ACTION"))
@@ -68,6 +84,8 @@ int cmd_update(int argc, const char **argv, const char *prefix)
 
 	if (mode == UPDATE_MODE_FAST_FORWARD)
 		return run_fast_forward();
+	if (mode == UPDATE_MODE_MERGE)
+		return run_merge();
 
 	return 1;
 }
diff --git a/t/t5563-update.sh b/t/t5563-update.sh
index 951df41ac4..833a5285da 100755
--- a/t/t5563-update.sh
+++ b/t/t5563-update.sh
@@ -42,4 +42,19 @@ test_expect_success 'non-fast-forward update' '
 	)
 '
 
+test_expect_success 'git update non-fast-forward with merge' '
+	test_when_finished "rm -rf test" &&
+	(
+	git clone . test &&
+	cd test &&
+	git checkout -b other master^ &&
+	>new &&
+	git add new &&
+	git commit -m new &&
+	git checkout -b test -t other &&
+	git reset --hard master &&
+	git update --merge
+	)
+'
+
 test_done
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 12/35] commit: support for multiple MERGE_MODE

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/commit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 190d215d43..f9dd155566 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1742,7 +1742,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		if (!stat(git_path_merge_mode(the_repository), &statbuf)) {
 			if (strbuf_read_file(&sb, git_path_merge_mode(the_repository), 0) < 0)
 				die_errno(_("could not read MERGE_MODE"));
-			if (!strcmp(sb.buf, "no-ff"))
+			if (strstr(sb.buf, "no-ff"))
 				allow_fast_forward = 0;
 		}
 		if (allow_fast_forward)
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 13/35] merge: add --reverse-parents option

[Felipe Contreras]

It would be useful to make `git update` reverse parents as desired.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/merge-options.txt |  4 ++++
 builtin/commit.c                |  5 +++++
 builtin/merge.c                 | 11 ++++++++++-
 t/t7600-merge.sh                | 26 ++++++++++++++++++++++++++
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index eb0aabd396..8917ed97b2 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -149,6 +149,10 @@ ifndef::git-pull[]
 	Note that not all merge strategies may support progress
 	reporting.
 
+--reverse-parents::
+--no-reverse-parents::
+	Reverse the order of parents in the merge commit.
+
 endif::git-pull[]
 
 --autostash::
diff --git a/builtin/commit.c b/builtin/commit.c
index f9dd155566..931738eeaf 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1724,6 +1724,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		FILE *fp;
 		int allow_fast_forward = 1;
 		struct commit_list **pptr = &parents;
+		int reverse_parents = 0;
 
 		if (!reflog_msg)
 			reflog_msg = "commit (merge)";
@@ -1744,9 +1745,13 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 				die_errno(_("could not read MERGE_MODE"));
 			if (strstr(sb.buf, "no-ff"))
 				allow_fast_forward = 0;
+			if (strstr(sb.buf, "reverse"))
+				reverse_parents = 1;
 		}
 		if (allow_fast_forward)
 			reduce_heads_replace(&parents);
+		if (reverse_parents)
+			parents = reverse_commit_list(parents);
 	} else {
 		if (!reflog_msg)
 			reflog_msg = is_from_cherry_pick(whence)
diff --git a/builtin/merge.c b/builtin/merge.c
index 1836f98f82..b9c6c43d8f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -91,6 +91,7 @@ static int signoff;
 static const char *sign_commit;
 static int autostash;
 static int no_verify;
+static int reverse_parents;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -306,6 +307,8 @@ static struct option builtin_merge_options[] = {
 	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
 	OPT_BOOL(0, "signoff", &signoff, N_("add a Signed-off-by trailer")),
 	OPT_BOOL(0, "no-verify", &no_verify, N_("bypass pre-merge-commit and commit-msg hooks")),
+	OPT_BOOL(0, "reverse-parents", &reverse_parents,
+		N_("reverse the order of parents")),
 	OPT_END()
 };
 
@@ -913,6 +916,8 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
 	pptr = commit_list_append(head, pptr);
 	pptr = commit_list_append(remoteheads->item, pptr);
 	prepare_to_commit(remoteheads);
+	if (reverse_parents)
+		parents = reverse_commit_list(parents);
 	if (commit_tree(merge_msg.buf, merge_msg.len, &result_tree, parents,
 			&result_commit, NULL, sign_commit))
 		die(_("failed to write commit object"));
@@ -937,6 +942,8 @@ static int finish_automerge(struct commit *head,
 	parents = remoteheads;
 	if (!head_subsumed || fast_forward == FF_NO)
 		commit_list_insert(head, &parents);
+	if (reverse_parents)
+		parents = reverse_commit_list(parents);
 	prepare_to_commit(remoteheads);
 	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
 			&result_commit, NULL, sign_commit))
@@ -1049,7 +1056,9 @@ static void write_merge_heads(struct commit_list *remoteheads)
 
 	strbuf_reset(&buf);
 	if (fast_forward == FF_NO)
-		strbuf_addstr(&buf, "no-ff");
+		strbuf_addstr(&buf, "no-ff ");
+	if (reverse_parents)
+		strbuf_addstr(&buf, "reverse ");
 	write_file_buf(git_path_merge_mode(the_repository), buf.buf, buf.len);
 	strbuf_release(&buf);
 }
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 10f8956665..660d13dd51 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -563,6 +563,16 @@ test_expect_success 'merge c0 with c1 (no-ff)' '
 
 test_debug 'git log --graph --decorate --oneline --all'
 
+test_expect_success 'merge c0 with c1 (--reverse-parents)' '
+	git reset --hard c0 &&
+	test_tick &&
+	git merge --no-ff --reverse-parents c1 &&
+	verify_merge file result.1 &&
+	verify_parents $c1 $c0
+'
+
+test_debug 'git log --graph --decorate --oneline --all'
+
 test_expect_success 'merge c0 with c1 (merge.ff=false)' '
 	git reset --hard c0 &&
 	test_config merge.ff "false" &&
@@ -940,6 +950,22 @@ test_expect_success 'merge annotated/signed tag w/ tracking' '
 	)
 '
 
+test_expect_success 'merge --reverse-parents --no-commit && commit' '
+	git reset --hard c0 &&
+	git merge --no-ff --reverse-parents --no-commit c1 &&
+	EDITOR=: git commit &&
+	verify_parents $c1 $c0
+'
+
+test_debug 'git log --graph --decorate --oneline --all'
+
+test_expect_success 'amending reverse merge commit' '
+	EDITOR=: git commit --amend &&
+	verify_parents $c1 $c0
+'
+
+test_debug 'git log --graph --decorate --oneline --all'
+
 test_expect_success GPG 'merge --ff-only tag' '
 	git reset --hard c0 &&
 	git commit --allow-empty -m "A newer commit" &&
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 14/35] update: reverse the order of parents

[Felipe Contreras]

The order of the parents in git pull is wrong, many big threads [1] [2]
have been started because of that.

There's no need for git update to repeat that mistake.

[1] https://lore.kernel.org/git/20130522115042.GA20649@inner.h.apk.li/
[2] https://lore.kernel.org/git/4ay6w9i74cygt6ii1b0db7wg.1398433713382@email.android.com/

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

diff --git a/builtin/update.c b/builtin/update.c
index 5946b383f5..94e83532a8 100644
--- a/builtin/update.c
+++ b/builtin/update.c
@@ -59,7 +59,7 @@ static int run_merge(void)
 	int ret;
 	struct strvec args = STRVEC_INIT;
 
-	strvec_pushl(&args, "merge", "FETCH_HEAD", NULL);
+	strvec_pushl(&args, "merge", "--reverse-parents", "FETCH_HEAD", NULL);
 
 	ret = run_command_v_opt(args.v, RUN_GIT_CMD);
 	strvec_clear(&args);
diff --git a/t/t5563-update.sh b/t/t5563-update.sh
index 833a5285da..72d6a357a1 100755
--- a/t/t5563-update.sh
+++ b/t/t5563-update.sh
@@ -53,7 +53,9 @@ test_expect_success 'git update non-fast-forward with merge' '
 	git commit -m new &&
 	git checkout -b test -t other &&
 	git reset --hard master &&
-	git update --merge
+	git update --merge &&
+	test_cmp_rev @^2 master &&
+	test_cmp_rev @^1 other
 	)
 '
 
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 15/35] update: fake a reverse order of parents in message

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/merge.c   |  1 +
 fmt-merge-msg.c   | 21 +++++++++++++++++++--
 fmt-merge-msg.h   |  3 ++-
 t/t5563-update.sh | 10 +++++++++-
 4 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index b9c6c43d8f..cb476958ad 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1136,6 +1136,7 @@ static void prepare_merge_message(struct strbuf *merge_names, struct strbuf *mer
 	opts.add_title = !have_message;
 	opts.shortlog_len = shortlog_len;
 	opts.credit_people = (0 < option_edit);
+	opts.reverse_parents = reverse_parents;
 
 	fmt_merge_msg(merge_names, merge_msg, &opts);
 	if (merge_msg->len)
diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
index 0f66818e0f..1f840711cc 100644
--- a/fmt-merge-msg.c
+++ b/fmt-merge-msg.c
@@ -432,6 +432,19 @@ static int dest_suppressed(const char *dest_branch)
 	return 0;
 }
 
+static void fmt_update_msg_title(struct strbuf *out, const char *current_branch)
+{
+	struct src_data *src_data;
+	strbuf_addf(out, "Merge branch '%s'", current_branch);
+	src_data = srcs.items[0].util;
+	if (src_data->branch.nr) {
+		const char *branch_name = src_data->branch.items[0].string;
+		if (!dest_suppressed(branch_name))
+			strbuf_addf(out, " into %s", branch_name);
+	}
+	strbuf_addch(out, '\n');
+}
+
 static void fmt_merge_msg_title(struct strbuf *out,
 				const char *current_branch)
 {
@@ -665,8 +678,12 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 			die("error in line %d: %.*s", i, len, p);
 	}
 
-	if (opts->add_title && srcs.nr)
-		fmt_merge_msg_title(out, current_branch);
+	if (opts->add_title && srcs.nr) {
+		if (opts->reverse_parents)
+			fmt_update_msg_title(out, current_branch);
+		else
+			fmt_merge_msg_title(out, current_branch);
+	}
 
 	if (origins.nr)
 		fmt_merge_msg_sigs(out);
diff --git a/fmt-merge-msg.h b/fmt-merge-msg.h
index f2ab0e0085..fbdb1c905e 100644
--- a/fmt-merge-msg.h
+++ b/fmt-merge-msg.h
@@ -7,7 +7,8 @@
 
 struct fmt_merge_msg_opts {
 	unsigned add_title:1,
-		credit_people:1;
+		credit_people:1,
+		reverse_parents:1;
 	int shortlog_len;
 };
 
diff --git a/t/t5563-update.sh b/t/t5563-update.sh
index 72d6a357a1..aabbf5a965 100755
--- a/t/t5563-update.sh
+++ b/t/t5563-update.sh
@@ -7,6 +7,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+check_msg () {
+	test "$2" != "master" && into=" into '$2'"
+	echo "Merge branch '$1'${into}" > expected
+	git log -1 --format=%s > actual &&
+	test_cmp expected actual
+}
+
 test_expect_success 'setup' '
 	echo one > file &&
 	git add file &&
@@ -55,7 +62,8 @@ test_expect_success 'git update non-fast-forward with merge' '
 	git reset --hard master &&
 	git update --merge &&
 	test_cmp_rev @^2 master &&
-	test_cmp_rev @^1 other
+	test_cmp_rev @^1 other &&
+	check_msg test other
 	)
 '
 
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 16/35] update: add --rebase mode

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-update.txt |  4 ++++
 builtin/update.c             | 20 +++++++++++++++++++-
 t/t5563-update.sh            | 17 +++++++++++++++++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-update.txt b/Documentation/git-update.txt
index 075653c6fd..0fefda1379 100644
--- a/Documentation/git-update.txt
+++ b/Documentation/git-update.txt
@@ -33,6 +33,10 @@ OPTIONS
 --merge::
 	Forces a merge.
 
+-r::
+--rebase::
+	Forces a rebase.
+
 SEE ALSO
 --------
 linkgit:git-fetch[1], linkgit:git-fast-forward[1],
diff --git a/builtin/update.c b/builtin/update.c
index 94e83532a8..989104421e 100644
--- a/builtin/update.c
+++ b/builtin/update.c
@@ -9,7 +9,8 @@
 
 enum update_mode_type {
 	UPDATE_MODE_FAST_FORWARD = 0,
-	UPDATE_MODE_MERGE
+	UPDATE_MODE_MERGE,
+	UPDATE_MODE_REBASE
 };
 
 static enum update_mode_type mode = UPDATE_MODE_FAST_FORWARD;
@@ -26,6 +27,9 @@ static struct option update_options[] = {
 	OPT_SET_INT_F('m', "merge", &mode,
 		N_("incorporate changes by merging"),
 		UPDATE_MODE_MERGE, PARSE_OPT_NONEG),
+	OPT_SET_INT_F('r', "rebase", &mode,
+		N_("incorporate changes by rebasing"),
+		UPDATE_MODE_REBASE, PARSE_OPT_NONEG),
 
 	OPT_END()
 };
@@ -66,6 +70,18 @@ static int run_merge(void)
 	return ret;
 }
 
+static int run_rebase(void)
+{
+	int ret;
+	struct strvec args = STRVEC_INIT;
+
+	strvec_pushl(&args, "rebase", "FETCH_HEAD", NULL);
+
+	ret = run_command_v_opt(args.v, RUN_GIT_CMD);
+	strvec_clear(&args);
+	return ret;
+}
+
 int cmd_update(int argc, const char **argv, const char *prefix)
 {
 	if (!getenv("GIT_REFLOG_ACTION"))
@@ -86,6 +102,8 @@ int cmd_update(int argc, const char **argv, const char *prefix)
 		return run_fast_forward();
 	if (mode == UPDATE_MODE_MERGE)
 		return run_merge();
+	if (mode == UPDATE_MODE_REBASE)
+		return run_rebase();
 
 	return 1;
 }
diff --git a/t/t5563-update.sh b/t/t5563-update.sh
index aabbf5a965..73e67f9783 100755
--- a/t/t5563-update.sh
+++ b/t/t5563-update.sh
@@ -67,4 +67,21 @@ test_expect_success 'git update non-fast-forward with merge' '
 	)
 '
 
+test_expect_success 'git update non-fast-forward with rebase' '
+	test_when_finished "rm -rf test" &&
+	(
+	git clone . test &&
+	cd test &&
+	git checkout -b other master^ &&
+	>new &&
+	git add new &&
+	git commit -m new &&
+	git checkout -b test -t other &&
+	git reset --hard master &&
+	git update --rebase &&
+	test_cmp_rev @^ other &&
+	test_must_fail git rev-parse --verify -q @^2
+	)
+'
+
 test_done
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 17/35] update: add mode configuation

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/config.txt        |  2 ++
 Documentation/config/update.txt |  4 ++++
 builtin/update.c                | 26 ++++++++++++++++++++++++++
 3 files changed, 32 insertions(+)
 create mode 100644 Documentation/config/update.txt

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fc4b49c0d4..998339e6fd 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -466,6 +466,8 @@ include::config/trace2.txt[]
 
 include::config/transfer.txt[]
 
+include::config/update.txt[]
+
 include::config/uploadarchive.txt[]
 
 include::config/uploadpack.txt[]
diff --git a/Documentation/config/update.txt b/Documentation/config/update.txt
new file mode 100644
index 0000000000..42a7fe1614
--- /dev/null
+++ b/Documentation/config/update.txt
@@ -0,0 +1,4 @@
+update.mode::
+	When `git update` is run, this determines the mode of operation,
+	possible values are 'fast-forward', 'merge', and 'rebase'. The default
+	is 'fast-forward'.
diff --git a/builtin/update.c b/builtin/update.c
index 989104421e..4a1b82a41d 100644
--- a/builtin/update.c
+++ b/builtin/update.c
@@ -5,9 +5,11 @@
 #include "builtin.h"
 #include "parse-options.h"
 #include "run-command.h"
+#include "config.h"
 #include "dir.h"
 
 enum update_mode_type {
+	UPDATE_MODE_INVALID = -1,
 	UPDATE_MODE_FAST_FORWARD = 0,
 	UPDATE_MODE_MERGE,
 	UPDATE_MODE_REBASE
@@ -34,6 +36,28 @@ static struct option update_options[] = {
 	OPT_END()
 };
 
+static enum update_mode_type update_mode_parse_value(const char *value)
+{
+	if (!strcmp(value, "fast-forward"))
+		return UPDATE_MODE_FAST_FORWARD;
+	if (!strcmp(value, "merge"))
+		return UPDATE_MODE_MERGE;
+	if (!strcmp(value, "rebase"))
+		return UPDATE_MODE_REBASE;
+
+	return UPDATE_MODE_INVALID;
+}
+
+static int git_update_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "update.mode")) {
+		mode = update_mode_parse_value(value);
+		return 0;
+	}
+
+	return git_default_config(var, value, cb);
+}
+
 static int run_fetch(void)
 {
 	struct strvec args = STRVEC_INIT;
@@ -87,6 +111,8 @@ int cmd_update(int argc, const char **argv, const char *prefix)
 	if (!getenv("GIT_REFLOG_ACTION"))
 		setenv("GIT_REFLOG_ACTION", "update", 0);
 
+	git_config(git_update_config, NULL);
+
 	argc = parse_options(argc, argv, prefix, update_options, update_usage, 0);
 
 	if (repo_read_index_unmerged(the_repository))
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 18/35] update: add per-branch mode configuration

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/config/branch.txt |  5 +++++
 Documentation/config/update.txt |  3 ++-
 builtin/update.c                | 18 ++++++++++++++++++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt
index cc5f3249fc..ba355f23e0 100644
--- a/Documentation/config/branch.txt
+++ b/Documentation/config/branch.txt
@@ -96,6 +96,11 @@ mode.
 it unless you understand the implications (see linkgit:git-rebase[1]
 for details).
 
+branch.<name>.updateMode::
+	When `git update` is run, this determines the mode of operation,
+	possible values are 'fast-forward', 'merge', and 'rebase'.
+	See `update.mode` for doing this in a non branch-specific manner.
+
 branch.<name>.description::
 	Branch description, can be edited with
 	`git branch --edit-description`. Branch description is
diff --git a/Documentation/config/update.txt b/Documentation/config/update.txt
index 42a7fe1614..e6db45b6c3 100644
--- a/Documentation/config/update.txt
+++ b/Documentation/config/update.txt
@@ -1,4 +1,5 @@
 update.mode::
 	When `git update` is run, this determines the mode of operation,
 	possible values are 'fast-forward', 'merge', and 'rebase'. The default
-	is 'fast-forward'.
+	is 'fast-forward'. See "branch.<name>.updateMode" for setting this on a
+	per-branch basis.
diff --git a/builtin/update.c b/builtin/update.c
index 4a1b82a41d..50905f5806 100644
--- a/builtin/update.c
+++ b/builtin/update.c
@@ -7,6 +7,7 @@
 #include "run-command.h"
 #include "config.h"
 #include "dir.h"
+#include "remote.h"
 
 enum update_mode_type {
 	UPDATE_MODE_INVALID = -1,
@@ -58,6 +59,22 @@ static int git_update_config(const char *var, const char *value, void *cb)
 	return git_default_config(var, value, cb);
 }
 
+static void get_branch_config(void)
+{
+	struct branch *cur_branch = branch_get("HEAD");
+	char *key;
+	const char *value;
+
+	if (!cur_branch)
+		return;
+
+	key = xstrfmt("branch.%s.updatemode", cur_branch->name);
+	if (!git_config_get_value(key, &value))
+		mode = update_mode_parse_value(value);
+
+	free(key);
+}
+
 static int run_fetch(void)
 {
 	struct strvec args = STRVEC_INIT;
@@ -112,6 +129,7 @@ int cmd_update(int argc, const char **argv, const char *prefix)
 		setenv("GIT_REFLOG_ACTION", "update", 0);
 
 	git_config(git_update_config, NULL);
+	get_branch_config();
 
 	argc = parse_options(argc, argv, prefix, update_options, update_usage, 0);
 
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 19/35] 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.

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 e8927fc2ff..a22293b7db 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -947,7 +947,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;
 
@@ -982,8 +981,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;
 
@@ -1065,13 +1064,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.32.0.36.g70aac2b1aa

[RFC PATCH 20/35] 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 a22293b7db..80e2f55cbc 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1053,7 +1053,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;
@@ -1068,11 +1067,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.32.0.36.g70aac2b1aa

[RFC PATCH 21/35] 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 80e2f55cbc..3e13f81084 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.32.0.36.g70aac2b1aa

[RFC PATCH 22/35] 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   | 7 +++++--
 builtin/pull.c               | 4 +++-
 t/t7601-merge-pull-config.sh | 8 ++++----
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 5c3fb67c01..d23128fa72 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -131,8 +131,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 3e13f81084..0d76b54186 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),
@@ -936,7 +938,7 @@ static void show_advice_pull_non_ff(void)
 		 "  git config pull.ff only       # fast-forward only\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"
+		 "preference for all repositories. You can also pass --rebase, --merge,\n"
 		 "or --ff-only on the command line to override the configured default per\n"
 		 "invocation.\n"));
 }
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.32.0.36.g70aac2b1aa

[RFC PATCH 23/35] 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 0d76b54186..94fc8b0739 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."));
@@ -949,7 +947,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"))
@@ -971,8 +968,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");
@@ -983,7 +980,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;
@@ -1043,17 +1040,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.32.0.36.g70aac2b1aa

[RFC PATCH 24/35] pull: move configuration fetches

[Felipe Contreras]

Now that we have REBASE_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 REBASE_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 94fc8b0739..61af8ea2cb 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -949,6 +949,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);
 
@@ -965,12 +968,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.32.0.36.g70aac2b1aa

[RFC PATCH 25/35] 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.

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 61af8ea2cb..2c2f0822d5 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1042,7 +1042,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.32.0.36.g70aac2b1aa

[RFC PATCH 26/35] 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.

To avoid some of the problems of git pull Linus Torvalds suggested a
configuration pull.merge [1], however, instead of having pull.merge and
pull.rebase, we can have pull.mode which works for both.

Additionally this would allow us to have a saner per-branch
configuration: branch.<name>.pullMode.

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.

[1] https://lore.kernel.org/git/CA+55aFz2Uvq4vmyjJPao5tS-uuVvKm6mbP7Uz8sdq1VMxMGJCw@mail.gmail.com/

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/config/pull.txt |  5 +++
 builtin/pull.c                | 68 ++++++++++++++++++++++++++++++++---
 rebase.c                      | 10 ++++++
 rebase.h                      |  9 +++++
 t/t5520-pull.sh               | 42 ++++++++++++++++++++++
 t/t7601-merge-pull-config.sh  | 14 ++++++++
 6 files changed, 143 insertions(+), 5 deletions(-)

diff --git a/Documentation/config/pull.txt b/Documentation/config/pull.txt
index 5404830609..646431a02d 100644
--- a/Documentation/config/pull.txt
+++ b/Documentation/config/pull.txt
@@ -29,6 +29,11 @@ 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'.
+
 pull.octopus::
 	The default merge strategy to use when pulling multiple branches
 	at once.
diff --git a/builtin/pull.c b/builtin/pull.c
index 2c2f0822d5..124575c32c 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,16 @@ 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;
+
+	if (!git_config_get_value("pull.mode", &value))
+		return parse_config_pull_mode("pull.mode", value);
+
+	return PULL_MODE_DEFAULT;
+}
+
 /**
  * Read config variables.
  */
@@ -931,9 +964,9 @@ static void show_advice_pull_non_ff(void)
 		 "discouraged. You can squelch this message by running one of the following\n"
 		 "commands sometime before your next pull:\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 config pull.mode merge  # the default strategy\n"
+		 "  git config pull.mode rebase\n"
+		 "  git config pull.ff only     # fast-forward only\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, --merge,\n"
@@ -968,6 +1001,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 e2c0c51022..663c15fcd7 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -452,6 +452,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
@@ -526,6 +536,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 &&
@@ -555,6 +576,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 &&
@@ -596,6 +627,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.32.0.36.g70aac2b1aa

[RFC PATCH 27/35] pull: add per-branch mode configuration

[Felipe Contreras]

Many people have argued that `git pull` should have a per-branch
configuration for its mode of operation (e.g. Linus Torvalds [1] and
Theodore Ts'o [2]).

branch.<name>.pullMode achieves that.

[1] https://lore.kernel.org/git/CA+55aFz2Uvq4vmyjJPao5tS-uuVvKm6mbP7Uz8sdq1VMxMGJCw@mail.gmail.com/
[2] https://lore.kernel.org/git/20130312212027.GE14792@thunk.org/

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/config/branch.txt |  5 +++++
 Documentation/config/pull.txt   |  3 ++-
 builtin/pull.c                  | 13 +++++++++++++
 t/t5520-pull.sh                 | 11 +++++++++++
 4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt
index ba355f23e0..5f412caf62 100644
--- a/Documentation/config/branch.txt
+++ b/Documentation/config/branch.txt
@@ -101,6 +101,11 @@ branch.<name>.updateMode::
 	possible values are 'fast-forward', 'merge', and 'rebase'.
 	See `update.mode` for doing this in a non branch-specific manner.
 
+branch.<name>.pullMode::
+	When `git pull` is run, this determines the mode of operation,
+	possible values are 'merge' and 'rebase'. See `pull.mode` for doing this
+	in a non branch-specific manner.
+
 branch.<name>.description::
 	Branch description, can be edited with
 	`git branch --edit-description`. Branch description is
diff --git a/Documentation/config/pull.txt b/Documentation/config/pull.txt
index 646431a02d..3fb9bfdfea 100644
--- a/Documentation/config/pull.txt
+++ b/Documentation/config/pull.txt
@@ -32,7 +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'.
+	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
diff --git a/builtin/pull.c b/builtin/pull.c
index 124575c32c..bb3c0b55f9 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -374,8 +374,21 @@ static enum rebase_type config_get_rebase(void)
 
 static enum pull_mode_type config_get_pull_mode(const char *repo)
 {
+	struct branch *curr_branch = branch_get("HEAD");
 	const char *value;
 
+	if (curr_branch) {
+		char *key = xstrfmt("branch.%s.pullmode", curr_branch->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);
 
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 663c15fcd7..59894dd15a 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -493,6 +493,17 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
 	test_cmp expect actual
 '
 
+test_expect_success 'branch..pullmode overrides pull.mode' '
+	git reset --hard before-rebase &&
+	test_config pull.mode rebase &&
+	test_config branch.to-rebase.pullmode merge &&
+	git pull . 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 &&
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 28/35] 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/742df4c2-2bc5-8a4b-8de1-cd5e48718398@redhat.com/
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/git/4ay6w9i74cygt6ii1b0db7wg.1398433713382@email.android.com

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/config/branch.txt |  4 +--
 Documentation/config/pull.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/branch.txt b/Documentation/config/branch.txt
index 5f412caf62..a61c80c90b 100644
--- a/Documentation/config/branch.txt
+++ b/Documentation/config/branch.txt
@@ -103,8 +103,8 @@ branch.<name>.updateMode::
 
 branch.<name>.pullMode::
 	When `git pull` is run, this determines the mode of operation,
-	possible values are 'merge' and 'rebase'. See `pull.mode` for doing this
-	in a non branch-specific manner.
+	possible values are 'fast-forward', 'merge', and 'rebase'.
+	See `pull.mode` for doing this in a non branch-specific manner.
 
 branch.<name>.description::
 	Branch description, can be edited with
diff --git a/Documentation/config/pull.txt b/Documentation/config/pull.txt
index 3fb9bfdfea..7b3dddf4d4 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/builtin/pull.c b/builtin/pull.c
index bb3c0b55f9..295c13fbe8 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -979,7 +979,7 @@ static void show_advice_pull_non_ff(void)
 		 "\n"
 		 "  git config pull.mode merge  # the default strategy\n"
 		 "  git config pull.mode rebase\n"
-		 "  git config pull.ff only     # fast-forward only\n"
+		 "  git config pull.mode fast-forward\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, --merge,\n"
@@ -1023,6 +1023,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:
@@ -1113,6 +1114,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, 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 59894dd15a..7ea558651d 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -869,4 +869,48 @@ test_expect_success 'git pull --rebase against local branch' '
 	test_cmp expect file2
 '
 
+setup_other () {
+	test_when_finished "git checkout main && 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 main
+}
+
+setup_ff () {
+	setup_other main
+}
+
+setup_non_ff () {
+	setup_other main^
+}
+
+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.32.0.36.g70aac2b1aa

[RFC PATCH 29/35] 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 295c13fbe8..cbc102ee45 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1114,12 +1114,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, 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, either merge or rebase.\n"));
 	}
 
 	if (opt_rebase >= REBASE_TRUE) {
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 30/35] pull: add diverging advice on fast-forward mode

[Felipe Contreras]

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

diff --git a/builtin/pull.c b/builtin/pull.c
index cbc102ee45..f4f822700a 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1118,8 +1118,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 		if (!mode && opt_verbosity >= 0)
 			show_advice_pull_non_ff();
 
-		if (mode == PULL_MODE_FAST_FORWARD)
+		if (mode == PULL_MODE_FAST_FORWARD) {
+			diverging_advice();
 			die(_("The pull was not fast-forward, either merge or rebase.\n"));
+		}
 	}
 
 	if (opt_rebase >= REBASE_TRUE) {
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 31/35] 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 f4f822700a..e304b22bd8 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 7ea558651d..b3b70d145f 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -597,6 +597,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.32.0.36.g70aac2b1aa

[RFC PATCH 32/35] pull: improve default warning

[Felipe Contreras]

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, it's better use --global in the configuration commands like we
did with push.default.

And finally, point to the documentation that explains what is a
non-fast-forward, and how to solve it:

  git help fast-forward

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

diff --git a/builtin/pull.c b/builtin/pull.c
index e304b22bd8..27ce2f2183 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -979,18 +979,19 @@ 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.mode merge  # the default strategy\n"
-		 "  git config pull.mode rebase\n"
-		 "  git config pull.mode fast-forward\n"
+		 "  git pull --merge # the default\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, --merge,\n"
-		 "or --ff-only on the command line to override the configured default per\n"
-		 "invocation.\n"));
+		 "You can quell this message by running one of the following commands:\n"
+		 "\n"
+		 "  git config --global pull.mode merge\n"
+		 "  git config --global pull.mode rebase\n"
+		 "  git config --global pull.mode fast-forward\n"
+		 "\n"
+		 "For more information check \"git help fast-forward\"."));
 }
 
 int cmd_pull(int argc, const char **argv, const char *prefix)
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 33/35] pull: advice of future changes

[Felipe Contreras]

Now that we have `pull.mode=fast-forward`, we can make it the default any
time we want to.

For now, simply explain the upcoming changes in the default warning, and
mention how to turn on the future default mode.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/pull.c               | 28 +++++++++++++------------
 t/t5520-pull.sh              |  8 ++++++++
 t/t7601-merge-pull-config.sh | 40 ++++++++++++++++++------------------
 3 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 27ce2f2183..4771c953d2 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -979,19 +979,21 @@ 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 discouraged;\n"
-		 "you need to specify if you want a merge, or a rebase.\n"
-		 "\n"
-		 "  git pull --merge # the default\n"
-		 "  git pull --rebase\n"
-		 "\n"
-		 "You can quell this message by running one of the following commands:\n"
-		 "\n"
-		 "  git config --global pull.mode merge\n"
-		 "  git config --global pull.mode rebase\n"
-		 "  git config --global pull.mode fast-forward\n"
-		 "\n"
-		 "For more information check \"git help fast-forward\"."));
+	advise(_("The pull was not a fast-forward, in the future you will have to choose\n"
+		"between a merge or a rebase.\n"
+		"\n"
+		"To quell this message you have two main options:\n"
+		"\n"
+		"1. Adopt the new behavior:\n"
+		"\n"
+		"  git config --global pull.mode fast-forward\n"
+		"\n"
+		"2. Maintain the current behavior:\n"
+		"\n"
+		"  git config --global pull.mode merge\n"
+		"\n"
+		"For now we will fall back to the traditional behavior: merge.\n"
+		"For more information check \"git help fast-forward\"."));
 }
 
 int cmd_pull(int argc, const char **argv, const char *prefix)
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index b3b70d145f..bc1c601b8b 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -923,4 +923,12 @@ test_expect_success 'non-fast-forward error message (pull.mode=fast-forward)' '
 	test_i18ngrep "The pull was not fast-forward" error
 '
 
+test_expect_success 'non-fast-forward warning (default)' '
+	setup_non_ff &&
+	git pull 2> error &&
+	cat error &&
+	test_i18ngrep "The pull was not a fast-forward" error &&
+	test_i18ngrep "in the future you will have to choose" error
+'
+
 test_done
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 47fd2e2d05..16d1e1ba47 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -30,65 +30,65 @@ test_expect_success 'setup' '
 test_expect_success 'pull.rebase not set' '
 	git reset --hard c0 &&
 	git pull . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep ! "in the future you will have to choose" 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_i18ngrep ! "in the future you will have to choose" err
 '
 
 test_expect_success 'pull.rebase not set and pull.ff=true' '
 	git reset --hard c0 &&
 	test_config pull.ff true &&
 	git pull . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep ! "in the future you will have to choose" err
 '
 
 test_expect_success 'pull.rebase not set and pull.ff=false' '
 	git reset --hard c0 &&
 	test_config pull.ff false &&
 	git pull . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep ! "in the future you will have to choose" err
 '
 
 test_expect_success 'pull.rebase not set and pull.ff=only' '
 	git reset --hard c0 &&
 	test_config pull.ff only &&
 	git pull . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep ! "in the future you will have to choose" err
 '
 
 test_expect_success 'pull.rebase not set and --rebase given' '
 	git reset --hard c0 &&
 	git pull --rebase . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep ! "in the future you will have to choose" err
 '
 
 test_expect_success 'pull.rebase not set and --merge given' '
 	git reset --hard c0 &&
 	git pull --merge . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep ! "in the future you will have to choose" err
 '
 
 test_expect_success 'pull.rebase not set and --ff given' '
 	git reset --hard c0 &&
 	git pull --ff . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep ! "in the future you will have to choose" err
 '
 
 test_expect_success 'pull.rebase not set and --no-ff given' '
 	git reset --hard c0 &&
 	git pull --no-ff . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep ! "in the future you will have to choose" err
 '
 
 test_expect_success 'pull.rebase not set and --ff-only given' '
 	git reset --hard c0 &&
 	git pull --ff-only . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep ! "in the future you will have to choose" err
 '
 
 test_expect_success 'pull.rebase not set (not-fast-forward)' '
@@ -96,65 +96,65 @@ test_expect_success 'pull.rebase not set (not-fast-forward)' '
 	git -c color.advice=always pull . c1 2>err &&
 	test_decode_color <err >decoded &&
 	test_i18ngrep "<YELLOW>hint: " decoded &&
-	test_i18ngrep "Pulling without specifying how to reconcile" decoded
+	test_i18ngrep "in the future you will have to choose" 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_i18ngrep ! "in the future you will have to choose" err
 '
 
 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 "in the future you will have to choose" 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 "in the future you will have to choose" 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 "in the future you will have to choose" err
 '
 
 test_expect_success 'pull.rebase not set and --rebase given (not-fast-forward)' '
 	git reset --hard c2 &&
 	git pull --rebase . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep ! "in the future you will have to choose" err
 '
 
 test_expect_success 'pull.rebase not set and --merge given (not-fast-forward)' '
 	git reset --hard c2 &&
 	git pull --merge . c1 2>err &&
-	test_i18ngrep ! "Pulling without specifying how to reconcile" err
+	test_i18ngrep ! "in the future you will have to choose" err
 '
 
 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 "in the future you will have to choose" 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 "in the future you will have to choose" 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 "in the future you will have to choose" err
 '
 
 test_expect_success 'merge c1 with c2' '
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 34/35] FUTURE: pull: enable ff-only mode by default

[Felipe Contreras]

The user has been warned that this change was coming and should have
already configured his/her preference.

It's time to flip the switch and make ff-only the default.

There's no need for the annoying warning anymore.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-pull.txt   |   3 +
 builtin/pull.c               |  30 +-------
 t/t5520-pull.sh              |  28 +++-----
 t/t7601-merge-pull-config.sh | 130 -----------------------------------
 4 files changed, 16 insertions(+), 175 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index d23128fa72..c2768fadb1 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -35,6 +35,9 @@ Default values for <repository> and <branch> are read from the
 "remote" and "merge" configuration for the current branch
 as set by linkgit:git-branch[1] `--track`.
 
+By default non-fast-forward merges fail, so you need to specify if you want to
+do a merge or a rebase.
+
 Assume the following history exists and the current branch is
 "`master`":
 
diff --git a/builtin/pull.c b/builtin/pull.c
index 4771c953d2..24c72e33c6 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -977,25 +977,6 @@ static int get_can_ff(struct object_id *orig_head, struct object_id *orig_merge_
 	return ret;
 }
 
-static void show_advice_pull_non_ff(void)
-{
-	advise(_("The pull was not a fast-forward, in the future you will have to choose\n"
-		"between a merge or a rebase.\n"
-		"\n"
-		"To quell this message you have two main options:\n"
-		"\n"
-		"1. Adopt the new behavior:\n"
-		"\n"
-		"  git config --global pull.mode fast-forward\n"
-		"\n"
-		"2. Maintain the current behavior:\n"
-		"\n"
-		"  git config --global pull.mode merge\n"
-		"\n"
-		"For now we will fall back to the traditional behavior: merge.\n"
-		"For more information check \"git help fast-forward\"."));
-}
-
 int cmd_pull(int argc, const char **argv, const char *prefix)
 {
 	const char *repo, **refspecs;
@@ -1123,14 +1104,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 	can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);
 
-	if (!can_ff) {
-		if (!mode && opt_verbosity >= 0)
-			show_advice_pull_non_ff();
-
-		if (mode == PULL_MODE_FAST_FORWARD) {
-			diverging_advice();
-			die(_("The pull was not fast-forward, either merge or rebase.\n"));
-		}
+	if ((!mode || mode == PULL_MODE_FAST_FORWARD) && !can_ff) {
+		diverging_advice();
+		die(_("The pull was not fast-forward, either merge or rebase.\n"));
 	}
 
 	if (opt_rebase >= REBASE_TRUE) {
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index bc1c601b8b..dea3ea8c37 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -897,38 +897,30 @@ setup_non_ff () {
 	setup_other main^
 }
 
-test_expect_success 'fast-forward (pull.mode=fast-forward)' '
+test_expect_success 'fast-forward (default)' '
 	setup_ff &&
-	git -c pull.mode=fast-forward pull
+	git pull
 '
 
-test_expect_success 'non-fast-forward (pull.mode=fast-forward)' '
+test_expect_success 'non-fast-forward (default)' '
 	setup_non_ff &&
-	test_must_fail git -c pull.mode=fast-forward pull
+	test_must_fail git pull
 '
 
-test_expect_success 'non-fast-forward with merge (pull.mode=fast-forward)' '
+test_expect_success 'non-fast-forward with merge (default)' '
 	setup_non_ff &&
-	git -c pull.mode=fast-forward pull --merge
+	git pull --merge
 '
 
-test_expect_success 'non-fast-forward with rebase (pull.mode=fast-forward)' '
+test_expect_success 'non-fast-forward with rebase (default)' '
 	setup_non_ff &&
-	git -c pull.mode=fast-forward pull --rebase
+	git pull --rebase
 '
 
-test_expect_success 'non-fast-forward error message (pull.mode=fast-forward)' '
+test_expect_success 'non-fast-forward error message (default)' '
 	setup_non_ff &&
-	test_must_fail git -c pull.mode=fast-forward pull 2> error &&
+	test_must_fail git pull 2> error &&
 	test_i18ngrep "The pull was not fast-forward" error
 '
 
-test_expect_success 'non-fast-forward warning (default)' '
-	setup_non_ff &&
-	git pull 2> error &&
-	cat error &&
-	test_i18ngrep "The pull was not a fast-forward" error &&
-	test_i18ngrep "in the future you will have to choose" error
-'
-
 test_done
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 16d1e1ba47..c6c44ec570 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -27,136 +27,6 @@ test_expect_success 'setup' '
 	git tag c3
 '
 
-test_expect_success 'pull.rebase not set' '
-	git reset --hard c0 &&
-	git pull . c1 2>err &&
-	test_i18ngrep ! "in the future you will have to choose" err
-'
-
-test_expect_success 'pull.mode set' '
-	git reset --hard c0 &&
-	test_config pull.mode merge &&
-	git pull . c1 2>err &&
-	test_i18ngrep ! "in the future you will have to choose" err
-'
-
-test_expect_success 'pull.rebase not set and pull.ff=true' '
-	git reset --hard c0 &&
-	test_config pull.ff true &&
-	git pull . c1 2>err &&
-	test_i18ngrep ! "in the future you will have to choose" err
-'
-
-test_expect_success 'pull.rebase not set and pull.ff=false' '
-	git reset --hard c0 &&
-	test_config pull.ff false &&
-	git pull . c1 2>err &&
-	test_i18ngrep ! "in the future you will have to choose" err
-'
-
-test_expect_success 'pull.rebase not set and pull.ff=only' '
-	git reset --hard c0 &&
-	test_config pull.ff only &&
-	git pull . c1 2>err &&
-	test_i18ngrep ! "in the future you will have to choose" err
-'
-
-test_expect_success 'pull.rebase not set and --rebase given' '
-	git reset --hard c0 &&
-	git pull --rebase . c1 2>err &&
-	test_i18ngrep ! "in the future you will have to choose" err
-'
-
-test_expect_success 'pull.rebase not set and --merge given' '
-	git reset --hard c0 &&
-	git pull --merge . c1 2>err &&
-	test_i18ngrep ! "in the future you will have to choose" err
-'
-
-test_expect_success 'pull.rebase not set and --ff given' '
-	git reset --hard c0 &&
-	git pull --ff . c1 2>err &&
-	test_i18ngrep ! "in the future you will have to choose" err
-'
-
-test_expect_success 'pull.rebase not set and --no-ff given' '
-	git reset --hard c0 &&
-	git pull --no-ff . c1 2>err &&
-	test_i18ngrep ! "in the future you will have to choose" err
-'
-
-test_expect_success 'pull.rebase not set and --ff-only given' '
-	git reset --hard c0 &&
-	git pull --ff-only . c1 2>err &&
-	test_i18ngrep ! "in the future you will have to choose" err
-'
-
-test_expect_success 'pull.rebase not set (not-fast-forward)' '
-	git reset --hard c2 &&
-	git -c color.advice=always pull . c1 2>err &&
-	test_decode_color <err >decoded &&
-	test_i18ngrep "<YELLOW>hint: " decoded &&
-	test_i18ngrep "in the future you will have to choose" decoded
-'
-
-test_expect_success 'pull.mode set' '
-	git reset --hard c2 &&
-	test_config pull.mode merge &&
-	git pull . c1 2>err &&
-	test_i18ngrep ! "in the future you will have to choose" err
-'
-
-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 "in the future you will have to choose" 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 "in the future you will have to choose" 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 "in the future you will have to choose" err
-'
-
-test_expect_success 'pull.rebase not set and --rebase given (not-fast-forward)' '
-	git reset --hard c2 &&
-	git pull --rebase . c1 2>err &&
-	test_i18ngrep ! "in the future you will have to choose" err
-'
-
-test_expect_success 'pull.rebase not set and --merge given (not-fast-forward)' '
-	git reset --hard c2 &&
-	git pull --merge . c1 2>err &&
-	test_i18ngrep ! "in the future you will have to choose" err
-'
-
-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 "in the future you will have to choose" 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 "in the future you will have to choose" 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 "in the future you will have to choose" err
-'
-
 test_expect_success 'merge c1 with c2' '
 	git reset --hard c1 &&
 	test -f c0.c &&
-- 
2.32.0.36.g70aac2b1aa

[RFC PATCH 35/35] !fixup FUTURE: pull: enable ff-only mode by default

[Felipe Contreras]

Fix all the tests.

TODO: some of these should probably not even use `git pull .` but
`git merge` instead.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t4013-diff-various.sh       |  2 +-
 t/t5520-pull.sh               | 16 ++++++++--------
 t/t5521-pull-options.sh       |  4 ++--
 t/t5524-pull-msg.sh           |  4 ++--
 t/t5553-set-upstream.sh       | 14 +++++++-------
 t/t5604-clone-reference.sh    |  4 ++--
 t/t6402-merge-rename.sh       | 16 ++++++++--------
 t/t6409-merge-subtree.sh      |  6 +++---
 t/t6417-merge-ours-theirs.sh  | 10 +++++-----
 t/t7603-merge-reduce-heads.sh |  2 +-
 10 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 7fadc985cc..48ae585e1a 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -65,7 +65,7 @@ test_expect_success setup '
 	export GIT_AUTHOR_DATE GIT_COMMITTER_DATE &&
 
 	git checkout master &&
-	git pull -s ours . side &&
+	git pull --merge -s ours . side &&
 
 	GIT_AUTHOR_DATE="2006-06-26 00:05:00 +0000" &&
 	GIT_COMMITTER_DATE="2006-06-26 00:05:00 +0000" &&
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index dea3ea8c37..ad4fe1998e 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -226,7 +226,7 @@ test_expect_success 'fail if the index has unresolved entries' '
 	test_commit modified2 file &&
 	git ls-files -u >unmerged &&
 	test_must_be_empty unmerged &&
-	test_must_fail git pull . second &&
+	test_must_fail git pull --merge . second &&
 	git ls-files -u >unmerged &&
 	test_file_not_empty unmerged &&
 	cp file expected &&
@@ -409,37 +409,37 @@ test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '
 
 test_expect_success 'pull succeeds with dirty working directory and merge.autostash set' '
 	test_config merge.autostash true &&
-	test_pull_autostash 2
+	test_pull_autostash 2 --merge
 '
 
 test_expect_success 'pull --autostash & merge.autostash=true' '
 	test_config merge.autostash true &&
-	test_pull_autostash 2 --autostash
+	test_pull_autostash 2 --merge --autostash
 '
 
 test_expect_success 'pull --autostash & merge.autostash=false' '
 	test_config merge.autostash false &&
-	test_pull_autostash 2 --autostash
+	test_pull_autostash 2 --merge --autostash
 '
 
 test_expect_success 'pull --autostash & merge.autostash unset' '
 	test_unconfig merge.autostash &&
-	test_pull_autostash 2 --autostash
+	test_pull_autostash 2 --merge --autostash
 '
 
 test_expect_success 'pull --no-autostash & merge.autostash=true' '
 	test_config merge.autostash true &&
-	test_pull_autostash_fail --no-autostash
+	test_pull_autostash_fail --merge --no-autostash
 '
 
 test_expect_success 'pull --no-autostash & merge.autostash=false' '
 	test_config merge.autostash false &&
-	test_pull_autostash_fail --no-autostash
+	test_pull_autostash_fail --merge --no-autostash
 '
 
 test_expect_success 'pull --no-autostash & merge.autostash unset' '
 	test_unconfig merge.autostash &&
-	test_pull_autostash_fail --no-autostash
+	test_pull_autostash_fail --merge --no-autostash
 '
 
 test_expect_success 'pull.rebase' '
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 63a688bdbf..361800a6f4 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -113,7 +113,7 @@ test_expect_success 'git pull --force' '
 	git pull two &&
 	test_commit A &&
 	git branch -f origin &&
-	git pull --all --force
+	git pull --all --force --merge
 	)
 '
 
@@ -179,7 +179,7 @@ test_expect_success 'git pull --allow-unrelated-histories' '
 	(
 		cd dst &&
 		test_must_fail git pull ../src side &&
-		git pull --allow-unrelated-histories ../src side
+		git pull --allow-unrelated-histories --merge ../src side
 	)
 '
 
diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh
index c278adaa5a..2f8a20de18 100755
--- a/t/t5524-pull-msg.sh
+++ b/t/t5524-pull-msg.sh
@@ -28,7 +28,7 @@ test_expect_success setup '
 test_expect_success pull '
 (
 	cd cloned &&
-	git pull --log &&
+	git pull --log --merge &&
 	git log -2 &&
 	git cat-file commit HEAD >result &&
 	grep Dollar result
@@ -41,7 +41,7 @@ test_expect_success '--log=1 limits shortlog length' '
 	git reset --hard HEAD^ &&
 	test "$(cat afile)" = original &&
 	test "$(cat bfile)" = added &&
-	git pull --log=1 &&
+	git pull --log=1 --merge &&
 	git log -3 &&
 	git cat-file commit HEAD >result &&
 	grep Dollar result &&
diff --git a/t/t5553-set-upstream.sh b/t/t5553-set-upstream.sh
index b1d614ce18..dabbea76d3 100755
--- a/t/t5553-set-upstream.sh
+++ b/t/t5553-set-upstream.sh
@@ -108,27 +108,27 @@ test_expect_success 'setup commit on main and other pull' '
 
 test_expect_success 'pull --set-upstream upstream main sets branch main but not other' '
 	clear_config main other &&
-	git pull --set-upstream upstream main &&
+	git pull --merge --set-upstream upstream main &&
 	check_config main upstream refs/heads/main &&
 	check_config_missing other
 '
 
 test_expect_success 'pull --set-upstream main:other2 does not set the branch other2' '
 	clear_config other2 &&
-	git pull --set-upstream upstream main:other2 &&
+	git pull --merge --set-upstream upstream main:other2 &&
 	check_config_missing other2
 '
 
 test_expect_success 'pull --set-upstream upstream other sets branch main' '
 	clear_config main other &&
-	git pull --set-upstream upstream other &&
+	git pull --merge --set-upstream upstream other &&
 	check_config main upstream refs/heads/other &&
 	check_config_missing other
 '
 
 test_expect_success 'pull --set-upstream upstream tag does not set the tag' '
 	clear_config three &&
-	git pull --tags --set-upstream upstream three &&
+	git pull --merge --tags --set-upstream upstream three &&
 	check_config_missing three
 '
 
@@ -144,16 +144,16 @@ test_expect_success 'pull --set-upstream http://nosuchdomain.example.com fails w
 
 test_expect_success 'pull --set-upstream upstream HEAD sets branch HEAD' '
 	clear_config main other &&
-	git pull --set-upstream upstream HEAD &&
+	git pull --merge --set-upstream upstream HEAD &&
 	check_config main upstream HEAD &&
 	git checkout other &&
-	git pull --set-upstream upstream HEAD &&
+	git pull --merge --set-upstream upstream HEAD &&
 	check_config other upstream HEAD
 '
 
 test_expect_success 'pull --set-upstream upstream with more than one branch does nothing' '
 	clear_config main three &&
-	git pull --set-upstream upstream main three &&
+	git pull --merge --set-upstream upstream main three &&
 	check_config_missing main &&
 	check_config_missing three
 '
diff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh
index e845d621f6..1c9b121814 100755
--- a/t/t5604-clone-reference.sh
+++ b/t/t5604-clone-reference.sh
@@ -87,7 +87,7 @@ test_expect_success 'updating origin' '
 '
 
 test_expect_success 'pulling changes from origin' '
-	git -C C pull origin
+	git -C C pull --merge origin
 '
 
 # the 2 local objects are commit and tree from the merge
@@ -96,7 +96,7 @@ test_expect_success 'that alternate to origin gets used' '
 '
 
 test_expect_success 'pulling changes from origin' '
-	git -C D pull origin
+	git -C D pull --merge origin
 '
 
 # the 5 local objects are expected; file3 blob, commit in A to add it
diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index 425dad97d5..833b799e25 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh
@@ -103,7 +103,7 @@ test_expect_success 'setup' '
 test_expect_success 'pull renaming branch into unrenaming one' \
 '
 	git show-branch &&
-	test_expect_code 1 git pull . white &&
+	test_expect_code 1 git pull --merge . white &&
 	git ls-files -s &&
 	git ls-files -u B >b.stages &&
 	test_line_count = 3 b.stages &&
@@ -121,7 +121,7 @@ test_expect_success 'pull renaming branch into another renaming one' \
 	rm -f B &&
 	git reset --hard &&
 	git checkout red &&
-	test_expect_code 1 git pull . white &&
+	test_expect_code 1 git pull --merge . white &&
 	git ls-files -u B >b.stages &&
 	test_line_count = 3 b.stages &&
 	git ls-files -s N >n.stages &&
@@ -137,7 +137,7 @@ test_expect_success 'pull unrenaming branch into renaming one' \
 '
 	git reset --hard &&
 	git show-branch &&
-	test_expect_code 1 git pull . main &&
+	test_expect_code 1 git pull --merge . main &&
 	git ls-files -u B >b.stages &&
 	test_line_count = 3 b.stages &&
 	git ls-files -s N >n.stages &&
@@ -153,7 +153,7 @@ test_expect_success 'pull conflicting renames' \
 '
 	git reset --hard &&
 	git show-branch &&
-	test_expect_code 1 git pull . blue &&
+	test_expect_code 1 git pull --merge . blue &&
 	git ls-files -u A >a.stages &&
 	test_line_count = 1 a.stages &&
 	git ls-files -u B >b.stages &&
@@ -173,7 +173,7 @@ test_expect_success 'interference with untracked working tree file' '
 	git reset --hard &&
 	git show-branch &&
 	echo >A this file should not matter &&
-	test_expect_code 1 git pull . white &&
+	test_expect_code 1 git pull --merge . white &&
 	test_path_is_file A
 '
 
@@ -183,7 +183,7 @@ test_expect_success 'interference with untracked working tree file' '
 	git show-branch &&
 	rm -f A &&
 	echo >A this file should not matter &&
-	test_expect_code 1 git pull . red &&
+	test_expect_code 1 git pull --merge . red &&
 	test_path_is_file A
 '
 
@@ -193,7 +193,7 @@ test_expect_success 'interference with untracked working tree file' '
 	git checkout -f main &&
 	git tag -f anchor &&
 	git show-branch &&
-	git pull . yellow &&
+	git pull --merge . yellow &&
 	test_path_is_missing M &&
 	git reset --hard anchor
 '
@@ -232,7 +232,7 @@ test_expect_success 'interference with untracked working tree file' '
 	git tag -f anchor &&
 	git show-branch &&
 	echo >M this file should not matter &&
-	git pull . main &&
+	git pull --merge . main &&
 	test_path_is_file M &&
 	! {
 		git ls-files -s |
diff --git a/t/t6409-merge-subtree.sh b/t/t6409-merge-subtree.sh
index d406b2343c..e1011f6325 100755
--- a/t/t6409-merge-subtree.sh
+++ b/t/t6409-merge-subtree.sh
@@ -100,7 +100,7 @@ test_expect_success 'merge update' '
 	git checkout -b topic_2 &&
 	git commit -m "update git-gui" &&
 	cd ../git &&
-	git pull -s subtree gui topic_2 &&
+	git pull --merge -s subtree gui topic_2 &&
 	git ls-files -s >actual &&
 	(
 		echo "100644 $o3 0	git-gui/git-gui.sh" &&
@@ -129,7 +129,7 @@ test_expect_success 'initial ambiguous subtree' '
 test_expect_success 'merge using explicit' '
 	cd ../git &&
 	git reset --hard topic_2 &&
-	git pull -Xsubtree=git-gui gui topic_2 &&
+	git pull --merge -Xsubtree=git-gui gui topic_2 &&
 	git ls-files -s >actual &&
 	(
 		echo "100644 $o3 0	git-gui/git-gui.sh" &&
@@ -142,7 +142,7 @@ test_expect_success 'merge using explicit' '
 test_expect_success 'merge2 using explicit' '
 	cd ../git &&
 	git reset --hard topic_2 &&
-	git pull -Xsubtree=git-gui2 gui topic_2 &&
+	git pull --merge -Xsubtree=git-gui2 gui topic_2 &&
 	git ls-files -s >actual &&
 	(
 		echo "100644 $o1 0	git-gui/git-gui.sh" &&
diff --git a/t/t6417-merge-ours-theirs.sh b/t/t6417-merge-ours-theirs.sh
index ac9aee9a66..06cebb2974 100755
--- a/t/t6417-merge-ours-theirs.sh
+++ b/t/t6417-merge-ours-theirs.sh
@@ -69,11 +69,11 @@ test_expect_success 'binary file with -Xours/-Xtheirs' '
 '
 
 test_expect_success 'pull passes -X to underlying merge' '
-	git reset --hard main && git pull -s recursive -Xours . side &&
-	git reset --hard main && git pull -s recursive -X ours . side &&
-	git reset --hard main && git pull -s recursive -Xtheirs . side &&
-	git reset --hard main && git pull -s recursive -X theirs . side &&
-	git reset --hard main && test_must_fail git pull -s recursive -X bork . side
+	git reset --hard main && git pull --merge -s recursive -Xours . side &&
+	git reset --hard main && git pull --merge -s recursive -X ours . side &&
+	git reset --hard main && git pull --merge -s recursive -Xtheirs . side &&
+	git reset --hard main && git pull --merge -s recursive -X theirs . side &&
+	git reset --hard main && test_must_fail git pull --merge -s recursive -X bork . side
 '
 
 test_expect_success SYMLINKS 'symlink with -Xours/-Xtheirs' '
diff --git a/t/t7603-merge-reduce-heads.sh b/t/t7603-merge-reduce-heads.sh
index 98948955ae..566b1c1a2a 100755
--- a/t/t7603-merge-reduce-heads.sh
+++ b/t/t7603-merge-reduce-heads.sh
@@ -68,7 +68,7 @@ test_expect_success 'merge c1 with c2, c3, c4, c5' '
 
 test_expect_success 'pull c2, c3, c4, c5 into c1' '
 	git reset --hard c1 &&
-	git pull . c2 c3 c4 c5 &&
+	git pull --merge . c2 c3 c4 c5 &&
 	test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
 	test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" &&
 	test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
-- 
2.32.0.36.g70aac2b1aa

Re: [RFC PATCH 01/35] merge: improve fatal fast-forward message

[Ævar Arnfjörð Bjarmason]

On Mon, Jul 05 2021, Felipe Contreras wrote:

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  builtin/merge.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index a8a843b1f5..05e631229d 100644
> --- 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(_("unable to fast-forward"));

I read the existing message a bit more like "this makes no sense
anymore" (correct) and the latter more like "we encountered an
error". Perhaps something like:

    "Can't merge X with Y, in --ff-only mode, would need to create a merge commit", tip_name

Re: [RFC PATCH 05/35] fast-forward: add advice for novices

[Ævar Arnfjörð Bjarmason]

On Mon, Jul 05 2021, Felipe Contreras wrote:

> It doesn't hurt showing it on `git merge --ff-only` too.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  advice.c        | 11 +++++++++++
>  advice.h        |  1 +
>  builtin/merge.c |  4 +++-
>  3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/advice.c b/advice.c
> index 0b9c89c48a..8f068c8be9 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -326,3 +326,14 @@ void detach_advice(const char *new_name)
>  
>  	fprintf(stderr, fmt, new_name);
>  }
> +
> +void diverging_advice(void)
> +{
> +	advise(_("Diverging branches can't be fast-forwarded, you need to either:\n"
> +		"\n"
> +		"\tgit merge\n"
> +		"\n"
> +		"or:\n"
> +		"\n"
> +		"\tgit rebase\n"));
> +}
> diff --git a/advice.h b/advice.h
> index bd26c385d0..6ce967c962 100644
> --- a/advice.h
> +++ b/advice.h
> @@ -97,5 +97,6 @@ void NORETURN die_resolve_conflict(const char *me);
>  void NORETURN die_conclude_merge(void);
>  void advise_on_updating_sparse_paths(struct string_list *pathspec_list);
>  void detach_advice(const char *new_name);
> +void diverging_advice(void);
>  
>  #endif /* ADVICE_H */
> diff --git a/builtin/merge.c b/builtin/merge.c
> index e396943d37..1836f98f82 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1625,8 +1625,10 @@ static int merge_common(int argc, const char **argv, const char *prefix,
>  		}
>  	}
>  
> -	if (fast_forward == FF_ONLY)
> +	if (fast_forward == FF_ONLY) {
> +		diverging_advice();
>  		die(_("unable to fast-forward"));
> +	}
>  
>  	if (autostash)
>  		create_autostash(the_repository,

...ah, and re my comment on the earlier patch here's where we're adding
advice.

I'd think just squash that into this, or mention in the commit message
"a later commit will add an advice() before this, where this new wording
will make more sense" or something...

Re: [RFC PATCH 10/35] update: add --ff option

[Ævar Arnfjörð Bjarmason]

On Mon, Jul 05 2021, Felipe Contreras wrote:

>  --------
>  linkgit:git-fetch[1], linkgit:git-fast-forward[1],
> diff --git a/builtin/update.c b/builtin/update.c
> index 1a69896aa8..34681fe21a 100644
> --- a/builtin/update.c
> +++ b/builtin/update.c
> @@ -7,12 +7,22 @@
>  #include "run-command.h"
>  #include "dir.h"
>  
> +enum update_mode_type {
> +	UPDATE_MODE_FAST_FORWARD = 0
> +};

Nit: If the value isn't important let's leave it out, also if you add a
trailing comma the subsequent commit where you add another value is less
churny. C supports that just fine.

Re: [RFC PATCH 11/35] update: add --merge mode

[Ævar Arnfjörð Bjarmason]

On Mon, Jul 05 2021, Felipe Contreras wrote:

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  Documentation/git-update.txt |  4 ++++
>  builtin/update.c             | 20 +++++++++++++++++++-
>  t/t5563-update.sh            | 15 +++++++++++++++
>  3 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-update.txt b/Documentation/git-update.txt
> index df778b7ef4..075653c6fd 100644
> --- a/Documentation/git-update.txt
> +++ b/Documentation/git-update.txt
> @@ -29,6 +29,10 @@ OPTIONS
>  --ff::
>  	Forces a fast-forward.
>  
> +-m::
> +--merge::
> +	Forces a merge.
> +
>  SEE ALSO
>  --------
>  linkgit:git-fetch[1], linkgit:git-fast-forward[1],
> diff --git a/builtin/update.c b/builtin/update.c
> index 34681fe21a..5946b383f5 100644
> --- a/builtin/update.c
> +++ b/builtin/update.c
> @@ -8,7 +8,8 @@
>  #include "dir.h"
>  
>  enum update_mode_type {
> -	UPDATE_MODE_FAST_FORWARD = 0
> +	UPDATE_MODE_FAST_FORWARD = 0,
> +	UPDATE_MODE_MERGE
>  };
>  
>  static enum update_mode_type mode = UPDATE_MODE_FAST_FORWARD;
> @@ -22,6 +23,9 @@ static struct option update_options[] = {
>  	OPT_SET_INT_F('f', "ff", &mode,
>  		N_("incorporate changes by fast-forwarding"),
>  		UPDATE_MODE_FAST_FORWARD, PARSE_OPT_NONEG),
> +	OPT_SET_INT_F('m', "merge", &mode,
> +		N_("incorporate changes by merging"),
> +		UPDATE_MODE_MERGE, PARSE_OPT_NONEG),
>  
>  	OPT_END()
>  };
> @@ -50,6 +54,18 @@ static int run_fast_forward(void)
>  	return ret;
>  }
>  
> +static int run_merge(void)
> +{
> +	int ret;
> +	struct strvec args = STRVEC_INIT;
> +
> +	strvec_pushl(&args, "merge", "FETCH_HEAD", NULL);
> +
> +	ret = run_command_v_opt(args.v, RUN_GIT_CMD);
> +	strvec_clear(&args);
> +	return ret;
> +}
> +
>  int cmd_update(int argc, const char **argv, const char *prefix)
>  {
>  	if (!getenv("GIT_REFLOG_ACTION"))
> @@ -68,6 +84,8 @@ int cmd_update(int argc, const char **argv, const char *prefix)
>  
>  	if (mode == UPDATE_MODE_FAST_FORWARD)
>  		return run_fast_forward();
> +	if (mode == UPDATE_MODE_MERGE)
> +		return run_merge();
>  
>  	return 1;
>  }
> diff --git a/t/t5563-update.sh b/t/t5563-update.sh
> index 951df41ac4..833a5285da 100755
> --- a/t/t5563-update.sh
> +++ b/t/t5563-update.sh
> @@ -42,4 +42,19 @@ test_expect_success 'non-fast-forward update' '
>  	)
>  '
>  
> +test_expect_success 'git update non-fast-forward with merge' '
> +	test_when_finished "rm -rf test" &&
> +	(
> +	git clone . test &&
> +	cd test &&
> +	git checkout -b other master^ &&
> +	>new &&

We usually indent the subshell...

> +	git add new &&
> +	git commit -m new &&

And if we can use test_commit, less verbose.

Re: [RFC PATCH 01/35] merge: improve fatal fast-forward message

[Felipe Contreras]

Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Jul 05 2021, Felipe Contreras wrote:
> 
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > ---
> >  builtin/merge.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/builtin/merge.c b/builtin/merge.c
> > index a8a843b1f5..05e631229d 100644
> > --- 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(_("unable to fast-forward"));
> 
> I read the existing message a bit more like "this makes no sense
> anymore" (correct) and the latter more like "we encountered an
> error".

I mean, this is the documentation of --ff-only:

  With `--ff-only`, resolve the merge as a fast-forward when possible.
  When not possible, refuse to merge and exit with a non-zero status.

So if you do `git merge --ff-only` you are telling git: "I want you to
exit with an error when the fast-forward is not possible".

If you do:

  % git merge --ff-only
  fatal: Not possible to fast-forward, aborting.

That "aborting" part is redundant; we know `git merge` should abort if
the fast-forward is not possible, we explicitely told git to do that.

Moreover the "fatal: " prefix also indicates that git aborted.

Then you have "Not possible to fast-forward", which if memory serves
well should be in lowercase (altghough can't find that in the
guidelines).

"unable to fast-forward" is simply a more succinct way of saying
"not possible to fast-forward". Sure, we are not explaining why,
although I can't think of any other reason why we could not fast-forward.

> Perhaps something like:
> 
>     "Can't merge X with Y, in --ff-only mode, would need to create a merge commit", tip_name

I don't think die() messages should be that big, how about?

  branches diverged, can't fast-forward

-- 
Felipe Contreras

Re: [RFC PATCH 05/35] fast-forward: add advice for novices

[Felipe Contreras]

Ævar Arnfjörð Bjarmason wrote:
> On Mon, Jul 05 2021, Felipe Contreras wrote:

> > --- a/builtin/merge.c
> > +++ b/builtin/merge.c
> > @@ -1625,8 +1625,10 @@ static int merge_common(int argc, const char **argv, const char *prefix,
> >  		}
> >  	}
> >  
> > -	if (fast_forward == FF_ONLY)
> > +	if (fast_forward == FF_ONLY) {
> > +		diverging_advice();
> >  		die(_("unable to fast-forward"));
> > +	}
> >  
> >  	if (autostash)
> >  		create_autostash(the_repository,
> 
> ...ah, and re my comment on the earlier patch here's where we're adding
> advice.
> 
> I'd think just squash that into this, or mention in the commit message
> "a later commit will add an advice() before this, where this new wording
> will make more sense" or something...

I don't think we lost any information in the previous patch. In fact
with my suggestion we gained a little bit.

-- 
Felipe Contreras

Re: [RFC PATCH 10/35] update: add --ff option

[Felipe Contreras]

Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Jul 05 2021, Felipe Contreras wrote:
> 
> >  --------
> >  linkgit:git-fetch[1], linkgit:git-fast-forward[1],
> > diff --git a/builtin/update.c b/builtin/update.c
> > index 1a69896aa8..34681fe21a 100644
> > --- a/builtin/update.c
> > +++ b/builtin/update.c
> > @@ -7,12 +7,22 @@
> >  #include "run-command.h"
> >  #include "dir.h"
> >  
> > +enum update_mode_type {
> > +	UPDATE_MODE_FAST_FORWARD = 0
> > +};
> 
> Nit: If the value isn't important let's leave it out,

Yes, in one instantiation of the series it did matter, but not in the
current one. I'll drop it.

> also if you add a trailing comma the subsequent commit where you add
> another value is less churny. C supports that just fine.

I know, and I actually prefer that style, but I've seen the comma
dropped in many instances in the current code base. I was just following
the current style.

But if you prefer it as well I'll add it.

-- 
Felipe Contreras

RE: [RFC PATCH 01/35] merge: improve fatal fast-forward message

[Randall S. Becker]

On July 6, 2021 4:40 PM, Felipe Contreras wrote:
>Subject: Re: [RFC PATCH 01/35] merge: improve fatal fast-forward message
>
>Ævar Arnfjörð Bjarmason wrote:
>>
>> On Mon, Jul 05 2021, Felipe Contreras wrote:
>>
>> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> > ---
>> >  builtin/merge.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/builtin/merge.c b/builtin/merge.c index
>> > a8a843b1f5..05e631229d 100644
>> > --- 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(_("unable to fast-forward"));
>>
>> I read the existing message a bit more like "this makes no sense
>> anymore" (correct) and the latter more like "we encountered an error".
>
>I mean, this is the documentation of --ff-only:
>
>  With `--ff-only`, resolve the merge as a fast-forward when possible.
>  When not possible, refuse to merge and exit with a non-zero status.
>
>So if you do `git merge --ff-only` you are telling git: "I want you to exit with an error when the fast-forward is not possible".
>
>If you do:
>
>  % git merge --ff-only
>  fatal: Not possible to fast-forward, aborting.
>
>That "aborting" part is redundant; we know `git merge` should abort if the fast-forward is not possible, we explicitely told git to do that.

`git merge` is a special operation where errors (conflicts, for one) may leave the repository in a merge pending state where you subsequently may have to use `git merge --abort` to reset the situation or `git add` to continue. The `aborting` output makes it clear that you do not have to do the `--abort` and *cannot* do the `add` because there was an implicit `--abort` done resulting from the failure. This is important information for the user.

Re: [RFC PATCH 11/35] update: add --merge mode

[Felipe Contreras]

Ævar Arnfjörð Bjarmason wrote:
> On Mon, Jul 05 2021, Felipe Contreras wrote:

> > --- a/t/t5563-update.sh
> > +++ b/t/t5563-update.sh
> > @@ -42,4 +42,19 @@ test_expect_success 'non-fast-forward update' '
> >  	)
> >  '
> >  
> > +test_expect_success 'git update non-fast-forward with merge' '
> > +	test_when_finished "rm -rf test" &&
> > +	(
> > +	git clone . test &&
> > +	cd test &&
> > +	git checkout -b other master^ &&
> > +	>new &&
> 
> We usually indent the subshell...

All right.

> > +	git add new &&
> > +	git commit -m new &&
> 
> And if we can use test_commit, less verbose.

OK. Sure.

FTR. These tests come from 2013, they don't fit my current style and I
wanted to rewrite them completely since there's a lot of duplicated
code. But I thought: what the hell, they work. Plus there's only so much
time I'm willing to spend on an RFC v1 of this series (which probably
won't be merged anyway).

Cheers.

-- 
Felipe Contreras

Re: [RFC PATCH 01/35] merge: improve fatal fast-forward message

[Junio C Hamano]

"Randall S. Becker" <rsbecker@nexbridge.com> writes:

>>If you do:
>>
>>  % git merge --ff-only
>>  fatal: Not possible to fast-forward, aborting.
>>
>>That "aborting" part is redundant; we know `git merge` should abort
> if the fast-forward is not possible, we explicitely told git to do
> that.
>
> `git merge` is a special operation where errors (conflicts, for one)
> may leave the repository in a merge pending state where you
> subsequently may have to use `git merge --abort` to reset the
> situation or `git add` to continue. The `aborting` output makes it
> clear that you do not have to do the `--abort` and *cannot* do the
> `add` because there was an implicit `--abort` done resulting from the
> failure. This is important information for the user.

If so, adding ", aborting" to the end is misleading.  In this
particular failure mode, the command pretends that the merge did not
even start.

RE: [RFC PATCH 01/35] merge: improve fatal fast-forward message

[Felipe Contreras]

Randall S. Becker wrote:
> On July 6, 2021 4:40 PM, Felipe Contreras wrote:
> >Subject: Re: [RFC PATCH 01/35] merge: improve fatal fast-forward message
> >
> >Ævar Arnfjörð Bjarmason wrote:
> >>
> >> On Mon, Jul 05 2021, Felipe Contreras wrote:
> >>
> >> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> >> > ---
> >> >  builtin/merge.c | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/builtin/merge.c b/builtin/merge.c index
> >> > a8a843b1f5..05e631229d 100644
> >> > --- 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(_("unable to fast-forward"));
> >>
> >> I read the existing message a bit more like "this makes no sense
> >> anymore" (correct) and the latter more like "we encountered an error".
> >
> >I mean, this is the documentation of --ff-only:
> >
> >  With `--ff-only`, resolve the merge as a fast-forward when possible.
> >  When not possible, refuse to merge and exit with a non-zero status.
> >
> >So if you do `git merge --ff-only` you are telling git: "I want you to exit with an error when the fast-forward is not possible".
> >
> >If you do:
> >
> >  % git merge --ff-only
> >  fatal: Not possible to fast-forward, aborting.
> >
> >That "aborting" part is redundant; we know `git merge` should abort if the fast-forward is not possible, we explicitely told git to do that.
> 
> `git merge` is a special operation where errors (conflicts, for one)
> may leave the repository in a merge pending state where you
> subsequently may have to use `git merge --abort` to reset the
> situation or `git add` to continue. The `aborting` output makes it
> clear that you do not have to do the `--abort` and *cannot* do the
> `add` because there was an implicit `--abort` done resulting from the
> failure.

But this is not a `git merge`, this is a `git merge --ff-only`; they are
different operations. There *never* is a need for `--abort` with
`git merge --ff-only`.

Anyway, the error message is meant for `git fast-forward` which
definitely doesn't need any `--abort`.

Initially I created a new variable to have a different error message for
`git merge --ff-only` and `git fast-forward`, precisely to avoid
changing the current error message of `git merge --ff-only` and thus
avoid any inertial comments like this one. But then I thought there was
no need to complicate the series when both can be improved at once.
Apparently that's not the case.

I guess I'll add it back.

Cheers.

-- 
Felipe Contreras

Re: [RFC PATCH 01/35] merge: improve fatal fast-forward message

[Felipe Contreras]

Junio C Hamano wrote:
> "Randall S. Becker" <rsbecker@nexbridge.com> writes:
> 
> >>If you do:
> >>
> >>  % git merge --ff-only
> >>  fatal: Not possible to fast-forward, aborting.
> >>
> >>That "aborting" part is redundant; we know `git merge` should abort
> > if the fast-forward is not possible, we explicitely told git to do
> > that.
> >
> > `git merge` is a special operation where errors (conflicts, for one)
> > may leave the repository in a merge pending state where you
> > subsequently may have to use `git merge --abort` to reset the
> > situation or `git add` to continue. The `aborting` output makes it
> > clear that you do not have to do the `--abort` and *cannot* do the
> > `add` because there was an implicit `--abort` done resulting from the
> > failure. This is important information for the user.
> 
> If so, adding ", aborting" to the end is misleading.  In this
> particular failure mode, the command pretends that the merge did not
> even start.

That's true.

Whatever is the case for that "aborting" to be there I don't think it's
adding any value.

-- 
Felipe Contreras

RE: [RFC PATCH 01/35] merge: improve fatal fast-forward message

[Randall S. Becker]

On July 6, 2021 5:12 PM, Felipe Contreras wrote:
>Randall S. Becker wrote:
>> On July 6, 2021 4:40 PM, Felipe Contreras wrote:
>> >Subject: Re: [RFC PATCH 01/35] merge: improve fatal fast-forward
>> >message
>> >
>> >Ævar Arnfjörð Bjarmason wrote:
>> >>
>> >> On Mon, Jul 05 2021, Felipe Contreras wrote:
>> >>
>> >> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> >> > ---
>> >> >  builtin/merge.c | 2 +-
>> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/builtin/merge.c b/builtin/merge.c index
>> >> > a8a843b1f5..05e631229d 100644
>> >> > --- 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(_("unable to fast-forward"));
>> >>
>> >> I read the existing message a bit more like "this makes no sense
>> >> anymore" (correct) and the latter more like "we encountered an error".
>> >
>> >I mean, this is the documentation of --ff-only:
>> >
>> >  With `--ff-only`, resolve the merge as a fast-forward when possible.
>> >  When not possible, refuse to merge and exit with a non-zero status.
>> >
>> >So if you do `git merge --ff-only` you are telling git: "I want you to exit with an error when the fast-forward is not possible".
>> >
>> >If you do:
>> >
>> >  % git merge --ff-only
>> >  fatal: Not possible to fast-forward, aborting.
>> >
>> >That "aborting" part is redundant; we know `git merge` should abort if the fast-forward is not possible, we explicitely told git to do that.
>>
>> `git merge` is a special operation where errors (conflicts, for one)
>> may leave the repository in a merge pending state where you
>> subsequently may have to use `git merge --abort` to reset the
>> situation or `git add` to continue. The `aborting` output makes it
>> clear that you do not have to do the `--abort` and *cannot* do the
>> `add` because there was an implicit `--abort` done resulting from the
>> failure.
>
>But this is not a `git merge`, this is a `git merge --ff-only`; they are different operations. There *never* is a need for `--abort` with `git
>merge --ff-only`.

Well, you know that and I know that, but having to explain this to every new git user who will operationally use git merge --ff-only within hours or days of their first clone is a different matter.

>Anyway, the error message is meant for `git fast-forward` which definitely doesn't need any `--abort`.
>
>Initially I created a new variable to have a different error message for `git merge --ff-only` and `git fast-forward`, precisely to avoid
>changing the current error message of `git merge --ff-only` and thus avoid any inertial comments like this one. But then I thought there
>was no need to complicate the series when both can be improved at once.
>Apparently that's not the case.
>
>I guess I'll add it back.

Thanks. I understand the purpose here.

-Randall

RE: [RFC PATCH 01/35] merge: improve fatal fast-forward message

[Randall S. Becker]

On July 6, 2021 4:56 PM, Junio C Hamano wrote:
>"Randall S. Becker" <rsbecker@nexbridge.com> writes:
>
>>>If you do:
>>>
>>>  % git merge --ff-only
>>>  fatal: Not possible to fast-forward, aborting.
>>>
>>>That "aborting" part is redundant; we know `git merge` should abort
>> if the fast-forward is not possible, we explicitely told git to do
>> that.
>>
>> `git merge` is a special operation where errors (conflicts, for one)
>> may leave the repository in a merge pending state where you
>> subsequently may have to use `git merge --abort` to reset the
>> situation or `git add` to continue. The `aborting` output makes it
>> clear that you do not have to do the `--abort` and *cannot* do the
>> `add` because there was an implicit `--abort` done resulting from the
>> failure. This is important information for the user.
>
>If so, adding ", aborting" to the end is misleading.  In this particular failure mode, the command pretends that the merge did not even
>start.

You know that, and I know that. I contend that git users do not generally want to care about failure modes. While a flow description might be instructive, I doubt many git users would look at it. We can only hope that front ends know how to make this clean for them.

-Randall

Re: [RFC PATCH 01/35] merge: improve fatal fast-forward message

[Junio C Hamano]

"Randall S. Becker" <rsbecker@nexbridge.com> writes:

> On July 6, 2021 4:56 PM, Junio C Hamano wrote:
>>"Randall S. Becker" <rsbecker@nexbridge.com> writes:
>>
>>>>If you do:
>>>>
>>>>  % git merge --ff-only
>>>>  fatal: Not possible to fast-forward, aborting.
>>>>
>>>>That "aborting" part is redundant; we know `git merge` should abort
>>> if the fast-forward is not possible, we explicitely told git to do
>>> that.
>>>
>>> `git merge` is a special operation where errors (conflicts, for one)
>>> may leave the repository in a merge pending state where you
>>> subsequently may have to use `git merge --abort` to reset the
>>> situation or `git add` to continue. The `aborting` output makes it
>>> clear that you do not have to do the `--abort` and *cannot* do the
>>> `add` because there was an implicit `--abort` done resulting from the
>>> failure. This is important information for the user.
>>
>>If so, adding ", aborting" to the end is misleading.  In this particular failure mode, the command pretends that the merge did not even
>>start.
>
> You know that, and I know that. I contend that git users do not
> generally want to care about failure modes. While a flow
> description might be instructive, I doubt many git users would
> look at it. We can only hope that front ends know how to make this
> clean for them.

They may not care about "failure modes" but they would want to know
that in this situation, unlike other times, they do not have to say
"git merge --abort" (or "git reset --hard").  Saying ", aborting"
does not help as much as saying say ", not starting a merge", for
example.

RE: [RFC PATCH 01/35] merge: improve fatal fast-forward message

[Felipe Contreras]

Randall S. Becker wrote:
> On July 6, 2021 5:12 PM, Felipe Contreras wrote:

> >But this is not a `git merge`, this is a `git merge --ff-only`; they
> >are different operations. There *never* is a need for `--abort` with
> >`git merge --ff-only`.
> 
> Well, you know that and I know that, but having to explain this to
> every new git user who will operationally use git merge --ff-only
> within hours or days of their first clone is a different matter.

New users don't do `git merge --ff-only`, they very likely don't do
`git merge` either, nor do they do `git pull --ff-only`. This is obvious
from the countless threads where users do `git pull` without knowing
that they will create a true merge by mistake.

Even less likely is that they will know about `git merge --abort`, and even
less that they will know both `git merge --ff-only` and
`git merge --abort`.

Even even less likely that they will mistakenly expect
`git merge --ff-only` to be left inside a temporary stated to be
resolved by `git merge --abort`.

But as Junio said, *if* such a hypothetical user exists, we don't want
to keep misleading them into thinking that a true merge was somehow
aborted, because it wasn't.

It's much better to simply let go the current error message.

Cheers.

-- 
Felipe Contreras

RE: [RFC PATCH 01/35] merge: improve fatal fast-forward message

[Randall S. Becker]

On July 6, 2021 5:54 PM Junio C Hamano wrote:
>"Randall S. Becker" <rsbecker@nexbridge.com> writes:
>
>> On July 6, 2021 4:56 PM, Junio C Hamano wrote:
>>>"Randall S. Becker" <rsbecker@nexbridge.com> writes:
>>>
>>>>>If you do:
>>>>>
>>>>>  % git merge --ff-only
>>>>>  fatal: Not possible to fast-forward, aborting.
>>>>>
>>>>>That "aborting" part is redundant; we know `git merge` should abort
>>>> if the fast-forward is not possible, we explicitely told git to do
>>>> that.
>>>>
>>>> `git merge` is a special operation where errors (conflicts, for one)
>>>> may leave the repository in a merge pending state where you
>>>> subsequently may have to use `git merge --abort` to reset the
>>>> situation or `git add` to continue. The `aborting` output makes it
>>>> clear that you do not have to do the `--abort` and *cannot* do the
>>>> `add` because there was an implicit `--abort` done resulting from
>>>> the failure. This is important information for the user.
>>>
>>>If so, adding ", aborting" to the end is misleading.  In this
>>>particular failure mode, the command pretends that the merge did not even start.
>>
>> You know that, and I know that. I contend that git users do not
>> generally want to care about failure modes. While a flow description
>> might be instructive, I doubt many git users would look at it. We can
>> only hope that front ends know how to make this clean for them.
>
>They may not care about "failure modes" but they would want to know that in this situation, unlike other times, they do not have to say
>"git merge --abort" (or "git reset --hard").  Saying ", aborting"
>does not help as much as saying say ", not starting a merge", for example.

I like that a lot, actually "not starting a merge" is much more helpful.