[PATCH v5 0/6] Reject non-ff pulls by default

[PATCH v5 0/6] Reject non-ff pulls by default

[Felipe Contreras]

It is very typical for Git newcomers to inadvertently create merges and worst:
inadvertently 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 avoid these problems and keep 'git pull' useful, it has been agreed that
'git pull' should barf by default if the merge is non-fast-forward.
Unfortunately this breaks backwards-compatibility, so we need to be careful
about the error messages we give, and that we provide enough information to our
users to move forward without distrupting their workflow too much.

With the proper error messages and documentation, it has been agreed that the
new behavior is OK.

These are the steps needed to achieve this:

4) Only allow fast-forward merges by default

We could pass --ff-only to `git merge`, however, if we do that we'll get an error like this:

  Not possible to fast-forward, aborting.

This is not friendly; we want an error that is user-friendly:

  The pull was not fast-forward, please either merge or rebase.
  If unsure, run 'git pull --merge'.

When we do this we want to give the users the option to go back to the previous
behavior, so a new configuration is needed.

3) Add merge-ff-only config

This option would trigger a check inside `git pull` itself, and error out with
the aforementioned message if it's not possible to do a fast-forward merge.

However, this option conflicts with --rebase, and --no-rebase. Solution below.

2) Add --merge option

Since we have a message that says "If unsure, run 'git pull --merge'", which is
more friendly than 'git pull --no-rebase', we should add this option, and
deprecate --no-rebase.

However, the documentation would become confusing if --merge is configured in
pull.rebase, instead, we want something like this:

  See `pull.mode`, `branch.<name>.pullmode` in linkgit:git-config[1] if you want
  to make `git pull` always use `--merge`.

1) Rename pull.rename to pull.mode and
   branch.<name>.rebase to branch.<name>.pullmode

This way the configurations and options remain consistent:

  git pull --merge
  pull.mode = merge
  branch.<name>.pullmode = merge

  git pull --rebase
  pull.mode = rebase
  branch.<name>.pullmode = rebase

  git pull --rebase=preserve
  pull.mode = rebase-preserve
  branch.<name>.pullmode = rebase-preserve

  git pull
  pull.mode = merge-ff-only
  branch.<name>.pullmode = merge-ff-only
 
This patch series does all the steps mentioned, but in reverse order, and in
addition updates the tests to use the new configurations instead.


Felipe Contreras (6):
  pull: rename pull.rename to pull.mode
  pull: migrate all the tests to pull.mode
  pull: refactor $rebase variable into $mode
  pull: add --merge option
  pull: add merge-ff-only option
  pull: only allow ff merges by default

 Documentation/config.txt     |  37 ++++++++-------
 Documentation/git-pull.txt   |  28 ++++++++++--
 branch.c                     |   4 +-
 builtin/remote.c             |  14 +++++-
 git-pull.sh                  | 105 ++++++++++++++++++++++++++++++++-----------
 t/t3200-branch.sh            |  40 ++++++++---------
 t/t4013-diff-various.sh      |   2 +-
 t/t5500-fetch-pack.sh        |   2 +-
 t/t5505-remote.sh            |   2 +-
 t/t5520-pull.sh              |  90 ++++++++++++++++++++++++-------------
 t/t5524-pull-msg.sh          |   2 +-
 t/t5601-clone.sh             |   4 +-
 t/t5700-clone-reference.sh   |   4 +-
 t/t6022-merge-rename.sh      |  20 ++++-----
 t/t6026-merge-attr.sh        |   2 +-
 t/t6029-merge-subtree.sh     |   6 +--
 t/t6037-merge-ours-theirs.sh |  10 ++---
 17 files changed, 245 insertions(+), 127 deletions(-)

-- 
1.9.2+fc1.3.gade8541

[PATCH v5 1/6] pull: rename pull.rename to pull.mode

[Felipe Contreras]

Also 'branch.<name>.rebase' to 'branch.<name>.pullmode'.

This way 'pull.mode' can be set to 'merge', and the default can be
something else.

The old configurations still work, but get deprecated.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/config.txt   | 34 +++++++++++++++++-----------------
 Documentation/git-pull.txt |  2 +-
 branch.c                   |  4 ++--
 builtin/remote.c           | 14 ++++++++++++--
 git-pull.sh                | 39 +++++++++++++++++++++++++++++++++++++--
 t/t3200-branch.sh          | 40 ++++++++++++++++++++--------------------
 t/t5601-clone.sh           |  4 ++--
 7 files changed, 91 insertions(+), 46 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c26a7c8..5978d35 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -708,7 +708,7 @@ branch.autosetupmerge::
 branch.autosetuprebase::
 	When a new branch is created with 'git branch' or 'git checkout'
 	that tracks another branch, this variable tells Git to set
-	up pull to rebase instead of merge (see "branch.<name>.rebase").
+	up pull to rebase instead of merge (see "branch.<name>.pullmode").
 	When `never`, rebase is never automatically set to true.
 	When `local`, rebase is set to true for tracked branches of
 	other local branches.
@@ -764,15 +764,15 @@ branch.<name>.mergeoptions::
 	option values containing whitespace characters are currently not
 	supported.
 
-branch.<name>.rebase::
-	When true, rebase the branch <name> on top of the fetched branch,
-	instead of merging the default branch from the default remote when
-	"git pull" is run. See "pull.rebase" for doing this in a non
-	branch-specific manner.
+branch.<name>.pullmode::
+	When "git pull" is run, this determines if it would either merge or
+	rebase the fetched branch. The possible values are 'merge',
+	'rebase', and 'rebase-preserve'. See "pull.mode" for doing this in a
+	non branch-specific manner.
 +
-	When preserve, also pass `--preserve-merges` along to 'git rebase'
-	so that locally committed merge commits will not be flattened
-	by running 'git pull'.
+	When 'rebase-preserve', also pass `--preserve-merges` along to
+	'git rebase' so that locally committed merge commits will not be
+	flattened by running 'git pull'.
 +
 *NOTE*: this is a possibly dangerous operation; do *not* use
 it unless you understand the implications (see linkgit:git-rebase[1]
@@ -1881,15 +1881,15 @@ pretty.<name>::
 	Note that an alias with the same name as a built-in format
 	will be silently ignored.
 
-pull.rebase::
-	When true, rebase branches on top of the fetched branch, instead
-	of merging the default branch from the default remote when "git
-	pull" is run. See "branch.<name>.rebase" for setting this on a
-	per-branch basis.
+pull.mode::
+	When "git pull" is run, this determines if it would either merge or
+	rebase the fetched branch. The possible values are 'merge',
+	'rebase', and 'rebase-preserve'. See "branch.<name>.pullmode" for doing
+	this in a non branch-specific manner.
 +
-	When preserve, also pass `--preserve-merges` along to 'git rebase'
-	so that locally committed merge commits will not be flattened
-	by running 'git pull'.
+	When 'rebase-preserve', also pass `--preserve-merges` along to
+	'git rebase' so that locally committed merge commits will not be
+	flattened by running 'git pull'.
 +
 *NOTE*: this is a possibly dangerous operation; do *not* use
 it unless you understand the implications (see linkgit:git-rebase[1]
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 200eb22..9a91b9f 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -117,7 +117,7 @@ locally created merge commits will not be flattened.
 +
 When false, merge the current branch into the upstream branch.
 +
-See `pull.rebase`, `branch.<name>.rebase` and `branch.autosetuprebase` in
+See `pull.mode`, `branch.<name>.pullmode` and `branch.autosetuprebase` in
 linkgit:git-config[1] if you want to make `git pull` always use
 `--rebase` instead of merging.
 +
diff --git a/branch.c b/branch.c
index 723a36b..63ce671 100644
--- a/branch.c
+++ b/branch.c
@@ -71,8 +71,8 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
 
 	if (rebasing) {
 		strbuf_reset(&key);
-		strbuf_addf(&key, "branch.%s.rebase", local);
-		git_config_set(key.buf, "true");
+		strbuf_addf(&key, "branch.%s.pullmode", local);
+		git_config_set(key.buf, "rebase");
 	}
 	strbuf_release(&key);
 
diff --git a/builtin/remote.c b/builtin/remote.c
index b3ab4cf..46d3c4d 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -264,7 +264,7 @@ static int config_read_branches(const char *key, const char *value, void *cb)
 		char *name;
 		struct string_list_item *item;
 		struct branch_info *info;
-		enum { REMOTE, MERGE, REBASE } type;
+		enum { REMOTE, MERGE, REBASE, PULLMODE } type;
 
 		key += 7;
 		if (ends_with(key, ".remote")) {
@@ -276,6 +276,9 @@ static int config_read_branches(const char *key, const char *value, void *cb)
 		} else if (ends_with(key, ".rebase")) {
 			name = xstrndup(key, strlen(key) - 7);
 			type = REBASE;
+		} else if (ends_with(key, ".pullmode")) {
+			name = xstrndup(key, strlen(key) - 9);
+			type = PULLMODE;
 		} else
 			return 0;
 
@@ -299,12 +302,19 @@ static int config_read_branches(const char *key, const char *value, void *cb)
 				space = strchr(value, ' ');
 			}
 			string_list_append(&info->merge, xstrdup(value));
-		} else {
+		} else if (type == REBASE) {
 			int v = git_config_maybe_bool(orig_key, value);
 			if (v >= 0)
 				info->rebase = v;
 			else if (!strcmp(value, "preserve"))
 				info->rebase = 1;
+		} else {
+			if (!strcmp(value, "rebase"))
+				info->rebase = 1;
+			else if (!strcmp(value, "merge"))
+				info->rebase = 0;
+			else if (!strcmp(value, "rebase-preserve"))
+				info->rebase = 1;
 		}
 	}
 	return 0;
diff --git a/git-pull.sh b/git-pull.sh
index 0a5aa2c..d4e25f1 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -15,6 +15,10 @@ require_work_tree_exists
 cd_to_toplevel
 
 
+warn () {
+	printf >&2 '%s\n' "$*"
+}
+
 die_conflict () {
     git diff-index --cached --name-status -r --ignore-submodules HEAD --
     if [ $(git config --bool --get advice.resolveConflict || echo true) = "true" ]; then
@@ -47,10 +51,41 @@ log_arg= verbosity= progress= recurse_submodules= verify_signatures=
 merge_args= edit= rebase_args=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
-rebase=$(bool_or_string_config branch.$curr_branch_short.rebase)
+mode=$(git config branch.${curr_branch_short}.pullmode)
+if test -z "$mode"
+then
+	mode=$(git config pull.mode)
+fi
+case "$mode" in
+merge)
+	rebase="false"
+	;;
+rebase)
+	rebase="true"
+	;;
+rebase-preserve)
+	rebase="preserve"
+	;;
+'')
+	;;
+*)
+	echo "Invalid value for 'mode'"
+	usage
+	exit 1
+	;;
+esac
 if test -z "$rebase"
 then
-	rebase=$(bool_or_string_config pull.rebase)
+	rebase=$(bool_or_string_config branch.$curr_branch_short.rebase)
+	if test -z "$rebase"
+	then
+		rebase=$(bool_or_string_config pull.rebase)
+	fi
+	if test -n "$rebase"
+	then
+		warn "$(gettext "The configurations pull.rebase and branch.<name>.rebase are deprecated.")"
+		warn "$(gettext "Please use pull.mode and branch.<name>.pullmode instead.")"
+	fi
 fi
 dry_run=
 while :
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fcdb867..b79aa75 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -559,7 +559,7 @@ test_expect_success 'autosetuprebase local on a tracked local branch' '
 	git branch --track myr1 mybase &&
 	test "$(git config branch.myr1.remote)" = . &&
 	test "$(git config branch.myr1.merge)" = refs/heads/mybase &&
-	test "$(git config branch.myr1.rebase)" = true
+	test "$(git config branch.myr1.pullmode)" = rebase
 '
 
 test_expect_success 'autosetuprebase always on a tracked local branch' '
@@ -571,7 +571,7 @@ test_expect_success 'autosetuprebase always on a tracked local branch' '
 	git branch --track myr2 mybase &&
 	test "$(git config branch.myr2.remote)" = . &&
 	test "$(git config branch.myr2.merge)" = refs/heads/mybase &&
-	test "$(git config branch.myr2.rebase)" = true
+	test "$(git config branch.myr2.pullmode)" = rebase
 '
 
 test_expect_success 'autosetuprebase remote on a tracked local branch' '
@@ -583,7 +583,7 @@ test_expect_success 'autosetuprebase remote on a tracked local branch' '
 	git branch --track myr3 mybase2 &&
 	test "$(git config branch.myr3.remote)" = . &&
 	test "$(git config branch.myr3.merge)" = refs/heads/mybase2 &&
-	! test "$(git config branch.myr3.rebase)" = true
+	! test "$(git config branch.myr3.pullmode)" = rebase
 '
 
 test_expect_success 'autosetuprebase never on a tracked local branch' '
