Re: [PATCH 02/10] builtin/refs.h: get worktrees without reading head info

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



shejialuo <shejialuo@xxxxxxxxx> writes:

> In "packed-backend.c", there are some functions such as "create_snapshot"
> and "next_record" which would check the correctness of the content of
> the "packed-ref" file. When anything is bad, the program will die.

So you're saying, `create_snapshot()` and `next_record()` exit the
program on any error. Okay that seems to be valid.

> It may seem that we have nothing relevant to above feature, because we
> are going to read and parse the raw "packed-ref" file without creating
> the snapshot and using the ref iterator to check the consistency.
>
> However, when using "get_worktrees" in "builtin/refs", we will parse the
> head information. If the referent of the "HEAD" is inside the
> "packed-ref", we will call "create_snapshot" and "next_record" functions
> to parse the "packed-ref" to get the head information. And if there are
> something wrong, the program will die.
>
> Although this behavior has no harm for the program, it will
> short-circuit the program. When the users execute "git refs verify" or
> "git fsck", we don't want to simply die the program but rather show the
> warnings or errors as many as possible to info the users. So, we should
> avoiding reading the head info.
>

This is a bit tricky here. If the information for the `HEAD` ref is
incorrect in the packed-refs, git would exit early. Which is what we're
trying to avoid in this patch, by using the `get_worktrees_internal()`
function.

However, I would question if this is the right approach. Shouldn't
`get_worktree()` failing indicate that the repository is invalid? In
that case does it really make sense to allow the user to even run `git
refs verify`? Isn't the prerequisite for running the `git-refs(1)`
command a valid repository?

Generally, I'd agree that we try to obtain all errors so that the user
can get a full picture. But exposing internal worktree functions so we
treat invalid repos as valid repos so we can do that, seems a bit of a
stretch.

> Fortunately, in 465a22b338 (worktree: skip reading HEAD when repairing
> worktrees, 2023-12-29), we have introduced a function
> "get_worktrees_internal" which allows us to get worktrees without
> reading head info.
>
> Create a new exposed function "get_worktrees_without_reading_head", then
> replace the "get_worktrees" in "builtin/refs" with the new created
> function.
>

[snip]

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux