{"thread":{"id":"46310","subject":"Should \"head\" also work for \"HEAD\" on case-insensitive FS?","startedAt":"2017-07-03T22:00:58Z","lastAt":"2017-07-27T15:26:42Z","messageCount":11,"participants":["Ævar Arnfjörð Bjarmason","Konstantin Khomoutov","Jeff King","Junio C Hamano","Kenneth Hsu","Michael Haggerty"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"323787","messageId":"87ziclb2pa.fsf@gmail.com","threadId":"46310","inReplyTo":null,"subject":"Should \"head\" also work for \"HEAD\" on case-insensitive FS?","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-07-03T22:00:49Z","receivedAt":"2017-07-03T22:00:58Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"I don't have a OSX box, but was helping a co-worker over Jabber the\nother day, and he pasted something like:\n\n    $ git merge-base github/master head\n\nWhich didn't work for me, and I thought he had a local \"head\" branch\nuntil realizing that of course we were just resolving HEAD on the FS.\n\nHas this come up before? I think it makes sense to warn/error about\nthese magic /HEAD/ revisions if they're not upper-case.\n\nThis is likely unintentional and purely some emergent effect of how it's\nimplemented, and leads to unportable git invocations.\n"},{"id":"323795","messageId":"20170704071909.phs4bf5ybdord2lv@tigra","threadId":"46310","inReplyTo":"87ziclb2pa.fsf@gmail.com","subject":"Re: Should \"head\" also work for \"HEAD\" on case-insensitive FS?","fromName":"Konstantin Khomoutov","fromEmail":"kostix+git@007spb.ru","sentAt":"2017-07-04T07:19:09Z","receivedAt":"2017-07-04T07:19:17Z","isPatch":false,"sender":{"key":"kostix+git@007spb.ru","avatar":null},"body":"On Tue, Jul 04, 2017 at 12:00:49AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> I don't have a OSX box, but was helping a co-worker over Jabber the\n> other day, and he pasted something like:\n> \n>     $ git merge-base github/master head\n> \n> Which didn't work for me, and I thought he had a local \"head\" branch\n> until realizing that of course we were just resolving HEAD on the FS.\n> \n> Has this come up before? I think it makes sense to warn/error about\n> these magic /HEAD/ revisions if they're not upper-case.\n> \n> This is likely unintentional and purely some emergent effect of how it's\n> implemented, and leads to unportable git invocations.\n\nJFTR this is one common case of confusion on Windows as well.\nTo the point that I saw people purposedly using \"head\" on StackOverflow\nquestions.  That is, they appear to think (for some reason) that\nbranches in Git have case-insensitive names and prefer to spell \"head\"\nsince it (supposedly) easier to type.\n\nI don't know what to do about it.\nIdeally we'd just have a way to perform a final check on the file into\nwhich a ref name was resolved to see its \"real\" name but I don't know\nwhether all popular filesystems are case preserving (HFS+ and NTFS are,\nIIRC) and even if they are, whether the appropriate platform-specific\nAPIs exists to perform such a check.\n\n"},{"id":"323797","messageId":"87van8boe9.fsf@gmail.com","threadId":"46310","inReplyTo":"20170704071909.phs4bf5ybdord2lv@tigra","subject":"Re: Should \"head\" also work for \"HEAD\" on case-insensitive FS?","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-07-04T08:24:30Z","receivedAt":"2017-07-04T08:24:39Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Jul 04 2017, Konstantin Khomoutov jotted:\n\n> On Tue, Jul 04, 2017 at 12:00:49AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> I don't have a OSX box, but was helping a co-worker over Jabber the\n>> other day, and he pasted something like:\n>>\n>>     $ git merge-base github/master head\n>>\n>> Which didn't work for me, and I thought he had a local \"head\" branch\n>> until realizing that of course we were just resolving HEAD on the FS.\n>>\n>> Has this come up before? I think it makes sense to warn/error about\n>> these magic /HEAD/ revisions if they're not upper-case.\n>>\n>> This is likely unintentional and purely some emergent effect of how it's\n>> implemented, and leads to unportable git invocations.\n>\n> JFTR this is one common case of confusion on Windows as well.\n> To the point that I saw people purposedly using \"head\" on StackOverflow\n> questions.  That is, they appear to think (for some reason) that\n> branches in Git have case-insensitive names and prefer to spell \"head\"\n> since it (supposedly) easier to type.\n>\n> I don't know what to do about it.\n> Ideally we'd just have a way to perform a final check on the file into\n> which a ref name was resolved to see its \"real\" name but I don't know\n> whether all popular filesystems are case preserving (HFS+ and NTFS are,\n> IIRC) and even if they are, whether the appropriate platform-specific\n> APIs exists to perform such a check.\n\nI think there's no easy way do this in the general case with the current\nref backend, because we rely on the FS to store the refs.\n\nBut I'm thinking of the more specific case where you specify\n{HEAD,FETCH_HEAD,ORIG_HEAD,MERGE_HEAD,CHERRY_PICK_HEAD} as non-upper\ncase, and we resolve it from .git/$NAME.\n\nSo the detection would not be checking whether the file on-disk has the\nsame casing, but knowing that if we resolve anything from .git/$NAME\nthen the string provided on the command-line must be upper-case.\n\nAlthough there is this:\n\n    $ git rev-parse HEAD\n    051ee1e7dd2c7b8bdc20f237eea3c7d5b1314280\n    $ git rev-parse WHATEVER\n    WHATEVER\n    fatal: ambiguous argument 'WHATEVER': unknown revision or path not in the working tree.\n    $ cp .git/{HEAD,WHATEVER}\n    $ git rev-parse WHATEVER\n    051ee1e7dd2c7b8bdc20f237eea3c7d5b1314280\n\nI.e. we allow any arbitrary ref sitting in .git/, but presumably we\ncould just record the original string the user provided so that this\ndies on OSX/Windows too:\n\n    $ cp .git/{HEAD,Whatever}\n    $ git rev-parse wHATEVER\n    wHATEVER\n    fatal: ambiguous argument 'wHATEVER': unknown revision or path not in the working tree.\n\nBut this may be a much deeper rabbit hole than I initially thought, I\nwas fishing to see if someone knew of a place in the code or WIP patch\nthat dealt with these special refs, but between the low-level machinery\n& sha1_name.c (and others) there may be no easy one place to do this...\n"},{"id":"323831","messageId":"20170705083611.jgxbp4sqogicfwdb@sigill.intra.peff.net","threadId":"46310","inReplyTo":"87van8boe9.fsf@gmail.com","subject":"Re: Should \"head\" also work for \"HEAD\" on case-insensitive FS?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-07-05T08:36:11Z","receivedAt":"2017-07-05T08:36:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 04, 2017 at 10:24:30AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> I.e. we allow any arbitrary ref sitting in .git/, but presumably we\n> could just record the original string the user provided so that this\n> dies on OSX/Windows too:\n> \n>     $ cp .git/{HEAD,Whatever}\n>     $ git rev-parse wHATEVER\n>     wHATEVER\n>     fatal: ambiguous argument 'wHATEVER': unknown revision or path not in the working tree.\n> \n> But this may be a much deeper rabbit hole than I initially thought, I\n> was fishing to see if someone knew of a place in the code or WIP patch\n> that dealt with these special refs, but between the low-level machinery\n> & sha1_name.c (and others) there may be no easy one place to do this...\n\nI think we talked at one point about allowing only [A-Z_] for top-level\nrefs. My recollection is that it generally seemed like a good idea, but\nI don't think we ever had patches.\n\nI think it would work to enforce it via check_refname_format(). That\nwould catch reading via dwim_ref(), which is what your example is\nhitting. But it should also prevent people from writing \".git/foo\" (or\nworse, \".git/config\") as a ref.\n\nI do think that's the tip of the iceberg for case-sensitivity problems\nwith refs, though. Because packed-refs is case-sensitive, I think you\ncan create some pretty confusing states on case-insensitive filesystems.\nFor example:\n\n  http://public-inbox.org/git/20150825052123.GA523@sigill.intra.peff.net/\n\nUltimately I think the path forward is to have a ref backend that\nbehaves uniformly (either because it avoids the filesystem, or because\nit encodes around the differences). See:\n\n  http://public-inbox.org/git/xmqqvb4udyf9.fsf@gitster.mtv.corp.google.com/\n\nand its reply.\n\n-Peff\n"},{"id":"323847","messageId":"xmqqshiaizhz.fsf@gitster.mtv.corp.google.com","threadId":"46310","inReplyTo":"20170705083611.jgxbp4sqogicfwdb@sigill.intra.peff.net","subject":"Re: Should \"head\" also work for \"HEAD\" on case-insensitive FS?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-05T17:07:20Z","receivedAt":"2017-07-05T17:07:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Ultimately I think the path forward is to have a ref backend that\n> behaves uniformly (either because it avoids the filesystem, or because\n> it encodes around the differences). See:\n>\n>   http://public-inbox.org/git/xmqqvb4udyf9.fsf@gitster.mtv.corp.google.com/\n>\n> and its reply.\n\nOnce Michael's packed-refs backend stabilizes, we may have a nice\ncalm period in the refs subsystem and I expect that this will become\na good medium-sized project for a contributor who does not have to \nbe so experienced (but not a complete newbie).\n\nIt needs to:\n\n - add icase-files-backend, preferrably sharing as much code as the\n   existing files-backend, in refs/.\n\n - design a mechanism to configure which refs backend to use at\n   runtime; as this has to be done fairly early in the control flow,\n   this will likely to use early configuration mechanism and will\n   probably need to be done in the set-up code, but doing it lazy\n   may even be nicer, as not all subcommands need access to refs.\n\nThanks for a pointer to the archive.\n"},{"id":"323885","messageId":"20170706033554.GA9195@lenny.localdomain","threadId":"46310","inReplyTo":"20170704071909.phs4bf5ybdord2lv@tigra","subject":"Re: Should \"head\" also work for \"HEAD\" on case-insensitive FS?","fromName":"Kenneth Hsu","fromEmail":"kennethhsu@gmail.com","sentAt":"2017-07-06T03:35:54Z","receivedAt":"2017-07-06T03:36:07Z","isPatch":false,"sender":{"key":"kennethhsu@gmail.com","avatar":null},"body":"On Tue, Jul 04, 2017 at 10:19:09AM +0300, Konstantin Khomoutov wrote:\n> On Tue, Jul 04, 2017 at 12:00:49AM +0200, Ævar Arnfjörð Bjarmason wrote:\n> \n> > I don't have a OSX box, but was helping a co-worker over Jabber the\n> > other day, and he pasted something like:\n> > \n> >     $ git merge-base github/master head\n> > \n> > Which didn't work for me, and I thought he had a local \"head\" branch\n> > until realizing that of course we were just resolving HEAD on the FS.\n> > \n> > Has this come up before? I think it makes sense to warn/error about\n> > these magic /HEAD/ revisions if they're not upper-case.\n> > \n> > This is likely unintentional and purely some emergent effect of how it's\n> > implemented, and leads to unportable git invocations.\n> \n> JFTR this is one common case of confusion on Windows as well.\n> To the point that I saw people purposedly using \"head\" on StackOverflow\n> questions.  That is, they appear to think (for some reason) that\n> branches in Git have case-insensitive names and prefer to spell \"head\"\n> since it (supposedly) easier to type.\n\nThe use of \"head\" also appears to be leading to some strange behavior\nwhen resolving refs on Windows.  See the following issue in the\ngit-for-windows project:\n\nhttps://github.com/git-for-windows/git/issues/1225\n\nIn summary, it seems that head and HEAD can resolve to different\nrevisions when in a worktree.\n"},{"id":"323915","messageId":"xmqqo9sxdwjp.fsf@gitster.mtv.corp.google.com","threadId":"46310","inReplyTo":"xmqqshiaizhz.fsf@gitster.mtv.corp.google.com","subject":"Re: Should \"head\" also work for \"HEAD\" on case-insensitive FS?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-06T22:34:34Z","receivedAt":"2017-07-06T22:34:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Once Michael's packed-refs backend stabilizes, we may have a nice\n> calm period in the refs subsystem and I expect that this will become\n> a good medium-sized project for a contributor who does not have to \n> be so experienced (but not a complete newbie).\n>\n> It needs to:\n>\n>  - add icase-files-backend, preferrably sharing as much code as the\n>    existing files-backend, in refs/.\n>\n>  - design a mechanism to configure which refs backend to use at\n>    runtime; as this has to be done fairly early in the control flow,\n>    this will likely to use early configuration mechanism and will\n>    probably need to be done in the set-up code, but doing it lazy\n>    may even be nicer, as not all subcommands need access to refs.\n>\n> Thanks for a pointer to the archive.\n\nSo here is an early WIP/illustration I did to see how involved such\na change would be, which should apply cleanly on top of 'pu'.\n\nI was pleasantly surprised how cleanly refs/files-backend.c\nseparates the notion of \"path\" and \"refname\".  Only two functions,\nfiles_reflog_path() and files_ref_path(), are responsible for taking\nthe refname and turning it to the pathname of an filesystem entity.\nOne one function, loose_fill_ref_dir(), is responsible for running\nreaddir() to find pathname, and turning the result into a refname.\nSo in theory, these three are the only things that need to know\nabout the \"encoding\".\n\nThe exact detail of the encoding used here is immaterial, but I just\nused \"encode uppercase letters and % as % followed by two hex\",\nwhich was simple enough.  Usual refs/heads/master and friends will\nnot have to be touched when encoded this way.  Perhaps the decoding\nside should be tweaked so that uppercase letters it sees needs to be\ndowncased to avoid \"refs/heads/Foo\" getting returned as \"Foo\" branch,\nas a \"Foo\" branch should have been encoded as \"refs/heads/%46oo\".\n\nHaving said that, this patch alone does not quite work yet.\n\n * In the repository discovery code, we have some logic that\n   hard-codes the path in the directory (which is a candidate for\n   being a repository) to check, like \"refs/\" and \"HEAD\".  In the\n   attached illustration patch, files_path_encode() special cases\n   \"HEAD\" so that it is not munged, which is a bit of ugly\n   workaround for this.\n\n * I haven't figured out why, but what refs.c calls \"pseudo refs\"\n   bypasses the files backend layer for some but not all operations,\n   which causes t1405-main-ref-store to fail.  The test creates a\n   \"pseudo ref\" FOO and then tries to remove it.  Creation seems to\n   follow the files-backend.c and thusly goes through the escaping;\n   refs.::refs_delete_ref() however does not consult files-backend.c\n   and fails to find and delete .git/FOO, because the creation side\n   encoded it as \".git/%46%4F%4F\".\n\nMichael is CC'ed as I thought it would be simpler to just ask about\nthe latter bullet point than digging it further myself ;-)\n\nThanks.\n\n refs/files-backend.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++-----\n 1 file changed, 56 insertions(+), 6 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 923e481e06..5bde77cbf8 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -23,6 +23,7 @@ struct ref_lock {\n struct files_ref_store {\n \tstruct ref_store base;\n \tunsigned int store_flags;\n+\tint encode_names;\n \n \tchar *gitdir;\n \tchar *gitcommondir;\n@@ -54,6 +55,9 @@ static struct ref_store *files_ref_store_create(const char *gitdir,\n \tbase_ref_store_init(ref_store, &refs_be_files);\n \trefs->store_flags = flags;\n \n+\t/* git_config_get_bool(\"core.escapeLooseRefNames\", &refs->encode_names); */\n+\trefs->encode_names = 1;\n+\n \trefs->gitdir = xstrdup(gitdir);\n \tget_common_dir_noenv(&sb, gitdir);\n \trefs->gitcommondir = strbuf_detach(&sb, NULL);\n@@ -102,6 +106,49 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store,\n \treturn refs;\n }\n \n+static void files_path_encode(struct files_ref_store *refs,\n+\t\t\t      struct strbuf *sb, const char *refname)\n+{\n+\tif (!refs->encode_names || !strcmp(refname, \"HEAD\")) {\n+\t\tstrbuf_addstr(sb, refname);\n+\t} else {\n+\t\tconst char *cp;\n+\n+\t\tfor (cp = refname; *cp; cp++) {\n+\t\t\tint ch = *cp;\n+\t\t\tif (('A' <= ch && ch <= 'Z') || (ch == '%'))\n+\t\t\t\tstrbuf_addf(sb, \"%%%02x\", ch);\n+\t\t\telse\n+\t\t\t\tstrbuf_addch(sb, ch);\n+\t\t}\n+\t}\n+}\n+\n+static int files_path_decode(struct files_ref_store *refs,\n+\t\t\t     struct strbuf *sb, const char *name, int namelen)\n+{\n+\tif (!refs->encode_names) {\n+\t\tstrbuf_add(sb, name, namelen);\n+\t} else {\n+\t\tsize_t origlen = sb->len;\n+\n+\t\twhile (namelen--) {\n+\t\t\tint ch = *name++;\n+\n+\t\t\tif (ch == '%') {\n+\t\t\t\tif (namelen < 2 || (ch = hex2chr(name)) < 0) {\n+\t\t\t\t\tstrbuf_setlen(sb, origlen);\n+\t\t\t\t\treturn -1;\n+\t\t\t\t}\n+\t\t\t\tnamelen -= 2;\n+\t\t\t\tname += 2;\n+\t\t\t}\n+\t\t\tstrbuf_addch(sb, ch);\n+\t\t}\n+\t}\n+\treturn 0;\n+}\n+\n static void files_reflog_path(struct files_ref_store *refs,\n \t\t\t      struct strbuf *sb,\n \t\t\t      const char *refname)\n@@ -118,15 +165,16 @@ static void files_reflog_path(struct files_ref_store *refs,\n \tswitch (ref_type(refname)) {\n \tcase REF_TYPE_PER_WORKTREE:\n \tcase REF_TYPE_PSEUDOREF:\n-\t\tstrbuf_addf(sb, \"%s/logs/%s\", refs->gitdir, refname);\n+\t\tstrbuf_addf(sb, \"%s/logs/\", refs->gitdir);\n \t\tbreak;\n \tcase REF_TYPE_NORMAL:\n-\t\tstrbuf_addf(sb, \"%s/logs/%s\", refs->gitcommondir, refname);\n+\t\tstrbuf_addf(sb, \"%s/logs/\", refs->gitcommondir);\n \t\tbreak;\n \tdefault:\n \t\tdie(\"BUG: unknown ref type %d of ref %s\",\n \t\t    ref_type(refname), refname);\n \t}\n+\tfiles_path_encode(refs, sb, refname);\n }\n \n static void files_ref_path(struct files_ref_store *refs,\n@@ -136,15 +184,16 @@ static void files_ref_path(struct files_ref_store *refs,\n \tswitch (ref_type(refname)) {\n \tcase REF_TYPE_PER_WORKTREE:\n \tcase REF_TYPE_PSEUDOREF:\n-\t\tstrbuf_addf(sb, \"%s/%s\", refs->gitdir, refname);\n+\t\tstrbuf_addf(sb, \"%s/\", refs->gitdir);\n \t\tbreak;\n \tcase REF_TYPE_NORMAL:\n-\t\tstrbuf_addf(sb, \"%s/%s\", refs->gitcommondir, refname);\n+\t\tstrbuf_addf(sb, \"%s/\", refs->gitcommondir);\n \t\tbreak;\n \tdefault:\n \t\tdie(\"BUG: unknown ref type %d of ref %s\",\n \t\t    ref_type(refname), refname);\n \t}\n+\tfiles_path_encode(refs, sb, refname);\n }\n \n /*\n@@ -174,7 +223,7 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,\n \t}\n \n \tstrbuf_init(&refname, dirnamelen + 257);\n-\tstrbuf_add(&refname, dirname, dirnamelen);\n+\tfiles_path_decode(refs, &refname, dirname, dirnamelen);\n \n \twhile ((de = readdir(d)) != NULL) {\n \t\tstruct object_id oid;\n@@ -185,7 +234,8 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,\n \t\t\tcontinue;\n \t\tif (ends_with(de->d_name, \".lock\"))\n \t\t\tcontinue;\n-\t\tstrbuf_addstr(&refname, de->d_name);\n+\t\tif (files_path_decode(refs, &refname, de->d_name, strlen(de->d_name)))\n+\t\t\tcontinue;\n \t\tstrbuf_addstr(&path, de->d_name);\n \t\tif (stat(path.buf, &st) < 0) {\n \t\t\t; /* silently ignore */\n\n\n\n\n"},{"id":"325174","messageId":"CAMy9T_HA1LcAo2pi27gPaRd4KZzTdT3tSYzV_CC8UBkbtk78RA@mail.gmail.com","threadId":"46310","inReplyTo":"CAMy9T_FmE=8xzjRJJRxLwQjoMStJx5sYO_xtODv2OEZm54DurA@mail.gmail.com","subject":"Fwd: Should \"head\" also work for \"HEAD\" on case-insensitive FS?","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-07-27T00:23:23Z","receivedAt":"2017-07-27T00:23:32Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Dang, I just noticed that I hit \"reply\" rather than \"reply-to-all\" on\nthe below email (stupid GMail default). Junio, your response to this\nemail accordingly went only to me.\n\nMichael\n\n---------- Forwarded message ----------\nFrom: Michael Haggerty <mhagger@alum.mit.edu>\nDate: Mon, Jul 10, 2017 at 7:52 AM\nSubject: Re: Should \"head\" also work for \"HEAD\" on case-insensitive FS?\nTo: Junio C Hamano <gitster@pobox.com>\n\n\nOn Fri, Jul 7, 2017 at 12:34 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> [...]\n> The exact detail of the encoding used here is immaterial, but I just\n> used \"encode uppercase letters and % as % followed by two hex\",\n> which was simple enough.  Usual refs/heads/master and friends will\n> not have to be touched when encoded this way.  Perhaps the decoding\n> side should be tweaked so that uppercase letters it sees needs to be\n> downcased to avoid \"refs/heads/Foo\" getting returned as \"Foo\" branch,\n> as a \"Foo\" branch should have been encoded as \"refs/heads/%46oo\".\n>\n> Having said that, this patch alone does not quite work yet.\n>\n>  * In the repository discovery code, we have some logic that\n>    hard-codes the path in the directory (which is a candidate for\n>    being a repository) to check, like \"refs/\" and \"HEAD\".  In the\n>    attached illustration patch, files_path_encode() special cases\n>    \"HEAD\" so that it is not munged, which is a bit of ugly\n>    workaround for this.\n>\n>  * I haven't figured out why, but what refs.c calls \"pseudo refs\"\n>    bypasses the files backend layer for some but not all operations,\n>    which causes t1405-main-ref-store to fail.  The test creates a\n>    \"pseudo ref\" FOO and then tries to remove it.  Creation seems to\n>    follow the files-backend.c and thusly goes through the escaping;\n>    refs.::refs_delete_ref() however does not consult files-backend.c\n>    and fails to find and delete .git/FOO, because the creation side\n>    encoded it as \".git/%46%4F%4F\".\n\nI think the most natural thing would be to use different encoding\nrules for pseudo-refs (references like \"HEAD\" and \"FETCH_HEAD\") and\nfor other references (those starting with \"refs/\").\n\nPseudo-refs (with the partial exception of \"HEAD\") are quite peculiar\nbeasts. They sometimes include other information besides the reference\nvalue and IIRC the refs code doesn't have any idea how to write or\nread those extra contents. I believe that \"HEAD\" is the only pseudo\nref for which reflogs are ever written. Pseudo-refs have to match\n/[A-Z_]+/ (see https://github.com/git/git/blob/8b2efe2a0fd93b8721879f796d848a9ce785647f/refs.c#L169-L173),\nso ISTM that there is no need to encode such references' filenames at\nall. (It's possible that the pattern could be made even stricter, like\n/[A-Z_]+HEAD/.) Moreover, IIRC, such references are never scanned for\n(as in for-each-refs) but rather are always asked for by name. So\ntheir names might never have to be *de*coded, either. On the other\nhand, when trying to look them up, it would be a good idea to verify\nthat the requested name satisfies the above naming rule. Other than\nthat, I believe it would be preferable to leave pseudo-refs untouched\nby your new encoding/decoding code.\n\nWhereas other references are typically lower-case, so it makes sense\nfor lower-case letters to be the ones that are passed through\ntransparently in such references' filenames (as in your scheme).\n\nBut...since we are talking about introducing a new loose reference\nfilename encoding, I think it would be a good idea to address a couple\nof related issues at the same time:\n\n* Some filesystems natively use Unicode, and insist on a particular\nUnicode normalization (NFC vs NFD), which might differ from the\n\"upstream\" normalization. So such reference names get munged when\nwritten as loose references. I'm not enough of an expert in Unicode to\nknow what the best solution is, except for the strong feeling that it\nwould require some cooperation from the rest of Git to ensure a good\nuser experience.\n\n* Another bad effect of our current loose reference encoding is that\nit prohibits references that D/F conflict with each other (like\n\"refs/heads/foo\" and \"refs/heads/foo/bar\") because\n\"$GIT_DIR/refs/heads/foo\" can't be a file and a directory at the same\ntime. Even if we don't want to support that, this problem also\nprevents us from storing reflogs for deleted references, which is a\nserious flaw. We could solve this problem by encoding \"directory\"\ncomponents of reference names differently than \"leaf\" components; for\nexample, the above references could be encoded as\n\"refs/heads.d/foo.ref\" and \"refs/heads.d/foo.d/bar.ref\".\n\nIt'd be nice to solve all of these related problems at the same time,\nbecause whatever encoding we choose now will have to be supported\nforever.\n\nMichael\n"},{"id":"325176","messageId":"CAPc5daXj4sBuWP0r6t0nArXt1DJW+9byT49M_g8LcjrqBMJnRg@mail.gmail.com","threadId":"46310","inReplyTo":"xmqqa84c6v41.fsf@gitster.mtv.corp.google.com","subject":"Fwd: Should \"head\" also work for \"HEAD\" on case-insensitive FS?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-27T00:49:47Z","receivedAt":"2017-07-27T00:59:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heh, then I'll forward my response and we are even ;-)\n\n\n---------- Forwarded message ----------\nFrom: Junio C Hamano <gitster@pobox.com>\nDate: Mon, Jul 10, 2017 at 10:48 AM\nSubject: Re: Should \"head\" also work for \"HEAD\" on case-insensitive FS?\nTo: Michael Haggerty <mhagger@alum.mit.edu>\n\n\nMichael Haggerty <mhagger@alum.mit.edu> writes:\n\n> I think the most natural thing would be to use different encoding\n> rules for pseudo-refs (references like \"HEAD\" and \"FETCH_HEAD\") and\n> for other references (those starting with \"refs/\").\n>\n> Pseudo-refs (with the partial exception of \"HEAD\") are quite peculiar\n> beasts....\n\nI agree with the reasoning, but what I am worried about is that\ntheir handling in the existing refs.c code may be leaky and/or\ninconsistent.\n\nWhat I saw was that a test have ended up with .git/%46%4F%4F when it\nwas told to create a ref \"FOO\" (which indicates that \"FOO\" was\npassed to the files backend), which later failed to read it back\nbecause the pseudo_ref handling refs.c wanted to see \".git/FOO\" on\nthe reading side.\n\nPerhaps it is only a bug in t/t1405-main-ref-store.sh?\n\n> But...since we are talking about introducing a new loose reference\n> filename encoding, ...\n\nYes, but that is an encoding detail I do not have to get involved\nand folks with platform needs can add more on top---we need to make\nsure that the places that encode and decode are identified in the\ncode first, and the things like \"FOO is encoded upon writing because\nfiles-backend is asked to write it, but not decoded because refs.c\nthinks it is pseudo-ref and does not give a say to files-backend\"\nshouldn't be happening before we can start working on the details of\nthe encoding.  Making a conscious decision that pseudo-refs are left\nas-is is OK, but we need to see both reading and writing side\nfollowing the same codepath to make that decision, which does not\nseem to be the case in the current code.\n"},{"id":"325186","messageId":"20170727143507.bezad7dnthx4nqtc@sigill.intra.peff.net","threadId":"46310","inReplyTo":"CAPc5daXj4sBuWP0r6t0nArXt1DJW+9byT49M_g8LcjrqBMJnRg@mail.gmail.com","subject":"Re: Fwd: Should \"head\" also work for \"HEAD\" on case-insensitive FS?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-07-27T14:35:08Z","receivedAt":"2017-07-27T14:35:15Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 26, 2017 at 05:49:47PM -0700, Junio C Hamano wrote:\n\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n> \n> > I think the most natural thing would be to use different encoding\n> > rules for pseudo-refs (references like \"HEAD\" and \"FETCH_HEAD\") and\n> > for other references (those starting with \"refs/\").\n> >\n> > Pseudo-refs (with the partial exception of \"HEAD\") are quite peculiar\n> > beasts....\n> \n> I agree with the reasoning, but what I am worried about is that\n> their handling in the existing refs.c code may be leaky and/or\n> inconsistent.\n> \n> What I saw was that a test have ended up with .git/%46%4F%4F when it\n> was told to create a ref \"FOO\" (which indicates that \"FOO\" was\n> passed to the files backend), which later failed to read it back\n> because the pseudo_ref handling refs.c wanted to see \".git/FOO\" on\n> the reading side.\n> \n> Perhaps it is only a bug in t/t1405-main-ref-store.sh?\n\nAn interesting related issue for pseudo-refs: if you encode HEAD as\n.git/%48%45%41%44, how will we recognize that directory as a git\nrepository? Detecting (and doing a sanity check on) \"HEAD\" is one of the\nkey mechanisms for deciding whether we are in a git repository.\n\nObviously an older version of git that doesn't know about the new\nencoding scheme wouldn't work on this repository anyway. But:\n\n  1. It should say \"this is a git repo, but not a vintage I understand\".\n     Not \"this isn't a git repo, I'll keep looking\".\n\n  2. How does a git version of the correct vintage decide \"this is a git\n     repo, so I'll check its config for extensions.refBackend, and a-ha,\n     they _do_ have a HEAD\". There's a chicken-and-egg problem.\n\nObviously for (2) we could teach that mechanism to look for the encoded\nHEAD file, too. But this is just one backend. What about a reftable or\nother non-filesystem store that keeps \"HEAD\" inside a file?\n\nI kind of wonder if more exotic ref storage backends should always just\nplace a dummy \"HEAD\" file that is enough to bootstrap the \"this is a git\nrepo\" process (for both new and old versions).\n\nThis is orthogonal to the rest of the pseudo-refs discussion, but just\nsomething I thought of while reading the thread.\n\n-Peff\n"},{"id":"325190","messageId":"xmqqlgn9oq8q.fsf@gitster.mtv.corp.google.com","threadId":"46310","inReplyTo":"20170727143507.bezad7dnthx4nqtc@sigill.intra.peff.net","subject":"Re: Fwd: Should \"head\" also work for \"HEAD\" on case-insensitive FS?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-27T15:26:29Z","receivedAt":"2017-07-27T15:26:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Wed, Jul 26, 2017 at 05:49:47PM -0700, Junio C Hamano wrote:\n>\n>> What I saw was that a test have ended up with .git/%46%4F%4F when it\n>> was told to create a ref \"FOO\" (which indicates that \"FOO\" was\n>> passed to the files backend), which later failed to read it back\n>> because the pseudo_ref handling refs.c wanted to see \".git/FOO\" on\n>> the reading side.\n>> \n>> Perhaps it is only a bug in t/t1405-main-ref-store.sh?\n>\n> An interesting related issue for pseudo-refs: if you encode HEAD as\n> .git/%48%45%41%44, how will we recognize that directory as a git\n> repository?\n\nYes, that is a valid point.  I may have forgot to explain why the\nsample change in my message upthread special cases \"HEAD\" and leaves\nit untouched, but it is done for this exact reason.\n\n>   1. It should say \"this is a git repo, but not a vintage I understand\".\n>      Not \"this isn't a git repo, I'll keep looking\".\n>\n>   2. How does a git version of the correct vintage decide \"this is a git\n>      repo, so I'll check its config for extensions.refBackend, and a-ha,\n>      they _do_ have a HEAD\". There's a chicken-and-egg problem.\n\nYes, exactly.\n"}]}