Re: [PATCH v2 2/8] builtin/refs: get worktrees without reading head info

[shejialuo <shejialuo@gmail.com>]

On Thu, Jan 30, 2025 at 10:04:45AM -0800, Junio C Hamano wrote:
> shejialuo <shejialuo@gmail.com> writes:
> 
> > 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.
> 
> "info" is not a verb; "inform"?
> 

Let me improve this in the next version.

> I can understand what you want to say with "show the warnings or
> errors as many as possible", but giving errors on the same issue
> many times is not what you meant

Yes, this is correct.

> ---rather, you want the checker to
> keep going and discover errors in many _other things_, after it
> finds a single error in "HEAD".
> 

I think this is a misunderstanding. Let me explain more to you.

1. If the content of the "HEAD" is not correct, we won't detect the
current directory as valid git repository.
2. If the referent of the "HEAD" is not in the "packed-refs", the
referent must be a loose ref or don't exist. In this situation, because
we will never touch the packed backend.
3. If the referent of the "HEAD" is in the "packed-refs", it will call
"create_snapshot" to create the snapshot. In this function, it would
call "verify_buffer_safe" to check the following things:
   1. Check the correctness of th header.
   2. Check via "verify_buffer_safe" method
   So, even the referent entry is not correct in the "packed-refs", the
   program won't die. But the above two cases will let the program die.

I want to say we cannot find any error in "HEAD" at now as above
described. From my perspective, we should retain the paragraph:

> Although this behavior has no harm for the program...

But we should change the statement

> 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
> avoid reading the head info.

to

    We should avoid reading the head information, which may execute the
    read operation in packed backend with stricter checks to die the
    program. Instead, we should continue to check other parts of the
    "packed-refs" file completely.

> With one reservation.  We still want to diagnose a broken "HEAD", so
> I'd probably strike this sentence out, and add a statement that says
> we still check the contents of "HEAD" elsewhere as a substitute at
> the end of the proposed commit log message, if I were writing it,
> after explaining the use of get_worktrees_without_reading_head()
> you did in the following two paragraphs (both of which read well).

I want to say that we cannot check the content of the "HEAD" itself. If
the content of "HEAD" is not correct, we cannot detect the current
directory as a valid git repository. So, there is no need to say "we
will check the contents of 'HEAD' else where".

I think the misunderstanding is that you think that if the "HEAD" is not
correct, the program will die but actually it is not.

Thanks,
Jialuo