Re: [PATCH v2 01/10] bundle: optionally skip reachability walk

From: Derrick Stolee <derrickstolee@github.com>
Date: Jan 24, 2023, 12:27 UTC
Message-ID: <771a2993-85bd-0831-0977-24204f84e206@github.com>
In-Reply-To: <xmqqtu0glw81.fsf@gitster.g>

On 1/23/2023 5:30 PM, Junio C Hamano wrote:

Show 16 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
>> A repository having some unreachable objects floating in the object
>> store is not corrupt.  As long as all the objects reachable from refs
>> are connected, that is a perfectly sane state.
>>
>> But allowing unbundling with the sanity check loosened WILL corrupt
>> it, at the moment you point some objects from the bundle with refs.
> 
> While all of the above is true, I think existing check done by
> bundle.c::verify_bundle() is stricter than necessary.  We make sure
> that the prerequiste objects exist and are reachable from the refs.
> But for the purpose of ensuring the health of the repo after the
> operation, it is also OK if the prerequisite objects exist and they
> pass connected.c::check_connected() test to reach existing refs.
> verify_bundle() that is used in unbundle() does not allow it.

Thank you for all of the detailed explanation, here and in other messages.

I'll focus on this area today and see what I can learn and how I can approach this problem in a different way. The current options that I see are:

 1. Leave verify_bundle() as-is and figure out how to refresh the
    refs. (This would remain a stricter check than necessary.)

 2. Find out how to modify verify_bundle() so it can do the more
    relaxed connectivity check.

 3. Take the connectivity check that fetch uses before updating
    refs and add that check before updating refs in the bundle URI
    code.

There could also be a combination of (2) and (3), or others I have not considered until I go poking around in the code.

I'll let you know what I find.

Thanks, -Stolee