Thread (7 messages) 7 messages, 4 authors, 2022-07-29

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

From: Eric Sunshine <hidden>
Date: 2022-07-15 08:29:28

On Fri, Jul 15, 2022 at 4:18 AM Johannes Schindelin via GitGitGadget
[off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
Signed-off-by: Johannes Schindelin <redacted>
---
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help