At 2024-05-18 00:14:47, "Junio C Hamano" <gitster@xxxxxxxxx> wrote:
>Patrick Steinhardt <ps@xxxxxx> writes:
>
>> Now there are two conflicting thoughts here:
>>
>> - Either we can now drop `REF_SKIP_OID_VERIFICATION` as the object IDs
>> should now be accessible.
>>
>> - Or we can avoid calling `reprepare_packed_git()` inside the loop and
>> instead call it once after we have fetched all bundles.
>>
>> The second one feels a bit like premature optimization to me. But the
>> first item does feel like it could help us to catch broken bundles
>> because we wouldn't end up creating refs for objects that neither we nor
>> the bundle have.
>
>I like the way your thoughts are structured around here.
>
>I do agree that the latter is a wrong approach---we shouldn't be
>trusting what came from elsewhere over the network without first
>checking. We should probably be running the "index-pack --fix-thin"
>the unbundling process runs with also the "--fsck-objects" option if
>we are not doing so already, and even then, we should make sure that
>the object we are making our ref point at have everything behind it.
Currently `unbundle` and all its callers are not adding "--fsck-objects".
There is a param `extra_index_pack_args` for `unbundle` but it is
mainly used for progress related options.
Personally I think data from bundles and data received via network
should be treated equally. For "fetch-pack" we now have some configs
such as "fetch.fsckobjects" and "transfer.fsckobjects" to decide the
behavior, these configs are invisible when we are fetching bundles.
So for bundles I think we have some different ways to go:
- Acknowledge the configs mentioned above and behave like
"fetch-pack".
- Add "--fsck-objects" as a default in `unbundle`.
- In `unbundle_from_file`, pass in "--fsck-objects" as
`extra_index_pack_args` for `unbundle` so this only affect the
bundle-uri related process.
What do you think?
Xing Xin
