shejialuo <shejialuo@xxxxxxxxx> writes: > In "packed-backend.c::create_snapshot", if there is a header (the line > which starts with '#'), we will check whether the line starts with "# > pack-refs with:". Before we port this check into "packed_fsck", let's > fix "create_snapshot" to check the prefix "# packed-ref with: " instead > of "# packed-ref with:" due to that we will always write a single > trailing space after the colon. A more important reason to be more strict is not "we will always write", but "we HAVE ALWAYS written", I think. > However, we need to consider other situations and discuss whether we > need to add checks. > > 1. If the header does not exist, we should not report an error to the > user. This is because in older Git version, we never write header in > the "packed-refs" file. Also, we do allow no header in "packed-refs" > in runtime. Yes. > 2. If the header content does not start with "# packed-ref with: ", we > should report an error just like what "create_snapshot" does. So, > create a new fsck message "badPackedRefHeader(ERROR)" for this. OK. > 3. If the header content is not the same as the constant string > "PACKED_REFS_HEADER". This is expected because we make it extensible > intentionally. So, there is no need to report. Nor there is any need to check for literal equality with the constant string. We may want to split the traits that are recorded on the "with:" line and see if there are ones that we do not recognise if only for curiosity, but because create_snapshot(), which is the only run-time consumer of this information, only uses the ones it recognises while ignoring everything else, presence of an unknown trait is not an error- or even warning-worthy event. Unless we are curious and want to emit "info" level message, there is not much point in checking the remainder of the header.
