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