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@xxxxxx>
> ---
> 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
