Re: [PATCH v3 1/6] odb: introduce interface to generate packfiles

Karthik Nayak <[email protected]> Thu, 20 Aug 2026 03:16:37 -0700
Newsgroups	org.kernel.vger.git
Message-ID	<CAOLa=ZSYyfOCs8Dr0Xdhv-=Q=j0z7vtfYqopDb07XuAm2PU84g@mail.gmail.com>
Patrick Steinhardt <[email protected]> writes:

> Packfiles have two primary use cases:
>
>   - They are used to store objects at rest in a Git repository.
>
>   - They are used on the transport layer to transfer objects between two
>     repositories.
>
> The first class is closely tied to a given object database backend, and
> as such this use is highly specific to how such a backend decides to
> store its data. This shows in git-pack-objects(1), which is used by
> git-repack(1) et al to optimize the object database, which supports lots
> of options that are closely coupled with how data is stored.
>
> But the second class is quite a lot more generic: we don't care about
> specifics of how the object database stores its objects, but to generate
> the packfiles we only care about the object graph itself. Still, this
> use case is also coupled with git-pack-objects(1).
>
> Unfortunately, because git-pack-objects(1) covers both classes, the
> result is that it is very hard to port the whole command to properly
> support pluggable object databases. There are simply way too many
> options that an alternative implementation will have a very hard time to
> support in the first place.
>
> And despite being hard to implement, it's also quite unnecessary to
> implement those backend-specific options. Optimizing the object database
> has already been made pluggable, and an alternative implementation is
> unlikely to care about cruft packs, unpacked objects, keep packs and the
> like. But we still need to make at least _parts_ of the packfile
> generation pluggable so that backends can generate packfiles for the
> transport layer itself.
>
> Introduce a new interface that lets backends generate a new packfile and
> implement that interface for the "files" backend. The options supported
> by the callback are exactly the set of options that are required for the
> transport layer, but nothing more.
>

Okay so we also provide the implementation for the files odb here, which
calls 'git-pack-objects(1)' to generate the packfile.

The changes look in order.

[snip]

> diff --git a/odb/source.h b/odb/source.h
> index d69f8e2d1c..e2129766fc 100644
> --- a/odb/source.h
> +++ b/odb/source.h
> @@ -278,6 +278,23 @@ struct odb_source {
>  	 */
>  	bool (*optimize_required)(struct odb_source *source,
>  				  const struct odb_optimize_options *opts);
> +
> +	/*
> +	 * This callback is expected to start generating a packfile with the
> +	 * given options. The pack shall be generated asynchronously so that
> +	 * the caller can consume the pack data and progress output while the
> +	 * pack is being generated.
> +	 *
> +	 * This callback is optional. Sources that cannot generate packfiles
> +	 * shall leave it unset.
> +	 *
> +	 * The callback is expected to return 0 on success and populate the
> +	 * `out` pointer with the pack generator, a negative error code
> +	 * otherwise.
> +	 */
> +	int (*generate_pack)(struct odb_source *source,
> +			     struct odb_pack_generator **out,
> +			     const struct odb_generate_pack_options *opts);
>  };
>

Nit: I see that `source` is unused anyways, do we need to pass it in? Or
is just for consistency?

>  /*
> @@ -520,4 +537,20 @@ static inline bool odb_source_optimize_required(struct odb_source *source,
>  	return source->optimize_required(source, opts);
>  }
>
> +/*
> + * Start generating a packfile from the given source with the given options.
> + * The pack is generated asynchronously; the caller is expected to consume the
> + * file descriptors exposed via the pack generator and to then wait for
> + * completion via `odb_pack_generator_finish()`.
> + *
> + * Returns 0 on success and populates the `out` pointer with the pack
> + * generator, a negative error code otherwise.
> + */
> +static inline int odb_source_generate_pack(struct odb_source *source,
> +					   struct odb_pack_generator **out,
> +					   const struct odb_generate_pack_options *opts)
> +{
> +	return source->generate_pack(source, out, opts);
> +}
> +
>  #endif
>
> --
> 2.55.0.822.g20453c30eb.dirty
signature.asc (application/pgp-signature, 690 B)