{"thread":{"id":"34219","subject":"[PATCH 0/6] push.default in the triangular world","startedAt":"2013-06-19T11:11:38Z","lastAt":"2013-06-20T22:05:52Z","messageCount":24,"participants":["Ramkumar Ramachandra","Junio C Hamano","Johan Herland","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"221368","messageId":"1371640304-26019-1-git-send-email-artagnon@gmail.com","threadId":"34219","inReplyTo":null,"subject":"[PATCH 0/6] push.default in the triangular world","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-19T11:11:38Z","receivedAt":"2013-06-19T11:11:38Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"[2/6] documents existing push.default modes properly, but doesn't\ntouch `simple`.  It incorporates feedback from Junio, Philip Oakley,\nMatthieu Moy.\n\n[3/6] gives `simple` an exciting new meaning.  I think it's an\nabsolutely fabulous default!  It's aimed at triangular people who\noccassionally need to do central stuff.\n\n[6/6] adds some new tests to illustrate how all these push.default\nmodes work in central and triangular workflows.\n\nThanks.\n\nRamkumar Ramachandra (6):\n  t/t5528-push-default: remove redundant test_config lines\n  config doc: rewrite push.default section\n  push: change `simple` to accommodate triangular workflows\n  push: remove dead code in setup_push_upstream()\n  t/t5528-push-default: generalize test_push_*\n  t/t5528-push-default: test pushdefault workflows\n\n Documentation/config.txt | 62 ++++++++++++++++++++++++++----------------------\n builtin/push.c           | 27 +++++++++++++++++----\n t/t5528-push-default.sh  | 46 +++++++++++++++++++++++++++++++----\n 3 files changed, 97 insertions(+), 38 deletions(-)\n\n-- \n1.8.3.1.454.g30263f3.dirty\n"},{"id":"221369","messageId":"1371640304-26019-2-git-send-email-artagnon@gmail.com","threadId":"34219","inReplyTo":"1371640304-26019-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/6] t/t5528-push-default: remove redundant test_config lines","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-19T11:11:39Z","receivedAt":"2013-06-19T11:11:39Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The line\n\n  test_config push.default upstream\n\nappears unnecessarily in two tests, as the final test_push_failure sets\npush.default before pushing anyway.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t5528-push-default.sh | 2 --\n 1 file changed, 2 deletions(-)\n\ndiff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\nindex 4736da8..69ce6bf 100755\n--- a/t/t5528-push-default.sh\n+++ b/t/t5528-push-default.sh\n@@ -48,7 +48,6 @@ test_expect_success '\"upstream\" pushes to configured upstream' '\n test_expect_success '\"upstream\" does not push on unconfigured remote' '\n \tgit checkout master &&\n \ttest_unconfig branch.master.remote &&\n-\ttest_config push.default upstream &&\n \ttest_commit three &&\n \ttest_push_failure upstream\n '\n@@ -57,7 +56,6 @@ test_expect_success '\"upstream\" does not push on unconfigured branch' '\n \tgit checkout master &&\n \ttest_config branch.master.remote parent1 &&\n \ttest_unconfig branch.master.merge &&\n-\ttest_config push.default upstream\n \ttest_commit four &&\n \ttest_push_failure upstream\n '\n-- \n1.8.3.1.454.g30263f3.dirty\n"},{"id":"221370","messageId":"1371640304-26019-3-git-send-email-artagnon@gmail.com","threadId":"34219","inReplyTo":"1371640304-26019-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/6] config doc: rewrite push.default section","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-19T11:11:40Z","receivedAt":"2013-06-19T11:11:40Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"4d3592 (Merge branch 'rr/triangle', 2013-04-07) introduced support for\ntriangular workflows in Git, but the push.default values still assume\ncentral workflows.  Rewrite the descriptions of `nothing`, `current`,\n`upstream` and `matching` for greater clarity, and explicitly explaining\nhow they behave in triangular workflows.  Leave `simple` as it is for\nthe moment, as we plan to change its meaning to accommodate triangular\nworkflows in a later patch.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Documentation/config.txt | 56 ++++++++++++++++++++++++++----------------------\n 1 file changed, 30 insertions(+), 26 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 7fd4035..9f04f74 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1826,39 +1826,43 @@ pull.twohead::\n \tThe default merge strategy to use when pulling a single branch.\n \n push.default::\n-\tDefines the action `git push` should take if no refspec is given\n-\ton the command line, no refspec is configured in the remote, and\n-\tno refspec is implied by any of the options given on the command\n-\tline. Possible values are:\n+\tDefines the action `git push` should take if no refspec is\n+\texplicitly given.  Different values are well-suited for\n+\tspecific workflows; for instance, in a purely central workflow\n+\t(i.e. the fetch source is equal to the push destination),\n+\t`upstream` is probably what you want.  Possible values are:\n +\n --\n-* `nothing` - do not push anything.\n-* `matching` - push all branches having the same name in both ends.\n-  This is for those who prepare all the branches into a publishable\n-  shape and then push them out with a single command.  It is not\n-  appropriate for pushing into a repository shared by multiple users,\n-  since locally stalled branches will attempt a non-fast forward push\n-  if other users updated the branch.\n-  +\n-  This is currently the default, but Git 2.0 will change the default\n-  to `simple`.\n-* `upstream` - push the current branch to its upstream branch\n-  (`tracking` is a deprecated synonym for this).\n-  With this, `git push` will update the same remote ref as the one which\n-  is merged by `git pull`, making `push` and `pull` symmetrical.\n-  See \"branch.<name>.merge\" for how to configure the upstream branch.\n+* `nothing` - do not push anything (error out) unless a refspec is\n+  explicitly given.  Very safe, but not very convenient.\n+\n+* `current` - push the current branch to update a branch with the same\n+  name on the receiving end.  Works in both central and non-central\n+  workflows.  Equivalent to pushing the refspec \"$branch\" ($branch is\n+  the name of the current branch).\n+\n+* `upstream` - push the current branch to a branch with the name\n+  branch.$branch.merge on the receiving end, and error out if the push\n+  destination is not the same as branch.$branch.remote.  The name\n+  \"upstream\" refers to \"@{u[pstream]}\" in linkgit:gitrevisions[7],\n+  which makes sense only if both branch.$branch.remote and\n+  branch.$branch.merge are set.  It makes sure that a `push` is\n+  symmetrical to `pull` in central workflows, and cannot be used in\n+  non-central workflows.\n+\n * `simple` - like `upstream`, but refuses to push if the upstream\n   branch's name is different from the local one. This is the safest\n   option and is well-suited for beginners. It will become the default\n   in Git 2.0.\n-* `current` - push the current branch to a branch of the same name.\n+\n+* `matching` - push all branches having the same name on both ends\n+  (essentially ignoring all newly created local branches).\n+  Well-suited for those who want to batch-update a specific set of\n+  branches they consistently work on.  Use with caution, especially\n+  when pushing with '--force'.  Equivalent to pushing the refspec \":\".\n+  This is currently the default, but Git 2.0 will change the default\n+  to `simple`.\n --\n-+\n-The `simple`, `current` and `upstream` modes are for those who want to\n-push out a single branch after finishing work, even when the other\n-branches are not yet ready to be pushed out. If you are working with\n-other people to push into the same shared repository, you would want\n-to use one of these.\n \n rebase.stat::\n \tWhether to show a diffstat of what changed upstream since the last\n-- \n1.8.3.1.454.g30263f3.dirty\n"},{"id":"221371","messageId":"1371640304-26019-4-git-send-email-artagnon@gmail.com","threadId":"34219","inReplyTo":"1371640304-26019-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/6] push: change `simple` to accommodate triangular workflows","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-19T11:11:41Z","receivedAt":"2013-06-19T11:11:41Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"When remote.pushdefault or branch.<name>.pushremote is set (a triangular\nworkflow feature), master@{u} != origin, and push.default is set to\n`upstream` or `simple`:\n\n  $ git push\n  fatal: You are pushing to remote 'origin', which is not the upstream of\n  your current branch 'master', without telling me what to push\n  to update which remote branch.\n\nUnfortunately, in the case of `upstream`, the very name indicates that\nit is only suitable for use in central workflows; let us not even\nattempt to give it a new meaning in triangular workflows, and error out\nas usual.  However, the `simple` does not have this problem: it is\npoised to be the default for Git 2.0, and we would definitely like it to\ndo something sensible in triangular workflows.\n\nDecouple `simple` from `upstream` completely, and change it to mean\n`current` with a safety feature: a `push` and `pull` should not be\nasymmetrical in the special case of central workflows.\n\nReported-by: Leandro Lucarella <leandro.lucarella@sociomantic.com>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Documentation/config.txt | 10 ++++++----\n builtin/push.c           | 21 ++++++++++++++++++++-\n t/t5528-push-default.sh  |  2 +-\n 3 files changed, 27 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 9f04f74..81628e8 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1850,10 +1850,12 @@ push.default::\n   symmetrical to `pull` in central workflows, and cannot be used in\n   non-central workflows.\n \n-* `simple` - like `upstream`, but refuses to push if the upstream\n-  branch's name is different from the local one. This is the safest\n-  option and is well-suited for beginners. It will become the default\n-  in Git 2.0.\n+* `simple` - a safer version of `current`; push the current branch to\n+  update a branch with the same name on the receiving end, with a\n+  safety feature: in central workflows, error out if\n+  branch.$branch.merge is set and not equal to $branch, to make sure\n+  that a `push` and `push` are never asymmetrical.  It will become the\n+  default in Git 2.0.\n \n * `matching` - push all branches having the same name on both ends\n   (essentially ignoring all newly created local branches).\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 2d84d10..d8d27d9 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -120,6 +120,25 @@ static const char message_detached_head_die[] =\n \t   \"\\n\"\n \t   \"    git push %s HEAD:<name-of-remote-branch>\\n\");\n \n+static void setup_push_simple(struct remote *remote)\n+{\n+\tstruct branch *branch = branch_get(NULL);\n+\tif (!branch)\n+\t\tdie(_(message_detached_head_die), remote->name);\n+\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n+\t\t/* No upstream configured */\n+\t\tgoto end;\n+\tif (branch->merge_nr != 1)\n+\t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n+\t\t    \"refusing to push.\"), branch->name);\n+\tif (!strcmp(branch->remote_name, remote->name) &&\n+\t\tstrcmp(branch->refname, branch->merge[0]->src))\n+\t\t/* Central workflow safety feature */\n+\t\tdie_push_simple(branch, remote);\n+end:\n+\tadd_refspec(branch->name);\n+}\n+\n static void setup_push_upstream(struct remote *remote, int simple)\n {\n \tstruct strbuf refspec = STRBUF_INIT;\n@@ -188,7 +207,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_SIMPLE:\n-\t\tsetup_push_upstream(remote, 1);\n+\t\tsetup_push_simple(remote);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_UPSTREAM:\ndiff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\nindex 69ce6bf..e54dd02 100755\n--- a/t/t5528-push-default.sh\n+++ b/t/t5528-push-default.sh\n@@ -85,7 +85,7 @@ test_expect_success 'push from/to new branch with current creates remote branch'\n test_expect_success 'push to existing branch, with no upstream configured' '\n \ttest_config branch.master.remote repo1 &&\n \tgit checkout master &&\n-\ttest_push_failure simple &&\n+\ttest_push_success simple master &&\n \ttest_push_failure upstream\n '\n \n-- \n1.8.3.1.454.g30263f3.dirty\n"},{"id":"221372","messageId":"1371640304-26019-5-git-send-email-artagnon@gmail.com","threadId":"34219","inReplyTo":"1371640304-26019-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 4/6] push: remove dead code in setup_push_upstream()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-19T11:11:42Z","receivedAt":"2013-06-19T11:11:42Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Now that simple has been decoupled from upstream in setup_push_simple(),\nremove the dead code in setup_push_upstream().\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n builtin/push.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex d8d27d9..783bacf 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -139,7 +139,7 @@ end:\n \tadd_refspec(branch->name);\n }\n \n-static void setup_push_upstream(struct remote *remote, int simple)\n+static void setup_push_upstream(struct remote *remote)\n {\n \tstruct strbuf refspec = STRBUF_INIT;\n \tstruct branch *branch = branch_get(NULL);\n@@ -161,8 +161,6 @@ static void setup_push_upstream(struct remote *remote, int simple)\n \t\t      \"your current branch '%s', without telling me what to push\\n\"\n \t\t      \"to update which remote branch.\"),\n \t\t    remote->name, branch->name);\n-\tif (simple && strcmp(branch->refname, branch->merge[0]->src))\n-\t\tdie_push_simple(branch, remote);\n \n \tstrbuf_addf(&refspec, \"%s:%s\", branch->name, branch->merge[0]->src);\n \tadd_refspec(refspec.buf);\n@@ -211,7 +209,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_UPSTREAM:\n-\t\tsetup_push_upstream(remote, 0);\n+\t\tsetup_push_upstream(remote);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_CURRENT:\n-- \n1.8.3.1.454.g30263f3.dirty\n"},{"id":"221373","messageId":"1371640304-26019-6-git-send-email-artagnon@gmail.com","threadId":"34219","inReplyTo":"1371640304-26019-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 5/6] t/t5528-push-default: generalize test_push_*","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-19T11:11:43Z","receivedAt":"2013-06-19T11:11:43Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The setup creates two bare repositories: repo1 and repo2, but\ntest_push_commit() hard-codes checking in repo1 for the actual output.\nGeneralize it and its caller, test_push_success(), to optionally accept\na third argument to specify the name of the repository to check for\nactual output.  We will use this in the next patch.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t5528-push-default.sh | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\nindex e54dd02..b599186 100755\n--- a/t/t5528-push-default.sh\n+++ b/t/t5528-push-default.sh\n@@ -15,17 +15,19 @@ test_expect_success 'setup bare remotes' '\n \n # $1 = local revision\n # $2 = remote revision (tested to be equal to the local one)\n+# $3 = [optional] repo to check for actual output (repo1 by default)\n check_pushed_commit () {\n \tgit log -1 --format='%h %s' \"$1\" >expect &&\n-\tgit --git-dir=repo1 log -1 --format='%h %s' \"$2\" >actual &&\n+\tgit --git-dir=\"${3:-repo1}\" log -1 --format='%h %s' \"$2\" >actual &&\n \ttest_cmp expect actual\n }\n \n # $1 = push.default value\n # $2 = expected target branch for the push\n+# $3 = [optional] repo to check for actual output (repo1 by default)\n test_push_success () {\n \tgit -c push.default=\"$1\" push &&\n-\tcheck_pushed_commit HEAD \"$2\"\n+\tcheck_pushed_commit HEAD \"$2\" \"$3\"\n }\n \n # $1 = push.default value\n-- \n1.8.3.1.454.g30263f3.dirty\n"},{"id":"221374","messageId":"1371640304-26019-7-git-send-email-artagnon@gmail.com","threadId":"34219","inReplyTo":"1371640304-26019-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 6/6] t/t5528-push-default: test pushdefault workflows","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-19T11:11:44Z","receivedAt":"2013-06-19T11:11:44Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Introduce test_pushdefault_workflows(), and test that all push.default\nmodes work with central and triangular workflows as expected.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t5528-push-default.sh | 36 ++++++++++++++++++++++++++++++++++++\n 1 file changed, 36 insertions(+)\n\ndiff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\nindex b599186..eabc09d 100755\n--- a/t/t5528-push-default.sh\n+++ b/t/t5528-push-default.sh\n@@ -39,6 +39,26 @@ test_push_failure () {\n \ttest_cmp expect actual\n }\n \n+# $1 = success or failure\n+# $2 = push.default value\n+# $3 = branch to check for actual output (master or foo)\n+# $4 = [optional] switch to triangular workflow\n+test_pushdefault_workflow () {\n+\tworkflow=central\n+\tpushdefault=parent1\n+\tif test -n \"${4-}\"; then\n+\t\tworkflow=triangular\n+\t\tpushdefault=parent2\n+\tfi\n+test_expect_success \"push.default = $2 $1 in $workflow workflows\" \"\n+\ttest_config branch.master.remote parent1 &&\n+\ttest_config branch.master.merge refs/heads/foo &&\n+\ttest_config remote.pushdefault $pushdefault &&\n+\ttest_commit commit-for-$2${4+-triangular} &&\n+\ttest_push_$1 $2 $3 ${4+repo2}\n+\"\n+}\n+\n test_expect_success '\"upstream\" pushes to configured upstream' '\n \tgit checkout master &&\n \ttest_config branch.master.remote parent1 &&\n@@ -115,4 +135,20 @@ test_expect_success 'push to existing branch, upstream configured with different\n \ttest_cmp expect-other-name actual-other-name\n '\n \n+## test_pushdefault_workflow() arguments:\n+# $1 = success or failure\n+# $2 = push.default value\n+# $3 = branch to check for actual output (master or foo)\n+# $4 = [optional] switch to triangular workflow\n+\n+test_pushdefault_workflow success current master  # breaks push/pull symmetry\n+test_pushdefault_workflow success upstream foo    # preserves push/pull symmetry\n+test_pushdefault_workflow failure simple master   # errors out on asymmetry\n+test_pushdefault_workflow success matching master # always works\n+\n+test_pushdefault_workflow success current master triangular  # always works\n+test_pushdefault_workflow failure upstream foo triangular    # always errors out\n+test_pushdefault_workflow success simple master triangular   # works like current\n+test_pushdefault_workflow success matching master triangular # always works\n+\n test_done\n-- \n1.8.3.1.454.g30263f3.dirty\n"},{"id":"221401","messageId":"7vbo71zyta.fsf@alter.siamese.dyndns.org","threadId":"34219","inReplyTo":"1371640304-26019-2-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 1/6] t/t5528-push-default: remove redundant test_config lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-19T19:26:57Z","receivedAt":"2013-06-19T19:26:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> The line\n>\n>   test_config push.default upstream\n>\n> appears unnecessarily in two tests, as the final test_push_failure sets\n> push.default before pushing anyway.\n>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n>  t/t5528-push-default.sh | 2 --\n>  1 file changed, 2 deletions(-)\n>\n> diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\n> index 4736da8..69ce6bf 100755\n> --- a/t/t5528-push-default.sh\n> +++ b/t/t5528-push-default.sh\n> @@ -48,7 +48,6 @@ test_expect_success '\"upstream\" pushes to configured upstream' '\n>  test_expect_success '\"upstream\" does not push on unconfigured remote' '\n>  \tgit checkout master &&\n>  \ttest_unconfig branch.master.remote &&\n> -\ttest_config push.default upstream &&\n>  \ttest_commit three &&\n>  \ttest_push_failure upstream\n>  '\n> @@ -57,7 +56,6 @@ test_expect_success '\"upstream\" does not push on unconfigured branch' '\n>  \tgit checkout master &&\n>  \ttest_config branch.master.remote parent1 &&\n>  \ttest_unconfig branch.master.merge &&\n> -\ttest_config push.default upstream\n>  \ttest_commit four &&\n>  \ttest_push_failure upstream\n>  '\n\nMakes sense, but I wonder if the only remaining test_config should\nalso be eradicated in the test after this one.\n\n    test_expect_success '\"upstream\" does not push when remotes do not match' '\n            git checkout master &&\n            test_config branch.master.remote parent1 &&\n            test_config branch.master.merge refs/heads/foo &&\n            test_config push.default upstream &&\n            test_commit five &&\n            test_must_fail git push parent2\n    '\n\nIt uses test_must_fail but that is only because test_push_failure is\nnot equipped to handle anything other than \"git push\" without\narguments.  If the helper were updated to do something like this:\n\n\tstrategy=$1; shift\n\t...\n\ttest_must_fail git -c push.default=\"$strategy\" push \"$@\"\n\nthen \"test_config push.default\" here can be removed and all the\ntests will end with test_push_success or test_push_failure.\n"},{"id":"221403","messageId":"7v7ghpzxhu.fsf@alter.siamese.dyndns.org","threadId":"34219","inReplyTo":"1371640304-26019-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 2/6] config doc: rewrite push.default section","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-19T19:55:25Z","receivedAt":"2013-06-19T19:55:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n>  push.default::\n> +\tDefines the action `git push` should take if no refspec is\n> +\texplicitly given.  Different values are well-suited for\n> +\tspecific workflows; for instance, in a purely central workflow\n> +\t(i.e. the fetch source is equal to the push destination),\n> +\t`upstream` is probably what you want.  Possible values are:\n>  +\n>  --\n> -* `nothing` - do not push anything.\n> -* `matching` - push all branches having the same name in both ends.\n> -  This is for those who prepare all the branches into a publishable\n> -  shape and then push them out with a single command.  It is not\n> -  appropriate for pushing into a repository shared by multiple users,\n> -  since locally stalled branches will attempt a non-fast forward push\n> -  if other users updated the branch.\n> -  +\n> -  This is currently the default, but Git 2.0 will change the default\n> -  to `simple`.\n> -* `upstream` - push the current branch to its upstream branch\n> -  (`tracking` is a deprecated synonym for this).\n> -  With this, `git push` will update the same remote ref as the one which\n> -  is merged by `git pull`, making `push` and `pull` symmetrical.\n> -  See \"branch.<name>.merge\" for how to configure the upstream branch.\n> +* `nothing` - do not push anything (error out) unless a refspec is\n> +  explicitly given.  Very safe, but not very convenient.\n\nDrop the last one, that does not add any value to the description.\n\n> +\n> +* `current` - push the current branch to update a branch with the same\n> +  name on the receiving end.  Works in both central and non-central\n> +  workflows.  Equivalent to pushing the refspec \"$branch\" ($branch is\n> +  the name of the current branch).\n\nI do not think this is a place to teach how various refspecs are\nspelled, but if you want to, then giving a complete command line\nwould be more appropriate.  Otherwise I'd suggest dropping\neverything after \"Equivalent to...\".\n\n> +* `upstream` - push the current branch to a branch with the name\n> +  branch.$branch.merge on the receiving end, and error out if the push\n> +  destination is not the same as branch.$branch.remote.\n\nWhile the above may be technically accurate, a mechanical definition\nhelp readers who read this for the first time may not be very\nhelpful.  What does the operation mean?\n\n\tpush the current branch back to the branch whose changes are\n\tusually integrated into the current branch with \"git pull\"\n\t(which is called @{upstream}).\n\n\tthis mode only makes sense if you are pushing to the same\n\trepository you would normally \"pull\" from (i.e. central\n\tworkflow).\n\n>  * `simple` - like `upstream`, but refuses to push if the upstream\n>    branch's name is different from the local one. This is the safest\n>    option and is well-suited for beginners. It will become the default\n>    in Git 2.0.\n\nOK.\n\n> +* `matching` - push all branches having the same name on both ends\n> +  (essentially ignoring all newly created local branches).\n\nThat misses the 'essense' of 'matching' by a wide margin, as \"newly\ncreated\" does not have anything to do with it.  Topic branches you\ndecided not to push long time ago and haven't pushed out so far will\nnot suddenly get pushed with \"git push\", either.\n\nThink of it this way.  Among your local branches, there are classes\nof branches that:\n\n (1) you would want to push now,\n (2) you would want to push when they are done, and\n (3) you would never want to push.\n\nOther \"push only single branch\" modes make this distinction by\nlimiting the push to only the first class (those you would want to\npush now), but it further limits that set to a single branch, which\nis the current one.  It is up to you to differentiate class 2+3\n(those you do not want to push now) from class 1 (the single one you\ndo want to push) and you indicate that choice by running 'git push'\nonly when you are on the branch you want to push out.\n\nThe 'matching' mode allows you to \"configure\" by having the other\nside remember which ones are in the class 1 (i.e. those that you\nwill push out when you say 'git push').  Instead of keeping a list\nof branches to be pushed in local configuration variable, you use\nthe set of branches your publishing point has.  It has a limitation\nthat you are not allowed to have the second category (i.e. at this\npoint of running 'git push' they are not ready to be pushed out),\nthough.\n\nPerhaps\n\n\t... on both ends.  This makes the repository you are pushing\n\tto remember the set of branches that will be pushed out\n\t(e.g. if you always push 'maint' and 'master' there and no\n\tother branches, the repository you push to will have these\n\ttwo branches, and your local 'maint' and 'master' will be\n\tpushed there).\n\n        To use this mode effectively, you have to make sure _all_\n\tthe branches you would push out are ready to be pushed out\n\tbefore running 'git push', as the whole point of this mode\n\tis to allow you to push all of the branches in one go.  If\n\tyou usually finish work on only one branch and push out the\n\tresult, while other branches are unfinished, this mode is\n\tnot for you.  Also this mode is not suitable for pushing\n\tinto a shared central repository, as other people may add\n\tnew branches there, or update the tip of existing branches\n\toutside your control.\n\nor something like that.\n"},{"id":"221404","messageId":"7v38sdzx8o.fsf@alter.siamese.dyndns.org","threadId":"34219","inReplyTo":"1371640304-26019-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-19T20:00:55Z","receivedAt":"2013-06-19T20:00:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> When remote.pushdefault or branch.<name>.pushremote is set (a triangular\n> workflow feature), master@{u} != origin, and push.default is set to\n> `upstream` or `simple`:\n>\n>   $ git push\n>   fatal: You are pushing to remote 'origin', which is not the upstream of\n>   your current branch 'master', without telling me what to push\n>   to update which remote branch.\n>\n> Unfortunately, in the case of `upstream`, the very name indicates that\n> it is only suitable for use in central workflows; let us not even\n> attempt to give it a new meaning in triangular workflows, and error out\n> as usual.\n\nSensible.\n\n> However, the `simple` does not have this problem: it is poised to\n> be the default for Git 2.0, and we would definitely like it to do\n> something sensible in triangular workflows.\n>\n> Decouple `simple` from `upstream` completely, and change it to mean\n> `current` with a safety feature: a `push` and `pull` should not be\n> asymmetrical in the special case of central workflows.\n\nDouble negation confused my parser.  'push' and 'pull' should be\nkept symmetrical in central workflows?\n\n> +* `simple` - a safer version of `current`; push the current branch to\n> +  update a branch with the same name on the receiving end, with a\n> +  safety feature: in central workflows, error out if\n> +  branch.$branch.merge is set and not equal to $branch,\n\nIf branch.$branch.merge is _not_ set, what happens in the current\ncode, and what should happen?\n\n> + to make sure\n> +  that a `push` and `push` are never asymmetrical.  It will become the\n> +  default in Git 2.0.\n\nDitto.\n\n>  * `matching` - push all branches having the same name on both ends\n>    (essentially ignoring all newly created local branches).\n> diff --git a/builtin/push.c b/builtin/push.c\n> index 2d84d10..d8d27d9 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -120,6 +120,25 @@ static const char message_detached_head_die[] =\n>  \t   \"\\n\"\n>  \t   \"    git push %s HEAD:<name-of-remote-branch>\\n\");\n>  \n> +static void setup_push_simple(struct remote *remote)\n> +{\n> +\tstruct branch *branch = branch_get(NULL);\n> +\tif (!branch)\n> +\t\tdie(_(message_detached_head_die), remote->name);\n\nOK.\n\n> +\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n> +\t\t/* No upstream configured */\n> +\t\tgoto end;\n\nWithout any configuration the current branch is pushed out, which\nloosens the safety we implemented in the current 'safer upstream'.\n\nI am not convinced this is a good change.  I am not convinced this is\na bad change, either, yet, but this loosening smells bad.\n\n> diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\n> index 69ce6bf..e54dd02 100755\n> --- a/t/t5528-push-default.sh\n> +++ b/t/t5528-push-default.sh\n> @@ -85,7 +85,7 @@ test_expect_success 'push from/to new branch with current creates remote branch'\n>  test_expect_success 'push to existing branch, with no upstream configured' '\n>  \ttest_config branch.master.remote repo1 &&\n>  \tgit checkout master &&\n> -\ttest_push_failure simple &&\n> +\ttest_push_success simple master &&\n>  \ttest_push_failure upstream\n>  '\n\nLikewise.\n"},{"id":"221405","messageId":"7vy5a5yino.fsf@alter.siamese.dyndns.org","threadId":"34219","inReplyTo":"1371640304-26019-5-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 4/6] push: remove dead code in setup_push_upstream()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-19T20:01:15Z","receivedAt":"2013-06-19T20:01:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Now that simple has been decoupled from upstream in setup_push_simple(),\n> remove the dead code in setup_push_upstream().\n\nGood.\n\n>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n>  builtin/push.c | 6 ++----\n>  1 file changed, 2 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/push.c b/builtin/push.c\n> index d8d27d9..783bacf 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -139,7 +139,7 @@ end:\n>  \tadd_refspec(branch->name);\n>  }\n>  \n> -static void setup_push_upstream(struct remote *remote, int simple)\n> +static void setup_push_upstream(struct remote *remote)\n>  {\n>  \tstruct strbuf refspec = STRBUF_INIT;\n>  \tstruct branch *branch = branch_get(NULL);\n> @@ -161,8 +161,6 @@ static void setup_push_upstream(struct remote *remote, int simple)\n>  \t\t      \"your current branch '%s', without telling me what to push\\n\"\n>  \t\t      \"to update which remote branch.\"),\n>  \t\t    remote->name, branch->name);\n> -\tif (simple && strcmp(branch->refname, branch->merge[0]->src))\n> -\t\tdie_push_simple(branch, remote);\n>  \n>  \tstrbuf_addf(&refspec, \"%s:%s\", branch->name, branch->merge[0]->src);\n>  \tadd_refspec(refspec.buf);\n> @@ -211,7 +209,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n>  \t\tbreak;\n>  \n>  \tcase PUSH_DEFAULT_UPSTREAM:\n> -\t\tsetup_push_upstream(remote, 0);\n> +\t\tsetup_push_upstream(remote);\n>  \t\tbreak;\n>  \n>  \tcase PUSH_DEFAULT_CURRENT:\n"},{"id":"221412","messageId":"7vd2rhydbh.fsf@alter.siamese.dyndns.org","threadId":"34219","inReplyTo":"1371640304-26019-6-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 5/6] t/t5528-push-default: generalize test_push_*","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-19T21:56:34Z","receivedAt":"2013-06-19T21:56:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> The setup creates two bare repositories: repo1 and repo2, but\n> test_push_commit() hard-codes checking in repo1 for the actual output.\n> Generalize it and its caller, test_push_success(), to optionally accept\n> a third argument to specify the name of the repository to check for\n> actual output.  We will use this in the next patch.\n>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n\nMakes sense.\n\n>  t/t5528-push-default.sh | 6 ++++--\n>  1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\n> index e54dd02..b599186 100755\n> --- a/t/t5528-push-default.sh\n> +++ b/t/t5528-push-default.sh\n> @@ -15,17 +15,19 @@ test_expect_success 'setup bare remotes' '\n>  \n>  # $1 = local revision\n>  # $2 = remote revision (tested to be equal to the local one)\n> +# $3 = [optional] repo to check for actual output (repo1 by default)\n>  check_pushed_commit () {\n>  \tgit log -1 --format='%h %s' \"$1\" >expect &&\n> -\tgit --git-dir=repo1 log -1 --format='%h %s' \"$2\" >actual &&\n> +\tgit --git-dir=\"${3:-repo1}\" log -1 --format='%h %s' \"$2\" >actual &&\n>  \ttest_cmp expect actual\n>  }\n>  \n>  # $1 = push.default value\n>  # $2 = expected target branch for the push\n> +# $3 = [optional] repo to check for actual output (repo1 by default)\n>  test_push_success () {\n>  \tgit -c push.default=\"$1\" push &&\n> -\tcheck_pushed_commit HEAD \"$2\"\n> +\tcheck_pushed_commit HEAD \"$2\" \"$3\"\n>  }\n>  \n>  # $1 = push.default value\n"},{"id":"221413","messageId":"7v8v25ycd4.fsf@alter.siamese.dyndns.org","threadId":"34219","inReplyTo":"1371640304-26019-7-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 6/6] t/t5528-push-default: test pushdefault workflows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-19T22:17:11Z","receivedAt":"2013-06-19T22:17:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Introduce test_pushdefault_workflows(), and test that all push.default\n> modes work with central and triangular workflows as expected.\n>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n>  t/t5528-push-default.sh | 36 ++++++++++++++++++++++++++++++++++++\n>  1 file changed, 36 insertions(+)\n>\n> diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\n> index b599186..eabc09d 100755\n> --- a/t/t5528-push-default.sh\n> +++ b/t/t5528-push-default.sh\n> @@ -39,6 +39,26 @@ test_push_failure () {\n>  \ttest_cmp expect actual\n>  }\n>  \n> +# $1 = success or failure\n> +# $2 = push.default value\n> +# $3 = branch to check for actual output (master or foo)\n> +# $4 = [optional] switch to triangular workflow\n> +test_pushdefault_workflow () {\n> +\tworkflow=central\n> +\tpushdefault=parent1\n> +\tif test -n \"${4-}\"; then\n> +\t\tworkflow=triangular\n> +\t\tpushdefault=parent2\n> +\tfi\n> +test_expect_success \"push.default = $2 $1 in $workflow workflows\" \"\n> +\ttest_config branch.master.remote parent1 &&\n> +\ttest_config branch.master.merge refs/heads/foo &&\n> +\ttest_config remote.pushdefault $pushdefault &&\n> +\ttest_commit commit-for-$2${4+-triangular} &&\n> +\ttest_push_$1 $2 $3 ${4+repo2}\n> +\"\n\nPlease indent the above; it is hard to spot where the actual test\nbegins and ends otherwise.\n\n> +}\n> +\n>  test_expect_success '\"upstream\" pushes to configured upstream' '\n>  \tgit checkout master &&\n>  \ttest_config branch.master.remote parent1 &&\n> @@ -115,4 +135,20 @@ test_expect_success 'push to existing branch, upstream configured with different\n>  \ttest_cmp expect-other-name actual-other-name\n>  '\n>  \n> +## test_pushdefault_workflow() arguments:\n> +# $1 = success or failure\n> +# $2 = push.default value\n> +# $3 = branch to check for actual output (master or foo)\n> +# $4 = [optional] switch to triangular workflow\n\nWhat happens in current/upstream/simple modes is affected by the\ncurrent branch, but it is unclear on what branch do these all run at\nthe first glance.  It would be helpful to add a comment here that\nsays what the starting condition is, something like:\n\n    # We are on 'master', which integrates with 'foo' from parent1\n    # Both parent1 and parent2 repositories have 'master' and 'foo'\n    # branches.\n\nhere.  It took me a while to tell what some of these comments wanted\nto say without such a comment.\n\n> +test_pushdefault_workflow success current master  # breaks push/pull symmetry\n\nOn 'master', push.default = current and we are in central workflow.\nBecause we do not use upstream but current, that deliberately breaks\nthe symmetry.  We make sure that 'master' is updated.\n\nLooks correct.\n\n> +test_pushdefault_workflow success upstream foo    # preserves push/pull symmetry\n\nWe use 'upstream'; current branch is set to integrate with 'foo' and\nthat is what is updated.  Looks good.\n\n> +test_pushdefault_workflow failure simple master   # errors out on asymmetry\n\nSimple is a safer form of 'current' in the central workflow.\nThe current branch is set to integrate with 'foo', which is\ndifferent name from the current branch'es name 'master', and the\npush fails.\n\nLooks correct, but do we want to make sure 'foo' is not updated\nhere as well?\n\n> +test_pushdefault_workflow success matching master # always works\n\nThis also should update 'foo'; do we want to make sure that happens\ntoo?\n\nOtherwise we won't be able to tell between matching and current.\n\n> +test_pushdefault_workflow success current master triangular  # always works\n\nOK.\n\n> +test_pushdefault_workflow failure upstream foo triangular    # always errors out\n\nOK.\n\n> +test_pushdefault_workflow success simple master triangular   # works like current\n\nOK, because in triangular it is like current.\n\n> +test_pushdefault_workflow success matching master triangular # always works\n\nOK.\n\nAs this step adds a useful helper function for testing, it appears\nto me that the helper can and should check the postcondition more\ncarefully, e.g. not just making sure what should be updated is\nupdated, but what should not be updated is not touched.\n\nOther than that, looks fine to me.\n\n> +\n>  test_done\n"},{"id":"221420","messageId":"7vk3lpwkt6.fsf@alter.siamese.dyndns.org","threadId":"34219","inReplyTo":"7v38sdzx8o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-20T02:57:41Z","receivedAt":"2013-06-20T02:57:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Without any configuration the current branch is pushed out, which\n> loosens the safety we implemented in the current 'safer upstream'.\n>\n> I am not convinced this is a good change.  I am not convinced this is\n> a bad change, either, yet, but this loosening smells bad.\n\nProvided that we would want to keep the \"Push the current one to the\nsame name but you have to have it set up as your integration source\"\nsafety for central workflow (which I am starting to think we\nshould), we would want something like this on top of your entire\nseries, I think.  The behaviour change can be seen in the revert of\none test you made to the test that expects \"simple\" to fail due to\nthe safety.\n\nThis patch is somewhat minimal in that it does not address other\nissues I raised in the review of the series; it only addresses the\n\"simple must be safe\" issue.\n\n builtin/push.c          | 60 +++++++++++++++++++++++++------------------------\n t/t5528-push-default.sh |  2 +-\n 2 files changed, 32 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 783bacf..84c4a90 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -120,29 +120,11 @@ static const char message_detached_head_die[] =\n \t   \"\\n\"\n \t   \"    git push %s HEAD:<name-of-remote-branch>\\n\");\n \n-static void setup_push_simple(struct remote *remote)\n-{\n-\tstruct branch *branch = branch_get(NULL);\n-\tif (!branch)\n-\t\tdie(_(message_detached_head_die), remote->name);\n-\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n-\t\t/* No upstream configured */\n-\t\tgoto end;\n-\tif (branch->merge_nr != 1)\n-\t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n-\t\t    \"refusing to push.\"), branch->name);\n-\tif (!strcmp(branch->remote_name, remote->name) &&\n-\t\tstrcmp(branch->refname, branch->merge[0]->src))\n-\t\t/* Central workflow safety feature */\n-\t\tdie_push_simple(branch, remote);\n-end:\n-\tadd_refspec(branch->name);\n-}\n-\n-static void setup_push_upstream(struct remote *remote)\n+static void setup_push_upstream(struct remote *remote, struct branch *branch,\n+\t\t\t\tint triangular)\n {\n \tstruct strbuf refspec = STRBUF_INIT;\n-\tstruct branch *branch = branch_get(NULL);\n+\n \tif (!branch)\n \t\tdie(_(message_detached_head_die), remote->name);\n \tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n@@ -156,16 +138,29 @@ static void setup_push_upstream(struct remote *remote)\n \tif (branch->merge_nr != 1)\n \t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n \t\t    \"refusing to push.\"), branch->name);\n-\tif (strcmp(branch->remote_name, remote->name))\n+\tif (triangular)\n \t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n \t\t      \"your current branch '%s', without telling me what to push\\n\"\n \t\t      \"to update which remote branch.\"),\n \t\t    remote->name, branch->name);\n \n+\tif (push_default == PUSH_DEFAULT_SIMPLE) {\n+\t\t/* Additional safety */\n+\t\tif (strcmp(branch->refname, branch->merge[0]->src))\n+\t\t\tdie_push_simple(branch, remote);\n+\t}\n+\n \tstrbuf_addf(&refspec, \"%s:%s\", branch->name, branch->merge[0]->src);\n \tadd_refspec(refspec.buf);\n }\n \n+static void setup_push_current(struct remote *remote, struct branch *branch)\n+{\n+\tif (!branch)\n+\t\tdie(_(message_detached_head_die), remote->name);\n+\tadd_refspec(branch->name);\n+}\n+\n static char warn_unspecified_push_default_msg[] =\n N_(\"push.default is unset; its implicit value is changing in\\n\"\n    \"Git 2.0 from 'matching' to 'simple'. To squelch this message\\n\"\n@@ -190,9 +185,16 @@ static void warn_unspecified_push_default_configuration(void)\n \twarning(\"%s\\n\", _(warn_unspecified_push_default_msg));\n }\n \n+static int is_workflow_triagular(struct remote *remote)\n+{\n+\tstruct remote *fetch_remote = remote_get(NULL);\n+\treturn (fetch_remote != remote);\n+}\n+\n static void setup_default_push_refspecs(struct remote *remote)\n {\n-\tstruct branch *branch;\n+\tstruct branch *branch = branch_get(NULL);\n+\tint triangular = is_workflow_triagular(remote);\n \n \tswitch (push_default) {\n \tdefault:\n@@ -205,18 +207,18 @@ static void setup_default_push_refspecs(struct remote *remote)\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_SIMPLE:\n-\t\tsetup_push_simple(remote);\n+\t\tif (triangular)\n+\t\t\tsetup_push_current(remote, branch);\n+\t\telse\n+\t\t\tsetup_push_upstream(remote, branch, triangular);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_UPSTREAM:\n-\t\tsetup_push_upstream(remote);\n+\t\tsetup_push_upstream(remote, branch, triangular);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_CURRENT:\n-\t\tbranch = branch_get(NULL);\n-\t\tif (!branch)\n-\t\t\tdie(_(message_detached_head_die), remote->name);\n-\t\tadd_refspec(branch->name);\n+\t\tsetup_push_current(remote, branch);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_NOTHING:\ndiff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\nindex eabc09d..36f5a63 100755\n--- a/t/t5528-push-default.sh\n+++ b/t/t5528-push-default.sh\n@@ -107,7 +107,7 @@ test_expect_success 'push from/to new branch with current creates remote branch'\n test_expect_success 'push to existing branch, with no upstream configured' '\n \ttest_config branch.master.remote repo1 &&\n \tgit checkout master &&\n-\ttest_push_success simple master &&\n+\ttest_push_failure simple &&\n \ttest_push_failure upstream\n '\n \n"},{"id":"221422","messageId":"7vhagtwjfa.fsf@alter.siamese.dyndns.org","threadId":"34219","inReplyTo":"7v7ghpzxhu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/6] config doc: rewrite push.default section","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-20T03:27:37Z","receivedAt":"2013-06-20T03:27:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> or something like that.\n\nJust for a completeness, in a patch form:\n\n Documentation/config.txt | 66 ++++++++++++++++++++++++++++--------------------\n 1 file changed, 39 insertions(+), 27 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 81628e8..4b7fc54 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1833,37 +1833,49 @@ push.default::\n \t`upstream` is probably what you want.  Possible values are:\n +\n --\n+\n * `nothing` - do not push anything (error out) unless a refspec is\n-  explicitly given.  Very safe, but not very convenient.\n+  explicitly given. This is primarily meant for people who want to\n+  avoid mistakes by always being explicit.\n \n * `current` - push the current branch to update a branch with the same\n   name on the receiving end.  Works in both central and non-central\n-  workflows.  Equivalent to pushing the refspec \"$branch\" ($branch is\n-  the name of the current branch).\n-\n-* `upstream` - push the current branch to a branch with the name\n-  branch.$branch.merge on the receiving end, and error out if the push\n-  destination is not the same as branch.$branch.remote.  The name\n-  \"upstream\" refers to \"@{u[pstream]}\" in linkgit:gitrevisions[7],\n-  which makes sense only if both branch.$branch.remote and\n-  branch.$branch.merge are set.  It makes sure that a `push` is\n-  symmetrical to `pull` in central workflows, and cannot be used in\n-  non-central workflows.\n-\n-* `simple` - a safer version of `current`; push the current branch to\n-  update a branch with the same name on the receiving end, with a\n-  safety feature: in central workflows, error out if\n-  branch.$branch.merge is set and not equal to $branch, to make sure\n-  that a `push` and `push` are never asymmetrical.  It will become the\n-  default in Git 2.0.\n-\n-* `matching` - push all branches having the same name on both ends\n-  (essentially ignoring all newly created local branches).\n-  Well-suited for those who want to batch-update a specific set of\n-  branches they consistently work on.  Use with caution, especially\n-  when pushing with '--force'.  Equivalent to pushing the refspec \":\".\n-  This is currently the default, but Git 2.0 will change the default\n-  to `simple`.\n+  workflows.\n+\n+* `upstream` - push the current branch back to the branch whose\n+  changes are usually integrated into the current branch (which is\n+  called `@{upstream}`).  This mode only makes sense if you are\n+  pushing to the same repository you would normally pull from\n+  (i.e. central workflow).\n+\n+* `simple` - a safer version of `current`; push the current branch\n+  to update a branch with the same name on the receiving end, with a\n+  safety feature: in central workflows, error out if your current\n+  branch is not set to integrate with the branch with the same name,\n+  to ensure that a `push` and a `push` are symmetrical.\n++\n+This mode will become the default in Git 2.0.\n+\n+* `matching` - push all branches having the same name on both ends.\n+  This makes the repository you are pushing to remember the set of\n+  branches that will be pushed out (e.g. if you always push 'maint'\n+  and 'master' there and no other branches, the repository you push\n+  to will have these two branches, and your local 'maint' and\n+  'master' will be pushed there).\n++\n+To use this mode effectively, you have to make sure _all_ the\n+branches you would push out are ready to be pushed out before\n+running 'git push', as the whole point of this mode is to allow you\n+to push all of the branches in one go.  If you usually finish work\n+on only one branch and push out the result, while other branches are\n+unfinished, this mode is not for you.  Also this mode is not\n+suitable for pushing into a shared central repository, as other\n+people may add new branches there, or update the tip of existing\n+branches outside your control.\n++\n+This is currently the default, but Git 2.0 will change the default\n+to `simple`.\n+\n --\n \n rebase.stat::\n-- \n1.8.3.1-674-gb27e881\n"},{"id":"221429","messageId":"CALKQrgf=gUTv22m_eBPVy+cQEfJn0w_G097a8nnJB1zJ48Lmpg@mail.gmail.com","threadId":"34219","inReplyTo":"7vhagtwjfa.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/6] config doc: rewrite push.default section","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-06-20T07:35:47Z","receivedAt":"2013-06-20T07:35:47Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Thu, Jun 20, 2013 at 5:27 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> +* `simple` - a safer version of `current`; push the current branch\n> +  to update a branch with the same name on the receiving end, with a\n> +  safety feature: in central workflows, error out if your current\n> +  branch is not set to integrate with the branch with the same name,\n> +  to ensure that a `push` and a `push` are symmetrical.\n\nto ensure that a `push` and a `pull` are symmetrical.\n\nOtherwise, this looks good to me.\n\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"221459","messageId":"CALkWK0=v25wC1r8ScUkKDhFjctZCDLJtpDx2g2avyYgJVmZCWg@mail.gmail.com","threadId":"34219","inReplyTo":"7vk3lpwkt6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-20T10:09:38Z","receivedAt":"2013-06-20T10:09:38Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n>> Decouple `simple` from `upstream` completely, and change it to mean\n>> `current` with a safety feature: a `push` and `pull` should not be\n>> asymmetrical in the special case of central workflows.\n>\n> Double negation confused my parser.  'push' and 'pull' should be\n> kept symmetrical in central workflows?\n\nThey're not the same thing.  It is very much intentional and intended:\nthe safety net is not to \"ensure that the push and pull are\nsymmetrical\" (i.e. among other things, error out if\nbranch.$branch.merge is unset), but rather \"ensure that the push and\npull are never asymmetrical\".\n\n>> Without any configuration the current branch is pushed out, which\n>> loosens the safety we implemented in the current 'safer upstream'.\n>>\n>> I am not convinced this is a good change.  I am not convinced this is\n>> a bad change, either, yet, but this loosening smells bad.\n>\n> Provided that we would want to keep the \"Push the current one to the\n> same name but you have to have it set up as your integration source\"\n> safety for central workflow (which I am starting to think we\n> should), we would want something like this on top of your entire\n> series, I think.  The behaviour change can be seen in the revert of\n> one test you made to the test that expects \"simple\" to fail due to\n> the safety.\n\nNow I'd like to question what you are labelling as \"safety\".  What is\nthe consequence of erroring out when branch.$branch.merge is unset\nwhen pushing using `upstream`?  For me, it only means additional\ninconvenience: any new branches I create can't be pushed out without\nexplicitly setting branch.$branch.merge to an \"invalid\" value.  What\nis invalid about it?  The fact that it doesn't exist, @{u} still\ndoesn't resolve, and git branch -u doesn't work.  Hell, even git push\n-u doesn't work!  So, what is this huge \"safety\" that can justify\ninconveniencing me like this?  By making sure that\nbranch.$branch.merge is set, my prompt responds immediately to\ndivergence, and this is awesome.  Predictably, I use git push -u when\nI push out a new branch with `current`.  So, unless you have a damn\ngood reason to inconvenience me in the name of safety,\nbranch.$branch.merge should default to refs/heads/$branch, unless set\nexplicitly.\n\nI didn't want to contaminate this series with an unrelated improvement\nto `upstream`, which is why you don't see the change here: it is\northogonal to designing a good `simple`, and I only brought it up to\nquestion the \"safety\" you're carrying over to `simple`; what\nobligation does `simple` have to carry over this \"feature\"?  I've made\nit clear that I want a clean break from `upstream`, and I find your\nproposal is very inelegant: `simple` has two modes of operation; when\nbranch.$branch.remote is equal to $pushremote, branch.$branch.merge\nmust be set and equal to $branch (the `upstream` mode); when\nbranch.$branch.remote is unequal to $pushremote, don't care about\nwhether branch.$branch.merge is set (the `current` mode).  My proposal\nis much smoother than this \"modal\" operation, no?\n"},{"id":"221506","messageId":"7vd2rgtwl3.fsf@alter.siamese.dyndns.org","threadId":"34219","inReplyTo":"CALkWK0=v25wC1r8ScUkKDhFjctZCDLJtpDx2g2avyYgJVmZCWg@mail.gmail.com","subject":"Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-20T19:23:52Z","receivedAt":"2013-06-20T19:23:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Junio C Hamano wrote:\n>>> Decouple `simple` from `upstream` completely, and change it to mean\n>>> `current` with a safety feature: a `push` and `pull` should not be\n>>> asymmetrical in the special case of central workflows.\n>>\n>> Double negation confused my parser.  'push' and 'pull' should be\n>> kept symmetrical in central workflows?\n>\n> They're not the same thing.  It is very much intentional and intended:\n> the safety net is not to \"ensure that the push and pull are\n> symmetrical\" (i.e. among other things, error out if\n> branch.$branch.merge is unset), but rather \"ensure that the push and\n> pull are never asymmetrical\".\n\nHmmmm....\n\n    not to \"ensure that the push and pull are symmetrical\"\n    rather \"ensure that the push and pull are never asymmetrical\".\n\nThey still talk the same thing to me.  What am I missing?\n\nAm I being clueless, or is there something else going on?\n\nYour \"among other things\", after reading it three times,\nunfortunately did not help clarify anything to me, so perhaps\nsomebody other than me (or you for that matter) who is more clueful\ncan help find a different way to explain the difference you are\ntrying to express to me.\n\nHelp, anybody?\n\n>> Provided that we would want to keep the \"Push the current one to the\n>> same name but you have to have it set up as your integration source\"\n>> safety for central workflow (which I am starting to think we\n>> should), we would want something like this on top of your entire\n>> series, I think.  The behaviour change can be seen in the revert of\n>> one test you made to the test that expects \"simple\" to fail due to\n>> the safety.\n>\n> Now I'd like to question what you are labelling as \"safety\".  What is\n> the consequence of erroring out when branch.$branch.merge is unset\n> when pushing using `upstream`?\n\nNothing noteworthy.\n\nI wasn't suggesting to change what `upstream` does at all.\n\nIf you do not have a `upstream` configured, we do not know what\nbranch you are integrating with, and the operation has failed in the\ncode even before we started adding triangular.\n\nI do not see a reason to change that in the triangular world;\n`upstream` is about the central workflow as you originally wrote in\nthe \"config.txt\" patch in this series.\n\nThe name of the branch the repository you fetch from and integrate\nwith does not have anything to do with the name you want to push\nyour derived work as to a different repository\n\nI thought we already discussed and agreed on this point.\n\n  http://thread.gmane.org/gmane.comp.version-control.git/227028/focus=227313\n\nThe conclusion is that using push.default=`upstream` is meaningless\nwhen you are using a triangular workflow.  If you are using a\ncentralized workflow, and if a branch does not have branch.*.merge\nconfigured, we do not know to which branch you are pushing it back,\nso we error out.\n\nWhat I am changing from the patch you posted with the \"how about\nthis on top\" patch back to the current behaviour is what 'simple'\ndoes for centralized workflow.\n\n> I didn't want to contaminate this series with an unrelated improvement\n> to `upstream`\n\nI think we share that, and it is not just `upstream`, but also\n`simple` when it is applied to folks who employ a centralized\nworkflow.  The safety we need to keep is the one we have had since\nday one of introducing 'simple' for them.\n\nWhen you are doing a centralized workflow, 'simple' was defined to\nbe 'upstream' with added restriction that the branch at the remote\nyou integrate with must have the same name as the current branch you\nare pushing, i.e.  in\n\n    [branch \"frotz\"]\n\tmerge = refs/heads/$branch\n\nthe value of $branch must be 'frotz'; otherwise 'simple' errors out.\n\n  http://thread.gmane.org/gmane.comp.version-control.git/194175/focus=196199\n\nNow, no existing series has casted in stone the best behaviour for\n`simple` in a triangular workflow.  With this series (and also with\nmy fixup patch I sent last night), it is defined to act as `current`,\nbut it may need a bit more safety to help new users avoid pushing\nbranches they did not intend to (perhaps pushing out `current` only\nwhen the branch with the same name already exists at the destination?\nI dunno).\n"},{"id":"221518","messageId":"E1FE663A82E6499AA9B598B4F60A5CE6@PhilipOakley","threadId":"34219","inReplyTo":"7vd2rgtwl3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2013-06-20T20:49:57Z","receivedAt":"2013-06-20T20:49:57Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" <gitster@pobox.com>\nSent: Thursday, June 20, 2013 8:23 PM\n> Ramkumar Ramachandra <artagnon@gmail.com> writes:\n>\n>> Junio C Hamano wrote:\n>>>> Decouple `simple` from `upstream` completely, and change it to mean\n>>>> `current` with a safety feature: a `push` and `pull` should not be\n>>>> asymmetrical in the special case of central workflows.\n>>>\n>>> Double negation confused my parser.  'push' and 'pull' should be\n>>> kept symmetrical in central workflows?\n>>\n>> They're not the same thing.  It is very much intentional and \n>> intended:\n>> the safety net is not to \"ensure that the push and pull are\n>> symmetrical\" (i.e. among other things, error out if\n>> branch.$branch.merge is unset), but rather \"ensure that the push and\n>> pull are never asymmetrical\".\n>\n> Hmmmm....\n>\n>    not to \"ensure that the push and pull are symmetrical\"\n>    rather \"ensure that the push and pull are never asymmetrical\".\n>\n> They still talk the same thing to me.  What am I missing?\n>\n> Am I being clueless, or is there something else going on?\n\nI think it is a case of the user having explicitly set push=Africa and \npull=Europe which can't be a setting for simple symmetry.\nBut then again I haven't been following the fine details. (that is there \nare some defaults that allow stuff to be half set)\n\n>\n> Your \"among other things\", after reading it three times,\n> unfortunately did not help clarify anything to me, so perhaps\n> somebody other than me (or you for that matter) who is more clueful\n> can help find a different way to explain the difference you are\n> trying to express to me.\n>\n> Help, anybody?\n>\n\nPhilip\n\n[...] \n"},{"id":"221520","messageId":"7vfvwcqyug.fsf@alter.siamese.dyndns.org","threadId":"34219","inReplyTo":"E1FE663A82E6499AA9B598B4F60A5CE6@PhilipOakley","subject":"Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-20T21:03:19Z","receivedAt":"2013-06-20T21:03:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" <philipoakley@iee.org> writes:\n\n> From: \"Junio C Hamano\" <gitster@pobox.com>\n> Sent: Thursday, June 20, 2013 8:23 PM\n>> Ramkumar Ramachandra <artagnon@gmail.com> writes:\n>>\n>>> Junio C Hamano wrote:\n>>>> Double negation confused my parser.  'push' and 'pull' should be\n>>>> kept symmetrical in central workflows?\n>>>\n>>> They're not the same thing.  It is very much intentional and\n>>> intended:\n>>> the safety net is not to \"ensure that the push and pull are\n>>> symmetrical\" (i.e. among other things, error out if\n>>> branch.$branch.merge is unset), but rather \"ensure that the push and\n>>> pull are never asymmetrical\".\n>>\n>> Hmmmm....\n>>\n>>    not to \"ensure that the push and pull are symmetrical\"\n>>    rather \"ensure that the push and pull are never asymmetrical\".\n>>\n>> They still talk the same thing to me.  What am I missing?\n>>\n>> Am I being clueless, or is there something else going on?\n>\n> I think it is a case of the user having explicitly set push=Africa and\n> pull=Europe which can't be a setting for simple symmetry.\n\nYeah but then that is not a discussion about central workflow.\n\nI can understand \"In a central workflow push and pull should be\nsymmetrical.\"  I can also, with a bit of double-negation brain\ntwisting, understand \"In a central workflow, push and pull should\nnot be asymmetrical.\"\n\nBut when I suggest to avoid double-negation, I was told that these\ntwo statements mean different things, and the original should not be\nrewritten to avoid double-negation, which is where my brain stopped\nand asked for help.\n"},{"id":"221522","messageId":"CALkWK0mTthYyHRkxqK2Z3uP3uUmtQADyZt7CERY_40iQSQyzAA@mail.gmail.com","threadId":"34219","inReplyTo":"7vd2rgtwl3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-20T21:22:28Z","receivedAt":"2013-06-20T21:22:28Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n>> They're not the same thing.  It is very much intentional and intended:\n>> the safety net is not to \"ensure that the push and pull are\n>> symmetrical\" (i.e. among other things, error out if\n>> branch.$branch.merge is unset), but rather \"ensure that the push and\n>> pull are never asymmetrical\".\n>\n>     not to \"ensure that the push and pull are symmetrical\"\n>     rather \"ensure that the push and pull are never asymmetrical\".\n>\n> They still talk the same thing to me.  What am I missing?\n\nNever mind the wording then.  I am proposing preventing asymmetry by\nexplicitly disallowing a push when $branch is different from\nbranch.$branch.merge, instead of shutting down immediately when\nbranch.$branch.merge is unset.\n\n>> Now I'd like to question what you are labelling as \"safety\".  What is\n>> the consequence of erroring out when branch.$branch.merge is unset\n>> when pushing using `upstream`?\n>\n> Nothing noteworthy.\n>\n> I wasn't suggesting to change what `upstream` does at all.\n\nNo, but I did.  I just argued for a sane default for\nbranch.$branch.merge (the part you snipped out).\n\n> The conclusion is that using push.default=`upstream` is meaningless\n> when you are using a triangular workflow.\n\nYes, and I agreed with that.\n\n> If you are using a\n> centralized workflow, and if a branch does not have branch.*.merge\n> configured, we do not know to which branch you are pushing it back,\n> so we error out.\n\nAnd I said: have a sane default.\n\n> What I am changing from the patch you posted with the \"how about\n> this on top\" patch back to the current behaviour is what 'simple'\n> does for centralized workflow.\n\nYes, I know.  I read the patch.\n\n> When you are doing a centralized workflow, 'simple' was defined\n\nAgain, I'm well aware what it _was_ defined as.  Was it not clear that\nI argued for a change from the very first email where I posted the\npatch and changed a test?  Do you have a counter-argument, or is it\nsimply that we must respect its historical meaning?\n\n> Now, no existing series has casted in stone the best behaviour for\n> `simple` in a triangular workflow.  With this series (and also with\n> my fixup patch I sent last night), it is defined to act as `current`,\n> but it may need a bit more safety to help new users avoid pushing\n> branches they did not intend to (perhaps pushing out `current` only\n> when the branch with the same name already exists at the destination?\n> I dunno).\n\nI see no reason to plan safety features in advance, especially since\nwe have neither branch.$branch.push nor @{push} yet.\n"},{"id":"221524","messageId":"7vbo70qx26.fsf@alter.siamese.dyndns.org","threadId":"34219","inReplyTo":"7vk3lpwkt6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-20T21:41:53Z","receivedAt":"2013-06-20T21:41:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Without any configuration the current branch is pushed out, which\n>> loosens the safety we implemented in the current 'safer upstream'.\n>>\n>> I am not convinced this is a good change.  I am not convinced this is\n>> a bad change, either, yet, but this loosening smells bad.\n>\n> Provided that we would want to keep the \"Push the current one to the\n> same name but you have to have it set up as your integration source\"\n> safety for central workflow (which I am starting to think we\n> should), we would want something like this on top of your entire\n> series, I think.  The behaviour change can be seen in the revert of\n> one test you made to the test that expects \"simple\" to fail due to\n> the safety.\n\nAnd with the small refactoring of setup_default_push_refspecs (the\nimportant part being that we grab branch in this function, not in\nits helper functions per push.default mode), branch.*.push can fall\nout rather naturally, like this patch on top.\n\n\nA footnote unrelated to this patch.\n\nThe function is_workflow_triangular() in the \"how about this\" patch\nneeds to be tweaked from the version I am responding to, in order to\ntake the case where fetch-remote is not defined into account, i.e.\n\n    static int is_workflow_triagular(struct remote *remote)\n    {\n            struct remote *fetch_remote = remote_get(NULL);\n            return (fetch_remote && fetch_remote != remote);\n    }\n\n\n\n\n builtin/push.c | 18 +++++++++++++++++-\n remote.c       |  5 +++++\n remote.h       |  2 ++\n 3 files changed, 24 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex f6c8047..a140b8e 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -185,6 +185,15 @@ static void warn_unspecified_push_default_configuration(void)\n \twarning(\"%s\\n\", _(warn_unspecified_push_default_msg));\n }\n \n+static void setup_per_branch_push(struct branch *branch)\n+{\n+\tstruct strbuf refspec = STRBUF_INIT;\n+\n+\tstrbuf_addf(&refspec, \"%s:%s\",\n+\t\t    branch->name, branch->push_name);\n+\tadd_refspec(refspec.buf);\n+}\n+\n static int is_workflow_triagular(struct remote *remote)\n {\n \tstruct remote *fetch_remote = remote_get(NULL);\n@@ -194,7 +203,14 @@ static int is_workflow_triagular(struct remote *remote)\n static void setup_default_push_refspecs(struct remote *remote)\n {\n \tstruct branch *branch = branch_get(NULL);\n-\tint triangular = is_workflow_triagular(remote);\n+\tint triangular;\n+\n+\tif (branch->push_name) {\n+\t\tsetup_per_branch_push(branch);\n+\t\treturn;\n+\t}\n+\n+\ttriangular = is_workflow_triagular(remote);\n \n \tswitch (push_default) {\n \tdefault:\ndiff --git a/remote.c b/remote.c\nindex e71f66d..e033fef 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -372,6 +372,11 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\t\tif (!value)\n \t\t\t\treturn config_error_nonbool(key);\n \t\t\tadd_merge(branch, xstrdup(value));\n+\t\t} else if (!strcmp(subkey, \".push\")) {\n+\t\t\tif (!value)\n+\t\t\t\treturn config_error_nonbool(key);\n+\t\t\tfree(branch->push_name);\n+\t\t\tbranch->push_name = xstrdup(value);\n \t\t}\n \t\treturn 0;\n \t}\ndiff --git a/remote.h b/remote.h\nindex cf56724..84e0f72 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -138,6 +138,8 @@ struct branch {\n \tstruct refspec **merge;\n \tint merge_nr;\n \tint merge_alloc;\n+\n+\tchar *push_name;\n };\n \n struct branch *branch_get(const char *name);\n"},{"id":"221526","messageId":"7v38scqwej.fsf@alter.siamese.dyndns.org","threadId":"34219","inReplyTo":"CALkWK0mTthYyHRkxqK2Z3uP3uUmtQADyZt7CERY_40iQSQyzAA@mail.gmail.com","subject":"Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-20T21:56:04Z","receivedAt":"2013-06-20T21:56:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Junio C Hamano wrote:\n>>> They're not the same thing.  It is very much intentional and intended:\n>>> the safety net is not to \"ensure that the push and pull are\n>>> symmetrical\" (i.e. among other things, error out if\n>>> branch.$branch.merge is unset), but rather \"ensure that the push and\n>>> pull are never asymmetrical\".\n>>\n>>     not to \"ensure that the push and pull are symmetrical\"\n>>     rather \"ensure that the push and pull are never asymmetrical\".\n>>\n>> They still talk the same thing to me.  What am I missing?\n>\n> Never mind the wording then.  I am proposing preventing asymmetry by\n> explicitly disallowing a push when $branch is different from\n> branch.$branch.merge, instead of shutting down immediately when\n> branch.$branch.merge is unset.\n\nWe always said \"upstream is to update the branch you fetch and\nintegrate with\", and tried to make sure the push destination is the\nbranch you configured the current branch (i.e. the one you are\ntrying to push out) to fetch and integrate with.  That is how we\nprevent asymmetry.\n\nWe fail if branch.$branch.merge is set to something else.  We also\nfail if branch.$branch.merge is *not* set, because by definition the\nbranch you are trying to push to in that scenario cannot be the\nbranch you fetch and integrat with by \"git pull [--rebase]\".\n\nI know your patch was attempting to change the behaviour for the\nlatter.  You are trying to let anything go if branch.*.merge is not\nset.  How would it help prevent assymetry?\n\n>>> Now I'd like to question what you are labelling as \"safety\".  What is\n>>> the consequence of erroring out when branch.$branch.merge is unset\n>>> when pushing using `upstream`?\n>>\n>> Nothing noteworthy.\n>>\n>> I wasn't suggesting to change what `upstream` does at all.\n>\n> No, but I did....\n\nReally?  Then where did this come from?\n\n> I didn't want to contaminate this series with an unrelated improvement\n> to `upstream`\n\n>> If you are using a\n>> centralized workflow, and if a branch does not have branch.*.merge\n>> configured, we do not know to which branch you are pushing it back,\n>> so we error out.\n>\n> And I said: have a sane default.\n\nLike you said, I do not want to contaminate this series with such an\nunrelated change.  Worse, you are trying to break a sane default by\nreplacing it with \"anything goes\".\n\nWe already have a sane default, which is to error out.  We do not\nneed your broken default.\n"},{"id":"221529","messageId":"7vy5a4phdr.fsf@alter.siamese.dyndns.org","threadId":"34219","inReplyTo":"7v38scqwej.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-20T22:05:52Z","receivedAt":"2013-06-20T22:05:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Like you said, I do not want to contaminate this series with such an\n> unrelated change.  Worse, you are trying to break a sane default by\n> replacing it with \"anything goes\".\n>\n> We already have a sane default, which is to error out.  We do not\n> need your broken default.\n\nThis came out as a bit stronger than I wanted to.  Add to the last:\n\n    Even if we later find out that changing the default to loosen it\n    to \"anything goes\" is a good idea, I do not think it belongs to\n    this series.\n"}]}