[PATCH 00/10] push: switch default from "matching" to "simple"

[PATCH 00/10] push: switch default from "matching" to "simple"

[Junio C Hamano]

We promised to change the behaviour of lazy "git push [there]" that
does not say what to push on the command line from "matching" to
"simple" in Git 2.0.  Even though the top-level RelNotes symbolic
link points at 1.8.2, we can change our mind to tag 2.0 at the end
of this cycle.

This carries out the threat ;-)

Mmany tests have assumed that the long established default will
never change and relied on the convenience of the "matching"
behaviour.  They need to be updated to either specify what they want
from the command line explicitly, or configure the default behaviour
they would expect.  And then finally we can flip the default.

Junio C Hamano (10):
  t5404: do not assume the "matching" push is the default
  t5505: do not assume the "matching" push is the default
  t5516: do not assume the "matching" push is the default
  t5517: do not assume the "matching" push is the default
  t5519: do not assume the "matching" push is the default
  t5531: do not assume the "matching" push is the default
  t7406: do not assume the "matching" push is the default
  t9400: do not assume the "matching" push is the default
  t9401: do not assume the "matching" push is the default
  push: switch default from "matching" to "simple"

 Documentation/config.txt        |  6 +++---
 builtin/push.c                  | 31 +++++++------------------------
 t/t5404-tracking-branches.sh    |  2 +-
 t/t5505-remote.sh               |  2 +-
 t/t5516-fetch-push.sh           | 10 +++++-----
 t/t5517-push-mirror.sh          |  2 +-
 t/t5519-push-alternates.sh      | 12 ++++++------
 t/t5531-deep-submodule-push.sh  |  1 +
 t/t7406-submodule-update.sh     |  4 ++--
 t/t9400-git-cvsserver-server.sh |  1 +
 t/t9401-git-cvsserver-crlf.sh   |  1 +
 11 files changed, 29 insertions(+), 43 deletions(-)

-- 
1.8.1.299.gc73b41f

[PATCH 01/10] t5404: do not assume the "matching" push is the default

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5404-tracking-branches.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh
index c240035..2b8c0ba 100755
--- a/t/t5404-tracking-branches.sh
+++ b/t/t5404-tracking-branches.sh
@@ -36,7 +36,7 @@ test_expect_success 'prepare pushable branches' '
 '
 
 test_expect_success 'mixed-success push returns error' '
-	test_must_fail git push
+	test_must_fail git push origin :
 '
 
 test_expect_success 'check tracking branches updated correctly after push' '
-- 
1.8.1.299.gc73b41f

[PATCH 02/10] t5505: do not assume the "matching" push is the default

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5505-remote.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index ccc55eb..6579a86 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -345,7 +345,7 @@ test_expect_success 'fetch mirrors do not act as mirrors during push' '
 	) &&
 	(cd mirror-fetch/child &&
 	 git branch -m renamed renamed2 &&
-	 git push parent
+	 git push parent :
 	) &&
 	(cd mirror-fetch/parent &&
 	 git rev-parse --verify renamed &&
-- 
1.8.1.299.gc73b41f

[PATCH 03/10] t5516: do not assume the "matching" push is the default

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5516-fetch-push.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b5417cc..1a8753d 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -247,7 +247,7 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf
 test_expect_success 'push with matching heads' '
 
 	mk_test heads/master &&
-	git push testrepo &&
+	git push testrepo : &&
 	check_push_result $the_commit heads/master
 
 '
@@ -276,7 +276,7 @@ test_expect_success 'push --force with matching heads' '
 	mk_test heads/master &&
 	git push testrepo : &&
 	git commit --amend -massaged &&
-	git push --force testrepo &&
+	git push --force testrepo : &&
 	! check_push_result $the_commit heads/master &&
 	git reset --hard $the_commit
 
@@ -507,7 +507,7 @@ test_expect_success 'push with config remote.*.pushurl' '
 	git checkout master &&
 	git config remote.there.url test2repo &&
 	git config remote.there.pushurl testrepo &&
-	git push there &&
+	git push there : &&
 	check_push_result $the_commit heads/master
 '
 
@@ -521,7 +521,7 @@ test_expect_success 'push with dry-run' '
 		cd testrepo &&
 		old_commit=$(git show-ref -s --verify refs/heads/master)
 	) &&
