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 <peff@xxxxxxxx> Signed-off-by: Matthieu Moy <Matthieu.Moy@xxxxxxx> --- Documentation/config.txt | 5 ++++- builtin/push.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- cache.h | 1 + config.c | 6 ++++-- t/t5528-push-default.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 95 insertions(+), 5 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index ddf6043..88d739a 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1688,6 +1688,9 @@ push.default:: repository shared by multiple users, since locally stalled branches will attempt a non-fast forward push if other users updated the branch remotely. This is the default. +* `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. * `upstream` - push the current branch to its upstream branch. See "branch.<name>.merge" for how to configure the upstream branch. This makes `git push` and `git pull` symmetrical in the sense that `push` @@ -1695,7 +1698,7 @@ push.default:: `git pull`. * `current` - push the current branch to a branch of the same name. + - The `current` and `upstream` modes are for those who want to + The `simple`, `current` and `upstream` modes are for those who want to push out a single branch after finishing work, even when the other branches are not yet ready to be pushed out. They are safe when pushing to a shared repository. diff --git a/builtin/push.c b/builtin/push.c index 6936713..dae8306 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -76,7 +76,40 @@ 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) +NORETURN die_push_simple(struct branch *branch, struct remote *remote) { + /* + * 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/"); + /* + * Don't show advice for people who explicitely set + * push.default. + */ + const char *advice_maybe = ""; + if (push_default == PUSH_DEFAULT_UNSPECIFIED) + advice_maybe = _("\n" + "To choose either option permanently, " + "see push.default in 'git help config'."); + 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" + "%s"), + remote->name, short_up ? short_up : branch->merge[0]->src, + remote->name, branch->name, advice_maybe); +} + +static void setup_push_upstream(struct remote *remote, int simple) { struct strbuf refspec = STRBUF_INIT; struct branch *branch = branch_get(NULL); @@ -103,6 +136,9 @@ 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)) { + die_push_simple(branch, remote); + } strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src); add_refspec(refspec.buf); @@ -119,8 +155,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 5bf59ff..b60d490 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, PUSH_DEFAULT_UNSPECIFIED diff --git a/config.c b/config.c index 68d3294..bfe0c79 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,8 +839,8 @@ 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, " - "tracking or current."); + return error("Must be one of nothing, matching, simple, " + "upstream or current."); } return 0; } diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh index da7d3d8..43dec43 100755 --- a/t/t5528-push-default.sh +++ b/t/t5528-push-default.sh @@ -82,4 +82,48 @@ 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_push_failure simple new-branch && + test_push_failure matching new-branch && + test_push_failure upstream new-branch +' + +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_push_failure simple master && + test_push_failure upstream master +' + +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 +' + +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_push_failure simple new-branch && + git --git-dir=repo1 log -1 --format="%h %s" "other-name" >expect-other-name && + test_push_success current master && + git --git-dir=repo1 log -1 --format="%h %s" "other-name" >actual-other-name && + test_cmp expect-other-name actual-other-name +' + test_done -- 1.7.10.234.ge65dd.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html