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

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

[Felipe Contreras]

Junio already sent a similar patch, but I think this is simpler.

Felipe Contreras (3):
  merge: simplify ff-only option
  t: replace pulls with merges
  pull: reject non-ff pulls by default

 Documentation/git-pull.txt             |  1 +
 builtin/merge.c                        | 20 ++++++++++----------
 git-pull.sh                            |  9 ++++++++-
 t/annotate-tests.sh                    |  2 +-
 t/t4200-rerere.sh                      |  2 +-
 t/t5500-fetch-pack.sh                  |  2 +-
 t/t5520-pull.sh                        | 33 +++++++++++++++++++++++++++++++++
 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               |  4 ++--
 t/t6037-merge-ours-theirs.sh           | 10 +++++-----
 t/t7603-merge-reduce-heads.sh          |  2 +-
 t/t9114-git-svn-dcommit-merge.sh       |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh |  2 +-
 16 files changed, 79 insertions(+), 38 deletions(-)

-- 
1.8.4-337-g7358a66-dirty

[PATCH 1/3] merge: simplify ff-only option

[Felipe Contreras]

No functional changes.

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

diff --git a/builtin/merge.c b/builtin/merge.c
index 34a6166..da9fc08 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -186,13 +186,6 @@ static int option_parse_n(const struct option *opt,
 	return 0;
 }
 
-static int option_parse_ff_only(const struct option *opt,
-			  const char *arg, int unset)
-{
-	fast_forward = FF_ONLY;
-	return 0;
-}
-
 static struct option builtin_merge_options[] = {
 	{ OPTION_CALLBACK, 'n', NULL, NULL, NULL,
 		N_("do not show a diffstat at the end of the merge"),
@@ -210,9 +203,9 @@ static struct option builtin_merge_options[] = {
 	OPT_BOOL('e', "edit", &option_edit,
 		N_("edit message before committing")),
 	OPT_SET_INT(0, "ff", &fast_forward, N_("allow fast-forward (default)"), FF_ALLOW),
-	{ OPTION_CALLBACK, 0, "ff-only", NULL, NULL,
+	{ OPTION_SET_INT, 0, "ff-only", &fast_forward, NULL,
 		N_("abort if fast-forward is not possible"),
-		PARSE_OPT_NOARG | PARSE_OPT_NONEG, option_parse_ff_only },
+		PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, FF_ONLY },
 	OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
 	OPT_BOOL(0, "verify-signatures", &verify_signatures,
 		N_("Verify that the named commit has a valid GPG signature")),
-- 
1.8.4-337-g7358a66-dirty

[PATCH 2/3] t: replace pulls with merges

[Felipe Contreras]

This is what the code intended.

No functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/annotate-tests.sh                    | 2 +-
 t/t4200-rerere.sh                      | 2 +-
 t/t9114-git-svn-dcommit-merge.sh       | 2 +-
 t/t9500-gitweb-standalone-no-errors.sh | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index d4e7f47..01deece 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -92,7 +92,7 @@ test_expect_success 'blame 2 authors + 1 branch2 author' '
 '
 
 test_expect_success 'merge branch1 & branch2' '
-	git pull . branch1
+	git merge branch1
 '
 
 test_expect_success 'blame 2 authors + 2 merged-in authors' '
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 7f6666f..cf19eb7 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -172,7 +172,7 @@ test_expect_success 'first postimage wins' '
 	git show second^:a1 | sed "s/To die: t/To die! T/" >a1 &&
 	git commit -q -a -m third &&
 
-	test_must_fail git pull . first &&
+	test_must_fail git merge first &&
 	# rerere kicked in
 	! grep "^=======\$" a1 &&
 	test_cmp expect a1
diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index f524d2f..d33d714 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -62,7 +62,7 @@ test_expect_success 'setup git mirror and merge' '
 	echo friend > README &&
 	cat tmp >> README &&
 	git commit -a -m "friend" &&
-	git pull . merge
+	git merge merge
 	'
 
 test_debug 'gitk --all & sleep 1'
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 6fca193..3864388 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -328,7 +328,7 @@ test_expect_success \
 	 git add b &&
 	 git commit -a -m "On branch" &&
 	 git checkout master &&
-	 git pull . b &&
+	 git merge b &&
 	 git tag merge_commit'
 
 test_expect_success \
-- 
1.8.4-337-g7358a66-dirty

[PATCH 3/3] pull: reject non-ff pulls by default

[Felipe Contreras]

For the full discussion:

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

The user still can specify 'git pull --merge' to restore the old
behavior, or 'git config pull.rebase false'.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-pull.txt    |  1 +
 builtin/merge.c               |  9 ++++++++-
 git-pull.sh                   |  9 ++++++++-
 t/t5500-fetch-pack.sh         |  2 +-
 t/t5520-pull.sh               | 33 +++++++++++++++++++++++++++++++++
 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      |  4 ++--
 t/t6037-merge-ours-theirs.sh  | 10 +++++-----
 t/t7603-merge-reduce-heads.sh |  2 +-
 12 files changed, 73 insertions(+), 25 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 6ef8d59..1833779 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -119,6 +119,7 @@ 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.
 
+--merge::
 --no-rebase::
 	Override earlier --rebase.
 
diff --git a/builtin/merge.c b/builtin/merge.c
index da9fc08..97b4205 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1437,8 +1437,15 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	if (fast_forward == FF_ONLY)
+	if (fast_forward == FF_ONLY) {
+		const char *msg = getenv("GIT_MERGE_FF_ONLY_HELP");
+		if (msg) {
+			fprintf(stderr, "%s\n", msg);
+			ret = 1;
+			goto done;
+		}
 		die(_("Not possible to fast-forward, aborting."));
+	}
 
 	/* We are going to make a new commit. */
 	git_committer_info(IDENT_STRICT);
diff --git a/git-pull.sh b/git-pull.sh
index f0df41c..c6b576b 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -113,7 +113,8 @@ do
 	-r|--r|--re|--reb|--reba|--rebas|--rebase)
 		rebase=true
 		;;
-	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
+	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase|\
+	-m|--m|--me|--mer|--merg|--merge)
 		rebase=false
 		;;
 	--recurse-submodules)
@@ -289,6 +290,12 @@ then
 	fi
 fi
 
+if test -z "$rebase$no_ff$ff_only${squash#--no-squash}"
+then
+	ff_only=--ff-only
+	export GIT_MERGE_FF_ONLY_HELP="The pull was not fast-forward, please either merge or rebase."
+fi
+
 merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 case "$rebase" in
 true)
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index fd2598e..f1a068f 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/t5520-pull.sh b/t/t5520-pull.sh
index ed4d9c8..c0c50a2 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -284,4 +284,37 @@ 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" &&
+	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" &&
+	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" &&
+	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
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 c680f78..6c7fdc1 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..0eeec04 100755
--- a/t/t6029-merge-subtree.sh
+++ b/t/t6029-merge-subtree.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..927e67c 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 --merge -s recursive -Xours . side &&
+	git reset --hard master && git pull --merge -s recursive -X ours . side &&
+	git reset --hard master && git pull --merge -s recursive -Xtheirs . side &&
+	git reset --hard master && git pull --merge -s recursive -X theirs . side &&
+	git reset --hard master && test_must_fail git pull --merge -s recursive -X bork . side
 '
 
 test_done
diff --git a/t/t7603-merge-reduce-heads.sh b/t/t7603-merge-reduce-heads.sh
index 9894895..566b1c1 100755
--- a/t/t7603-merge-reduce-heads.sh
+++ b/t/t7603-merge-reduce-heads.sh
@@ -68,7 +68,7 @@ test_expect_success 'merge c1 with c2, c3, c4, c5' '
 
 test_expect_success 'pull c2, c3, c4, c5 into c1' '
 	git reset --hard c1 &&
-	git pull . c2 c3 c4 c5 &&
+	git pull --merge . c2 c3 c4 c5 &&
 	test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
 	test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" &&
 	test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
-- 
1.8.4-337-g7358a66-dirty

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

[Junio C Hamano]

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

> Junio already sent a similar patch, but I think this is simpler.

