From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Josip Sokcevic <sokcevic@google.com>
Subject: Re: [PATCH] cache: add fake_lstat()
Date: Thu, 14 Sep 2023 15:00:43 -0700 [thread overview]
Message-ID: <xmqqr0n0h0tw.fsf@gitster.g> (raw)
In-Reply-To: <xmqqcyykig1l.fsf@gitster.g> (Junio C. Hamano's message of "Thu, 14 Sep 2023 14:46:46 -0700")
And your diff-lib change may look like this on top. This one
unfortunately is completely untested, as I do not use macOS.
Reviewing, testing, and improving it is highly appreciated.
----- >8 ---------- >8 ---------- >8 -----
Subject: diff-lib: fix check_removed() when fsmonitor is active
`git diff-index` may return incorrect deleted entries when fsmonitor
is used in a repository with git submodules. This can be observed on
Mac machines, but it can affect all other supported platforms too.
If fsmonitor is used, `stat *st` is left uninitialied if cache_entry
has CE_FSMONITOR_VALID bit set. But, there are three call sites
that rely on stat afterwards, which can result in incorrect results.
We can fill members of "struct stat" that matters well enough using
the information we have in "struct cache_entry" that fsmonitor told
us is up-to-date to solve this.
Helped-by: Josip Sokcevic <sokcevic@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff-lib.c | 9 ++++++++-
t/t7527-builtin-fsmonitor.sh | 5 +++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git c/diff-lib.c w/diff-lib.c
index d8aa777a73..7799747a97 100644
--- c/diff-lib.c
+++ w/diff-lib.c
@@ -38,8 +38,15 @@
*/
static int check_removed(const struct index_state *istate, const struct cache_entry *ce, struct stat *st)
{
+ int stat_err;
+
assert(is_fsmonitor_refreshed(istate));
- if (!(ce->ce_flags & CE_FSMONITOR_VALID) && lstat(ce->name, st) < 0) {
+
+ if (!(ce->ce_flags & CE_FSMONITOR_VALID))
+ stat_err = lstat(ce->name, st);
+ else
+ stat_err = fake_lstat(ce, st);
+ if (stat_err < 0) {
if (!is_missing_file_error(errno))
return -1;
return 1;
diff --git c/t/t7527-builtin-fsmonitor.sh w/t/t7527-builtin-fsmonitor.sh
index 0c241d6c14..78503158fd 100755
--- c/t/t7527-builtin-fsmonitor.sh
+++ w/t/t7527-builtin-fsmonitor.sh
@@ -809,6 +809,11 @@ my_match_and_clean () {
status --porcelain=v2 >actual.without &&
test_cmp actual.with actual.without &&
+ git -C super --no-optional-locks diff-index --name-status HEAD >actual.with &&
+ git -C super --no-optional-locks -c core.fsmonitor=false \
+ diff-index --name-status HEAD >actual.without &&
+ test_cmp actual.with actual.without &&
+
git -C super/dir_1/dir_2/sub reset --hard &&
git -C super/dir_1/dir_2/sub clean -d -f
}
next prev parent reply other threads:[~2023-09-14 22:00 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-14 21:46 [PATCH] cache: add fake_lstat() Junio C Hamano
2023-09-14 22:00 ` Junio C Hamano [this message]
2023-09-15 0:17 ` Josip Sokcevic
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqr0n0h0tw.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=sokcevic@google.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.
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 NNTP newsgroup(s).