On Tue, May 13, 2025 at 09:30:57AM -0700, Junio C Hamano wrote: > shejialuo <shejialuo@xxxxxxxxx> writes: > > > During fsck, an empty "packed-refs" gives an error; this is unwarranted. > > The runtime code paths would accept an empty "packed-refs" file, such as > > "create_snapshot" would simply return the "snapshot" without checking > > the content of "packed-refs". > > Perhaps "unwarranted" is now too strong a word; we still want to > consider it an anomaly (that is why emptyPackedRefsFile warning is > introduced after all). I think the problem description you want to > here in the above pragraph is that fsck giving an error and runtime > completely silent is inconsistent. > > Side note: and you'd probably want to say what "an error" > reported here is. The problem, if I understand correctly, is > that the code assumes the file won't be empty and instead has at > least one line in it (even when there are no refs packed, there > is the file header line) and insists that all lines must be well > terminated---if we tolerate an empty file, of course such a > check will fail, as there is no terminating LF in a file with 0 > lines in it. > Good idea, I will improve the commit message. > And because versions of Git that are not too ancient never wrote an > empty packed-refs file, and often having an empty file there is/was > a sign of a filesystem-level issue, the way we want resolve this > inconsistency is not make everybody totally silent but notice and > report the anomaly. > That's right. I should talk about this problem. > > But we need to consider the fsck message type carefully, it is not > > appropriate that we use "FSCK_ERROR". This is because we would > > definitely break the compatibility. Let's create a "FSCK_INFO" message > > id EMPTY_PACKED_REFS_FILE" to indicate that "packed-refs" is empty. > > OK. > > > Signed-off-by: shejialuo <shejialuo@xxxxxxxxx> > > --- > > Documentation/fsck-msgids.adoc | 6 ++++++ > > fsck.h | 1 + > > refs/packed-backend.c | 9 +++++++++ > > t/t0602-reffiles-fsck.sh | 17 +++++++++++++++++ > > 4 files changed, 33 insertions(+) > > > > diff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc > > index 9601fff228..0ba4f9a27e 100644 > > --- a/Documentation/fsck-msgids.adoc > > +++ b/Documentation/fsck-msgids.adoc > > @@ -59,6 +59,12 @@ > > `emptyName`:: > > (WARN) A path contains an empty name. > > > > +`emptyPackedRefsFile`:: > > + (INFO) "packed-refs" file is empty. Report to the > > + git@xxxxxxxxxxxxxxx mailing list if you see this error. As only > > + very early versions of Git would create such an empty > > + "packed_refs" file, we might tighten this rule in the future. > > I am not too happy to see "Report to ..." and everything after that > here, primarily because it takes one extra step for the user to find > it out when they see such an informational message. There are other > existing error classes, like refMissingNewline, etc., that have the > same problem. One thing to make it easier for the users to report > is to put it in the error/info messages themselves, but I think it > is OK to make such a clean-up (including the existing offenders) > after the dust settles from this topic. > That's right. Actually we introduce redirection here. I think I will improve this in the next release cycle. I'll add this into my TODO list.
