Re: [RFC PATCH] multi-pack-index: allow operating without pack files

[Derrick Stolee <stolee@gmail.com>]

On 8/20/2021 3:55 PM, Johannes Berg wrote:
> Technically, multi-pack-index doesn't need pack files to exist,
> but add_packed_git() today checks whether it exists or not.

Having a multi-pack-index is supposed to indicate that we have
these objects in the objects/pack directory within the specified
pack-files.

I understand your goal to relax a condition of the multi-pack-index
file, but it's triggered by a flag during write and that choice
isn't persisted into the file. There is no way for a later Git
process to understand that the multi-pack-index doesn't actually
guarantee object existence.

And in a completely other side: one would think that including
a pack-file in the multi-pack-index would allow deleting the .idx
file, but there are a few reasons why we do not (including
interactions with third-party tools).

So, I'm not necessarily on board with this change unless
something is added to the multi-pack-index file (such as a new
version 2 and an optional chunk understood by that version) that
tells future Git processes that the .pack files might not exist.
I'm still not sure what Git should do about that other than stop
reading the multi-pack-index and ignore its contents.

> -struct packed_git *add_packed_git(const char *path, size_t path_len, int local)
> +struct packed_git *_add_packed_git(const char *path, size_t path_len, int local,
> +				   int require_pack)

The only obvious thing that I noticed in the code is that we
typically use <function name>_1() as a way to create a static
version that is called by the global version.

>  struct packed_git *add_packed_git(const char *path, size_t path_len, int local);
> +struct packed_git *_add_packed_git(const char *path, size_t path_len, int local,
> +				   int require_pack);

...oh, but that's not what you're doing. What you could do
instead is convert the 'local' parameter into a 'flags' parameter
(I think we have started to prefer 'enum's recently) and create
MIDX_FLAG_LOCAL and MIDX_FLAG_PACKS_OPTIONAL flag values. That
avoids multiple methods and minimizes the change to existing
callers.

Thanks,
-Stolee