From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Eric Sunshine <sunshine@sunshineco.com>,
Johannes Schindelin <johannes.schindelin@gmx.de>,
Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH v2] lstat(mingw): correctly detect ENOTDIR scenarios
Date: Thu, 28 Jul 2022 14:29:18 +0000 [thread overview]
Message-ID: <pull.1291.v2.git.1659018558989.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1291.git.1657872416216.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Files' attributes can indicate more than just whether they are files or
directories. It was reported in Git for Windows that on certain network
shares, this let to a nasty problem trying to create tags:
$ 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 with all types of network shares.
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/
in the indicated instance the following commands worked around the bug:
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 bug is in the code added in 4b0abd5c695 (mingw: let
lstat() fail with errno == ENOTDIR when appropriate, 2016-01-26) that
emulates the POSIX behavior where `lstat()` should return `ENOENT` if
the file or directory simply does not exist but could be created, and
`ENOTDIR` if there is no file or directory nor could there be because a
leading path already exists and is not a directory.
In that code, the return value of `GetFileAttributesW()` is interpreted
as an enum value, not as a bit field, so that a perfectly fine leading
directory can be misdetected as "not a directory".
As a consequence, the `read_refs_internal()` function would return
`ENOTDIR`, suggesting not only that the tag in the `git tag` invocation
above does not exist, but that it cannot even be created.
Let's fix the code so that it interprets the return value of the
`GetFileAtrtibutesW()` call correctly.
This fixes https://github.com/git-for-windows/git/issues/3727
Reported-by: Pierre Garnier <pgarnier@mega.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Fix the lstat() emulation on Windows
One particular code path in the lstat() emulation on Windows was broken.
This fixes https://github.com/git-for-windows/git/issues/3727
Changes since v1:
* Thanks to Eric's excellent review, the reporter and I dug deeper and
figured out the real bug (and fix).
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1291%2Fdscho%2Fenotdir-and-enoent-can-indicate-missing-refs-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1291/dscho/enotdir-and-enoent-can-indicate-missing-refs-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1291
Range-diff vs v1:
1: c3d51a755ba < -: ----------- refs: work around network caching on Windows
-: ----------- > 1: 2ebe899736e lstat(mingw): correctly detect ENOTDIR scenarios
compat/mingw.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 545e952a588..3b85bb02536 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -471,8 +471,8 @@ static int has_valid_directory_prefix(wchar_t *wfilename)
wfilename[n] = L'\0';
attributes = GetFileAttributesW(wfilename);
wfilename[n] = c;
- if (attributes == FILE_ATTRIBUTE_DIRECTORY ||
- attributes == FILE_ATTRIBUTE_DEVICE)
+ if (attributes &
+ (FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_DEVICE))
return 1;
if (attributes == INVALID_FILE_ATTRIBUTES)
switch (GetLastError()) {
base-commit: 4b0abd5c695c87bf600e57b6a5c7d6844707d34c
--
gitgitgadget
next prev parent reply other threads:[~2022-07-28 14:32 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-15 8:06 [PATCH] refs: work around network caching on Windows Johannes Schindelin via GitGitGadget
2022-07-15 8:29 ` Eric Sunshine
2022-07-15 12:11 ` Johannes Schindelin
2022-07-15 17:47 ` Junio C Hamano
2022-07-15 8:30 ` Ævar Arnfjörð Bjarmason
2022-07-28 14:29 ` Johannes Schindelin via GitGitGadget [this message]
2022-07-28 17:37 ` [PATCH v2] lstat(mingw): correctly detect ENOTDIR scenarios Junio C Hamano
2022-07-28 23:27 ` Eric Sunshine
2022-07-29 10:05 ` [PATCH v3] " Johannes Schindelin via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.1291.v2.git.1659018558989.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=johannes.schindelin@gmx.de \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).