@@ -595,7 +595,7 @@ test_expect_success 'autosetuprebase never on a tracked local branch' '
 	git branch --track myr4 mybase2 &&
 	test "$(git config branch.myr4.remote)" = . &&
 	test "$(git config branch.myr4.merge)" = refs/heads/mybase2 &&
-	! test "$(git config branch.myr4.rebase)" = true
+	! test "$(git config branch.myr4.pullmode)" = rebase
 '
 
 test_expect_success 'autosetuprebase local on a tracked remote branch' '
@@ -606,7 +606,7 @@ test_expect_success 'autosetuprebase local on a tracked remote branch' '
 	git branch --track myr5 local/master &&
 	test "$(git config branch.myr5.remote)" = local &&
 	test "$(git config branch.myr5.merge)" = refs/heads/master &&
-	! test "$(git config branch.myr5.rebase)" = true
+	! test "$(git config branch.myr5.pullmode)" = rebase
 '
 
 test_expect_success 'autosetuprebase never on a tracked remote branch' '
@@ -617,7 +617,7 @@ test_expect_success 'autosetuprebase never on a tracked remote branch' '
 	git branch --track myr6 local/master &&
 	test "$(git config branch.myr6.remote)" = local &&
 	test "$(git config branch.myr6.merge)" = refs/heads/master &&
-	! test "$(git config branch.myr6.rebase)" = true
+	! test "$(git config branch.myr6.pullmode)" = rebase
 '
 
 test_expect_success 'autosetuprebase remote on a tracked remote branch' '
@@ -628,7 +628,7 @@ test_expect_success 'autosetuprebase remote on a tracked remote branch' '
 	git branch --track myr7 local/master &&
 	test "$(git config branch.myr7.remote)" = local &&
 	test "$(git config branch.myr7.merge)" = refs/heads/master &&
-	test "$(git config branch.myr7.rebase)" = true
+	test "$(git config branch.myr7.pullmode)" = rebase
 '
 
 test_expect_success 'autosetuprebase always on a tracked remote branch' '
@@ -639,7 +639,7 @@ test_expect_success 'autosetuprebase always on a tracked remote branch' '
 	git branch --track myr8 local/master &&
 	test "$(git config branch.myr8.remote)" = local &&
 	test "$(git config branch.myr8.merge)" = refs/heads/master &&
-	test "$(git config branch.myr8.rebase)" = true
+	test "$(git config branch.myr8.pullmode)" = rebase
 '
 
 test_expect_success 'autosetuprebase unconfigured on a tracked remote branch' '
@@ -650,7 +650,7 @@ test_expect_success 'autosetuprebase unconfigured on a tracked remote branch' '
 	git branch --track myr9 local/master &&
 	test "$(git config branch.myr9.remote)" = local &&
 	test "$(git config branch.myr9.merge)" = refs/heads/master &&
-	test "z$(git config branch.myr9.rebase)" = z
+	test "z$(git config branch.myr9.pullmode)" = z
 '
 
 test_expect_success 'autosetuprebase unconfigured on a tracked local branch' '
@@ -661,7 +661,7 @@ test_expect_success 'autosetuprebase unconfigured on a tracked local branch' '
 	git branch --track myr10 mybase2 &&
 	test "$(git config branch.myr10.remote)" = . &&
 	test "$(git config branch.myr10.merge)" = refs/heads/mybase2 &&
-	test "z$(git config branch.myr10.rebase)" = z
+	test "z$(git config branch.myr10.pullmode)" = z
 '
 
 test_expect_success 'autosetuprebase unconfigured on untracked local branch' '
@@ -671,7 +671,7 @@ test_expect_success 'autosetuprebase unconfigured on untracked local branch' '
 	git branch --no-track myr11 mybase2 &&
 	test "z$(git config branch.myr11.remote)" = z &&
 	test "z$(git config branch.myr11.merge)" = z &&
-	test "z$(git config branch.myr11.rebase)" = z
+	test "z$(git config branch.myr11.pullmode)" = z
 '
 
 test_expect_success 'autosetuprebase unconfigured on untracked remote branch' '
@@ -681,7 +681,7 @@ test_expect_success 'autosetuprebase unconfigured on untracked remote branch' '
 	git branch --no-track myr12 local/master &&
 	test "z$(git config branch.myr12.remote)" = z &&
 	test "z$(git config branch.myr12.merge)" = z &&
-	test "z$(git config branch.myr12.rebase)" = z
+	test "z$(git config branch.myr12.pullmode)" = z
 '
 
 test_expect_success 'autosetuprebase never on an untracked local branch' '
@@ -692,7 +692,7 @@ test_expect_success 'autosetuprebase never on an untracked local branch' '
 	git branch --no-track myr13 mybase2 &&
 	test "z$(git config branch.myr13.remote)" = z &&
 	test "z$(git config branch.myr13.merge)" = z &&
-	test "z$(git config branch.myr13.rebase)" = z
+	test "z$(git config branch.myr13.pullmode)" = z
 '
 
 test_expect_success 'autosetuprebase local on an untracked local branch' '
@@ -703,7 +703,7 @@ test_expect_success 'autosetuprebase local on an untracked local branch' '
 	git branch --no-track myr14 mybase2 &&
 	test "z$(git config branch.myr14.remote)" = z &&
 	test "z$(git config branch.myr14.merge)" = z &&
-	test "z$(git config branch.myr14.rebase)" = z
+	test "z$(git config branch.myr14.pullmode)" = z
 '
 
 test_expect_success 'autosetuprebase remote on an untracked local branch' '
@@ -714,7 +714,7 @@ test_expect_success 'autosetuprebase remote on an untracked local branch' '
 	git branch --no-track myr15 mybase2 &&
 	test "z$(git config branch.myr15.remote)" = z &&
 	test "z$(git config branch.myr15.merge)" = z &&
-	test "z$(git config branch.myr15.rebase)" = z
+	test "z$(git config branch.myr15.pullmode)" = z
 '
 
 test_expect_success 'autosetuprebase always on an untracked local branch' '
@@ -725,7 +725,7 @@ test_expect_success 'autosetuprebase always on an untracked local branch' '
 	git branch --no-track myr16 mybase2 &&
 	test "z$(git config branch.myr16.remote)" = z &&
 	test "z$(git config branch.myr16.merge)" = z &&
-	test "z$(git config branch.myr16.rebase)" = z
+	test "z$(git config branch.myr16.pullmode)" = z
 '
 
 test_expect_success 'autosetuprebase never on an untracked remote branch' '
@@ -736,7 +736,7 @@ test_expect_success 'autosetuprebase never on an untracked remote branch' '
 	git branch --no-track myr17 local/master &&
 	test "z$(git config branch.myr17.remote)" = z &&
 	test "z$(git config branch.myr17.merge)" = z &&
-	test "z$(git config branch.myr17.rebase)" = z
+	test "z$(git config branch.myr17.pullmode)" = z
 '
 
 test_expect_success 'autosetuprebase local on an untracked remote branch' '
@@ -747,7 +747,7 @@ test_expect_success 'autosetuprebase local on an untracked remote branch' '
 	git branch --no-track myr18 local/master &&
 	test "z$(git config branch.myr18.remote)" = z &&
 	test "z$(git config branch.myr18.merge)" = z &&
-	test "z$(git config branch.myr18.rebase)" = z
+	test "z$(git config branch.myr18.pullmode)" = z
 '
 
 test_expect_success 'autosetuprebase remote on an untracked remote branch' '
@@ -758,7 +758,7 @@ test_expect_success 'autosetuprebase remote on an untracked remote branch' '
 	git branch --no-track myr19 local/master &&
 	test "z$(git config branch.myr19.remote)" = z &&
 	test "z$(git config branch.myr19.merge)" = z &&
-	test "z$(git config branch.myr19.rebase)" = z
+	test "z$(git config branch.myr19.pullmode)" = z
 '
 
 test_expect_success 'autosetuprebase always on an untracked remote branch' '
@@ -769,7 +769,7 @@ test_expect_success 'autosetuprebase always on an untracked remote branch' '
 	git branch --no-track myr20 local/master &&
 	test "z$(git config branch.myr20.remote)" = z &&
 	test "z$(git config branch.myr20.merge)" = z &&
-	test "z$(git config branch.myr20.rebase)" = z
+	test "z$(git config branch.myr20.pullmode)" = z
 '
 
 test_expect_success 'autosetuprebase always on detached HEAD' '
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 5e67035..0e91b67 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -191,8 +191,8 @@ test_expect_success 'clone respects global branch.autosetuprebase' '
 		rm -fr dst &&
 		git clone src dst &&
 		cd dst &&
-		actual="z$(git config branch.master.rebase)" &&
-		test ztrue = $actual
+		actual="$(git config branch.master.pullmode)" &&
+		test "$actual" = rebase
 	)
 '
 
-- 
1.9.2+fc1.3.gade8541

Re: [PATCH v5 1/6] pull: rename pull.rename to pull.mode

[Marat Radchenko]

Felipe Contreras wrote
> [PATCH v5 1/6] pull: rename pull.rename to pull.mode

s/pull.rename/pull.rebase/



--
View this message in context: http://git.661346.n2.nabble.com/PATCH-v5-0-6-Reject-non-ff-pulls-by-default-tp7609118p7609129.html
Sent from the git mailing list archive at Nabble.com.

Re: [PATCH v5 1/6] pull: rename pull.rename to pull.mode

[Felipe Contreras]

Marat Radchenko wrote:
> Felipe Contreras wrote
> > [PATCH v5 1/6] pull: rename pull.rename to pull.mode
> 
> s/pull.rename/pull.rebase/

Right. Will fix.

-- 
Felipe Contreras

Re: [PATCH v5 1/6] pull: rename pull.rename to pull.mode

[Philip Oakley]

From: "Felipe Contreras" <felipe.contreras@gmail.com>
> Also 'branch.<name>.rebase' to 'branch.<name>.pullmode'.

Sorry I haven't commented earlier. Because the 0/6 explanation isn't a
commit, a few extra words would be useful to capture what the 0/6 cover
letter said to start the patch series cleanly/clearly e.g. start with

    Begin the "Reject non-ff pulls by default" process by creating new
    config variables which will allow extra options, to replace the old 
pull
    configuration options.

I didn't immediately grasp why the 'replacement' was happening, rather
than it being a creation and a deprecation.

>
> This way 'pull.mode' can be set to 'merge', and the default can be
> something else.
>
> The old configurations still work, but get deprecated.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> Documentation/config.txt   | 34 +++++++++++++++++-----------------
> Documentation/git-pull.txt |  2 +-
> branch.c                   |  4 ++--
> builtin/remote.c           | 14 ++++++++++++--
> git-pull.sh                | 39
> +++++++++++++++++++++++++++++++++++++--
> t/t3200-branch.sh          | 40
> ++++++++++++++++++++--------------------
> t/t5601-clone.sh           |  4 ++--
> 7 files changed, 91 insertions(+), 46 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index c26a7c8..5978d35 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -708,7 +708,7 @@ branch.autosetupmerge::
> branch.autosetuprebase::
>  When a new branch is created with 'git branch' or 'git checkout'
>  that tracks another branch, this variable tells Git to set
> - up pull to rebase instead of merge (see "branch.<name>.rebase").
> + up pull to rebase instead of merge (see "branch.<name>.pullmode").
>  When `never`, rebase is never automatically set to true.
>  When `local`, rebase is set to true for tracked branches of
>  other local branches.
> @@ -764,15 +764,15 @@ branch.<name>.mergeoptions::
>  option values containing whitespace characters are currently not
>  supported.
>
> -branch.<name>.rebase::
> - When true, rebase the branch <name> on top of the fetched branch,
> - instead of merging the default branch from the default remote when
> - "git pull" is run. See "pull.rebase" for doing this in a non
> - branch-specific manner.
> +branch.<name>.pullmode::
> + When "git pull" is run, this determines if it would either merge or
> + rebase the fetched branch. The possible values are 'merge',
> + 'rebase', and 'rebase-preserve'. See "pull.mode" for doing this in a
> + non branch-specific manner.

I'd think it useful to add that:
    branch.<name>.rebase is deprecated.
given the large amount of internet cruft about this older config
variable name

