Skip to content

Commit b55e677

Browse files
moygitster
authored andcommitted
push: introduce new push.default mode "simple"
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 used to push to shared repositories, and hard to explain to beginners in some contexts. 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 to the upstream branch, but only if it has the same name remotely. If not, give an error that suggests 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 if someone shows us wrong. Original-patch-by: Jeff King <[email protected]> Signed-off-by: Matthieu Moy <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 321e75c commit b55e677

File tree

5 files changed

+98
-5
lines changed

5 files changed

+98
-5
lines changed

Documentation/config.txt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1686,13 +1686,16 @@ push.default::
16861686
appropriate for pushing into a repository shared by multiple users,
16871687
since locally stalled branches will attempt a non-fast forward push
16881688
if other users updated the branch. This is the default.
1689+
* `simple` - like `upstream`, but refuses to push if the upstream
1690+
branch's name is different from the local one. This is the safest
1691+
option and is well-suited for beginners.
16891692
* `upstream` - push the current branch to its upstream branch.
16901693
With this, `git push` will update the same remote ref as the one which
16911694
is merged by `git pull`, making `push` and `pull` symmetrical.
16921695
See "branch.<name>.merge" for how to configure the upstream branch.
16931696
* `current` - push the current branch to a branch of the same name.
16941697
+
1695-
The `current` and `upstream` modes are for those who want to
1698+
The `simple`, `current` and `upstream` modes are for those who want to
16961699
push out a single branch after finishing work, even when the other
16971700
branches are not yet ready to be pushed out. If you are working with
16981701
other people to push into the same shared repository, you would want

builtin/push.c

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,44 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
7676
return remote->url_nr;
7777
}
7878

79-
static void setup_push_upstream(struct remote *remote)
79+
static NORETURN int die_push_simple(struct branch *branch, struct remote *remote) {
80+
/*
81+
* There's no point in using shorten_unambiguous_ref here,
82+
* as the ambiguity would be on the remote side, not what
83+
* we have locally. Plus, this is supposed to be the simple
84+
* mode. If the user is doing something crazy like setting
85+
* upstream to a non-branch, we should probably be showing
86+
* them the big ugly fully qualified ref.
87+
*/
88+
const char *advice_maybe = "";
89+
const char *short_upstream =
90+
skip_prefix(branch->merge[0]->src, "refs/heads/");
91+
92+
if (!short_upstream)
93+
short_upstream = branch->merge[0]->src;
94+
/*
95+
* Don't show advice for people who explicitely set
96+
* push.default.
97+
*/
98+
if (push_default == PUSH_DEFAULT_UNSPECIFIED)
99+
advice_maybe = _("\n"
100+
"To choose either option permanently, "
101+
"see push.default in 'git help config'.");
102+
die(_("The upstream branch of your current branch does not match\n"
103+
"the name of your current branch. To push to the upstream branch\n"
104+
"on the remote, use\n"
105+
"\n"
106+
" git push %s HEAD:%s\n"
107+
"\n"
108+
"To push to the branch of the same name on the remote, use\n"
109+
"\n"
110+
" git push %s %s\n"
111+
"%s"),
112+
remote->name, short_upstream,
113+
remote->name, branch->name, advice_maybe);
114+
}
115+
116+
static void setup_push_upstream(struct remote *remote, int simple)
80117
{
81118
struct strbuf refspec = STRBUF_INIT;
82119
struct branch *branch = branch_get(NULL);
@@ -103,6 +140,8 @@ static void setup_push_upstream(struct remote *remote)
103140
"your current branch '%s', without telling me what to push\n"
104141
"to update which remote branch."),
105142
remote->name, branch->name);
143+
if (simple && strcmp(branch->refname, branch->merge[0]->src))
144+
die_push_simple(branch, remote);
106145

