Hi Eric, On Fri, 15 Jul 2022, Eric Sunshine wrote: > On Fri, Jul 15, 2022 at 4:18 AM Johannes Schindelin via GitGitGadget > <gitgitgadget@xxxxxxxxx> wrote: > > Network shares sometimes use aggressive caching, in which case a > > just-created directory might not be immediately visible to Git. > > > > One symptom of this scenario is the following error: > > > > $ git tag -a -m "automatic tag creation" test_dir/test_tag > > fatal: cannot lock ref 'refs/tags/test_dir/test_tag': unable to resolve reference 'refs/tags/test_dir/test_tag': Not a directory > > > > Note: This does not necessarily happen in all Windows setups. One setup > > where it _did_ happen is a Windows Server 2019 VM, and as hinted in > > > > http://woshub.com/slow-network-shared-folder-refresh-windows-server/ > > > > the following commands worked around it: > > > > Set-SmbClientConfiguration -DirectoryCacheLifetime 0 > > Set-SmbClientConfiguration -FileInfoCacheLifetime 0 > > Set-SmbClientConfiguration -FileNotFoundCacheLifetime 0 > > > > This would impact performance negatively, though, as it essentially > > turns off all caching, therefore we do not want to require users to do > > that just to be able to use Git on Windows. > > > > The underlying culprit is that `GetFileAttributesExW()` that is called from > > `mingw_lstat()` can raise an error `ERROR_PATH_NOT_FOUND`, which is > > translated to `ENOTDIR`, as opposed to `ENOENT` as expected on Linux. > > > > Therefore, when trying to read a ref, let's allow `ENOTDIR` in addition > > to `ENOENT` to indicate that said ref is missing. > > > > This fixes https://github.com/git-for-windows/git/issues/3727 > > > > Signed-off-by: Pierre Garnier <pgarnier@xxxxxxxx> > > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> > > --- > > diff --git a/refs/files-backend.c b/refs/files-backend.c > > @@ -381,7 +381,7 @@ stat_ref: > > - if (myerr != ENOENT || skip_packed_refs) > > + if ((myerr != ENOENT && myerr != ENOTDIR) || skip_packed_refs) > > diff --git a/refs/packed-backend.c b/refs/packed-backend.c > > @@ -480,7 +480,7 @@ static int load_contents(struct snapshot *snapshot) > > - if (errno == ENOENT) { > > + if (errno == ENOENT || errno == ENOTDIR) { > > The first question which popped into my mind upon reading the patch > was why these changes need to be made to files-backend.c and > packed-backend.c rather than "fixing" mingw_lstat() to return ENOENT > instead of ENOTDIR. I already had started crafting a mail to explain that I do not want to take the risk of changing the code to map `ERROR_PATH_NOT_FOUND` to `ENOENT` instead of `ENOTDIR`, as we only have one central function to map Windows' error codes to POSIX `errno` values. It would therefore affect many more code paths than just `mingw_lstat()`. Can you imagine my surprise when I looked up the link to that mapping function so that I could paste it in my reply to make my point, only to see that that error is _already_ mapped to `ENOENT`? Look here for yourself: https://github.com/git/git/blob/v2.37.1/compat/mingw.c#L121 So where is the bug? It is somewhere completely different: https://github.com/git/git/blob/v2.37.1/compat/mingw.c#L847-L851 Essentially, Windows does not give us the equivalent of POSIX' `ENOTDIR`: For something like `C:\a\b\c` we only get `ERROR_PATH_NOT_FOUND`, no matter whether `C:\a` is missing or whether it is a file (POSIX would want an `ENOENT` in the former and `ENOTDIR` in the latter case, and unfortunately Git fully relies on these POSIX semantics). In 4b0abd5c695 (mingw: let lstat() fail with errno == ENOTDIR when appropriate, 2016-01-26), we introduced code to emulate POSIX semantics: we laboriously look through the path components to see whether there is anything in the way that would prevent us from creating the parent directory. It must be somewhere in that emulation where things go wrong, still. I asked Pierre to debug a bit more to get to the bottom of the problem. So: thank you for your "silly" question, it did point to the need for more investigation. Ciao, Dscho
