{"thread":{"id":"58167","subject":"[PATCH] refs: work around network caching on Windows","startedAt":"2022-07-15T08:07:06Z","lastAt":"2022-07-29T10:06:00Z","messageCount":9,"participants":["Johannes Schindelin via GitGitGadget","Eric Sunshine","Ævar Arnfjörð Bjarmason","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"459126","messageId":"pull.1291.git.1657872416216.gitgitgadget@gmail.com","threadId":"58167","inReplyTo":null,"subject":"[PATCH] refs: work around network caching on Windows","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-15T08:06:56Z","receivedAt":"2022-07-15T08:07:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Pierre Garnier <pgarnier@mega.com>\n\nNetwork shares sometimes use aggressive caching, in which case a\njust-created directory might not be immediately visible to Git.\n\nOne symptom of this scenario is the following error:\n\n\t$ git tag -a -m \"automatic tag creation\"  test_dir/test_tag\n\tfatal: cannot lock ref 'refs/tags/test_dir/test_tag': unable to resolve reference 'refs/tags/test_dir/test_tag': Not a directory\n\nNote: This does not necessarily happen in all Windows setups. One setup\nwhere it _did_ happen is a Windows Server 2019 VM, and as hinted in\n\n\thttp://woshub.com/slow-network-shared-folder-refresh-windows-server/\n\nthe following commands worked around it:\n\n\tSet-SmbClientConfiguration -DirectoryCacheLifetime 0\n\tSet-SmbClientConfiguration -FileInfoCacheLifetime 0\n\tSet-SmbClientConfiguration -FileNotFoundCacheLifetime 0\n\nThis would impact performance negatively, though, as it essentially\nturns off all caching, therefore we do not want to require users to do\nthat just to be able to use Git on Windows.\n\nThe underlying culprit is that `GetFileAttributesExW()` that is called from\n`mingw_lstat()` can raise an error `ERROR_PATH_NOT_FOUND`, which is\ntranslated to `ENOTDIR`, as opposed to `ENOENT` as expected on Linux.\n\nTherefore, when trying to read a ref, let's allow `ENOTDIR` in addition\nto `ENOENT` to indicate that said ref is missing.\n\nThis fixes https://github.com/git-for-windows/git/issues/3727\n\nSigned-off-by: Pierre Garnier <pgarnier@mega.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    refs: work around network caching on Windows\n    \n    While it is enough on Linux to look at ENOENT when allowing for missing\n    refs, on Windows we also need to allow ENOTDIR.\n    \n    This fixes https://github.com/git-for-windows/git/issues/3727\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1291%2Fdscho%2Fenotdir-and-enoent-can-indicate-missing-refs-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1291/dscho/enotdir-and-enoent-can-indicate-missing-refs-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1291\n\n refs/files-backend.c  | 2 +-\n refs/packed-backend.c | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 8db7882aacb..b2a880f62f0 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -381,7 +381,7 @@ stat_ref:\n \tif (lstat(path, &st) < 0) {\n \t\tint ignore_errno;\n \t\tmyerr = errno;\n-\t\tif (myerr != ENOENT || skip_packed_refs)\n+\t\tif ((myerr != ENOENT && myerr != ENOTDIR) || skip_packed_refs)\n \t\t\tgoto out;\n \t\tif (refs_read_raw_ref(refs->packed_ref_store, refname, oid,\n \t\t\t\t      referent, type, &ignore_errno)) {\ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex 97b68377673..23d478627a7 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -480,7 +480,7 @@ static int load_contents(struct snapshot *snapshot)\n \n \tfd = open(snapshot->refs->path, O_RDONLY);\n \tif (fd < 0) {\n-\t\tif (errno == ENOENT) {\n+\t\tif (errno == ENOENT || errno == ENOTDIR) {\n \t\t\t/*\n \t\t\t * This is OK; it just means that no\n \t\t\t * \"packed-refs\" file has been written yet,\n\nbase-commit: bbea4dcf42b28eb7ce64a6306cdde875ae5d09ca\n-- \ngitgitgadget\n"},{"id":"459128","messageId":"CAPig+cT17vZcsf2pGQh-S6UjZib3=4Am7RVf=gQq_sDZzKD08w@mail.gmail.com","threadId":"58167","inReplyTo":"pull.1291.git.1657872416216.gitgitgadget@gmail.com","subject":"Re: [PATCH] refs: work around network caching on Windows","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-07-15T08:29:11Z","receivedAt":"2022-07-15T08:29:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 15, 2022 at 4:18 AM Johannes Schindelin via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> Network shares sometimes use aggressive caching, in which case a\n> just-created directory might not be immediately visible to Git.\n>\n> One symptom of this scenario is the following error:\n>\n>         $ git tag -a -m \"automatic tag creation\"  test_dir/test_tag\n>         fatal: cannot lock ref 'refs/tags/test_dir/test_tag': unable to resolve reference 'refs/tags/test_dir/test_tag': Not a directory\n>\n> Note: This does not necessarily happen in all Windows setups. One setup\n> where it _did_ happen is a Windows Server 2019 VM, and as hinted in\n>\n>         http://woshub.com/slow-network-shared-folder-refresh-windows-server/\n>\n> the following commands worked around it:\n>\n>         Set-SmbClientConfiguration -DirectoryCacheLifetime 0\n>         Set-SmbClientConfiguration -FileInfoCacheLifetime 0\n>         Set-SmbClientConfiguration -FileNotFoundCacheLifetime 0\n>\n> This would impact performance negatively, though, as it essentially\n> turns off all caching, therefore we do not want to require users to do\n> that just to be able to use Git on Windows.\n>\n> The underlying culprit is that `GetFileAttributesExW()` that is called from\n> `mingw_lstat()` can raise an error `ERROR_PATH_NOT_FOUND`, which is\n> translated to `ENOTDIR`, as opposed to `ENOENT` as expected on Linux.\n>\n> Therefore, when trying to read a ref, let's allow `ENOTDIR` in addition\n> to `ENOENT` to indicate that said ref is missing.\n>\n> This fixes https://github.com/git-for-windows/git/issues/3727\n>\n> Signed-off-by: Pierre Garnier <pgarnier@mega.com>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> @@ -381,7 +381,7 @@ stat_ref:\n> -               if (myerr != ENOENT || skip_packed_refs)\n> +               if ((myerr != ENOENT && myerr != ENOTDIR) || skip_packed_refs)\n> diff --git a/refs/packed-backend.c b/refs/packed-backend.c\n> @@ -480,7 +480,7 @@ static int load_contents(struct snapshot *snapshot)\n> -               if (errno == ENOENT) {\n> +               if (errno == ENOENT || errno == ENOTDIR) {\n\nThe first question which popped into my mind upon reading the patch\nwas why these changes need to be made to files-backend.c and\npacked-backend.c rather than \"fixing\" mingw_lstat() to return ENOENT\ninstead of ENOTDIR. Patching mingw_lstat() seems more tractable and\nless likely to lead to discovery of other code in the future which\nneeds to be patched in a similar way to how files-backend.c and\npacked-backend.c are being patched here.\n\nPerhaps it's a silly question and the answer is perfectly obvious to\nfolks directly involved in Git development on Windows, but the commit\nmessage doesn't seem to answer it for people who don't have such\ninside knowledge.\n"},{"id":"459129","messageId":"220715.86a69a22oy.gmgdl@evledraar.gmail.com","threadId":"58167","inReplyTo":"pull.1291.git.1657872416216.gitgitgadget@gmail.com","subject":"Re: [PATCH] refs: work around network caching on Windows","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-15T08:30:04Z","receivedAt":"2022-07-15T08:35:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 15 2022, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Pierre Garnier <pgarnier@mega.com>\n>\n> Network shares sometimes use aggressive caching, in which case a\n> just-created directory might not be immediately visible to Git.\n>\n> One symptom of this scenario is the following error:\n>\n> \t$ git tag -a -m \"automatic tag creation\"  test_dir/test_tag\n> \tfatal: cannot lock ref 'refs/tags/test_dir/test_tag': unable to resolve reference 'refs/tags/test_dir/test_tag': Not a directory\n>\n> Note: This does not necessarily happen in all Windows setups. One setup\n> where it _did_ happen is a Windows Server 2019 VM, and as hinted in\n>\n> \thttp://woshub.com/slow-network-shared-folder-refresh-windows-server/\n>\n> the following commands worked around it:\n>\n> \tSet-SmbClientConfiguration -DirectoryCacheLifetime 0\n> \tSet-SmbClientConfiguration -FileInfoCacheLifetime 0\n> \tSet-SmbClientConfiguration -FileNotFoundCacheLifetime 0\n>\n> This would impact performance negatively, though, as it essentially\n> turns off all caching, therefore we do not want to require users to do\n> that just to be able to use Git on Windows.\n>\n> The underlying culprit is that `GetFileAttributesExW()` that is called from\n> `mingw_lstat()` can raise an error `ERROR_PATH_NOT_FOUND`, which is\n> translated to `ENOTDIR`, as opposed to `ENOENT` as expected on Linux.\n>\n> Therefore, when trying to read a ref, let's allow `ENOTDIR` in addition\n> to `ENOENT` to indicate that said ref is missing.\n>\n> This fixes https://github.com/git-for-windows/git/issues/3727\n\nThis really has much wider implications, as we hard depend on POSIX\nsemantics in various other places. E.g. we'll the SHA-1 collision\ndetection sanity check (not sha1dc, the \"does it exist?\") would be racy\non such a system, wouldn't it?\n\n>  refs/files-backend.c  | 2 +-\n>  refs/packed-backend.c | 2 +-\n>  2 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index 8db7882aacb..b2a880f62f0 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -381,7 +381,7 @@ stat_ref:\n>  \tif (lstat(path, &st) < 0) {\n>  \t\tint ignore_errno;\n>  \t\tmyerr = errno;\n> -\t\tif (myerr != ENOENT || skip_packed_refs)\n> +\t\tif ((myerr != ENOENT && myerr != ENOTDIR) || skip_packed_refs)\n>  \t\t\tgoto out;\n>  \t\tif (refs_read_raw_ref(refs->packed_ref_store, refname, oid,\n>  \t\t\t\t      referent, type, &ignore_errno)) {\n> diff --git a/refs/packed-backend.c b/refs/packed-backend.c\n> index 97b68377673..23d478627a7 100644\n> --- a/refs/packed-backend.c\n> +++ b/refs/packed-backend.c\n> @@ -480,7 +480,7 @@ static int load_contents(struct snapshot *snapshot)\n>  \n>  \tfd = open(snapshot->refs->path, O_RDONLY);\n>  \tif (fd < 0) {\n> -\t\tif (errno == ENOENT) {\n> +\t\tif (errno == ENOENT || errno == ENOTDIR) {\n>  \t\t\t/*\n>  \t\t\t * This is OK; it just means that no\n>  \t\t\t * \"packed-refs\" file has been written yet,\n>\n> base-commit: bbea4dcf42b28eb7ce64a6306cdde875ae5d09ca\n\nSo I'm skeptical that this can work at all, but in any case wrapping\nthis non-POSIX hack in an #ifdef for the relevant platform is somtething\nI really think we should have here, or \"#ifdef NON_POSIX_FS_HACK\" or\nsomething.\n\nYou don't want to be carefully reviewing this code thinking wtf, only to\ndiscover later that it's impossible on a well-behaved FS.\n\nAlso, NFS has similar options (which I've seen hard break git repos &\ncorrupt them in the past)< how do its various dangerous caching options\nbehave in these scenarios?\n\nIOW if we're supporting non-POSIX behavior on platform A, are we\ninadvertently making the non-POSIX behavior on platform B even worse?\nEven more of a reason to wrap it in ifdefs...\n\nBut I really think the answer to this is similar to brian's FAQ patches\nfor git repos on \"cloud mounts\", I.e. document carefully that it's\nlikely to corrupt your repo in unexpected ways.\n\nSo I'd be much more comfortable with a workaround that stole what we do\nfor the *.lock spinning here, i.e. we'd detect this errno, say \"wtf,\nnon-POSIX?\" then spin for N ms, and hope to get \"past the race\".\n\nThat would be guaranteed not to suffer from odd corruption issues (as\nthe behavior wouldn't change, we'd just wait and hope to \"catch up\")>\n\nWouldn't that be narrower & better here?\n"},{"id":"459140","messageId":"8o57s19q-r69s-660n-1202-7o05182s00p0@tzk.qr","threadId":"58167","inReplyTo":"CAPig+cT17vZcsf2pGQh-S6UjZib3=4Am7RVf=gQq_sDZzKD08w@mail.gmail.com","subject":"Re: [PATCH] refs: work around network caching on Windows","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-07-15T12:11:09Z","receivedAt":"2022-07-15T12:11:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric,\n\nOn Fri, 15 Jul 2022, Eric Sunshine wrote:\n\n> On Fri, Jul 15, 2022 at 4:18 AM Johannes Schindelin via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n> > Network shares sometimes use aggressive caching, in which case a\n> > just-created directory might not be immediately visible to Git.\n> >\n> > One symptom of this scenario is the following error:\n> >\n> >         $ git tag -a -m \"automatic tag creation\"  test_dir/test_tag\n> >         fatal: cannot lock ref 'refs/tags/test_dir/test_tag': unable to resolve reference 'refs/tags/test_dir/test_tag': Not a directory\n> >\n> > Note: This does not necessarily happen in all Windows setups. One setup\n> > where it _did_ happen is a Windows Server 2019 VM, and as hinted in\n> >\n> >         http://woshub.com/slow-network-shared-folder-refresh-windows-server/\n> >\n> > the following commands worked around it:\n> >\n> >         Set-SmbClientConfiguration -DirectoryCacheLifetime 0\n> >         Set-SmbClientConfiguration -FileInfoCacheLifetime 0\n> >         Set-SmbClientConfiguration -FileNotFoundCacheLifetime 0\n> >\n> > This would impact performance negatively, though, as it essentially\n> > turns off all caching, therefore we do not want to require users to do\n> > that just to be able to use Git on Windows.\n> >\n> > The underlying culprit is that `GetFileAttributesExW()` that is called from\n> > `mingw_lstat()` can raise an error `ERROR_PATH_NOT_FOUND`, which is\n> > translated to `ENOTDIR`, as opposed to `ENOENT` as expected on Linux.\n> >\n> > Therefore, when trying to read a ref, let's allow `ENOTDIR` in addition\n> > to `ENOENT` to indicate that said ref is missing.\n> >\n> > This fixes https://github.com/git-for-windows/git/issues/3727\n> >\n> > Signed-off-by: Pierre Garnier <pgarnier@mega.com>\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> > diff --git a/refs/files-backend.c b/refs/files-backend.c\n> > @@ -381,7 +381,7 @@ stat_ref:\n> > -               if (myerr != ENOENT || skip_packed_refs)\n> > +               if ((myerr != ENOENT && myerr != ENOTDIR) || skip_packed_refs)\n> > diff --git a/refs/packed-backend.c b/refs/packed-backend.c\n> > @@ -480,7 +480,7 @@ static int load_contents(struct snapshot *snapshot)\n> > -               if (errno == ENOENT) {\n> > +               if (errno == ENOENT || errno == ENOTDIR) {\n>\n> The first question which popped into my mind upon reading the patch\n> was why these changes need to be made to files-backend.c and\n> packed-backend.c rather than \"fixing\" mingw_lstat() to return ENOENT\n> instead of ENOTDIR.\n\nI already had started crafting a mail to explain that I do not want to\ntake the risk of changing the code to map `ERROR_PATH_NOT_FOUND` to\n`ENOENT` instead of `ENOTDIR`, as we only have one central function to map\nWindows' error codes to POSIX `errno` values. It would therefore affect\nmany more code paths than just `mingw_lstat()`.\n\nCan you imagine my surprise when I looked up the link to that mapping\nfunction so that I could paste it in my reply to make my point, only to\nsee that that error is _already_ mapped to `ENOENT`? Look here for\nyourself: https://github.com/git/git/blob/v2.37.1/compat/mingw.c#L121\n\nSo where is the bug? It is somewhere completely different:\nhttps://github.com/git/git/blob/v2.37.1/compat/mingw.c#L847-L851\n\nEssentially, Windows does not give us the equivalent of POSIX' `ENOTDIR`:\nFor something like `C:\\a\\b\\c` we only get `ERROR_PATH_NOT_FOUND`, no\nmatter whether `C:\\a` is missing or whether it is a file (POSIX would want\nan `ENOENT` in the former and `ENOTDIR` in the latter case, and\nunfortunately Git fully relies on these POSIX semantics).\n\nIn 4b0abd5c695 (mingw: let lstat() fail with errno == ENOTDIR when\nappropriate, 2016-01-26), we introduced code to emulate POSIX semantics:\nwe laboriously look through the path components to see whether there is\nanything in the way that would prevent us from creating the parent\ndirectory.\n\nIt must be somewhere in that emulation where things go wrong, still. I\nasked Pierre to debug a bit more to get to the bottom of the problem.\n\nSo: thank you for your \"silly\" question, it did point to the need for more\ninvestigation.\n\nCiao,\nDscho\n"},{"id":"459162","messageId":"xmqqy1wuz2rd.fsf@gitster.g","threadId":"58167","inReplyTo":"CAPig+cT17vZcsf2pGQh-S6UjZib3=4Am7RVf=gQq_sDZzKD08w@mail.gmail.com","subject":"Re: [PATCH] refs: work around network caching on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-15T17:47:50Z","receivedAt":"2022-07-15T17:48:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>> The underlying culprit is that `GetFileAttributesExW()` that is called from\n>> `mingw_lstat()` can raise an error `ERROR_PATH_NOT_FOUND`, which is\n>> translated to `ENOTDIR`, as opposed to `ENOENT` as expected on Linux.\n>> ...\n>> @@ -480,7 +480,7 @@ static int load_contents(struct snapshot *snapshot)\n>> -               if (errno == ENOENT) {\n>> +               if (errno == ENOENT || errno == ENOTDIR) {\n>\n> The first question which popped into my mind upon reading the patch\n> was why these changes need to be made to files-backend.c and\n> packed-backend.c rather than \"fixing\" mingw_lstat() to return ENOENT\n> instead of ENOTDIR. Patching mingw_lstat() seems more tractable and\n> less likely to lead to discovery of other code in the future which\n> needs to be patched in a similar way to how files-backend.c and\n> packed-backend.c are being patched here.\n>\n> Perhaps it's a silly question and the answer is perfectly obvious to\n> folks directly involved in Git development on Windows, but the commit\n> message doesn't seem to answer it for people who don't have such\n> inside knowledge.\n\nFWIW, I had the same reaction.  ENOTDIR does not mean an attempt to\naccess \"test_dir/test_tag\" failed because \"test_dir\" did not\nexist---it means \"test_dir\" exists as something that is not a\ndirectory (hence there is no way \"test_dir/test_file\" to exist).\n\nIn the example scenario in the proposed log message, a new tag\n\"test_dir/test_tag\" is created, and (even though the proposed log\nmessage does not explicitly say so) presumably, \"test_dir\" needs to\nbe created while doing so.  A failure to access \"test_dir/test_file\"\ndue to lack of \"test_dir\" shouldn't report ENOTDIR but should report\nENOENT.  So something is fishy.\n\nUnless the internal implementation of the filesystem creates a\nplaceholder that is not a directory at \"test_dir\", returns the\ncontrol to the application, and does further work to turn that\nplaceholder object into a directory in the background, and the\napplication attempts to create \"test_dir/test_tag\" in the meantime,\nracing with the filesystem, or something silly like that?\n\nIt sounds like a platform specific bug that is not specific to the\nref-files subsystem.  If it can be fixed at lstat() emulation, that\nwould benefit other checks for ENOENT.\n\nHaving said all that.\n\nStepping back a bit, one situation where we would want to special\ncase ENOENT is when we have an optional file at known location and\nit is OK for the file to be missing.  We may attempt to read from\n\"$XDG_CONFIG_HOME/git/config\" and it may fail due to ENOENT or\nENOTDIR because the user may not be using XDG config location at all\n(hence $XDG_CONFIG_HOME or XDG_CONFIG_HOME/git may be missing) or\nthe leading directories may be there but not the file at the leaf\nlevel.  In such a use case, we should ignore ENOTDIR just like we\nignore ENOENT as an error that does not matter.\n\nIn any case, the posted patch may hide a repository corruption from\nthe codepath affected and cause it to silently return a bogus\nanswer.  The first hunk touches read_ref_internal() where \"path\"\nvariable contains the path we expect to find the on-disk loose ref\nfile\n\n\tif (lstat(path, &st) < 0) {\n\t\tint ignore_errno;\n\t\tmyerr = errno;\n\t\tif (myerr != ENOENT || skip_packed_refs)\n\t\t\tgoto out;\n\t\tif (refs_read_raw_ref(refs->packed_ref_store, refname, oid,\n\t\t\t\t      referent, type, &ignore_errno)) {\n\t\t\tmyerr = ENOENT;\n\t\t\tgoto out;\n\t\t}\n\t\tret = 0;\n\t\tgoto out;\n\t}\n\nThe idea is that a ref does not have to exist as a loose ref file,\nso an error from lstat() is not immediately an error.  If the ref\nwere previously packed, then we should fall back to read it from the\npacked-ref file.\n\nSo we say that if the error is *NOT* ENOENT, we jump to 'out' label\nto report the failed lstat() as an error\".  Otherwise we proceed to\nattempt to read from the packed-ref file.  Is there any case where\nan attempt to read from a loose ref fails with ENOTDIR and it is OK\nthat we can proceed without reading from the packed-refs file?\n\nIf we have a branch \"js/foo\" that is packed, and then if we removed\nit, and then created a branch \"js\", the original \"js/foo\" should not\nbe in the packed-refs file, but supposed a repository is corrupt and\n\"js/foo\" remains in the packed-refs file.  Now imagine that we ask\nfor \"js/foo\".  What happens?\n\nWe fail to lstat \".git/refs/heads/js/foo\" due to ENOTDIR, because\nthe \"js\" branch exists loose at \".git/refs/heads/js\".  In the\noriginal, because ENOTDIR is not ENOENT, we jumped to \"out\" to\nreport the error.  The patched code to allow ENOTDIR will instead\nhappily read the stale version of \"js/foo\" out of the packed-refs\nfile.\n\n"},{"id":"460072","messageId":"pull.1291.v2.git.1659018558989.gitgitgadget@gmail.com","threadId":"58167","inReplyTo":"pull.1291.git.1657872416216.gitgitgadget@gmail.com","subject":"[PATCH v2] lstat(mingw): correctly detect ENOTDIR scenarios","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-28T14:29:18Z","receivedAt":"2022-07-28T14:32:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nFiles' attributes can indicate more than just whether they are files or\ndirectories. It was reported in Git for Windows that on certain network\nshares, this let to a nasty problem trying to create tags:\n\n\t$ git tag -a -m \"automatic tag creation\"  test_dir/test_tag\n\tfatal: cannot lock ref 'refs/tags/test_dir/test_tag': unable to resolve reference 'refs/tags/test_dir/test_tag': Not a directory\n\nNote: This does not necessarily happen with all types of network shares.\nOne setup where it _did_ happen is a Windows Server 2019 VM, and as\nhinted in\n\n\thttp://woshub.com/slow-network-shared-folder-refresh-windows-server/\n\nin the indicated instance the following commands worked around the bug:\n\n\tSet-SmbClientConfiguration -DirectoryCacheLifetime 0\n\tSet-SmbClientConfiguration -FileInfoCacheLifetime 0\n\tSet-SmbClientConfiguration -FileNotFoundCacheLifetime 0\n\nThis would impact performance negatively, though, as it essentially\nturns off all caching, therefore we do not want to require users to do\nthat just to be able to use Git on Windows.\n\nThe underlying bug is in the code added in 4b0abd5c695 (mingw: let\nlstat() fail with errno == ENOTDIR when appropriate, 2016-01-26) that\nemulates the POSIX behavior where `lstat()` should return `ENOENT` if\nthe file or directory simply does not exist but could be created, and\n`ENOTDIR` if there is no file or directory nor could there be because a\nleading path already exists and is not a directory.\n\nIn that code, the return value of `GetFileAttributesW()` is interpreted\nas an enum value, not as a bit field, so that a perfectly fine leading\ndirectory can be misdetected as \"not a directory\".\n\nAs a consequence, the `read_refs_internal()` function would return\n`ENOTDIR`, suggesting not only that the tag in the `git tag` invocation\nabove does not exist, but that it cannot even be created.\n\nLet's fix the code so that it interprets the return value of the\n`GetFileAtrtibutesW()` call correctly.\n\nThis fixes https://github.com/git-for-windows/git/issues/3727\n\nReported-by: Pierre Garnier <pgarnier@mega.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    Fix the lstat() emulation on Windows\n    \n    One particular code path in the lstat() emulation on Windows was broken.\n    \n    This fixes https://github.com/git-for-windows/git/issues/3727\n    \n    Changes since v1:\n    \n     * Thanks to Eric's excellent review, the reporter and I dug deeper and\n       figured out the real bug (and fix).\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1291%2Fdscho%2Fenotdir-and-enoent-can-indicate-missing-refs-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1291/dscho/enotdir-and-enoent-can-indicate-missing-refs-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1291\n\nRange-diff vs v1:\n\n 1:  c3d51a755ba < -:  ----------- refs: work around network caching on Windows\n -:  ----------- > 1:  2ebe899736e lstat(mingw): correctly detect ENOTDIR scenarios\n\n\n compat/mingw.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 545e952a588..3b85bb02536 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -471,8 +471,8 @@ static int has_valid_directory_prefix(wchar_t *wfilename)\n \t\twfilename[n] = L'\\0';\n \t\tattributes = GetFileAttributesW(wfilename);\n \t\twfilename[n] = c;\n-\t\tif (attributes == FILE_ATTRIBUTE_DIRECTORY ||\n-\t\t\t\tattributes == FILE_ATTRIBUTE_DEVICE)\n+\t\tif (attributes &\n+\t\t    (FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_DEVICE))\n \t\t\treturn 1;\n \t\tif (attributes == INVALID_FILE_ATTRIBUTES)\n \t\t\tswitch (GetLastError()) {\n\nbase-commit: 4b0abd5c695c87bf600e57b6a5c7d6844707d34c\n-- \ngitgitgadget\n"},{"id":"460133","messageId":"xmqq8rodcf5i.fsf@gitster.g","threadId":"58167","inReplyTo":"pull.1291.v2.git.1659018558989.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] lstat(mingw): correctly detect ENOTDIR scenarios","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-28T17:37:13Z","receivedAt":"2022-07-28T17:37:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> In that code, the return value of `GetFileAttributesW()` is interpreted\n> as an enum value, not as a bit field, so that a perfectly fine leading\n> directory can be misdetected as \"not a directory\".\n\nNicely analyzed.  So after all ENOTDIR was a buggy response and by\nfixing it we help all callers of lstat(), as pointed out in the\nearlier review.\n\n>      * Thanks to Eric's excellent review, the reporter and I dug deeper and\n>        figured out the real bug (and fix).\n\nYeah, thanks all.\n\nWill queue.\n\n"},{"id":"460163","messageId":"CAPig+cSpjchBUMi9qjLTu_3B3BbVDFKueeTKJNOgR4BtA-8arQ@mail.gmail.com","threadId":"58167","inReplyTo":"pull.1291.v2.git.1659018558989.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] lstat(mingw): correctly detect ENOTDIR scenarios","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-07-28T23:27:41Z","receivedAt":"2022-07-28T23:27:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jul 28, 2022 at 10:29 AM Johannes Schindelin via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> Files' attributes can indicate more than just whether they are files or\n> directories. It was reported in Git for Windows that on certain network\n> shares, this let to a nasty problem trying to create tags:\n\ns/let/led/\n\n(not worth a re-roll)\n\n>         $ git tag -a -m \"automatic tag creation\"  test_dir/test_tag\n>         fatal: cannot lock ref 'refs/tags/test_dir/test_tag': unable to resolve reference 'refs/tags/test_dir/test_tag': Not a directory\n>\n> Note: This does not necessarily happen with all types of network shares.\n> One setup where it _did_ happen is a Windows Server 2019 VM, and as\n> hinted in\n>\n>         http://woshub.com/slow-network-shared-folder-refresh-windows-server/\n>\n> in the indicated instance the following commands worked around the bug:\n>\n>         Set-SmbClientConfiguration -DirectoryCacheLifetime 0\n>         Set-SmbClientConfiguration -FileInfoCacheLifetime 0\n>         Set-SmbClientConfiguration -FileNotFoundCacheLifetime 0\n>\n> This would impact performance negatively, though, as it essentially\n> turns off all caching, therefore we do not want to require users to do\n> that just to be able to use Git on Windows.\n>\n> The underlying bug is in the code added in 4b0abd5c695 (mingw: let\n> lstat() fail with errno == ENOTDIR when appropriate, 2016-01-26) that\n> emulates the POSIX behavior where `lstat()` should return `ENOENT` if\n> the file or directory simply does not exist but could be created, and\n> `ENOTDIR` if there is no file or directory nor could there be because a\n> leading path already exists and is not a directory.\n>\n> In that code, the return value of `GetFileAttributesW()` is interpreted\n> as an enum value, not as a bit field, so that a perfectly fine leading\n> directory can be misdetected as \"not a directory\".\n>\n> As a consequence, the `read_refs_internal()` function would return\n> `ENOTDIR`, suggesting not only that the tag in the `git tag` invocation\n> above does not exist, but that it cannot even be created.\n>\n> Let's fix the code so that it interprets the return value of the\n> `GetFileAtrtibutesW()` call correctly.\n>\n> This fixes https://github.com/git-for-windows/git/issues/3727\n>\n> Reported-by: Pierre Garnier <pgarnier@mega.com>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n"},{"id":"460193","messageId":"pull.1291.v3.git.1659089152877.gitgitgadget@gmail.com","threadId":"58167","inReplyTo":"pull.1291.v2.git.1659018558989.gitgitgadget@gmail.com","subject":"[PATCH v3] lstat(mingw): correctly detect ENOTDIR scenarios","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-29T10:05:52Z","receivedAt":"2022-07-29T10:06:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nFiles' attributes can indicate more than just whether they are files or\ndirectories. It was reported in Git for Windows that on certain network\nshares, this led to a nasty problem trying to create tags:\n\n\t$ git tag -a -m \"automatic tag creation\"  test_dir/test_tag\n\tfatal: cannot lock ref 'refs/tags/test_dir/test_tag': unable to resolve reference 'refs/tags/test_dir/test_tag': Not a directory\n\nNote: This does not necessarily happen with all types of network shares.\nOne setup where it _did_ happen is a Windows Server 2019 VM, and as\nhinted in\n\n\thttp://woshub.com/slow-network-shared-folder-refresh-windows-server/\n\nin the indicated instance the following commands worked around the bug:\n\n\tSet-SmbClientConfiguration -DirectoryCacheLifetime 0\n\tSet-SmbClientConfiguration -FileInfoCacheLifetime 0\n\tSet-SmbClientConfiguration -FileNotFoundCacheLifetime 0\n\nThis would impact performance negatively, though, as it essentially\nturns off all caching, therefore we do not want to require users to do\nthat just to be able to use Git on Windows.\n\nThe underlying bug is in the code added in 4b0abd5c695 (mingw: let\nlstat() fail with errno == ENOTDIR when appropriate, 2016-01-26) that\nemulates the POSIX behavior where `lstat()` should return `ENOENT` if\nthe file or directory simply does not exist but could be created, and\n`ENOTDIR` if there is no file or directory nor could there be because a\nleading path already exists and is not a directory.\n\nIn that code, the return value of `GetFileAttributesW()` is interpreted\nas an enum value, not as a bit field, so that a perfectly fine leading\ndirectory can be misdetected as \"not a directory\".\n\nAs a consequence, the `read_refs_internal()` function would return\n`ENOTDIR`, suggesting not only that the tag in the `git tag` invocation\nabove does not exist, but that it cannot even be created.\n\nLet's fix the code so that it interprets the return value of the\n`GetFileAttributesW()` call correctly.\n\nThis fixes https://github.com/git-for-windows/git/issues/3727\n\nReported-by: Pierre Garnier <pgarnier@mega.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    Fix the lstat() emulation on Windows\n    \n    One particular code path in the lstat() emulation on Windows was broken.\n    \n    This fixes https://github.com/git-for-windows/git/issues/3727\n    \n    Changes since v2:\n    \n     * Fixed typos in the commit message (I wanted to write \"led\" but had\n       written \"let\" instead, thanks once again, Eric!, and I managed to\n       misspell a function name).\n    \n    Changes since v1:\n    \n     * Thanks to Eric's excellent review, the reporter and I dug deeper and\n       figured out the real bug (and fix).\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1291%2Fdscho%2Fenotdir-and-enoent-can-indicate-missing-refs-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1291/dscho/enotdir-and-enoent-can-indicate-missing-refs-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1291\n\nRange-diff vs v2:\n\n 1:  2ebe899736e ! 1:  b4f08ee0d7c lstat(mingw): correctly detect ENOTDIR scenarios\n     @@ Commit message\n      \n          Files' attributes can indicate more than just whether they are files or\n          directories. It was reported in Git for Windows that on certain network\n     -    shares, this let to a nasty problem trying to create tags:\n     +    shares, this led to a nasty problem trying to create tags:\n      \n                  $ git tag -a -m \"automatic tag creation\"  test_dir/test_tag\n                  fatal: cannot lock ref 'refs/tags/test_dir/test_tag': unable to resolve reference 'refs/tags/test_dir/test_tag': Not a directory\n     @@ Commit message\n          above does not exist, but that it cannot even be created.\n      \n          Let's fix the code so that it interprets the return value of the\n     -    `GetFileAtrtibutesW()` call correctly.\n     +    `GetFileAttributesW()` call correctly.\n      \n          This fixes https://github.com/git-for-windows/git/issues/3727\n      \n\n\n compat/mingw.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 545e952a588..3b85bb02536 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -471,8 +471,8 @@ static int has_valid_directory_prefix(wchar_t *wfilename)\n \t\twfilename[n] = L'\\0';\n \t\tattributes = GetFileAttributesW(wfilename);\n \t\twfilename[n] = c;\n-\t\tif (attributes == FILE_ATTRIBUTE_DIRECTORY ||\n-\t\t\t\tattributes == FILE_ATTRIBUTE_DEVICE)\n+\t\tif (attributes &\n+\t\t    (FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_DEVICE))\n \t\t\treturn 1;\n \t\tif (attributes == INVALID_FILE_ATTRIBUTES)\n \t\t\tswitch (GetLastError()) {\n\nbase-commit: 4b0abd5c695c87bf600e57b6a5c7d6844707d34c\n-- \ngitgitgadget\n"}]}