> +
> - When preserve, also pass `--preserve-merges` along to 'git rebase'
> - so that locally committed merge commits will not be flattened
> - by running 'git pull'.
> + When 'rebase-preserve', also pass `--preserve-merges` along to
> + 'git rebase' so that locally committed merge commits will not be
> + flattened by running 'git pull'.
> +
> *NOTE*: this is a possibly dangerous operation; do *not* use
> it unless you understand the implications (see linkgit:git-rebase[1]
> @@ -1881,15 +1881,15 @@ pretty.<name>::
>  Note that an alias with the same name as a built-in format
>  will be silently ignored.
>
> -pull.rebase::
> - When true, rebase branches on top of the fetched branch, instead
> - of merging the default branch from the default remote when "git
> - pull" is run. See "branch.<name>.rebase" for setting this on a
> - per-branch basis.
> +pull.mode::
> + When "git pull" is run, this determines if it would either merge or
> + rebase the fetched branch. The possible values are 'merge',
> + 'rebase', and 'rebase-preserve'. See "branch.<name>.pullmode" for
> doing
> + this in a non branch-specific manner.

Add?:
    pull.rebase is deprecated.

> +
> - When preserve, also pass `--preserve-merges` along to 'git rebase'
> - so that locally committed merge commits will not be flattened
> - by running 'git pull'.
> + When 'rebase-preserve', also pass `--preserve-merges` along to
> + 'git rebase' so that locally committed merge commits will not be
> + flattened by running 'git pull'.
> +
> *NOTE*: this is a possibly dangerous operation; do *not* use
> it unless you understand the implications (see linkgit:git-rebase[1]
> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
> index 200eb22..9a91b9f 100644
> --- a/Documentation/git-pull.txt
> +++ b/Documentation/git-pull.txt
> @@ -117,7 +117,7 @@ locally created merge commits will not be
> flattened.
> +
> When false, merge the current branch into the upstream branch.
> +
> -See `pull.rebase`, `branch.<name>.rebase` and
> `branch.autosetuprebase` in
> +See `pull.mode`, `branch.<name>.pullmode` and
> `branch.autosetuprebase` in
> linkgit:git-config[1] if you want to make `git pull` always use
> `--rebase` instead of merging.
> +
> diff --git a/branch.c b/branch.c
> index 723a36b..63ce671 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -71,8 +71,8 @@ void install_branch_config(int flag, const char
> *local, const char *origin, cons
>
>  if (rebasing) {
>  strbuf_reset(&key);
> - strbuf_addf(&key, "branch.%s.rebase", local);
> - git_config_set(key.buf, "true");
> + strbuf_addf(&key, "branch.%s.pullmode", local);
> + git_config_set(key.buf, "rebase");
>  }
>  strbuf_release(&key);
>
> diff --git a/builtin/remote.c b/builtin/remote.c
> index b3ab4cf..46d3c4d 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -264,7 +264,7 @@ static int config_read_branches(const char *key,
> const char *value, void *cb)
>  char *name;
>  struct string_list_item *item;
>  struct branch_info *info;
> - enum { REMOTE, MERGE, REBASE } type;
> + enum { REMOTE, MERGE, REBASE, PULLMODE } type;
>
>  key += 7;
>  if (ends_with(key, ".remote")) {
> @@ -276,6 +276,9 @@ static int config_read_branches(const char *key,
> const char *value, void *cb)
>  } else if (ends_with(key, ".rebase")) {
>  name = xstrndup(key, strlen(key) - 7);
>  type = REBASE;
> + } else if (ends_with(key, ".pullmode")) {
> + name = xstrndup(key, strlen(key) - 9);
> + type = PULLMODE;
>  } else
>  return 0;
>
> @@ -299,12 +302,19 @@ static int config_read_branches(const char *key,
> const char *value, void *cb)
>  space = strchr(value, ' ');
>  }
>  string_list_append(&info->merge, xstrdup(value));
> - } else {
> + } else if (type == REBASE) {
>  int v = git_config_maybe_bool(orig_key, value);
>  if (v >= 0)
>  info->rebase = v;
>  else if (!strcmp(value, "preserve"))
>  info->rebase = 1;
> + } else {
> + if (!strcmp(value, "rebase"))
> + info->rebase = 1;
> + else if (!strcmp(value, "merge"))
> + info->rebase = 0;
> + else if (!strcmp(value, "rebase-preserve"))
> + info->rebase = 1;
>  }
>  }
>  return 0;
> diff --git a/git-pull.sh b/git-pull.sh
> index 0a5aa2c..d4e25f1 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -15,6 +15,10 @@ require_work_tree_exists
> cd_to_toplevel
>
>
> +warn () {
> + printf >&2 '%s\n' "$*"
> +}
> +
> die_conflict () {
>     git diff-index --cached --name-status -r --ignore-submodules
> HEAD --
>     if [ $(git config --bool --get advice.resolveConflict || echo
> true) = "true" ]; then
> @@ -47,10 +51,41 @@ log_arg= verbosity= progress= recurse_submodules=
> verify_signatures=
> merge_args= edit= rebase_args=
> curr_branch=$(git symbolic-ref -q HEAD)
> curr_branch_short="${curr_branch#refs/heads/}"
> -rebase=$(bool_or_string_config branch.$curr_branch_short.rebase)
> +mode=$(git config branch.${curr_branch_short}.pullmode)
> +if test -z "$mode"
> +then
> + mode=$(git config pull.mode)
> +fi
> +case "$mode" in
> +merge)
> + rebase="false"
> + ;;
> +rebase)
> + rebase="true"
> + ;;
> +rebase-preserve)
> + rebase="preserve"
> + ;;
> +'')
> + ;;
> +*)
> + echo "Invalid value for 'mode'"
> + usage
> + exit 1
> + ;;
> +esac
> if test -z "$rebase"
> then
> - rebase=$(bool_or_string_config pull.rebase)
> + rebase=$(bool_or_string_config branch.$curr_branch_short.rebase)
> + if test -z "$rebase"
> + then
> + rebase=$(bool_or_string_config pull.rebase)
> + fi
> + if test -n "$rebase"
> + then
> + warn "$(gettext "The configurations pull.rebase and
> branch.<name>.rebase are deprecated.")"
> + warn "$(gettext "Please use pull.mode and branch.<name>.pullmode
> instead.")"
> + fi
> fi
> dry_run=
> while :
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index fcdb867..b79aa75 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -559,7 +559,7 @@ test_expect_success 'autosetuprebase local on a
> tracked local branch' '
>  git branch --track myr1 mybase &&
>  test "$(git config branch.myr1.remote)" = . &&
>  test "$(git config branch.myr1.merge)" = refs/heads/mybase &&
> - test "$(git config branch.myr1.rebase)" = true
> + test "$(git config branch.myr1.pullmode)" = rebase
> '
>
> test_expect_success 'autosetuprebase always on a tracked local branch'
> '
> @@ -571,7 +571,7 @@ test_expect_success 'autosetuprebase always on a
> tracked local branch' '
>  git branch --track myr2 mybase &&
>  test "$(git config branch.myr2.remote)" = . &&
>  test "$(git config branch.myr2.merge)" = refs/heads/mybase &&
> - test "$(git config branch.myr2.rebase)" = true
> + test "$(git config branch.myr2.pullmode)" = rebase
> '
>
> test_expect_success 'autosetuprebase remote on a tracked local branch'
> '
> @@ -583,7 +583,7 @@ test_expect_success 'autosetuprebase remote on a
> tracked local branch' '
>  git branch --track myr3 mybase2 &&
>  test "$(git config branch.myr3.remote)" = . &&
>  test "$(git config branch.myr3.merge)" = refs/heads/mybase2 &&
> - ! test "$(git config branch.myr3.rebase)" = true
> + ! test "$(git config branch.myr3.pullmode)" = rebase
> '
>
> test_expect_success 'autosetuprebase never on a tracked local branch'
> '
> @@ -595,7 +595,7 @@ test_expect_success 'autosetuprebase never on a
> tracked local branch' '
>  git branch --track myr4 mybase2 &&
>  test "$(git config branch.myr4.remote)" = . &&
>  test "$(git config branch.myr4.merge)" = refs/heads/mybase2 &&
> - ! test "$(git config branch.myr4.rebase)" = true
> + ! test "$(git config branch.myr4.pullmode)" = rebase
> '
>
> test_expect_success 'autosetuprebase local on a tracked remote branch'
> '
> @@ -606,7 +606,7 @@ test_expect_success 'autosetuprebase local on a
> tracked remote branch' '
>  git branch --track myr5 local/master &&
>  test "$(git config branch.myr5.remote)" = local &&
>  test "$(git config branch.myr5.merge)" = refs/heads/master &&
> - ! test "$(git config branch.myr5.rebase)" = true
> + ! test "$(git config branch.myr5.pullmode)" = rebase
> '
>
> test_expect_success 'autosetuprebase never on a tracked remote branch'
> '
> @@ -617,7 +617,7 @@ test_expect_success 'autosetuprebase never on a
> tracked remote branch' '
>  git branch --track myr6 local/master &&
>  test "$(git config branch.myr6.remote)" = local &&
>  test "$(git config branch.myr6.merge)" = refs/heads/master &&
> - ! test "$(git config branch.myr6.rebase)" = true
> + ! test "$(git config branch.myr6.pullmode)" = rebase
> '
>
> test_expect_success 'autosetuprebase remote on a tracked remote
> branch' '
> @@ -628,7 +628,7 @@ test_expect_success 'autosetuprebase remote on a
> tracked remote branch' '
>  git branch --track myr7 local/master &&
>  test "$(git config branch.myr7.remote)" = local &&
>  test "$(git config branch.myr7.merge)" = refs/heads/master &&
> - test "$(git config branch.myr7.rebase)" = true
> + test "$(git config branch.myr7.pullmode)" = rebase
> '
>
> test_expect_success 'autosetuprebase always on a tracked remote
> branch' '
> @@ -639,7 +639,7 @@ test_expect_success 'autosetuprebase always on a
> tracked remote branch' '
>  git branch --track myr8 local/master &&
>  test "$(git config branch.myr8.remote)" = local &&
>  test "$(git config branch.myr8.merge)" = refs/heads/master &&
> - test "$(git config branch.myr8.rebase)" = true
> + test "$(git config branch.myr8.pullmode)" = rebase
> '
>
> test_expect_success 'autosetuprebase unconfigured on a tracked remote
> branch' '
> @@ -650,7 +650,7 @@ test_expect_success 'autosetuprebase unconfigured
> on a tracked remote branch' '
>  git branch --track myr9 local/master &&
>  test "$(git config branch.myr9.remote)" = local &&
>  test "$(git config branch.myr9.merge)" = refs/heads/master &&
> - test "z$(git config branch.myr9.rebase)" = z
> + test "z$(git config branch.myr9.pullmode)" = z
> '
>
> test_expect_success 'autosetuprebase unconfigured on a tracked local
> branch' '
> @@ -661,7 +661,7 @@ test_expect_success 'autosetuprebase unconfigured
> on a tracked local branch' '
>  git branch --track myr10 mybase2 &&
>  test "$(git config branch.myr10.remote)" = . &&
>  test "$(git config branch.myr10.merge)" = refs/heads/mybase2 &&
> - test "z$(git config branch.myr10.rebase)" = z
> + test "z$(git config branch.myr10.pullmode)" = z
> '
>
> test_expect_success 'autosetuprebase unconfigured on untracked local
> branch' '
> @@ -671,7 +671,7 @@ test_expect_success 'autosetuprebase unconfigured
> on untracked local branch' '
>  git branch --no-track myr11 mybase2 &&
>  test "z$(git config branch.myr11.remote)" = z &&
>  test "z$(git config branch.myr11.merge)" = z &&
> - test "z$(git config branch.myr11.rebase)" = z
> + test "z$(git config branch.myr11.pullmode)" = z
> '
>
> test_expect_success 'autosetuprebase unconfigured on untracked remote
> branch' '
> @@ -681,7 +681,7 @@ test_expect_success 'autosetuprebase unconfigured
> on untracked remote branch' '
>  git branch --no-track myr12 local/master &&
>  test "z$(git config branch.myr12.remote)" = z &&
>  test "z$(git config branch.myr12.merge)" = z &&
> - test "z$(git config branch.myr12.rebase)" = z
> + test "z$(git config branch.myr12.pullmode)" = z
> '
>
> test_expect_success 'autosetuprebase never on an untracked local
> branch' '
> @@ -692,7 +692,7 @@ test_expect_success 'autosetuprebase never on an
> untracked local branch' '
>  git branch --no-track myr13 mybase2 &&
>  test "z$(git config branch.myr13.remote)" = z &&
>  test "z$(git config branch.myr13.merge)" = z &&
> - test "z$(git config branch.myr13.rebase)" = z
> + test "z$(git config branch.myr13.pullmode)" = z
> '
>
> test_expect_success 'autosetuprebase local on an untracked local
> branch' '
> @@ -703,7 +703,7 @@ test_expect_success 'autosetuprebase local on an
> untracked local branch' '
>  git branch --no-track myr14 mybase2 &&
>  test "z$(git config branch.myr14.remote)" = z &&
>  test "z$(git config branch.myr14.merge)" = z &&
> - test "z$(git config branch.myr14.rebase)" = z
> + test "z$(git config branch.myr14.pullmode)" = z
> '
>
> test_expect_success 'autosetuprebase remote on an untracked local
> branch' '
> @@ -714,7 +714,7 @@ test_expect_success 'autosetuprebase remote on an
> untracked local branch' '
>  git branch --no-track myr15 mybase2 &&
>  test "z$(git config branch.myr15.remote)" = z &&
>  test "z$(git config branch.myr15.merge)" = z &&
> - test "z$(git config branch.myr15.rebase)" = z
> + test "z$(git config branch.myr15.pullmode)" = z
> '
>
> test_expect_success 'autosetuprebase always on an untracked local
> branch' '
> @@ -725,7 +725,7 @@ test_expect_success 'autosetuprebase always on an
> untracked local branch' '
>  git branch --no-track myr16 mybase2 &&
>  test "z$(git config branch.myr16.remote)" = z &&
>  test "z$(git config branch.myr16.merge)" = z &&
> - test "z$(git config branch.myr16.rebase)" = z
> + test "z$(git config branch.myr16.pullmode)" = z
> '
>
> test_expect_success 'autosetuprebase never on an untracked remote
> branch' '
> @@ -736,7 +736,7 @@ test_expect_success 'autosetuprebase never on an
> untracked remote branch' '
>  git branch --no-track myr17 local/master &&
>  test "z$(git config branch.myr17.remote)" = z &&
>  test "z$(git config branch.myr17.merge)" = z &&
> - test "z$(git config branch.myr17.rebase)" = z
> + test "z$(git config branch.myr17.pullmode)" = z
> '
>
> test_expect_success 'autosetuprebase local on an untracked remote
> branch' '
> @@ -747,7 +747,7 @@ test_expect_success 'autosetuprebase local on an
> untracked remote branch' '
>  git branch --no-track myr18 local/master &&
>  test "z$(git config branch.myr18.remote)" = z &&
>  test "z$(git config branch.myr18.merge)" = z &&
> - test "z$(git config branch.myr18.rebase)" = z
> + test "z$(git config branch.myr18.pullmode)" = z
> '
>
> test_expect_success 'autosetuprebase remote on an untracked remote
> branch' '
> @@ -758,7 +758,7 @@ test_expect_success 'autosetuprebase remote on an
> untracked remote branch' '
>  git branch --no-track myr19 local/master &&
>  test "z$(git config branch.myr19.remote)" = z &&
>  test "z$(git config branch.myr19.merge)" = z &&
> - test "z$(git config branch.myr19.rebase)" = z
> + test "z$(git config branch.myr19.pullmode)" = z
> '
>
> test_expect_success 'autosetuprebase always on an untracked remote
> branch' '
> @@ -769,7 +769,7 @@ test_expect_success 'autosetuprebase always on an
> untracked remote branch' '
>  git branch --no-track myr20 local/master &&
>  test "z$(git config branch.myr20.remote)" = z &&
>  test "z$(git config branch.myr20.merge)" = z &&
> - test "z$(git config branch.myr20.rebase)" = z
> + test "z$(git config branch.myr20.pullmode)" = z
> '
>
> test_expect_success 'autosetuprebase always on detached HEAD' '
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index 5e67035..0e91b67 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -191,8 +191,8 @@ test_expect_success 'clone respects global
> branch.autosetuprebase' '
>  rm -fr dst &&
>  git clone src dst &&
>  cd dst &&
> - actual="z$(git config branch.master.rebase)" &&
> - test ztrue = $actual
> + actual="$(git config branch.master.pullmode)" &&
> + test "$actual" = rebase
>  )
> '
>
> -- 
> 1.9.2+fc1.3.gade8541
>
>

