push: truly use "simple" as default, not "upstream"

The plan for the push.default transition had all along been to use the "simple" method rather than "upstream" as a default if the user did not specify their own push.default value. Commit 11037ee (push: switch default from "matching" to "simple", 2013-01-04) tried to implement that by moving PUSH_DEFAULT_UNSPECIFIED in our switch statement to fall-through to the PUSH_DEFAULT_SIMPLE case. When the commit that became 11037ee was originally written, that would have been enough. We would fall through to calling setup_push_upstream() with the "simple" parameter set to 1. However, it was delayed for a while until we were ready to make the transition in Git 2.0. And in the meantime, commit ed2b182 (push: change `simple` to accommodate triangular workflows, 2013-06-19) threw a monkey wrench into the works. That commit drops the "simple" parameter to setup_push_upstream, and instead checks whether the global "push_default" is PUSH_DEFAULT_SIMPLE. This is right when the user has explicitly configured push.default to simple, but wrong when we are a fall-through for the "unspecified" case. We never noticed because our push.default tests do not cover the case of the variable being totally unset; they only check the "simple" behavior itself. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Nov 26, 2014 at 22:43 UTC 00a6fa0720283b93eb011adcfea850fe21345548
2 files changed +34 -6
builtin/push.c
+4 -4
@@ -162,7 +162,7 @@ static const char message_detached_head_die[] =
162 " git push %s HEAD:<name-of-remote-branch>\n");
163
164 static void setup_push_upstream(struct remote *remote, struct branch *branch,
165 - int triangular)
165 + int triangular, int simple)
166 {
167 struct strbuf refspec = STRBUF_INIT;
168
@@ -185,7 +185,7 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,
185 "to update which remote branch."),
186 remote->name, branch->name);
187
188 - if (push_default == PUSH_DEFAULT_SIMPLE) {
188 + if (simple) {
189 /* Additional safety */
190 if (strcmp(branch->refname, branch->merge[0]->src))
191 die_push_simple(branch, remote);
@@ -258,11 +258,11 @@ static void setup_default_push_refspecs(struct remote *remote)
258 if (triangular)
259 setup_push_current(remote, branch);
260 else
261 - setup_push_upstream(remote, branch, triangular);
261 + setup_push_upstream(remote, branch, triangular, 1);
262 break;
263
264 case PUSH_DEFAULT_UPSTREAM:
265 - setup_push_upstream(remote, branch, triangular);
265 + setup_push_upstream(remote, branch, triangular, 0);
266 break;
267
268 case PUSH_DEFAULT_CURRENT:
t/t5528-push-default.sh
+30 -2
@@ -26,7 +26,7 @@ check_pushed_commit () {
26 # $2 = expected target branch for the push
27 # $3 = [optional] repo to check for actual output (repo1 by default)
28 test_push_success () {
29 - git -c push.default="$1" push &&
29 + git ${1:+-c push.default="$1"} push &&
30 check_pushed_commit HEAD "$2" "$3"
31 }
32
@@ -34,7 +34,7 @@ test_push_success () {
34 # check that push fails and does not modify any remote branch
35 test_push_failure () {
36 git --git-dir=repo1 log --no-walk --format='%h %s' --all >expect &&
37 - test_must_fail git -c push.default="$1" push &&
37 + test_must_fail git ${1:+-c push.default="$1"} push &&
38 git --git-dir=repo1 log --no-walk --format='%h %s' --all >actual &&
39 test_cmp expect actual
40 }
@@ -172,4 +172,32 @@ test_pushdefault_workflow success simple master triangular
172 # master is updated (parent2 does not have foo)
173 test_pushdefault_workflow success matching master triangular
174
175 +# default tests, when no push-default is specified. This
176 +# should behave the same as "simple" in non-triangular
177 +# settings, and as "current" otherwise.
178 +
179 +test_expect_success 'default behavior allows "simple" push' '
180 + test_config branch.master.remote parent1 &&
181 + test_config branch.master.merge refs/heads/master &&
182 + test_config remote.pushdefault parent1 &&
183 + test_commit default-master-master &&
184 + test_push_success "" master
185 +'
186 +
187 +test_expect_success 'default behavior rejects non-simple push' '
188 + test_config branch.master.remote parent1 &&
189 + test_config branch.master.merge refs/heads/foo &&
190 + test_config remote.pushdefault parent1 &&
191 + test_commit default-master-foo &&
192 + test_push_failure ""
193 +'
194 +
195 +test_expect_success 'default triangular behavior acts like "current"' '
196 + test_config branch.master.remote parent1 &&
197 + test_config branch.master.merge refs/heads/foo &&
198 + test_config remote.pushdefault parent2 &&
199 + test_commit default-triangular &&
200 + test_push_success "" master repo2
201 +'
202 +
203 test_done