{"thread":{"id":"34076","subject":"[PATCH 3/4] t/push-default: generalize test_push_{success, commit}","startedAt":"2013-06-09T17:13:16Z","lastAt":"2013-06-13T17:45:31Z","messageCount":16,"participants":["Ramkumar Ramachandra","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"220027","messageId":"1370798000-2358-1-git-send-email-artagnon@gmail.com","threadId":"34076","inReplyTo":null,"subject":"[PATCH 0/4] Fix triangular workflows for upstream, simple","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-09T17:13:16Z","receivedAt":"2013-06-09T17:13:16Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> It might be a beginning of a change in the right direction (I didn't\n> check the codeflow), but given that the test that comes after the\n> one you removed is looking at branch->merge[0] and deciding what to\n> do, and branch.$name.merge should *never* affect anything when\n> remote.pushdefault or branch.$name.pushremote is in use while\n> deciding what is pushed out or if the push is allowed, I do not\n> think the removal of these five lines alone can possibly \"work\".\n\nI just spent hours breaking my head thinking about why you said my\npatch cannot possibly \"work\".  I give up.\n\nThe artificial limitation was introduced by you in 135dade, and here's\nthe relevant part of the commit message:\n\n  - The current branch does have its remote and upstream branch configured,\n    but the user said \"git push $there\", where $there is not the remote\n    named by \"branch.$name.remote\".  By definition, no branch at repository\n    $there is set to integrate with the current branch in this case, and\n    this push is not meant to update any branch at the remote repository\n    $there.\n\nWhat definition is this, and why does it only apply to\nupstream/simple?  Did you implicitly assume that upstream/simple are\nonly meant to be used in centralized workflows?\n\nThanks.\n\nRamkumar Ramachandra (4):\n  t/push-default: remove redundant test_config lines\n  push: make upstream, simple work with pushdefault\n  t/push-default: generalize test_push_{success, commit}\n  t/push-default: test pushdefault with all modes\n\n builtin/push.c          |  5 -----\n t/t5528-push-default.sh | 30 ++++++++++++++++++++++++------\n 2 files changed, 24 insertions(+), 11 deletions(-)\n\n-- \n1.8.3.247.g485169c\n"},{"id":"220028","messageId":"1370798000-2358-2-git-send-email-artagnon@gmail.com","threadId":"34076","inReplyTo":"1370798000-2358-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/4] t/push-default: remove redundant test_config lines","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-09T17:13:17Z","receivedAt":"2013-06-09T17:13:17Z","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.247.g485169c\n"},{"id":"220029","messageId":"1370798000-2358-3-git-send-email-artagnon@gmail.com","threadId":"34076","inReplyTo":"1370798000-2358-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/4] push: make upstream, simple work with pushdefault","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-09T17:13:18Z","receivedAt":"2013-06-09T17:13:18Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"rr/triangle (4d35924, 2013-04-07) introduced support for triangular\nworkflows, but did not think through the effect of the new configuration\nvariables in the upstream and simple modes.  When remote.pushdefault or\nbranch.<name>.pushremote is set, and push.default is set to upstream or\nsimple, this happens (master@{u} != origin):\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\nThe configuration variables work as expected with push.default = current\nand matching, which makes the above unexpected and counter-intuitive.\nIt happens because upstream and simple were designed with central\nworkflows in mind.  Even when these configuration variables are not set,\n\n  $ git push origin\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\nThis artificial limitation imposed on setup_push_upstream() was\nintroduced by 135dad (push: error out when the \"upstream\" semantics does\nnot make sense, 2012-03-30), but has no basis; a push.default value\nshould only dictate a refspec, and the push destination is orthogonal to\nthis.  Remove this artificial limitation, fixing the bug.  Only one test\nneeds to be changed subtly.\n\nReported-by: Leandro Lucarella <leandro.lucarella@sociomantic.com>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n builtin/push.c          | 5 -----\n t/t5528-push-default.sh | 4 ++--\n 2 files changed, 2 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 2d84d10..b253a64 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -137,11 +137,6 @@ static void setup_push_upstream(struct remote *remote, int simple)\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\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 \tif (simple && strcmp(branch->refname, branch->merge[0]->src))\n \t\tdie_push_simple(branch, remote);\n \ndiff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\nindex 69ce6bf..4e4824e 100755\n--- a/t/t5528-push-default.sh\n+++ b/t/t5528-push-default.sh\n@@ -60,13 +60,13 @@ test_expect_success '\"upstream\" does not push on unconfigured branch' '\n \ttest_push_failure upstream\n '\n \n-test_expect_success '\"upstream\" does not push when remotes do not match' '\n+test_expect_success '\"upstream\" pushes when remotes do not match' '\n \tgit checkout master &&\n \ttest_config branch.master.remote parent1 &&\n \ttest_config branch.master.merge refs/heads/foo &&\n \ttest_config push.default upstream &&\n \ttest_commit five &&\n-\ttest_must_fail git push parent2\n+\tgit push parent2\n '\n \n test_expect_success 'push from/to new branch with upstream, matching and simple' '\n-- \n1.8.3.247.g485169c\n"},{"id":"220026","messageId":"1370798000-2358-4-git-send-email-artagnon@gmail.com","threadId":"34076","inReplyTo":"1370798000-2358-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/4] t/push-default: generalize test_push_{success, commit}","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-09T17:13:19Z","receivedAt":"2013-06-09T17:13:19Z","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 4e4824e..9ddd3a9 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.247.g485169c\n"},{"id":"220030","messageId":"1370798000-2358-5-git-send-email-artagnon@gmail.com","threadId":"34076","inReplyTo":"1370798000-2358-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 4/4] t/push-default: test pushdefault with all modes","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-09T17:13:20Z","receivedAt":"2013-06-09T17:13:20Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Introduce test_pushdefault_with_mode() to test that remote.pushdefault\nworks with all the push.default modes correctly.  Exercise it with all\nfour modes to illustrate how triangular workflows work, and to guard\nagainst regressions.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t5528-push-default.sh | 18 ++++++++++++++++++\n 1 file changed, 18 insertions(+)\n\ndiff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\nindex 9ddd3a9..9762515 100755\n--- a/t/t5528-push-default.sh\n+++ b/t/t5528-push-default.sh\n@@ -39,6 +39,19 @@ test_push_failure () {\n \ttest_cmp expect actual\n }\n \n+# $1 = push.default value\n+# $2 = branch.master.merge value (master or foo)\n+# $3 = branch to check for actual output (master or foo)\n+test_pushdefault_with_mode () {\n+test_expect_success \"push.default = $1 works with remote.pushdefault\" \"\n+\ttest_config branch.master.remote parent1 &&\n+\ttest_config branch.master.merge refs/heads/$2 &&\n+\ttest_config remote.pushdefault parent2 &&\n+\ttest_commit commit-for-$1 &&\n+\ttest_push_success $1 $3 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 +128,9 @@ 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_with_mode \"matching\" \"foo\" \"master\"\n+test_pushdefault_with_mode \"upstream\" \"foo\" \"foo\"\n+test_pushdefault_with_mode \"simple\" \"master\" \"master\"\n+test_pushdefault_with_mode \"current\" \"foo\" \"master\"\n+\n test_done\n-- \n1.8.3.247.g485169c\n"},{"id":"220203","messageId":"7vip1moq3k.fsf@alter.siamese.dyndns.org","threadId":"34076","inReplyTo":"1370798000-2358-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 2/4] push: make upstream, simple work with pushdefault","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-09T22:51:11Z","receivedAt":"2013-06-09T22:51: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> rr/triangle (4d35924, 2013-04-07) introduced support for triangular\n> workflows, but did not think through the effect of the new configuration\n> variables in the upstream and simple modes.  When remote.pushdefault or\n> branch.<name>.pushremote is set, and push.default is set to upstream or\n> simple, this happens (master@{u} != origin):\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> The configuration variables work as expected with push.default = current\n> and matching, which makes the above unexpected and counter-intuitive.\n> It happens because upstream and simple were designed with central\n> workflows in mind.  Even when these configuration variables are not set,\n>\n>   $ git push origin\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\nI am not sure what you mean by artificial.\n\nIf you have push.default set upstream or simple, then a push run\nwhile on the branch 'foo' will figure out what happens when you do a\nfetch by looking at 'branch.foo.merge' to find the branch we are set\nto integrate from 'branch.foo.remote' remote.  The simple further\nsays that branch name must be the same as 'foo'.\n\nAnd that is what setup_push_UPSTREAM() is designed to do.  Rejecting\na call that breaks the precondition is perfectly the right thing to\ndo: if you set to push to \"upstream\" and if you are trying to push\nto a different remote, for example.\n\nThe triangle topic changed the precondition without updating the\nlogic to check it, which was the bug, not the original check.\n\nAlso, it is still unclear to me why push.default = upstream in a\ntriangular setting should compute that the current branch 'foo' is\nset to integrate with 'master' from the remote 'origin' and assume\nthat updating branch 'master' in a different repository, which may\nor may not have anything to do with 'master' at 'origin', is a sane\nthing to do.\n\nWe do not have the branch.$name.push yet and if we did, that would\nbe the value to be used.  So I agree that we need to default to\nsomething, but I do not think this patch has justified that updating\n'master' (i.e. branch.foo.merge) in a repository that is totally\nunrelated to the branch branch.foo.merge's name was taken from is a\nbetter default than updating 'foo' itself.  Both look like a random\nchoice at least to me.\n\nI actually am OK with 'upstream' that rejects triangular, while\nmaking 'simple' do something different [*1*].\n\n\"push.default = upstream\" is to \"push back to where I fetch and\nintegrate with\", which by definition makes no sense in triangular\nworkflow.  But 'simple' will be the new default and it does not say\nanything about \"what I fetch and integrate with\" at all, so we can\nchoose a sensible default when branch.$name.push does not say what\nto update.\n\n\n[Footnote]\n\n*1* I offhand do not know what that \"something else\" is, but it\nwould probably be closer to 'current'.  In an extreme case, consider\nthat there may no branch.$name.remote or branch.$name.merge at all\n(the user is the top-level integrator).\n\nRemoving the \"remote name must match when using 'upstream'\" check in\nthe setup_push_upstream() is not sufficient if we want to allow the\nremote.pushdefault for such a user with push.default = simple mode,\nbecause presense of branch.$name.remote and branch.$name.merge is\nchecked very early in the function, and lack of them will error out.\n"},{"id":"220262","messageId":"CALkWK0mesZay8Cpi6yTvhUG=136=9JLyFUZXm8t_fMOrY0F62Q@mail.gmail.com","threadId":"34076","inReplyTo":"7vip1moq3k.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] push: make upstream, simple work with pushdefault","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-10T08:43:45Z","receivedAt":"2013-06-10T08:43:45Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> I am not sure what you mean by artificial.\n\nBy artificial, I mean that the precondition is absolutely unnecessary\nfor the code following it to work.  The precondition was introduced in\na separate commit, specifically denying one usecase because the author\n(you) thought that made sense.\n\n> If you have push.default set upstream or simple, then a push run\n> while on the branch 'foo' will figure out what happens when you do a\n> fetch by looking at 'branch.foo.merge' to find the branch we are set\n> to integrate from 'branch.foo.remote' remote.  The simple further\n> says that branch name must be the same as 'foo'.\n\nI understand this perfectly well, as evidenced by the tests I've\nwritten out in 4/4.\n\n> And that is what setup_push_UPSTREAM() is designed to do.  Rejecting\n> a call that breaks the precondition is perfectly the right thing to\n> do: if you set to push to \"upstream\" and if you are trying to push\n> to a different remote, for example.\n>\n> The triangle topic changed the precondition without updating the\n> logic to check it, which was the bug, not the original check.\n\nDid I claim otherwise?  :)\nThe topic is about fixing a bug introduced by rr/triangle.\n\n> I actually am OK with 'upstream' that rejects triangular, while\n> making 'simple' do something different [*1*].\n\nOkay, so you haven't outlined a solution either.  Like I said in the\ncover-letter, I've spent hours breaking my head and can't figure out a\nbetter solution.  The bigger problem is that upstream/ simple were\ndesigned with only central workflows in mind.  How do we deal with it\nnow?\n\nYes, upstream/ simple work only when @{u} resolves, and I haven't\nchanged this (because we don't have a well-defined meaning for what\nbranch.<name>.merge without a branch.<name>.remote means).  My\nargument is very simple: no push.default mode should not dictate the\npush destination; only the push refspec.  What makes sense to the user\nis an upstream/ simple that works as expected with pushdefault: do the\ntests I've outlined in 4/4 not make sense?  *scratches head*\n\nI don't understand why upstream/ simple should _not_ push to a\ndifferent destination from the one being merged from.  I'll repeat:\npush source/ destination is orthogonal to push refspec, and\npush.default modes dictate the refspec.\n"},{"id":"220267","messageId":"7v4nd6l31d.fsf@alter.siamese.dyndns.org","threadId":"34076","inReplyTo":"CALkWK0mesZay8Cpi6yTvhUG=136=9JLyFUZXm8t_fMOrY0F62Q@mail.gmail.com","subject":"Re: [PATCH 2/4] push: make upstream, simple work with pushdefault","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-10T09:37:34Z","receivedAt":"2013-06-10T09:37: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> I don't understand why upstream/ simple should _not_ push to a\n> different destination from the one being merged from.  I'll repeat:\n> push source/ destination is orthogonal to push refspec, and\n> push.default modes dictate the refspec.\n\nThat is where we differ.  You keep repeating that without justifying\nwhy reusing the branch upstream would have chosen in a non-triangular\nworkflow is a good idea in the first place.\n\nLet's step back a bit and think.  In a workflow where we both agree\nthat \"upstream\" mode makes sense, i.e. central repository workflow,\nwe may have a \"triangle\" topic in our shared repository, and I may\nhave my own topic forked from it (i.e. set to fetch that \"triangle\"\nand integrate my work with it).  The topic is to work on one aspect\nof triangular workflow, e.g. \"pushbranch\" (i.e. I am not worried\nabout how we choose which remote to push to, but interested in what\nbranch at that remote is updated).\n\nIt makes sense for me to push the result back to the upstream,\ni.e. \"triangle\" branch at the shared remote where I forked from, to\ncollaborate with other people (e.g. you) whose shared purpose is to\nimprove the triangular workflow.  You may be doing the same, and\nyour local branch may be called \"triangle\" (bag of various\ntriangular workflow improvements), or \"pushremote\" (a specific\nsubtopic).  In either case, you push to \"upstream\" to update\n\"triangle\".\n\nWhat should happen in a triangular workflow, where I may be fetching\nand integrating from your repository to work on triangular workflow\nin general (let's say that you are the owner of that topic) to\nadvance the support for that workflow.  The topic in your repository\nmay be called \"triangle\" to improve triangular workflow in general,\nand again my branch would be \"pushbranch\" to improve one specific\naspect of it.\n\nAs we are using triangular workflow, the place *I* push to is under\nmy control, not the shared maintainer one or your repository, and\nI'll be requesting you to pull that topic from me.\n\nNow think _why_ I renamed the branch on my end, i.e. not calling\nthat branch in question \"triangle\" that is the blanket name for the\ncollective effort but calling it with a more specific name\n\"pushbranch\", in the first place.\n\nIt is to clarify that my part of the effort is that particular\nsubarea of the topic.  Wouldn't it be a lot more sensible to use\nthat more specific name when publishing, rather than \"triangle\"\nname, and ask you to pull that branch that describes what I\nconcentrated on?\n\nPush destination and fetch source, when they are different, refer to\ndifferent repositories by definition, and they are in different\nadministration domains.  Why should the name used to publish my work\nwhich is a specialization of what I built on (i.e. your \"triangle\")\nlose the specialization from its name when I push it out?  We can\nafford not to lose information in the branch name.\n\nThe difference between the shared repository case and triangular\nworkflow primarily comes from the fact that the final integration\nhappens at a different place.  In the shared repository case, the\nintegration happens in each participant's local repository and push\nupdates the shared history at the known name.  If my \"pushbranch\"\nspecialization diverted from the overall \"triangle\" effort, I would\nfirst integrate others work and then update the remote, and at that\npoint, what I am pushing is no longer an isolated specialized effort,\nbut the result of its integration into a larger topic.\n\nAnd once the more specialized topic is integrated into a more\ngeneric branch it was forked from, it makes sense to update that\nmore generic branch at the shared repository.\n\nIn the triangular case, however, you as the \"triangle\" topic owner\nhas the choice to fetch and integrate from me or not to do so.  And\nthe integration happens when you decide to fetch and integrate.\nThere is no reason whatsoever to lose the more specific name until\nthat integration happens into your (meaning: the owner of the more\ngeneral \"triangle\" topic) \"bag of various triangular improvements\"\ntopic.\n\nImagine the case I forked two branches from your \"triangle\" topic\nand pushing them to my publishing repository using the triangular\nworkflow.  Why should I attempt to update a single \"triangle\" branch\nwith both of these topics?\n\nThe name under which the local branch is published needs a sensible\ndefault (when branch.$name.push is not specified), and I agree that\nyou would get the name of the branch my work was forked from if you\nreuse the \"upstream\" code.  I am saying that it does not necessarily\ngive us a good default.\n"},{"id":"220274","messageId":"CALkWK0mY5=H6FoUZCOXTYykEV1f=3wrP21WPXj1v4VBCeOxocg@mail.gmail.com","threadId":"34076","inReplyTo":"7v4nd6l31d.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] push: make upstream, simple work with pushdefault","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-10T11:48:31Z","receivedAt":"2013-06-10T11:48:31Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> The name under which the local branch is published needs a sensible\n> default (when branch.$name.push is not specified), and I agree that\n> you would get the name of the branch my work was forked from if you\n> reuse the \"upstream\" code.  I am saying that it does not necessarily\n> give us a good default.\n\nSee, that's the problem: push.default is simply a default refspec to\npush to for all branches, that can be _overridden_ by\nbranch.<name>.push.  Getting an override is not going to solve the\nproblem we are facing: what to do when branch.<name>.push is\nunspecified?  Fall back to push.default, right?\n\n> Now think _why_ I renamed the branch on my end, i.e. not calling\n> that branch in question \"triangle\" that is the blanket name for the\n> collective effort but calling it with a more specific name\n> \"pushbranch\", in the first place.\n\nLook, it's very simple.  upstream was built to support the case when\nthe local branch name is different from the remote branch name, but it\nwas too specialized for central workflows.  How do we extend it for\ntriangular workflows?\n\nJust like we introduced branch.<name>.pushremote to override\nbranch.<name>.remote, we get branch.<name>.push to override\nbranch.<name>.merge.  If branch.<name>.pushremote is unset, where do\nthe pushes go?  branch.<name>.remote.  If branch.<name>.push is\nunspecified, what is the refspec to be pushed?  branch.<name>.merge\n(when push.default = upstream) [*1*].\n\nWhat does this mean?  I publish the branch \"triangle\" on ram (what my\nlocal branch is called or what push.default I use is irrelevant).  You\nhave a branch called pushremote with branch.pushremote.remote set to\nram, remote.pushdefault set to junio, branch.pushremote.merge set to\nrefs/heads/triangle, and push.default set to upstream.\n\n  # on jc's machine; on branch pushremote\n  $ git pull\n  # integrates changes from ram's triangle just fine\n  $ git push\n  # publishes the branch as triangle on remote junio\n\nI rewrite my branch, and you have 4 commits based on my branch:\n\n  $ git rebase --onto @{u} @~4\n\nPerfect.\n\nThe only limitation is that you don't choose the branch name on junio.\n When we get branch.pushremote.push, you'll be able to set it to\nrefs/heads/pushremote, making push.default inconsequential.  Done.\n\n[Footnotes]\n\n*1* remote.pushdefault overrides branch.<name>.remote, while\npush.default will be overridden by a future branch.<name>.push.  Not\nexactly elegant, is it?\n"},{"id":"220304","messageId":"7vwqq2j7c9.fsf@alter.siamese.dyndns.org","threadId":"34076","inReplyTo":"CALkWK0mY5=H6FoUZCOXTYykEV1f=3wrP21WPXj1v4VBCeOxocg@mail.gmail.com","subject":"Re: [PATCH 2/4] push: make upstream, simple work with pushdefault","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-10T15:47:34Z","receivedAt":"2013-06-10T15:47: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> Junio C Hamano wrote:\n>> The name under which the local branch is published needs a sensible\n>> default (when branch.$name.push is not specified), and I agree that\n>> you would get the name of the branch my work was forked from if you\n>> reuse the \"upstream\" code.  I am saying that it does not necessarily\n>> give us a good default.\n>\n> See, that's the problem: push.default is simply a default refspec to\n> push to for all branches, that can be _overridden_ by\n> branch.<name>.push.\n\nThink again and remember that Git is distributed.\n\nAs you say, the push.default configuration specifies the mode that\ncomputes what remote branches are updated.  Where we differ is that\nI consider \"triangle\" branch at your repository I fetch from and\n\"triangle\" branch at the repository I push to (in the triangular\nworkflow) different entities, just like \"triangle\" branch in my\nlocal repository is different from both.  branch.triangle.merge\nconfiguration is used to link the first and the third, and the first\nis called \"upstream\" of the third for this reason.  It does not have\nanything to do with the second.  You seem to think branch.*.merge is\nused to link the second with the first, and I do not think it makes\nany sense.\n\n> Getting an override is not going to solve the\n> problem we are facing: what to do when branch.<name>.push is\n> unspecified?  Fall back to push.default, right?\n\nThat is not the issue.  We both are solving it by falling back to\npush.default, but the problem is that the mental picture you have\nfor push.default has not been updated to support the triangular\nworld better.  In the centralized world, the first and the second\n(in the above description) remote branches, both of which are named\n\"triangle\", are the same, because they are in the same remote\nrepository.  Trying to reuse that in the triangular world is *not*\nautomatically being orthogonal and better; it may be simply keeping\nthings inconvenient by choosing a wrong definition, which is what I\nam trying to avoid by thinking things through.\n\n>> Now think _why_ I renamed the branch on my end, i.e. not calling\n>> that branch in question \"triangle\" that is the blanket name for the\n>> collective effort but calling it with a more specific name\n>> \"pushbranch\", in the first place.\n>\n> Look, it's very simple.  upstream was built to support the case when\n> the local branch name is different from the remote branch name, but it\n> was too specialized for central workflows.  How do we extend it for\n> triangular workflows?\n\n\"upstream\" is for the case where you push back to your \"upstream\",\nthat which you fetch from and integrate with.  It is perfectly fine\nto extend it for triangular workflow by erroring it out when it is\nused, as you do *not* want to update \"upstream\" in that workflow.\nYou are sending your work to your publishing repository.\n\n> Just like we introduced branch.<name>.pushremote to override\n> branch.<name>.remote, we get branch.<name>.push to override\n> branch.<name>.merge.  If branch.<name>.pushremote is unset, where do\n> the pushes go?  branch.<name>.remote.  If branch.<name>.push is\n> unspecified, what is the refspec to be pushed?  branch.<name>.merge\n> (when push.default = upstream) [*1*].\n>\n> What does this mean?\n\nDoesn't it just mean that you do not understand \"distributed\"?  \n\nWhat makes you think that the repository you fetch from and the\nrepository you push to, which are by definition different\nrepositories, have to share the same namespace?\n\n>  I publish the branch \"triangle\" on ram (what my\n> local branch is called or what push.default I use is irrelevant).  You\n> have a branch called pushremote with branch.pushremote.remote set to\n> ram, remote.pushdefault set to junio, branch.pushremote.merge set to\n> refs/heads/triangle, and push.default set to upstream.\n>\n>   # on jc's machine; on branch pushremote\n>   $ git pull\n>   # integrates changes from ram's triangle just fine\n>   $ git push\n>   # publishes the branch as triangle on remote junio\n\nOnly if I want to publish the result of the work forked from your\n\"triangle\" as my \"triangle\", but that is not the case.  A fork to be\nintegrated by other is by definition more specialized than the\noriginal, and I would publish my \"pushbranch\" subtopic as such, not\nas \"triangle\".\n\nIt appears that the message you are responding to failed to convey\nthe primary point, so it may not be very useful for me to repeat it\nhere, but anyway.\n\n> [Footnotes]\n>\n> *1* remote.pushdefault overrides branch.<name>.remote, while\n> push.default will be overridden by a future branch.<name>.push.  Not\n> exactly elegant, is it?\n\nBut that is not what is happening, unless you keep thinking\n\"push.default decides the name of the branch regardless of what\nrepository it lives in\", which is where our difference lies, I\nthink.\n\nImagine the case where I forked two branches from your \"triangle\"\ntopic and pushing them to my repository using the triangular\nworkflow.  By your definition, they will both try to push to\n\"triangle\", which means you, the \"triangle\" topic maintainer, cannot\nreceive two independent pull requests from me.  You can only get a\nsingle branch that pre-merges both, and if you want to get one but\nnot the other, you have to do the untangling of these two topics.\n\nThe solution to the above becomes clear and simple, once you stop\nthinking that the branch namespace you have has to be the same as\nthe branch namespaces other people have (which was limitation\nimposed by central workflow, to which triangular has no reason to be\nbound to).  Pushing to \"upstream\" by tying the name I fetch from and\nthe name I publish the result as is not appropriate in triangular\nworkflow, so just fail it.\n\nI am not saying that you have to pick one to use for push.default\namong the remaining ones (i.e. matching, current, what else?).  It\nis very plausible that the triangular workflow wants a different\nlogic to pick what branches are to be updated and how.  Perhaps we\nwould want something that is capable of mapping your local branch\nname to a branch name suitable in your publishing repository, and I\nam not opposed to have such a mode.\n\nBut I think it _is_ a short-sighted mistake to consider \"upstream\"\nthe appropriate choice, if the reason to think so is only because\nthat is the only one that does any form of mapping in the current\nsystem that was specifically designed to support workflow that\npushes back to where you fetched from to integrate with\n(e.g. centralized).  We need to first ask if its mapping (i.e. what\nthe branch at \"upstream\" repository you merge into your local branch\nin question is named) is the appropriate mapping to be used also for\npushing.  And I asked that question in the message you are\nresponding to (and my answer is it is not suitable).\n"},{"id":"220312","messageId":"CALkWK0mfWs3LBocvn=QMcX0_ZCROx4Nw=+xjpCbKLQU0JzSdXg@mail.gmail.com","threadId":"34076","inReplyTo":"7vwqq2j7c9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] push: make upstream, simple work with pushdefault","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-10T16:07:01Z","receivedAt":"2013-06-10T16:07:01Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> Only if I want to publish the result of the work forked from your\n> \"triangle\" as my \"triangle\", but that is not the case.  A fork to be\n> integrated by other is by definition more specialized than the\n> original, and I would publish my \"pushbranch\" subtopic as such, not\n> as \"triangle\".\n\nOkay, so it should _never_ push out as \"triangle\", because it is an\ninsane default.\n\n> But that is not what is happening, unless you keep thinking\n> \"push.default decides the name of the branch regardless of what\n> repository it lives in\", which is where our difference lies, I\n> think.\n\nI assumed that we want all push.default modes to do something\n\"sensible\" in both central and triangular workflows.  push.default\ndictating a push destination would break triangular workflows.  It's\nvery clear to me.\n\n> Imagine the case where I forked two branches from your \"triangle\"\n> topic and pushing them to my repository using the triangular\n> workflow.  By your definition, they will both try to push to\n> \"triangle\", which means you, the \"triangle\" topic maintainer, cannot\n> receive two independent pull requests from me.  You can only get a\n> single branch that pre-merges both, and if you want to get one but\n> not the other, you have to do the untangling of these two topics.\n\nOkay, you've made your point.  Due to namespace conflict, the solution\nI proposed is not at all a \"sane\" default.  It's just confusing and\nBad (TM).\n\n> I am not saying that you have to pick one to use for push.default\n> among the remaining ones (i.e. matching, current, what else?).  It\n> is very plausible that the triangular workflow wants a different\n> logic to pick what branches are to be updated and how.  Perhaps we\n> would want something that is capable of mapping your local branch\n> name to a branch name suitable in your publishing repository, and I\n> am not opposed to have such a mode.\n\nOkay, we'll have to do some sort of split and mark push.default =\nupstream/ simple suitable-only-for-centralized-workflows, or something\nto that effect (deprecation?) :|\n\nI'll try to think of something.\n"},{"id":"220361","messageId":"7vobbdhjeg.fsf@alter.siamese.dyndns.org","threadId":"34076","inReplyTo":"CALkWK0mfWs3LBocvn=QMcX0_ZCROx4Nw=+xjpCbKLQU0JzSdXg@mail.gmail.com","subject":"Re: [PATCH 2/4] push: make upstream, simple work with pushdefault","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-10T19:09:59Z","receivedAt":"2013-06-10T19:09:59Z","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>> I am not saying that you have to pick one to use for push.default\n>> among the remaining ones (i.e. matching, current, what else?).  It\n>> is very plausible that the triangular workflow wants a different\n>> logic to pick what branches are to be updated and how.  Perhaps we\n>> would want something that is capable of mapping your local branch\n>> name to a branch name suitable in your publishing repository, and I\n>> am not opposed to have such a mode.\n>\n> Okay, we'll have to do some sort of split and mark push.default =\n> upstream/ simple suitable-only-for-centralized-workflows, or something\n> to that effect (deprecation?) :|\n\nAmong the current ones, I think \"upstream\" is the only one that has\nthe \"branch I fetch and integrate with is the one I want to update\nwith my result\" connotation.  The \"current\" and \"matching\" modes\ndetermine what gets pushed solely between the local repository you\nare pushing from and the remote repository you are pushing to,\nwithout getting \"what do I fetch and integrate with\" in the\nequation.\n\nAs an extension to \"upstream\", the current implementation of\n\"simple\" of course has the same issue, but because the name \"simple\"\ndoes not inherently have such \"branch I fetch and integrate with is\nthe one I want to update with my result\" connotation, we can clean\nup its semantics to match the new reality after triangular workflow.\n\nIf you recall the earlier discussion on \"@{publish} which is\ndifferent from @{upstream}\", one idea to allow mapping on the push\nend was to introduce \"push.default = single\" that would act as\n\"upstream\" when in \"branch I fetch and integrate with is the same\nbranch at the same repository the one I want to update with my\nresult\" workflow, and in a triangular workflow maps the branch being\npushed using remote.$name.push refspecs (if exists).\n\nI think extending it further to act as 'current' if no push refspecs\nare set for the remote you push to in a triangular workflow might be\na way to go if we want the mapping flexibility without having to set\nbranch.$name.push to each and every branch.\n"},{"id":"220701","messageId":"CALkWK0mDDKETE1O=g7X5s5MJvU7UA_NQiMNESor_8=phW0WR-A@mail.gmail.com","threadId":"34076","inReplyTo":"7vobbdhjeg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] push: make upstream, simple work with pushdefault","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-13T09:10:44Z","receivedAt":"2013-06-13T09:10:44Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> If you recall the earlier discussion on \"@{publish} which is\n> different from @{upstream}\", one idea to allow mapping on the push\n> end was to introduce \"push.default = single\" that would act as\n> \"upstream\" when in \"branch I fetch and integrate with is the same\n> branch at the same repository the one I want to update with my\n> result\" workflow, and in a triangular workflow maps the branch being\n> pushed using remote.$name.push refspecs (if exists).\n\nI'm still resisting this idea, because I don't like these special-case\npush.default modes.  If possible, I want to avoid introducing another\none to the existing mess.\n\n> [...]\n\nOkay, so what you're saying makes sense.  I'm cooking the following idea:\n\n- current: push \"$(HEAD)\".  No restrictions on destination.  The most\ngeneric, sensible, and extensible one, in my opinion.\n\n- matching: push \":\" to the destination specified by the current\nbranch. [since I cannot know what I'm pushing in advance, I think this\nis generally ugly]\n\n- upstream: In the special case when fetch source is equal to push\ndestination, push \"$(HEAD):$(branch.$(HEAD).merge)\".  Otherwise,\nfallback to current.  Useful in central workflows.\n\n- simple: [still haven't thought about what to do with this; I'm\ngenerally not in favor of artificially crippling functionality by\nerroring out]\n\nJust like upstream respects branch.<name>.merge, current respects\nbranch.<name>.push, making branch-level ref mapping in triangular\nworkflows possible.  Finally, remote.<name>.push is entirely\northogonal to all this, and is respected no matter what.\n\nAm I making any sense?\n"},{"id":"220739","messageId":"7vehc6vtvt.fsf@alter.siamese.dyndns.org","threadId":"34076","inReplyTo":"CALkWK0mDDKETE1O=g7X5s5MJvU7UA_NQiMNESor_8=phW0WR-A@mail.gmail.com","subject":"Re: [PATCH 2/4] push: make upstream, simple work with pushdefault","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-13T16:48:54Z","receivedAt":"2013-06-13T16:48:54Z","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>\n>> [...]\n>\n> Okay, so what you're saying makes sense.\n\nThat is not a very useful style of quoting; what did you just agree to?\n\nI think we should step back and clarify the \"to which repository the\npush goes\" and \"what branch(es) in that chosen repository is\nupdated\".  The former is determined by your original \"triangular\"\ntopic in the recent world order.\n\nThe push.default specifies the logic/algorithm to decide the latter,\nwhen there is no stronger configuration is given (e.g. the push\nrefspecs in remote.*.push, and branch.*.push).\n\n> - current: push \"$(HEAD)\".  No restrictions on destination.\n\nThis updates the branch with the same name the current branch on the\npushing side.\n\n> - matching: push \":\" to the destination specified by the current\n> branch.\n\nThis updates the branches in the destination repository, for which\nbranches with the same name exists on the pushing side.\n\n> - upstream: In the special case when fetch source is equal to push\n> destination, push \"$(HEAD):$(branch.$(HEAD).merge)\".  Otherwise,\n> fallback to current.  Useful in central workflows.\n\nThat looks to me as an inconsistent description.  If you are not\npushing to where you fetched from, that is not even central.\n\nThis is mean to update the branch that is fetched from and is\nintegrated with the current branch with the tip of the current\nbranch, so the branch at the destination repository that gets\nupdated is branch.$current.merge.  It further means that the\nrepository being pushed to must be the same as the repository we\nfetch from; otherwise it is an error.\n\n> - simple: [still haven't thought about what to do with this; I'm\n> generally not in favor of artificially crippling functionality by\n> erroring out]\n\nIn a central workflow (i.e. repository we fetch from to update the\ncurrent branch is the same as the repository we push the tip of this\nbranch to), this works the same as upstream, but the configured\nbranch.$current.merge has to be the same as the name of the current\nbranch in the local repository; otherwise it is an error.\n\nIn a triangular workflow TBD, but I think doing the same as current\nmay be a good starting point.\n\n> Just like upstream respects branch.<name>.merge, current respects\n> branch.<name>.push, making branch-level ref mapping in triangular\n> workflows possible.\n\nI do not know we want to make branch.*.push linked to current.  If\nit is set, shouldn't that apply when push.default is \"matching\" and\nother values?  That is why I threw it in the same category as the\ntraditional push refspecs in remote.*.push in the early part of this\nmessage.\n"},{"id":"220746","messageId":"7vk3lxvrj0.fsf@alter.siamese.dyndns.org","threadId":"34076","inReplyTo":"7vehc6vtvt.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] push: make upstream, simple work with pushdefault","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-13T17:39:47Z","receivedAt":"2013-06-13T17:39:47Z","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> Ramkumar Ramachandra <artagnon@gmail.com> writes:\n>\n>> Junio C Hamano wrote:\n>>\n>>> [...]\n>>\n>> Okay, so what you're saying makes sense.\n>\n> That is not a very useful style of quoting; what did you just agree to?\n\nAhh.\n\nI took your response as \"I'm still resisting [to what was quoted\nbefore it].  But to the part [...] that I am not quoting I would\nagree\", hence my question.\n\nDid you mean \"I'm still resisting, but after reading [...] I think\nit makes sense\"?  If so, please discard my question.\n"},{"id":"220747","messageId":"CALkWK0kx=NRzYpDmr-PeswZi=eYYzycqc19phfpwsgnbOb1PZQ@mail.gmail.com","threadId":"34076","inReplyTo":"7vk3lxvrj0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] push: make upstream, simple work with pushdefault","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-13T17:45:31Z","receivedAt":"2013-06-13T17:45:31Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> Did you mean \"I'm still resisting, but after reading [...] I think\n> it makes sense\"?  If so, please discard my question.\n\nSorry about the lack of clarity.  I agreed with most of what you said,\nand I outlined how we could possibly turn it into an implementation.\nStill haven't found a solution.\n"}]}