Re: [PATCH v5 1/6] pull: rename pull.rename to pull.mode

[Felipe Contreras]

Philip Oakley wrote:
> From: "Felipe Contreras" <felipe.contreras@gmail.com>
> > Also 'branch.<name>.rebase' to 'branch.<name>.pullmode'.
> 
> Sorry I haven't commented earlier. Because the 0/6 explanation isn't a
> commit, a few extra words would be useful to capture what the 0/6 cover
> letter said to start the patch series cleanly/clearly e.g. start with
> 
>     Begin the "Reject non-ff pulls by default" process by creating new
>     config variables which will allow extra options, to replace the old 
> pull
>     configuration options.
> 
> I didn't immediately grasp why the 'replacement' was happening, rather
> than it being a creation and a deprecation.

How about this:

> > This way 'pull.mode' can be set to 'merge', and the default can be
> > something else.

This will allow us to eventually add a 'merge-ff-only' option, and
eventually set it as default to solve the problem of not rejecting
non-ff pulls by default.

> > The old configurations still work, but get deprecated.
> >
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

> > --- a/Documentation/config.txt
> > +++ b/Documentation/config.txt

> > @@ -764,15 +764,15 @@ branch.<name>.mergeoptions::
> >  option values containing whitespace characters are currently not
> >  supported.
> >
> > -branch.<name>.rebase::
> > - When true, rebase the branch <name> on top of the fetched branch,
> > - instead of merging the default branch from the default remote when
> > - "git pull" is run. See "pull.rebase" for doing this in a non
> > - branch-specific manner.
> > +branch.<name>.pullmode::
> > + When "git pull" is run, this determines if it would either merge or
> > + rebase the fetched branch. The possible values are 'merge',
> > + 'rebase', and 'rebase-preserve'. See "pull.mode" for doing this in a
> > + non branch-specific manner.
> 
> I'd think it useful to add that:
>     branch.<name>.rebase is deprecated.
> given the large amount of internet cruft about this older config
> variable name

I don't recall deprecated configurations in the documentation, but I
don't see a problem adding that either.

-- 
Felipe Contreras

Re: [PATCH v5 1/6] pull: rename pull.rename to pull.mode

[Philip Oakley]

From: "Felipe Contreras" <felipe.contreras@gmail.com>
> Philip Oakley wrote:
>> From: "Felipe Contreras" <felipe.contreras@gmail.com>
>> > Also 'branch.<name>.rebase' to 'branch.<name>.pullmode'.
>>
>> Sorry I haven't commented earlier. Because the 0/6 explanation isn't 
>> a
>> commit, a few extra words would be useful to capture what the 0/6 
>> cover
>> letter said to start the patch series cleanly/clearly e.g. start with
>>
>>     Begin the "Reject non-ff pulls by default" process by creating 
>> new
>>     config variables which will allow extra options, to replace the 
>> old
>> pull
>>     configuration options.
>>
>> I didn't immediately grasp why the 'replacement' was happening, 
>> rather
>> than it being a creation and a deprecation.
>
> How about this:
>
>> > This way 'pull.mode' can be set to 'merge', and the default can be
>> > something else.

My initial problem was the word sequence order which, in the original, 
started with the 'replace' statement, which tripped me up. So I was 
looking for some preparatory wording to set the scene and avoid the 
trip.
(I'm assuming you were noting that you already had the subsequent "This 
way 'pull.mode'.." statement, unfortunately I'd already tripped up by 
then in the original, hence the suggestions for the scene setting)

>
> This will allow us to eventually add a 'merge-ff-only' option, and
> eventually set it as default to solve the problem of not rejecting
> non-ff pulls by default.
>
>> > The old configurations still work, but get deprecated.
>> >
>> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>
>> > --- a/Documentation/config.txt
>> > +++ b/Documentation/config.txt
>
>> > @@ -764,15 +764,15 @@ branch.<name>.mergeoptions::
>> >  option values containing whitespace characters are currently not
>> >  supported.
>> >
>> > -branch.<name>.rebase::
>> > - When true, rebase the branch <name> on top of the fetched branch,
>> > - instead of merging the default branch from the default remote 
>> > when
>> > - "git pull" is run. See "pull.rebase" for doing this in a non
>> > - branch-specific manner.
>> > +branch.<name>.pullmode::
>> > + When "git pull" is run, this determines if it would either merge 
>> > or
>> > + rebase the fetched branch. The possible values are 'merge',
>> > + 'rebase', and 'rebase-preserve'. See "pull.mode" for doing this 
>> > in a
>> > + non branch-specific manner.
>>
>> I'd think it useful to add that:
>>     branch.<name>.rebase is deprecated.
>> given the large amount of internet cruft about this older config
>> variable name
>
> I don't recall deprecated configurations in the documentation, but I
> don't see a problem adding that either.

I'm sure that Junio will be able to form an opinion, which could go 
either way. On the one hand we try to avoid endless deprecation notices 
that then never get removed. On the other, we have the internet advice 
to set those old variables, so its worth associating them with their 
newer replacement with the warning. It'll be some else's judgment call 
at the end of the day.

>
> -- 
> Felipe Contreras
>
--
Philip 

Re: [PATCH v5 1/6] pull: rename pull.rename to pull.mode

[Richard Hansen]

On 2014-04-29 07:17, Felipe Contreras wrote:
> Also 'branch.<name>.rebase' to 'branch.<name>.pullmode'.
> 
> This way 'pull.mode' can be set to 'merge', and the default can be
> something else.
> 
> The old configurations still work, but get deprecated.

Should users be warned if both pull.rebase and pull.mode are set?  (Also
if both branch.<name>.rebase and branch.<name>.pullmode are set.)

Hmm, thinking about it more, I think I prefer it to *not* warn if both
are set (as is currently coded).  If the user set *mode then presumably
they've read the documentation and know that *mode is the right way to
configure the behavior, so a warning would only serve to badger them
into cleaning up their config file.  And it would annoy users that keep
their ~/.gitconfig in a dotfiles repo shared between machines with
different versions of Git.

I prefer a deprecation note in the documentation as Philip suggests, but
I'm OK without one too.

-Richard


> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  Documentation/config.txt   | 34 +++++++++++++++++-----------------
>  Documentation/git-pull.txt |  2 +-
>  branch.c                   |  4 ++--
>  builtin/remote.c           | 14 ++++++++++++--
>  git-pull.sh                | 39 +++++++++++++++++++++++++++++++++++++--
>  t/t3200-branch.sh          | 40 ++++++++++++++++++++--------------------
>  t/t5601-clone.sh           |  4 ++--
>  7 files changed, 91 insertions(+), 46 deletions(-)
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index c26a7c8..5978d35 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -708,7 +708,7 @@ branch.autosetupmerge::
>  branch.autosetuprebase::
>  	When a new branch is created with 'git branch' or 'git checkout'
>  	that tracks another branch, this variable tells Git to set
> -	up pull to rebase instead of merge (see "branch.<name>.rebase").
> +	up pull to rebase instead of merge (see "branch.<name>.pullmode").
>  	When `never`, rebase is never automatically set to true.
>  	When `local`, rebase is set to true for tracked branches of
>  	other local branches.
> @@ -764,15 +764,15 @@ branch.<name>.mergeoptions::
>  	option values containing whitespace characters are currently not
>  	supported.
>  
> -branch.<name>.rebase::
> -	When true, rebase the branch <name> on top of the fetched branch,
> -	instead of merging the default branch from the default remote when
> -	"git pull" is run. See "pull.rebase" for doing this in a non
> -	branch-specific manner.
> +branch.<name>.pullmode::
> +	When "git pull" is run, this determines if it would either merge or
> +	rebase the fetched branch. The possible values are 'merge',
> +	'rebase', and 'rebase-preserve'. See "pull.mode" for doing this in a
> +	non branch-specific manner.
>  +
> -	When preserve, also pass `--preserve-merges` along to 'git rebase'
> -	so that locally committed merge commits will not be flattened
> -	by running 'git pull'.
> +	When 'rebase-preserve', also pass `--preserve-merges` along to
> +	'git rebase' so that locally committed merge commits will not be
> +	flattened by running 'git pull'.
>  +
>  *NOTE*: this is a possibly dangerous operation; do *not* use
>  it unless you understand the implications (see linkgit:git-rebase[1]
> @@ -1881,15 +1881,15 @@ pretty.<name>::
>  	Note that an alias with the same name as a built-in format
>  	will be silently ignored.
>  
> -pull.rebase::
> -	When true, rebase branches on top of the fetched branch, instead
> -	of merging the default branch from the default remote when "git
> -	pull" is run. See "branch.<name>.rebase" for setting this on a
> -	per-branch basis.
> +pull.mode::
> +	When "git pull" is run, this determines if it would either merge or
> +	rebase the fetched branch. The possible values are 'merge',
> +	'rebase', and 'rebase-preserve'. See "branch.<name>.pullmode" for doing
> +	this in a non branch-specific manner.
>  +
> -	When preserve, also pass `--preserve-merges` along to 'git rebase'
> -	so that locally committed merge commits will not be flattened
> -	by running 'git pull'.
> +	When 'rebase-preserve', also pass `--preserve-merges` along to
> +	'git rebase' so that locally committed merge commits will not be
> +	flattened by running 'git pull'.
>  +
>  *NOTE*: this is a possibly dangerous operation; do *not* use
>  it unless you understand the implications (see linkgit:git-rebase[1]
> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
> index 200eb22..9a91b9f 100644
> --- a/Documentation/git-pull.txt
> +++ b/Documentation/git-pull.txt
> @@ -117,7 +117,7 @@ locally created merge commits will not be flattened.
>  +
>  When false, merge the current branch into the upstream branch.
>  +
> -See `pull.rebase`, `branch.<name>.rebase` and `branch.autosetuprebase` in
> +See `pull.mode`, `branch.<name>.pullmode` and `branch.autosetuprebase` in
>  linkgit:git-config[1] if you want to make `git pull` always use
>  `--rebase` instead of merging.
>  +
> diff --git a/branch.c b/branch.c
> index 723a36b..63ce671 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -71,8 +71,8 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
>  
>  	if (rebasing) {
>  		strbuf_reset(&key);
> -		strbuf_addf(&key, "branch.%s.rebase", local);
> -		git_config_set(key.buf, "true");
> +		strbuf_addf(&key, "branch.%s.pullmode", local);
> +		git_config_set(key.buf, "rebase");
>  	}
>  	strbuf_release(&key);
>  
> diff --git a/builtin/remote.c b/builtin/remote.c
> index b3ab4cf..46d3c4d 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -264,7 +264,7 @@ static int config_read_branches(const char *key, const char *value, void *cb)
>  		char *name;
>  		struct string_list_item *item;
>  		struct branch_info *info;
> -		enum { REMOTE, MERGE, REBASE } type;
> +		enum { REMOTE, MERGE, REBASE, PULLMODE } type;
>  
>  		key += 7;
>  		if (ends_with(key, ".remote")) {
> @@ -276,6 +276,9 @@ static int config_read_branches(const char *key, const char *value, void *cb)
>  		} else if (ends_with(key, ".rebase")) {
>  			name = xstrndup(key, strlen(key) - 7);
>  			type = REBASE;
> +		} else if (ends_with(key, ".pullmode")) {
> +			name = xstrndup(key, strlen(key) - 9);
> +			type = PULLMODE;
>  		} else
>  			return 0;
>  
> @@ -299,12 +302,19 @@ static int config_read_branches(const char *key, const char *value, void *cb)
>  				space = strchr(value, ' ');
>  			}
>  			string_list_append(&info->merge, xstrdup(value));
> -		} else {
> +		} else if (type == REBASE) {
>  			int v = git_config_maybe_bool(orig_key, value);
>  			if (v >= 0)
>  				info->rebase = v;
>  			else if (!strcmp(value, "preserve"))
>  				info->rebase = 1;
> +		} else {
> +			if (!strcmp(value, "rebase"))
> +				info->rebase = 1;
> +			else if (!strcmp(value, "merge"))
> +				info->rebase = 0;
> +			else if (!strcmp(value, "rebase-preserve"))
> +				info->rebase = 1;
>  		}
>  	}
>  	return 0;
> diff --git a/git-pull.sh b/git-pull.sh
> index 0a5aa2c..d4e25f1 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -15,6 +15,10 @@ require_work_tree_exists
>  cd_to_toplevel
>  
>  
> +warn () {
> +	printf >&2 '%s\n' "$*"
> +}
> +
>  die_conflict () {
>      git diff-index --cached --name-status -r --ignore-submodules HEAD --
>      if [ $(git config --bool --get advice.resolveConflict || echo true) = "true" ]; then
> @@ -47,10 +51,41 @@ log_arg= verbosity= progress= recurse_submodules= verify_signatures=
>  merge_args= edit= rebase_args=
>  curr_branch=$(git symbolic-ref -q HEAD)
>  curr_branch_short="${curr_branch#refs/heads/}"
> -rebase=$(bool_or_string_config branch.$curr_branch_short.rebase)
> +mode=$(git config branch.${curr_branch_short}.pullmode)
> +if test -z "$mode"
> +then
> +	mode=$(git config pull.mode)
> +fi
> +case "$mode" in
> +merge)
> +	rebase="false"
> +	;;
> +rebase)
> +	rebase="true"
> +	;;
> +rebase-preserve)
> +	rebase="preserve"
> +	;;
> +'')
> +	;;
> +*)
> +	echo "Invalid value for 'mode'"
> +	usage
> +	exit 1
> +	;;
> +esac
>  if test -z "$rebase"
>  then
> -	rebase=$(bool_or_string_config pull.rebase)
> +	rebase=$(bool_or_string_config branch.$curr_branch_short.rebase)
> +	if test -z "$rebase"
> +	then
> +		rebase=$(bool_or_string_config pull.rebase)
> +	fi
> +	if test -n "$rebase"
> +	then
> +		warn "$(gettext "The configurations pull.rebase and branch.<name>.rebase are deprecated.")"
> +		warn "$(gettext "Please use pull.mode and branch.<name>.pullmode instead.")"
> +	fi
>  fi
>  dry_run=
>  while :
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index fcdb867..b79aa75 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -559,7 +559,7 @@ test_expect_success 'autosetuprebase local on a tracked local branch' '
>  	git branch --track myr1 mybase &&
>  	test "$(git config branch.myr1.remote)" = . &&
>  	test "$(git config branch.myr1.merge)" = refs/heads/mybase &&
> -	test "$(git config branch.myr1.rebase)" = true
> +	test "$(git config branch.myr1.pullmode)" = rebase
>  '
>  
>  test_expect_success 'autosetuprebase always on a tracked local branch' '
> @@ -571,7 +571,7 @@ test_expect_success 'autosetuprebase always on a tracked local branch' '
>  	git branch --track myr2 mybase &&
>  	test "$(git config branch.myr2.remote)" = . &&
>  	test "$(git config branch.myr2.merge)" = refs/heads/mybase &&
> -	test "$(git config branch.myr2.rebase)" = true
> +	test "$(git config branch.myr2.pullmode)" = rebase
>  '
>  
>  test_expect_success 'autosetuprebase remote on a tracked local branch' '
> @@ -583,7 +583,7 @@ test_expect_success 'autosetuprebase remote on a tracked local branch' '
>  	git branch --track myr3 mybase2 &&
>  	test "$(git config branch.myr3.remote)" = . &&
>  	test "$(git config branch.myr3.merge)" = refs/heads/mybase2 &&
> -	! test "$(git config branch.myr3.rebase)" = true
> +	! test "$(git config branch.myr3.pullmode)" = rebase
>  '
>  
>  test_expect_success 'autosetuprebase never on a tracked local branch' '
> @@ -595,7 +595,7 @@ test_expect_success 'autosetuprebase never on a tracked local branch' '
>  	git branch --track myr4 mybase2 &&
>  	test "$(git config branch.myr4.remote)" = . &&
>  	test "$(git config branch.myr4.merge)" = refs/heads/mybase2 &&
> -	! test "$(git config branch.myr4.rebase)" = true
> +	! test "$(git config branch.myr4.pullmode)" = rebase
>  '
>  
>  test_expect_success 'autosetuprebase local on a tracked remote branch' '
> @@ -606,7 +606,7 @@ test_expect_success 'autosetuprebase local on a tracked remote branch' '
>  	git branch --track myr5 local/master &&
>  	test "$(git config branch.myr5.remote)" = local &&
>  	test "$(git config branch.myr5.merge)" = refs/heads/master &&
> -	! test "$(git config branch.myr5.rebase)" = true
> +	! test "$(git config branch.myr5.pullmode)" = rebase
>  '
>  
>  test_expect_success 'autosetuprebase never on a tracked remote branch' '
> @@ -617,7 +617,7 @@ test_expect_success 'autosetuprebase never on a tracked remote branch' '
>  	git branch --track myr6 local/master &&
>  	test "$(git config branch.myr6.remote)" = local &&
>  	test "$(git config branch.myr6.merge)" = refs/heads/master &&
> -	! test "$(git config branch.myr6.rebase)" = true
> +	! test "$(git config branch.myr6.pullmode)" = rebase
>  '
>  
>  test_expect_success 'autosetuprebase remote on a tracked remote branch' '
> @@ -628,7 +628,7 @@ test_expect_success 'autosetuprebase remote on a tracked remote branch' '
>  	git branch --track myr7 local/master &&
>  	test "$(git config branch.myr7.remote)" = local &&
>  	test "$(git config branch.myr7.merge)" = refs/heads/master &&
> -	test "$(git config branch.myr7.rebase)" = true
> +	test "$(git config branch.myr7.pullmode)" = rebase
>  '
>  
>  test_expect_success 'autosetuprebase always on a tracked remote branch' '
> @@ -639,7 +639,7 @@ test_expect_success 'autosetuprebase always on a tracked remote branch' '
>  	git branch --track myr8 local/master &&
>  	test "$(git config branch.myr8.remote)" = local &&
>  	test "$(git config branch.myr8.merge)" = refs/heads/master &&
> -	test "$(git config branch.myr8.rebase)" = true
> +	test "$(git config branch.myr8.pullmode)" = rebase
>  '
>  
>  test_expect_success 'autosetuprebase unconfigured on a tracked remote branch' '
> @@ -650,7 +650,7 @@ test_expect_success 'autosetuprebase unconfigured on a tracked remote branch' '
>  	git branch --track myr9 local/master &&
>  	test "$(git config branch.myr9.remote)" = local &&
>  	test "$(git config branch.myr9.merge)" = refs/heads/master &&
> -	test "z$(git config branch.myr9.rebase)" = z
> +	test "z$(git config branch.myr9.pullmode)" = z
>  '
>  
>  test_expect_success 'autosetuprebase unconfigured on a tracked local branch' '
> @@ -661,7 +661,7 @@ test_expect_success 'autosetuprebase unconfigured on a tracked local branch' '
>  	git branch --track myr10 mybase2 &&
>  	test "$(git config branch.myr10.remote)" = . &&
>  	test "$(git config branch.myr10.merge)" = refs/heads/mybase2 &&
> -	test "z$(git config branch.myr10.rebase)" = z
> +	test "z$(git config branch.myr10.pullmode)" = z
>  '
>  
>  test_expect_success 'autosetuprebase unconfigured on untracked local branch' '
> @@ -671,7 +671,7 @@ test_expect_success 'autosetuprebase unconfigured on untracked local branch' '
>  	git branch --no-track myr11 mybase2 &&
>  	test "z$(git config branch.myr11.remote)" = z &&
>  	test "z$(git config branch.myr11.merge)" = z &&
> -	test "z$(git config branch.myr11.rebase)" = z
> +	test "z$(git config branch.myr11.pullmode)" = z
>  '
>  
>  test_expect_success 'autosetuprebase unconfigured on untracked remote branch' '
> @@ -681,7 +681,7 @@ test_expect_success 'autosetuprebase unconfigured on untracked remote branch' '
>  	git branch --no-track myr12 local/master &&
>  	test "z$(git config branch.myr12.remote)" = z &&
>  	test "z$(git config branch.myr12.merge)" = z &&
> -	test "z$(git config branch.myr12.rebase)" = z
> +	test "z$(git config branch.myr12.pullmode)" = z
>  '
>  
>  test_expect_success 'autosetuprebase never on an untracked local branch' '
> @@ -692,7 +692,7 @@ test_expect_success 'autosetuprebase never on an untracked local branch' '
>  	git branch --no-track myr13 mybase2 &&
>  	test "z$(git config branch.myr13.remote)" = z &&
>  	test "z$(git config branch.myr13.merge)" = z &&
> -	test "z$(git config branch.myr13.rebase)" = z
> +	test "z$(git config branch.myr13.pullmode)" = z
>  '
>  
>  test_expect_success 'autosetuprebase local on an untracked local branch' '
> @@ -703,7 +703,7 @@ test_expect_success 'autosetuprebase local on an untracked local branch' '
>  	git branch --no-track myr14 mybase2 &&
>  	test "z$(git config branch.myr14.remote)" = z &&
>  	test "z$(git config branch.myr14.merge)" = z &&
> -	test "z$(git config branch.myr14.rebase)" = z
> +	test "z$(git config branch.myr14.pullmode)" = z
>  '
>  
>  test_expect_success 'autosetuprebase remote on an untracked local branch' '
> @@ -714,7 +714,7 @@ test_expect_success 'autosetuprebase remote on an untracked local branch' '
>  	git branch --no-track myr15 mybase2 &&
>  	test "z$(git config branch.myr15.remote)" = z &&
>  	test "z$(git config branch.myr15.merge)" = z &&
> -	test "z$(git config branch.myr15.rebase)" = z
> +	test "z$(git config branch.myr15.pullmode)" = z
>  '
>  
>  test_expect_success 'autosetuprebase always on an untracked local branch' '
> @@ -725,7 +725,7 @@ test_expect_success 'autosetuprebase always on an untracked local branch' '
>  	git branch --no-track myr16 mybase2 &&
>  	test "z$(git config branch.myr16.remote)" = z &&
>  	test "z$(git config branch.myr16.merge)" = z &&
> -	test "z$(git config branch.myr16.rebase)" = z
> +	test "z$(git config branch.myr16.pullmode)" = z
>  '
>  
>  test_expect_success 'autosetuprebase never on an untracked remote branch' '
> @@ -736,7 +736,7 @@ test_expect_success 'autosetuprebase never on an untracked remote branch' '
>  	git branch --no-track myr17 local/master &&
>  	test "z$(git config branch.myr17.remote)" = z &&
>  	test "z$(git config branch.myr17.merge)" = z &&
> -	test "z$(git config branch.myr17.rebase)" = z
> +	test "z$(git config branch.myr17.pullmode)" = z
>  '
>  
>  test_expect_success 'autosetuprebase local on an untracked remote branch' '
> @@ -747,7 +747,7 @@ test_expect_success 'autosetuprebase local on an untracked remote branch' '
>  	git branch --no-track myr18 local/master &&
>  	test "z$(git config branch.myr18.remote)" = z &&
>  	test "z$(git config branch.myr18.merge)" = z &&
> -	test "z$(git config branch.myr18.rebase)" = z
> +	test "z$(git config branch.myr18.pullmode)" = z
>  '
>  
>  test_expect_success 'autosetuprebase remote on an untracked remote branch' '
> @@ -758,7 +758,7 @@ test_expect_success 'autosetuprebase remote on an untracked remote branch' '
>  	git branch --no-track myr19 local/master &&
>  	test "z$(git config branch.myr19.remote)" = z &&
>  	test "z$(git config branch.myr19.merge)" = z &&
> -	test "z$(git config branch.myr19.rebase)" = z
> +	test "z$(git config branch.myr19.pullmode)" = z
>  '
>  
>  test_expect_success 'autosetuprebase always on an untracked remote branch' '
> @@ -769,7 +769,7 @@ test_expect_success 'autosetuprebase always on an untracked remote branch' '
>  	git branch --no-track myr20 local/master &&
>  	test "z$(git config branch.myr20.remote)" = z &&
>  	test "z$(git config branch.myr20.merge)" = z &&
> -	test "z$(git config branch.myr20.rebase)" = z
> +	test "z$(git config branch.myr20.pullmode)" = z
>  '
>  
>  test_expect_success 'autosetuprebase always on detached HEAD' '
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index 5e67035..0e91b67 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -191,8 +191,8 @@ test_expect_success 'clone respects global branch.autosetuprebase' '
>  		rm -fr dst &&
>  		git clone src dst &&
>  		cd dst &&
> -		actual="z$(git config branch.master.rebase)" &&
> -		test ztrue = $actual
> +		actual="$(git config branch.master.pullmode)" &&
> +		test "$actual" = rebase
>  	)
>  '
>  
> 