I agree that this is simpler, but I am not sure if the behaviour is
necessarily better (note that this is different from saying "I think
the behaviour of this patch is worse").  The motivation I read from
the original discussion was that new people did "git pull" (no other
parameters) to "sync my tree with the central repository" as if it
were SVN, and because we are not SVN, projects that prefer rebases
were unhappy, and the other one was to address *only* that use case.
I do not personally like that special casing (i.e. "only when no
'integrate with what from where' is given"), and applying the "you
must be explicit between rebase and merge" like this series does
uniformly might (or might not) be a good thing.  I dunno.

The difference in changes needed to the test suite is illustrative;
this series affects any use of "git pull" (with or without explicit
"what to integrate with and from where"), unlike the other one that
only affects the case where "git pull" was not given "what to
integrate with and from where".  I think an earlier draft I did for
the previous one did not special case "only when no 'integrate with
what from where' is given" and had to touch all the places in the
test in a similar way.

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

[Felipe Contreras]

On Tue, Sep 3, 2013 at 12:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Junio already sent a similar patch, but I think this is simpler.
>
> I agree that this is simpler, but I am not sure if the behaviour is
> necessarily better (note that this is different from saying "I think
> the behaviour of this patch is worse").  The motivation I read from
> the original discussion was that new people did "git pull" (no other
> parameters) to "sync my tree with the central repository" as if it
> were SVN, and because we are not SVN, projects that prefer rebases
> were unhappy, and the other one was to address *only* that use case.
> I do not personally like that special casing (i.e. "only when no
> 'integrate with what from where' is given"), and applying the "you
> must be explicit between rebase and merge" like this series does
> uniformly might (or might not) be a good thing.  I dunno.

As I already said; there's is essentially no difference between "git
pull" and "git pull origin".

> The difference in changes needed to the test suite is illustrative;
> this series affects any use of "git pull" (with or without explicit
> "what to integrate with and from where"), unlike the other one that
> only affects the case where "git pull" was not given "what to
> integrate with and from where".  I think an earlier draft I did for
> the previous one did not special case "only when no 'integrate with
> what from where' is given" and had to touch all the places in the
> test in a similar way.

Yeah, that version affects less, but it also doesn't achieve what we
actually want.

Either way, that's why I sent another version that doesn't need
modifications on the tests.

-- 
Felipe Contreras

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

[Junio C Hamano]

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

> On Tue, Sep 3, 2013 at 12:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> Junio already sent a similar patch, but I think this is simpler.
>>
>> I agree that this is simpler, but I am not sure if the behaviour is
>> necessarily better (note that this is different from saying "I think
>> the behaviour of this patch is worse").  The motivation I read from
>> the original discussion was that new people did "git pull" (no other
>> parameters) to "sync my tree with the central repository" as if it
>> were SVN, and because we are not SVN, projects that prefer rebases
>> were unhappy, and the other one was to address *only* that use case.
>> I do not personally like that special casing (i.e. "only when no
>> 'integrate with what from where' is given"), and applying the "you
>> must be explicit between rebase and merge" like this series does
>> uniformly might (or might not) be a good thing.  I dunno.
>
> As I already said; there's is essentially no difference between "git
> pull" and "git pull origin".

We know what you said earlier. That does not make it right or wrong,
but I do not think it is in line with the original discussion (that
is why John Keeping is kept on the Cc: line).

>> The difference in changes needed to the test suite is illustrative;
>> this series affects any use of "git pull" (with or without explicit
>> "what to integrate with and from where"), unlike the other one that
>> only affects the case where "git pull" was not given "what to
>> integrate with and from where".  I think an earlier draft I did for
>> the previous one did not special case "only when no 'integrate with
>> what from where' is given" and had to touch all the places in the
>> test in a similar way.
>
> Yeah, that version affects less, but it also doesn't achieve what we
> actually want.

I do not think we know what we want is to affect "git pull origin".

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

[Felipe Contreras]

On Tue, Sep 3, 2013 at 5:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Tue, Sep 3, 2013 at 12:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>> Junio already sent a similar patch, but I think this is simpler.
>>>
>>> I agree that this is simpler, but I am not sure if the behaviour is
>>> necessarily better (note that this is different from saying "I think
>>> the behaviour of this patch is worse").  The motivation I read from
>>> the original discussion was that new people did "git pull" (no other
>>> parameters) to "sync my tree with the central repository" as if it
>>> were SVN, and because we are not SVN, projects that prefer rebases
>>> were unhappy, and the other one was to address *only* that use case.
>>> I do not personally like that special casing (i.e. "only when no
>>> 'integrate with what from where' is given"), and applying the "you
>>> must be explicit between rebase and merge" like this series does
>>> uniformly might (or might not) be a good thing.  I dunno.
>>
>> As I already said; there's is essentially no difference between "git
>> pull" and "git pull origin".
>
> We know what you said earlier. That does not make it right or wrong,
> but I do not think it is in line with the original discussion (that
> is why John Keeping is kept on the Cc: line).

And nobody provided any argument against that claim. People staying
silent doesn't make it wrong.

>>> The difference in changes needed to the test suite is illustrative;
>>> this series affects any use of "git pull" (with or without explicit
>>> "what to integrate with and from where"), unlike the other one that
>>> only affects the case where "git pull" was not given "what to
>>> integrate with and from where".  I think an earlier draft I did for
>>> the previous one did not special case "only when no 'integrate with
>>> what from where' is given" and had to touch all the places in the
>>> test in a similar way.
>>
>> Yeah, that version affects less, but it also doesn't achieve what we
>> actually want.
>
> I do not think we know what we want is to affect "git pull origin".

Of course we do.

What we want is to make "git pull" more user-friendly, specially to
newcomers, and specially those that come from centralized VCS, where
"tool pull" updates the checkout, and thus we want "git pull" not to
create merges inadvertently, and the best way to do that is to warn
the user that the merge is non-fast-forward, and he should do a merge
or rebase.

The fact that a particular user might have learned about remotes and
did "git pull origin" instead is irrelevant, he would still want to be
warned about non-fast-forward merges.

Everybody would want to be warned about that by default, I know I
would, I might even start using 'git pull' again, and so countless
people that have stopped using 'git pull' precisely for this reason.

-- 
Felipe Contreras

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

[John Keeping]

On Tue, Sep 03, 2013 at 03:38:58PM -0700, Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > On Tue, Sep 3, 2013 at 12:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >> Felipe Contreras <felipe.contreras@gmail.com> writes:
> >>
> >>> Junio already sent a similar patch, but I think this is simpler.
> >>
> >> I agree that this is simpler, but I am not sure if the behaviour is
> >> necessarily better (note that this is different from saying "I think
> >> the behaviour of this patch is worse").  The motivation I read from
> >> the original discussion was that new people did "git pull" (no other
> >> parameters) to "sync my tree with the central repository" as if it
> >> were SVN, and because we are not SVN, projects that prefer rebases
> >> were unhappy, and the other one was to address *only* that use case.
> >> I do not personally like that special casing (i.e. "only when no
> >> 'integrate with what from where' is given"), and applying the "you
> >> must be explicit between rebase and merge" like this series does
> >> uniformly might (or might not) be a good thing.  I dunno.
> >
> > As I already said; there's is essentially no difference between "git
> > pull" and "git pull origin".
> 
> We know what you said earlier. That does not make it right or wrong,
> but I do not think it is in line with the original discussion (that
> is why John Keeping is kept on the Cc: line).

I think there are two distinct uses for pull, which boil down to:

    (1) git pull
    (2) git pull $remote $branch

For (1) a merge is almost always the wrong thing to do since it will be
backwards and break --first-parent.

But for (2) a merge is almost always the correct thing to do (in fact it
may even be correct to create a merge commit even when this fast
forwards) because this most likely comes for a pull request workflow.

> I do not think we know what we want is to affect "git pull origin".

I consider "git pull $remote" to be an artifact of the way git-pull is
implemented on top of git-fetch; perhaps I'm missing something but I
can't see a scenario where this is useful.  In the series currently in
"next", we treat this as (2) above but that's primarily because it is
difficult to differentiate these in git-pull.sh without adding code to
understand all of the options to git-fetch (or at least those that can
accept unstuck arguments).

Changing this so that "git pull $remote" is treated as (1) would be
better, but I think it is more important to avoid catching case (1) in
the same net which is why jc/pull-training-wheel simply checks if "$#"
is zero; the cost of getting this completely right outweighed the
benefit of getting code in that will catch 99% of users.

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

[Jeff King]

On Wed, Sep 04, 2013 at 09:10:47AM +0100, John Keeping wrote:

> I think there are two distinct uses for pull, which boil down to:
> 
>     (1) git pull
>     (2) git pull $remote $branch
> 
> For (1) a merge is almost always the wrong thing to do since it will be
> backwards and break --first-parent.

Is it always wrong? You are assuming a topic-branch workflow where
--first-parent is actually meaningful. What about a centralized workflow
where everyone works on "master"? The correct thing to do on a non-ff
push in that case is "git pull && git push". Some people would argue
that the pull should rebase there, but I think there are valid arguments
either way. We can discuss in that direction if you want.

I can perhaps buy the argument that it is better to help people who are
using a topic branch workflow (which we generally want to encourage) to
avoid making backwards merges, and the cost is that people with sloppy
workflows will have to do more work / configuration. But we should be
clear that this is a tradeoff we are making.

The patch in jc/pull-training-wheel talks about annoying old timers, but
I think you may also be annoying clueless new users who simply want an
svn-like workflow without thinking too hard about it.

> > I do not think we know what we want is to affect "git pull origin".
> 
> I consider "git pull $remote" to be an artifact of the way git-pull is
> implemented on top of git-fetch; perhaps I'm missing something but I
> can't see a scenario where this is useful.

Imagine a workflow where each topic is in its own repository instead of
in its own branch inside a repository. Or where each developer has his
or her own repository, but everybody just works on the master branch of
their repository (or perhaps uses branches, but keeps master as a stable
base). Alice is the integration manager; Bob tells her that he has work
ready to integrate.  She runs "git pull ~bob/project", which will merge
Bob's HEAD.

This is not very different from the kernel workflow, where Linus may do
a "git pull $remote" to fetch a sub-system maintainer's work, except
that these days people typically mark the to-be-integrated work in a
"for-linus" branch or tag. However, you can find many "Merge git://"
entries even in recent kernel history.

I think this kind of pull would fall into the same situation as your (2)
above.

-Peff

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

[John Keeping]

On Wed, Sep 04, 2013 at 05:25:27AM -0400, Jeff King wrote:
> On Wed, Sep 04, 2013 at 09:10:47AM +0100, John Keeping wrote:
> 
> > I think there are two distinct uses for pull, which boil down to:
> > 
> >     (1) git pull
> >     (2) git pull $remote $branch
> > 
> > For (1) a merge is almost always the wrong thing to do since it will be
> > backwards and break --first-parent.
> 
> Is it always wrong? You are assuming a topic-branch workflow where
> --first-parent is actually meaningful. What about a centralized workflow
> where everyone works on "master"? The correct thing to do on a non-ff
> push in that case is "git pull && git push". Some people would argue
> that the pull should rebase there, but I think there are valid arguments
> either way. We can discuss in that direction if you want.

I'm one of the people who argues that it should rebase there ;-)  The
point of jc/pull-training-wheel is to help users think about that.

> I can perhaps buy the argument that it is better to help people who are
> using a topic branch workflow (which we generally want to encourage) to
> avoid making backwards merges, and the cost is that people with sloppy
> workflows will have to do more work / configuration. But we should be
> clear that this is a tradeoff we are making.
> 
> The patch in jc/pull-training-wheel talks about annoying old timers, but
> I think you may also be annoying clueless new users who simply want an
> svn-like workflow without thinking too hard about it.

The scenario I have is a central repository where some developers use a
topic branch workflow but others are less familiar with Git and don't
really think about what they're doing.

> > > I do not think we know what we want is to affect "git pull origin".
> > 
> > I consider "git pull $remote" to be an artifact of the way git-pull is
> > implemented on top of git-fetch; perhaps I'm missing something but I
> > can't see a scenario where this is useful.
> 
> Imagine a workflow where each topic is in its own repository instead of
> in its own branch inside a repository. Or where each developer has his
> or her own repository, but everybody just works on the master branch of
> their repository (or perhaps uses branches, but keeps master as a stable
> base). Alice is the integration manager; Bob tells her that he has work
> ready to integrate.  She runs "git pull ~bob/project", which will merge
> Bob's HEAD.
> 
> This is not very different from the kernel workflow, where Linus may do
> a "git pull $remote" to fetch a sub-system maintainer's work, except
> that these days people typically mark the to-be-integrated work in a
> "for-linus" branch or tag. However, you can find many "Merge git://"
> entries even in recent kernel history.
> 
> I think this kind of pull would fall into the same situation as your (2)
> above.

OK - so I was missing this.  Given this, the jc/pull-training-wheel
series is doing the right thing here.

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

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> On Tue, Sep 03, 2013 at 03:38:58PM -0700, Junio C Hamano wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>> 
>> > On Tue, Sep 3, 2013 at 12:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> >> Felipe Contreras <felipe.contreras@gmail.com> writes:
>> >>
>> >>> Junio already sent a similar patch, but I think this is simpler.
>> >>
>> >> I agree that this is simpler, but I am not sure if the behaviour is
>> >> necessarily better (note that this is different from saying "I think
>> >> the behaviour of this patch is worse").  The motivation I read from
>> >> the original discussion was that new people did "git pull" (no other
>> >> parameters) to "sync my tree with the central repository" as if it
>> >> were SVN, and because we are not SVN, projects that prefer rebases
>> >> were unhappy, and the other one was to address *only* that use case.
>> >> I do not personally like that special casing (i.e. "only when no
>> >> 'integrate with what from where' is given"), and applying the "you
>> >> must be explicit between rebase and merge" like this series does
>> >> uniformly might (or might not) be a good thing.  I dunno.
>> >
>> > As I already said; there's is essentially no difference between "git
>> > pull" and "git pull origin".
>> 
>> We know what you said earlier. That does not make it right or wrong,
>> but I do not think it is in line with the original discussion (that
>> is why John Keeping is kept on the Cc: line).
>
> I think there are two distinct uses for pull, which boil down to:
>
>     (1) git pull
>     (2) git pull $remote $branch
>
> For (1) a merge is almost always the wrong thing to do since it will be
> backwards and break --first-parent.
>
> But for (2) a merge is almost always the correct thing to do (in fact it
> may even be correct to create a merge commit even when this fast
> forwards) because this most likely comes for a pull request workflow.
>
>> I do not think we know what we want is to affect "git pull origin".

I didn't mean to limit this to "with an explicit 'from where'
without 'which branch'", but it appears you took it that way.
I should have added:

    I do not think we know what we want is to affect "git pull
    origin master", either.

to clarify.  But it seems that Peff's later message in this thread
already clarifies this point for me ;-)


> I consider "git pull $remote" to be an artifact of the way git-pull is
> implemented on top of git-fetch; perhaps I'm missing something but I
> can't see a scenario where this is useful.  In the series currently in
> "next", we treat this as (2) above but that's primarily because it is
> difficult to differentiate these in git-pull.sh without adding code to
> understand all of the options to git-fetch (or at least those that can
> accept unstuck arguments).
>
> Changing this so that "git pull $remote" is treated as (1) would be
> better, but I think it is more important to avoid catching case (1) in
> the same net which is why jc/pull-training-wheel simply checks if "$#"
> is zero; the cost of getting this completely right outweighed the
> benefit of getting code in that will catch 99% of users.

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

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> I think there are two distinct uses for pull, which boil down to:
>
>     (1) git pull
>     (2) git pull $remote $branch
>
> For (1) a merge is almost always the wrong thing to do since it will be
> backwards and break --first-parent.
>
> But for (2) a merge is almost always the correct thing to do (in fact it
> may even be correct to create a merge commit even when this fast
> forwards) because this most likely comes for a pull request workflow.

Peff already covered (1)---it is highly doubtful that a merge is
"almost always wrong".  In fact, if that _were_ the case, we should
simply be defaulting to rebase, not failing the command and asking
between merge and rebase like jc/pull-training-wheel topic did.

We simply do not know what the user wants, as it heavily depends on
the project, so we ask the user to choose one (and stick to it).


I am not sure about (2), either.  Is it really "almost always the
correct thing to do"?  I tend to think myself that (2) is a lot more
likely to prefer merging than (1) would, but I certainly wouldn't
say "almost always".  Again if "almost always" were the case,
wouldn't it make sense for that mode of invocation of the command to
even defeat "pull.rebase" configuration and default to merge, unless
explicitly told to "pull --rebase" from the command line?

(the last question is rhetoric, if anybody is wondering).

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

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
> John Keeping <john@keeping.me.uk> writes:
>
>> I think there are two distinct uses for pull, which boil down to:
>>
>>     (1) git pull
>>     (2) git pull $remote $branch
>>
>> For (1) a merge is almost always the wrong thing to do since it will 
>> be
>> backwards and break --first-parent.
>>
>> But for (2) a merge is almost always the correct thing to do (in fact 
>> it
>> may even be correct to create a merge commit even when this fast
>> forwards) because this most likely comes for a pull request workflow.
>
> Peff already covered (1)---it is highly doubtful that a merge is
> "almost always wrong".  In fact, if that _were_ the case, we should
> simply be defaulting to rebase, not failing the command and asking
> between merge and rebase like jc/pull-training-wheel topic did.
>
> We simply do not know what the user wants, as it heavily depends on
> the project, so we ask the user to choose one (and stick to it).

We only offer a limited list. It won't be sufficient for all use cases. 
It wasn't for me.

The ability to say 'stop' if it doen't match expectations, as 
the --no-ff option would give, would be a help, as the user can then 
decide what to do (read the manual or `google` the problem perhaps ;-). 
the option of having a hook (if suggested), while suitable for advanced 
users won't help those that need that help, rather a few simple safe 
options are needed.

I generally support the ability to set an option to reject non-ff pulls.

>
> I am not sure about (2), either.  Is it really "almost always the
> correct thing to do"?  I tend to think myself that (2) is a lot more
> likely to prefer merging than (1) would, but I certainly wouldn't
> say "almost always".  Again if "almost always" were the case,
> wouldn't it make sense for that mode of invocation of the command to
> even defeat "pull.rebase" configuration and default to merge, unless
> explicitly told to "pull --rebase" from the command line?
>
> (the last question is rhetoric, if anybody is wondering).
> --
Philip 

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

[Junio C Hamano]

"Philip Oakley" <philipoakley@iee.org> writes:

> From: "Junio C Hamano" <gitster@pobox.com>
>> John Keeping <john@keeping.me.uk> writes:
>>
>>> I think there are two distinct uses for pull, which boil down to:
>>>
>>>     (1) git pull
>> ...
>> Peff already covered (1)---it is highly doubtful that a merge is
>> "almost always wrong".  In fact, if that _were_ the case, we should
>> simply be defaulting to rebase, not failing the command and asking
>> between merge and rebase like jc/pull-training-wheel topic did.
>>
>> We simply do not know what the user wants, as it heavily depends on
>> the project, so we ask the user to choose one (and stick to it).
>
> We only offer a limited list. It won't be sufficient for all use
> cases. It wasn't for me.

Very interesting. Tell us more.

When "git pull" stops because what was fetched in FETCH_HEAD does
not fast-forward, then what did _you_ do (and with the knowledge you
currently have, what would you do)?  In a single project, would you
choose to sometimes rebase and sometimes merge, and if so, what is
the choice depend on?  "When I am on these selected branches, I want
to merge, but on other branches I want to rebase?"

If that is the issue you are trying to raise (I cannot tell yet), a
repository-wide "pull.rebase = merge/rebase" is still too blunt an
instrument, but then "branch.<name>.rebase" can be set to
selectively override it, so that case is covered.

	[pull]
        	rebase = merge
	[branch "po/topic"]
        	rebase = yes

Are there cases where you do not want to either rebase nor merge?
If so what do you want to do after "git pull" fetches from the other
side?  Nothing?

	Side note: a knee-jerk response to a "yes" answer to the
	last question from me has always been "then why are you
	running 'git pull' in the first place. The next paragraph is
	my attempt to extend my imagination a bit, stepping outside
	that reaction.

I can imagine users might want to say "when I am on these small
number of branches, I want to merge (or rebase), but when I am on
other, majority of my branches, because they are private, unfinished
and unpublished work, please stop me from accidentally messing their
histories with changes from upstream or anywhere else for that
matter".  If that is the issue you are trying to raise, because
there is no

	[pull]
        	rebase = fail
	[branch "master"]
        	rebase = yes

to force "git pull" to fail by default on any branch while allowing
it to rebase (or merge, for that matter) only on a few selected
branches, we fall a bit short.

Which can be solved by adding the above "fail" option, and then
renaming them to "pull.integrate" and "branch.<name>.integrate" to
clarify what these variables are about (it is no longer "do you
rebase or not---if you choose not to rebase, by definition you are
going to merge", as there is a third choice to "fail"), while
retaining "pull.rebase" and "branch.<name>.rebase" as a deprecated
synonym.

Am I on the right track following (eh, rather "trying to guess")
what you are trying to get at?

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

[John Keeping]

On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
> Are there cases where you do not want to either rebase nor merge?
> If so what do you want to do after "git pull" fetches from the other
> side?  Nothing?

One other thing that I can see being useful occasionally is:

    git rebase @{u}@{1} --onto @{u}

which allows local commits to be replayed onto a rewritten upstream
branch.

Although I agree with your side note below that people doing this may be
better off fetching and then updating their local branch, particularly
if @{1} is not the correct reflog entry for the upstream when they
created the branch.

> 	Side note: a knee-jerk response to a "yes" answer to the
> 	last question from me has always been "then why are you
> 	running 'git pull' in the first place. The next paragraph is
> 	my attempt to extend my imagination a bit, stepping outside
> 	that reaction.

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

[John Szakmeister]

On Wed, Sep 4, 2013 at 6:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
[snip]
> When "git pull" stops because what was fetched in FETCH_HEAD does
> not fast-forward, then what did _you_ do (and with the knowledge you
> currently have, what would you do)?  In a single project, would you
> choose to sometimes rebase and sometimes merge, and if so, what is
> the choice depend on?  "When I am on these selected branches, I want
> to merge, but on other branches I want to rebase?"

Our team isn't quite proficient enough yet to have a completely rebase
workflow... though we might have less of a problem if we did.  So,
several interesting points.  Most of the time, `git pull` would be a
fast-forward merge.  We typically perform the merges of topic branches
server-side--we have a build server who checks to make sure the result
would be successful--and we just hit the big green button on the Merge
button for the pull request (we use GitHub Enterprise at the moment).

However, nearly as often, we just merge the branch locally because
someone on the team is doing some manual testing, and it's just
convenient to finish the process on the command line.  What
occasionally happens is that you merge the topic locally, but someone
else has introduced a new commit to master.  We try to preserve the
mainline ordering of commits, so `git pull` doing a merge underneath
the hood is undesirable (it moves the newly introduced commit off to
the side).  Rebasing your current master branch is not the answer
either, because it picks up the commits introduced by the topic branch
and rebases those to--at least with the -p option, and without it, the
results are just as bad).  Instead, we want to unfold our work,
fast-forward merge the upstream, and the replay our actions--namely
remerge the topic branch.  It often ends up translating to this:

   $ git reset --hard HEAD~1
   $ git merge --ff-only @{u}
   $ git merge topic
   $ git push

So what I really want isn't quite rebase.  I'm not sure any of the
proposed solutions would work.  It'd be really nice to replay only the
mainline commits, without affecting commits introduced from a topic
branch.

At any rate, this preserves the ordering we desire, but feels like a
less than optimal process.

-John

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

[John Keeping]

On Thu, Sep 05, 2013 at 07:01:03AM -0400, John Szakmeister wrote:
> On Wed, Sep 4, 2013 at 6:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
> [snip]
> > When "git pull" stops because what was fetched in FETCH_HEAD does
> > not fast-forward, then what did _you_ do (and with the knowledge you
> > currently have, what would you do)?  In a single project, would you
> > choose to sometimes rebase and sometimes merge, and if so, what is
> > the choice depend on?  "When I am on these selected branches, I want
> > to merge, but on other branches I want to rebase?"
> 
> Our team isn't quite proficient enough yet to have a completely rebase
> workflow... though we might have less of a problem if we did.  So,
> several interesting points.  Most of the time, `git pull` would be a
> fast-forward merge.  We typically perform the merges of topic branches
> server-side--we have a build server who checks to make sure the result
> would be successful--and we just hit the big green button on the Merge
> button for the pull request (we use GitHub Enterprise at the moment).
> 
> However, nearly as often, we just merge the branch locally because
> someone on the team is doing some manual testing, and it's just
> convenient to finish the process on the command line.  What
> occasionally happens is that you merge the topic locally, but someone
> else has introduced a new commit to master.  We try to preserve the
> mainline ordering of commits, so `git pull` doing a merge underneath
> the hood is undesirable (it moves the newly introduced commit off to
> the side).  Rebasing your current master branch is not the answer
> either, because it picks up the commits introduced by the topic branch
> and rebases those to--at least with the -p option, and without it, the
> results are just as bad).  Instead, we want to unfold our work,
> fast-forward merge the upstream, and the replay our actions--namely
> remerge the topic branch.  It often ends up translating to this:
> 
>    $ git reset --hard HEAD~1
>    $ git merge --ff-only @{u}
>    $ git merge topic
>    $ git push
> 
> So what I really want isn't quite rebase.  I'm not sure any of the
> proposed solutions would work.  It'd be really nice to replay only the
> mainline commits, without affecting commits introduced from a topic
> branch.

Does "git rebase --preserve-merges" do what you want here?

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

[John Szakmeister]

On Thu, Sep 5, 2013 at 7:38 AM, John Keeping <john@keeping.me.uk> wrote:
> On Thu, Sep 05, 2013 at 07:01:03AM -0400, John Szakmeister wrote:
[snip]
>> So what I really want isn't quite rebase.  I'm not sure any of the
>> proposed solutions would work.  It'd be really nice to replay only the
>> mainline commits, without affecting commits introduced from a topic
>> branch.
>
> Does "git rebase --preserve-merges" do what you want here?

No, unfortunately, it does not.  If the topic branch was not based on
the current tip of master, "git rebase --preserve-merges" will rebase
the commits of the topic branch as well.  So this:

       Q -- R -- S     (topic)
     /            \
    A -- B ------- D   (master)

Will become this after "git rebase --preserve-merges @{u}":

                 Q' -- R' -- S'    (topic')
               /              \
    A -- B -- C -------------- D'  (master)

It's unfortunate for a couple of reasons.  First, we don't want Q, R,
and S rebased--we just want the merge replayed.  Secondly, it gets
more confusing because Q, R, and S were rebased, but the topic branch
wasn't actually touched.  So topic still contains Q, R, and S, but
master now contains Q', R', and S'.  What we actually want is:

       Q -- R -- S     (topic)
     /            \
    A -- B -- C -- D'  (master)

HTH!

-John

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

[Greg Troxel]

[-- Attachment #1: Type: text/plain, Size: 2365 bytes --]


Junio C Hamano <gitster@pobox.com> writes:

> Peff already covered (1)---it is highly doubtful that a merge is
> "almost always wrong".  In fact, if that _were_ the case, we should
> simply be defaulting to rebase, not failing the command and asking
> between merge and rebase like jc/pull-training-wheel topic did.
>
> We simply do not know what the user wants, as it heavily depends on
> the project, so we ask the user to choose one (and stick to it).

From my experience leading the first large project using git at BBN,
evolving a workflow (most work on topic branches, which are rebased,
banning 'git pull'-created merge commits, and explicit merge commits to
preserve --first-parent, basically), and seeing many people struggle to
learn all this, my take is that a user who does not understand non-ff
merge vs ff-merge vs rebase will end up doing the wrong thing.  So two
thoughts:

  In the glorious future, perhaps git could have a way to express a
  machine-parseable representation of the workflow and rules for a repo,
  so that these choices could be made accordingly.

  In the current world, I think it makes sense to error out when there
  are multiple reasonable choices depending on workflow.

One of my team members, Richard Hansen, has argued to us that 'git pull'
is harmful, essentially because it creates non-ff merges sometimes,
while our rules say those should be rebased out.  So we use

[alias]
	up = !git remote update -p && git merge --ff-only @{u}

which acts like pull if ff merge works, and otherwise errors out.

I think the key question is: can a user who doesn't really understand
the implications of ff vs non-ff merges and the local workflow rules
actually function ok, or do they need to stop and go back and
understand.  I'm in the "you just have to take the time to understand"
camp, which led to us having a semi-custom syllabus from github training
covering rebase, explicit vs ff merges and the consequences for
first-parent history, etc.

Therefore, I think "git pull" should do the update (perhaps of just the
remote corresponding to @{u}, perhaps without -p) and a --ff-only merge,
absent a configuration asking for non-ff merge or rebase.  (Arguably, an
ff merge is a degenerate case of rebase and also of the ff/non-ff merge,
so it's safe with either policy.)

Greg

[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]

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

[Richard Hansen]

On 2013-09-04 18:59, Junio C Hamano wrote:
> "Philip Oakley" <philipoakley@iee.org> writes:
> 
>> From: "Junio C Hamano" <gitster@pobox.com>
>>> John Keeping <john@keeping.me.uk> writes:
>>>
>>>> I think there are two distinct uses for pull, which boil down to:
>>>>
>>>>     (1) git pull
>>> ...
>>> Peff already covered (1)---it is highly doubtful that a merge is
>>> "almost always wrong".  In fact, if that _were_ the case, we should
>>> simply be defaulting to rebase, not failing the command and asking
>>> between merge and rebase like jc/pull-training-wheel topic did.
>>>
>>> We simply do not know what the user wants, as it heavily depends on
>>> the project, so we ask the user to choose one (and stick to it).
>>
>> We only offer a limited list. It won't be sufficient for all use
>> cases. It wasn't for me.
> 
> Very interesting. Tell us more.

I'm a bit late to the discussion, but I wanted to chime in.  I detest
'git pull' and discourage everyone I meet from using it.  See:
<http://stackoverflow.com/questions/15316601/why-is-git-pull-considered-harmful>
for my reasons.

Instead, I encourage people to do this:

   git config --global alias.up '!git remote update -p; git merge
--ff-only @{u}'

and tell them to run 'git up' whenever they would be tempted to use a
plain 'git pull'.

I usually work with a central repository with topic branches.  I follow
this rule of thumb:
  * When merging a "same-named" branch (e.g., origin/foo into foo, foo
    into origin/foo), it should always be a fast-forward.  This may
    require rebasing.
  * When merging a "differently-named" branch (e.g., feature.xyz into
    master), it should never be a fast-forward.

In distributed workflows, I think of 'git pull <collaborator-repo>
<their-branch>' as merging a differently-named branch (I wouldn't be
merging if they hadn't told me that a separate feature they were working
on is complete), so I generally want the merge commit.  But when I do a
'git pull' without extra arguments, I'm updating a same-named branch so
I never want a merge.

When merging a differently-named branch, I prefer the merge --no-ff to
be preceded by a rebase to get a nice, pretty graph:

       * merge feature.xyz  <- master
       |\
       | * xyz part 3/3
       | * xyz part 2/3
       | * xyz part 1/3
       |/
       * merge feature.foo
       |\
       | * foo part 2/2
       | * foo part 1/2
       |/
       * merge feature.bar
       |\
       ...

The explicit merge has several benefits:
  * It clearly communicates to others that the feature is done.
  * It makes it easier to revert the entire feature by reverting the
    merge if necessary.
  * It allows our continuous integration tool to skip over the
    work-in-progress commits and test only complete features.
  * It makes it easier to review the entire feature in one diff.
  * 'git log --first-parent' shows a high-level summary of the changes
    over time, while a normal 'git log' shows the details.

> 
> When "git pull" stops because what was fetched in FETCH_HEAD does
> not fast-forward, then what did _you_ do (and with the knowledge you
> currently have, what would you do)?

I stop and review what's going on, then make a decision:
  * usually it's a rebase
  * sometimes it's a rebase --onto (because the branch was
    force-updated to undo a particularly bad commit)
  * sometimes it's a rebase -p (because there's an explicit merge of a
    different branch that I want to keep)
  * sometimes it's a reset --hard (my changes were made obsolete by a
    different upstream change)
  * sometimes it's a merge
  * sometimes I do nothing.  This is a fairly regular pattern:  I'm in
    the middle of working on something that I know will conflict with
    some changes that were just pushed upstream, and I want to finish
    my changes before starting the rebase.  My collaborator contacts me
    and asks, "Would you take a look at the changes I just pushed?"  If
    I type 'git pull' out of habit to get the commits, then I'll make a
    mess of my work-in-progress work tree.  If I type 'git up' out of
    habit, then the merge --ff-only will fail as expected and I can
    quickly review the commits without messing with my work tree or
    HEAD.

Even if I always rebase or always merge, I want to briefly review what
changed in the remote branch *before* I start the rebase.  This helps me
understand the conflicts I might encounter.

Thus, ff-only always works for me.  I might have to type a second merge
or rebase command, but that's OK -- it gives me an opportunity to think
about what I want first.  Non-ff merges are rare enough that the
interruption isn't annoying at all.

> In a single project, would you
> choose to sometimes rebase and sometimes merge, and if so, what is
> the choice depend on?  "When I am on these selected branches, I want
> to merge, but on other branches I want to rebase?"

My choice depends on the circumstances of the divergence.  It's never as
simple as branch X always has this policy while branch Y has that policy.

> Are there cases where you do not want to either rebase nor merge?
> If so what do you want to do after "git pull" fetches from the other
> side?  Nothing?
> 
> 	Side note: a knee-jerk response to a "yes" answer to the
> 	last question from me has always been "then why are you
> 	running 'git pull' in the first place.

Habit/muscle memory/I'm tired and not thinking 100% clearly.

-Richard

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

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
>> Are there cases where you do not want to either rebase nor merge?
>> If so what do you want to do after "git pull" fetches from the other
>> side?  Nothing?
>
> One other thing that I can see being useful occasionally is:
>
>     git rebase @{u}@{1} --onto @{u}
>
> which allows local commits to be replayed onto a rewritten upstream
> branch.

Sure, that would make sense.

I somehow thought that rebase by default looked in the reflog to do
exactly that. Perhaps I am not remembering correctly.

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

[John Keeping]

On Thu, Sep 05, 2013 at 12:18:45PM -0700, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> 
> > On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
> >> Are there cases where you do not want to either rebase nor merge?
> >> If so what do you want to do after "git pull" fetches from the other
> >> side?  Nothing?
> >
> > One other thing that I can see being useful occasionally is:
> >
> >     git rebase @{u}@{1} --onto @{u}
> >
> > which allows local commits to be replayed onto a rewritten upstream
> > branch.
> 
> Sure, that would make sense.
> 
> I somehow thought that rebase by default looked in the reflog to do
> exactly that. Perhaps I am not remembering correctly.

It just does @{upstream} by default, which tends to get messy if the
upstream has been rewritten.

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

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>> From: "Junio C Hamano" <gitster@pobox.com>
>>> John Keeping <john@keeping.me.uk> writes:
>>>
>>>> I think there are two distinct uses for pull, which boil down to:
>>>>
>>>>     (1) git pull
>>> ...
>>> Peff already covered (1)---it is highly doubtful that a merge is
>>> "almost always wrong".  In fact, if that _were_ the case, we should
>>> simply be defaulting to rebase, not failing the command and asking
>>> between merge and rebase like jc/pull-training-wheel topic did.
>>>
>>> We simply do not know what the user wants, as it heavily depends on
>>> the project, so we ask the user to choose one (and stick to it).
>>
>> We only offer a limited list. It won't be sufficient for all use
>> cases. It wasn't for me.
>
> Very interesting. Tell us more.
>
What I do now is avoid Pull because of the hassle of fixing anything
that may have gone wrong.

Instead I now use a 'git fetch', followed by a 'push . (+etc:etc)' once 
I understand what I've got, or what I need to do different if wasn't a 
simple fast forward 'pull'.

> When "git pull" stops because what was fetched in FETCH_HEAD does
> not fast-forward, then what did _you_ do (and with the knowledge you
> currently have, what would you do)?  In a single project, would you
> choose to sometimes rebase and sometimes merge, and if so, what is
> the choice depend on?  "When I am on these selected branches, I want
> to merge, but on other branches I want to rebase?"
>

In my case I have two home machines (main Windows machine and an 
occasional Linux laptop, though not directly networked together) and 
github as my level group, and have MSysGit and git/git (on github) as 
true upstreams, though they haven't been named that way [Aside: we are 
short of a good name for one's 'across-stream server' that one uses for 
backup/transfer such as github].

I general now use a forced update to bring my local machine up to date 
relative to whatever is upstream or on my across stream server, such as 
when transferring development from one machine to the other (where 
overwrite is the desired action) - e.g. when testing on the Linux laptop 
and a few corrections, before patch preparation on the Windows machine 
(different levels of familiarity).

I occasionally will need to rebase my topic onto an updated git/master 
or git/pu if it is to be submitted upstream (patches to the list) or if 
upstream has moved, though I want to choose where I will rebase the 
topic onto. I don't need merging in that scenario, as I see those via 
your git repo ;-)

It's not clear to me that a single default that uses a merge or rebase, 
without a 'stop if' criteria would be of any help in my situation.

My thoughts are that the options on a fetch-pull are for the branch to 
be:
* Overwritte (--force) (i.e. a conflict scenario)
* Stop if not-ff (conflict scenario, this patch series)
* rebase existing onto tracked [not a conflict in terms of initiation]
* merge existing into tracked [not a conflict in terms of initiation]
* fast-forward (bring tracked onto existing) [desired]

Philip

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> I can imagine users might want to say "when I am on these small
> number of branches, I want to merge (or rebase), but when I am on
> other, majority of my branches, because they are private, unfinished
> and unpublished work, please stop me from accidentally messing their
> histories with changes from upstream or anywhere else for that
> matter".  If that is the issue you are trying to raise, because
> there is no
>
> 	[pull]
>         	rebase = fail
> 	[branch "master"]
>         	rebase = yes
>
> to force "git pull" to fail by default on any branch while allowing
> it to rebase (or merge, for that matter) only on a few selected
> branches, we fall a bit short.
>
> Which can be solved by adding the above "fail" option, and then
> renaming them to "pull.integrate" and "branch.<name>.integrate" to
> clarify what these variables are about (it is no longer "do you
> rebase or not---if you choose not to rebase, by definition you are
> going to merge", as there is a third choice to "fail"), while
> retaining "pull.rebase" and "branch.<name>.rebase" as a deprecated
> synonym.

The first step of such an enhancement may look like this patch.  It
introduces "pull.integrate" and "branch.<name>.integrate" that will
eventually deprecate "*.rebase", but at this step only supports
values "rebase" and "merge" (i.e. no "fail" yet).

The steps after this change would be to

 * Enhance addition to t5520 made by 949e0d8e (pull: require choice
   between rebase/merge on non-fast-forward pull, 2013-06-27) to
   make sure that setting pull.integrate and branch.<name>.integrate
   will squelch the safety added by that patch;

 * Teach "branch.c" to set "branch.<name>.integrate" either instead
   of or in addition to "branch.<name>.rebase", and adjust tests
   that expect to see "branch.<name>.rebase" to expect to see that
   "branch.<name>.integrate" is set to "rebase";

 * Add "fail" to the set of valid values for "*.integrate", and teach
   "git pull" honor it; and

 * Update builtin/remote.c to show cases where branch.<name>.integrate
   is set to "fail" in some different way.

I do not plan to do these follow-up steps myself soonish (hint,
hint).

 builtin/remote.c | 12 ++++++++++--
 git-pull.sh      | 60 +++++++++++++++++++++++++++++++++++++-------------------
 2 files changed, 50 insertions(+), 22 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 5e54d36..d3b6d0b5 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -274,7 +274,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, INTEGRATE } type;
 
 		key += 7;
 		if (!postfixcmp(key, ".remote")) {
@@ -286,6 +286,9 @@ static int config_read_branches(const char *key, const char *value, void *cb)
 		} else if (!postfixcmp(key, ".rebase")) {
 			name = xstrndup(key, strlen(key) - 7);
 			type = REBASE;
+		} else if (!postfixcmp(key, ".integrate")) {
+			name = xstrndup(key, strlen(key) - 10);
+			type = INTEGRATE;
 		} else
 			return 0;
 
@@ -309,8 +312,13 @@ 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) {
 			info->rebase = git_config_bool(orig_key, value);
+		} else if (type == INTEGRATE) {
+			if (!value)
+				return config_error_nonbool(orig_key);
+			info->rebase = !strcmp(value, "rebase");
+		}
 	}
 	return 0;
 }
diff --git a/git-pull.sh b/git-pull.sh
index 88c198f..5c557b7 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -45,16 +45,34 @@ merge_args= edit=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
 
-# See if we are configured to rebase by default.
-# The value $rebase is, throughout the main part of the code:
+# See what we are configured to do by default.
+# The value $integration is, throughout the main part of the code:
 #    (empty) - the user did not have any preference
-#    true    - the user told us to integrate by rebasing
-#    false   - the user told us to integrate by merging
-rebase=$(git config --bool branch.$curr_branch_short.rebase)
-if test -z "$rebase"
-then
-	rebase=$(git config --bool pull.rebase)
-fi
+#    rebase  - the user told us to integrate by rebasing
+#    merge   - the user told us to integrate by merging
+
+integration=
+
+set_integration () {
+	integration=$(git config branch.$curr_branch_short.integrate)
+	test -n "$integration" && return
+
+	case "$(git config --bool branch.$curr_branch_short.rebase)" in
+	true)	integration=rebase ;;
+	false)	integration=merge ;;
+	esac
+	test -n "$integration" && return
+
+	integration=$(git config pull.integrate)
+	test -n "$integration" && return
+
+	case "$(git config --bool pull.rebase)" in
+	true)	integration=rebase ;;
+	false)	integration=merge ;;
+	esac
+}
+
+set_integration
 
 dry_run=
 while :
