Re: [PATCH v2 6/9] midx: load multi-pack indices via their source

[Taylor Blau <me@ttaylorr.com>]

On Thu, Aug 07, 2025 at 10:09:56AM +0200, Patrick Steinhardt wrote:
> 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.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/multi-pack-index.c  | 18 ++++++++++++--
>  midx.c                      | 57 ++++++++++++++++++++-------------------------
>  midx.h                      |  6 ++---
>  t/helper/test-read-midx.c   | 25 ++++++++++++--------
>  t/t5319-multi-pack-index.sh |  8 +++----
>  5 files changed, 62 insertions(+), 52 deletions(-)
>
> diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
> index aa25b06f9d..e4a9305af3 100644
> --- a/builtin/multi-pack-index.c
> +++ b/builtin/multi-pack-index.c
> @@ -64,12 +64,20 @@ static int parse_object_dir(const struct option *opt, const char *arg,
>  	char **value = opt->value;
>  	free(*value);
>  	if (unset)
> -		*value = xstrdup(repo_get_object_directory(the_repository));
> +		*value = xstrdup(the_repository->objects->sources->path);
>  	else
>  		*value = real_pathdup(arg, 1);
>  	return 0;
>  }
>
> +static struct odb_source *handle_object_dir_option(struct repository *repo)
> +{
> +	struct odb_source *source = odb_find_source(repo->objects, opts.object_dir);
> +	if (!source)
> +		source = odb_add_to_alternates_memory(repo->objects, opts.object_dir);
> +	return source;
> +}
> +

Hmm... I admit that I am pretty unfamiliar with the --object-dir option
here and only have a vague recollection of why it was added in the first
place.

I am not sure one way or another if adding the user-provided directory
as an in-memory alternate is the right thing to do here.

Stolee (CC'd) would you mind sharing your thoughts on this?

Thanks,
Taylor