Re: [PATCH 4/8] midx: track whether we have loaded the MIDX

From: Justin Tobler <jltobler@gmail.com>
Date: Jul 10, 2025, 22:16 UTC
Message-ID: <kmprav735ntlgo3x47ptxyhqc6c5n7tyyub5c545gw2g4achng@sst72rcazce4>
In-Reply-To: <20250709-b4-pks-midx-via-odb-alternate-v1-4-f31150d21331@pks.im>

On 25/07/09 09:54AM, Patrick Steinhardt wrote:

Show 11 quoted lines

> When calling `prepare_multi_pack_index_one()` we know to skip loading a
> multi-pack index that we have already loaded beforehand. While this
> works well in case there actually is a multi-pack index, it doesn't work
> when we already tried to load a nonexistent one.
> 
> This doesn't cause problems with the current layout, where users
> typically iterate through MIDXs via the linked list stored in the object
> database. But that linked list is going away, and those users will
> instead have to call `get_multi_pack_index()` for each object source. So
> if one of those sources doesn't have an MIDX, we may end up trying to
> repeatedly load it even though we know it doesn't exist.

IIUC, in its current form `get_multi_pack_index()` returns the global list of MIDXs. The MIDXs are loaded when calling `prepare_packed_git()` into both the global `r->objects->multi_pack_index` and `source->multi_pack_index` for each source as appropriate.

Looking at `prepare_packed_git()`, it checks `r->objects->packed_git_initialized` to see if it has already been initialized. If the intent is to start calling `get_multi_pack_index()` for each source individually, doesn't `prepare_packed_git()` still only execute once regardless already?

> Address this issue by introducing a new variable that tracks whether we
> have tried to load multi-pack index of a given source.

The contents of the patch look good, but I'm not entirely sure introducing a separate variable to track if the source has attempted to load a MIDX is useful.

-Justin