Re:Re: [PATCH] bundle-uri: refresh packed_git if unbundle succeed

From: Xing Xin <bupt_xingxin@163.com>
Date: May 20, 2024, 11:48 UTC
Message-ID: <5e61daf6.ba64.18f95d49895.Coremail.bupt_xingxin@163.com>
In-Reply-To: <xmqqikzcl8js.fsf@gitster.g>

At 2024-05-18 00:14:47, "Junio C Hamano" <gitster@pobox.com> wrote:

Show 23 quoted lines

>Patrick Steinhardt <ps@pks.im> 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