patch, 8 partsodb: track multi-pack-indices via their object sources

> 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.

> Refactor `prepare_multi_pack_index_one()` so that it works on a specific
> source, which allows us to easily store a pointer to the multi-pack
> index inside of it. For now, this pointer exists next to the existing
> linked list we have in the object database. Users will be adjusted in
> subsequent patches to instead use the per-source pointers.

> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  midx.c     | 19 +++++++++++--------
>  midx.h     |  7 ++++---
>  odb.h      | 11 +++++++++--
>  packfile.c |  4 +++-
>  4 files changed, 27 insertions(+), 14 deletions(-)
> 
> 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)
>  {
> +	struct repository *r = source->odb->repo;
>  	struct multi_pack_index *m;
> -	struct multi_pack_index *m_search;
>  
>  	prepare_repo_settings(r);
>  	if (!r->settings.core_multi_pack_index)
>  		return 0;
>  
> -	for (m_search = r->objects->multi_pack_index; m_search; m_search = m_search->next)
> -		if (!strcmp(object_dir, m_search->object_dir))
> -			return 1;

> -	m = load_multi_pack_index(r, object_dir, local);
> +	if (source->multi_pack_index)
> +		return 1;
>  
> +	m = load_multi_pack_index(r, source->path, local);
>  	if (m) {
>  		struct multi_pack_index *mp = r->objects->multi_pack_index;
>  		if (mp) {
>  			m->next = mp->next;
>  			mp->next = m;
> -		} else
> +		} else {
>  			r->objects->multi_pack_index = m;
> +		}
> +		source->multi_pack_index = m;

> +
>  		return 1;
>  	}
>  
> @@ -837,6 +838,8 @@ void clear_midx_file(struct repository *r)
>  	if (r->objects && r->objects->multi_pack_index) {
>  		close_midx(r->objects->multi_pack_index);
>  		r->objects->multi_pack_index = NULL;
> +		for (struct odb_source *source = r->objects->sources; source; source = source->next)
> +			source->multi_pack_index = NULL;
>  	}
>  
>  	if (remove_path(midx.buf))
> 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;
>  
>  #define MIDX_SIGNATURE 0x4d494458 /* "MIDX" */
>  #define MIDX_VERSION 1
> @@ -123,7 +124,7 @@ int fill_midx_entry(struct repository *r, const struct object_id *oid, struct pa
>  int midx_contains_pack(struct multi_pack_index *m,
>  		       const char *idx_or_pack_name);
>  int midx_preferred_pack(struct multi_pack_index *m, uint32_t *pack_int_id);
> -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);
>  
>  /*
>   * Variant of write_midx_file which writes a MIDX containing only the packs
> diff --git a/odb.h b/odb.h
> index e922f256802..8e79c7be520 100644
> --- a/odb.h
> +++ b/odb.h
> @@ -9,10 +9,11 @@
>  #include "string-list.h"
>  #include "thread-utils.h"
>  
> +struct multi_pack_index;
>  struct oidmap;
>  struct oidtree;
> -struct strbuf;
>  struct repository;
> +struct strbuf;
>  
>  /*
>   * Compute the exact path an alternate is at and returns it. In case of
> @@ -55,6 +56,13 @@ struct odb_source {
>  	/* Map between object IDs for loose objects. */
>  	struct loose_object_map *loose_map;
>  
> +	/*
> +	 * private data
> +	 *
> +	 * should only be accessed directly by packfile.c and midx.c
> +	 */
> +	struct multi_pack_index *multi_pack_index;
> +
>  	/*
>  	 * This is a temporary object store created by the tmp_objdir
>  	 * facility. Disable ref updates since the objects in the store
> @@ -75,7 +83,6 @@ struct odb_source {
>  };
>  
>  struct packed_git;
> -struct multi_pack_index;
>  struct cached_object_entry;
>  
>  /*
> diff --git a/packfile.c b/packfile.c
> index af9ccfdba62..16efc2fdca3 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -372,6 +372,8 @@ void close_object_store(struct object_database *o)
>  	if (o->multi_pack_index) {
>  		close_midx(o->multi_pack_index);
>  		o->multi_pack_index = NULL;
> +		for (struct odb_source *source = o->sources; source; source = source->next)
> +			source->multi_pack_index = NULL;

>  	}
>  
>  	close_commit_graph(o);
> @@ -1037,7 +1039,7 @@ static void prepare_packed_git(struct repository *r)
>  	odb_prepare_alternates(r->objects);
>  	for (source = r->objects->sources; source; source = source->next) {
>  		int local = (source == r->objects->sources);
> -		prepare_multi_pack_index_one(r, source->path, local);
> +		prepare_multi_pack_index_one(source, local);
>  		prepare_packed_git_one(r, source->path, local);
>  	}
>  	rearrange_packed_git(r);
> 
> -- 
> 2.50.1.327.g047016eb4a.dirty
> 
> 

> 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.

> Refactor `prepare_multi_pack_index_one()` so that it works on a specific
> source, which allows us to easily store a pointer to the multi-pack
> index inside of it. For now, this pointer exists next to the existing
> linked list we have in the object database. Users will be adjusted in
> subsequent patches to instead use the per-source pointers.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  midx.c     | 19 +++++++++++--------
>  midx.h     |  7 ++++---
>  odb.h      | 11 +++++++++--
>  packfile.c |  4 +++-
>  4 files changed, 27 insertions(+), 14 deletions(-)
>
> 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)

>  {
> +	struct repository *r = source->odb->repo;
>  	struct multi_pack_index *m;
> -	struct multi_pack_index *m_search;
>
>  	prepare_repo_settings(r);
>  	if (!r->settings.core_multi_pack_index)
>  		return 0;
>
> -	for (m_search = r->objects->multi_pack_index; m_search; m_search = m_search->next)
> -		if (!strcmp(object_dir, m_search->object_dir))
> -			return 1;
> -
> -	m = load_multi_pack_index(r, object_dir, local);
> +	if (source->multi_pack_index)
> +		return 1;

> +	m = load_multi_pack_index(r, source->path, local);
>  	if (m) {
>  		struct multi_pack_index *mp = r->objects->multi_pack_index;
>  		if (mp) {
>  			m->next = mp->next;
>  			mp->next = m;
> -		} else
> +		} else {
>  			r->objects->multi_pack_index = m;
> +		}

> @@ -837,6 +838,8 @@ void clear_midx_file(struct repository *r)
>  	if (r->objects && r->objects->multi_pack_index) {
>  		close_midx(r->objects->multi_pack_index);
>  		r->objects->multi_pack_index = NULL;
> +		for (struct odb_source *source = r->objects->sources; source; source = source->next)
> +			source->multi_pack_index = NULL;

> 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;

> diff --git a/odb.h b/odb.h
> index e922f256802..8e79c7be520 100644
> --- a/odb.h
> +++ b/odb.h
> @@ -9,10 +9,11 @@
>  #include "string-list.h"
>  #include "thread-utils.h"
>
> +struct multi_pack_index;
>  struct oidmap;
>  struct oidtree;
> -struct strbuf;
>  struct repository;
> +struct strbuf;

> 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.

> 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.
> 
> Ahh..., after applying part of this series locally, the documentation in
> "struct odb_source" clarifies that (at least as of yet) there is one
> source per alternate, along with a source for the local object database,
> and that these collection of sources comprise the object database.
> 
> Makes sense.

> 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.

> > 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.

> > +		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.

> > 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 +

> load the MIDX. As part of this refactoring we started to store a pointer
> to the MIDX in `struct odb_source` itself.
> 
> Refactor loading of packfiles in the same way: instead of passing in the
> object directory, we now pass in the source for which we want to load
> packfiles. This allows us to simplify the code because we don't have to
> search for a corresponding MIDX anymore, but we can instead directly use
> the MIDX that we have already prepared beforehand.

> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  packfile.c | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/packfile.c b/packfile.c
> index 16efc2fdca3..b43dd2fe6cb 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -935,22 +935,17 @@ static void prepare_pack(const char *full_name, size_t full_name_len,
>  		report_garbage(PACKDIR_FILE_GARBAGE, full_name);
>  }
>  
> -static void prepare_packed_git_one(struct repository *r, char *objdir, int local)
> +static void prepare_packed_git_one(struct odb_source *source, int local)
>  {
> -	struct prepare_pack_data data;
>  	struct string_list garbage = STRING_LIST_INIT_DUP;
> +	struct prepare_pack_data data = {
> +		.m = source->multi_pack_index,
> +		.r = source->odb->repo,
> +		.garbage = &garbage,
> +		.local = local,
> +	};
>  
> -	data.m = r->objects->multi_pack_index;
> -
> -	/* look for the multi-pack-index for this object directory */
> -	while (data.m && strcmp(data.m->object_dir, objdir))
> -		data.m = data.m->next;

