{"thread":{"id":"21363","subject":"[PATCH] push: support remote branches in guess_ref DWIM","startedAt":"2009-10-26T21:33:53Z","lastAt":"2009-10-28T00:01:04Z","messageCount":5,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"125952","messageId":"20091026213353.GA27871@sigio.peff.net","threadId":"21363","inReplyTo":null,"subject":"[PATCH] push: support remote branches in guess_ref DWIM","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-26T21:33:53Z","receivedAt":"2009-10-26T21:33:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we get an unqualified dest refspec like \"foo\", we try\nto guess its full ref path (like \"refs/heads/foo\") based on\nthe source ref. Commit f8aae12 mapped local heads to remote\nheads, and local tags to remote tags.\n\nThis commit maps local tracking branches under\n\"refs/remotes\" to remote branch heads, so\n\n  git push origin origin/branch:renamed-branch\n\npushes to refs/heads/renamed-branch.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis came from a discussion on IRC. I don't see any reason not to do\nthis; would people really expect it to push into refs/remotes/ on the\nother side (right now, it complains and dies)?\n\nA related issue (which exists even without this patch) is that doing\nthis:\n\n  master:remotes/incoming/master\n\nwill create \"refs/heads/remotes/incoming/master\". Perhaps we should DWYM\na little more and recognize \"heads\", \"remotes\", and \"tags\" as special.\n\n remote.c              |    2 +-\n t/t5516-fetch-push.sh |    9 +++++++++\n 2 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 73d33f2..1a7c81e 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -998,7 +998,7 @@ static char *guess_ref(const char *name, struct ref *peer)\n \tif (!r)\n \t\treturn NULL;\n \n-\tif (!prefixcmp(r, \"refs/heads/\"))\n+\tif (!prefixcmp(r, \"refs/heads/\") || !prefixcmp(r, \"refs/remotes/\"))\n \t\tstrbuf_addstr(&buf, \"refs/heads/\");\n \telse if (!prefixcmp(r, \"refs/tags/\"))\n \t\tstrbuf_addstr(&buf, \"refs/tags/\");\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 6889a53..692c773 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -383,6 +383,15 @@ test_expect_success 'push tag with non-existant, incomplete dest' '\n \n '\n \n+test_expect_success 'push remote branch with non-existant, incomplete dest' '\n+\n+\tmk_test &&\n+\tgit update-ref refs/remotes/foo/bar master &&\n+\tgit push testrepo foo/bar:branch &&\n+\tcheck_push_result $the_commit heads/branch\n+\n+'\n+\n test_expect_success 'push sha1 with non-existant, incomplete dest' '\n \n \tmk_test &&\n-- \n1.6.5.1.201.g7feba\n"},{"id":"125963","messageId":"7v8wexn34i.fsf@alter.siamese.dyndns.org","threadId":"21363","inReplyTo":"20091026213353.GA27871@sigio.peff.net","subject":"Re: [PATCH] push: support remote branches in guess_ref DWIM","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-26T23:31:57Z","receivedAt":"2009-10-26T23:31:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> When we get an unqualified dest refspec like \"foo\", we try\n> to guess its full ref path (like \"refs/heads/foo\") based on\n> the source ref. Commit f8aae12 mapped local heads to remote\n> heads, and local tags to remote tags.\n>\n> This commit maps local tracking branches under\n> \"refs/remotes\" to remote branch heads, so\n>\n>   git push origin origin/branch:renamed-branch\n>\n> pushes to refs/heads/renamed-branch.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> This came from a discussion on IRC. I don't see any reason not to do\n> this; would people really expect it to push into refs/remotes/ on the\n> other side (right now, it complains and dies)?\n\nAs _our_ origin can never be _their_ origin if we are clone of them, I do\nnot think anybody sane would expect it to push into refs/remotes/origin/\nto begin with.\n\nBut \"not in refs/remotes/\" does not automatically mean \"the only sensible\nplace is refs/heads\", does it?  \"We do not know what kind of mistake the\nuser is trying to make\" could be more plausible answer, and in that case,\n\"complain and die\" may be a more valid course of action.\n\nFor example,\n\n    git push origin origin/master:refs/heads/master\n\nis most likely to be a mistake.  The only situation something similar to\nthis makes sense is where you pushed out a bogus commit earlier and are\ntrying to correct it perhaps with\n\n\tgit push origin +origin/master@{1}:refs/heads/master\n\nbut you need to force it and go back in the reflog.\n\nI know you are doing this DWIM only when $dst does not exist over there,\nand the \"pushing old master back to master\" above is a different scenario,\nbut if \"git push origin origin/master:master\" shouldn't update their local\nmaster, \"git push origin origin/master:naster\" shouldn't update their\nlocal naster, in order to avoid confusion arising from inconsistency, no?\n\nAnother possibility is to see the type (not refname) of $src and push all\ncommits to refs/heads/$dst and everything else (most likely tags) to\nrefs/tags/$dst, to simplify the rule to make it easier to explain and\nremember, but there are lightweight tags that make this idea unworkable.\n\n> A related issue (which exists even without this patch) is that doing\n> this:\n>\n>   master:remotes/incoming/master\n>\n> will create \"refs/heads/remotes/incoming/master\". Perhaps we should DWYM\n> a little more and recognize \"heads\", \"remotes\", and \"tags\" as special.\n\nYes, it is an independent issue; I think correcting this DWIM (or at least\n\"warning\" if not refusing to create remotes/ under refs/heads/) might be a\ngood idea.\n"},{"id":"125980","messageId":"20091027014525.GA29583@sigio.peff.net","threadId":"21363","inReplyTo":"7v8wexn34i.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: support remote branches in guess_ref DWIM","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-27T01:45:25Z","receivedAt":"2009-10-27T01:45:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 26, 2009 at 04:31:57PM -0700, Junio C Hamano wrote:\n\n> As _our_ origin can never be _their_ origin if we are clone of them, I do\n> not think anybody sane would expect it to push into refs/remotes/origin/\n> to begin with.\n\nOK, I agree.\n\n> But \"not in refs/remotes/\" does not automatically mean \"the only sensible\n> place is refs/heads\", does it?  \"We do not know what kind of mistake the\n> user is trying to make\" could be more plausible answer, and in that case,\n> \"complain and die\" may be a more valid course of action.\n\nThe thing is that I can't think of another sensible place. And this\nsensible place is useful for one particular action: renaming a remote\nbranch, like this:\n\n  $ git fetch ;# presumably gets origin/branch\n  $ git push origin/branch:renamed-branch\n\nwhich is much nicer than exposing clueless users to\n\":refs/heads/renamed-branch\".\n\n> For example,\n> \n>     git push origin origin/master:refs/heads/master\n> \n> is most likely to be a mistake.  The only situation something similar to\n> this makes sense is where you pushed out a bogus commit earlier and are\n> trying to correct it perhaps with\n\nI'm not sure why it's likely to be a mistake. I've given the one use\ncase I can think of where you _do_ want to do it. But I'm not sure why\nyou would accidentally provide something in refs/remotes, or not want to\nbe pushing to refs/heads. Where _else_ do you push, except for tags?\n\nAm I missing some part of your argument?\n\n> > A related issue (which exists even without this patch) is that doing\n> > this:\n> >\n> >   master:remotes/incoming/master\n> >\n> > will create \"refs/heads/remotes/incoming/master\". Perhaps we should DWYM\n> > a little more and recognize \"heads\", \"remotes\", and \"tags\" as special.\n> \n> Yes, it is an independent issue; I think correcting this DWIM (or at least\n> \"warning\" if not refusing to create remotes/ under refs/heads/) might be a\n> good idea.\n\nOK, I'll try to work up a patch.\n\n-Peff\n"},{"id":"126063","messageId":"7vaazc31sj.fsf@alter.siamese.dyndns.org","threadId":"21363","inReplyTo":"20091027014525.GA29583@sigio.peff.net","subject":"Re: [PATCH] push: support remote branches in guess_ref DWIM","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-27T22:33:16Z","receivedAt":"2009-10-27T22:33:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Oct 26, 2009 at 04:31:57PM -0700, Junio C Hamano wrote:\n>\n>> As _our_ origin can never be _their_ origin if we are clone of them, I do\n>> not think anybody sane would expect it to push into refs/remotes/origin/\n>> to begin with.\n>\n> OK, I agree.\n>\n>> But \"not in refs/remotes/\" does not automatically mean \"the only sensible\n>> place is refs/heads\", does it?  \"We do not know what kind of mistake the\n>> user is trying to make\" could be more plausible answer, and in that case,\n>> \"complain and die\" may be a more valid course of action.\n>\n> The thing is that I can't think of another sensible place. And this\n> sensible place is useful for one particular action: renaming a remote\n> branch, like this:\n>\n>   $ git fetch ;# presumably gets origin/branch\n>   $ git push origin/branch:renamed-branch\n>\n> which is much nicer than exposing clueless users to\n> \":refs/heads/renamed-branch\".\n\nYou would need to expose \":refs/heads/branch\" to make this a rename, not a\ncopy, wouldn't you?\n\n>> For example,\n>> \n>>     git push origin origin/master:refs/heads/master\n>> \n>> is most likely to be a mistake.  The only situation something similar to\n>> this makes sense is where you pushed out a bogus commit earlier and are\n>> trying to correct it perhaps with\n>\n> I'm not sure why it's likely to be a mistake.\n> ...\n> Am I missing some part of your argument?\n\nI do not see much point (other than your \"rename\" example) in pushing what\nyou got from the other end without changing anything yourself back to the\nsame remote.\n\nThere was a thread in which people argued that the primary thing that is\ndangerous in this sequence\n\n    $ git checkout origin/next; work-commit; work-commit; ...\n\nis when you leave the detached HEAD state without saving it to a local\nbranch.  I think what is more dangerous is that it will not give the user\na solid understanding that these commits do _not_ change origin/next in\nany way.  After doing the above, it is understandable that a novice would\nmistakenly think: \"I started from origin/next and built some, let's push\nthe result\".\n\n\tSide note: This is the reason why I think Dscho's \"git checkout\n        next\" that dwims to \"-t -b next origin/next\" is OK (perhaps on the\n        right side of the borderline), because it is more clear that this\n        is about creating and using your own \"next\", compared to the\n        existing \"-t origin/next\" shorthand.  The latter risks imprinting\n        a misconception on an uninitiated that we are somehow working on\n        origin/next.\n\nWith such a misconception, you will see the likely mistake here, too:\n\n    $ git push origin origin/next:refs/heads/next\n\nIf \"rename\" is the _only_ valid reason you might want to push what you got\nfrom there back to the same remote, _and_ if \"rename\" is something very\noften needed, I think we would prefer to have a way to support that\noperation directly, instead of more general DWIM that would risk passing\nmistakes like the above unwarned.\n\n> ...  But I'm not sure why\n> you would accidentally provide something in refs/remotes, or not want to\n> be pushing to refs/heads. Where _else_ do you push, except for tags?\n\nThe above \"checking out origin/next\" illustrates the mistake on the $src\nside, not on the $dst side.  That's why my alternative solution in the\nprevious message was to see the type of $src and push commits to local\nbranches, exactly because \"where else do you push\".  \n\nIOW, it's between \"prevent push with dubious $src from happening in the\nfirst place\" vs \"give them rope\".  Historically I always sided with the\nlatter camp, but I am trying to play a devil's advocate for a change ;-).\n"},{"id":"126070","messageId":"20091028000104.GA9426@sigill.intra.peff.net","threadId":"21363","inReplyTo":"7vaazc31sj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: support remote branches in guess_ref DWIM","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-28T00:01:04Z","receivedAt":"2009-10-28T00:01:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 27, 2009 at 03:33:16PM -0700, Junio C Hamano wrote:\n\n> >   $ git fetch ;# presumably gets origin/branch\n> >   $ git push origin/branch:renamed-branch\n> >\n> > which is much nicer than exposing clueless users to\n> > \":refs/heads/renamed-branch\".\n> \n> You would need to expose \":refs/heads/branch\" to make this a rename, not a\n> copy, wouldn't you?\n\nYeah, you're right. This was based on an actual user request, and I\ndidn't think too closely about the other steps. But since the deletion\nis of an existing branch, you should be able to do that without\nrefs/heads. So:\n\n  $ git push origin origin/branch:renamed-branch\n  $ git push origin :branch\n\nWhich of course you could do in one command if you wanted to live\n(more) dangerously.\n\n> > Am I missing some part of your argument?\n> \n> I do not see much point (other than your \"rename\" example) in pushing what\n> you got from the other end without changing anything yourself back to the\n> same remote.\n\nI don't either; my hope was that we can make that case a little bit\neasier without creating undue hardship for anybody else.\n\n> There was a thread in which people argued that the primary thing that is\n> dangerous in this sequence\n> \n>     $ git checkout origin/next; work-commit; work-commit; ...\n> \n> is when you leave the detached HEAD state without saving it to a local\n> branch.  I think what is more dangerous is that it will not give the user\n> a solid understanding that these commits do _not_ change origin/next in\n> any way.  After doing the above, it is understandable that a novice would\n> mistakenly think: \"I started from origin/next and built some, let's push\n> the result\".\n\nI suppose it's possible. I don't have any evidence that users actually\nthink that way.\n\n> With such a misconception, you will see the likely mistake here, too:\n> \n>     $ git push origin origin/next:refs/heads/next\n> \n> If \"rename\" is the _only_ valid reason you might want to push what you got\n> from there back to the same remote, _and_ if \"rename\" is something very\n> often needed, I think we would prefer to have a way to support that\n> operation directly, instead of more general DWIM that would risk passing\n> mistakes like the above unwarned.\n\nOK, I can buy that. It would be much nicer even to support explicit\nrenaming (in fact, the user request started with that, and I just didn't\nwant to give them an answer that involved refs/heads/, which I think is\nunnecessarily scary to users).\n\n> IOW, it's between \"prevent push with dubious $src from happening in the\n> first place\" vs \"give them rope\".  Historically I always sided with the\n> latter camp, but I am trying to play a devil's advocate for a change ;-).\n\nHeh. Thanks for explaining your thinking.\n\nLet's scrap this for now, then. Remote rename doesn't actually come up\nvery often, and I agree that it would be nice to have an actual atomic\nmovement, which is what people really want.\n\n-Peff\n"}]}