107146
strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
108147
add_refspec(refspec.buf);
@@ -119,8 +158,12 @@ static void setup_default_push_refspecs(struct remote *remote)
119158
add_refspec(":");
120159
break;
121160

161+
case PUSH_DEFAULT_SIMPLE:
162+
setup_push_upstream(remote, 1);
163+
break;
164+
122165
case PUSH_DEFAULT_UPSTREAM:
123-
setup_push_upstream(remote);
166+
setup_push_upstream(remote, 0);
124167
break;
125168

126169
case PUSH_DEFAULT_CURRENT:

cache.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,7 @@ enum rebase_setup_type {
624624
enum push_default_type {
625625
PUSH_DEFAULT_NOTHING = 0,
626626
PUSH_DEFAULT_MATCHING,
627+
PUSH_DEFAULT_SIMPLE,
627628
PUSH_DEFAULT_UPSTREAM,
628629
PUSH_DEFAULT_CURRENT,
629630
PUSH_DEFAULT_UNSPECIFIED

config.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,8 @@ static int git_default_push_config(const char *var, const char *value)
829829
push_default = PUSH_DEFAULT_NOTHING;
830830
else if (!strcmp(value, "matching"))
831831
push_default = PUSH_DEFAULT_MATCHING;
832+
else if (!strcmp(value, "simple"))
833+
push_default = PUSH_DEFAULT_SIMPLE;
832834
else if (!strcmp(value, "upstream"))
833835
push_default = PUSH_DEFAULT_UPSTREAM;
834836
else if (!strcmp(value, "tracking")) /* deprecated */
@@ -837,8 +839,8 @@ static int git_default_push_config(const char *var, const char *value)
837839
push_default = PUSH_DEFAULT_CURRENT;
838840
else {
839841
error("Malformed value for %s: %s", var, value);
840-
return error("Must be one of nothing, matching, "
841-
"tracking or current.");
842+
return error("Must be one of nothing, matching, simple, "
843+
"upstream or current.");
842844
}
843845
return 0;
844846
}

t/t5528-push-default.sh

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,48 @@ test_expect_success '"upstream" does not push when remotes do not match' '
7171
test_must_fail git push parent2
7272
'
7373

74+
test_expect_success 'push from/to new branch with upstream, matching and simple' '
75+
git checkout -b new-branch &&
76+
test_push_failure simple &&
77+
test_push_failure matching &&
78+
test_push_failure upstream
79+
'
80+
81+
test_expect_success 'push from/to new branch with current creates remote branch' '
82+
test_config branch.new-branch.remote repo1 &&
83+
git checkout new-branch &&
84+
test_push_success current new-branch
85+
'
86+
87+
test_expect_success 'push to existing branch, with no upstream configured' '
88+
test_config branch.master.remote repo1 &&
89+
git checkout master &&
90+
test_push_failure simple &&
91+
test_push_failure upstream
92+
'
93+
94+
test_expect_success 'push to existing branch, upstream configured with same name' '
95+
test_config branch.master.remote repo1 &&
96+
test_config branch.master.merge refs/heads/master &&
97+
git checkout master &&
98+
test_commit six &&
99+
test_push_success upstream master &&
100+
test_commit seven &&
101+
test_push_success simple master
102+
'
103+
104+
test_expect_success 'push to existing branch, upstream configured with different name' '
105+
test_config branch.master.remote repo1 &&
106+
test_config branch.master.merge refs/heads/other-name &&
107+
git checkout master &&
108+
test_commit eight &&
109+
test_push_success upstream other-name &&
110+
test_commit nine &&
111+
test_push_failure simple &&
112+
git --git-dir=repo1 log -1 --format="%h %s" "other-name" >expect-other-name &&
113+
test_push_success current master &&
114+
git --git-dir=repo1 log -1 --format="%h %s" "other-name" >actual-other-name &&
115+
test_cmp expect-other-name actual-other-name
116+
'
117+
74118
test_done

0 commit comments

Comments
 (0)