> -
> -	data.r = r;
> -	data.garbage = &garbage;
> -	data.local = local;
> -
> -	for_each_file_in_pack_dir(objdir, prepare_pack, &data);
> +	for_each_file_in_pack_dir(source->path, prepare_pack, &data);
>  
>  	report_pack_garbage(data.garbage);
>  	string_list_clear(data.garbage, 0);
> @@ -1040,7 +1035,7 @@ static void prepare_packed_git(struct repository *r)
>  	for (source = r->objects->sources; source; source = source->next) {
>  		int local = (source == r->objects->sources);
>  		prepare_multi_pack_index_one(source, local);
> -		prepare_packed_git_one(r, source->path, local);
> +		prepare_packed_git_one(source, local);
>  	}
>  	rearrange_packed_git(r);
>  
> 
> -- 
> 2.50.1.327.g047016eb4a.dirty
> 
> 

> packfiles. This allows us to simplify the code because we don't have to
> search for a corresponding MIDX anymore, but we can instead directly use
> the MIDX that we have already prepared beforehand.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  packfile.c | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/packfile.c b/packfile.c
> index 16efc2fdca3..b43dd2fe6cb 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -935,22 +935,17 @@ static void prepare_pack(const char *full_name, size_t full_name_len,
>  		report_garbage(PACKDIR_FILE_GARBAGE, full_name);
>  }
>
> -static void prepare_packed_git_one(struct repository *r, char *objdir, int local)
> +static void prepare_packed_git_one(struct odb_source *source, int local)
>  {
> -	struct prepare_pack_data data;
>  	struct string_list garbage = STRING_LIST_INIT_DUP;
> +	struct prepare_pack_data data = {
> +		.m = source->multi_pack_index,
> +		.r = source->odb->repo,
> +		.garbage = &garbage,
> +		.local = local,
> +	};
>
> -	data.m = r->objects->multi_pack_index;
> -
> -	/* look for the multi-pack-index for this object directory */
> -	while (data.m && strcmp(data.m->object_dir, objdir))
> -		data.m = data.m->next;

> On Wed, Jul 09, 2025 at 09:54:50AM +0200, Patrick Steinhardt wrote:
> > In the preceding commit we have refactored how we load multi-pack
> > indices so that we take take the source as input for which we want to
> > load the MIDX. As part of this refactoring we started to store a pointer
> > to the MIDX in `struct odb_source` itself.
> 
> The first sentence here is a little confusing, but may read more clearly
> if written as:
> 
>     In the preceding commit we refactored how we load multi-pack
>     indices to take a corresponding "source" as input.

> > diff --git a/packfile.c b/packfile.c
> > index 16efc2fdca3..b43dd2fe6cb 100644
> > --- a/packfile.c
> > +++ b/packfile.c
> > @@ -935,22 +935,17 @@ static void prepare_pack(const char *full_name, size_t full_name_len,
> >  		report_garbage(PACKDIR_FILE_GARBAGE, full_name);
> >  }
> >
> > -static void prepare_packed_git_one(struct repository *r, char *objdir, int local)
> > +static void prepare_packed_git_one(struct odb_source *source, int local)
> >  {
> > -	struct prepare_pack_data data;
> >  	struct string_list garbage = STRING_LIST_INIT_DUP;
> > +	struct prepare_pack_data data = {
> > +		.m = source->multi_pack_index,
> > +		.r = source->odb->repo,
> > +		.garbage = &garbage,
> > +		.local = local,
> > +	};
> >
> > -	data.m = r->objects->multi_pack_index;
> > -
> > -	/* look for the multi-pack-index for this object directory */
> > -	while (data.m && strcmp(data.m->object_dir, objdir))
> > -		data.m = data.m->next;
> 
> Right, since we know that the MIDX corresponding to this source belongs
> to the same "object_dir" path. Having an ASSERT() here may make that
> more clear, but this change looks correct to me.
> 
> I am still a little unclear on how sources and ODBs correspond to one
> another, but under my working assumption from the previous patch, I
> think this is right.

> When calling `close_midx()` we not only close the multi-pack index for
> one object source, but instead we iterate through the whole linked list
> of MIDXs to close all of them. This linked list is about to go away in
> favor of using the new per-source pointer to its respective MIDX.
> 
> Refactor the function to iterate through sources instead.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  midx.c     | 11 ++++++-----
>  packfile.c | 10 +++++-----
>  2 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/midx.c b/midx.c
> index a91231bfcdf..416b3e8b54f 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -401,7 +401,6 @@ void close_midx(struct multi_pack_index *m)
>  	if (!m)
>  		return;
>  
> -	close_midx(m->next);

>  	close_midx(m->base_midx);
>  
>  	munmap((unsigned char *)m->data, m->data_len);
> @@ -835,11 +834,13 @@ void clear_midx_file(struct repository *r)
>  
>  	get_midx_filename(r->hash_algo, &midx, r->objects->sources->path);
>  
> -	if (r->objects && r->objects->multi_pack_index) {
> -		close_midx(r->objects->multi_pack_index);
> -		r->objects->multi_pack_index = NULL;
> -		for (struct odb_source *source = r->objects->sources; source; source = source->next)
> +	if (r->objects) {
> +		for (struct odb_source *source = r->objects->sources; source; source = source->next) {
> +			if (source->multi_pack_index)
> +				close_midx(source->multi_pack_index);
>  			source->multi_pack_index = NULL;
> +		}
> +		r->objects->multi_pack_index = NULL;

>  	}
>  
>  	if (remove_path(midx.buf))
> diff --git a/packfile.c b/packfile.c
> index b43dd2fe6cb..546c161d0c1 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -369,12 +369,12 @@ void close_object_store(struct object_database *o)
>  		else
>  			close_pack(p);
>  
> -	if (o->multi_pack_index) {
> -		close_midx(o->multi_pack_index);
> -		o->multi_pack_index = NULL;
> -		for (struct odb_source *source = o->sources; source; source = source->next)
> -			source->multi_pack_index = NULL;
> +	for (struct odb_source *source = o->sources; source; source = source->next) {
> +		if (source->multi_pack_index)
> +			close_midx(source->multi_pack_index);
> +		source->multi_pack_index = NULL;
>  	}
> +	o->multi_pack_index = NULL;

> On 25/07/09 09:54AM, Patrick Steinhardt wrote:
> > When calling `close_midx()` we not only close the multi-pack index for
> > one object source, but instead we iterate through the whole linked list
> > of MIDXs to close all of them. This linked list is about to go away in
> > favor of using the new per-source pointer to its respective MIDX.
> > 
> > Refactor the function to iterate through sources instead.
> 
> The `close_midx()` function itself is not iterating though the sources.
> Rather each of the callsites are now resposible to ensure `close_midx()`
> is called separately for each source. It might be nice to clarify this a
> bit in the message.
> 
> I also noticed that there are several other existing `close_midx()`
> callsites that we leave as-is. Each of these sites though looks like
> they don't care about globally closing all MIDXs so they should be fine.
> This might also we worth mentioning.

> When calling `close_midx()` we not only close the multi-pack index for
> one object source, but instead we iterate through the whole linked list
> of MIDXs to close all of them. This linked list is about to go away in
> favor of using the new per-source pointer to its respective MIDX.
>
> Refactor the function to iterate through sources instead.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  midx.c     | 11 ++++++-----
>  packfile.c | 10 +++++-----
>  2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/midx.c b/midx.c
> index a91231bfcdf..416b3e8b54f 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -401,7 +401,6 @@ void close_midx(struct multi_pack_index *m)
>  	if (!m)
>  		return;
>
> -	close_midx(m->next);

>  	close_midx(m->base_midx);
>
>  	munmap((unsigned char *)m->data, m->data_len);
> @@ -835,11 +834,13 @@ void clear_midx_file(struct repository *r)
>
>  	get_midx_filename(r->hash_algo, &midx, r->objects->sources->path);
>
> -	if (r->objects && r->objects->multi_pack_index) {
> -		close_midx(r->objects->multi_pack_index);
> -		r->objects->multi_pack_index = NULL;
> -		for (struct odb_source *source = r->objects->sources; source; source = source->next)
> +	if (r->objects) {
> +		for (struct odb_source *source = r->objects->sources; source; source = source->next) {
> +			if (source->multi_pack_index)
> +				close_midx(source->multi_pack_index);
>  			source->multi_pack_index = NULL;

>  	}
>
>  	if (remove_path(midx.buf))
> diff --git a/packfile.c b/packfile.c
> index b43dd2fe6cb..546c161d0c1 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -369,12 +369,12 @@ void close_object_store(struct object_database *o)
>  		else
>  			close_pack(p);
>
> -	if (o->multi_pack_index) {
> -		close_midx(o->multi_pack_index);
> -		o->multi_pack_index = NULL;
> -		for (struct odb_source *source = o->sources; source; source = source->next)
> -			source->multi_pack_index = NULL;
> +	for (struct odb_source *source = o->sources; source; source = source->next) {

> On Wed, Jul 09, 2025 at 09:54:51AM +0200, Patrick Steinhardt wrote:
> > diff --git a/midx.c b/midx.c
> > index a91231bfcdf..416b3e8b54f 100644
> > --- a/midx.c
> > +++ b/midx.c
> > @@ -835,11 +834,13 @@ void clear_midx_file(struct repository *r)
> >
> >  	get_midx_filename(r->hash_algo, &midx, r->objects->sources->path);
> >
> > -	if (r->objects && r->objects->multi_pack_index) {
> > -		close_midx(r->objects->multi_pack_index);
> > -		r->objects->multi_pack_index = NULL;
> > -		for (struct odb_source *source = r->objects->sources; source; source = source->next)
> > +	if (r->objects) {
> > +		for (struct odb_source *source = r->objects->sources; source; source = source->next) {
> > +			if (source->multi_pack_index)
> > +				close_midx(source->multi_pack_index);
> >  			source->multi_pack_index = NULL;
> 
> ...and then this line would NULL the now-free()'d memory out.
> 
> But instead we are directly iterating through the sources and both
> closing and NULL-ing out their respective MIDXs (if any).
> 
> As an aside: I know we do the C99-style for loop with declarations in
> many places, but in this instance it seems to have produced an awfully
> long line. I wonder if in this instance it would be better to write:
> 
>     struct odb_source *source;
>     for (source = r->objects->sources; source; source = source->next) {
>         /* ... */
>     }
> 
> That's still a little lengthy, but it's fewer than 80 characters ;-).

> > +		}
> > +		r->objects->multi_pack_index = NULL;
> 
> Presumably this pointer will go away at some point in the future as
> well?

> 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.

> 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.

> On Thu, Jul 10, 2025 at 05:16:12PM -0500, Justin Tobler wrote:
> > 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?
> 
> I was wondering the same thing. Perhaps that packed_git_initialized
> field is going away sometime in the future and this is its logical
> replacement (at least for MIDXs)?
> 
> In either case, that would be worth clarifying. If it's not doing
> anything (i.e., because we have no plans to get rid of
> packed_git_initialized), then I agree that this patch could probably be
> dropped, but I suspect that I don't have the full picture in my head yet.
> 
> > > 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.
> 
> Yup.

> The function `get_multi_pack_index()` loads multi-pack indices via
> `prepare_packed_git()` and then returns the linked list of multi-pack
> indices that is stored in `struct object_database`. That list is in the
> process of being removed though in favor of storing the MIDX as part of
> the object database source it belongs to.
> 
> Refactor `get_multi_pack_index()` so that it returns the multi-pack
> index for a single object source. Callers are now expected to call this
> function for each source they are interested in. This requires them to
> iterate through alternates, so we have to prepare alternate object
> sources before doing so.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/pack-objects.c |  9 ++++++---
>  builtin/repack.c       |  4 ++--
>  midx-write.c           | 22 ++--------------------
>  object-name.c          | 21 ++++++++++++++-------
>  pack-bitmap.c          | 20 ++++++++++++++------
>  packfile.c             | 30 +++++++++++-------------------
>  packfile.h             |  3 +--
>  7 files changed, 50 insertions(+), 59 deletions(-)
> 
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 5781dec9808..f889e69e07d 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1701,7 +1701,6 @@ static int want_object_in_pack_mtime(const struct object_id *oid,
>  {
>  	int want;
>  	struct list_head *pos;
> -	struct multi_pack_index *m;
>  
>  	if (!exclude && local && has_loose_object_nonlocal(oid))
>  		return 0;
> @@ -1721,9 +1720,13 @@ static int want_object_in_pack_mtime(const struct object_id *oid,
>  		*found_offset = 0;
>  	}
>  
> -	for (m = get_multi_pack_index(the_repository); m; m = m->next) {
> +	odb_prepare_alternates(the_repository->objects);

> +
> +	for (struct odb_source *source = the_repository->objects->sources; source; source = source->next) {
> +		struct multi_pack_index *m = get_multi_pack_index(source);
>  		struct pack_entry e;
> -		if (fill_midx_entry(the_repository, oid, &e, m)) {
> +
> +		if (m && fill_midx_entry(the_repository, oid, &e, m)) {
>  			want = want_object_in_pack_one(e.p, oid, exclude, found_pack, found_offset, found_mtime);
>  			if (want != -1)
>  				return want;
> diff --git a/builtin/repack.c b/builtin/repack.c
> index 9bbf032b6dd..5956df5d927 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -218,9 +218,9 @@ static void mark_packs_for_deletion(struct existing_packs *existing,
>  static void remove_redundant_pack(const char *dir_name, const char *base_name)
>  {
>  	struct strbuf buf = STRBUF_INIT;
> -	struct multi_pack_index *m = get_local_multi_pack_index(the_repository);
> +	struct multi_pack_index *m = get_multi_pack_index(the_repository->objects->sources);
>  	strbuf_addf(&buf, "%s.pack", base_name);
> -	if (m && midx_contains_pack(m, buf.buf))
> +	if (m && m->local && midx_contains_pack(m, buf.buf))
>  		clear_midx_file(the_repository);
>  	strbuf_insertf(&buf, 0, "%s/", dir_name);
>  	unlink_pack_path(buf.buf, 1);
> diff --git a/midx-write.c b/midx-write.c
> index f2cfb85476e..c1ae62d3549 100644
> --- a/midx-write.c
> +++ b/midx-write.c
> @@ -916,26 +916,8 @@ static int write_midx_bitmap(struct write_midx_context *ctx,
>  static struct multi_pack_index *lookup_multi_pack_index(struct repository *r,
>  							const char *object_dir)
>  {
> -	struct multi_pack_index *result = NULL;
> -	struct multi_pack_index *cur;
> -	char *obj_dir_real = real_pathdup(object_dir, 1);
> -	struct strbuf cur_path_real = STRBUF_INIT;
> -
> -	/* Ensure the given object_dir is local, or a known alternate. */
> -	odb_find_source(r->objects, obj_dir_real);
> -
> -	for (cur = get_multi_pack_index(r); cur; cur = cur->next) {
> -		strbuf_realpath(&cur_path_real, cur->object_dir, 1);
> -		if (!strcmp(obj_dir_real, cur_path_real.buf)) {
> -			result = cur;
> -			goto cleanup;
> -		}
> -	}
> -
> -cleanup:
> -	free(obj_dir_real);
> -	strbuf_release(&cur_path_real);
> -	return result;
> +	struct odb_source *source = odb_find_source(r->objects, object_dir);
> +	return get_multi_pack_index(source);

>  }
>  
>  static int fill_packs_from_midx(struct write_midx_context *ctx,
> diff --git a/object-name.c b/object-name.c
> index ddafe7f9b13..1e7fdcb90a8 100644
> --- a/object-name.c
> +++ b/object-name.c
> @@ -198,16 +198,19 @@ static void unique_in_pack(struct packed_git *p,
>  
>  static void find_short_packed_object(struct disambiguate_state *ds)
>  {
> -	struct multi_pack_index *m;
>  	struct packed_git *p;
>  
>  	/* Skip, unless oids from the storage hash algorithm are wanted */
>  	if (ds->bin_pfx.algo && (&hash_algos[ds->bin_pfx.algo] != ds->repo->hash_algo))
>  		return;
>  
> -	for (m = get_multi_pack_index(ds->repo); m && !ds->ambiguous;
> -	     m = m->next)
> -		unique_in_midx(m, ds);
> +	odb_prepare_alternates(ds->repo->objects);
> +	for (struct odb_source *source = ds->repo->objects->sources; source && !ds->ambiguous; source = source->next) {
> +		struct multi_pack_index *m = get_multi_pack_index(source);
> +		if (m)
> +			unique_in_midx(m, ds);
> +	}
> +
>  	for (p = get_packed_git(ds->repo); p && !ds->ambiguous;
>  	     p = p->next)
>  		unique_in_pack(p, ds);
> @@ -792,11 +795,15 @@ static void find_abbrev_len_for_pack(struct packed_git *p,
>  
>  static void find_abbrev_len_packed(struct min_abbrev_data *mad)
>  {
> -	struct multi_pack_index *m;
>  	struct packed_git *p;
>  
> -	for (m = get_multi_pack_index(mad->repo); m; m = m->next)
> -		find_abbrev_len_for_midx(m, mad);
> +	odb_prepare_alternates(mad->repo->objects);
> +	for (struct odb_source *source = mad->repo->objects->sources; source; source = source->next) {
> +		struct multi_pack_index *m = get_multi_pack_index(source);
> +		if (m)
> +			find_abbrev_len_for_midx(m, mad);
> +	}
> +
>  	for (p = get_packed_git(mad->repo); p; p = p->next)
>  		find_abbrev_len_for_pack(p, mad);
>  }
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 0a4af199c05..7b51d381837 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -692,14 +692,16 @@ static int open_midx_bitmap(struct repository *r,
>  			    struct bitmap_index *bitmap_git)
>  {
>  	int ret = -1;
> -	struct multi_pack_index *midx;
>  
>  	assert(!bitmap_git->map);
>  
> -	for (midx = get_multi_pack_index(r); midx; midx = midx->next) {
> -		if (!open_midx_bitmap_1(bitmap_git, midx))
> +	odb_prepare_alternates(r->objects);
> +	for (struct odb_source *source = r->objects->sources; source; source = source->next) {
> +		struct multi_pack_index *midx = get_multi_pack_index(source);
> +		if (midx && !open_midx_bitmap_1(bitmap_git, midx))
>  			ret = 0;
>  	}
> +
>  	return ret;
>  }
>  
> @@ -3307,9 +3309,15 @@ int verify_bitmap_files(struct repository *r)
>  {
>  	int res = 0;
>  
> -	for (struct multi_pack_index *m = get_multi_pack_index(r);
> -	     m; m = m->next) {
> -		char *midx_bitmap_name = midx_bitmap_filename(m);
> +	odb_prepare_alternates(r->objects);
> +	for (struct odb_source *source = r->objects->sources; source; source = source->next) {
> +		struct multi_pack_index *m = get_multi_pack_index(source);
> +		char *midx_bitmap_name;
> +
> +		if (!m)
> +			continue;
> +
> +		midx_bitmap_name = midx_bitmap_filename(m);
>  		res |= verify_bitmap_file(r->hash_algo, midx_bitmap_name);
>  		free(midx_bitmap_name);
>  	}
> diff --git a/packfile.c b/packfile.c
> index e5d9d7ac8bc..e1ced050451 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -963,14 +963,17 @@ static void prepare_packed_git(struct repository *r);
>  unsigned long repo_approximate_object_count(struct repository *r)
>  {
>  	if (!r->objects->approximate_object_count_valid) {
> -		unsigned long count;
> -		struct multi_pack_index *m;
> +		unsigned long count = 0;
>  		struct packed_git *p;
>  
>  		prepare_packed_git(r);
> -		count = 0;
> -		for (m = get_multi_pack_index(r); m; m = m->next)
> -			count += m->num_objects;
> +
> +		for (struct odb_source *source = r->objects->sources; source; source = source->next) {
> +			struct multi_pack_index *m = get_multi_pack_index(source);
> +			if (m)
> +				count += m->num_objects;
> +		}
> +
>  		for (p = r->objects->packed_git; p; p = p->next) {
>  			if (open_pack_index(p))
>  				continue;
> @@ -1074,21 +1077,10 @@ struct packed_git *get_packed_git(struct repository *r)
>  	return r->objects->packed_git;
>  }
>  
> -struct multi_pack_index *get_multi_pack_index(struct repository *r)
> -{
> -	prepare_packed_git(r);
> -	return r->objects->multi_pack_index;
> -}
> -
> -struct multi_pack_index *get_local_multi_pack_index(struct repository *r)
> +struct multi_pack_index *get_multi_pack_index(struct odb_source *source)
>  {
> -	struct multi_pack_index *m = get_multi_pack_index(r);
> -
> -	/* no need to iterate; we always put the local one first (if any) */
> -	if (m && m->local)
> -		return m;
> -
> -	return NULL;
> +	prepare_packed_git(source->odb->repo);
> +	return source->multi_pack_index;

>  }
>  
>  struct packed_git *get_all_packs(struct repository *r)
> diff --git a/packfile.h b/packfile.h
> index 53c3b7d3b43..f16753f2a9b 100644
> --- a/packfile.h
> +++ b/packfile.h
> @@ -147,8 +147,7 @@ void install_packed_git(struct repository *r, struct packed_git *pack);
>  
>  struct packed_git *get_packed_git(struct repository *r);
>  struct list_head *get_packed_git_mru(struct repository *r);
> -struct multi_pack_index *get_multi_pack_index(struct repository *r);
> -struct multi_pack_index *get_local_multi_pack_index(struct repository *r);
> +struct multi_pack_index *get_multi_pack_index(struct odb_source *source);
>  struct packed_git *get_all_packs(struct repository *r);
>  
>  /*
> 
> -- 
> 2.50.1.327.g047016eb4a.dirty
> 
> 

> The function `get_multi_pack_index()` loads multi-pack indices via
> `prepare_packed_git()` and then returns the linked list of multi-pack
> indices that is stored in `struct object_database`. That list is in the
> process of being removed though in favor of storing the MIDX as part of
> the object database source it belongs to.
>
> Refactor `get_multi_pack_index()` so that it returns the multi-pack
> index for a single object source. Callers are now expected to call this
> function for each source they are interested in. This requires them to
> iterate through alternates, so we have to prepare alternate object
> sources before doing so.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/pack-objects.c |  9 ++++++---
>  builtin/repack.c       |  4 ++--
>  midx-write.c           | 22 ++--------------------
>  object-name.c          | 21 ++++++++++++++-------
>  pack-bitmap.c          | 20 ++++++++++++++------
>  packfile.c             | 30 +++++++++++-------------------
>  packfile.h             |  3 +--
>  7 files changed, 50 insertions(+), 59 deletions(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 5781dec9808..f889e69e07d 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1701,7 +1701,6 @@ static int want_object_in_pack_mtime(const struct object_id *oid,
>  {
>  	int want;
>  	struct list_head *pos;
> -	struct multi_pack_index *m;
>
>  	if (!exclude && local && has_loose_object_nonlocal(oid))
>  		return 0;
> @@ -1721,9 +1720,13 @@ static int want_object_in_pack_mtime(const struct object_id *oid,
>  		*found_offset = 0;
>  	}
>
> -	for (m = get_multi_pack_index(the_repository); m; m = m->next) {
> +	odb_prepare_alternates(the_repository->objects);
> +
> +	for (struct odb_source *source = the_repository->objects->sources; source; source = source->next) {
> +		struct multi_pack_index *m = get_multi_pack_index(source);

>  			want = want_object_in_pack_one(e.p, oid, exclude, found_pack, found_offset, found_mtime);
>  			if (want != -1)
>  				return want;
> diff --git a/builtin/repack.c b/builtin/repack.c
> index 9bbf032b6dd..5956df5d927 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -218,9 +218,9 @@ static void mark_packs_for_deletion(struct existing_packs *existing,
>  static void remove_redundant_pack(const char *dir_name, const char *base_name)
>  {
>  	struct strbuf buf = STRBUF_INIT;
> -	struct multi_pack_index *m = get_local_multi_pack_index(the_repository);
> +	struct multi_pack_index *m = get_multi_pack_index(the_repository->objects->sources);

>  		clear_midx_file(the_repository);
>  	strbuf_insertf(&buf, 0, "%s/", dir_name);
>  	unlink_pack_path(buf.buf, 1);
> diff --git a/midx-write.c b/midx-write.c
> index f2cfb85476e..c1ae62d3549 100644
> --- a/midx-write.c
> +++ b/midx-write.c
> @@ -916,26 +916,8 @@ static int write_midx_bitmap(struct write_midx_context *ctx,
>  static struct multi_pack_index *lookup_multi_pack_index(struct repository *r,
>  							const char *object_dir)
>  {
> -	struct multi_pack_index *result = NULL;
> -	struct multi_pack_index *cur;
> -	char *obj_dir_real = real_pathdup(object_dir, 1);
> -	struct strbuf cur_path_real = STRBUF_INIT;
> -
> -	/* Ensure the given object_dir is local, or a known alternate. */
> -	odb_find_source(r->objects, obj_dir_real);
> -
> -	for (cur = get_multi_pack_index(r); cur; cur = cur->next) {
> -		strbuf_realpath(&cur_path_real, cur->object_dir, 1);
> -		if (!strcmp(obj_dir_real, cur_path_real.buf)) {
> -			result = cur;
> -			goto cleanup;
> -		}
> -	}
> -
> -cleanup:
> -	free(obj_dir_real);
> -	strbuf_release(&cur_path_real);
> -	return result;
> +	struct odb_source *source = odb_find_source(r->objects, object_dir);
> +	return get_multi_pack_index(source);

>  }
>
>  static int fill_packs_from_midx(struct write_midx_context *ctx,
> diff --git a/object-name.c b/object-name.c
> index ddafe7f9b13..1e7fdcb90a8 100644
> --- a/object-name.c
> +++ b/object-name.c
> @@ -198,16 +198,19 @@ static void unique_in_pack(struct packed_git *p,
>
>  static void find_short_packed_object(struct disambiguate_state *ds)
>  {
> -	struct multi_pack_index *m;
>  	struct packed_git *p;
>
>  	/* Skip, unless oids from the storage hash algorithm are wanted */
>  	if (ds->bin_pfx.algo && (&hash_algos[ds->bin_pfx.algo] != ds->repo->hash_algo))
>  		return;
>
> -	for (m = get_multi_pack_index(ds->repo); m && !ds->ambiguous;
> -	     m = m->next)
> -		unique_in_midx(m, ds);
> +	odb_prepare_alternates(ds->repo->objects);
> +	for (struct odb_source *source = ds->repo->objects->sources; source && !ds->ambiguous; source = source->next) {
> +		struct multi_pack_index *m = get_multi_pack_index(source);
> +		if (m)
> +			unique_in_midx(m, ds);
> +	}
> +

> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 0a4af199c05..7b51d381837 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -692,14 +692,16 @@ static int open_midx_bitmap(struct repository *r,
>  			    struct bitmap_index *bitmap_git)
>  {
>  	int ret = -1;
> -	struct multi_pack_index *midx;
>
>  	assert(!bitmap_git->map);
>
> -	for (midx = get_multi_pack_index(r); midx; midx = midx->next) {
> -		if (!open_midx_bitmap_1(bitmap_git, midx))
> +	odb_prepare_alternates(r->objects);
> +	for (struct odb_source *source = r->objects->sources; source; source = source->next) {
> +		struct multi_pack_index *midx = get_multi_pack_index(source);
> +		if (midx && !open_midx_bitmap_1(bitmap_git, midx))
>  			ret = 0;
>  	}

> @@ -3307,9 +3309,15 @@ int verify_bitmap_files(struct repository *r)
>  {
>  	int res = 0;
>
> -	for (struct multi_pack_index *m = get_multi_pack_index(r);
> -	     m; m = m->next) {
> -		char *midx_bitmap_name = midx_bitmap_filename(m);
> +	odb_prepare_alternates(r->objects);
> +	for (struct odb_source *source = r->objects->sources; source; source = source->next) {
> +		struct multi_pack_index *m = get_multi_pack_index(source);
> +		char *midx_bitmap_name;
> +
> +		if (!m)
> +			continue;
> +
> +		midx_bitmap_name = midx_bitmap_filename(m);

> diff --git a/packfile.c b/packfile.c
> index e5d9d7ac8bc..e1ced050451 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -963,14 +963,17 @@ static void prepare_packed_git(struct repository *r);
>  unsigned long repo_approximate_object_count(struct repository *r)
>  {
>  	if (!r->objects->approximate_object_count_valid) {
> -		unsigned long count;
> -		struct multi_pack_index *m;
> +		unsigned long count = 0;
>  		struct packed_git *p;
>
>  		prepare_packed_git(r);

> -		count = 0;
> -		for (m = get_multi_pack_index(r); m; m = m->next)
> -			count += m->num_objects;
> +
> +		for (struct odb_source *source = r->objects->sources; source; source = source->next) {
> +			struct multi_pack_index *m = get_multi_pack_index(source);
> +			if (m)
> +				count += m->num_objects;

> On Wed, Jul 09, 2025 at 09:54:53AM +0200, Patrick Steinhardt wrote:
> > diff --git a/builtin/repack.c b/builtin/repack.c
> > index 9bbf032b6dd..5956df5d927 100644
> > --- a/builtin/repack.c
> > +++ b/builtin/repack.c
> > @@ -218,9 +218,9 @@ static void mark_packs_for_deletion(struct existing_packs *existing,
> >  static void remove_redundant_pack(const char *dir_name, const char *base_name)
> >  {
> >  	struct strbuf buf = STRBUF_INIT;
> > -	struct multi_pack_index *m = get_local_multi_pack_index(the_repository);
> > +	struct multi_pack_index *m = get_multi_pack_index(the_repository->objects->sources);
> 
> Is the first source always guaranteed to be the local one? I assume that the
> answer here is "yes", and there are certainly other places in the code
> where we make a similar assumption. But just wanted to make sure as
> this popped into my head while reading.
> 
> >  	strbuf_addf(&buf, "%s.pack", base_name);
> > -	if (m && midx_contains_pack(m, buf.buf))
> > +	if (m && m->local && midx_contains_pack(m, buf.buf))
> 
> ...hmm, maybe not?

> > diff --git a/midx-write.c b/midx-write.c
> > index f2cfb85476e..c1ae62d3549 100644
> > --- a/midx-write.c
> > +++ b/midx-write.c
> > @@ -916,26 +916,8 @@ static int write_midx_bitmap(struct write_midx_context *ctx,
> >  static struct multi_pack_index *lookup_multi_pack_index(struct repository *r,
> >  							const char *object_dir)
> >  {
> > -	struct multi_pack_index *result = NULL;
> > -	struct multi_pack_index *cur;
> > -	char *obj_dir_real = real_pathdup(object_dir, 1);
> > -	struct strbuf cur_path_real = STRBUF_INIT;
> > -
> > -	/* Ensure the given object_dir is local, or a known alternate. */
> > -	odb_find_source(r->objects, obj_dir_real);
> > -
> > -	for (cur = get_multi_pack_index(r); cur; cur = cur->next) {
> > -		strbuf_realpath(&cur_path_real, cur->object_dir, 1);
> > -		if (!strcmp(obj_dir_real, cur_path_real.buf)) {
> > -			result = cur;
> > -			goto cleanup;
> > -		}
> > -	}
> > -
> > -cleanup:
> > -	free(obj_dir_real);
> > -	strbuf_release(&cur_path_real);
> > -	return result;
> > +	struct odb_source *source = odb_find_source(r->objects, object_dir);
> > +	return get_multi_pack_index(source);
> 
> When I first read this I wondered what would happen if we passed in an
> unknown object_dir such that odb_find_source() returned NULL. But that
> function will never return NULL, and instead will die() if given a bogus
> object_dir.
> 
> So this is fine, though I would have imagined that we'd return NULL
> within odb_find_source() and let the caller die() (or not).

> > diff --git a/object-name.c b/object-name.c
> > index ddafe7f9b13..1e7fdcb90a8 100644
> > --- a/object-name.c
> > +++ b/object-name.c
> > @@ -198,16 +198,19 @@ static void unique_in_pack(struct packed_git *p,
> >
> >  static void find_short_packed_object(struct disambiguate_state *ds)
> >  {
> > -	struct multi_pack_index *m;
> >  	struct packed_git *p;
> >
> >  	/* Skip, unless oids from the storage hash algorithm are wanted */
> >  	if (ds->bin_pfx.algo && (&hash_algos[ds->bin_pfx.algo] != ds->repo->hash_algo))
> >  		return;
> >
> > -	for (m = get_multi_pack_index(ds->repo); m && !ds->ambiguous;
> > -	     m = m->next)
> > -		unique_in_midx(m, ds);
> > +	odb_prepare_alternates(ds->repo->objects);
> > +	for (struct odb_source *source = ds->repo->objects->sources; source && !ds->ambiguous; source = source->next) {
> > +		struct multi_pack_index *m = get_multi_pack_index(source);
> > +		if (m)
> > +			unique_in_midx(m, ds);
> > +	}
> > +
> 
> Makes sense, though now having seen this pattern a few times, I am
> wondering if it would be worth it to add a utility function that takes a
> callback and iterates over the various MIDXs. But perhaps that is taking
> DRY-ing things up a little too far ;-).
> 
> For what it's worth, I do think that what you wrote here makes more
> logical sense: MIDXs are tied to individual alternate object DBs, which
> here means that there is one MIDX per "struct odb_source". It just is a
> little more verbose to type out.

> In the preceding commits we have migrated all users of the linked list
> of multi-pack indices to instead use those stored in the object database
> sources. Remove those now-unused pointers.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  midx.c     | 16 ++--------------
>  midx.h     |  2 --
>  odb.h      |  7 -------
>  packfile.c |  1 -
>  4 files changed, 2 insertions(+), 24 deletions(-)
> 
> diff --git a/midx.c b/midx.c
> index 6d3a166fa01..27623e8cbb7 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -726,7 +726,6 @@ int midx_preferred_pack(struct multi_pack_index *m, uint32_t *pack_int_id)
>  int prepare_multi_pack_index_one(struct odb_source *source, int local)
>  {
>  	struct repository *r = source->odb->repo;
> -	struct multi_pack_index *m;
>  
>  	if (source->multi_pack_index_loaded)
>  		return !!source->multi_pack_index;
> @@ -735,19 +734,9 @@ int prepare_multi_pack_index_one(struct odb_source *source, int local)
>  	if (!r->settings.core_multi_pack_index)
>  		return 0;
>  
> -	m = load_multi_pack_index(r, source->path, local);
> -	if (m) {
> -		struct multi_pack_index *mp = r->objects->multi_pack_index;
> -		if (mp) {
> -			m->next = mp->next;
> -			mp->next = m;
> -		} else {
> -			r->objects->multi_pack_index = m;
> -		}
> -		source->multi_pack_index = m;
> -	}
> -
> +	source->multi_pack_index = load_multi_pack_index(r, source->path, local);

>  	source->multi_pack_index_loaded = 1;
> +
>  	return !!source->multi_pack_index;
>  }
>  
> @@ -840,7 +829,6 @@ void clear_midx_file(struct repository *r)
>  			source->multi_pack_index = NULL;
>  			source->multi_pack_index_loaded = 0;
>  		}
> -		r->objects->multi_pack_index = NULL;
>  	}
>  
>  	if (remove_path(midx.buf))
> diff --git a/midx.h b/midx.h
> index b1626a9a7c4..c4192c92d44 100644
> --- a/midx.h
> +++ b/midx.h
> @@ -35,8 +35,6 @@ struct repository;
>  	"GIT_TEST_MULTI_PACK_INDEX_WRITE_INCREMENTAL"
>  
>  struct multi_pack_index {
> -	struct multi_pack_index *next;

> -
>  	const unsigned char *data;
>  	size_t data_len;
>  
> diff --git a/odb.h b/odb.h
> index b39534dd55b..8ff523a33de 100644
> --- a/odb.h
> +++ b/odb.h
> @@ -124,13 +124,6 @@ struct object_database {
>  	struct commit_graph *commit_graph;
>  	unsigned commit_graph_attempted : 1; /* if loading has been attempted */
>  
> -	/*
> -	 * private data
> -	 *
> -	 * should only be accessed directly by packfile.c and midx.c
> -	 */
> -	struct multi_pack_index *multi_pack_index;

> -
>  	/*
>  	 * private data
>  	 *
> diff --git a/packfile.c b/packfile.c
> index 3eeec20906b..453a38395bb 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -375,7 +375,6 @@ void close_object_store(struct object_database *o)
>  		source->multi_pack_index = NULL;
>  		source->multi_pack_index_loaded = 0;
>  	}
> -	o->multi_pack_index = NULL;
>  
>  	close_commit_graph(o);
>  }
> 
> -- 
> 2.50.1.327.g047016eb4a.dirty
> 
> 

> This patch series thus refactors the codebase to stop tracking MIDX's
> globally. Instead, they are being pushed down one level so that every
> `struct odb_source` has an optional MIDX itself. This simplifies some of
> our code and will make it easier in a future iteration to move the data
> into a packfile-specific object source backend.
>
> This series is built on top of a30f80fde92 (The eighth batch,
> 2025-07-08) with "ps/object-store" at 841a03b4046 (odb: rename
> `read_object_with_reference()`, 2025-07-01) merged into it.

> Patrick Steinhardt <ps@pks.im> writes:
>
> > This patch series thus refactors the codebase to stop tracking MIDX's
> > globally. Instead, they are being pushed down one level so that every
> > `struct odb_source` has an optional MIDX itself. This simplifies some of
> > our code and will make it easier in a future iteration to move the data
> > into a packfile-specific object source backend.
> >
> > This series is built on top of a30f80fde92 (The eighth batch,
> > 2025-07-08) with "ps/object-store" at 841a03b4046 (odb: rename
> > `read_object_with_reference()`, 2025-07-01) merged into it.
>
> You do not have to deal with it just yet, but FYI, another topic in
> flight has a commit that adds a few more callers to a function this
> topic renames away.  Namely, 5ee86c27 (repack: exclude cruft pack(s)
> from the MIDX where possible, 2025-06-23).

> If this topic needs to be rerolled after the other topic graduates
> to 'master', we may need to see this topic rebased on a newer
> 'master' with something like the attached patch squashed in, but
> because the other topic is at least a few more days away from
> 'next', and it might still need another final finishing touch
> iteration, let's keep these two topics independent from each other a
> bit longer, and let me deal with this trivial semantic conflict
> resolution, at least for now.
>
> Thanks.
>
> diff --git a/builtin/repack.c b/builtin/repack.c
> index a74b2ca7f3..21723866b9 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c

> On Wed, Jul 09, 2025 at 03:04:44PM -0700, Junio C Hamano wrote:
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> > > This patch series thus refactors the codebase to stop tracking MIDX's
> > > globally. Instead, they are being pushed down one level so that every
> > > `struct odb_source` has an optional MIDX itself. This simplifies some of
> > > our code and will make it easier in a future iteration to move the data
> > > into a packfile-specific object source backend.
> > >
> > > This series is built on top of a30f80fde92 (The eighth batch,
> > > 2025-07-08) with "ps/object-store" at 841a03b4046 (odb: rename
> > > `read_object_with_reference()`, 2025-07-01) merged into it.
> >
> > You do not have to deal with it just yet, but FYI, another topic in
> > flight has a commit that adds a few more callers to a function this
> > topic renames away.  Namely, 5ee86c27 (repack: exclude cruft pack(s)
> > from the MIDX where possible, 2025-06-23).
> 
> Yup, there are a handful of new get_local_multi_pack_index() calls in
> that topic.
> 
> > If this topic needs to be rerolled after the other topic graduates
> > to 'master', we may need to see this topic rebased on a newer
> > 'master' with something like the attached patch squashed in, but
> > because the other topic is at least a few more days away from
> > 'next', and it might still need another final finishing touch
> > iteration, let's keep these two topics independent from each other a
> > bit longer, and let me deal with this trivial semantic conflict
> > resolution, at least for now.
> >
> > Thanks.
> >
> > diff --git a/builtin/repack.c b/builtin/repack.c
> > index a74b2ca7f3..21723866b9 100644
> > --- a/builtin/repack.c
> > +++ b/builtin/repack.c
> 
> Assuming that in these three cases that the first entry in
> the_repository->objects->sources refers to the local object database,
> then I agree with the proposed changes.
> 
> Thanks for flagging it :-).

> Hi,
> 
> multi-pack-indices are tracked via `struct multi_pack_index`. This data
> structure is stored 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 object 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 an object source has an MIDX, quite awkward.
> 
>   - Multi-pack indices are an implementation detail of how efficient
>     access for packfiles work. As such, they are neither relevant in the
>     context of loose objects, nor in a potential future where we have
>     pluggable backends.
> 
> This patch series thus refactors the codebase to stop tracking MIDX's
> globally. Instead, they are being pushed down one level so that every
> `struct odb_source` has an optional MIDX itself. This simplifies some of
> our code and will make it easier in a future iteration to move the data
> into a packfile-specific object source backend.
> 
> Changes in v2:
>   - Changed the base of this series. It is now built on top of
>     a30f80fde92 (The eighth batch, 2025-07-08) with "ps/object-store" at
>     841a03b4046 (odb: rename `read_object_with_reference()`, 2025-07-01)
>     and "tb/midx-avoid-cruft-packs" at 5ee86c273bf (repack: exclude
>     cruft pack(s) from the MIDX where possible, 2025-06-23) merged into
>     it.
>   - Re-explain the split between object databases and object sources
>     to help readers out a bit, given that this is a rather recent
>     change.
>   - Rename `struct odb_source::multi_pack_index` to `struct
>     odb_source::midx`.
>   - Fix some overly long lines when looping through the individual
>     sources.
>   - Drop the patch that guards re-loading MIDXs, as we already have the
>     guard via `packed_git_initialized`.
>   - Remove some while-at-it changes to make the diffs easier to read.
>   - Link to v1: https://lore.kernel.org/r/20250709-b4-pks-midx-via-odb-alternate-v1-0-f31150d21331@pks.im

> Changes in v2:
>   - Changed the base of this series. It is now built on top of
>     a30f80fde92 (The eighth batch, 2025-07-08) with "ps/object-store" at
>     841a03b4046 (odb: rename `read_object_with_reference()`, 2025-07-01)
>     and "tb/midx-avoid-cruft-packs" at 5ee86c273bf (repack: exclude
>     cruft pack(s) from the MIDX where possible, 2025-06-23) merged into
>     it.
>   - Re-explain the split between object databases and object sources
>     to help readers out a bit, given that this is a rather recent
>     change.
>   - Rename `struct odb_source::multi_pack_index` to `struct
>     odb_source::midx`.
>   - Fix some overly long lines when looping through the individual
>     sources.
>   - Drop the patch that guards re-loading MIDXs, as we already have the
>     guard via `packed_git_initialized`.
>   - Remove some while-at-it changes to make the diffs easier to read.
>   - Link to v1: https://lore.kernel.org/r/20250709-b4-pks-midx-via-odb-alternate-v1-0-f31150d21331@pks.im

> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Changes in v2:
> >   - Changed the base of this series. It is now built on top of
> >     a30f80fde92 (The eighth batch, 2025-07-08) with "ps/object-store" at
> >     841a03b4046 (odb: rename `read_object_with_reference()`, 2025-07-01)
> >     and "tb/midx-avoid-cruft-packs" at 5ee86c273bf (repack: exclude
> >     cruft pack(s) from the MIDX where possible, 2025-06-23) merged into
> >     it.
> >   - Re-explain the split between object databases and object sources
> >     to help readers out a bit, given that this is a rather recent
> >     change.
> >   - Rename `struct odb_source::multi_pack_index` to `struct
> >     odb_source::midx`.
> >   - Fix some overly long lines when looping through the individual
> >     sources.
> >   - Drop the patch that guards re-loading MIDXs, as we already have the
> >     guard via `packed_git_initialized`.
> >   - Remove some while-at-it changes to make the diffs easier to read.
> >   - Link to v1: https://lore.kernel.org/r/20250709-b4-pks-midx-via-odb-alternate-v1-0-f31150d21331@pks.im
> 
> Shall we mark the topic for 'next' now?
> 
> We haven't seen any comments on this iteration.

> On Wed, Jul 23, 2025 at 02:22:08PM -0700, Junio C Hamano wrote:
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> > > Changes in v2:
> > >   - Changed the base of this series. It is now built on top of
> > >     a30f80fde92 (The eighth batch, 2025-07-08) with "ps/object-store" at
> > >     841a03b4046 (odb: rename `read_object_with_reference()`, 2025-07-01)
> > >     and "tb/midx-avoid-cruft-packs" at 5ee86c273bf (repack: exclude
> > >     cruft pack(s) from the MIDX where possible, 2025-06-23) merged into
> > >     it.
> > >   - Re-explain the split between object databases and object sources
> > >     to help readers out a bit, given that this is a rather recent
> > >     change.
> > >   - Rename `struct odb_source::multi_pack_index` to `struct
> > >     odb_source::midx`.
> > >   - Fix some overly long lines when looping through the individual
> > >     sources.
> > >   - Drop the patch that guards re-loading MIDXs, as we already have the
> > >     guard via `packed_git_initialized`.
> > >   - Remove some while-at-it changes to make the diffs easier to read.
> > >   - Link to v1: https://lore.kernel.org/r/20250709-b4-pks-midx-via-odb-alternate-v1-0-f31150d21331@pks.im
> >
> > Shall we mark the topic for 'next' now?
> >
> > We haven't seen any comments on this iteration.
>
> Almost all of the comments on the previous version were about style, so
> nothing significant has changed in this version except for a couple of
> renames and style fixes. Which means that I'm fine with the comments I
> got for v1, but I wouldn't mind waiting two or three more days until
> this gets merged down.