-	git push --dry-run testrepo &&
+	git push --dry-run testrepo : &&
 	check_push_result $old_commit heads/master
 '
 
@@ -981,7 +981,7 @@ test_expect_success 'push --porcelain --dry-run rejected' '
 
 test_expect_success 'push --prune' '
 	mk_test heads/master heads/second heads/foo heads/bar &&
-	git push --prune testrepo &&
+	git push --prune testrepo : &&
 	check_push_result $the_commit heads/master &&
 	check_push_result $the_first_commit heads/second &&
 	! check_push_result $the_first_commit heads/foo heads/bar
-- 
1.8.1.299.gc73b41f

[PATCH 04/10] t5517: do not assume the "matching" push is the default

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5517-push-mirror.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5517-push-mirror.sh b/t/t5517-push-mirror.sh
index e2ad260..12a5dfb 100755
--- a/t/t5517-push-mirror.sh
+++ b/t/t5517-push-mirror.sh
@@ -256,7 +256,7 @@ test_expect_success 'remote.foo.mirror=no has no effect' '
 		git branch keep master &&
 		git push --mirror up &&
 		git branch -D keep &&
-		git push up
+		git push up :
 	) &&
 	(
 		cd mirror &&
-- 
1.8.1.299.gc73b41f

[PATCH 05/10] t5519: do not assume the "matching" push is the default

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5519-push-alternates.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t5519-push-alternates.sh b/t/t5519-push-alternates.sh
index c00c9b0..11fcd37 100755
--- a/t/t5519-push-alternates.sh
+++ b/t/t5519-push-alternates.sh
@@ -40,7 +40,7 @@ test_expect_success 'alice works and pushes' '
 		cd alice-work &&
 		echo more >file &&
 		git commit -a -m second &&
-		git push ../alice-pub
+		git push ../alice-pub :
 	)
 '
 
@@ -57,7 +57,7 @@ test_expect_success 'bob fetches from alice, works and pushes' '
 		git pull ../alice-pub master &&
 		echo more bob >file &&
 		git commit -a -m third &&
-		git push ../bob-pub
+		git push ../bob-pub :
 	) &&
 
 	# Check that the second commit by Alice is not sent
@@ -86,7 +86,7 @@ test_expect_success 'alice works and pushes again' '
 		cd alice-work &&
 		echo more alice >file &&
 		git commit -a -m fourth &&
-		git push ../alice-pub
+		git push ../alice-pub :
 	)
 '
 
@@ -99,7 +99,7 @@ test_expect_success 'bob works and pushes' '
 		cd bob-work &&
 		echo yet more bob >file &&
 		git commit -a -m fifth &&
-		git push ../bob-pub
+		git push ../bob-pub :
 	)
 '
 
@@ -115,7 +115,7 @@ test_expect_success 'alice works and pushes yet again' '
 		git commit -a -m sixth.2 &&
 		echo more and more alice >>file &&
 		git commit -a -m sixth.3 &&
-		git push ../alice-pub
+		git push ../alice-pub :
 	)
 '
 
@@ -136,7 +136,7 @@ test_expect_success 'bob works and pushes again' '
 		git hash-object -t commit -w commit &&
 		echo even more bob >file &&
 		git commit -a -m seventh &&
-		git push ../bob-pub
+		git push ../bob-pub :
 	)
 '
 
-- 
1.8.1.299.gc73b41f

[PATCH 06/10] t5531: do not assume the "matching" push is the default

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5531-deep-submodule-push.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 1947c28..8c16e04 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -16,6 +16,7 @@ test_expect_success setup '
 		(
 			cd gar/bage &&
 			git init &&
+			git config push.default matching &&
 			>junk &&
 			git add junk &&
 			git commit -m "Initial junk"
-- 
1.8.1.299.gc73b41f

[PATCH 07/10] t7406: do not assume the "matching" push is the default

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t7406-submodule-update.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index feaec6c..c675ce6 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -565,14 +565,14 @@ test_expect_success 'submodule add places git-dir in superprojects git-dir recur
 	   git log > ../../../expected
 	  ) &&
 	  git commit -m "added subsubmodule" &&