@@ -119,11 +137,11 @@ do
 		merge_args="$merge_args$xx "
 		;;
 	-r|--r|--re|--reb|--reba|--rebas|--rebase)
-		rebase=true
+		integration=rebase
 		;;
 	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase|\
 	-m|--m|--me|--mer|--merg|--merge)
-		rebase=false
+		integration=merge
 		;;
 	--recurse-submodules)
 		recurse_submodules=--recurse-submodules
@@ -166,7 +184,7 @@ error_on_no_merge_candidates () {
 		esac
 	done
 
-	if test true = "$rebase"
+	if test "$integration" = rebase
 	then
 		op_type=rebase
 		op_prep=against
@@ -180,7 +198,8 @@ error_on_no_merge_candidates () {
 	remote=$(git config "branch.$curr_branch.remote")
 
 	if [ $# -gt 1 ]; then
-		if [ "$rebase" = true ]; then
+		if test "$integration" = rebase
+		then
 			printf "There is no candidate for rebasing against "
 		else
 			printf "There are no candidates for merging "
@@ -203,7 +222,8 @@ error_on_no_merge_candidates () {
 	exit 1
 }
 
-test true = "$rebase" && {
+if test "$integration" = rebase
+then
 	if ! git rev-parse -q --verify HEAD >/dev/null
 	then
 		# On an unborn branch
@@ -227,7 +247,7 @@ test true = "$rebase" && {
 			break
 		fi
 	done
-}
+fi
 
 orig_head=$(git rev-parse -q --verify HEAD)
 git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok "$@" || exit 1
@@ -269,7 +289,7 @@ case "$merge_head" in
 	then
 		die "$(gettext "Cannot merge multiple branches into empty head")"
 	fi
-	if test true = "$rebase"
+	if test "$integration" = rebase
 	then
 		die "$(gettext "Cannot rebase onto multiple branches")"
 	fi
@@ -279,7 +299,7 @@ case "$merge_head" in
 	# trigger this check when we will say "fast-forward" or "already
 	# up-to-date".
 	merge_head=${merge_head% }
-	if test -z "$rebase$no_ff$ff_only${squash#--no-squash}" &&
+	if test -z "$integration$no_ff$ff_only${squash#--no-squash}" &&
 		test -n "$orig_head" &&
 		test $# = 0 &&
 		! git merge-base --is-ancestor "$orig_head" "$merge_head" &&
@@ -311,7 +331,7 @@ then
 	exit
 fi
 
-if test true = "$rebase"
+if test "$integration" = rebase
 then
 	o=$(git show-branch --merge-base $curr_branch $merge_head $oldremoteref)
 	if test "$oldremoteref" = "$o"
@@ -321,8 +341,8 @@ then
 fi
 
 merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
-case "$rebase" in
-true)
+case "$integration" in
+rebase)
 	eval="git-rebase $diffstat $strategy_args $merge_args $verbosity"
 	eval="$eval --onto $merge_head ${oldremoteref:-$merge_head}"
 	;;

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

[Junio C Hamano]

"Philip Oakley" <philipoakley@iee.org> writes:

> It's not clear to me that a single default that uses a merge or
> rebase, without a 'stop if' criteria would be of any help in my
> situation.
>
> My thoughts are that the options on a fetch-pull are for the branch to
> be:
> * Overwritte (--force) (i.e. a conflict scenario)
> * Stop if not-ff (conflict scenario, this patch series)
> * rebase existing onto tracked [not a conflict in terms of initiation]
> * merge existing into tracked [not a conflict in terms of initiation]
> * fast-forward (bring tracked onto existing) [desired]

In short, it sounds to me like that the answer to my question is
"what I do depends too much on what I did on my other machine that
is not even directly connected to this matchine, so there is no way
to formulate it as a concrete workflow---I need to inspect what I
get from the central repository and decide the next step anyway, so
I just want 'git pull' not to do anything".

Among the things that were suggested so far (the 'pull' update that
has been cooking in the 'next' branch, Felipe's tightening to apply
the same logic to 'git pull $there $that' as well as 'git pull', and
being able to set "pull.rebase = fail" and renaming the variable to
something like "pull.integrate = fail"), only the last one seems to
be the solution to your particular case.  The other two would not
help such an ad-hoc (non)workflow very much either way.

Am I reading you correctly?  If so, I sent out the first (or zeroth)
step to add something like that separately.

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

[Jonathan Nieder]

John Keeping wrote:
> On Thu, Sep 05, 2013 at 12:18:45PM -0700, Junio C Hamano wrote:

>> I somehow thought that rebase by default looked in the reflog to do
>> exactly that. Perhaps I am not remembering correctly.
>
> It just does @{upstream} by default, which tends to get messy if the
> upstream has been rewritten.

Maybe Junio is thinking of 'git pull --rebase', which walks the reflog
until it finds an ancestor of the current branch and uses that as the
<upstream> parameter to rebase.

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

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> John Keeping wrote:
>> On Thu, Sep 05, 2013 at 12:18:45PM -0700, Junio C Hamano wrote:
>
>>> I somehow thought that rebase by default looked in the reflog to do
>>> exactly that. Perhaps I am not remembering correctly.
>>
>> It just does @{upstream} by default, which tends to get messy if the
>> upstream has been rewritten.
>
> Maybe Junio is thinking of 'git pull --rebase', which walks the reflog
> until it finds an ancestor of the current branch and uses that as the
> <upstream> parameter to rebase.

You're right.

It makes me wonder why we did that one inside pull and not in
rebase, though.

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

[John Keeping]

On Fri, Sep 06, 2013 at 03:14:25PM -0700, Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
> 
> > John Keeping wrote:
> >> On Thu, Sep 05, 2013 at 12:18:45PM -0700, Junio C Hamano wrote:
> >
> >>> I somehow thought that rebase by default looked in the reflog to do
> >>> exactly that. Perhaps I am not remembering correctly.
> >>
> >> It just does @{upstream} by default, which tends to get messy if the
> >> upstream has been rewritten.
> >
> > Maybe Junio is thinking of 'git pull --rebase', which walks the reflog
> > until it finds an ancestor of the current branch and uses that as the
> > <upstream> parameter to rebase.
> 
> You're right.
> 
> It makes me wonder why we did that one inside pull and not in
> rebase, though.

I'd never realised "pull --rebase" does that - it's exactly what I want
sometimes and I normally do fetch followed by rebase to get more control
over the process.

Perhaps we should do something like this (with added tests and
documentation)?

-- >8 --
Subject: [PATCH] rebase: use reflog to find common base with upstream

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 git-rebase.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/git-rebase.sh b/git-rebase.sh
index 8d7659a..5e3013d 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -428,6 +428,14 @@ then
 			error_on_missing_default_upstream "rebase" "rebase" \
 				"against" "git rebase <branch>"
 		fi
+		for reflog in $(git rev-list -g "$upstream_name" 2>/dev/null)
+		do
+			if test "$reflog" = "$(git merge-base "$reflog" HEAD)"
+			then
+				upstream_name=$reflog
+				break
+			fi
+		done
 		;;
 	*)	upstream_name="$1"
 		shift
-- 
1.8.4.239.g2332621

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

[Felipe Contreras]

On Thu, Sep 5, 2013 at 3:06 AM, John Keeping <john@keeping.me.uk> wrote:
> On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
>> Are there cases where you do not want to either rebase nor merge?
>> If so what do you want to do after "git pull" fetches from the other
>> side?  Nothing?
>
> One other thing that I can see being useful occasionally is:
>
>     git rebase @{u}@{1} --onto @{u}
>
> which allows local commits to be replayed onto a rewritten upstream
> branch.
>
> Although I agree with your side note below that people doing this may be
> better off fetching and then updating their local branch, particularly
> if @{1} is not the correct reflog entry for the upstream when they
> created the branch.

That's why after recognizing the fact the you can't find the branch
point of a branch in Git, I decided to write patches to support the
@{tail} shorthand, which is basically the point where the branch was
created, or rebased to:

https://github.com/felipec/git/commits/fc/base

And if 'git rebase' was fixed to ignore the commits already in the
rebased onto branch, almost always what you would want to do is 'git
rebase @{tail} --onto @{upstream}'.

-- 
Felipe Contreras

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

[Felipe Contreras]

On Fri, Sep 6, 2013 at 5:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> John Keeping wrote:
>>> On Thu, Sep 05, 2013 at 12:18:45PM -0700, Junio C Hamano wrote:
>>
>>>> I somehow thought that rebase by default looked in the reflog to do
>>>> exactly that. Perhaps I am not remembering correctly.
>>>
>>> It just does @{upstream} by default, which tends to get messy if the
>>> upstream has been rewritten.
>>
>> Maybe Junio is thinking of 'git pull --rebase', which walks the reflog
>> until it finds an ancestor of the current branch and uses that as the
>> <upstream> parameter to rebase.
>
> You're right.
>
> It makes me wonder why we did that one inside pull and not in
> rebase, though.

Because there's a huge difference between:

git rebase @{u}@{1} --onto @{u}

And

git rebase @{u}

I was in the process of fixing that, but you stopped me.

-- 
Felipe Contreras

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

[Felipe Contreras]

On Wed, Sep 4, 2013 at 5:59 PM, Junio C Hamano <gitster@pobox.com> wrote:

> Which can be solved by adding the above "fail" option, and then
> renaming them to "pull.integrate" and "branch.<name>.integrate" to
> clarify what these variables are about (it is no longer "do you
> rebase or not---if you choose not to rebase, by definition you are
> going to merge", as there is a third choice to "fail"), while
> retaining "pull.rebase" and "branch.<name>.rebase" as a deprecated
> synonym.

All these names are completely unintuitive. First of all, why
"integrate"? Integrate what to what? And then, why "fail"? Fail on
what circumstances? Always?

My proposal that does:

  pull.mode = merge/rebase/merge-ff-only

Is way more intuitive.

-- 
Felipe Contreras

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

[Felipe Contreras]

On Wed, Sep 4, 2013 at 4:25 AM, Jeff King <peff@peff.net> wrote:

> The patch in jc/pull-training-wheel talks about annoying old timers, but
> I think you may also be annoying clueless new users who simply want an
> svn-like workflow without thinking too hard about it.

How? Subversion would complain if you have local changes when you do
'svn pull', there's no notion of remotes, branches and merges are
rare, and forget about rebases.

>> > I do not think we know what we want is to affect "git pull origin".
>>
>> I consider "git pull $remote" to be an artifact of the way git-pull is
>> implemented on top of git-fetch; perhaps I'm missing something but I
>> can't see a scenario where this is useful.
>
> Imagine a workflow where each topic is in its own repository instead of
> in its own branch inside a repository. Or where each developer has his
> or her own repository, but everybody just works on the master branch of
> their repository (or perhaps uses branches, but keeps master as a stable
> base). Alice is the integration manager; Bob tells her that he has work
> ready to integrate.  She runs "git pull ~bob/project", which will merge
> Bob's HEAD.

These integrators should know what they are doing, so they can do 'git
pull --merge', or better 'git config pull.mode merge', as Linus
himself suggested (or something like that).

The defaults should care most about the clueless users.

-- 
Felipe Contreras

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

[Jeff King]

On Sat, Sep 07, 2013 at 09:52:16PM -0500, Felipe Contreras wrote:

> On Wed, Sep 4, 2013 at 4:25 AM, Jeff King <peff@peff.net> wrote:
> 
> > The patch in jc/pull-training-wheel talks about annoying old timers, but
> > I think you may also be annoying clueless new users who simply want an
> > svn-like workflow without thinking too hard about it.
> 
> How? Subversion would complain if you have local changes when you do
> 'svn pull', there's no notion of remotes, branches and merges are
> rare, and forget about rebases.

By "svn-like", I mean the people whose workflow is:

  $ hack hack hack
  $ git commit
  $ git push ;# oops, somebody else pushed in the meantime
  $ git pull
  $ git push

without using branches or worrying about the shape of history. I do not
know what you mean by "svn pull", since that command does not exist
(unless you are talking about svk?). In subversion, that workflow would
be:

  $ hack hack hack
  $ svn commit ;# oops, somebody else committed in the meantime
  $ svn update
  $ svn commit

Those people would now have to learn enough to choose between merge and
rebase when running the "git pull".

It may be OK to say "we do not care about that case, and it is a good
thing that they learn enough to make the choice consciously." But I do
think they exist.

> >> > I do not think we know what we want is to affect "git pull origin".
> >>
> >> I consider "git pull $remote" to be an artifact of the way git-pull is
> >> implemented on top of git-fetch; perhaps I'm missing something but I
> >> can't see a scenario where this is useful.
> >
> > Imagine a workflow where each topic is in its own repository instead of
> > in its own branch inside a repository. Or where each developer has his
> > or her own repository, but everybody just works on the master branch of
> > their repository (or perhaps uses branches, but keeps master as a stable
> > base). Alice is the integration manager; Bob tells her that he has work
> > ready to integrate.  She runs "git pull ~bob/project", which will merge
> > Bob's HEAD.
> 
> These integrators should know what they are doing, so they can do 'git
> pull --merge', or better 'git config pull.mode merge', as Linus
> himself suggested (or something like that).
> 
> The defaults should care most about the clueless users.

In this part of the email you are quoting I was not intending to say
anything about clueless users at all, nor even about what defaults there
are. John indicated that he could not imagine a scenario of "git pull
$remote", so I gave an example.

-Peff

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

[Felipe Contreras]

On Sat, Sep 7, 2013 at 11:18 PM, Jeff King <peff@peff.net> wrote:
> On Sat, Sep 07, 2013 at 09:52:16PM -0500, Felipe Contreras wrote:
>
>> On Wed, Sep 4, 2013 at 4:25 AM, Jeff King <peff@peff.net> wrote:
>>
>> > The patch in jc/pull-training-wheel talks about annoying old timers, but
>> > I think you may also be annoying clueless new users who simply want an
>> > svn-like workflow without thinking too hard about it.
>>
>> How? Subversion would complain if you have local changes when you do
>> 'svn pull', there's no notion of remotes, branches and merges are
>> rare, and forget about rebases.
>
> By "svn-like", I mean the people whose workflow is:
>
>   $ hack hack hack
>   $ git commit
>   $ git push ;# oops, somebody else pushed in the meantime
>   $ git pull
>   $ git push

But that's not svn-like at all.

> without using branches or worrying about the shape of history. I do not
> know what you mean by "svn pull", since that command does not exist
> (unless you are talking about svk?). In subversion, that workflow would
> be:
>
>   $ hack hack hack
>   $ svn commit ;# oops, somebody else committed in the meantime
>   $ svn update
>   $ svn commit
>
> Those people would now have to learn enough to choose between merge and
> rebase when running the "git pull".

But that's only if they don't care about the shape of history. In my
experience the people that cling more to centralized VCS do not like
merges, so they rebase everything to make it a straight line. That is
much more "svn-like".

So chances are they are already doing 'git pull --rebase' (or
similar), so their workflow wouldn't be affected.

> It may be OK to say "we do not care about that case, and it is a good
> thing that they learn enough to make the choice consciously." But I do
> think they exist.

Yeah, I'm sure they exist, but they are a tiny minority compared to
the amount of people who don't actually understand what 'git pull' is
doing and do merges by mistake.

-- 
Felipe Contreras

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

[Jeff King]

On Sat, Sep 07, 2013 at 11:37:13PM -0500, Felipe Contreras wrote:

> > By "svn-like", I mean the people whose workflow is:
> >
> >   $ hack hack hack
> >   $ git commit
> >   $ git push ;# oops, somebody else pushed in the meantime
> >   $ git pull
> >   $ git push
> 
> But that's not svn-like at all.

It's not if you understand the difference between merge-then-commit and
commit-then-merge. But for a clueless user who has been told "replace
svn commit" with "git commit && git push" and replace "svn update" with
"git pull", it is quite similar.

> > Those people would now have to learn enough to choose between merge and
> > rebase when running the "git pull".
> 
> But that's only if they don't care about the shape of history. In my
> experience the people that cling more to centralized VCS do not like
> merges, so they rebase everything to make it a straight line. That is
> much more "svn-like".
> 
> So chances are they are already doing 'git pull --rebase' (or
> similar), so their workflow wouldn't be affected.

I think we are talking about two classes of users. People who truly
don't care about the shape of history will also not care about using
"git pull --rebase", because the only reason to use it is to impact the
shape of history.

I agree there is also a set of people coming from the centralized vcs
world who want to keep a linear history.

-Peff

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

[Felipe Contreras]

On Sat, Sep 7, 2013 at 11:43 PM, Jeff King <peff@peff.net> wrote:
> On Sat, Sep 07, 2013 at 11:37:13PM -0500, Felipe Contreras wrote:
>
>> > By "svn-like", I mean the people whose workflow is:
>> >
>> >   $ hack hack hack
>> >   $ git commit
>> >   $ git push ;# oops, somebody else pushed in the meantime
>> >   $ git pull
>> >   $ git push
>>
>> But that's not svn-like at all.
>
> It's not if you understand the difference between merge-then-commit and
> commit-then-merge. But for a clueless user who has been told "replace
> svn commit" with "git commit && git push" and replace "svn update" with
> "git pull", it is quite similar.

Well, yeah, but if they are so clueless they have to be told what to
do, they can be told to do 'git pull --merge' instead, no?

>> > Those people would now have to learn enough to choose between merge and
>> > rebase when running the "git pull".
>>
>> But that's only if they don't care about the shape of history. In my
>> experience the people that cling more to centralized VCS do not like
>> merges, so they rebase everything to make it a straight line. That is
>> much more "svn-like".
>>
>> So chances are they are already doing 'git pull --rebase' (or
>> similar), so their workflow wouldn't be affected.
>
> I think we are talking about two classes of users. People who truly
> don't care about the shape of history will also not care about using
> "git pull --rebase", because the only reason to use it is to impact the
> shape of history.
>
> I agree there is also a set of people coming from the centralized vcs
> world who want to keep a linear history.

Yeah, and based on the evidence, one set of people is much much larger
than the other; the people that care what the history look like.

Either way, we can start by making it a warning, and then an error,
and if more people complain that they have to do 'git pull --merge'
now (I bet there won't be any), then you would be right, and we
revert. No problem.

-- 
Felipe Contreras

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

[Jeff King]

On Sun, Sep 08, 2013 at 12:09:34AM -0500, Felipe Contreras wrote:

> > It's not if you understand the difference between merge-then-commit and
> > commit-then-merge. But for a clueless user who has been told "replace
> > svn commit" with "git commit && git push" and replace "svn update" with
> > "git pull", it is quite similar.
> 
> Well, yeah, but if they are so clueless they have to be told what to
> do, they can be told to do 'git pull --merge' instead, no?

I think it's fine to tell them to do "git pull --merge". What I'd worry
more about is somebody who is suddenly presented with the choice between
"--rebase" and "--merge" and doesn't know which to choose. We've created a
cognitive load on the user, and even more load if they choose --rebase
and don't quite understand what it means.

The current warning message in jc/pull-training-wheel is quite neutral
between the two options. Perhaps we should lean more towards merging?

I guess that works against John's case, though, which is clueless people
working on a project that _does_ care about the shape of history. At
least they would have to stop and think for a moment, though, which
might help (and maybe convince them to ask more clueful project
members). I don't know.

-Peff

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

[Felipe Contreras]

On Sun, Sep 8, 2013 at 12:21 AM, Jeff King <peff@peff.net> wrote:
> On Sun, Sep 08, 2013 at 12:09:34AM -0500, Felipe Contreras wrote:
>
>> > It's not if you understand the difference between merge-then-commit and
>> > commit-then-merge. But for a clueless user who has been told "replace
>> > svn commit" with "git commit && git push" and replace "svn update" with
>> > "git pull", it is quite similar.
>>
>> Well, yeah, but if they are so clueless they have to be told what to
>> do, they can be told to do 'git pull --merge' instead, no?
>
> I think it's fine to tell them to do "git pull --merge". What I'd worry
> more about is somebody who is suddenly presented with the choice between
> "--rebase" and "--merge" and doesn't know which to choose. We've created a
> cognitive load on the user, and even more load if they choose --rebase
> and don't quite understand what it means.

If that happens they will go back to the guy that told them to run
those commands.

Fortunately there probably are very few of these users.

> The current warning message in jc/pull-training-wheel is quite neutral
> between the two options. Perhaps we should lean more towards merging?

I don't like that message. I would like this for the deprecation period:

"The pull was not fast-forward, in the future you would have to choose
a merge or a rebase, merging automatically for now. Read 'man git
pull' for more help."

Then when obsolete:

The pull was not fast-forward, please either merge or rebase.

"Any more babysitting with essay long messages is counter-productive
to the vast majority of Git users."

> I guess that works against John's case, though, which is clueless people
> working on a project that _does_ care about the shape of history. At
> least they would have to stop and think for a moment, though, which
> might help (and maybe convince them to ask more clueful project
> members). I don't know.

Or google 'git pull' 'git merge' 'git rebase' or 'git non-fast-forward'.

-- 
Felipe Contreras

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

[Richard Hansen]

On 2013-09-07 22:41, Felipe Contreras wrote:
> On Wed, Sep 4, 2013 at 5:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 
>> Which can be solved by adding the above "fail" option, and then
>> renaming them to "pull.integrate" and "branch.<name>.integrate" to
>> clarify what these variables are about (it is no longer "do you
>> rebase or not---if you choose not to rebase, by definition you are
>> going to merge", as there is a third choice to "fail"), while
>> retaining "pull.rebase" and "branch.<name>.rebase" as a deprecated
>> synonym.
> 
> All these names are completely unintuitive. First of all, why
> "integrate"? Integrate what to what? And then, why "fail"? Fail on
> what circumstances? Always?
> 
> My proposal that does:
> 
>   pull.mode = merge/rebase/merge-ff-only
> 
> Is way more intuitive.

+1

What about something like:

    pull.mergeoptions (defaults to --ff-only)
    pull.rebaseoptions (defaults to empty?  --preserve-merges?)
    branch.<name>.pull.mergeoptions (defaults to pull.mergeoptions)
    branch.<name>.pull.rebaseoptions (defaults to pull.rebaseoptions)

-Richard

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

[Jeff King]

On Sun, Sep 08, 2013 at 01:17:42AM -0500, Felipe Contreras wrote:

> > I think it's fine to tell them to do "git pull --merge". What I'd worry
> > more about is somebody who is suddenly presented with the choice between
> > "--rebase" and "--merge" and doesn't know which to choose. We've created a
> > cognitive load on the user, and even more load if they choose --rebase
> > and don't quite understand what it means.
> 
> If that happens they will go back to the guy that told them to run
> those commands.

I think "the guy" may be git itself. For example, here is a possible
session with jc/pull-training-wheel:

  $ git push
  To ...
   ! [rejected]        master -> master (non-fast-forward)
  error: failed to push some refs to '...'
  hint: Updates were rejected because the tip of your current branch is behind
  hint: its remote counterpart. Integrate the remote changes (e.g.
  hint: 'git pull ...') before pushing again.
  hint: See the 'Note about fast-forwards' in 'git push --help' for details.

  $ git pull
  The pull does not fast-forward; please specify
  if you want to merge or rebase.

  Use either

      git pull --rebase
      git pull --merge

  You can also use 'git config pull.rebase true' (if you want --rebase) or
  'git config pull.rebase false' (if you want --merge) to set this once for
  this project and forget about it.

The user is pointed at "pull" from "push", and then gets presented with
the "merge or rebase" choice. It may be that the advice you can find by
googling "merge vs rebase" is enough to then help the person along
(and/or we may need to improve the manpages in that respect).

I am genuinely curious what people in favor of this feature would want
to say in the documentation to a user encountering this choice for the
first time. In my experience, rebasing introduces more complications,
specifically:

  1. the merge is "backwards" with respect to ours/theirs

  2. you may end up with difficult conflict resolution due to repeated
     changes over the same section of code. E.g., you write some buggy
     code and then fix it, but upstream has changed the same area.
     Rebasing involves first resolving your buggy version with the
     upstream code, and then resolving the fix on top of the previous
     resolution.

  3. rewriting of commits found in other branches, which then need
     rebased on top of the branch you just rebased

  4. a previously bug-free commit can show a bug after the rebase if
     other parts of the project changed (whereas with a merge, the bug
     would be attributable to the merge)

I know those are all balanced by some advantages of rebasing, but I also
think they are things that can be troublesome for a user who does not
fully grok the rebase process. I'm just wondering if we should mention
both, but steer people towards merging as the safer alternative (you
might have ugly history, but you are less likely to create a mess with
duplicate commits or badly-resolved conflicts).

> Fortunately there probably are very few of these users.

Maybe. I am not sure how one would measure.

If you are interested, I can ask the opinion of some of the GitHub
trainers. They see a lot of new users and have a sense of what kinds of
confusion come up most frequently, what kinds of workflows they tend to
see, etc. Their experience may be biased towards corporate-ish users,
though, because those are the people who pay for training.

> > The current warning message in jc/pull-training-wheel is quite neutral
> > between the two options. Perhaps we should lean more towards merging?
> 
> I don't like that message. I would like this for the deprecation period:
> 
> "The pull was not fast-forward, in the future you would have to choose
> a merge or a rebase, merging automatically for now. Read 'man git
> pull' for more help."
> 
> Then when obsolete:
> 
> The pull was not fast-forward, please either merge or rebase.

A deprecation message helps people who are making the transition from an
older behavior to a newer one. It cannot help new users who start with a
git version after the deprecation period.

> "Any more babysitting with essay long messages is counter-productive
> to the vast majority of Git users."

I think that is what we have advice.* for.

-Peff

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

[Felipe Contreras]

On Sun, Sep 8, 2013 at 1:54 AM, Jeff King <peff@peff.net> wrote:
> On Sun, Sep 08, 2013 at 01:17:42AM -0500, Felipe Contreras wrote:
>
>> > I think it's fine to tell them to do "git pull --merge". What I'd worry
>> > more about is somebody who is suddenly presented with the choice between
>> > "--rebase" and "--merge" and doesn't know which to choose. We've created a
>> > cognitive load on the user, and even more load if they choose --rebase
>> > and don't quite understand what it means.
>>
>> If that happens they will go back to the guy that told them to run
>> those commands.
>
> I think "the guy" may be git itself. For example, here is a possible
> session with jc/pull-training-wheel:
>
>   $ git push

Who told him to use 'git push'? Certainly not git.

>   To ...
>    ! [rejected]        master -> master (non-fast-forward)
>   error: failed to push some refs to '...'
>   hint: Updates were rejected because the tip of your current branch is behind
>   hint: its remote counterpart. Integrate the remote changes (e.g.
>   hint: 'git pull ...') before pushing again.
>   hint: See the 'Note about fast-forwards' in 'git push --help' for details.
>
>   $ git pull

>   The pull does not fast-forward; please specify
>   if you want to merge or rebase.
>
>   Use either
>
>       git pull --rebase
>       git pull --merge
>
>   You can also use 'git config pull.rebase true' (if you want --rebase) or
>   'git config pull.rebase false' (if you want --merge) to set this once for
>   this project and forget about it.

Why stop there? Post the whole man page already.

Moreover, it's overly verbose on all the wrong and irrelevant
information. If you are going to waste precious screen state, explain
wth a "non fast-forward" is; people can figure out what a merge is,
and maybe a rebase, but a "non fast-forward" definitely not.

> The user is pointed at "pull" from "push", and then gets presented with
> the "merge or rebase" choice. It may be that the advice you can find by
> googling "merge vs rebase" is enough to then help the person along
> (and/or we may need to improve the manpages in that respect).

Yes, but that's not the use-case we are talking about. You mentioned
specifically a "svn-like" worfklow where the guy was told by somebody
else to replace the svn commands with git ones.

If we are talking about a guy that is learning git, that's and
entirely different case.

> I am genuinely curious what people in favor of this feature would want
> to say in the documentation to a user encountering this choice for the
> first time. In my experience, rebasing introduces more complications,
> specifically:

Yes, but it's what the user might want.

> I know those are all balanced by some advantages of rebasing, but I also
> think they are things that can be troublesome for a user who does not
> fully grok the rebase process. I'm just wondering if we should mention
> both, but steer people towards merging as the safer alternative (you
> might have ugly history, but you are less likely to create a mess with
> duplicate commits or badly-resolved conflicts).

The purpose of this change in the code is not to change the user
behavior. The choice of merge vs. rebase is entirely up to the user,
and we are not changing that.

The purpose of this change is to avoid doing a merge when the user
wanted a rebase, or maybe something more complicated. So a rebase
being complicated is not an issue, because we know that's what the
user wants, that's the whole reason we are trying to avoid the
automated merge.

>> Fortunately there probably are very few of these users.
>
> Maybe. I am not sure how one would measure.
>
> If you are interested, I can ask the opinion of some of the GitHub
> trainers. They see a lot of new users and have a sense of what kinds of
> confusion come up most frequently, what kinds of workflows they tend to
> see, etc. Their experience may be biased towards corporate-ish users,
> though, because those are the people who pay for training.

Ask. I'm sure they will tell you doing merges by mistake with 'git
pull' is an issue.

>> > The current warning message in jc/pull-training-wheel is quite neutral
>> > between the two options. Perhaps we should lean more towards merging?
>>
>> I don't like that message. I would like this for the deprecation period:
>>
>> "The pull was not fast-forward, in the future you would have to choose
>> a merge or a rebase, merging automatically for now. Read 'man git
>> pull' for more help."
>>
>> Then when obsolete:
>>
>> The pull was not fast-forward, please either merge or rebase.
>
> A deprecation message helps people who are making the transition from an
> older behavior to a newer one. It cannot help new users who start with a
> git version after the deprecation period.

The new users are told to either merge or rebase, if they don't know
what that means, they will go on look it up, just like they looked up
the 'git pull' command in the first place.

>> "Any more babysitting with essay long messages is counter-productive
>> to the vast majority of Git users."
>
> I think that is what we have advice.* for.

I don't understand what that means.

-- 
Felipe Contreras

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

[Jeff King]

On Sun, Sep 08, 2013 at 02:15:17AM -0500, Felipe Contreras wrote:

> > I think "the guy" may be git itself. For example, here is a possible
> > session with jc/pull-training-wheel:
> >
> >   $ git push
> 
> Who told him to use 'git push'? Certainly not git.

Any of the hundreds of existing tutorials that teach basic git commands
like "push"?

> >   To ...
> >    ! [rejected]        master -> master (non-fast-forward)
> >   error: failed to push some refs to '...'
> >   hint: Updates were rejected because the tip of your current branch is behind
> >   hint: its remote counterpart. Integrate the remote changes (e.g.
> >   hint: 'git pull ...') before pushing again.
> >   hint: See the 'Note about fast-forwards' in 'git push --help' for details.
> [...]
> 
> Why stop there? Post the whole man page already.
> 
> Moreover, it's overly verbose on all the wrong and irrelevant
> information. If you are going to waste precious screen state, explain
> wth a "non fast-forward" is; people can figure out what a merge is,
> and maybe a rebase, but a "non fast-forward" definitely not.

Note that I was not trying to defend any of the messages, but only
showing a plausible mechanism by which a user with basic knowledge that
he wants to push may arrive at the question "what is the difference
between merge and rebase?".

If you want to suggest revisions for the push message, go ahead. The
push advice _is_ an attempt to define non-fast-forwards in plain
language without taking up too much space, but perhaps you can do
better. You could even suggest omitting it entirely, but I'm not sure if
that is a good idea. It was not added in a vacuum; we lacked that advice
for many years, and people complained about it quite a bit until it was
added.

> > The user is pointed at "pull" from "push", and then gets presented with
> > the "merge or rebase" choice. It may be that the advice you can find by
> > googling "merge vs rebase" is enough to then help the person along
> > (and/or we may need to improve the manpages in that respect).
> 
> Yes, but that's not the use-case we are talking about. You mentioned
> specifically a "svn-like" worfklow where the guy was told by somebody
> else to replace the svn commands with git ones.

No, I mentioned an "svn-like" workflow. I didn't say anything about how
they were told. They might have been told by a co-worker, or read a
brief tutorial on git, or read something like "Git-SVN Crash Course".

> If we are talking about a guy that is learning git, that's and
> entirely different case.

That is certainly what I meant to be talking about.

> The purpose of this change in the code is not to change the user
> behavior. The choice of merge vs. rebase is entirely up to the user,
> and we are not changing that.

Right, but by not doing anything by default, you are forcing the user to
make a decision. Right now, we strongly encourage merging by making it
the default, and you have to learn about rebasing separately. But a
message that mentions them both as equals is going to lead to extra work
for the user; they have to figure out which one is most appropriate. My
concern is that this is non-trivial for new users, and that they may end
up arbitrarily picking rebase, which is probably not doing them any
favors if they do not understand it.

For clueful users, choosing between the two is not hard. But some people
seem to have trouble understanding the DAG. I don't know how large a
group that is, and how any pain caused by this change might compare to
the times it will help.

> > If you are interested, I can ask the opinion of some of the GitHub
> > trainers. They see a lot of new users and have a sense of what kinds of
> > confusion come up most frequently, what kinds of workflows they tend to
> > see, etc. Their experience may be biased towards corporate-ish users,
> > though, because those are the people who pay for training.
> 
> Ask. I'm sure they will tell you doing merges by mistake with 'git
> pull' is an issue.

I've sent an email. I'll post the response when I get it.

> >> "Any more babysitting with essay long messages is counter-productive
> >> to the vast majority of Git users."
> >
> > I think that is what we have advice.* for.
> 
> I don't understand what that means.

It means that some time ago, after many people complained that git did
not give enough hints, we added many hints. Some people who did not need
these hints would want to disable them, and we have the "advice.*"
config options to do so. So we can have a longer message for new users,
and a shorter one for people who do not want to be bothered with the
long advice.

-Peff

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

[Philip Oakley]

From: "Felipe Contreras" <felipe.contreras@gmail.com>
Sent: Sunday, September 08, 2013 3:34 AM
> On Thu, Sep 5, 2013 at 3:06 AM, John Keeping <john@keeping.me.uk> 
> wrote:
>> On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
>>> Are there cases where you do not want to either rebase nor merge?
>>> If so what do you want to do after "git pull" fetches from the other
>>> side?  Nothing?
>>
>> One other thing that I can see being useful occasionally is:
>>
>>     git rebase @{u}@{1} --onto @{u}
>>
>> which allows local commits to be replayed onto a rewritten upstream
>> branch.
>>
>> Although I agree with your side note below that people doing this may 
>> be
>> better off fetching and then updating their local branch, 
>> particularly
>> if @{1} is not the correct reflog entry for the upstream when they
>> created the branch.
>
> That's why after recognizing the fact the you can't find the branch
> point of a branch in Git, I decided to write patches to support the
> @{tail} shorthand, which is basically the point where the branch was
> created, or rebased to:
>
> https://github.com/felipec/git/commits/fc/base
>
> And if 'git rebase' was fixed to ignore the commits already in the
> rebased onto branch, almost always what you would want to do is 'git
> rebase @{tail} --onto @{upstream}'.
>
The use case that trips me up (i.e. doesn't fit the above) is when I 
have a branch that may need rebasing on (onto) pu, or may need rebasing 
on master, or next, depending on what others have been doing.

As a Distributed VCS (i.e. others doing work independently), a rebase 
always has the possibility that the world has moved on and one has to 
adapt to the new world order by moving location (--onto somewhere new), 
not just fixing up the house (patch conflicts). When the update order is 
unknown there is no guaranteed solution (IIUC).

You are right that mostly what one wants to do is stick with ones 
current location and patch up conflicts, it's just that one din't want 
any conflicts in the first place (i.e. the fast forward check). 

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

[Felipe Contreras]

On Sun, Sep 8, 2013 at 3:01 AM, Philip Oakley <philipoakley@iee.org> wrote:
> From: "Felipe Contreras" <felipe.contreras@gmail.com>
> Sent: Sunday, September 08, 2013 3:34 AM
>
>> On Thu, Sep 5, 2013 at 3:06 AM, John Keeping <john@keeping.me.uk> wrote:
>>>
>>> On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
>>>>
>>>> Are there cases where you do not want to either rebase nor merge?
>>>> If so what do you want to do after "git pull" fetches from the other
>>>> side?  Nothing?
>>>
>>>
>>> One other thing that I can see being useful occasionally is:
>>>
>>>     git rebase @{u}@{1} --onto @{u}
>>>
>>> which allows local commits to be replayed onto a rewritten upstream
>>> branch.
>>>
>>> Although I agree with your side note below that people doing this may be
>>> better off fetching and then updating their local branch, particularly
>>> if @{1} is not the correct reflog entry for the upstream when they
>>> created the branch.
>>
>>
>> That's why after recognizing the fact the you can't find the branch
>> point of a branch in Git, I decided to write patches to support the
>> @{tail} shorthand, which is basically the point where the branch was
>> created, or rebased to:
>>
>> https://github.com/felipec/git/commits/fc/base
>>
>> And if 'git rebase' was fixed to ignore the commits already in the
>> rebased onto branch, almost always what you would want to do is 'git
>> rebase @{tail} --onto @{upstream}'.
>>
> The use case that trips me up (i.e. doesn't fit the above) is when I have a
> branch that may need rebasing on (onto) pu, or may need rebasing on master,
> or next, depending on what others have been doing.

Yes, so you would do:

% git rebase --onto pu

Which would be translated to:

% git rebase @{tail} --onto pu

What's the problem?

> As a Distributed VCS (i.e. others doing work independently), a rebase always
> has the possibility that the world has moved on and one has to adapt to the
> new world order by moving location (--onto somewhere new), not just fixing
> up the house (patch conflicts). When the update order is unknown there is no
> guaranteed solution (IIUC).

Yeah, but almost always you want to rebase onto @{upstream}.

-- 
Felipe Contreras

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

[Philip Oakley]

From: "Felipe Contreras" <felipe.contreras@gmail.com>
To: "Philip Oakley" <philipoakley@iee.org>
Cc: "John Keeping" <john@keeping.me.uk>; "Junio C Hamano" 
<gitster@pobox.com>; <git@vger.kernel.org>; "Andreas Krey" 
<a.krey@gmx.de>
Sent: Sunday, September 08, 2013 9:16 AM
Subject: Re: [PATCH 0/3] Reject non-ff pulls by default


> On Sun, Sep 8, 2013 at 3:01 AM, Philip Oakley <philipoakley@iee.org> 
> wrote:
>> From: "Felipe Contreras" <felipe.contreras@gmail.com>
>> Sent: Sunday, September 08, 2013 3:34 AM
>>
>>> On Thu, Sep 5, 2013 at 3:06 AM, John Keeping <john@keeping.me.uk> 
>>> wrote:
>>>>
>>>> On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
>>>>>
>>>>> Are there cases where you do not want to either rebase nor merge?
>>>>> If so what do you want to do after "git pull" fetches from the 
>>>>> other
>>>>> side?  Nothing?
>>>>
>>>>
>>>> One other thing that I can see being useful occasionally is:
>>>>
>>>>     git rebase @{u}@{1} --onto @{u}
>>>>
>>>> which allows local commits to be replayed onto a rewritten upstream
>>>> branch.
>>>>
>>>> Although I agree with your side note below that people doing this 
>>>> may be
>>>> better off fetching and then updating their local branch, 
>>>> particularly
>>>> if @{1} is not the correct reflog entry for the upstream when they
>>>> created the branch.
>>>
>>>
>>> That's why after recognizing the fact the you can't find the branch
>>> point of a branch in Git, I decided to write patches to support the
>>> @{tail} shorthand, which is basically the point where the branch was
>>> created, or rebased to:
>>>
>>> https://github.com/felipec/git/commits/fc/base
>>>
>>> And if 'git rebase' was fixed to ignore the commits already in the
>>> rebased onto branch, almost always what you would want to do is 'git
>>> rebase @{tail} --onto @{upstream}'.
>>>
>> The use case that trips me up (i.e. doesn't fit the above) is when I 
>> have a
>> branch that may need rebasing on (onto) pu, or may need rebasing on 
>> master,
>> or next, depending on what others have been doing.
>
> Yes, so you would do:
>
> % git rebase --onto pu
>
> Which would be translated to:
>
> % git rebase @{tail} --onto pu
>
> What's the problem?
>
The 'problem' is (would be) that I don't yet know that I would need 
the --onto pu until I discover (how?) that the default rebase would 
result in conflicts.

>> As a Distributed VCS (i.e. others doing work independently), a rebase 
>> always
>> has the possibility that the world has moved on and one has to adapt 
>> to the
>> new world order by moving location (--onto somewhere new), not just 
>> fixing
>> up the house (patch conflicts). When the update order is unknown 
>> there is no
>> guaranteed solution (IIUC).
>
> Yeah, but almost always you want to rebase onto @{upstream}.

Yeah, but almost always you want to "check" first *before* starting. 
That is, 'git rebase --abort' should not be required from the (user's 
selected /git's) default invocation.

--
Philip Oakley 

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

[Felipe Contreras]

On Sun, Sep 8, 2013 at 2:50 AM, Jeff King <peff@peff.net> wrote:
> On Sun, Sep 08, 2013 at 02:15:17AM -0500, Felipe Contreras wrote:
>
>> > I think "the guy" may be git itself. For example, here is a possible
>> > session with jc/pull-training-wheel:
>> >
>> >   $ git push
>>
>> Who told him to use 'git push'? Certainly not git.
>
> Any of the hundreds of existing tutorials that teach basic git commands
> like "push"?

You can't use a tutorial out there that just tells you to replace svn
commands with git alternatives, go to work and mess up the repository
history.

I'm trying to take the point of view of your hypothetical user working
on a repository where history is not important, but it seems more and
more than this person is just not real. If it's OK to push crappy
merges, somebody must have told him that was OK and provided him with
the commands.

If it's just some random person that read some random tutorial from
'svn' -> 'git' working on a random repository that happens to accept
merges all over the place. Well I think that's a very very exceptional
situation.

And this person still wouldn't have a problem finding another tutorial
explaining what a merge is.

>> >   To ...
>> >    ! [rejected]        master -> master (non-fast-forward)
>> >   error: failed to push some refs to '...'
>> >   hint: Updates were rejected because the tip of your current branch is behind
>> >   hint: its remote counterpart. Integrate the remote changes (e.g.
>> >   hint: 'git pull ...') before pushing again.
>> >   hint: See the 'Note about fast-forwards' in 'git push --help' for details.
>> [...]
>>
>> Why stop there? Post the whole man page already.
>>
>> Moreover, it's overly verbose on all the wrong and irrelevant
>> information. If you are going to waste precious screen state, explain
>> wth a "non fast-forward" is; people can figure out what a merge is,
>> and maybe a rebase, but a "non fast-forward" definitely not.
>
> Note that I was not trying to defend any of the messages, but only
> showing a plausible mechanism by which a user with basic knowledge that
> he wants to push may arrive at the question "what is the difference
> between merge and rebase?".

Yes, and this person would have to read it online, like everything
else, because clearly Git documentation would do a bad job at it.
That's why the online documentation was needed in the first place.

The first hits of 'git merge vs rebase' are rather useful:
http://mislav.uniqpath.com/2013/02/merge-vs-rebase/
http://stackoverflow.com/questions/16336014/git-merge-vs-rebase
http://www.derekgourlay.com/archives/428
http://blog.sourcetreeapp.com/2012/08/21/merge-or-rebase/
http://git-scm.com/book/en/Git-Branching-Rebasing

Notice how none of the results point to official documentation,
precisely because it's not really useful.

> If you want to suggest revisions for the push message, go ahead. The
> push advice _is_ an attempt to define non-fast-forwards in plain
> language without taking up too much space, but perhaps you can do
> better.

I definitely can, but you would disagree.

But anyway, you are relying on the user having pushed first, what if
he is pulling first, or what if he doesn't have write access and is
only pulling?

> You could even suggest omitting it entirely, but I'm not sure if
> that is a good idea. It was not added in a vacuum; we lacked that advice
> for many years, and people complained about it quite a bit until it was
> added.

I would have to see the evidence, as I have never seen any complaints
about that. The complains are about the UI, and they still remain.

>> > The user is pointed at "pull" from "push", and then gets presented with
>> > the "merge or rebase" choice. It may be that the advice you can find by
>> > googling "merge vs rebase" is enough to then help the person along
>> > (and/or we may need to improve the manpages in that respect).
>>
>> Yes, but that's not the use-case we are talking about. You mentioned
>> specifically a "svn-like" worfklow where the guy was told by somebody
>> else to replace the svn commands with git ones.
>
> No, I mentioned an "svn-like" workflow. I didn't say anything about how
> they were told. They might have been told by a co-worker, or read a
> brief tutorial on git, or read something like "Git-SVN Crash Course".

Once again, this doesn't make any sense. People can't just push crap
merges to any repository.

>> If we are talking about a guy that is learning git, that's and
>> entirely different case.
>
> That is certainly what I meant to be talking about.

If he is learning Git, then he will be looking for the meaning of a
merge and a rebase. The fact that the repository accepts crappy merges
wouldn't be relevant.

>> The purpose of this change in the code is not to change the user
>> behavior. The choice of merge vs. rebase is entirely up to the user,
>> and we are not changing that.
>
> Right, but by not doing anything by default, you are forcing the user to
> make a decision.

No, it would be a warning first, he wouldn't be *forced* to make a
decision, only after the deprecation period is over.

Then yes, if by then he hasn't learned that what he wants is a merge,
he would be forced to learn it.

> Right now, we strongly encourage merging by making it
> the default, and you have to learn about rebasing separately. But a
> message that mentions them both as equals is going to lead to extra work
> for the user; they have to figure out which one is most appropriate.

No, they don't need to figure out which is most appropriate, they only
need to figure out they have been doing merges all along.

My warning message achieves precisely that:

"The pull was not fast-forward, in the future you would have to choose
a merge or a rebase, merging automatically for now. For more
information read 'git
pull --help'."

The part "merging automatically for now". This teaches the user that
'git pull' is doing a merge, so by the time 'git pull' errors out, he
knows he wants a merge, all he needs to figure out is how to do it,
and 'git pull --help' would tell him that. Perhaps adding a "(git pull
--merge)" to the deprecation warning would help, but I still don't see
the need in the final error.

Once again, nobody is forcing anybody to change their workflows.

> My
> concern is that this is non-trivial for new users, and that they may end
> up arbitrarily picking rebase, which is probably not doing them any
> favors if they do not understand it.

Why would they pick a rebase? If git tells them 'git pull' is doing a
merge for months, why would they choose to do something different?

> For clueful users, choosing between the two is not hard. But some people
> seem to have trouble understanding the DAG. I don't know how large a
> group that is, and how any pain caused by this change might compare to
> the times it will help.

They don't need to learn what's more appropriate, they can keep doing
what they have been doing.

>> > If you are interested, I can ask the opinion of some of the GitHub
>> > trainers. They see a lot of new users and have a sense of what kinds of
>> > confusion come up most frequently, what kinds of workflows they tend to
>> > see, etc. Their experience may be biased towards corporate-ish users,
>> > though, because those are the people who pay for training.
>>
>> Ask. I'm sure they will tell you doing merges by mistake with 'git
>> pull' is an issue.
>
> I've sent an email. I'll post the response when I get it.
>
>> >> "Any more babysitting with essay long messages is counter-productive
>> >> to the vast majority of Git users."
>> >
>> > I think that is what we have advice.* for.
>>
>> I don't understand what that means.
>
> It means that some time ago, after many people complained that git did
> not give enough hints, we added many hints. Some people who did not need
> these hints would want to disable them, and we have the "advice.*"
> config options to do so. So we can have a longer message for new users,
> and a shorter one for people who do not want to be bothered with the
> long advice.

I don't see Junio's proposal being affected by this advice thing.

And I have used and contributed to Git for many years, used it since
day one, and this is the first time I hear about it. I doubt even a
tiny fraction of Git users know about it. Where is the documentation
about that?

-- 
Felipe Contreras

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

[Felipe Contreras]

On Sun, Sep 8, 2013 at 3:42 AM, Philip Oakley <philipoakley@iee.org> wrote:

> The 'problem' is (would be) that I don't yet know that I would need the
> --onto pu until I discover (how?) that the default rebase would result in
> conflicts.

I don't see what that has to do with an invocation of 'git rebase'
without arguments, and @{tail}. There's absolutely no way Git can
figure out for you which is the appropriate place for you to rebase
onto.

However, it shouldn't be too difficult to write a tool that checks
multiple commits and tells you on top of which ones a rebase could
work, but I don't think 'git rebase' is the right place.

-- 
Felipe Contreras

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

[Philip Oakley]

From: "Felipe Contreras" <felipe.contreras@gmail.com>
Sent: Sunday, September 08, 2013 9:49 AM
> On Sun, Sep 8, 2013 at 3:42 AM, Philip Oakley <philipoakley@iee.org> 
> wrote:
>
>> The 'problem' is (would be) that I don't yet know that I would need 
>> the
>> --onto pu until I discover (how?) that the default rebase would 
>> result in
>> conflicts.
>
> I don't see what that has to do with an invocation of 'git rebase'
> without arguments, and @{tail}.

>         There's absolutely no way Git can
> figure out for you which is the appropriate place for you to rebase
> onto.
.. which was my point. I may not have explained it that well.

Given that Git can't figure it out, we should stop trying in such cases.

>
> However, it shouldn't be too difficult to write a tool that checks
> multiple commits and tells you on top of which ones a rebase could
> work, but I don't think 'git rebase' is the right place.

That's an SOS approach (Success Oriented Script)[1] that presumes the 
user is already better than they are - The Kruger Dunning paper [2] 
offers some insight into capability misconceptions (at all levels).

--
regards

Philip
--
[1] in the original it was a "Success Oriented Schedule" - one of those 
plans that hopeful managers put together on late running projects that 
amount to wishful thinking that hopefully garners them enough time to 
make a little progress and update their 'success stories'.
[2] http://dx.doi.org/10.1111%2F1467-8721.01235 "Why People Fail to 
Recognize Their Own Incompetence". Though the corollaries (People fail 
to recognise their own skills, and hence they/we mishandle our 
communications) are just as (IMHO more) important. 

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

[John Keeping]

On Sun, Sep 08, 2013 at 02:54:20AM -0400, Jeff King wrote:
> I am genuinely curious what people in favor of this feature would want
> to say in the documentation to a user encountering this choice for the
> first time. In my experience, rebasing introduces more complications,
> specifically:
> 
>   1. the merge is "backwards" with respect to ours/theirs
> 
>   2. you may end up with difficult conflict resolution due to repeated
>      changes over the same section of code. E.g., you write some buggy
>      code and then fix it, but upstream has changed the same area.
>      Rebasing involves first resolving your buggy version with the
>      upstream code, and then resolving the fix on top of the previous
>      resolution.
> 
>   3. rewriting of commits found in other branches, which then need
>      rebased on top of the branch you just rebased
> 
>   4. a previously bug-free commit can show a bug after the rebase if
>      other parts of the project changed (whereas with a merge, the bug
>      would be attributable to the merge)
> 
> I know those are all balanced by some advantages of rebasing, but I also
> think they are things that can be troublesome for a user who does not
> fully grok the rebase process. I'm just wondering if we should mention
> both, but steer people towards merging as the safer alternative (you
> might have ugly history, but you are less likely to create a mess with
> duplicate commits or badly-resolved conflicts).

The really correct thing to do here is to encourage a feature branch
workflow, but in my experience people are happier to walk through a
rebase than to switch over to feature branches completely.

An alternative pull mode would be:

    git reset --keep @{u} &&
    git merge @{-1}

which gets a sensible history shape without any of your disadvantages
above.  But that didn't go anywhere last time it came up [1] [2].

[1] http://article.gmane.org/gmane.comp.version-control.git/210246
[2] http://article.gmane.org/gmane.comp.version-control.git/210625

> > Fortunately there probably are very few of these users.
> 
> Maybe. I am not sure how one would measure.
> 
> If you are interested, I can ask the opinion of some of the GitHub
> trainers. They see a lot of new users and have a sense of what kinds of
> confusion come up most frequently, what kinds of workflows they tend to
> see, etc. Their experience may be biased towards corporate-ish users,
> though, because those are the people who pay for training.

I expect corporate environments are the ones in which this is relevant.
Open source projects that care about the shape of history can have one
person able to write to the central repository who can enforce the
policy they want.  This tends to be more difficult in a corporate
environment, particularly one that was previously using a centralised
VCS.

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

[Philip Oakley]

From: "Philip Oakley" <philipoakley@iee.org>
> [2] http://dx.doi.org/10.1111%2F1467-8721.01235 "Why People Fail to 
> Recognize Their Own Incompetence".

Oops, That's behind a paywall, and a more recent variant.

>                                          Though the corollaries 
> (People fail to recognise their own skills, and hence they/we 
> mishandle our communications) are just as (IMHO more) important.

I believe this is the on-line version of the original 1999 paper
http://mastercodeprofessional.com/library_files/Kruger-Dunning---Unskilled_and_Unaware_of_It_(2009).pdf

The section 5.1. "The Burden of Expertise" discusses my point above.
"they [Experts] fail to realize that their proficiency is not 
necessarily shared by their peers."

Philip

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

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]

On Sat, Sep 07, 2013 at 11:37:13PM -0500, Felipe Contreras wrote:
> On Sat, Sep 7, 2013 at 11:18 PM, Jeff King <peff@peff.net> wrote:
> > By "svn-like", I mean the people whose workflow is:
> >
> >   $ hack hack hack
> >   $ git commit
> >   $ git push ;# oops, somebody else pushed in the meantime
> >   $ git pull
> >   $ git push

It's possible that some teams at work may be using this workflow.  It's
more likely that there would be a rebase if the push failed, but some
teams might do a merge.  I don't know because we don't dictate workflow
to individual teams for the reasons I get into below.  Regardless,
merges are our typical workflow, so forcing rebase mode all the time
wouldn't be appropriate for us.

> >   $ hack hack hack
> >   $ svn commit ;# oops, somebody else committed in the meantime
> >   $ svn update
> >   $ svn commit
> >
> > Those people would now have to learn enough to choose between merge and
> > rebase when running the "git pull".
> 
> But that's only if they don't care about the shape of history. In my
> experience the people that cling more to centralized VCS do not like
> merges, so they rebase everything to make it a straight line. That is
> much more "svn-like".
> 
> So chances are they are already doing 'git pull --rebase' (or
> similar), so their workflow wouldn't be affected.

We end up squashing each project branch into one commit (usually using
git reset --soft), so we don't care about the shape of history.  Over
the course of a project branch, in fact, there may be many merges from
the main release branches (including other projects), so history is
going to be very messy otherwise.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Junio C Hamano]

Richard Hansen <rhansen@bbn.com> writes:

> On 2013-09-07 22:41, Felipe Contreras wrote:
>> On Wed, Sep 4, 2013 at 5:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> 
>>> Which can be solved by adding the above "fail" option, and then
>>> renaming them to "pull.integrate" and "branch.<name>.integrate" to
>>> clarify what these variables are about (it is no longer "do you
>>> rebase or not---if you choose not to rebase, by definition you are
>>> going to merge", as there is a third choice to "fail"), while
>>> retaining "pull.rebase" and "branch.<name>.rebase" as a deprecated
>>> synonym.
>> 
>> All these names are completely unintuitive. First of all, why
>> "integrate"? Integrate what to what? And then, why "fail"? Fail on
>> what circumstances? Always?
>> 
>> My proposal that does:
>> 
>>   pull.mode = merge/rebase/merge-ff-only
>> 
>> Is way more intuitive.
>
> +1
>
> What about something like:
>
>     pull.mergeoptions (defaults to --ff-only)
>     pull.rebaseoptions (defaults to empty?  --preserve-merges?)
>     branch.<name>.pull.mergeoptions (defaults to pull.mergeoptions)
>     branch.<name>.pull.rebaseoptions (defaults to pull.rebaseoptions)

As "pull" has two distinct phases "fetch" and "merge/rebase", your
mergeoptions/rebaseoptions is much better than "mode", which does
not tell which phase of "pull" the mode refers to. It is clear that
they apply to the process to integrate the history obtained from
the other side and your own history into one history.

But it does not help Philip's case, if I understand correctly, where
running "git pull" on some branches is always a mistake and the user
wants it to stop at "fetch the history and objects needed to
complete the history from the other side" phase without proceeding
to the "then integrate the history from the other side and the
history of your branch into one" step, which may be done with either
merge or rebase.  Even if we ignore that "always fail, do not do
anything" use case, your two seemingly independent "mergeoptions"
and "rebaseoptions" do not tell us which one is preferred between
merge and rebase.  A single

    pull.<someoption> = rebase | merge [| always-fail]

makes that choice in a clear way, I think.

Regarding the verb "integrate".

We used to explain "pull" is a "fetch" followed by a "merge".  With
more people using "git pull --rebase", the word "merge" used in that
explanation of "pull" stopped being generic enough.  Simplarily the
"upstream branch" of local branch X is "what you fetch and merge to
update the branch X" but that 'merge' can be 'rebase'.  We needed a
verb to call the process of integrate the two histories into one.

"git pull --help" since 153d7265 (pull: change the description to
"integrate" changes, 2013-07-07) uses that verb [*1*].

And that is where the name of the single configuration to pick how
to integrate the history obtained by the first phase of "pull" came
from.


[Footnote]

*1* I suspect that there may still be places in the documentation
that have not been updated since the days back when the only valid
way to integrate two lines of histories was to merge, and updating
them may be a low-hanging fruit. Hint, hint.
 

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

[Richard Hansen]

On 2013-09-08 14:10, Junio C Hamano wrote:
> Richard Hansen <rhansen@bbn.com> writes:
>> What about something like:
>>
>>     pull.mergeoptions (defaults to --ff-only)
>>     pull.rebaseoptions (defaults to empty?  --preserve-merges?)
>>     branch.<name>.pull.mergeoptions (defaults to pull.mergeoptions)
>>     branch.<name>.pull.rebaseoptions (defaults to pull.rebaseoptions)
> 
[snip]
> But it does not help Philip's case, if I understand correctly, where
> running "git pull" on some branches is always a mistake and the user
> wants it to stop at "fetch the history and objects needed to
> complete the history from the other side" phase without proceeding
> to the "then integrate the history from the other side and the
> history of your branch into one" step, which may be done with either
> merge or rebase.

How about:

    branch.<name>.pull.defaultIntegrationMode = merge | rebase | none
        If 'merge', pull acts like 'git pull --merge' by default,
        merging the other commits into this branch.
        If 'rebase', pull acts like 'git pull --rebase' by default,
        rebasing this branch onto the other commits.
        If 'none', pull acts like 'git fetch' by default.
        Default: whatever pull.defaultIntegrationMode is set to.

    branch.<name>.pull.mergeoptions
        Arguments to pass to 'git merge' during the merge phase of
        'git pull --merge'.
        Default: whatever pull.mergeoptions is set to.

    branch.<name>.pull.rebaseoptions
        Arguments to pass to 'git rebase' during the rebase phase of
        'git pull --rebase'.
        Default: whatever pull.rebaseoptions is set to.

    pull.defaultIntegrationMode = rebase | merge | none
        See branch.<name>.pull.defaultIntegrationMode.
        Default: merge

    pull.mergeoptions
        See branch.<name>.pull.mergeoptions.
        Default: empty, but warn that a future version will change
        this to --ff-only.

    pull.rebaseoptions
        See branch.<name>.pull.rebaseoptions.
        Default: empty, but warn that a future version will change
        this to --preserve-merges?

There's probably a better alternative to the term 'defaultIntegrationMode'.

We could even add a defaultIntegrationMode = merge-there that
reverses the parent order (the other commits become the first parent,
the current branch becomes the second parent).

-Richard

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

[Felipe Contreras]

On Sun, Sep 8, 2013 at 12:26 PM, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> On Sat, Sep 07, 2013 at 11:37:13PM -0500, Felipe Contreras wrote:
>> On Sat, Sep 7, 2013 at 11:18 PM, Jeff King <peff@peff.net> wrote:

>> >   $ hack hack hack
>> >   $ svn commit ;# oops, somebody else committed in the meantime
>> >   $ svn update
>> >   $ svn commit
>> >
>> > Those people would now have to learn enough to choose between merge and
>> > rebase when running the "git pull".
>>
>> But that's only if they don't care about the shape of history. In my
>> experience the people that cling more to centralized VCS do not like
>> merges, so they rebase everything to make it a straight line. That is
>> much more "svn-like".
>>
>> So chances are they are already doing 'git pull --rebase' (or
>> similar), so their workflow wouldn't be affected.
>
> We end up squashing each project branch into one commit (usually using
> git reset --soft), so we don't care about the shape of history.  Over
> the course of a project branch, in fact, there may be many merges from
> the main release branches (including other projects), so history is
> going to be very messy otherwise.

Yeah, but the key question at hand in this discussion is; what happens
when 'git pull' stops working for them, and they don't know what to
do, will they choose 'git pull --rebase' by mistake?

I say the answer is no, because:

1) As you say in your scenario, somebody is telling these guys what to
do, so when 'git pull' fails, somebody will figure out that they were
doing a merge, so 'git pull --merge' is what they want to type from
now on.

2) Git itself would be warning them for months that a 'non
fast-forward was found, and a merge will be done for them', so when
the warning turns to an error, they'll know they want a merge, so
they'll do 'git pull --merge', either because the warning told them
that's git was doing all along, or because they figured that out by
googling, or reading the man page, or whatever.

Either way, it would not be a big deal for these people, their
user-experience wouldn't be totally broken by this proposed change,
and that is the important conclusion.

-- 
Felipe Contreras

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

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
Sent: Sunday, September 08, 2013 7:10 PM
> Richard Hansen <rhansen@bbn.com> writes:
>
>> On 2013-09-07 22:41, Felipe Contreras wrote:
>>> On Wed, Sep 4, 2013 at 5:59 PM, Junio C Hamano <gitster@pobox.com> 
>>> wrote:
>>>
>>>> Which can be solved by adding the above "fail" option, and then
>>>> renaming them to "pull.integrate" and "branch.<name>.integrate" to
>>>> clarify what these variables are about (it is no longer "do you
>>>> rebase or not---if you choose not to rebase, by definition you are
>>>> going to merge", as there is a third choice to "fail"), while
>>>> retaining "pull.rebase" and "branch.<name>.rebase" as a deprecated
>>>> synonym.
>>>
>>> All these names are completely unintuitive. First of all, why
>>> "integrate"? Integrate what to what? And then, why "fail"? Fail on
>>> what circumstances? Always?
>>>
>>> My proposal that does:
>>>
>>>   pull.mode = merge/rebase/merge-ff-only
>>>
>>> Is way more intuitive.
>>
>> +1
>>
>> What about something like:
>>
>>     pull.mergeoptions (defaults to --ff-only)
>>     pull.rebaseoptions (defaults to empty?  --preserve-merges?)
>>     branch.<name>.pull.mergeoptions (defaults to pull.mergeoptions)
>>     branch.<name>.pull.rebaseoptions (defaults to pull.rebaseoptions)
>
> As "pull" has two distinct phases "fetch" and "merge/rebase", your
> mergeoptions/rebaseoptions is much better than "mode", which does
> not tell which phase of "pull" the mode refers to. It is clear that
> they apply to the process to integrate the history obtained from
> the other side and your own history into one history.
>
> But it does not help Philip's case, if I understand correctly, where
> running "git pull" on some branches is always a mistake

Not quite always, it's when it won't fast forward

>                                     and the user
> wants it to stop at "fetch the history and objects needed to
> complete the history from the other side" phase without proceeding
> to the "then integrate the history from the other side and the
> history of your branch into one" step,

Yes, it/Git should stop and wait for instructions...

>                   which may be done with either
> merge or rebase.

Here I would typically rebase onto an adjusted destination, e.g. onto 
pu, or maybe next, rather than master (or vice versa depending on 
expectations). That is its a feature branch that needs to decide what 
it's on top of (well, I need to decide ;-)

>            Even if we ignore that "always fail, do not do
> anything" use case, your two seemingly independent "mergeoptions"
> and "rebaseoptions" do not tell us which one is preferred between
> merge and rebase.  A single
>
>    pull.<someoption> = rebase | merge [| always-fail]
>
> makes that choice in a clear way, I think.

or 'fail on non-ff' (which may or may not be the users, or Git's 
default, as per the series title ;-)

>
> Regarding the verb "integrate".
>
> We used to explain "pull" is a "fetch" followed by a "merge".  With
> more people using "git pull --rebase", the word "merge" used in that
> explanation of "pull" stopped being generic enough.  Simplarily the
> "upstream branch" of local branch X is "what you fetch and merge to
> update the branch X" but that 'merge' can be 'rebase'.  We needed a
> verb to call the process of integrate the two histories into one.
>
> "git pull --help" since 153d7265 (pull: change the description to
> "integrate" changes, 2013-07-07) uses that verb [*1*].
>
> And that is where the name of the single configuration to pick how
> to integrate the history obtained by the first phase of "pull" came
> from.
>
>
> [Footnote]
>
> *1* I suspect that there may still be places in the documentation
> that have not been updated since the days back when the only valid
> way to integrate two lines of histories was to merge, and updating
> them may be a low-hanging fruit. Hint, hint.
>
>
--
Philip

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

[Felipe Contreras]

On Sun, Sep 8, 2013 at 1:10 PM, Junio C Hamano <gitster@pobox.com> wrote:

>     pull.<someoption> = rebase | merge [| always-fail]
>
> makes that choice in a clear way, I think.
>
> Regarding the verb "integrate".

I doubt anybody thinks of pull being an "integration", and even if it
is, it's still doesn't explain what 'integration = merge' means. To be
human friendly you would need to say 'integration-type' or
'integration-kind', or 'integration-mode', then a human would
understand, "oh yeah, the mode I'm using to integrated is a merge, got
ya'.

But why bother with yet another useless concept the user has to learn?
The user doesn't need to learn about this concept of "integration",
all the user wants is to map:

git pull --rebase
=> pull.<name> = rebase

git pull --merge
pull.<name> = merge

That's it. And my proposed name, 'mode' does the trick just fine.

pull.mode = rebase | merge | merge-no-ff

-- 
Felipe Contreras

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

[Ramkumar Ramachandra]

Felipe Contreras wrote:
> On Sun, Sep 8, 2013 at 1:10 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
>>     pull.<someoption> = rebase | merge [| always-fail]
>>
>> makes that choice in a clear way, I think.

The core issue is that users rarely want to merge locally: that's the
maintainer's job. Users simply want to rebase, and develop on
different branches that they will rebase onto origin. I like Felipe's
idea for using a pull.mode.

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

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2727 bytes --]

On Sun, Sep 08, 2013 at 05:38:50PM -0500, Felipe Contreras wrote:
> Yeah, but the key question at hand in this discussion is; what happens
> when 'git pull' stops working for them, and they don't know what to
> do, will they choose 'git pull --rebase' by mistake?

I agree, they will not choose git pull --rebase by mistake.

> I say the answer is no, because:
> 
> 1) As you say in your scenario, somebody is telling these guys what to
> do, so when 'git pull' fails, somebody will figure out that they were
> doing a merge, so 'git pull --merge' is what they want to type from
> now on.

Yes, that would be me.  My hesitance here is that as the one usually
driving git updates (which so far have happened once a year), I will end
up retraining forty developers.  I don't think the current behavior is
broken or really problematic at all: merging has always been the
default, and people have come to expect that.  People using workflows
that don't want merge have always either needed to set a configuration
option or use --rebase.  As the man page says, --rebase is unsafe, and
that's why it's not the default.

I would be much less unhappy with your earlier change that did not
affect uses with arguments.  That would limit the number of use cases
affected.

> 2) Git itself would be warning them for months that a 'non
> fast-forward was found, and a merge will be done for them', so when
> the warning turns to an error, they'll know they want a merge, so
> they'll do 'git pull --merge', either because the warning told them
> that's git was doing all along, or because they figured that out by
> googling, or reading the man page, or whatever.

Again, you assume that git updates happen on a regular basis, and you
assume that most developers really know what happens under the hood.

I don't see a warning now; in fact, I see:

  vauxhall ok % git status
  # On branch master
  # Your branch and 'upstream/master' have diverged,
  # and have 1 and 128 different commits each, respectively.
  #   (use "git pull" to merge the remote branch into yours)
  #

The current behavior of git is to explicitly encourage this behavior,
and now you want to make it not work.  I think this change is a bad
idea, and I think the number of changes required to the test suite
indicates that.  If there's going to be a change here, it should have a
deprecation period with the above message changed and appropriate
warnings, not a flag day; your patches don't do that.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Felipe Contreras]

On Sun, Sep 8, 2013 at 7:01 PM, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> On Sun, Sep 08, 2013 at 05:38:50PM -0500, Felipe Contreras wrote:
>> Yeah, but the key question at hand in this discussion is; what happens
>> when 'git pull' stops working for them, and they don't know what to
>> do, will they choose 'git pull --rebase' by mistake?
>
> I agree, they will not choose git pull --rebase by mistake.
>
>> I say the answer is no, because:
>>
>> 1) As you say in your scenario, somebody is telling these guys what to
>> do, so when 'git pull' fails, somebody will figure out that they were
>> doing a merge, so 'git pull --merge' is what they want to type from
>> now on.
>
> Yes, that would be me.  My hesitance here is that as the one usually
> driving git updates (which so far have happened once a year), I will end
> up retraining forty developers.  I don't think the current behavior is
> broken or really problematic at all: merging has always been the
> default, and people have come to expect that.

