[PATCH 1/3] 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.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 builtin/push.c        |   47 +++++++++++++++++++++++++++++++++++++++++++++--
 cache.h               |    1 +
 config.c              |    4 +++-
 t/t5516-fetch-push.sh |   33 +++++++++++++++++++++++++++++++++
 4 files changed, 82 insertions(+), 3 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index d315475..4602cd8 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -65,7 +65,17 @@ static void set_refspecs(const char **refs, int nr)
 	}
 }
 
-static void setup_push_upstream(struct remote *remote)
+static int push_url_of_remote(struct remote *remote, const char ***url_p)
+{
+	if (remote->pushurl_nr) {
+		*url_p = remote->pushurl;
+		return remote->pushurl_nr;
+	}
+	*url_p = remote->url;
+	return remote->url_nr;
+}
+
+static void setup_push_upstream(struct remote *remote, int simple)
 {
 	struct strbuf refspec = STRBUF_INIT;
 	struct branch *branch = branch_get(NULL);
@@ -87,6 +97,35 @@ static void setup_push_upstream(struct remote *remote)
 	if (branch->merge_nr != 1)
 		die(_("The current branch %s has multiple upstream branches, "
 		    "refusing to push."), branch->name);
+	if (strcmp(branch->remote_name, remote->name))
+		die(_("You are pushing to remote '%s', which is not the upstream of\n"
+		      "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"),
+		    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);
 }
@@ -99,8 +138,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 806bf2b..f1c1bb8 100644
--- a/cache.h
+++ b/cache.h
@@ -624,6 +624,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
 };
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/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b5417cc..f4f9d06 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -497,6 +497,39 @@ test_expect_success 'push with config remote.*.push = HEAD' '
 	check_push_result $the_first_commit heads/local
 '
 
+test_expect_success 'push without argument (push.default)' '
+	mk_test heads/master &&
+
+	test_debug "echo no remote branch" &&
+	git checkout $the_first_commit -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_debug "echo existing branch, no upstream configured" &&
+	git config branch.new-branch.remote there &&
+	git -c push.default=current push &&
+	git -c push.default=simple push &&
+	git -c push.default=matching push &&
+	git config --unset branch.new-branch.remote &&
+	test_must_fail git -c push.default=simple push &&
+
+	test_debug "echo upstream configured" &&
+	git push --set-upstream testrepo new-branch &&
+	git -c push.default=simple push &&
+	check_push_result $the_first_commit heads/new-branch &&
+	git config branch.new-branch.merge other-branch &&
+	test_must_fail git -c push.default=simple push &&
+	git -c push.default=upstream push &&
+	check_push_result $the_first_commit heads/other-branch &&
+	git push --set-upstream there new-branch &&
+
+	test_debug "echo advance local commit" &&
+	git merge $the_commit &&
+	git -c push.default=simple push &&
+	check_push_result $the_commit heads/new-branch
+'
+
 # clean up the cruft left with the previous one
 git config --remove-section remote.there
 git config --remove-section branch.master
-- 
1.7.10.140.g8c333