-	  git push
+	  git push origin :
 	 ) &&
 	 (cd .git/modules/deeper/submodule/modules/subsubmodule &&
 	  git log > ../../../../../actual
 	 ) &&
 	 git add deeper/submodule &&
 	 git commit -m "update submodule" &&
-	 git push &&
+	 git push origin : &&
 	 test_cmp actual expected
 	)
 '
-- 
1.8.1.299.gc73b41f

[PATCH 08/10] t9400: do not assume the "matching" push is the default

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t9400-git-cvsserver-server.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 9502f24..0431386 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -36,6 +36,7 @@ export CVSROOT CVS_SERVER
 
 rm -rf "$CVSWORK" "$SERVERDIR"
 test_expect_success 'setup' '
+  git config push.default matching &&
   echo >empty &&
   git add empty &&
   git commit -q -m "First Commit" &&
-- 
1.8.1.299.gc73b41f

[PATCH 09/10] t9401: do not assume the "matching" push is the default

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t9401-git-cvsserver-crlf.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh
index 1c5bc84..8c3db76 100755
--- a/t/t9401-git-cvsserver-crlf.sh
+++ b/t/t9401-git-cvsserver-crlf.sh
@@ -84,6 +84,7 @@ export CVSROOT CVS_SERVER
 
 rm -rf "$CVSWORK" "$SERVERDIR"
 test_expect_success 'setup' '
+    git config push.default matching &&
     echo "Simple text file" >textfile.c &&
     echo "File with embedded NUL: Q <- there" | q_to_nul > binfile.bin &&
     mkdir subdir &&
-- 
1.8.1.299.gc73b41f

[PATCH 10/10] push: switch default from "matching" to "simple"

[Junio C Hamano]

We promised to change the behaviour of lazy "git push [there]" that
does not say what to push on the command line from "matching" to
"simple" in Git 2.0.

This finally flips that bit.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt |  6 +++---
 builtin/push.c           | 31 +++++++------------------------
 2 files changed, 10 insertions(+), 27 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index bf8f911..770eefe 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1750,15 +1750,15 @@ push.default::
   since locally stalled branches will attempt a non-fast forward push
   if other users updated the branch.
   +
-  This is currently the default, but Git 2.0 will change the default
-  to `simple`.
+  This used to be the default, and stale web sites may still say so,
+  but Git 2.0 has changed the default to `simple`.
 * `upstream` - push the current branch to its upstream branch.
   With this, `git push` will update the same remote ref as the one which
   is merged by `git pull`, making `push` and `pull` symmetrical.
   See "branch.<name>.merge" for how to configure the upstream branch.
 * `simple` - like `upstream`, but refuses to push if the upstream
   branch's name is different from the local one. This is the safest
-  option and is well-suited for beginners. It will become the default
+  option and is well-suited for beginners. It has become the default
   in Git 2.0.
 * `current` - push the current branch to a branch of the same name.
 --
diff --git a/builtin/push.c b/builtin/push.c
index db9ba30..9f7c252 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -24,7 +24,6 @@ static int progress = -1;
 static const char **refspec;
 static int refspec_nr;
 static int refspec_alloc;
-static int default_matching_used;
 
 static void add_refspec(const char *ref)
 {
@@ -148,9 +147,9 @@ static void setup_push_upstream(struct remote *remote, int simple)
 }
 
 static char warn_unspecified_push_default_msg[] =