It may not be broken for you, but it is for other people. Would you be
so egocentric as to ignore everybody else because "it works for you"?

> People using workflows
> that don't want merge have always either needed to set a configuration
> option or use --rebase.  As the man page says, --rebase is unsafe, and
> that's why it's not the default.

Yes, but the problem is that people using other workflows end up
avoiding 'git pull' at all, so at the end of the day we have one core
command that the majority of users avoid, that's not good.

> I would be much less unhappy with your earlier change that did not
> affect uses with arguments.  That would limit the number of use cases
> affected.

I have no problem with:
git pull $remote $branch

Allowing non-fast-forward merges.

And:
git pull $remote
git pull

Not allowing them by default.

But the problem is that it's not easy to implement.

Either way, I'll venture that you don't want 'git pull $remote' to
change, so it would be a waste of the time to try to get the above to
work.

>> 2) Git itself would be warning them for months that a 'non
>> fast-forward was found, and a merge will be done for them', so when
>> the warning turns to an error, they'll know they want a merge, so
>> they'll do 'git pull --merge', either because the warning told them
>> that's git was doing all along, or because they figured that out by
>> googling, or reading the man page, or whatever.
>
> Again, you assume that git updates happen on a regular basis, and you
> assume that most developers really know what happens under the hood.

No. The developers don't have to know what happens under the hood, Git
would be telling them "WARNING: we are doing a merge", what else is
the developer to think, but that 'git pull' is doing a merge?