[PATCH v5 2/6] pull: migrate all the tests to pull.mode

[Felipe Contreras]

And branch.$name.pullmode.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t5505-remote.sh |  2 +-
 t/t5520-pull.sh   | 54 +++++++++++++++++++++++-------------------------------
 2 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index ac79dd9..76376e4 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -181,7 +181,7 @@ test_expect_success 'show' '
 		git branch -d -r origin/master &&
 		git config --add remote.two.url ../two &&
 		git config --add remote.two.pushurl ../three &&
-		git config branch.rebase.rebase true &&
+		git config branch.rebase.pullmode rebase &&
 		git config branch.octopus.merge "topic-a topic-b topic-c" &&
 		(
 			cd ../one &&
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 227d293..01ad17a 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -123,26 +123,26 @@ test_expect_success '--rebase' '
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
 '
-test_expect_success 'pull.rebase' '
+test_expect_success 'pull.mode=rebase' '
 	git reset --hard before-rebase &&
-	test_config pull.rebase true &&
+	test_config pull.mode rebase &&
 	git pull . copy &&
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
 '
 
-test_expect_success 'branch.to-rebase.rebase' '
+test_expect_success 'branch.to-rebase.pullmode=rebase' '
 	git reset --hard before-rebase &&
-	test_config branch.to-rebase.rebase true &&
+	test_config branch.to-rebase.pullmode rebase &&
 	git pull . copy &&
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
 '
 
-test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
+test_expect_success 'branch.to-rebase.pullmode should override pull.mode' '
 	git reset --hard before-rebase &&
-	test_config pull.rebase true &&
-	test_config branch.to-rebase.rebase false &&
+	test_config pull.mode merge &&
+	test_config branch.to-rebase.pullmode merge &&
 	git pull . copy &&
 	test $(git rev-parse HEAD^) != $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
@@ -150,7 +150,7 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
 
 # add a feature branch, keep-merge, that is merged into master, so the
 # test can try preserving the merge commit (or not) with various
-# --rebase flags/pull.rebase settings.
+# --rebase flags/pull.mode settings.
 test_expect_success 'preserve merge setup' '
 	git reset --hard before-rebase &&
 	git checkout -b keep-merge second^ &&
@@ -160,48 +160,40 @@ test_expect_success 'preserve merge setup' '
 	git tag before-preserve-rebase
 '
 
-test_expect_success 'pull.rebase=false create a new merge commit' '
+test_expect_success 'pull.mode=merge create a new merge commit' '
 	git reset --hard before-preserve-rebase &&
-	test_config pull.rebase false &&
+	test_config pull.mode merge &&
 	git pull . copy &&
 	test $(git rev-parse HEAD^1) = $(git rev-parse before-preserve-rebase) &&
 	test $(git rev-parse HEAD^2) = $(git rev-parse copy) &&
 	test file3 = $(git show HEAD:file3.t)
 '
 
-test_expect_success 'pull.rebase=true flattens keep-merge' '
+test_expect_success 'pull.mode=rebase flattens keep-merge' '
 	git reset --hard before-preserve-rebase &&
-	test_config pull.rebase true &&
+	test_config pull.mode rebase &&
 	git pull . copy &&
 	test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&
 	test file3 = $(git show HEAD:file3.t)
 '
 
-test_expect_success 'pull.rebase=1 is treated as true and flattens keep-merge' '
+test_expect_success 'pull.mode=rebase-preserve rebases and merges keep-merge' '
 	git reset --hard before-preserve-rebase &&
-	test_config pull.rebase 1 &&
-	git pull . copy &&
-	test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&
-	test file3 = $(git show HEAD:file3.t)
-'
-
-test_expect_success 'pull.rebase=preserve rebases and merges keep-merge' '
-	git reset --hard before-preserve-rebase &&
-	test_config pull.rebase preserve &&
+	test_config pull.mode rebase-preserve &&
 	git pull . copy &&
 	test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&
 	test $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)
 '
 
-test_expect_success 'pull.rebase=invalid fails' '
+test_expect_success 'pull.mode=invalid fails' '
 	git reset --hard before-preserve-rebase &&
-	test_config pull.rebase invalid &&
+	test_config pull.mode invalid &&
 	! git pull . copy
 '
 
 test_expect_success '--rebase=false create a new merge commit' '
 	git reset --hard before-preserve-rebase &&
-	test_config pull.rebase true &&
+	test_config pull.mode rebase &&
 	git pull --rebase=false . copy &&
 	test $(git rev-parse HEAD^1) = $(git rev-parse before-preserve-rebase) &&
 	test $(git rev-parse HEAD^2) = $(git rev-parse copy) &&
@@ -210,7 +202,7 @@ test_expect_success '--rebase=false create a new merge commit' '
 
 test_expect_success '--rebase=true rebases and flattens keep-merge' '
 	git reset --hard before-preserve-rebase &&
-	test_config pull.rebase preserve &&
+	test_config pull.mode rebase-preserve &&
 	git pull --rebase=true . copy &&
 	test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&
 	test file3 = $(git show HEAD:file3.t)
@@ -218,7 +210,7 @@ test_expect_success '--rebase=true rebases and flattens keep-merge' '
 
 test_expect_success '--rebase=preserve rebases and merges keep-merge' '
 	git reset --hard before-preserve-rebase &&
-	test_config pull.rebase true &&
+	test_config pull.mode rebase &&
 	git pull --rebase=preserve . copy &&
 	test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&
 	test $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)
@@ -229,9 +221,9 @@ test_expect_success '--rebase=invalid fails' '
 	! git pull --rebase=invalid . copy
 '
 
-test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-merge' '
+test_expect_success '--rebase overrides pull.mode=rebase-preserve and flattens keep-merge' '
 	git reset --hard before-preserve-rebase &&
-	test_config pull.rebase preserve &&
+	test_config pull.mode rebase-preserve &&
 	git pull --rebase . copy &&
 	test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&
 	test file3 = $(git show HEAD:file3.t)
@@ -279,7 +271,7 @@ test_expect_success 'rebased upstream + fetch + pull --rebase' '
 
 '
 
-test_expect_success 'pull --rebase dies early with dirty working directory' '
+test_expect_success 'pull --mode=rebase dies early with dirty working directory' '
 
 	git checkout to-rebase &&
 	git update-ref refs/remotes/me/copy copy^ &&
@@ -287,7 +279,7 @@ test_expect_success 'pull --rebase dies early with dirty working directory' '
 	git rebase --onto $COPY copy &&
 	test_config branch.to-rebase.remote me &&
 	test_config branch.to-rebase.merge refs/heads/copy &&
-	test_config branch.to-rebase.rebase true &&
+	test_config branch.to-rebase.pullmode rebase &&
 	echo dirty >> file &&
 	git add file &&
 	test_must_fail git pull &&
-- 
1.9.2+fc1.3.gade8541

[PATCH v5 3/6] pull: refactor $rebase variable into $mode

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-pull.sh | 65 ++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 34 insertions(+), 31 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index d4e25f1..3dfd856 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -57,16 +57,11 @@ then
 	mode=$(git config pull.mode)
 fi
 case "$mode" in
-merge)
-	rebase="false"
-	;;
-rebase)
-	rebase="true"
+merge|rebase|'')
 	;;
 rebase-preserve)
-	rebase="preserve"
-	;;
-'')
+	mode="rebase"
+	rebase_args="--preserve-merges"
 	;;
 *)
 	echo "Invalid value for 'mode'"
@@ -74,7 +69,8 @@ rebase-preserve)
 	exit 1
 	;;
 esac
-if test -z "$rebase"
+
+if test -z "$mode"
 then
 	rebase=$(bool_or_string_config branch.$curr_branch_short.rebase)
 	if test -z "$rebase"
@@ -153,10 +149,10 @@ do
 		rebase="${1#*=}"
 		;;
 	-r|--r|--re|--reb|--reba|--rebas|--rebase)
-		rebase=true
+		mode=rebase
 		;;
 	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
-		rebase=false
+		mode=
 		;;
 	--recurse-submodules)
 		recurse_submodules=--recurse-submodules
@@ -187,19 +183,26 @@ do
 	shift
 done
 
