From 185c0874b1adea696b47d2fb78aca9a282a86abb Mon Sep 17 00:00:00 2001 From: Matthieu Moy Date: Tue, 24 Apr 2012 09:50:00 +0200 Subject: [PATCH 1/7] Documentation: explain push.default option a bit more The previous documentation was explaining _what_ the options were doing, but were of little help explaining _why_ a user should set his default to either of the options. Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- Documentation/config.txt | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index fb386abc51..5f14871d2a 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1680,12 +1680,24 @@ push.default:: line. Possible values are: + * `nothing` - do not push anything. -* `matching` - push all matching branches. - All branches having the same name in both ends are considered to be - matching. This is the default. +* `matching` - push all branches having the same name in both ends. + This is for those who prepare all the branches into a publishable + shape and then push them out with a single command. It is not + appropriate for pushing into a repository shared by multiple users, + since locally stalled branches will attempt a non-fast forward push + if other users updated the branch. This is the default. * `upstream` - push the current branch to its upstream branch. + With this, `git push` will update the same remote ref as the one which + is merged by `git pull`, making `push` and `pull` symmetrical. + See "branch..merge" for how to configure the upstream branch. * `tracking` - deprecated synonym for `upstream`. * `current` - push the current branch to a branch of the same name. + + + The `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. If you are working with + other people to push into the same shared repository, you would want + to use one of these. rebase.stat:: Whether to show a diffstat of what changed upstream since the last From 628ab0ea1046800b9520c07d809db886b1ec3693 Mon Sep 17 00:00:00 2001 From: Matthieu Moy Date: Tue, 24 Apr 2012 09:50:01 +0200 Subject: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking' It's been deprecated since 53c4031 (Johan Herland, Wed Feb 16 2011, push.default: Rename 'tracking' to 'upstream'), so it's OK to remove it from documentation (even though it's still supported) to make the explanations more readable. Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- Documentation/config.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 5f14871d2a..9617c53951 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1690,7 +1690,6 @@ push.default:: With this, `git push` will update the same remote ref as the one which is merged by `git pull`, making `push` and `pull` symmetrical. See "branch..merge" for how to configure the upstream branch. -* `tracking` - deprecated synonym for `upstream`. * `current` - push the current branch to a branch of the same name. + The `current` and `upstream` modes are for those who want to From 321e75c5dc6d6abc76e2349000e93c809ddfcdab Mon Sep 17 00:00:00 2001 From: Matthieu Moy Date: Tue, 24 Apr 2012 09:50:02 +0200 Subject: [PATCH 3/7] t5528-push-default.sh: add helper functions Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- t/t5528-push-default.sh | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh index c334c51a07..99e5519263 100755 --- a/t/t5528-push-default.sh +++ b/t/t5528-push-default.sh @@ -13,16 +13,36 @@ test_expect_success 'setup bare remotes' ' git push parent2 HEAD ' +# $1 = local revision +# $2 = remote revision (tested to be equal to the local one) +check_pushed_commit () { + git log -1 --format='%h %s' "$1" >expect && + git --git-dir=repo1 log -1 --format='%h %s' "$2" >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 "$2" +} + +# $1 = push.default value +# check that push fails and does not modify any remote branch +test_push_failure () { + git --git-dir=repo1 log --no-walk --format='%h %s' --all >expect && + test_must_fail git -c push.default="$1" push && + git --git-dir=repo1 log --no-walk --format='%h %s' --all >actual && + test_cmp expect actual +} + test_expect_success '"upstream" pushes to configured upstream' ' git checkout master && test_config branch.master.remote parent1 && test_config branch.master.merge refs/heads/foo && - 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 + test_push_success upstream foo ' test_expect_success '"upstream" does not push on unconfigured remote' ' @@ -30,7 +50,7 @@ test_expect_success '"upstream" does not push on unconfigured remote' ' test_unconfig branch.master.remote && test_config push.default upstream && test_commit three && - test_must_fail git push + test_push_failure upstream ' test_expect_success '"upstream" does not push on unconfigured branch' ' @@ -39,7 +59,7 @@ test_expect_success '"upstream" does not push on unconfigured branch' ' test_unconfig branch.master.merge && test_config push.default upstream test_commit four && - test_must_fail git push + test_push_failure upstream ' test_expect_success '"upstream" does not push when remotes do not match' ' From b55e67752242b449a4577c05341fd6b2fea4d310 Mon Sep 17 00:00:00 2001 From: Matthieu Moy Date: Tue, 24 Apr 2012 09:50:03 +0200 Subject: [PATCH 4/7] 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 Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- Documentation/config.txt | 5 ++++- builtin/push.c | 47 ++++++++++++++++++++++++++++++++++++++-- cache.h | 1 + config.c | 6 +++-- t/t5528-push-default.sh | 44 +++++++++++++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 5 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 9617c53951..7f5ad1ce06 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1686,13 +1686,16 @@ push.default:: appropriate for pushing into a repository shared by multiple users, since locally stalled branches will attempt a non-fast forward push if other users updated the branch. 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. With this, `git push` will update the same remote ref as the one which is merged by `git pull`, making `push` and `pull` symmetrical. See "branch..merge" for how to configure the upstream branch. * `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. If you are working with other people to push into the same shared repository, you would want diff --git a/builtin/push.c b/builtin/push.c index 693671315e..08ccb891c6 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -76,7 +76,44 @@ 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 NORETURN int 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 *advice_maybe = ""; + const char *short_upstream = + skip_prefix(branch->merge[0]->src, "refs/heads/"); + + if (!short_upstream) + short_upstream = branch->merge[0]->src; + /* + * Don't show advice for people who explicitely set + * push.default. + */ + 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_upstream, + 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 +140,8 @@ 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 +158,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 5bf59ff5c3..b60d49085e 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 68d32940f3..bfe0c79aea 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 99e5519263..4736da8f36 100755 --- a/t/t5528-push-default.sh +++ b/t/t5528-push-default.sh @@ -71,4 +71,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 && + test_push_failure matching && + test_push_failure upstream +' + +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 && + test_push_failure upstream +' + +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 && + 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 From aecff47da67af69dae4e53749277ca2728e31e36 Mon Sep 17 00:00:00 2001 From: Clemens Buchacher Date: Tue, 24 Apr 2012 09:50:04 +0200 Subject: [PATCH 5/7] t5570: use explicit push refspec The default mode for push without arguments will change. Some warnings are about to be enabled for such use, which causes some t5570 tests to fail because they do not expect this output. Fix this by passing an explicit refspec to git push. To that end, change the calling conventions of test_remote_error in order to accomodate extra command arguments. Signed-off-by: Clemens Buchacher Signed-off-by: Junio C Hamano Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- t/t5570-git-daemon.sh | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh index 7cbc9994a3..a3a4e47e1d 100755 --- a/t/t5570-git-daemon.sh +++ b/t/t5570-git-daemon.sh @@ -103,14 +103,12 @@ test_remote_error() esac done - if test $# -ne 3 - then - error "invalid number of arguments" - fi - + msg=$1 + shift cmd=$1 - repo=$2 - msg=$3 + shift + repo=$1 + shift || error "invalid number of arguments" if test -x "$GIT_DAEMON_DOCUMENT_ROOT_PATH/$repo" then @@ -122,7 +120,7 @@ test_remote_error() fi fi - test_must_fail git "$cmd" "$GIT_DAEMON_URL/$repo" 2>output && + test_must_fail git "$cmd" "$GIT_DAEMON_URL/$repo" "$@" 2>output && echo "fatal: remote error: $msg: /$repo" >expect && test_cmp expect output ret=$? @@ -131,18 +129,18 @@ test_remote_error() } msg="access denied or repository not exported" -test_expect_success 'clone non-existent' "test_remote_error clone nowhere.git '$msg'" -test_expect_success 'push disabled' "test_remote_error push repo.git '$msg'" -test_expect_success 'read access denied' "test_remote_error -x fetch repo.git '$msg'" -test_expect_success 'not exported' "test_remote_error -n fetch repo.git '$msg'" +test_expect_success 'clone non-existent' "test_remote_error '$msg' clone nowhere.git " +test_expect_success 'push disabled' "test_remote_error '$msg' push repo.git master" +test_expect_success 'read access denied' "test_remote_error -x '$msg' fetch repo.git " +test_expect_success 'not exported' "test_remote_error -n '$msg' fetch repo.git " stop_git_daemon start_git_daemon --informative-errors -test_expect_success 'clone non-existent' "test_remote_error clone nowhere.git 'no such repository'" -test_expect_success 'push disabled' "test_remote_error push repo.git 'service not enabled'" -test_expect_success 'read access denied' "test_remote_error -x fetch repo.git 'no such repository'" -test_expect_success 'not exported' "test_remote_error -n fetch repo.git 'repository not exported'" +test_expect_success 'clone non-existent' "test_remote_error 'no such repository' clone nowhere.git " +test_expect_success 'push disabled' "test_remote_error 'service not enabled' push repo.git master" +test_expect_success 'read access denied' "test_remote_error -x 'no such repository' fetch repo.git " +test_expect_success 'not exported' "test_remote_error -n 'repository not exported' fetch repo.git " stop_git_daemon test_done From 67804c2731f9028ac246f7cea6fd9652386bab33 Mon Sep 17 00:00:00 2001 From: Matthieu Moy Date: Tue, 24 Apr 2012 09:50:05 +0200 Subject: [PATCH 6/7] push: document the future default change for push.default (matching -> simple) It is too early to start warning loudly about the future default change in favor of 'simple', since many users use different versions of Git, and would be harmed if we advised them to explicitely set 'push.default=simple' when using old versions of Git. Still, we want to document the upcomming change so that: * Users who may be affected by the change get one more chance to know it in advance. * We actually commit to changing the default, and avoid repeating past errors. Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- Documentation/config.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 7f5ad1ce06..f724fc64b2 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1685,10 +1685,14 @@ push.default:: shape and then push them out with a single command. It is not appropriate for pushing into a repository shared by multiple users, since locally stalled branches will attempt a non-fast forward push - if other users updated the branch. This is the default. + if other users updated the branch. + + + This is currently the default, but Git 2.0 will change the default + to `simple`. * `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. + option and is well-suited for beginners. It will become the default + in Git 2.0. * `upstream` - push the current branch to its upstream branch. With this, `git push` will update the same remote ref as the one which is merged by `git pull`, making `push` and `pull` symmetrical. From f4d80d2639f8ef55c99c3b035c0312969acc7f01 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 24 Apr 2012 12:21:12 -0700 Subject: [PATCH 7/7] push.default doc: explain simple after upstream As the "simple" mode is described in terms of what "upstream" does, swap the order of these two entries so that the reader sees "upstream" first and then reads "simple" with the knowledge of what "upstream" does. Signed-off-by: Junio C Hamano --- Documentation/config.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index f724fc64b2..6a4c130602 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1689,14 +1689,14 @@ push.default:: + This is currently the default, but Git 2.0 will change the default to `simple`. -* `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. It will become the default - in Git 2.0. * `upstream` - push the current branch to its upstream branch. With this, `git push` will update the same remote ref as the one which is merged by `git pull`, making `push` and `pull` symmetrical. See "branch..merge" for how to configure the upstream branch. +* `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. It will become the default + in Git 2.0. * `current` - push the current branch to a branch of the same name. + The `simple`, `current` and `upstream` modes are for those who want to