As for the updates, yes, I assume updates happen at least each three
months. If your company updates each year, I don't see what much more
we can do to you help you. Doing a single change per year is certainly
going to hold the project back.

Fortunately this was only point 2), there's still point 1); you can
tell them to use 'git pull --merge' from now on, and since you update
once every year, you can do it while you give the training for the
year.

Or there's another option:

3) Distribute Git in your company with /etc/gitconfig having pull.mode
= merge. This way nothing will change.

I think we are being very accommodating to your company's use-case
which is very far from the norm. Even in the absolute worst case
scenario, you would have to tell people to use 'git pull --merge'
instead, is that really so horrible? Should we really halt Git's
progress because you would have to tell people to type nine extra
characters or run one configuration command?

> I don't see a warning now; in fact, I see:
>
>   vauxhall ok % git status
>   # On branch master
>   # Your branch and 'upstream/master' have diverged,
>   # and have 1 and 128 different commits each, respectively.
>   #   (use "git pull" to merge the remote branch into yours)
>   #
>
> The current behavior of git is to explicitly encourage this behavior,
> and now you want to make it not work.

Yes, that's why it's a change.

> I think this change is a bad
> idea, and I think the number of changes required to the test suite
> indicates that.  If there's going to be a change here, it should have a
> deprecation period with the above message changed and appropriate
> warnings, not a flag day; your patches don't do that.

