On Thu, Jul 10, 2025 at 07:10:02PM -0400, Taylor Blau wrote: > On Wed, Jul 09, 2025 at 09:54:49AM +0200, Patrick Steinhardt wrote: > > Multi-pack indices are tracked via `struct multi_pack_index`. This data > > structure is stored as a linked list inside `struct object_database`, > > which is the global database that spans across all of the object > > sources. > > > > This layout causes two problems: > > > > - Multi-pack indices aren't global to an object database, but instead > > there can be one multi-pack index per source. This creates a > > mismatch between the on-disk layout and how things are organized in > > the object database subsystems and makes some parts, like figuring > > out whether a source has an MIDX, quite awkward. > > This is a little confusing to me. What do we consider to be an "object > database", and what do we consider to be a "source"? > > For instance, if I have a repository with one or more alternates, I > would imagine that each alternate is a separate "source", and the > sources together comprise the object database. Does that match the way > you're thinking about it? > > If so, that makes sense. But if not (i.e., we consider all alternates to > belong to the same object database and share a single source), then I > don't know how this will interact with the existing MIDX alternates > mechanism. > > Some clarification here would be helpful, I think. I'll add some, sure. This whole infra is currently developing, so it doesn't hurt to reiterate some of the points that have already landed. > > diff --git a/midx.c b/midx.c > > index 3c5bc821730..a91231bfcdf 100644 > > --- a/midx.c > > +++ b/midx.c > > @@ -724,28 +724,29 @@ int midx_preferred_pack(struct multi_pack_index *m, uint32_t *pack_int_id) > > return 0; > > } > > > > -int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local) > > +int prepare_multi_pack_index_one(struct odb_source *source, int local) > > OK, so the combination of a repository and "object_dir" path becomes a > new "struct odb_source", which makes sense. But shouldn't "local" also > be a property of the odb_source? > > I am not familiar enough with the odb_source work thus far to know > whether that is a good idea or not, but just something that stood out > to me while reading. Yes, you're right on point. I have a follow-up patch series that'll eventually get rid of some duplicate information that is now present in the ODB source connected to an MIDX. [snip] > > + source->multi_pack_index = m; > > As an aside, I see that we're calling this new field "multi_pack_index". > I wonder if it would be better to call it "midx", since typing out > "multi_pack_index" is a little verbose, and "midx" is a common > abbreviation that I don't think we would cause any confusion with. Sure, I can do that. > > diff --git a/midx.h b/midx.h > > index 9d1374cbd58..b1626a9a7c4 100644 > > --- a/midx.h > > +++ b/midx.h > > @@ -3,11 +3,12 @@ > > > > #include "string-list.h" > > > > +struct bitmapped_pack; > > +struct git_hash_algo; > > struct object_id; > > +struct odb_source; > > struct pack_entry; > > struct repository; > > -struct bitmapped_pack; > > -struct git_hash_algo; > > (I'm nit-picking, but a similar note here that the unrelated changes > make it harder to see what is actually going on in this hunk, which is > adding a declaration for "struct odb_source".) > > > diff --git a/odb.h b/odb.h > > index e922f256802..8e79c7be520 100644 > > --- a/odb.h > > +++ b/odb.h > > @@ -9,10 +
