git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 5/8] packfile: refactor `get_multi_pack_index()` to work on sources

From
Justin Tobler <jltobler@gmail.com>
Date
Jul 10, 2025, 22:35 UTC
Message-ID
<5ordhipbyhkkwx64jfr26ojirs77tumowxy6g5yhibo6ylb4uv@v6wwpjnqomcv>
In-Reply-To
<20250709-b4-pks-midx-via-odb-alternate-v1-5-f31150d21331@pks.im>
On 25/07/09 09:54AM, Patrick Steinhardt wrote:
Show 41 quoted lines
> 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);

Previously this explicit call to `odb_prepare_alternates()` was not necessary because it was done when `prepare_packed_git()` was invoked. Now that we iterate though the sources upfront to call `get_multi_pack_index()` on, we need to prepare those sources first. Makes sense.

Show 56 quoted lines
> +
> +	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);
Nice and simple :)
Show 142 quoted lines
>  }
>  
>  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;
Now get_multi_pack_index() provides the MIDX for the specified source.
Show 22 quoted lines
>  }
>  
>  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
> 
>
Previous: Patrick SteinhardtNext: Taylor Blau
Message 22 of 43 in “odb: track multi-pack-indices via their object sources”
  1. 0/8 odb: track multi-pack-indices via their object sourcesPatrick Steinhardt, Jul 9, 2025
  2. 1/8 midx: start tracking per object database sourcePatrick Steinhardt, Jul 9, 2025
  3. Justin ToblerJul 10, 2025
  4. Taylor BlauJul 10, 2025
  5. Taylor BlauJul 10, 2025
  6. Patrick SteinhardtJul 15, 2025
  7. Patrick SteinhardtJul 15, 2025
  8. 2/8 packfile: refactor `prepare_packed_git_one()` to work on sourcesPatrick Steinhardt, Jul 9, 2025
  9. Justin ToblerJul 10, 2025
  10. Taylor BlauJul 10, 2025
  11. Patrick SteinhardtJul 15, 2025
  12. 3/8 midx: stop using linked list when closing MIDXPatrick Steinhardt, Jul 9, 2025
  13. Justin ToblerJul 10, 2025
  14. Patrick SteinhardtJul 15, 2025
  15. Taylor BlauJul 10, 2025
  16. Patrick SteinhardtJul 15, 2025
  17. 4/8 midx: track whether we have loaded the MIDXPatrick Steinhardt, Jul 9, 2025
  18. Justin ToblerJul 10, 2025
  19. Taylor BlauJul 10, 2025
  20. Patrick SteinhardtJul 15, 2025
  21. 5/8 packfile: refactor `get_multi_pack_index()` to work on sourcesPatrick Steinhardt, Jul 9, 2025
  22. Justin ToblerJul 10, 2025
  23. Taylor BlauJul 10, 2025
  24. Patrick SteinhardtJul 15, 2025
  25. 6/8 packfile: stop using linked MIDX list in `find_pack_entry()`Patrick Steinhardt, Jul 9, 2025
  26. 7/8 packfile: stop using linked MIDX list in `get_all_packs()`Patrick Steinhardt, Jul 9, 2025
  27. 8/8 midx: remove now-unused linked list of multi-pack indicesPatrick Steinhardt, Jul 9, 2025
  28. Justin ToblerJul 10, 2025
  29. Junio C HamanoJul 9, 2025
  30. Taylor BlauJul 10, 2025
  31. Patrick SteinhardtJul 15, 2025
  32. 0/7 odb: track multi-pack-indices via their object sourcesPatrick Steinhardt, Jul 15, 2025
  33. 1/7 midx: start tracking per object database sourcePatrick Steinhardt, Jul 15, 2025
  34. 2/7 packfile: refactor `prepare_packed_git_one()` to work on sourcesPatrick Steinhardt, Jul 15, 2025
  35. 3/7 midx: stop using linked list when closing MIDXPatrick Steinhardt, Jul 15, 2025
  36. 4/7 packfile: refactor `get_multi_pack_index()` to work on sourcesPatrick Steinhardt, Jul 15, 2025
  37. 5/7 packfile: stop using linked MIDX list in `find_pack_entry()`Patrick Steinhardt, Jul 15, 2025
  38. 6/7 packfile: stop using linked MIDX list in `get_all_packs()`Patrick Steinhardt, Jul 15, 2025
  39. 7/7 midx: remove now-unused linked list of multi-pack indicesPatrick Steinhardt, Jul 15, 2025
  40. Justin ToblerJul 15, 2025
  41. Junio C HamanoJul 23, 2025
  42. Patrick SteinhardtJul 24, 2025
  43. Taylor BlauAug 12, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.