Subject: Re: [PATCH v1 2/4] ref: add regular ref content check for files backend

From: Junio C Hamano <[email-masked]>
Date: Tue, 20 Aug 2024 09:49:23 -0700

shejialuo <[email-masked]> writes:

> We implicitly reply on "git-fsck(1)" to check the consistency of regular

"reply" -> "rely", I think.

> refs. However, when parsing the regular refs for files backend, we allow
> the ref content to end with no newline or contain some garbages. We
> should warn the user about above situations.

Hmph, should we?  

If the content is short (e.g., in SHA-1 repository it only has 39
hexdigit) even if that may be sufficient to uniquely name the
object, we should warn about it, of course.  A file that has
64-hexdigit with a terminating LF at the end may be a valid file to
be in $GIT_DIR/refs/ hierarchy in a SHA-256 repository, but such a
file in a SHA-1 repository should also be subject to a warning, as
it could be a sign that somebody screwed up object format
conversion.

But a file that has only 40-hexdigit without a terminating LF at the
end?  Or a file that has 40-hexdigit followed by a CRLF instead of
LF?  Or a file that has the identical content as a valid ref on its
first line, but has extra stuff on its second and subsequent lines?

What does the name-to-object-name-mapping layer (aka "get_oid" API)
do when they see such a file in the $GIT_DIR/refs/ hierarchy?  If
they are treated as valid ref in the "normal" code path, it needs a
strong justification to tighten the rules retroactively, much
stronger than "Our current code, and any of our older versions,
would have written such a file as a loose ref with our code."

"What are we protecting us from with this tightening?" is the
question we should be asking ourselves, when evaluating each of
these new rules that fsck used not to care about.