From 3cb8a5ff1756f11fe60b9c6ed3f9fe593e9c6d62 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Wed, 19 Jun 2013 16:41:39 +0530 Subject: [PATCH 1/5] t/t5528-push-default: remove redundant test_config lines The line test_config push.default upstream appears unnecessarily in two tests, as the final test_push_failure sets push.default before pushing anyway. Signed-off-by: Ramkumar Ramachandra Signed-off-by: Junio C Hamano --- t/t5528-push-default.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh index 4736da8f36..69ce6bfda8 100755 --- a/t/t5528-push-default.sh +++ b/t/t5528-push-default.sh @@ -48,7 +48,6 @@ test_expect_success '"upstream" pushes to configured upstream' ' test_expect_success '"upstream" does not push on unconfigured remote' ' git checkout master && test_unconfig branch.master.remote && - test_config push.default upstream && test_commit three && test_push_failure upstream ' @@ -57,7 +56,6 @@ test_expect_success '"upstream" does not push on unconfigured branch' ' git checkout master && test_config branch.master.remote parent1 && test_unconfig branch.master.merge && - test_config push.default upstream test_commit four && test_push_failure upstream ' From 87a70e4ce8bb3bdbb3048a5eb837f6b5b2eff8f9 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Wed, 19 Jun 2013 16:41:40 +0530 Subject: [PATCH 2/5] config doc: rewrite push.default section 4d35924e (Merge branch 'rr/triangle', 2013-04-07) introduced support for triangular workflows, but the push.default values still assume central workflows. Rewrite the descriptions of `nothing`, `current`, `upstream` and `matching` for greater clarity, and explicitly explain how they should behave in triangular workflows. Leave `simple` as it is for the moment, as we plan to change its meaning to accommodate triangular workflows in a later patch. Signed-off-by: Ramkumar Ramachandra Signed-off-by: Junio C Hamano --- Documentation/config.txt | 72 ++++++++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 28 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 7fd4035cb5..5d8ff1a8f5 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1826,39 +1826,55 @@ pull.twohead:: The default merge strategy to use when pulling a single branch. push.default:: - Defines the action `git push` should take if no refspec is given - on the command line, no refspec is configured in the remote, and - no refspec is implied by any of the options given on the command - line. Possible values are: + Defines the action `git push` should take if no refspec is + explicitly given. Different values are well-suited for + specific workflows; for instance, in a purely central workflow + (i.e. the fetch source is equal to the push destination), + `upstream` is probably what you want. Possible values are: + -- -* `nothing` - do not push anything. -* `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 currently the default, but Git 2.0 will change the default - to `simple`. -* `upstream` - push the current branch to its upstream branch - (`tracking` is a deprecated synonym for this). - 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. + +* `nothing` - do not push anything (error out) unless a refspec is + explicitly given. This is primarily meant for people who want to + avoid mistakes by always being explicit. + +* `current` - push the current branch to update a branch with the same + name on the receiving end. Works in both central and non-central + workflows. + +* `upstream` - push the current branch back to the branch whose + changes are usually integrated into the current branch (which is + called `@{upstream}`). This mode only makes sense if you are + pushing to the same repository you would normally pull from + (i.e. central workflow). + * `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. --- + option and is well-suited for beginners. + -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 -to use one of these. +This mode will become the default in Git 2.0. + +* `matching` - push all branches having the same name on both ends. + This makes the repository you are pushing to remember the set of + branches that will be pushed out (e.g. if you always push 'maint' + and 'master' there and no other branches, the repository you push + to will have these two branches, and your local 'maint' and + 'master' will be pushed there). ++ +To use this mode effectively, you have to make sure _all_ the +branches you would push out are ready to be pushed out before +running 'git push', as the whole point of this mode is to allow you +to push all of the branches in one go. If you usually finish work +on only one branch and push out the result, while other branches are +unfinished, this mode is not for you. Also this mode is not +suitable for pushing into a shared central repository, as other +people may add new branches there, or update the tip of existing +branches outside your control. ++ +This is currently the default, but Git 2.0 will change the default +to `simple`. + +-- rebase.stat:: Whether to show a diffstat of what changed upstream since the last From ed2b18292bfeedc98c9e2b6bd8a35d8001dab2fc Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Wed, 19 Jun 2013 16:41:41 +0530 Subject: [PATCH 3/5] push: change `simple` to accommodate triangular workflows When remote.pushdefault or branch..pushremote is set to a remote that is different from where you usually fetch from (i.e. a triangular workflow), master@{u} != origin, and push.default is set to `upstream` or `simple` would fail with this error: $ git push fatal: You are pushing to remote 'origin', which is not the upstream of your current branch 'master', without telling me what to push to update which remote branch. The very name of "upstream" indicates that it is only suitable for use in central workflows; let us not even attempt to give it a new meaning in triangular workflows, and error out as before. However, the `simple` does not have to share this error. It is poised to be the default for Git 2.0, and we would like it to do something sensible in triangular workflows. Redefine "simple" as "safer upstream" for centralized workflow as before, but work as "current" for triangular workflow. We may want to make it "safer current", but that is a separate issue. Reported-by: Leandro Lucarella Signed-off-by: Ramkumar Ramachandra Signed-off-by: Junio C Hamano --- Documentation/config.txt | 10 +++++++--- builtin/push.c | 43 +++++++++++++++++++++++++++++----------- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 5d8ff1a8f5..cae687072e 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1848,9 +1848,13 @@ push.default:: pushing to the same repository you would normally pull from (i.e. central workflow). -* `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. +* `simple` - in centralized workflow, work like `upstream` with an + added safety to refuse to push if the upstream branch's name is + different from the local one. ++ +When pushing to a remote that is different from the remote you normally +pull from, work as `current`. This is the safest option and is suited +for beginners. + This mode will become the default in Git 2.0. diff --git a/builtin/push.c b/builtin/push.c index 2d84d10720..a2580a94f5 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -120,10 +120,11 @@ static const char message_detached_head_die[] = "\n" " git push %s HEAD:\n"); -static void setup_push_upstream(struct remote *remote, int simple) +static void setup_push_upstream(struct remote *remote, struct branch *branch, + int triangular) { struct strbuf refspec = STRBUF_INIT; - struct branch *branch = branch_get(NULL); + if (!branch) die(_(message_detached_head_die), remote->name); if (!branch->merge_nr || !branch->merge || !branch->remote_name) @@ -137,18 +138,29 @@ static void setup_push_upstream(struct remote *remote, int simple) 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)) + if (triangular) 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)) - die_push_simple(branch, remote); + + if (push_default == PUSH_DEFAULT_SIMPLE) { + /* Additional safety */ + if (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); } +static void setup_push_current(struct remote *remote, struct branch *branch) +{ + if (!branch) + die(_(message_detached_head_die), remote->name); + add_refspec(branch->name); +} + static char warn_unspecified_push_default_msg[] = N_("push.default is unset; its implicit value is changing in\n" "Git 2.0 from 'matching' to 'simple'. To squelch this message\n" @@ -173,9 +185,16 @@ static void warn_unspecified_push_default_configuration(void) warning("%s\n", _(warn_unspecified_push_default_msg)); } +static int is_workflow_triangular(struct remote *remote) +{ + struct remote *fetch_remote = remote_get(NULL); + return (fetch_remote && fetch_remote != remote); +} + static void setup_default_push_refspecs(struct remote *remote) { - struct branch *branch; + struct branch *branch = branch_get(NULL); + int triangular = is_workflow_triangular(remote); switch (push_default) { default: @@ -188,18 +207,18 @@ static void setup_default_push_refspecs(struct remote *remote) break; case PUSH_DEFAULT_SIMPLE: - setup_push_upstream(remote, 1); + if (triangular) + setup_push_current(remote, branch); + else + setup_push_upstream(remote, branch, triangular); break; case PUSH_DEFAULT_UPSTREAM: - setup_push_upstream(remote, 0); + setup_push_upstream(remote, branch, triangular); break; case PUSH_DEFAULT_CURRENT: - branch = branch_get(NULL); - if (!branch) - die(_(message_detached_head_die), remote->name); - add_refspec(branch->name); + setup_push_current(remote, branch); break; case PUSH_DEFAULT_NOTHING: From 396243fa479cd27bef1ced2ea5a9e4cf20f85a4b Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Wed, 19 Jun 2013 16:41:43 +0530 Subject: [PATCH 4/5] t/t5528-push-default: generalize test_push_* The setup creates two bare repositories: repo1 and repo2, but test_push_commit() hard-codes checking in repo1 for the actual output. Generalize it and its caller, test_push_success(), to optionally accept a third argument to specify the name of the repository to check for actual output. We will use this in the next patch. Signed-off-by: Ramkumar Ramachandra Signed-off-by: Junio C Hamano --- t/t5528-push-default.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh index 69ce6bfda8..db58e7ffb3 100755 --- a/t/t5528-push-default.sh +++ b/t/t5528-push-default.sh @@ -15,17 +15,19 @@ test_expect_success 'setup bare remotes' ' # $1 = local revision # $2 = remote revision (tested to be equal to the local one) +# $3 = [optional] repo to check for actual output (repo1 by default) check_pushed_commit () { git log -1 --format='%h %s' "$1" >expect && - git --git-dir=repo1 log -1 --format='%h %s' "$2" >actual && + git --git-dir="${3:-repo1}" log -1 --format='%h %s' "$2" >actual && test_cmp expect actual } # $1 = push.default value # $2 = expected target branch for the push +# $3 = [optional] repo to check for actual output (repo1 by default) test_push_success () { git -c push.default="$1" push && - check_pushed_commit HEAD "$2" + check_pushed_commit HEAD "$2" "$3" } # $1 = push.default value From 6e1696b7c46f02310a882b4e761a29d3a0cf278f Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Wed, 19 Jun 2013 16:41:44 +0530 Subject: [PATCH 5/5] t/t5528-push-default: test pushdefault workflows Introduce test_pushdefault_workflows(), and test that all push.default modes work with central and triangular workflows as expected. Signed-off-by: Ramkumar Ramachandra Signed-off-by: Junio C Hamano --- t/t5528-push-default.sh | 57 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh index db58e7ffb3..6a5ac3add4 100755 --- a/t/t5528-push-default.sh +++ b/t/t5528-push-default.sh @@ -39,6 +39,26 @@ test_push_failure () { test_cmp expect actual } +# $1 = success or failure +# $2 = push.default value +# $3 = branch to check for actual output (master or foo) +# $4 = [optional] switch to triangular workflow +test_pushdefault_workflow () { + workflow=central + pushdefault=parent1 + if test -n "${4-}"; then + workflow=triangular + pushdefault=parent2 + fi + test_expect_success "push.default = $2 $1 in $workflow workflows" " + test_config branch.master.remote parent1 && + test_config branch.master.merge refs/heads/foo && + test_config remote.pushdefault $pushdefault && + test_commit commit-for-$2${4+-triangular} && + test_push_$1 $2 $3 ${4+repo2} + " +} + test_expect_success '"upstream" pushes to configured upstream' ' git checkout master && test_config branch.master.remote parent1 && @@ -115,4 +135,41 @@ test_expect_success 'push to existing branch, upstream configured with different test_cmp expect-other-name actual-other-name ' +# We are on 'master', which integrates with 'foo' from parent1 +# remote (set in test_pushdefault_workflow helper). Push to +# parent1 in centralized, and push to parent2 in triangular workflow. +# The parent1 repository has 'master' and 'foo' branches, while +# the parent2 repository has only 'master' branch. +# +# test_pushdefault_workflow() arguments: +# $1 = success or failure +# $2 = push.default value +# $3 = branch to check for actual output (master or foo) +# $4 = [optional] switch to triangular workflow + +# update parent1's master (which is not our upstream) +test_pushdefault_workflow success current master + +# update parent1's foo (which is our upstream) +test_pushdefault_workflow success upstream foo + +# upsream is foo which is not the name of the current branch +test_pushdefault_workflow failure simple master + +# master and foo are updated +test_pushdefault_workflow success matching master + +# master is updated +test_pushdefault_workflow success current master triangular + +# upstream mode cannot be used in triangular +test_pushdefault_workflow failure upstream foo triangular + +# in triangular, 'simple' works as 'current' and update the branch +# with the same name. +test_pushdefault_workflow success simple master triangular + +# master is updated (parent2 does not have foo) +test_pushdefault_workflow success matching master triangular + test_done