[PATCH 2/4] push: introduce new push.default mode "simple"

[Matthieu Moy <Matthieu.Moy@imag.fr>]

When calling "git push" without argument, we want to allow Git to do
something simple to explain and safe. push.default=matching is unsafe
when use to push to shared repositories, and hard to explain to beginners
in some context. It is debatable whether 'upstream' or 'current' is the
safest or the easiest to explain, so introduce a new mode called 'simple'
that is the intersection of them: push the upstream branch, but only if
it has the same name remotely. If not, give an error that suggest the
right command to push explicitely to 'upstream' or 'current'.

A question is whether to allow pushing when no upstream is configured. An
argument in favor of allowing the push is that it makes the new mode work
in more cases. On the other hand, refusing to push when no upstream is
configured encourages the user to set the upstream, which will be
beneficial on the next pull. Lacking better argument, we chose to deny
the push, because it will be easier to change in the future is someone
shows us wrong.

Original-patch-by: Jeff King <peff@peff.net>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Except for the broken-ness, this adds the last line in the warning message:

"To chose either option permanently, read about push.default in git-config(1)"

 Documentation/config.txt |    3 +++
 builtin/push.c           |   32 +++++++++++++++++++++--
 cache.h                  |    1 +
 config.c                 |    4 ++-
 t/t5528-push-default.sh  |   63 +++++++++++++++++++++++++++++++++++++++++++---
 5 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 368a770..05d1472 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1697,6 +1697,9 @@ push.default::
   This option allows publishing a branch to a remote repository using
   the same naming convention locally and remotely, in a more
   conservative and safer way than `matching`.
