{"thread":{"id":"46029","subject":"[Bug] cloning a repository with a default MASTER branch tries to check out the master branch","startedAt":"2017-05-22T21:57:45Z","lastAt":"2017-05-31T04:43:57Z","messageCount":17,"participants":["Félix Saparelli","Junio C Hamano","Samuel Lijin","Jeff King","Philip Oakley"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"320485","messageId":"CACQm2Y1QtKD3M6weNhGrAQSLV8hLF4pKcpHDD7iUc78aWrt6Cw@mail.gmail.com","threadId":"46029","inReplyTo":null,"subject":"[Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Félix Saparelli","fromEmail":"felix@passcod.name","sentAt":"2017-05-22T21:42:12Z","receivedAt":"2017-05-22T21:57:45Z","isPatch":false,"sender":{"key":"felix@passcod.name","avatar":null},"body":"Hi,\n\nI created a git repository that, for joke reasons, has a single branch\ncalled MASTER (in uppercase). Upon cloning this repo, git attempts to\ncheckout the master branch (in lowercase), which does not exist.\nChecking out the MASTER branch manually afterwards works.\n\n$ git clone git@github.com:passcod/UPPERCASE-NPM.git\nCloning into 'UPPERCASE-NPM'...\nremote: Counting objects: 14, done.\nremote: Compressing objects: 100% (11/11), done.\nremote: Total 14 (delta 3), reused 14 (delta 3), pack-reused 0\nReceiving objects: 100% (14/14), done.\nResolving deltas: 100% (3/3), done.\nwarning: remote HEAD refers to nonexistent ref, unable to checkout.\n\n$ cd UPPERCASE-NPM\n$ ls -a\n. .. .git\n$ git branch\n$ git checkout MASTER\nBranch MASTER set up to track remote branch MASTER from origin.\nSwitched to a new branch 'MASTER'\n$ ls -a\n. .. .git NPM package.json README\n$ git branch\n* MASTER\n\nSome platform information:\n\n$ git version\ngit version 2.12.2\n\n$ uname -a\nLinux felix-probook 4.10.13-1-ARCH #1 SMP PREEMPT Thu Apr 27 12:15:09\nCEST 2017 x86_64 GNU/Linux\n\nGit was installed from the default Arch Linux package.\n\nThanks,\nFélix\n\n"},{"id":"320518","messageId":"xmqqa864mea3.fsf@gitster.mtv.corp.google.com","threadId":"46029","inReplyTo":"CACQm2Y1QtKD3M6weNhGrAQSLV8hLF4pKcpHDD7iUc78aWrt6Cw@mail.gmail.com","subject":"Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-23T03:40:36Z","receivedAt":"2017-05-23T03:40:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Félix Saparelli writes:\n\n> I created a git repository that, for joke reasons, has a single branch\n> called MASTER (in uppercase). Upon cloning this repo, git attempts to\n> checkout the master branch (in lowercase), which does not exist.\n\nSee what Git told you carefully and you can guess, I think.\n\n> $ git clone git@github.com:passcod/UPPERCASE-NPM.git\n> Cloning into 'UPPERCASE-NPM'...\n> remote: Counting objects: 14, done.\n> remote: Compressing objects: 100% (11/11), done.\n> remote: Total 14 (delta 3), reused 14 (delta 3), pack-reused 0\n> Receiving objects: 100% (14/14), done.\n> Resolving deltas: 100% (3/3), done.\n> warning: remote HEAD refers to nonexistent ref, unable to checkout.\n\nSo you have MASTER but not master, but your HEAD still is pointing\nat non-existing master. As HEAD is the way the remote tells the\nclone what the default branch to be checked out is, the command\nreports \"we cannot do a checkout of the default branch.\"\n\n"},{"id":"320531","messageId":"CAJZjrdWXsuKrFzedLvyE-1mvD0RC5XYDkmvV3wRNWXjFAVt4iQ@mail.gmail.com","threadId":"46029","inReplyTo":"CACQm2Y1QtKD3M6weNhGrAQSLV8hLF4pKcpHDD7iUc78aWrt6Cw@mail.gmail.com","subject":"Re: [Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-23T08:01:06Z","receivedAt":"2017-05-23T08:01:52Z","isPatch":false,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"For some reason the repo on GH does not have a HEAD pointer:\n\n$ git ls-remote https://github.com/passcod/UPPERCASE-NPM.git\nefc7dbfd6ca155d5d19ce67eb98603896062f35a refs/heads/MASTER\ne60ea8e6ec45ec45ff44ac8939cb4105b16477da refs/pull/1/head\nf35a73dcb151d336dc3d30c9a2c7423ecdb7bd1c refs/pull/2/head\n0d9b3a1268ff39350e04a7183af0add912b686e6 refs/tags/V1.0.0\nefc7dbfd6ca155d5d19ce67eb98603896062f35a refs/tags/V1.0.1\n\nI'm not sure how you managed to do that, since GH rejects attempts to\ndelete the current branch, but I believe if you set the default branch\nto MASTER it will work correctly.\n\nOn Mon, May 22, 2017 at 5:42 PM, Félix Saparelli wrote:\n> Hi,\n>\n> I created a git repository that, for joke reasons, has a single branch\n> called MASTER (in uppercase). Upon cloning this repo, git attempts to\n> checkout the master branch (in lowercase), which does not exist.\n> Checking out the MASTER branch manually afterwards works.\n>\n> $ git clone git@github.com:passcod/UPPERCASE-NPM.git\n> Cloning into 'UPPERCASE-NPM'...\n> remote: Counting objects: 14, done.\n> remote: Compressing objects: 100% (11/11), done.\n> remote: Total 14 (delta 3), reused 14 (delta 3), pack-reused 0\n> Receiving objects: 100% (14/14), done.\n> Resolving deltas: 100% (3/3), done.\n> warning: remote HEAD refers to nonexistent ref, unable to checkout.\n>\n> $ cd UPPERCASE-NPM\n> $ ls -a\n> . .. .git\n> $ git branch\n> $ git checkout MASTER\n> Branch MASTER set up to track remote branch MASTER from origin.\n> Switched to a new branch 'MASTER'\n> $ ls -a\n> . .. .git NPM package.json README\n> $ git branch\n> * MASTER\n>\n> Some platform information:\n>\n> $ git version\n> git version 2.12.2\n>\n> $ uname -a\n> Linux felix-probook 4.10.13-1-ARCH #1 SMP PREEMPT Thu Apr 27 12:15:09\n> CEST 2017 x86_64 GNU/Linux\n>\n> Git was installed from the default Arch Linux package.\n>\n> Thanks,\n> Félix\n>\n"},{"id":"320554","messageId":"20170523121159.zxxp43o6es7lzcj3@sigill.intra.peff.net","threadId":"46029","inReplyTo":"CAJZjrdWXsuKrFzedLvyE-1mvD0RC5XYDkmvV3wRNWXjFAVt4iQ@mail.gmail.com","subject":"Re: [Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-23T12:12:00Z","receivedAt":"2017-05-23T12:12:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 23, 2017 at 04:01:06AM -0400, Samuel Lijin wrote:\n\n> For some reason the repo on GH does not have a HEAD pointer:\n> \n> $ git ls-remote https://github.com/passcod/UPPERCASE-NPM.git\n> efc7dbfd6ca155d5d19ce67eb98603896062f35a refs/heads/MASTER\n> e60ea8e6ec45ec45ff44ac8939cb4105b16477da refs/pull/1/head\n> f35a73dcb151d336dc3d30c9a2c7423ecdb7bd1c refs/pull/2/head\n> 0d9b3a1268ff39350e04a7183af0add912b686e6 refs/tags/V1.0.0\n> efc7dbfd6ca155d5d19ce67eb98603896062f35a refs/tags/V1.0.1\n> \n> I'm not sure how you managed to do that, since GH rejects attempts to\n> delete the current branch, but I believe if you set the default branch\n> to MASTER it will work correctly.\n\nThe HEAD branch on the server side is still pointing at\nrefs/heads/master. But since that doesn't exist, upload-pack doesn't\nadvertise HEAD at all (it has no sha1).\n\nYou can recreate this situation with:\n\n cd /some/repo\n git init --bare dst.git\n git push dst.git HEAD:refs/heads/MASTER\n git ls-remote dst.git\n\nNo current-branch deletion required.\n\nFélix should be able to re-point the server-side HEAD to MASTER via\nGitHub's web interface.\n\nGitHub's post-receive hook usually does that automatically if you push a\nbranch to a repository with an unborn HEAD. But I think there may be\ncorner cases where it gets confused.\n\n-Peff\n"},{"id":"320603","messageId":"76BD8B6A1511438B8CCB79C616F3BC5B@PhilipOakley","threadId":"46029","inReplyTo":"xmqqa864mea3.fsf@gitster.mtv.corp.google.com","subject":"Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2017-05-23T23:24:52Z","receivedAt":"2017-05-23T23:25:02Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" \n> Félix Saparelli writes:\n>\n>> I created a git repository that, for joke reasons, has a single branch\n>> called MASTER (in uppercase). Upon cloning this repo, git attempts to\n>> checkout the master branch (in lowercase), which does not exist.\n>\n> See what Git told you carefully and you can guess, I think.\n\nguessing is rarely a good user experience ;-) however...\n>\n>> $ git clone git@github.com:passcod/UPPERCASE-NPM.git\n>> Cloning into 'UPPERCASE-NPM'...\n>> remote: Counting objects: 14, done.\n>> remote: Compressing objects: 100% (11/11), done.\n>> remote: Total 14 (delta 3), reused 14 (delta 3), pack-reused 0\n>> Receiving objects: 100% (14/14), done.\n>> Resolving deltas: 100% (3/3), done.\n>> warning: remote HEAD refers to nonexistent ref, unable to checkout.\n\nPerhaps here the warning message could include the value of the ref provided \nby the remote's HEAD which would then more clearly indicate to the user what \nwas expected.\n\nI haven't had chance to look at how easy that maybe in the code - perhaps a \nbit of low hanging fruit for someone?\n\n>\n> So you have MASTER but not master, but your HEAD still is pointing\n> at non-existing master. As HEAD is the way the remote tells the\n> clone what the default branch to be checked out is, the command\n> reports \"we cannot do a checkout of the default branch.\"\n>\n--\nPhilip \n\n"},{"id":"320661","messageId":"20170524141947.2gguzcvyu6lch373@sigill.intra.peff.net","threadId":"46029","inReplyTo":"76BD8B6A1511438B8CCB79C616F3BC5B@PhilipOakley","subject":"Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-24T14:19:48Z","receivedAt":"2017-05-24T14:19:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 24, 2017 at 12:24:52AM +0100, Philip Oakley wrote:\n\n> > > $ git clone git@github.com:passcod/UPPERCASE-NPM.git\n> > > Cloning into 'UPPERCASE-NPM'...\n> > > remote: Counting objects: 14, done.\n> > > remote: Compressing objects: 100% (11/11), done.\n> > > remote: Total 14 (delta 3), reused 14 (delta 3), pack-reused 0\n> > > Receiving objects: 100% (14/14), done.\n> > > Resolving deltas: 100% (3/3), done.\n> > > warning: remote HEAD refers to nonexistent ref, unable to checkout.\n> \n> Perhaps here the warning message could include the value of the ref provided\n> by the remote's HEAD which would then more clearly indicate to the user what\n> was expected.\n> \n> I haven't had chance to look at how easy that maybe in the code - perhaps a\n> bit of low hanging fruit for someone?\n\nUnfortunately, it can't, because the ref doesn't exist:\n\n $ git ls-remote git://github.com/passcod/UPPERCASE-NPM.git\n efc7dbfd6ca155d5d19ce67eb98603896062f35a\trefs/heads/MASTER\n e60ea8e6ec45ec45ff44ac8939cb4105b16477da\trefs/pull/1/head\n f35a73dcb151d336dc3d30c9a2c7423ecdb7bd1c\trefs/pull/2/head\n 0d9b3a1268ff39350e04a7183af0add912b686e6\trefs/tags/V1.0.0\n efc7dbfd6ca155d5d19ce67eb98603896062f35a\trefs/tags/V1.0.1\n\nThere is no HEAD line at all, so we have no information about it on the\nclient side. Likewise, if you run with GIT_TRACE_PACKET=1, you'll see\nthat the capabilities line does not include a symref marker either.\n\nSo if we wanted to improve this, I think the first step would be for the\nserver to start sending symref lines for HEAD, even when it does not\nresolve to anything.\n\nThat would also make a case like this:\n\n git -C dst.git symbolic-ref refs/heads/does-not-exist\n git clone dst.git local\n\nuse \"does-not-exist\" as the default branch name in our local clone\n(rather than just falling back to \"master\", which presumably the other\nside never plans to use).\n\n-Peff\n"},{"id":"320685","messageId":"xmqqshjtg1kh.fsf@gitster.mtv.corp.google.com","threadId":"46029","inReplyTo":"20170524141947.2gguzcvyu6lch373@sigill.intra.peff.net","subject":"Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-25T01:36:14Z","receivedAt":"2017-05-25T01:36:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Unfortunately, it can't, because the ref doesn't exist:\n>\n> $ git ls-remote git://github.com/passcod/UPPERCASE-NPM.git\n> efc7dbfd6ca155d5d19ce67eb98603896062f35a\trefs/heads/MASTER\n> e60ea8e6ec45ec45ff44ac8939cb4105b16477da\trefs/pull/1/head\n> f35a73dcb151d336dc3d30c9a2c7423ecdb7bd1c\trefs/pull/2/head\n> 0d9b3a1268ff39350e04a7183af0add912b686e6\trefs/tags/V1.0.0\n> efc7dbfd6ca155d5d19ce67eb98603896062f35a\trefs/tags/V1.0.1\n>\n> There is no HEAD line at all, so we have no information about it on the\n> client side. Likewise, if you run with GIT_TRACE_PACKET=1, you'll see\n> that the capabilities line does not include a symref marker either.\n>\n> So if we wanted to improve this, I think the first step would be for the\n> server to start sending symref lines for HEAD, even when it does not\n> resolve to anything.\n\nYup, noticed the same and I agree with your conclusion.\n"},{"id":"320689","messageId":"xmqqa861fx34.fsf@gitster.mtv.corp.google.com","threadId":"46029","inReplyTo":"xmqqshjtg1kh.fsf@gitster.mtv.corp.google.com","subject":"Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-25T03:13:03Z","receivedAt":"2017-05-25T03:13:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> Unfortunately, it can't, because the ref doesn't exist:\n>>\n>> $ git ls-remote git://github.com/passcod/UPPERCASE-NPM.git\n>> efc7dbfd6ca155d5d19ce67eb98603896062f35a\trefs/heads/MASTER\n>> e60ea8e6ec45ec45ff44ac8939cb4105b16477da\trefs/pull/1/head\n>> f35a73dcb151d336dc3d30c9a2c7423ecdb7bd1c\trefs/pull/2/head\n>> 0d9b3a1268ff39350e04a7183af0add912b686e6\trefs/tags/V1.0.0\n>> efc7dbfd6ca155d5d19ce67eb98603896062f35a\trefs/tags/V1.0.1\n>>\n>> There is no HEAD line at all, so we have no information about it on the\n>> client side. Likewise, if you run with GIT_TRACE_PACKET=1, you'll see\n>> that the capabilities line does not include a symref marker either.\n>>\n>> So if we wanted to improve this, I think the first step would be for the\n>> server to start sending symref lines for HEAD, even when it does not\n>> resolve to anything.\n>\n> Yup, noticed the same and I agree with your conclusion.\n\nWe probably should make head_ref_namespaced() to take the\nresolve_flags that is passed down thru refs_read_ref_full() down to\nrefs_resolve_ref_unsafe(). For the purpose of the first call to it\nin upload-pack.c to call back find_symref(), we do not need and want\nto say RESOLVE_REF_READING (which requires a symref to be pointing\nat an existing ref). I suspect all the other calls (there are 2\nother in unload-pack.c and yet another in http-backend.c) to the\nfunction should keep passing RESOLVE_REF_READING, as they do want to\nomit a symbolic ref that points at an unborn branch.\n\n"},{"id":"320715","messageId":"20170525155924.hk5jskennph6tta3@sigill.intra.peff.net","threadId":"46029","inReplyTo":"xmqqa861fx34.fsf@gitster.mtv.corp.google.com","subject":"Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-25T15:59:24Z","receivedAt":"2017-05-25T15:59:44Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 25, 2017 at 12:13:03PM +0900, Junio C Hamano wrote:\n\n> >> So if we wanted to improve this, I think the first step would be for the\n> >> server to start sending symref lines for HEAD, even when it does not\n> >> resolve to anything.\n> >\n> > Yup, noticed the same and I agree with your conclusion.\n> \n> We probably should make head_ref_namespaced() to take the\n> resolve_flags that is passed down thru refs_read_ref_full() down to\n> refs_resolve_ref_unsafe(). For the purpose of the first call to it\n> in upload-pack.c to call back find_symref(), we do not need and want\n> to say RESOLVE_REF_READING (which requires a symref to be pointing\n> at an existing ref). I suspect all the other calls (there are 2\n> other in unload-pack.c and yet another in http-backend.c) to the\n> function should keep passing RESOLVE_REF_READING, as they do want to\n> omit a symbolic ref that points at an unborn branch.\n\nThat would make head_ref() not function-pointer compatible with all the\nother for_each_ref functions. I don't know how much that matters. The\nrevision.c parser does use function pointers, but it doesn't handle HEAD\nspecially.\n\nSo I kind of wonder if that code should simply be calling\nresolve_ref_unsafe() itself in the first place. We know the only value\nit's going to get is HEAD.\n\nOTOH, it's possible that we would eventually want to report all symrefs,\nincluding ones we find while traversing the refs. And in that sense, all\nof the for_each_ref functions would want to learn about this. But for\nref advertisement I think that would need a protocol change anyway, so\nI'm not sure it's worth worrying about.\n\nThe just-HEAD case could look like:\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 97da13e6a..04a913ad1 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -959,28 +959,25 @@ static int send_ref(const char *refname, const struct object_id *oid,\n \treturn 0;\n }\n \n-static int find_symref(const char *refname, const struct object_id *oid,\n-\t\t int flag, void *cb_data)\n+static void find_symref(const char *refname, struct string_list *out)\n {\n \tconst char *symref_target;\n \tstruct string_list_item *item;\n \tstruct object_id unused;\n+\tint flag;\n \n-\tif ((flag & REF_ISSYMREF) == 0)\n-\t\treturn 0;\n \tsymref_target = resolve_ref_unsafe(refname, 0, unused.hash, &flag);\n \tif (!symref_target || (flag & REF_ISSYMREF) == 0)\n-\t\tdie(\"'%s' is a symref but it is not?\", refname);\n-\titem = string_list_append(cb_data, refname);\n+\t\treturn;\n+\titem = string_list_append(out, refname);\n \titem->util = xstrdup(symref_target);\n-\treturn 0;\n }\n \n static void upload_pack(void)\n {\n \tstruct string_list symref = STRING_LIST_INIT_DUP;\n \n-\thead_ref_namespaced(find_symref, &symref);\n+\tfind_symref(\"HEAD\", &symref);\n \n \tif (advertise_refs || !stateless_rpc) {\n \t\treset_timeout();\n"},{"id":"320728","messageId":"20170525191115.tqd6zlj5mxqls4wp@sigill.intra.peff.net","threadId":"46029","inReplyTo":"20170525155924.hk5jskennph6tta3@sigill.intra.peff.net","subject":"Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-25T19:11:15Z","receivedAt":"2017-05-25T19:11:23Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 25, 2017 at 11:59:24AM -0400, Jeff King wrote:\n\n> The just-HEAD case could look like:\n\nThis patch does work, in the sense that upload-pack advertises the\nunborn HEAD symref. But the client doesn't actually do anything with it.\nThe capability parsing happens in get_remote_heads(), which passes the\ndata out by adding an annotation to the \"struct ref\" list. But of course\nwe have no HEAD ref to annotate.\n\nSo either get_remote_heads() would have to start returning a bogus HEAD\nref (with a null sha1, I guess, which all callers would have to\nrecognize). Or clone (and probably \"remote set-head -a\") would have to\nstart reaching across the transport-module boundary and asking for any\nsymref values for \"HEAD\". I'm not excited about more special-casing of\n\"HEAD\", though. In theory we'd want this for other symrefs in the long\nrun, and it would be nice if clients were ready to handle that (even if\nthe protocol isn't quite there).\n\nI dunno. I was thinking there might be a quick tweak, but I'm wondering\nif this arcane case is worth the restructuring we'd have to do to\nsupport it. It only comes up when you've moved the server repo's HEAD to\nan unborn branch _and_ you have other refs (since otherwise we don't\neven send capabilities at all!).\n\n-Peff\n"},{"id":"320774","messageId":"xmqqy3tkcy9i.fsf@gitster.mtv.corp.google.com","threadId":"46029","inReplyTo":"20170525191115.tqd6zlj5mxqls4wp@sigill.intra.peff.net","subject":"Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-25T23:28:09Z","receivedAt":"2017-05-25T23:28:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I dunno. I was thinking there might be a quick tweak, but I'm wondering\n> if this arcane case is worth the restructuring we'd have to do to\n> support it. It only comes up when you've moved the server repo's HEAD to\n> an unborn branch _and_ you have other refs (since otherwise we don't\n> even send capabilities at all!).\n\nThanks for digging. You made me to start doubting it is worth\ndoing, too.\n"},{"id":"320849","messageId":"CEF93B59EA3F49608B41892952454B1D@PhilipOakley","threadId":"46029","inReplyTo":"20170525191115.tqd6zlj5mxqls4wp@sigill.intra.peff.net","subject":"Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2017-05-26T20:00:57Z","receivedAt":"2017-05-26T20:01:07Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"been trying to keep up...\n\nFrom: \"Jeff King\" \n> On Thu, May 25, 2017 at 11:59:24AM -0400, Jeff King wrote:\n>\n>> The just-HEAD case could look like:\n>\n> This patch does work, in the sense that upload-pack advertises the\n> unborn HEAD symref. But the client doesn't actually do anything with it.\n> The capability parsing happens in get_remote_heads(), which passes the\n> data out by adding an annotation to the \"struct ref\" list. But of course\n> we have no HEAD ref to annotate.\n>\n> So either get_remote_heads() would have to start returning a bogus HEAD\n> ref (with a null sha1, I guess, which all callers would have to\n> recognize). Or clone (and probably \"remote set-head -a\") would have to\n> start reaching across the transport-module boundary and asking for any\n> symref values for \"HEAD\". I'm not excited about more special-casing of\n> \"HEAD\", though. In theory we'd want this for other symrefs in the long\n> run, and it would be nice if clients were ready to handle that (even if\n> the protocol isn't quite there).\n>\n> I dunno. I was thinking there might be a quick tweak, but I'm wondering\n> if this arcane case is worth the restructuring we'd have to do to\n> support it. It only comes up when you've moved the server repo's HEAD to\n> an unborn branch _and_ you have other refs (since otherwise we don't\n> even send capabilities at all!).\n>\n> -Peff\n\nMy original comment regarding Felix's report was based on when I was looking \nat the bundle code's disambiguation of refs which matched HEAD's sha1. Hence \nI had a mis-remembered impression that the HEAD - symref matching was \navaibable.\n\nAt that time, Junio had suggested that, at least in the bundle file, the \nHEAD symref could be advertised immediately after a nul on the ref line.\n\nAt least for regular git, the sha1and symref value would included in the \nread line, and the current string processing [1] would not notice the extra \nsymref data. This extra data could then be read (if present) from the end of \nthe line, and the HEAD symref set.\n\nWhat I then noticed (but didn't report to the list) was the option of adding \nthat extra info to the PKLINE protocol.\n\n\nIn technical\\pack-protocol.txt #L136;158-160\nReference Discovery:\n\nIf HEAD is a valid ref, HEAD MUST appear as the first advertised\nref. If HEAD is not a valid ref, HEAD MUST NOT appear in the\nadvertisement list at all, but other refs may still appear.\n-\n\nSo, (for both upload pack, and bundle refs) the place to hide the HEAD is \nafter the later ref that HEAD points to.\n e.g.(updating the example at #L147):\n 00441d3fcd5ced445d1abc402225c0b8a1299641f497 \nrefs/heads/integration\\0HEAD[LF]\n\nThe potential issue is if there is a passed ref that is HEAD, but that HEAD \nitself isn't passed (especially for bundle)\n<\\from my notes>\n--\n\nHowever given the discussion about an unborn HEAD, the option here would be \nto also pass the NULL sha for the symref and then add the annotation 'HEAD' \nafter an extra \\0, in the same way that an active symref could be annotated \nwith the '\\0HEAD'. This would kill two birds with one stone!\n\nThese are still protocol changes but should squeeze into the existing \nprocessing using the \\0 trick.\n\nIn the absence of the information, and the multi-use of the warning \nfunction, the current message is possible the best we can get.\n\nPhilip\n\n[1] the question would be whether other git versions also use the same \nstring processing so could be 'fooled' in the same way? I'd be interested to \nknow if that was a possibility.\n\n\n\n"},{"id":"320850","messageId":"FC1CD6DCF3C541CD90DAD51559E1CB64@PhilipOakley","threadId":"46029","inReplyTo":"CEF93B59EA3F49608B41892952454B1D@PhilipOakley","subject":"Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2017-05-26T21:17:11Z","receivedAt":"2017-05-27T01:12:00Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"adding and updating an example..\nFrom: \"Philip Oakley\" \n> been trying to keep up...\n>\n> From: \"Jeff King\" \n>> On Thu, May 25, 2017 at 11:59:24AM -0400, Jeff King wrote:\n>>\n>>> The just-HEAD case could look like:\n>>\n>> This patch does work, in the sense that upload-pack advertises the\n>> unborn HEAD symref. But the client doesn't actually do anything with it.\n>> The capability parsing happens in get_remote_heads(), which passes the\n>> data out by adding an annotation to the \"struct ref\" list. But of course\n>> we have no HEAD ref to annotate.\n>>\n>> So either get_remote_heads() would have to start returning a bogus HEAD\n>> ref (with a null sha1, I guess, which all callers would have to\n>> recognize). Or clone (and probably \"remote set-head -a\") would have to\n>> start reaching across the transport-module boundary and asking for any\n>> symref values for \"HEAD\". I'm not excited about more special-casing of\n>> \"HEAD\", though. In theory we'd want this for other symrefs in the long\n>> run, and it would be nice if clients were ready to handle that (even if\n>> the protocol isn't quite there).\n>>\n>> I dunno. I was thinking there might be a quick tweak, but I'm wondering\n>> if this arcane case is worth the restructuring we'd have to do to\n>> support it. It only comes up when you've moved the server repo's HEAD to\n>> an unborn branch _and_ you have other refs (since otherwise we don't\n>> even send capabilities at all!).\n>>\n>> -Peff\n>\n> My original comment regarding Felix's report was based on when I was \n> looking at the bundle code's disambiguation of refs which matched HEAD's \n> sha1. Hence I had a mis-remembered impression that the HEAD - symref \n> matching was avaibable.\n>\n> At that time, Junio had suggested that, at least in the bundle file, the \n> HEAD symref could be advertised immediately after a nul on the ref line.\n>\n> At least for regular git, the sha1and symref value would included in the \n> read line, and the current string processing [1] would not notice the \n> extra symref data. This extra data could then be read (if present) from \n> the end of the line, and the HEAD symref set.\n>\n> What I then noticed (but didn't report to the list) was the option of \n> adding that extra info to the PKLINE protocol.\n>\n> \n> In technical\\pack-protocol.txt #L136;158-160\n> Reference Discovery:\n>\n> If HEAD is a valid ref, HEAD MUST appear as the first advertised\n> ref. If HEAD is not a valid ref, HEAD MUST NOT appear in the\n> advertisement list at all, but other refs may still appear.\n> -\n>\n> So, (for both upload pack, and bundle refs) the place to hide the HEAD is \n> after the later ref that HEAD points to.\n> e.g.(updating the example at #L147):\n> 00441d3fcd5ced445d1abc402225c0b8a1299641f497 \n> refs/heads/integration\\0HEAD[LF]\n>\n> The potential issue is if there is a passed ref that is HEAD, but that \n> HEAD itself isn't passed (especially for bundle)\n> <\\from my notes>\n> --\n>\n> However given the discussion about an unborn HEAD, the option here would \n> be to also pass the NULL sha for the symref and then add the annotation \n> 'HEAD' after an extra \\0, in the same way that an active symref could be \n> annotated with the '\\0HEAD'. This would kill two birds with one stone!\n>\n> These are still protocol changes but should squeeze into the existing \n> processing using the \\0 trick.\n>\n> In the absence of the information, and the multi-use of the warning \n> function, the current message is possible the best we can get.\n>\n> Philip\n>\n> [1] the question would be whether other git versions also use the same \n> string processing so could be 'fooled' in the same way? I'd be interested \n> to know if that was a possibility.\n>\nUpdating the original example with the suggestion of adding the unborn ref \nand a \\0HEAD marker (sort order may be an issue if it is the first entry \nwhich 'clashes' with the capability string... - I've been lenient here)\n\n $ git ls-remote git://github.com/passcod/UPPERCASE-NPM.git\n efc7dbfd6ca155d5d19ce67eb98603896062f35a refs/heads/MASTER\n 0000000000000000000000000000000000000000 refs/heads/master\\0HEAD\n e60ea8e6ec45ec45ff44ac8939cb4105b16477da refs/pull/1/head\n f35a73dcb151d336dc3d30c9a2c7423ecdb7bd1c refs/pull/2/head\n 0d9b3a1268ff39350e04a7183af0add912b686e6 refs/tags/V1.0.0\n efc7dbfd6ca155d5d19ce67eb98603896062f35a refs/tags/V1.0.1\n\n--\nPhilip\n\n"},{"id":"320875","messageId":"xmqqh905c0tm.fsf@gitster.mtv.corp.google.com","threadId":"46029","inReplyTo":"CEF93B59EA3F49608B41892952454B1D@PhilipOakley","subject":"Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-27T23:55:01Z","receivedAt":"2017-05-27T23:55:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> However given the discussion about an unborn HEAD, the option here\n> would be to also pass the NULL sha for the symref and then add the\n> annotation 'HEAD' after an extra \\0, in the same way that an active\n> symref could be annotated with the '\\0HEAD'. This would kill two birds\n> with one stone!\n\nAre you aware of the symref capability that is already advertised in\nthe initial upload-pack response? Right now, we do so only when\nHEAD actually points at something, and the earlier suggestion by\nPeff is to do so unconditionally, even when HEAD is dangling.\n\nExisting clients that are symref aware do not do anything (good or\nbad) when a HEAD that is dangling [*1*] is advertised, so such a\nchange will not hurt (but it would not help by itself either).\nAncient clients that are not even aware of the symref are not\naffected.\n\nThen new clients _could_ start paying attention to the advertised\nHEAD that is dangling.\n\nThe bundle transport is a different beast. I do not think it\nadvertises where HEAD is pointing at, whether it is dangling or\nnot.\n\n\n[Footnote]\n\n*1* A HEAD symref that is advertised in the upload-pack response is\n dangling when its pointee does not appear in the set of refs\n that are advertised. Félix's case would have shown HEAD\n pointing at refs/heads/master in the symref capability extension,\n but the list of refs and their values would not have included\n that ref (there was only refs/heads/MASTER \"for joke reasons\").\n"},{"id":"320884","messageId":"48CB2547397B471C99F54CFB3F113C3F@PhilipOakley","threadId":"46029","inReplyTo":"xmqqh905c0tm.fsf@gitster.mtv.corp.google.com","subject":"Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2017-05-28T11:21:50Z","receivedAt":"2017-05-28T11:22:00Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" \n> \"Philip Oakley\" writes:\n>\n>> However given the discussion about an unborn HEAD, the option here\n>> would be to also pass the NULL sha for the symref and then add the\n>> annotation 'HEAD' after an extra \\0, in the same way that an active\n>> symref could be annotated with the '\\0HEAD'. This would kill two birds\n>> with one stone!\n>\n> Are you aware of the symref capability that is already advertised in\n> the initial upload-pack response? Right now, we do so only when\n> HEAD actually points at something, and the earlier suggestion by\n> Peff is to do so unconditionally, even when HEAD is dangling.\n\nThe suggestion is the otherway around. I would argue (as a viewpoint) that \nwhat we advertise are object IDs and their associated refs, sorted by ref \nname. (I'm thinking of the git/Documentation/technical/pack-protocol.txt \nhere). My suggestion was that when we get to the sorted ref that HEAD points \nto (including the unborn oid) that we annotate that ref.\n\nI didn't quite follow Peff's suggestion as to where the list change went and \nif that was a protocol change.\n\nThere are two current fault scenarios.\na. The currently reported case where HEAD has an unborn symref\nb. The multiple ref HEAD case, where the HEAD oid matches multiple \nadvertised refs, and the correct symref is not the first listed (which is \nthe case I had looked at a few years ago, a prompted me to respond).\n\n\nWith the above discussions, we would have both a NULL oid for the unborn \n(sym)ref sent (if needed), and the (sym)ref for HEAD would have the extra \nannotation. That would at least not break the protocol rule that \"If HEAD is \nnot a valid ref, HEAD MUST NOT appear in the advertisement list at all\" (it \nis now an annotation to another valid ref [or the unborn symref]).\n>\n> Existing clients that are symref aware do not do anything (good or\n> bad) when a HEAD that is dangling [*1*] is advertised, so such a\n> change will not hurt (but it would not help by itself either).\n> Ancient clients that are not even aware of the symref are not\n> affected.\n>\n> Then new clients _could_ start paying attention to the advertised\n> HEAD that is dangling.\n\nMy main step was that for case b, so that we don't need to \nguess_remote_head(). The annotation would have already told us.\n\n>\n> The bundle transport is a different beast. I do not think it\n> advertises where HEAD is pointing at, whether it is dangling or\n> not.\n\nMy understanding was that the Bundle was a tiny wrapper and that it has that \nsame protocol header, which was then decoded using the same code. Hence the \nhope that it could fix both the bundle and remote clone problems. I'd just \navoided stepping into remote clone arena because of other implementations.\n>\n>\n> [Footnote]\n>\n> *1* A HEAD symref that is advertised in the upload-pack response is\n> dangling when its pointee does not appear in the set of refs\n> that are advertised. Félix's case would have shown HEAD\n> pointing at refs/heads/master in the symref capability extension,\n> but the list of refs and their values would not have included\n> that ref (there was only refs/heads/MASTER \"for joke reasons\").\n\nI hope I haven't confused the different parts of the negotiation and \ntransfer, which can be confusing.\n\nPhilip \n\n"},{"id":"320887","messageId":"xmqqtw459m0q.fsf@gitster.mtv.corp.google.com","threadId":"46029","inReplyTo":"48CB2547397B471C99F54CFB3F113C3F@PhilipOakley","subject":"Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-28T12:57:41Z","receivedAt":"2017-05-28T12:57:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> From: \"Junio C Hamano\" \n>> \"Philip Oakley\" writes:\n>>\n>>> However given the discussion about an unborn HEAD, the option here\n>>> would be to also pass the NULL sha for the symref and then add the\n>>> annotation 'HEAD' after an extra \\0, in the same way that an active\n>>> symref could be annotated with the '\\0HEAD'. This would kill two birds\n>>> with one stone!\n>>\n>> Are you aware of the symref capability that is already advertised in\n>> the initial upload-pack response? Right now, we do so only when\n>> HEAD actually points at something, and the earlier suggestion by\n>> Peff is to do so unconditionally, even when HEAD is dangling.\n>\n> The suggestion is the otherway around. I would argue (as a viewpoint)\n> that what we advertise are object IDs and their associated refs,\n> sorted by ref name. (I'm thinking of the\n> git/Documentation/technical/pack-protocol.txt here). My suggestion was\n\nThat's not the part of the protocol I explained Peff's suggestion to\nyou about. That's ref advertisement proper, and its first line has\na trailing NUL followed by \"protocol capability\" list. There is one\n\"capability\" that tells the receiver specifically about HEAD symref\n(if and only if HEAD is a symref). There are two reasons why the\ncurrent code does not help even though that necessary protocol bits\nare *already* there (i.e. you do not need any protocol extension).\nOne is that existing servers do not use the symref capability for\nHEAD if HEAD is pointing at an unborn branch (i.e. dangling). The\nother is that the existing code sitting on the receiving end is not\nprepared to handle one, even if the server end sent one.\n"},{"id":"321110","messageId":"20170531044349.bcb6jlte5gmo6p6v@sigill.intra.peff.net","threadId":"46029","inReplyTo":"48CB2547397B471C99F54CFB3F113C3F@PhilipOakley","subject":"Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-31T04:43:49Z","receivedAt":"2017-05-31T04:43:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, May 28, 2017 at 12:21:50PM +0100, Philip Oakley wrote:\n\n> > Are you aware of the symref capability that is already advertised in\n> > the initial upload-pack response? Right now, we do so only when\n> > HEAD actually points at something, and the earlier suggestion by\n> > Peff is to do so unconditionally, even when HEAD is dangling.\n> \n> The suggestion is the otherway around. I would argue (as a viewpoint) that\n> what we advertise are object IDs and their associated refs, sorted by ref\n> name. (I'm thinking of the git/Documentation/technical/pack-protocol.txt\n> here). My suggestion was that when we get to the sorted ref that HEAD points\n> to (including the unborn oid) that we annotate that ref.\n\nIt's hard to do that in a backwards-compatible way. The reason the\nsymref capability works like it does (and supports only HEAD) is that\nthe bits after the NUL are treated as a capability string, and older\nclients will actually _replace_ any earlier capability string they saw\nwith the new one. So:\n\n 1234abcd refs/heads/a\\0symref=refs/heads/target1\n 5678abcd refs/heads/b\\0symref=refs/heads/target2\n\ndoesn't work as you'd like.\n\nBut even if it did, I don't think that solves the dangling symref\nproblem by itself. As you note, we'd need to advertise a null sha1 line\nand annotate that. I didn't test, but I'd suspect that's another\ncompatibility problem; clients will probably choke on the null sha1.\n\n> I didn't quite follow Peff's suggestion as to where the list change\n> went and if that was a protocol change.\n\nThe current protocol just advertises symref=source:target in the\ncapability line, where \"source\" does not have to be the ref that is\ncurrently being advertised at all. So you are free to do:\n\n 1234abcd refs/heads/unrelated\\0...symref=HEAD:refs/heads/target\n\nBut the server does not bother to do so when the HEAD symref doesn't\npoint to anything. We could fix that without changing the protocol\nsyntax. It's possible that some picky client would complain that we\nmentioned the HEAD symref even though it was not advertised, but\ncertainly older git.git clients are fine with it. They'd still need a\nclient-side update in order to do something useful with it, but at least\nits presence would not make older clients behave any worse.\n\n-Peff\n"}]}