From: Karthik Nayak Date: Thu, 07 Aug 2025 08:51:33 GMT Subject: Re: [PATCH 5/8] midx: load multi-pack indices via their source Message-ID: In-Reply-To: Karthik Nayak writes: > Patrick Steinhardt writes: > >> To load a multi-pack index the caller is expected to pass both the >> repository and the object directory where the multi-pack index is >> located. While this works, this layout has a couple of downsides: >> >> - We need to pass in information reduntant with the owning source, >> namely its object directory and whether the source is local or not. >> >> - We don't have access to the source when loading the multi-pack >> index. If we had that access, we could store a pointer to the owning >> source in the MIDX and thus deduplicate some information. >> >> - Multi-pack indices are inherently specific to the object source and >> its format. With the goal of pluggable object backends in mind we >> will eventually want the backends to own the logic of reading and >> writing multi-pack indices. Making the logic work on top of object >> sources is a step into that direction. >> >> Refactor loading of multi-pack indices accordingly. >> >> This surfaces one small problem though: git-multi-pack-index(1) and our >> MIDX test helper both know to read and write multi-pack-indices located >> in a different object directory. This issue is addressed by adding the >> user-provided object directory as an in-memory alternate. >> > > Doesn't this commit only fix the 'read' side of things i.e. > 'cmd_multi_pack_index_verify'. > > Shouldn't we squash the next commit into this? Otherwise, writing midx > present in a different object directory is broken as of this commit no? > For e.g. in 'cmd_multi_pack_index_expire' we call > 'handle_object_dir_option' which would add it as an alternate obd, but > we don't use the return value at all. > > [snip] Reading into the next commit, I see these paths internally call 'lookup_multi_pack_index' which would find the right source, so this commit does work as expected. So all good.