On 4/17/2023 6:01 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
>> if (!hashfile_checksum_valid((const unsigned char *)p->revindex_map, p->revindex_size)) {
>> error(_("invalid checksum"));
>> - return -1;
>> + res = -1;
>> }
>>
>> - return 0;
>> + /* This may fail due to a broken .idx. */
>> + if (create_pack_revindex_in_memory(p))
>> + return res;
>
> Here, if the revindex file itself is self consistent, res would
> still be 0, so we will end up silently returning. Is the idea that
> while whatever causes in-memory revindex fail to be computed is a
> concern from the point of view of the repository's health, it would
> be caught elsewhere as a problem for the <pack,idx> pair we are
> seeing here?
This is something I noticed during a test suite run, where the .idx
was corrupt, so the .rev file could not be checked. In this situation,
we are not using the .rev file for anything since Git processes would
fail to lookup objects in the associated packfile.
In this case, we have no way to validate the rest of the .rev file's
contents, but at least we checked its header and checksum, which is
the best we can hope to do in this case.
Thanks,
-Stolee