My patches pretty much do nothing else but introduce a warning.
Nothing is broken, nothing is changed in the test suite:

http://article.gmane.org/gmane.comp.version-control.git/233669

You are confusing my proposal with Junio's one.

Also, my proposal was to enable this behavior (pull.mode =
merge-ff-only) only for Git v2.0, which might happen probably way
later than a year from now, so you your users might actually see the
warning after all. But yeah, that's _my_ proposal.

-- 
Felipe Contreras

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

[Felipe Contreras]

On Sun, Sep 8, 2013 at 7:29 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:

> My patches pretty much do nothing else but introduce a warning.
> Nothing is broken, nothing is changed in the test suite:
>
> http://article.gmane.org/gmane.comp.version-control.git/233669
>
> You are confusing my proposal with Junio's one.

Actually my mistake. My patches don't even add a warning, so nothing
is changed at all (unless you manually configure pull.mode =
merge-ff-only).

I only suggested to add the warning, but didn't actually implement it.
I'll do that soon.

-- 
Felipe Contreras

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

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]

On Sun, Sep 08, 2013 at 07:36:21PM -0500, Felipe Contreras wrote:
> On Sun, Sep 8, 2013 at 7:29 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> 
> > My patches pretty much do nothing else but introduce a warning.
> > Nothing is broken, nothing is changed in the test suite:
> >
> > http://article.gmane.org/gmane.comp.version-control.git/233669
> >
> > You are confusing my proposal with Junio's one.
> 
> Actually my mistake. My patches don't even add a warning, so nothing
> is changed at all (unless you manually configure pull.mode =
> merge-ff-only).
> 
> I only suggested to add the warning, but didn't actually implement it.
> I'll do that soon.