+* `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.
 
 rebase.stat::
 	Whether to show a diffstat of what changed upstream since the last
diff --git a/builtin/push.c b/builtin/push.c
index 6936713..ba0d6a0 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -76,7 +76,7 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
 	return remote->url_nr;
 }
 
-static void setup_push_upstream(struct remote *remote)
+static void setup_push_upstream(struct remote *remote, int simple)
 {
 	struct strbuf refspec = STRBUF_INIT;
 	struct branch *branch = branch_get(NULL);
@@ -103,6 +103,30 @@ static void setup_push_upstream(struct remote *remote)
 		      "your current branch '%s', without telling me what to push\n"
 		      "to update which remote branch."),
 		    remote->name, branch->name);
+	if (simple && strcmp(branch->refname, branch->merge[0]->src)) {
+		/*
+		 * There's no point in using shorten_unambiguous_ref here,
+		 * as the ambiguity would be on the remote side, not what
+		 * we have locally. Plus, this is supposed to be the simple
+		 * mode. If the user is doing something crazy like setting
+		 * upstream to a non-branch, we should probably be showing
+		 * them the big ugly fully qualified ref.
+		 */
+		const char *short_up = skip_prefix(branch->merge[0]->src, "refs/heads/");
+		die(_("The upstream branch of your current branch does not match\n"
+		      "the name of your current branch.  To push to the upstream branch\n"
+		      "on the remote, use\n"
+		      "\n"
+		      "    git push %s HEAD:%s\n"
+		      "\n"
+		      "To push to the branch of the same name on the remote, use\n"
+		      "\n"
+		      "    git push %s %s\n"
+		      "\n"
+		      "To chose either option permanently, read about push.default in git-config(1)\n"),
+		    remote->name, short_up ? short_up : branch->merge[0]->src,
+		    remote->name, branch->name);
+	}
 
 	strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
 	add_refspec(refspec.buf);
@@ -119,8 +143,12 @@ static void setup_default_push_refspecs(struct remote *remote)
 		add_refspec(":");
 		break;
 
+	case PUSH_DEFAULT_SIMPLE:
+		setup_push_upstream(remote, 1);
+		break;
+
 	case PUSH_DEFAULT_UPSTREAM:
-		setup_push_upstream(remote);
+		setup_push_upstream(remote, 0);
 		break;
 
 	case PUSH_DEFAULT_CURRENT:
diff --git a/cache.h b/cache.h
index d8f6f1e..5e419a1 100644
--- a/cache.h
+++ b/cache.h
@@ -580,6 +580,7 @@ enum rebase_setup_type {
 enum push_default_type {
 	PUSH_DEFAULT_NOTHING = 0,
 	PUSH_DEFAULT_MATCHING,
+	PUSH_DEFAULT_SIMPLE,
 	PUSH_DEFAULT_UPSTREAM,
 	PUSH_DEFAULT_CURRENT,
 	PUSH_DEFAULT_UNSPECIFIED
diff --git a/config.c b/config.c
index 68d3294..024bc74 100644
--- a/config.c
+++ b/config.c
@@ -829,6 +829,8 @@ static int git_default_push_config(const char *var, const char *value)
 			push_default = PUSH_DEFAULT_NOTHING;
 		else if (!strcmp(value, "matching"))
 			push_default = PUSH_DEFAULT_MATCHING;
+		else if (!strcmp(value, "simple"))
+			push_default = PUSH_DEFAULT_SIMPLE;
 		else if (!strcmp(value, "upstream"))
 			push_default = PUSH_DEFAULT_UPSTREAM;
 		else if (!strcmp(value, "tracking")) /* deprecated */
@@ -837,7 +839,7 @@ static int git_default_push_config(const char *var, const char *value)
 			push_default = PUSH_DEFAULT_CURRENT;
 		else {
 			error("Malformed value for %s: %s", var, value);
-			return error("Must be one of nothing, matching, "
+			return error("Must be one of simple, nothing, matching, "
 				     "tracking or current.");
 		}
 		return 0;
diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index c334c51..949dbdf 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh
@@ -13,6 +13,22 @@ test_expect_success 'setup bare remotes' '
 	git push parent2 HEAD
 '
 
+# $1 = local revision
+# $2 = remote repository
+# $3 = remote revision (tested to be equal to the local one)
+check_pushed_commit () {
+	git rev-parse "$1" > expect &&
+	git --git-dir="$2" rev-parse "$3" > actual &&
+	test_cmp expect actual
+}
+
+# $1 = push.default value
+# $2 = expected target branch for the push
+test_push_success () {
+	git -c push.default="$1" push &&
+	check_pushed_commit HEAD repo1 "$2"
+}
+
 test_expect_success '"upstream" pushes to configured upstream' '
 	git checkout master &&
 	test_config branch.master.remote parent1 &&
@@ -20,9 +36,7 @@ test_expect_success '"upstream" pushes to configured upstream' '
 	test_config push.default upstream &&
 	test_commit two &&
 	git push &&
-	echo two >expect &&
-	git --git-dir=repo1 log -1 --format=%s foo >actual &&
-	test_cmp expect actual
+	check_pushed_commit HEAD repo1 foo
 '
 
 test_expect_success '"upstream" does not push on unconfigured remote' '
@@ -51,4 +65,47 @@ test_expect_success '"upstream" does not push when remotes do not match' '
 	test_must_fail git push parent2
 '
 
+test_expect_success 'push from/to new branch with upstream, matching and simple' '
+	git checkout -b new-branch &&
+	test_must_fail git -c push.default=simple push &&
+	test_must_fail git -c push.default=matching push &&
+	test_must_fail git -c push.default=upstream push
+'
+
+test_expect_success 'push from/to new branch with current creates remote branch' '
+	test_config branch.new-branch.remote repo1 &&
+	git checkout new-branch &&
+	test_push_success current new-branch
+'
+
+test_expect_success 'push to existing branch, with no upstream configured' '
+	test_config branch.master.remote repo1 &&
+	git checkout master &&
+	test_must_fail git -c push.default=simple push &&
+	test_must_fail git -c push.default=upstream push
+'
+
+test_expect_success 'push to existing branch, upstream configured with same name' '
+	test_config branch.master.remote repo1 &&
+	test_config branch.master.merge refs/heads/master &&
+	git checkout master &&
+	test_commit six &&
+	test_push_success upstream master &&
+	test_commit seven &&
+	test_push_success simple master &&
+	check_pushed_commit HEAD repo1 master
+'
+
+test_expect_success 'push to existing branch, upstream configured with different name' '
+	test_config branch.master.remote repo1 &&
+	test_config branch.master.merge refs/heads/other-name &&
+	git checkout master &&
+	test_commit eight &&
+	test_push_success upstream other-name &&
+	test_commit nine &&
+	test_must_fail git -c push.default=simple push &&
+	test_push_success current master &&
+	test_must_fail check_pushed_commit HEAD repo1 other-name
+'
+
 test_done
-- 
1.7.10.140.g8c333