Re: [PATCH] refs: work around network caching on Windows

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Jul 15, 2022 at 4:18 AM Johannes Schindelin via GitGitGadget
<gitgitgadget@gmail.com> 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@mega.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 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. Patching mingw_lstat() seems more tractable and
less likely to lead to discovery of other code in the future which
needs to be patched in a similar way to how files-backend.c and
packed-backend.c are being patched here.

Perhaps it's a silly question and the answer is perfectly obvious to
folks directly involved in Git development on Windows, but the commit
message doesn't seem to answer it for people who don't have such
inside knowledge.