From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Sixt <j6t@kdbg.org>, Git Mailing List <git@vger.kernel.org>
Subject: [PATCH 4/3] fsck: check even zero-entry index files
Date: Sun, 26 Feb 2023 17:29:43 -0500 [thread overview]
Message-ID: <Y/vdV4bjorvRYoaR@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqr0uf0y4b.fsf@gitster.g>
On Fri, Feb 24, 2023 at 09:30:44AM -0800, Junio C Hamano wrote:
> So we had a separate worktree with its index pointing at an object
> by its resolve-undo (or cache-tree) extension, but somehow lost that
> object to gc (I agree with your assessment that it should no longer
> happen since 2017). gc these days knows about looking at the index
> of all worktrees, finds the issue, and stops for safety. fsck that
> is run in the primary worktree may not have noticed but fsck run
> from that worktree would notice the issue.
>
> Sounds like a frustrating one.
>
> Thanks, both, for finding and fixing.
I saw that this hit next, but I had a few fixups that I had planned to
squash in. I saw you got the leak-fix one, but I have one more. Since
this is the end of the cycle, we _could_ just squash it in when we
rewind next. But having now written it as a patch on top, I think the
explanation kind of merits its own commit.
-- >8 --
Subject: [PATCH] fsck: check even zero-entry index files
In fb64ca526a (fsck: check index files in all worktrees, 2023-02-24), we
swapped out a call to vanilla repo_read_index() for a series of
read_index_from() calls, one per worktree. The code for the latter was
copied from add_index_objects_to_pending(), which checks for a positive
return value from the index reading function, and we do the same here in
fsck now.
But this is probably the wrong thing. I had interpreted the check as
"don't operate on the index struct if there was an error". But in
reality, if there is an error then the index-reading code will simply
die (which admittedly is not great for fsck, but that is not a new
problem).
The return value here is actually the number of entries read. So it
makes sense for add_index_objects_to_pending() to ignore a zero-entry
index (there is nothing to add). But for fsck, we would still want to
check any extensions, etc (though presumably it is unlikely to have them
in an empty index, I don't think it's impossible).
So we should ignore the return value from read_index_from() entirely.
This matches the behavior before fb64ca526a, when we ignored the return
value from repo_read_index().
Signed-off-by: Jeff King <peff@peff.net>
---
On top of jk/fsck-indices-in-worktrees.
builtin/fsck.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 1b032eebb1..64614b43b2 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -1007,9 +1007,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
* while we're examining the index.
*/
path = xstrdup(worktree_git_path(wt, "index"));
- if (read_index_from(&istate, path,
- get_worktree_git_dir(wt)) > 0)
- fsck_index(&istate, path, wt->is_current);
+ read_index_from(&istate, path, get_worktree_git_dir(wt));
+ fsck_index(&istate, path, wt->is_current);
discard_index(&istate);
free(path);
}
--
2.40.0.rc0.479.g8b3a13b6b0
next prev parent reply other threads:[~2023-02-26 22:30 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-18 9:38 Bug: fsck and repack don't agree when a worktree index extension is "broken" Johannes Sixt
2023-02-24 8:05 ` [PATCH 0/3] fsck index files from all worktrees Jeff King
2023-02-24 8:07 ` [PATCH 1/3] fsck: factor out index fsck Jeff King
2023-02-24 8:09 ` [PATCH 2/3] fsck: check index files in all worktrees Jeff King
2023-02-24 8:45 ` Jeff King
2023-02-24 8:12 ` [PATCH 3/3] fsck: mention file path for index errors Jeff King
2023-05-11 6:39 ` Eric Sunshine
2023-05-11 16:17 ` Jeff King
2023-05-11 16:28 ` Eric Sunshine
2023-05-11 17:01 ` Jeff King
2023-06-29 18:21 ` Eric Sunshine
2023-06-29 19:37 ` Junio C Hamano
2023-06-01 12:15 ` Andreas Schwab
2023-06-01 14:04 ` Jeff King
2023-02-24 17:30 ` [PATCH 0/3] fsck index files from all worktrees Junio C Hamano
2023-02-26 22:29 ` Jeff King [this message]
2023-02-27 12:09 ` [PATCH 4/3] fsck: check even zero-entry index files Derrick Stolee
2023-02-27 15:58 ` Junio C Hamano
2023-02-26 21:49 ` [PATCH 0/3] fsck index files from all worktrees Johannes Sixt
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=Y/vdV4bjorvRYoaR@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).