On 25/07/09 09:54AM, Patrick Steinhardt wrote: > 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