I still wouldn't be crazy about the change, but if there's a warning, I
could live with it.  I think that's probably the best course of action
if there's going to be a change here.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Matthieu Moy]

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

> On Sun, Sep 8, 2013 at 7:01 PM, brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
>
>> Yes, that would be me.  My hesitance here is that as the one usually
>> driving git updates (which so far have happened once a year), I will end
>> up retraining forty developers.  I don't think the current behavior is
>> broken or really problematic at all: merging has always been the
>> default, and people have come to expect that.
>
> It may not be broken for you, but it is for other people. Would you be
> so egocentric as to ignore everybody else because "it works for you"?

It's not a matter of "works for me". Git currently "works" for all use
cases because you can already merge or rebase. The proposed changes are
not about allowing the behavior that works, but disallowing the behavior
that doesn't.

I agree that allowing people to reject non-ff merge is a good idea.

I strongly disagree that this should eventually become the default,
though. I think it should really remain an opt-in (possibly with some
non-scary warning advertizing for the feature).

First, the discussions on this thread show that it's hard to find the
right behavior. My guess is that it's hard because we're trying to think
for the users. I've used GNU Arch for a while, and this VCS was trying
to impose what the developer thought was good for me. I had to fight
with the tool whenever I tried to do something "non-standard". I don't
want to go back there. Preventing _users_ to do something because _we_
considered it was bad for them is wrong IMHO.

I already mentionned another reason in
http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=229162 :
"git rebase" is hard to use for many people. With "git merge", doing
things wrong isn't so bad. If you forget to commit after a conflicted
merge, you'll mix your changes with the merge, this is bad, but it
works. With "git rebase", if you forget to "git rebase --continue" after
a conflict, you end up in detached HEAD, with part of your own changes
discarded. If my students end up in this situation, they'll stop using
Git and exchange files by email.

"git pull" is one of the first things one learns with Git, and
_requiring_ users to chose between merge and rebase is a nonsense at
this time of learning.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

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

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> First, the discussions on this thread show that it's hard to find the
> right behavior. My guess is that it's hard because we're trying to think
> for the users. I've used GNU Arch for a while, and this VCS was trying
> to impose what the developer thought was good for me. I had to fight
> with the tool whenever I tried to do something "non-standard". I don't
> want to go back there.

Fond memories of tla comes back to me as well ... ;-)

> Preventing _users_ to do something because _we_ considered it was
> bad for them is wrong IMHO.
> I already mentionned another reason in
> http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=229162 :
> "git rebase" is hard to use for many people.
> ...
> "git pull" is one of the first things one learns with Git, and
> _requiring_ users to chose between merge and rebase is a nonsense at
> this time of learning.

After I re-read that message, I am starting to think that the topic
that has been cooking in 'next' that attempts to catch "git pull"
(no "from where, integrate with what" parameters) may already be bad
by that standard. Brian Carlson's comments on the impact on existing
users seems to the same direction to me.

You are in favor of an _option_ to allow people to forbid a pull in
a non-ff situation, and I think other people are also in
agreement. So perhaps:

 - drop jc/pull-training-wheel and revert its merge from 'next';

 - update Felipe's series with a bit of tweak to make it less
   impactful by demoting error into warning and advice.

would be a good way forward?

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

[Jeff King]

On Mon, Sep 09, 2013 at 11:47:45AM -0700, Junio C Hamano wrote:

> You are in favor of an _option_ to allow people to forbid a pull in
> a non-ff situation, and I think other people are also in
> agreement. So perhaps:
> 
>  - drop jc/pull-training-wheel and revert its merge from 'next';
> 
>  - update Felipe's series with a bit of tweak to make it less
>    impactful by demoting error into warning and advice.
> 
> would be a good way forward?

I think that would address the concern I raised, because it does not
create a roadblock to new users accomplishing their task. They can
ignore the warning, or choose "merge" as the default to shut up the
warning (and it is easy to choose that if you are confused, because it
is what git is doing by default alongside the warning).

-Peff

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

[Jeff King]

On Sun, Sep 08, 2013 at 11:03:52AM +0100, John Keeping wrote:

> > I know those are all balanced by some advantages of rebasing, but I also
> > think they are things that can be troublesome for a user who does not
> > fully grok the rebase process. I'm just wondering if we should mention
> > both, but steer people towards merging as the safer alternative (you
> > might have ugly history, but you are less likely to create a mess with
> > duplicate commits or badly-resolved conflicts).
> 
> The really correct thing to do here is to encourage a feature branch
> workflow, but in my experience people are happier to walk through a
> rebase than to switch over to feature branches completely.
> 
> An alternative pull mode would be:
> 
>     git reset --keep @{u} &&
>     git merge @{-1}
> 
> which gets a sensible history shape without any of your disadvantages
> above.  But that didn't go anywhere last time it came up [1] [2].

FWIW, that approach makes some sense to me. De-coupling for a moment the
idea of "what is the default" from "what are the options", it seems like
doing a reverse-merge would be a good option to have in the toolbox.

It would also have other uses beyond "git pull". For example, in
development of GitHub itself, we use topic branches. But before merging
them to master, we often test-deploy the topic to the live site. Before
doing so, you have to merge the topic with the latest master to make
sure you are not un-deploying anybody else's recently graduated topics.