-case "$rebase" in
-preserve)
-	rebase=true
-	rebase_args=--preserve-merges
-	;;
-true|false|'')
-	;;
-*)
-	echo "Invalid value for --rebase, should be true, false, or preserve"
-	usage
-	exit 1
-	;;
-esac
+if test -n "$rebase"
+then
+	case "$rebase" in
+	true)
+		mode="rebase"
+		;;
+	false)
+		mode="merge"
+		;;
+	preserve)
+		mode="rebase"
+		rebase_args=--preserve-merges
+		;;
+	*)
+		echo "Invalid value for --rebase, should be true, false, or preserve"
+		usage
+		exit 1
+		;;
+	esac
+fi
 
 error_on_no_merge_candidates () {
 	exec >&2
@@ -213,7 +216,7 @@ error_on_no_merge_candidates () {
 		esac
 	done
 
-	if test true = "$rebase"
+	if test "$mode" = rebase
 	then
 		op_type=rebase
 		op_prep=against
@@ -226,7 +229,7 @@ error_on_no_merge_candidates () {
 	remote=$(git config "branch.$curr_branch_short.remote")
 
 	if [ $# -gt 1 ]; then
-		if [ "$rebase" = true ]; then
+		if [ "$mode" = rebase ]; then
 			printf "There is no candidate for rebasing against "
 		else
 			printf "There are no candidates for merging "
@@ -249,7 +252,7 @@ error_on_no_merge_candidates () {
 	exit 1
 }
 
-test true = "$rebase" && {
+test "$mode" = rebase && {
 	if ! git rev-parse -q --verify HEAD >/dev/null
 	then
 		# On an unborn branch
@@ -306,7 +309,7 @@ case "$merge_head" in
 	then
 		die "$(gettext "Cannot merge multiple branches into empty head")"
 	fi
-	if test true = "$rebase"
+	if test "$mode" = rebase
 	then
 		die "$(gettext "Cannot rebase onto multiple branches")"
 	fi
@@ -327,7 +330,7 @@ then
 	exit
 fi
 
-if test true = "$rebase"
+if test "$mode" = rebase
 then
 	o=$(git show-branch --merge-base $curr_branch $merge_head $oldremoteref)
 	if test "$oldremoteref" = "$o"
@@ -337,8 +340,8 @@ then
 fi
 
 merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
-case "$rebase" in
-true)
+case "$mode" in
+rebase)
 	eval="git-rebase $diffstat $strategy_args $merge_args $rebase_args $verbosity"
 	eval="$eval --onto $merge_head ${oldremoteref:-$merge_head}"
 	;;
-- 
1.9.2+fc1.3.gade8541

[PATCH v5 4/6] pull: add --merge option

[Felipe Contreras]

Also, deprecate --no-rebase since there's no need for it any more.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-pull.txt | 8 ++++++--
 git-pull.sh                | 6 +++++-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 9a91b9f..767bca3 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -127,8 +127,12 @@ 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.
++
+See `pull.mode`, `branch.<name>.pullmode` in linkgit:git-config[1] if you want
+to make `git pull` always use `--merge`.
 
 Options related to fetching
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/git-pull.sh b/git-pull.sh
index 3dfd856..26e4e55 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -151,8 +151,12 @@ do
 	-r|--r|--re|--reb|--reba|--rebas|--rebase)
 		mode=rebase
 		;;
+	-m|--m|--me|--mer|--merg|--merge)
+		mode=merge
+		;;
 	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
-		mode=
+		mode=merge
+		warn "$(gettext "--no-rebase is deprecated, please use --merge instead")"
 		;;
 	--recurse-submodules)
 		recurse_submodules=--recurse-submodules
-- 
1.9.2+fc1.3.gade8541

[PATCH v5 5/6] pull: add merge-ff-only option

[Felipe Contreras]

It is very typical for Git newcomers to inadvertently create merges and
worst; inadvertently 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 avoid 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 =
merge-ff-only.

Later on this mode can be enabled by default (e.g. in v2.0).

For the full discussion you can read:

http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=225305

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/config.txt |  7 +++++--
 git-pull.sh              | 15 ++++++++++++++-
 t/t5520-pull.sh          | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5978d35..4ebd3b5 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1884,8 +1884,11 @@ pretty.<name>::
 pull.mode::
 	When "git pull" is run, this determines if it would either merge or
 	rebase the fetched branch. The possible values are 'merge',
-	'rebase', and 'rebase-preserve'. See "branch.<name>.pullmode" for doing
-	this in a non branch-specific manner.
+	'rebase', 'merge-ff-only,' and 'rebase-preserve'.
+	If 'merge-ff-only' is specified, the merge will only succeed if it's
+	fast-forward.
+	See "branch.<name>.pullmode" for doing this in a non branch-specific
+	manner.
 +
 	When 'rebase-preserve', also pass `--preserve-merges` along to
 	'git rebase' so that locally committed merge commits will not be
diff --git a/git-pull.sh b/git-pull.sh
index 26e4e55..946cbbe 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -57,7 +57,7 @@ then
 	mode=$(git config pull.mode)
 fi
 case "$mode" in
-merge|rebase|'')
+merge|rebase|merge-ff-only|'')
 	;;
 rebase-preserve)
 	mode="rebase"
@@ -83,6 +83,7 @@ then
 		warn "$(gettext "Please use pull.mode and branch.<name>.pullmode instead.")"
 	fi
 fi
+test -z "$mode" && mode=merge
 dry_run=
 while :
 do
@@ -318,6 +319,18 @@ case "$merge_head" in
 		die "$(gettext "Cannot rebase onto multiple branches")"
 	fi
 	;;
+*)
+	# check if a non-fast-foward merge would be needed
+	merge_head=${merge_head% }
+	if test "$mode" = merge-ff-only -a -z "$no_ff$ff_only${squash#--no-squash}" &&
+		test -n "$orig_head" &&
+		! git merge-base --is-ancestor "$orig_head" "$merge_head" &&
+		! git merge-base --is-ancestor "$merge_head" "$orig_head"
+	then
+		die "$(gettext "The pull was not fast-forward, please either merge or rebase.
+If unsure, run 'git pull --merge'.")"
+	fi
+	;;
 esac
 
 # Pulling into unborn branch: a shorthand for branching off
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 01ad17a..2e2b476 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -365,4 +365,40 @@ test_expect_success 'git pull --rebase against local branch' '
 	test file = "$(cat file2)"
 '
 
+test_expect_success 'git pull fast-forward' '
+	test_when_finished "git checkout master && git branch -D other test" &&
+	test_config pull.mode merge-ff-only &&
+	git checkout -b other master &&
+	>new &&
+	git add new &&
+	git commit -m new &&
+	git checkout -b test -t other &&
+	git reset --hard master &&
+	git pull
+'
+
+test_expect_success 'git pull non-fast-forward' '
+	test_when_finished "git checkout master && git branch -D other test" &&
+	test_config pull.mode merge-ff-only &&
+	git checkout -b other master^ &&
+	>new &&
+	git add new &&
+	git commit -m new &&
+	git checkout -b test -t other &&
+	git reset --hard master &&
+	test_must_fail git pull
+'
+
+test_expect_success 'git pull non-fast-forward (merge)' '
+	test_when_finished "git checkout master && git branch -D other test" &&
+	test_config pull.mode merge-ff-only &&
+	git checkout -b other master^ &&
+	>new &&
+	git add new &&
+	git commit -m new &&
+	git checkout -b test -t other &&
+	git reset --hard master &&
+	git pull --merge
+'
+
 test_done
-- 
1.9.2+fc1.3.gade8541

[PATCH v5 6/6] pull: only allow ff merges by default

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-pull.txt   | 18 ++++++++++++++++++
 git-pull.sh                  |  2 +-
 t/t4013-diff-various.sh      |  2 +-
 t/t5500-fetch-pack.sh        |  2 +-
 t/t5524-pull-msg.sh          |  2 +-
 t/t5700-clone-reference.sh   |  4 ++--
 t/t6022-merge-rename.sh      | 20 ++++++++++----------
 t/t6026-merge-attr.sh        |  2 +-
 t/t6029-merge-subtree.sh     |  6 +++---
 t/t6037-merge-ours-theirs.sh | 10 +++++-----
 10 files changed, 43 insertions(+), 25 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 767bca3..ca8e951 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -23,6 +23,7 @@ More precisely, 'git pull' runs 'git fetch' with the given
 parameters and calls 'git merge' to merge the retrieved branch
 heads into the current branch.
 With `--rebase`, it runs 'git rebase' instead of 'git merge'.
+With `--merge`, it forces the merge, even if it's non-fast forward.
 
 <repository> should be the name of a remote repository as
 passed to linkgit:git-fetch[1].  <refspec> can name an
@@ -41,11 +42,28 @@ Assume the following history exists and the current branch is
 ------------
 	  A---B---C master on origin
 	 /
+    D---E master
+------------
+
+Then `git pull` will merge in a fast-foward way up to the new master.
+
+------------
+    D---E---A---B---C master, origin/master
+------------
+
+However, a non-fast-foward case looks very different.
+
+------------
+	  A---B---C origin/master
+	 /
     D---E---F---G master
 	^
 	origin/master in your repository
 ------------
 
+By default `git pull` will fail on these situations, however, most likely
+you would want to force a merge, which you can do with `git pull --merge`.
+
 Then "`git pull`" will fetch and replay the changes from the remote
 `master` branch since it diverged from the local `master` (i.e., `E`)
 until its current commit (`C`) on top of `master` and record the
diff --git a/git-pull.sh b/git-pull.sh
index 946cbbe..8cf8f68 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -83,7 +83,7 @@ then
 		warn "$(gettext "Please use pull.mode and branch.<name>.pullmode instead.")"
 	fi
 fi
-test -z "$mode" && mode=merge
+test -z "$mode" && mode=merge-ff-only
 dry_run=
 while :
 do
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index e77c09c..1840767 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -64,7 +64,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/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 5b2b1c2..f735cfe 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -259,7 +259,7 @@ test_expect_success 'clone shallow object count' '
 test_expect_success 'pull in shallow repo with missing merge base' '
 	(
 		cd shallow &&
-		test_must_fail git pull --depth 4 .. A
+		test_must_fail git pull --merge --depth 4 .. A
 	)
 '
 
diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh
index 8cccecc..ec9f413 100755
--- a/t/t5524-pull-msg.sh
+++ b/t/t5524-pull-msg.sh
@@ -25,7 +25,7 @@ test_expect_success setup '
 test_expect_success pull '
 (
 	cd cloned &&
-	git pull --log &&
+	git pull --merge --log &&
 	git log -2 &&
 	git cat-file commit HEAD >result &&
 	grep Dollar result
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 6537911..306badf 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -94,7 +94,7 @@ cd "$base_dir"
 
 test_expect_success 'pulling changes from origin' \
 'cd C &&
-git pull origin'
+git pull --merge origin'
 
 cd "$base_dir"
 
@@ -109,7 +109,7 @@ cd "$base_dir"
 
 test_expect_success 'pulling changes from origin' \
 'cd D &&
-git pull origin'
+git pull --merge origin'
 
 cd "$base_dir"
 
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index a89dfbe..f63300f 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -100,7 +100,7 @@ git checkout master'
 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 &&
@@ -118,7 +118,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 &&
@@ -134,7 +134,7 @@ test_expect_success 'pull unrenaming branch into renaming one' \
 '
 	git reset --hard &&
 	git show-branch &&
-	test_expect_code 1 git pull . master &&
+	test_expect_code 1 git pull --merge . master &&
 	git ls-files -u B >b.stages &&
 	test_line_count = 3 b.stages &&
 	git ls-files -s N >n.stages &&
@@ -150,7 +150,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 &&
@@ -170,7 +170,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
 '
 
@@ -180,7 +180,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
 '
 
@@ -190,7 +190,7 @@ test_expect_success 'interference with untracked working tree file' '
 	git checkout -f master &&
 	git tag -f anchor &&
 	git show-branch &&
-	git pull . yellow &&
+	git pull --merge . yellow &&
 	test_path_is_missing M &&
 	git reset --hard anchor
 '
@@ -203,7 +203,7 @@ test_expect_success 'updated working tree file should prevent the merge' '
 	git show-branch &&
 	echo >>M one line addition &&
 	cat M >M.saved &&
-	test_expect_code 128 git pull . yellow &&
+	test_expect_code 128 git pull --merge . yellow &&
 	test_cmp M M.saved &&
 	rm -f M.saved
 '
@@ -217,7 +217,7 @@ test_expect_success 'updated working tree file should prevent the merge' '
 	echo >>M one line addition &&
 	cat M >M.saved &&
 	git update-index M &&
-	test_expect_code 128 git pull . yellow &&
+	test_expect_code 128 git pull --merge . yellow &&
 	test_cmp M M.saved &&
 	rm -f M.saved
 '
@@ -229,7 +229,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 . master &&
+	git pull --merge . master &&
 	test_path_is_file M &&
 	! {
 		git ls-files -s |
diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
index 5e43997..5428f19 100755
--- a/t/t6026-merge-attr.sh
+++ b/t/t6026-merge-attr.sh
@@ -172,7 +172,7 @@ test_expect_success 'up-to-date merge without common ancestor' '
 	test_tick &&
 	(
 		cd repo1 &&
-		git pull ../repo2 master
+		git pull --merge ../repo2 master
 	)
 '
 
diff --git a/t/t6029-merge-subtree.sh b/t/t6029-merge-subtree.sh
index 73fc240..0e626d1 100755
--- a/t/t6029-merge-subtree.sh
+++ b/t/t6029-merge-subtree.sh
@@ -69,7 +69,7 @@ test_expect_success 'merge update' '
 	git checkout -b master2 &&
 	git commit -m "update git-gui" &&
 	cd ../git &&
-	git pull -s subtree gui master2 &&
+	git pull --merge -s subtree gui master2 &&
 	git ls-files -s >actual &&
 	(
 		echo "100644 $o3 0	git-gui/git-gui.sh"
@@ -98,7 +98,7 @@ test_expect_success 'initial ambiguous subtree' '
 test_expect_success 'merge using explicit' '
 	cd ../git &&
 	git reset --hard master2 &&
-	git pull -Xsubtree=git-gui gui master2 &&
+	git pull --merge -Xsubtree=git-gui gui master2 &&
 	git ls-files -s >actual &&
 	(
 		echo "100644 $o3 0	git-gui/git-gui.sh"
@@ -111,7 +111,7 @@ test_expect_success 'merge using explicit' '
 test_expect_success 'merge2 using explicit' '
 	cd ../git &&
 	git reset --hard master2 &&
-	git pull -Xsubtree=git-gui2 gui master2 &&
+	git pull --merge -Xsubtree=git-gui2 gui master2 &&
 	git ls-files -s >actual &&
 	(
 		echo "100644 $o1 0	git-gui/git-gui.sh"
diff --git a/t/t6037-merge-ours-theirs.sh b/t/t6037-merge-ours-theirs.sh
index 3889eca..8e67d2a 100755
--- a/t/t6037-merge-ours-theirs.sh
+++ b/t/t6037-merge-ours-theirs.sh
@@ -66,11 +66,11 @@ test_expect_success 'binary file with -Xours/-Xtheirs' '
 '
 
 test_expect_success 'pull passes -X to underlying merge' '
-	git reset --hard master && git pull -s recursive -Xours . side &&
-	git reset --hard master && git pull -s recursive -X ours . side &&
-	git reset --hard master && git pull -s recursive -Xtheirs . side &&
-	git reset --hard master && git pull -s recursive -X theirs . side &&
-	git reset --hard master && test_must_fail git pull -s recursive -X bork . side
+	git reset --hard master && git pull -m -s recursive -Xours . side &&
+	git reset --hard master && git pull -m -s recursive -X ours . side &&
+	git reset --hard master && git pull -m -s recursive -Xtheirs . side &&
+	git reset --hard master && git pull -m -s recursive -X theirs . side &&
+	git reset --hard master && test_must_fail git pull -m -s recursive -X bork . side
 '
 
 test_done
-- 
1.9.2+fc1.3.gade8541

Re: [PATCH v5 0/6] Reject non-ff pulls by default

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> These are the steps needed to achieve this:

The overall progression (this comment is only about the design, not
the implementation) looks almost sensible, but I may have missed
some issues because the presentation was done in reverse.

In the following comment, I'll flip the presentation order to better
show natural progression of what the users will see.

> 1) Rename pull.rename to pull.mode and
>    branch.<name>.rebase to branch.<name>.pullmode
>
> This way the configurations and options remain consistent:
>
>   git pull --merge
>   pull.mode = merge
>   branch.<name>.pullmode = merge
>
>   git pull --rebase
>   pull.mode = rebase
>   branch.<name>.pullmode = rebase
>
>   git pull --rebase=preserve
>   pull.mode = rebase-preserve
>   branch.<name>.pullmode = rebase-preserve
>
>   git pull
>   pull.mode = merge-ff-only
>   branch.<name>.pullmode = merge-ff-only

Until the "--merge" option is added, "pull.mode = merge" cannot be
the same as "git pull --merge".  I think you either need to squash
these two steps into one, or flip the order of them.

> 2) Add --merge option
>
> Since we have a message that says "If unsure, run 'git pull --merge'", which is
> more friendly than 'git pull --no-rebase', we should add this option, and
> deprecate --no-rebase.

Obviously s/have a/will have a/, but the intention is good.

> However, the documentation would become confusing if --merge is configured in
> pull.rebase, instead, we want something like this:
>
>   See `pull.mode`, `branch.<name>.pullmode` in linkgit:git-config[1] if you want
>   to make `git pull` always use `--merge`.

It gets unclear to me how the transition is planned around here.  Is
this a correct paraphrasing of these four steps?

    - Add pull.mode (and its branch-specific friend) and "pull
      --merge" so that people can set the former to "merge" or train
      their fingers to type the latter to keep doing the
      fetch-and-merge (your steps 1 and 2)

    - Add ff-only to pull.mode (your step 3)

    - With the endgame of "out of box Git without any configuration
      refuses 'git pull' (without --merge/--rebase) that does not
      fast forward" in mind, start warning "In the future you will
      have to either set pull.mode (and/or its friends) or type
      "pull --merge" (or "pull --rebase") when the endgame version
      of 'git pull' would fail with the error message, but still do
      as was asked to do as before.  At this step, existing users
      can set pull.mode to "merge" or "rebase" or whatever to
      squelch the warning.

    - Flip the default.  By the time this happens, thanks to the
      previous step to warn beforehand, nobody needs to see the
      warning. (your step 4)

If that is the rough outline, I think it is sensible.

> 3) Add merge-ff-only config
>
> This option would trigger a check inside `git pull` itself, and error out with
> the aforementioned message if it's not possible to do a fast-forward merge.
>
> However, this option conflicts with --rebase, and --no-rebase. Solution below.

Am I reading you correctly that setting "pull.mode = ff-only" will
require you to explicitly say "git pull --merge/--rebase"?  If that
is the case, I think the step makes sense.

> 4) Only allow fast-forward merges by default
>
> We could pass --ff-only to `git merge`, however, if we do that we'll get an error like this:
>
>   Not possible to fast-forward, aborting.
>
> This is not friendly; we want an error that is user-friendly:
>
>   The pull was not fast-forward, please either merge or rebase.
>   If unsure, run 'git pull --merge'.
>
> When we do this we want to give the users the option to go back to the previous
> behavior, so a new configuration is needed.

Re: [PATCH v5 0/6] Reject non-ff pulls by default

[Felipe Contreras]

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > These are the steps needed to achieve this:
> 
> The overall progression (this comment is only about the design, not
> the implementation) looks almost sensible, but I may have missed
> some issues because the presentation was done in reverse.
> 
> In the following comment, I'll flip the presentation order to better
> show natural progression of what the users will see.
> 
> > 1) Rename pull.rename to pull.mode and
> >    branch.<name>.rebase to branch.<name>.pullmode
> >
> > This way the configurations and options remain consistent:
> >
> >   git pull --merge
> >   pull.mode = merge
> >   branch.<name>.pullmode = merge
> >
> >   git pull --rebase
> >   pull.mode = rebase
> >   branch.<name>.pullmode = rebase
> >
> >   git pull --rebase=preserve
> >   pull.mode = rebase-preserve
> >   branch.<name>.pullmode = rebase-preserve
> >
> >   git pull
> >   pull.mode = merge-ff-only
> >   branch.<name>.pullmode = merge-ff-only
> 
> Until the "--merge" option is added, "pull.mode = merge" cannot be
> the same as "git pull --merge".  I think you either need to squash
> these two steps into one, or flip the order of them.

Yeah, but the documentation of --merge should mention `pull.mode` and
`branch.<name>.pullmode`. If I do --merge first I would have to mention
pull.rebase and branch.<name>.rebase, which is weird.

I think it's more sensible to do the less visible changes first.

> > 2) Add --merge option
> >
> > Since we have a message that says "If unsure, run 'git pull --merge'", which is
> > more friendly than 'git pull --no-rebase', we should add this option, and
> > deprecate --no-rebase.
> 
> Obviously s/have a/will have a/, but the intention is good.
> 
> > However, the documentation would become confusing if --merge is configured in
> > pull.rebase, instead, we want something like this:
> >
> >   See `pull.mode`, `branch.<name>.pullmode` in linkgit:git-config[1] if you want
> >   to make `git pull` always use `--merge`.
> 
> It gets unclear to me how the transition is planned around here.  Is
> this a correct paraphrasing of these four steps?
> 
>     - Add pull.mode (and its branch-specific friend) and "pull
>       --merge" so that people can set the former to "merge" or train
>       their fingers to type the latter to keep doing the
>       fetch-and-merge (your steps 1 and 2)
> 
>     - Add ff-only to pull.mode (your step 3)

Correct.

>     - With the endgame of "out of box Git without any configuration
>       refuses 'git pull' (without --merge/--rebase) that does not
>       fast forward" in mind, start warning "In the future you will
>       have to either set pull.mode (and/or its friends) or type
>       "pull --merge" (or "pull --rebase") when the endgame version
>       of 'git pull' would fail with the error message, but still do
>       as was asked to do as before.  At this step, existing users
>       can set pull.mode to "merge" or "rebase" or whatever to
>       squelch the warning.
> 
>     - Flip the default.  By the time this happens, thanks to the
>       previous step to warn beforehand, nobody needs to see the
>       warning. (your step 4)

This is what my last version of the series did[1]. However, my plan was
to land this in 1.x so users could see the warning, and then flip the
switch on 2.0.

This plan, however, fell off the cliff.

> If that is the rough outline, I think it is sensible.
> 
> > 3) Add merge-ff-only config
> >
> > This option would trigger a check inside `git pull` itself, and error out with
> > the aforementioned message if it's not possible to do a fast-forward merge.
> >
> > However, this option conflicts with --rebase, and --no-rebase. Solution below.
> 
> Am I reading you correctly that setting "pull.mode = ff-only" will
> require you to explicitly say "git pull --merge/--rebase"?  If that
> is the case, I think the step makes sense.

pull.mode = merge-ff-only

[1] http://article.gmane.org/gmane.comp.version-control.git/235951

-- 
Felipe Contreras

Re: [PATCH v5 0/6] Reject non-ff pulls by default

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

>>     - With the endgame of "out of box Git without any configuration
>>       refuses 'git pull' (without --merge/--rebase) that does not
>>       fast forward" in mind, start warning "In the future you will
>>       have to either set pull.mode (and/or its friends) or type
>>       "pull --merge" (or "pull --rebase") when the endgame version
>>       of 'git pull' would fail with the error message, but still do
>>       as was asked to do as before.  At this step, existing users
>>       can set pull.mode to "merge" or "rebase" or whatever to
>>       squelch the warning.
>> 
>>     - Flip the default.  By the time this happens, thanks to the
>>       previous step to warn beforehand, nobody needs to see the
>>       warning. (your step 4)
>
> This is what my last version of the series did[1]. However, my plan was
> to land this in 1.x so users could see the warning, and then flip the
> switch on 2.0.
>
> This plan, however, fell off the cliff.

Yeah, I see that $gmane/234488 explains why the second step in the
previous one stopped.  I guess it was in expecting a reroll state,
waiting for that other topic (I do not remember offhand) to
graduate.

I see nothing touching the affected codepaths now, so this time
around we may have a better chance, perhaps?

Re: [PATCH v5 0/6] Reject non-ff pulls by default

[Felipe Contreras]

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> >>     - With the endgame of "out of box Git without any configuration
> >>       refuses 'git pull' (without --merge/--rebase) that does not
> >>       fast forward" in mind, start warning "In the future you will
> >>       have to either set pull.mode (and/or its friends) or type
> >>       "pull --merge" (or "pull --rebase") when the endgame version
> >>       of 'git pull' would fail with the error message, but still do
> >>       as was asked to do as before.  At this step, existing users
> >>       can set pull.mode to "merge" or "rebase" or whatever to
> >>       squelch the warning.
> >> 
> >>     - Flip the default.  By the time this happens, thanks to the
> >>       previous step to warn beforehand, nobody needs to see the
> >>       warning. (your step 4)
> >
> > This is what my last version of the series did[1]. However, my plan was
> > to land this in 1.x so users could see the warning, and then flip the
> > switch on 2.0.
> >
> > This plan, however, fell off the cliff.
> 
> Yeah, I see that $gmane/234488 explains why the second step in the
> previous one stopped.  I guess it was in expecting a reroll state,
> waiting for that other topic (I do not remember offhand) to
> graduate.
> 
> I see nothing touching the affected codepaths now, so this time
> around we may have a better chance, perhaps?

A chance of what? Do you want me to reroll to include the future
backwards-incompatible change warning? Should I include the patch that
turns the switch?

-- 
Felipe Contreras

Re: [PATCH v5 0/6] Reject non-ff pulls by default

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Junio C Hamano wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>> ...
>> > This plan, however, fell off the cliff.
>> 
>> Yeah, I see that $gmane/234488 explains why the second step in the
>> previous one stopped.  I guess it was in expecting a reroll state,
>> waiting for that other topic (I do not remember offhand) to
>> graduate.
>> 
>> I see nothing touching the affected codepaths now, so this time
>> around we may have a better chance, perhaps?
>
> A chance of what?

A chance not to have the plan "fell off the cliff".

> Do you want me to reroll to include the future
> backwards-incompatible change warning?

Yes, as I said in the other thread ($gmane/247758), if you do not
have that step yet (I haven't looked at the implementation while
discussing the design in this thread and looking at patches in other
topics) I'd strongly recommend adding one.

> Should I include the patch that turns the switch?

Sorry, but you lost me here.  Turns what switch to what value?

Re: [PATCH v5 0/6] Reject non-ff pulls by default

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Junio C Hamano wrote:
> ...
>> Until the "--merge" option is added, "pull.mode = merge" cannot be
>> the same as "git pull --merge".  I think you either need to squash
>> these two steps into one, or flip the order of them.
>
> Yeah, but the documentation of --merge should mention `pull.mode` and
> `branch.<name>.pullmode`. If I do --merge first I would have to mention
> pull.rebase and branch.<name>.rebase, which is weird.

And the point of your step (1) to introduce pull.mode is to fix the
weirdness, so in that sense, it makes even more sense to do the
"--merge" first and then pull.mode the second.

If you first add --merge with an awkward documentation in the first
step and then correct that awkwardness in the second step that adds
pull.mode (oh, by the way, we need to pay attention to pull.rename
as a fallback at least for a while), that would show a clear
justification why pull.mode is a good idea.

> I think it's more sensible to do the less visible changes first.

The people who discover pull.mode and set it to "merge" will be
greeted with an error with that step.

So it appears that squashing these two (and possibly also the
addition of merge-ff-only) into a single step would be the only
alternative, if you want to avoid the "introduce something that
shows the awkwardness of the situation and immediately fix it"
approach.