-N_("push.default is unset; its implicit value is changing in\n"
+N_("push.default is unset; its implicit value has changed in\n"
    "Git 2.0 from 'matching' to 'simple'. To squelch this message\n"
-   "and maintain the current behavior after the default changes, use:\n"
+   "and maintain the traditional behavior, use:\n"
    "\n"
    "  git config --global push.default matching\n"
    "\n"
@@ -175,14 +174,14 @@ static void setup_default_push_refspecs(struct remote *remote)
 {
 	switch (push_default) {
 	default:
-	case PUSH_DEFAULT_UNSPECIFIED:
-		default_matching_used = 1;
-		warn_unspecified_push_default_configuration();
-		/* fallthru */
 	case PUSH_DEFAULT_MATCHING:
 		add_refspec(":");
 		break;
 
+	case PUSH_DEFAULT_UNSPECIFIED:
+		warn_unspecified_push_default_configuration();
+		/* fallthru */
+
 	case PUSH_DEFAULT_SIMPLE:
 		setup_push_upstream(remote, 1);
 		break;
@@ -208,12 +207,6 @@ static const char message_advice_pull_before_push[] =
 	   "before pushing again.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
-static const char message_advice_use_upstream[] =
-	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
-	   "counterpart. If you did not intend to push that branch, you may want to\n"
-	   "specify branches to push or set the 'push.default' configuration variable\n"
-	   "to 'simple', 'current' or 'upstream' to push only the current branch.");
-
 static const char message_advice_checkout_pull_push[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
 	   "counterpart. Check out this branch and merge the remote changes\n"
@@ -227,13 +220,6 @@ static void advise_pull_before_push(void)
 	advise(_(message_advice_pull_before_push));
 }
 
-static void advise_use_upstream(void)
-{
-	if (!advice_push_non_ff_default || !advice_push_nonfastforward)
-		return;
-	advise(_(message_advice_use_upstream));
-}
-
 static void advise_checkout_pull_push(void)
 {
 	if (!advice_push_non_ff_matching || !advice_push_nonfastforward)
@@ -272,10 +258,7 @@ static int push_with_options(struct transport *transport, int flags)
 		advise_pull_before_push();
 		break;
 	case NON_FF_OTHER:
-		if (default_matching_used)
-			advise_use_upstream();
-		else
-			advise_checkout_pull_push();
+		advise_checkout_pull_push();
 		break;
 	}
 
-- 
1.8.1.299.gc73b41f

Re: [PATCH 00/10] push: switch default from "matching" to "simple"

[Matthieu Moy]

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

> We promised to change the behaviour of lazy "git push [there]" that
> does not say what to push on the command line from "matching" to
> "simple" in Git 2.0.  Even though the top-level RelNotes symbolic
> link points at 1.8.2, we can change our mind to tag 2.0 at the end
> of this cycle.

I had a quick look at the series, it sounds straightforward and correct.
Thanks,

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

[PATCH 11/10] doc: push.default is no longer "matching"

[Junio C Hamano]

The documentation for the command said that `push.default` is the
default without referring to the releavant manual page.

Now `simple` is the default behaviour. Document it right there where
we say we take the default value from `push.default`, and remove the
description of old default being 'matching'.

Also reword cryptic desription of `--all`.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I skimmed our two tutorials, just to be sure, but they do not
   discuss 'git push', so there was nothing to update there.

   I *think* we are minimally ready to switch the default behaviour
   now, but there may be other places that need similar adjustment.
   An independent audit is highly appreciated.

 Documentation/git-push.txt | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 8b637d3..f326afb 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -36,10 +36,14 @@ OPTIONS[[OPTIONS]]
 	The format of a <refspec> parameter is an optional plus
 	`+`, followed by the source ref <src>, followed
 	by a colon `:`, followed by the destination ref <dst>.
-	It is used to specify with what <src> object the <dst> ref
-	in the remote repository is to be updated.  If not specified,
+	It is used to specify what <src> object the <dst> ref
+	in the remote repository is to be updated to.  If no
+	<refspec> is specified on the command line, and if no
+	<refspec> is configured for the <repository>,
 	the behavior of the command is controlled by the `push.default`
-	configuration variable.
+	configuration variable, and if it is unset, the `simple`
+	behaviour is used (see lingit:git-config[1] and look
+	for `push.default`).
 +
 The <src> is often the name of the branch you would want to push, but
 it can be any arbitrary "SHA-1 expression", such as `master~4` or
@@ -65,14 +69,11 @@ the remote repository.
 The special refspec `:` (or `+:` to allow non-fast-forward updates)
 directs git to push "matching" branches: for every branch that exists on
 the local side, the remote side is updated if a branch of the same name
-already exists on the remote side.  This is the default operation mode
-if no explicit refspec is found (that is neither on the command line
-nor in any Push line of the corresponding remotes file---see below) and
-no `push.default` configuration variable is set.
+already exists on the remote side.
 
 --all::
-	Instead of naming each ref to push, specifies that all
-	refs under `refs/heads/` be pushed.
+	Push all branches (i.e. refs under `refs/heads/`); cannot be
+	used with other <refspec>.
 
 --prune::
 	Remove remote branches that don't have a local counterpart. For example
-- 
1.8.1.317.ga1ba510