You can do so by creating a temporary merge branch and deploying that,
or you can simply merge master back into the topic. We generally choose
the latter, because it leaves any conflict resolution in an obvious
place (and doesn't need repeating).

-Peff

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

[Jeff King]

On Sun, Sep 08, 2013 at 03:50:46AM -0400, Jeff King wrote:

> > > If you are interested, I can ask the opinion of some of the GitHub
> > > trainers. They see a lot of new users and have a sense of what kinds of
> > > confusion come up most frequently, what kinds of workflows they tend to
> > > see, etc. Their experience may be biased towards corporate-ish users,
> > > though, because those are the people who pay for training.
> > 
> > Ask. I'm sure they will tell you doing merges by mistake with 'git
> > pull' is an issue.
> 
> I've sent an email. I'll post the response when I get it.

Here is what I sent them (I am leaving both my mail and theirs unedited
to avoid any "telephone"-like confusion in trying to summarize):

        Right now, running "git pull" will always create a merge, unless
        the user has specifically configured it to perform a rebase.
        Some people find this problematic, because the project may care
        about the order of merges (e.g., so that --first-parent
        traversals do the right thing), and some users may accidentally
        do "backwards" merges from a main branch into a topic (either
        because they are clueless, or because they simply forgot).

        There is a proposal being considered to have "git pull" do
        nothing by default, but instead ask the user to specify whether
        to merge or rebase (with the option of setting a config value if
        you want it to do one by default).

        One concern I have is that new users may run across this
        relatively early. For example, the first time they "git push"
        and get a non-fast-forward because somebody else has already
        pushed, git suggests to run "git pull". At which point they will
        have to decide whether to merge or rebase. So what I'd like your
        opinions on is:

          1. Do new users have trouble with the concept of rebase vs
             merge?  How would they handle this change of behavior?

          2. Do new users have trouble with rebases in general? There
             are some complications over doing a normal merge, but I
             don't know how often they trip people up in practice.

And the responses I got were:

        1. New users definitely have trouble distinguishing between
        rebase and merge. Even people who have been using Git for a
        while on a basic level are sometimes confused by this.

        2. Most people we teach—even the ones who have been using Git
        for a while—don't know what a rebase is at all. They've heard of
        it, but they don't get it. It takes careful explanation to get
        the concept across and explain why it is not the same thing as a
        merge.

        Speaking for myself, about half of the time in the Foundations
        class I'll explain `pull --rebase` and `branch.autosetuprebase`.
        (Whether we get to it depends on class interest and ability.)
        When we do address that topic, we always recommend that
        rebase-on-pull is the right thing to do, since the merges Git
        creates are just noise that makes history hard to work with in
        the ways you have pointed out. (For smart classes, I like to
        make the analogy of Git to a distributed database, and point out
        how the merge on pull is just Git's mechanism for resolving
        split-brain writes. I explain that those merges aren't a
        deficiency in Git; they're just what has to happen by default.
        The fact that Git handles split-brain writes so well by itself
        is amazing.)

        My input would be to continue to have `pull` merge by default.
        Those merges aren't great, but new users won't have any idea how
        to make a decision about them at that point. As it is, it just
        works, and it works quite elegantly. Once you start to learn
        some things, you can tune Git up to work even more elegantly by
        rebasing, but having to understand that concept and make a
        decision on your first (or second or third or twentieth) pull is
        probably asking too much.

and:

        Just a few more elements to add:

        * I have been teaching rebase and what it means in _some_ of my
        Git Foundations classes as of late.  But "some" means there are
        a majority that do not get it.

        * These are the people that get "formal" training on Git.  What
        about all the newbies?  They really won't have a foundation for
        what these two "flavors" mean.

        * The merge is very different from what Subversion presents as a
        default.  That's a possible point in the "option's favor."

        * In the end though, the "simplest thing that works" should be
        the default without a choice.  To me, a choice implies knowledge
        of the benefits of each option.  I would say that the majority
        of our Git students do not, at the beginning of Git usage,
        understand the difference.

I did not specifically ask in the original about whether backwards
merges were a problem, though I think that is touched on in the
responses.

If you'd like me to ask something specifically, I can relay the
question, or I can ask them to come join the discussion here.

-Peff

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

[John Keeping]

On Mon, Sep 09, 2013 at 03:52:31PM -0400, Jeff King wrote:
> On Mon, Sep 09, 2013 at 11:47:45AM -0700, Junio C Hamano wrote:
> 
> > You are in favor of an _option_ to allow people to forbid a pull in
> > a non-ff situation, and I think other people are also in
> > agreement. So perhaps:
> > 
> >  - drop jc/pull-training-wheel and revert its merge from 'next';
> > 
> >  - update Felipe's series with a bit of tweak to make it less
> >    impactful by demoting error into warning and advice.
> > 
> > would be a good way forward?
> 
> I think that would address the concern I raised, because it does not
> create a roadblock to new users accomplishing their task. They can
> ignore the warning, or choose "merge" as the default to shut up the
> warning (and it is easy to choose that if you are confused, because it
> is what git is doing by default alongside the warning).

I think we need to make sure that we give instructions for how to go
back if the default hasn't done what you wanted.  Something like this:

    Your pull did not fast-forward, so Git has merged '$upstream' into
    your branch, which may not be correct for your project.  If you
    would rather rebase your changes, run

        git rebase

    See "pull.mode" in git-config(1) to suppress this message in the
    future.

?

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

[Jeff King]

On Mon, Sep 09, 2013 at 09:24:35PM +0100, John Keeping wrote:

> > I think that would address the concern I raised, because it does not
> > create a roadblock to new users accomplishing their task. They can
> > ignore the warning, or choose "merge" as the default to shut up the
> > warning (and it is easy to choose that if you are confused, because it
> > is what git is doing by default alongside the warning).
> 
> I think we need to make sure that we give instructions for how to go
> back if the default hasn't done what you wanted.  Something like this:
> 
>     Your pull did not fast-forward, so Git has merged '$upstream' into
>     your branch, which may not be correct for your project.  If you
>     would rather rebase your changes, run
> 
>         git rebase
> 
>     See "pull.mode" in git-config(1) to suppress this message in the
>     future.

Yes, that's a good point. I don't know if just "git rebase" is the right
advice, though; it would depend on whether we were actually pulling from
the upstream or not.

I wonder if we have sufficient information at the time of the warning to
print out the actual "git rebase" invocation that would rebase as if
they had run "pull --rebase". I think we may have to do a little
refactoring around the base selection from the reflog (IIRC, git-pull
does not even calculate it at all if you are not using --rebase).

It is also depending on "git rebase" throwing away the merge commit we
just created. Which I think should happen always if you have not
configured anything (though perhaps we will eventually support a pull
mode that does "rebase -p", you would not see this warning with that
option anyway). But another option would be to simply tell them:

  git reset --keep HEAD^
  git pull --rebase [X...]

where "[X...]" is the arguments they gave to rebase in the first place.
That looks a little less friendly, though.

-Peff

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

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> You are in favor of an _option_ to allow people to forbid a pull in
> a non-ff situation, and I think other people are also in
> agreement.

Yes. Having an option can't harm anybody, and there's a clear demand for
that.

> So perhaps:
>
>  - drop jc/pull-training-wheel and revert its merge from 'next';
>
>  - update Felipe's series with a bit of tweak to make it less
>    impactful by demoting error into warning and advice.
>
> would be a good way forward?

I didn't follow very closely the discussions and patch series, but that
would sound right to me. The last version of Felipes' patch series
already gives a warning only, but the wording and commit message implies
that this will become an error in the future (this is the part with
which I disagree).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

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

[Matthieu Moy]

John Keeping <john@keeping.me.uk> writes:

> I think we need to make sure that we give instructions for how to go
> back if the default hasn't done what you wanted.  Something like this:
>
>     Your pull did not fast-forward, so Git has merged '$upstream' into
>     your branch, which may not be correct for your project.  If you
>     would rather rebase your changes, run
>
>         git rebase
>
>     See "pull.mode" in git-config(1) to suppress this message in the
>     future.

Sounds good to me. One option is to display the warning on the
command-line, and another option is to show it in COMMIT_EDITMSG (since
we now default to showing it even for non-conflicted merges).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

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

[Jeff King]

On Mon, Sep 09, 2013 at 10:50:31PM +0200, Matthieu Moy wrote:

> John Keeping <john@keeping.me.uk> writes:
> 
> > I think we need to make sure that we give instructions for how to go
> > back if the default hasn't done what you wanted.  Something like this:
> >
> >     Your pull did not fast-forward, so Git has merged '$upstream' into
> >     your branch, which may not be correct for your project.  If you
> >     would rather rebase your changes, run
> >
> >         git rebase
> >
> >     See "pull.mode" in git-config(1) to suppress this message in the
> >     future.
> 
> Sounds good to me. One option is to display the warning on the
> command-line, and another option is to show it in COMMIT_EDITMSG (since
> we now default to showing it even for non-conflicted merges).

I hadn't though of that, but showing it in COMMIT_EDITMSG is a great
moment, because you are notifying the user _before_ they create a merge
commit. So the backout/switch procedure is "cancel this by giving an
empty message, then re-run git pull --rebase".

On the other hand, if we run into conflicts, you'd probably want to let
them know before asking them to resolve them all. So perhaps a separate
message would be needed for that case (to suggest "reset --merge && git
pull --rebase").

-Peff

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

[John Keeping]

On Mon, Sep 09, 2013 at 04:44:16PM -0400, Jeff King wrote:
> On Mon, Sep 09, 2013 at 09:24:35PM +0100, John Keeping wrote:
> 
> > > I think that would address the concern I raised, because it does not
> > > create a roadblock to new users accomplishing their task. They can
> > > ignore the warning, or choose "merge" as the default to shut up the
> > > warning (and it is easy to choose that if you are confused, because it
> > > is what git is doing by default alongside the warning).
> > 
> > I think we need to make sure that we give instructions for how to go
> > back if the default hasn't done what you wanted.  Something like this:
> > 
> >     Your pull did not fast-forward, so Git has merged '$upstream' into
> >     your branch, which may not be correct for your project.  If you
> >     would rather rebase your changes, run
> > 
> >         git rebase
> > 
> >     See "pull.mode" in git-config(1) to suppress this message in the
> >     future.
> 
> Yes, that's a good point. I don't know if just "git rebase" is the right
> advice, though; it would depend on whether we were actually pulling from
> the upstream or not.
> 
> I wonder if we have sufficient information at the time of the warning to
> print out the actual "git rebase" invocation that would rebase as if
> they had run "pull --rebase". I think we may have to do a little
> refactoring around the base selection from the reflog (IIRC, git-pull
> does not even calculate it at all if you are not using --rebase).

We can probably do something like:

    opts=
    if git merge-base --is-ancestor "$orig_head" "$merge_head"
    then
        opts=$merge_head
    else
        opts="$orig_head --onto $merge_head"
    fi

so that "git rebase $opts" is the right thing.  Most users then get the
simple "git rebase $merge_head" variant.

> It is also depending on "git rebase" throwing away the merge commit we
> just created. Which I think should happen always if you have not
> configured anything (though perhaps we will eventually support a pull
> mode that does "rebase -p", you would not see this warning with that
> option anyway). But another option would be to simply tell them:
> 
>   git reset --keep HEAD^
>   git pull --rebase [X...]
> 
> where "[X...]" is the arguments they gave to rebase in the first place.
> That looks a little less friendly, though.

Yeah, I think we should keep it simple if possible.  In my experience
people are relatively happy to run a single "make things right" command
but less so if there's a sequence of steps to be performed.

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

[Philip Oakley]

From: "Jeff King" <peff@peff.net>
Sent: Monday, September 09, 2013 9:53 PM
> On Mon, Sep 09, 2013 at 10:50:31PM +0200, Matthieu Moy wrote:
>
>> John Keeping <john@keeping.me.uk> writes:
>>
>> > I think we need to make sure that we give instructions for how to 
>> > go
>> > back if the default hasn't done what you wanted.  Something like 
>> > this:
>> >
>> >     Your pull did not fast-forward, so Git has merged '$upstream' 
>> > into
>> >     your branch, which may not be correct for your project.  If you
>> >     would rather rebase your changes, run
>> >
>> >         git rebase
>> >
>> >     See "pull.mode" in git-config(1) to suppress this message in 
>> > the
>> >     future.
>>
>> Sounds good to me. One option is to display the warning on the
>> command-line, and another option is to show it in COMMIT_EDITMSG 
>> (since
>> we now default to showing it even for non-conflicted merges).
>
> I hadn't though of that, but showing it in COMMIT_EDITMSG is a great
> moment, because you are notifying the user _before_ they create a 
> merge
> commit. So the backout/switch procedure is "cancel this by giving an
> empty message, then re-run git pull --rebase".
>
> On the other hand, if we run into conflicts, you'd probably want to 
> let
> them know before asking them to resolve them all. So perhaps a 
> separate
> message would be needed for that case (to suggest "reset --merge && 
> git
> pull --rebase").

In fact this [running into conflicts unexpectedly] is usually my use 
case, which I mis-described as a no-ff in an earlier reply.

Usually I'd want a clean rebase before submitting patches, but I can see 
other uses cases where there is a desire that branches show where they 
started so rebase wouldn't be appropriate.

It should not be necessary to give prescriptions about how to backout of 
a difficult corner, rather give details about how to go forward after 
stopping safely and early. The urge to press on (various proposals)  may 
not be the right thing.

Philip 

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

[Richard Hansen]

On 2013-09-09 16:44, Jeff King wrote:
> On Mon, Sep 09, 2013 at 09:24:35PM +0100, John Keeping wrote:
>> I think we need to make sure that we give instructions for how to go
>> back if the default hasn't done what you wanted.  Something like this:
>>
>>     Your pull did not fast-forward, so Git has merged '$upstream' into
>>     your branch, which may not be correct for your project.  If you
>>     would rather rebase your changes, run
>>
>>         git rebase
>>
>>     See "pull.mode" in git-config(1) to suppress this message in the
>>     future.
> 
> Yes, that's a good point. I don't know if just "git rebase" is the right
> advice, though; it would depend on whether we were actually pulling from
> the upstream or not.

Another reason 'git rebase' might not be the right advice:  We don't
want to encourage users to flatten intentional merges.  For example:

    $ git checkout master
    $ git merge --no-ff just-finished-feature-branch
    $ git push
     ! [rejected]        master -> master (non-fast-forward)
    $ git pull
    WARNING: Your pull did not fast-forward [...] run git rebase

If 'git rebase' is run here, the commits on just-finished-feature-branch
will be linearized onto @{u}, which is not what the user wants.

Perhaps one could argue that a user that gets into this situation and is
normally comfortable running 'git rebase' is already experienced enough
to know to ignore the advice to run 'git rebase'.

(Sidenote:  Unfortunately there's not an easy way to recover from this
case without adding another merge commit.  But that's a topic for
another thread.)

-Richard

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

[Felipe Contreras]

On Mon, Sep 9, 2013 at 3:17 PM, Jeff King <peff@peff.net> wrote:
> On Sun, Sep 08, 2013 at 03:50:46AM -0400, Jeff King wrote:
>
>> > > If you are interested, I can ask the opinion of some of the GitHub
>> > > trainers. They see a lot of new users and have a sense of what kinds of
>> > > confusion come up most frequently, what kinds of workflows they tend to
>> > > see, etc. Their experience may be biased towards corporate-ish users,
>> > > though, because those are the people who pay for training.
>> >
>> > Ask. I'm sure they will tell you doing merges by mistake with 'git
>> > pull' is an issue.
>>
>> I've sent an email. I'll post the response when I get it.
>
> Here is what I sent them (I am leaving both my mail and theirs unedited
> to avoid any "telephone"-like confusion in trying to summarize):
>
>         Right now, running "git pull" will always create a merge, unless
>         the user has specifically configured it to perform a rebase.
>         Some people find this problematic, because the project may care
>         about the order of merges (e.g., so that --first-parent
>         traversals do the right thing), and some users may accidentally
>         do "backwards" merges from a main branch into a topic (either
>         because they are clueless, or because they simply forgot).
>
>         There is a proposal being considered to have "git pull" do
>         nothing by default, but instead ask the user to specify whether
>         to merge or rebase (with the option of setting a config value if
>         you want it to do one by default).
>
>         One concern I have is that new users may run across this
>         relatively early. For example, the first time they "git push"
>         and get a non-fast-forward because somebody else has already
>         pushed, git suggests to run "git pull". At which point they will
>         have to decide whether to merge or rebase. So what I'd like your
>         opinions on is:
>
>           1. Do new users have trouble with the concept of rebase vs
>              merge?  How would they handle this change of behavior?
>
>           2. Do new users have trouble with rebases in general? There
>              are some complications over doing a normal merge, but I
>              don't know how often they trip people up in practice.
>
> And the responses I got were:
>
>         1. New users definitely have trouble distinguishing between
>         rebase and merge. Even people who have been using Git for a
>         while on a basic level are sometimes confused by this.
>
>         2. Most people we teach—even the ones who have been using Git
>         for a while—don't know what a rebase is at all. They've heard of
>         it, but they don't get it. It takes careful explanation to get
>         the concept across and explain why it is not the same thing as a
>         merge.
>
>         Speaking for myself, about half of the time in the Foundations
>         class I'll explain `pull --rebase` and `branch.autosetuprebase`.
>         (Whether we get to it depends on class interest and ability.)
>         When we do address that topic, we always recommend that
>         rebase-on-pull is the right thing to do, since the merges Git
>         creates are just noise that makes history hard to work with in
>         the ways you have pointed out. (For smart classes, I like to
>         make the analogy of Git to a distributed database, and point out
>         how the merge on pull is just Git's mechanism for resolving
>         split-brain writes. I explain that those merges aren't a
>         deficiency in Git; they're just what has to happen by default.
>         The fact that Git handles split-brain writes so well by itself
>         is amazing.)
>
>         My input would be to continue to have `pull` merge by default.
>         Those merges aren't great, but new users won't have any idea how
>         to make a decision about them at that point. As it is, it just
>         works, and it works quite elegantly. Once you start to learn
>         some things, you can tune Git up to work even more elegantly by
>         rebasing, but having to understand that concept and make a
>         decision on your first (or second or third or twentieth) pull is
>         probably asking too much.
>
> and:
>
>         Just a few more elements to add:
>
>         * I have been teaching rebase and what it means in _some_ of my
>         Git Foundations classes as of late.  But "some" means there are
>         a majority that do not get it.
>
>         * These are the people that get "formal" training on Git.  What
>         about all the newbies?  They really won't have a foundation for
>         what these two "flavors" mean.
>
>         * The merge is very different from what Subversion presents as a
>         default.  That's a possible point in the "option's favor."
>
>         * In the end though, the "simplest thing that works" should be
>         the default without a choice.  To me, a choice implies knowledge
>         of the benefits of each option.  I would say that the majority
>         of our Git students do not, at the beginning of Git usage,
>         understand the difference.

Wall these concerns can be tackled with an error message that says:

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

-- 
Felipe Contreras

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

[Felipe Contreras]

On Mon, Sep 9, 2013 at 3:24 PM, John Keeping <john@keeping.me.uk> wrote:
> On Mon, Sep 09, 2013 at 03:52:31PM -0400, Jeff King wrote:
>> On Mon, Sep 09, 2013 at 11:47:45AM -0700, Junio C Hamano wrote:
>>
>> > You are in favor of an _option_ to allow people to forbid a pull in
>> > a non-ff situation, and I think other people are also in
>> > agreement. So perhaps:
>> >
>> >  - drop jc/pull-training-wheel and revert its merge from 'next';
>> >
>> >  - update Felipe's series with a bit of tweak to make it less
>> >    impactful by demoting error into warning and advice.
>> >
>> > would be a good way forward?
>>
>> I think that would address the concern I raised, because it does not
>> create a roadblock to new users accomplishing their task. They can
>> ignore the warning, or choose "merge" as the default to shut up the
>> warning (and it is easy to choose that if you are confused, because it
>> is what git is doing by default alongside the warning).
>
> I think we need to make sure that we give instructions for how to go
> back if the default hasn't done what you wanted.  Something like this:
>
>     Your pull did not fast-forward, so Git has merged '$upstream' into
>     your branch, which may not be correct for your project.  If you
>     would rather rebase your changes, run
>
>         git rebase
>
>     See "pull.mode" in git-config(1) to suppress this message in the
>     future.

And you propose to show that every single time the user does a 'git
pull'' that results in a non-fast-forward merge? Isn't that what 'git
pull --help' is for?

-- 
Felipe Contreras

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

[Felipe Contreras]

On Mon, Sep 9, 2013 at 2:18 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Sun, Sep 8, 2013 at 7:01 PM, brian m. carlson
>> <sandals@crustytoothpaste.net> wrote:
>>
>>> Yes, that would be me.  My hesitance here is that as the one usually
>>> driving git updates (which so far have happened once a year), I will end
>>> up retraining forty developers.  I don't think the current behavior is
>>> broken or really problematic at all: merging has always been the
>>> default, and people have come to expect that.
>>
>> It may not be broken for you, but it is for other people. Would you be
>> so egocentric as to ignore everybody else because "it works for you"?
>
> It's not a matter of "works for me". Git currently "works" for all use
> cases because you can already merge or rebase. The proposed changes are
> not about allowing the behavior that works, but disallowing the behavior
> that doesn't.

If it works for all use cases why are we discussing this?

Hint: because it doesn't.

> I agree that allowing people to reject non-ff merge is a good idea.
>
> I strongly disagree that this should eventually become the default,
> though. I think it should really remain an opt-in (possibly with some
> non-scary warning advertizing for the feature).

That defeats the whole purpose of the proposal, which means that you
don't understand the problem.

The problem is the newcomers, and the newcomers will most definitely
not activate a configuration option to tell them that they are doing
something potentially undesirable.

By the time they learn about pull.mode, they probably already know
what a rebase is. So what is the point of the configuration in the
first place?

> First, the discussions on this thread show that it's hard to find the
> right behavior. My guess is that it's hard because we're trying to think
> for the users. I've used GNU Arch for a while, and this VCS was trying
> to impose what the developer thought was good for me. I had to fight
> with the tool whenever I tried to do something "non-standard". I don't
> want to go back there. Preventing _users_ to do something because _we_
> considered it was bad for them is wrong IMHO.

We are not preventing anybody from anything. The user can do 'git pull
--merge', the user can set 'pull.mode = merge', the user can do
anything he wants.

> I already mentionned another reason in
> http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=229162 :
> "git rebase" is hard to use for many people. With "git merge", doing
> things wrong isn't so bad. If you forget to commit after a conflicted
> merge, you'll mix your changes with the merge, this is bad, but it
> works. With "git rebase", if you forget to "git rebase --continue" after
> a conflict, you end up in detached HEAD, with part of your own changes
> discarded. If my students end up in this situation, they'll stop using
> Git and exchange files by email.

That doesn't mean anything, you are assuming the user will do 'git
pull --rebase', and there's no rationale as to why they would end up
doing that.

> "git pull" is one of the first things one learns with Git, and
> _requiring_ users to chose between merge and rebase is a nonsense at
> this time of learning.

Let's use another core command as an illustration.

'git commit' by default "prevents" users from creating commits without
first adding changes to the staging area, and since it's a concept
unique to Git, it's fair to say that none of the newcomers understand
why 'git commit' is failing, the error messages is not particularly
useful either.

Following your rationale, by default 'git commit' should behave like
'git commit --all', and add all the changes in the work tree to the
new commit when there's no changes in the staging area, that would be
the easiest for the newcomers, but we don't do that, we "force" them
to understand what the staging area is, or do 'git commit --all', most
of the newcomers do the later.

So, if we draw a parallel with with pull, 'git pull --merge' is like
'git commit --all'; if they don't know what they are doing, that's
what they should type, and when the pull is non-fast-forward we error
out, just like we error out when there's nothing on the staging area.

-- 
Felipe Contreras

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

[John Keeping]

On Mon, Sep 09, 2013 at 06:02:35PM -0500, Felipe Contreras wrote:
> On Mon, Sep 9, 2013 at 3:24 PM, John Keeping <john@keeping.me.uk> wrote:
> > On Mon, Sep 09, 2013 at 03:52:31PM -0400, Jeff King wrote:
> >> On Mon, Sep 09, 2013 at 11:47:45AM -0700, Junio C Hamano wrote:
> >>
> >> > You are in favor of an _option_ to allow people to forbid a pull in
> >> > a non-ff situation, and I think other people are also in
> >> > agreement. So perhaps:
> >> >
> >> >  - drop jc/pull-training-wheel and revert its merge from 'next';
> >> >
> >> >  - update Felipe's series with a bit of tweak to make it less
> >> >    impactful by demoting error into warning and advice.
> >> >
> >> > would be a good way forward?
> >>
> >> I think that would address the concern I raised, because it does not
> >> create a roadblock to new users accomplishing their task. They can
> >> ignore the warning, or choose "merge" as the default to shut up the
> >> warning (and it is easy to choose that if you are confused, because it
> >> is what git is doing by default alongside the warning).
> >
> > I think we need to make sure that we give instructions for how to go
> > back if the default hasn't done what you wanted.  Something like this:
> >
> >     Your pull did not fast-forward, so Git has merged '$upstream' into
> >     your branch, which may not be correct for your project.  If you
> >     would rather rebase your changes, run
> >
> >         git rebase
> >
> >     See "pull.mode" in git-config(1) to suppress this message in the
> >     future.
> 
> And you propose to show that every single time the user does a 'git
> pull'' that results in a non-fast-forward merge? Isn't that what 'git
> pull --help' is for?

Only if the user has not given an explicit mode (either on the command
line or in their config) and possibly if an advice.pullNonFF variable is
not set to false.  I think that matches what Git does elsewhere.

git-pull(1) provides quite a lot more information that I think a new Git
user would be comfortable with.  There certainly is not a quick way to
find out how to fix this error and I don't think it makes sense to add
one because we'll still be presenting the user with all of the other
content and they won't have any way to know what they can safely ignore
and what they have to read and understand.

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

[Matthieu Moy]

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

> The problem is the newcomers, and the newcomers will most definitely
> not activate a configuration option to tell them that they are doing
> something potentially undesirable.

I teach Git to 200 newcommers each year. All of them run "git pull" the
first day, but believe me, very few of them want to know what a rebase
is at that time.

(I also work with experienced computer scientists, and actually, very
few of them want to know what a rebase is either :-( )

> By the time they learn about pull.mode, they probably already know
> what a rebase is. So what is the point of the configuration in the
> first place?
[...]
> That doesn't mean anything, you are assuming the user will do 'git
> pull --rebase', and there's no rationale as to why they would end up
> doing that.

So, you insist in asking the user to chose between rebase and merge, but
you also insist that they will not chose rebase? So, why ask?

> 'git commit' by default "prevents" users from creating commits without
> first adding changes to the staging area, and since it's a concept
> unique to Git, it's fair to say that none of the newcomers understand
> why 'git commit' is failing, the error messages is not particularly
> useful either.

I don't particularly agree that not defaulting to --all was a good idea,
but that's another topic.

But the error message is rather clear:

  no changes added to commit (use "git add" and/or "git commit -a")

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

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

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> You are in favor of an _option_ to allow people to forbid a pull in
>> a non-ff situation, and I think other people are also in
>> agreement.
>
> Yes. Having an option can't harm anybody, and there's a clear demand for
> that.
>
>> So perhaps:
>>
>>  - drop jc/pull-training-wheel and revert its merge from 'next';
>>
>>  - update Felipe's series with a bit of tweak to make it less
>>    impactful by demoting error into warning and advice.
>>
>> would be a good way forward?
>
> I didn't follow very closely the discussions and patch series, but that
> would sound right to me. The last version of Felipes' patch series
> already gives a warning only, but the wording and commit message implies
> that this will become an error in the future (this is the part with
> which I disagree).

OK, the first step to drop jc/pull-training-wheel from 'next' has
been done. I _think_ the one that starts at $gmane/234295 is the
newer incarnation of the patches in this thread, but that seems to
do a lot more than what the patches in this thread did, and it also
badly interacts with another topic in flight that updates git-pull,
so I have a topic branch for it but haven't merged to 'pu' yet.

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

[Felipe Contreras]

On Tue, Sep 10, 2013 at 3:26 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> The problem is the newcomers, and the newcomers will most definitely
>> not activate a configuration option to tell them that they are doing
>> something potentially undesirable.
>
> I teach Git to 200 newcommers each year. All of them run "git pull" the
> first day, but believe me, very few of them want to know what a rebase
> is at that time.

And who says they have to? This is a straw man argument.

May of them don't want to know what the staging area is, that's why
they run 'git commit --all', and just like that they can run 'git pull
--merge'.

>> By the time they learn about pull.mode, they probably already know
>> what a rebase is. So what is the point of the configuration in the
>> first place?
> [...]
>> That doesn't mean anything, you are assuming the user will do 'git
>> pull --rebase', and there's no rationale as to why they would end up
>> doing that.
>
> So, you insist in asking the user to chose between rebase and merge, but
> you also insist that they will not chose rebase? So, why ask?

Because as you said, they don't know what that is.

>> 'git commit' by default "prevents" users from creating commits without
>> first adding changes to the staging area, and since it's a concept
>> unique to Git, it's fair to say that none of the newcomers understand
>> why 'git commit' is failing, the error messages is not particularly
>> useful either.
>
> I don't particularly agree that not defaulting to --all was a good idea,
> but that's another topic.

It the same topic, the project already made a choice, and precisely
because of the same reasoning that 'git commit --all' is required,
'git pull --merge' should be required.

> But the error message is rather clear:
>
>   no changes added to commit (use "git add" and/or "git commit -a")

And we can do the same:

"Read more with 'git pull --help' or do 'git pull --merge'."

-- 
Felipe Contreras

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

[Matthieu Moy]

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

> On Tue, Sep 10, 2013 at 3:26 AM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>
>> So, you insist in asking the user to chose between rebase and merge, but
>> you also insist that they will not chose rebase? So, why ask?
>
> Because as you said, they don't know what that is.

That does not answer my question: why ask?

Look around you what people say about Git. See how many complain about
Git not exposing enough complexity to the user. See how many would
complain about Git not advertising rebase enough. Then, look how many
complain about Git exposing too much complexity and making it too easy
to use features like rebase.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

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

[Felipe Contreras]

On Wed, Sep 11, 2013 at 6:38 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Tue, Sep 10, 2013 at 3:26 AM, Matthieu Moy
>> <Matthieu.Moy@grenoble-inp.fr> wrote:
>>
>>> So, you insist in asking the user to chose between rebase and merge, but
>>> you also insist that they will not chose rebase? So, why ask?
>>
>> Because as you said, they don't know what that is.
>
> That does not answer my question: why ask?

If you have to ask, then you haven't read the commit messages, the
cover letter, or the relevant discussion. Even Linus Torvalds agreed
this was a good change.

> Look around you what people say about Git. See how many complain about
> Git not exposing enough complexity to the user. See how many would
> complain about Git not advertising rebase enough. Then, look how many
> complain about Git exposing too much complexity and making it too easy
> to use features like rebase.

And see how many are confused by Git doing something they never told
it to do, and then being totally lost because they are in the middle
of a state they don't understand, and how many do merges by mistake.

There's a reason why Git user-base considers 'git pull' dangerous.

-- 
Felipe Contreras