[PATCH v2 0/7] builtin/receive-pack: support pluggable packfile writes

[Justin Tobler <jltobler@gmail.com>]

Greetings,

With bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB
transactions, 2026-07-10), git-receive-pack(1) started using the ODB
transaction interfaces to stage incoming objects. While this brought the
command closer to being ODB backend agnostic, the underlying
git-index-pack(1) and git-unpack-objects(1) processes used to actually
write the objects to the transaction are still fundamentally tied to the
"files" backend.

This series aims to address this by introducing a generic
`odb_transaction_write_pack()` transaction interface to handle writing
the incoming packfile to the transaction. The existing logic in
git-receive-pack(1) that spawns the child processes to write the
packfile becomes the "files" backend implementation of this interface.

Changes since V1:
- Changed the "release" interface name to "finalize" and updated it to
  return error codes.
- Marked some function parameters as const.
- Unpack limit configuration is now resolved in the ODB transaction
  backend instead of wiring it through the interface.
- When writing a packfile to the transaction, now only the transaction
  source is prepared.
- Updated some commit messages.
- Updated some code formatting.

Thanks for the review,
-Justin

Justin Tobler (7):
  odb/transaction: add transaction finalize interface
  builtin/receive-pack: pass shallow file explicitly
  builtin/receive-pack: read unpack limit config lazily
  builtin/receive-pack: lift global state out of unpack()
  builtin/receive-pack: report unpack errors via strbuf
  builtin/receive-pack: explicitly pass packfile fd
  odb/transaction: add transaction interface to write packfiles

 builtin/add.c            |   3 +-
 builtin/receive-pack.c   | 211 +++++++++------------------------------
 builtin/unpack-objects.c |   1 +
 builtin/update-index.c   |   2 +
 cache-tree.c             |   4 +-
 object-file.c            | 184 +++++++++++++++++++++++++++++++++-
 odb/transaction.c        |  21 ++++
 odb/transaction.h        |  78 +++++++++++++++
 read-cache.c             |   4 +-
 9 files changed, 339 insertions(+), 169 deletions(-)

Range-diff against v1:
1:  d0a4b632bd ! 1:  10efcc22e4 odb/transaction: add transaction release interface
    @@ Metadata
     Author: Justin Tobler <jltobler@gmail.com>
     
      ## Commit message ##
    -    odb/transaction: add transaction release interface
    +    odb/transaction: add transaction finalize interface
     
         When committing an ODB transaction via `odb_transaction_commit()`, the
         staged objects are made visible and the underlying transaction is freed
    @@ Commit message
         the case with the existing "files" backend in git-receive-pack(1).
     
         Stop freeing the transaction in `odb_transaction_commit()` and introduce
    -    `odb_transaction_release()` to explicitly clean up the transaction
    -    accordingly. Note that the release interface also provides an optional
    +    `odb_transaction_finalize()` to explicitly clean up the transaction
    +    accordingly. Note that the finalize interface also provides an optional
         callback for any backend-specific deferred cleanup. In a subsequent
         commit, the "files" transaction backend will use this to remove ".keep"
         files generated for packfiles received via git-receive-pack(1) after
    -    references have been updated.
    +    references have been updated. In preparation for this, the
    +    `odb_transaction_finalize()` call site in git-receive-pack(1) is made
    +    after the reference updates are finished.
     
         Signed-off-by: Justin Tobler <jltobler@gmail.com>
     
    @@ builtin/add.c: int cmd_add(int argc,
      	free(ps_matched);
      	dir_clear(&dir);
      	clear_pathspec(&pathspec);
    -+	odb_transaction_release(transaction);
    ++	odb_transaction_finalize(transaction);
      	return exit_status;
      }
     
    @@ builtin/receive-pack.c: int cmd_receive_pack(int argc,
      		use_keepalive = KEEPALIVE_ALWAYS;
      		execute_commands(commands, unpack_status, &si, transaction,
      				 &push_options);
    -+		odb_transaction_release(transaction);
    ++		odb_transaction_finalize(transaction);
      		delete_tempfile(&pack_lockfile);
      		sigchain_push(SIGPIPE, SIG_IGN);
      		if (report_status_v2)
    @@ builtin/unpack-objects.c: static void unpack_all(void)
      		display_progress(progress, i + 1);
      	}
      	odb_transaction_commit(transaction);
    -+	odb_transaction_release(transaction);
    ++	odb_transaction_finalize(transaction);
      	stop_progress(&progress);
      
      	if (delta_list)
    @@ builtin/update-index.c: int cmd_update_index(int argc,
      			 */
      			if (transaction && verbose) {
      				odb_transaction_commit(transaction);
    -+				odb_transaction_release(transaction);
    ++				odb_transaction_finalize(transaction);
      				transaction = NULL;
      			}
      
    @@ builtin/update-index.c: int cmd_update_index(int argc,
      	 * By now we have added all of the new objects
      	 */
      	odb_transaction_commit(transaction);
    -+	odb_transaction_release(transaction);
    ++	odb_transaction_finalize(transaction);
      
      	if (split_index > 0) {
      		if (repo_config_get_split_index(the_repository) == 0)
    @@ cache-tree.c: int cache_tree_update(struct index_state *istate, int flags)
     -	if (!inflight)
     +	if (!inflight) {
      		odb_transaction_commit(transaction);
    -+		odb_transaction_release(transaction);
    ++		odb_transaction_finalize(transaction);
     +	}
      	trace2_region_leave("cache_tree", "update", istate->repo);
      	trace_performance_leave("cache_tree_update");
    @@ object-file.c: int index_fd(struct index_state *istate, struct object_id *oid,
     -			if (!inflight)
     +			if (!inflight) {
      				odb_transaction_commit(transaction);
    -+				odb_transaction_release(transaction);
    ++				odb_transaction_finalize(transaction);
     +			}
      		} else {
      			ret = hash_blob_stream(&stream,
    @@ odb/transaction.c: int odb_transaction_commit(struct odb_transaction *transactio
      
      	ret = transaction->commit(transaction);
      	transaction->source->odb->transaction = NULL;
    --	free(transaction);
    - 
    - 	return ret;
    - }
    - 
    -+void odb_transaction_release(struct odb_transaction *transaction)
    ++
    ++	return ret;
    ++}
    ++
    ++int odb_transaction_finalize(struct odb_transaction *transaction)
     +{
    -+	if (!transaction)
    -+		return;
    ++	int ret = 0;
     +
    -+	if (transaction->release)
    -+		transaction->release(transaction);
    ++	if (!transaction)
    ++		return 0;
     +
    -+	free(transaction);
    -+}
    ++	if (transaction->finalize)
    ++		ret = transaction->finalize(transaction);
     +
    - int odb_transaction_write_object_stream(struct odb_transaction *transaction,
    - 					struct odb_write_stream *stream,
    - 					size_t len, struct object_id *oid)
    + 	free(transaction);
    + 
    + 	return ret;
     
      ## odb/transaction.h ##
     @@ odb/transaction.h: struct odb_transaction {
    @@ odb/transaction.h: struct odb_transaction {
     +	/*
     +	 * Optional ODB source specific callback invoked when the transaction
     +	 * needs to perform any deferred cleanup after objects have been
    -+	 * committed.
    ++	 * committed. Returns 0 on success, a negative error code otherwise.
     +	 */
    -+	void (*release)(struct odb_transaction *transaction);
    ++	int (*finalize)(struct odb_transaction *transaction);
     +
      	/*
      	 * This callback is expected to write the given object stream into
    @@ odb/transaction.h: static inline void odb_transaction_begin_or_die(struct object
      int odb_transaction_commit(struct odb_transaction *transaction);
      
     +/*
    -+ * Releases an ODB transaction, performing any deferred cleanup and freeing it.
    ++ * Finalizes an ODB transaction, performing any deferred cleanup and freeing it.
     + * Must be called for every successfully started transaction. Note that, if the
    -+ * specified transaction is NULL, the function is a no-op.
    ++ * specified transaction is NULL, the function is a no-op. Returns 0 on success,
    ++ * a negative error code otherwise.
     + */
    -+void odb_transaction_release(struct odb_transaction *transaction);
    ++int odb_transaction_finalize(struct odb_transaction *transaction);
     +
      /*
       * Writes the object in the provided stream into the transaction. The resulting
    @@ read-cache.c: int add_files_to_cache(struct repository *repo, const char *prefix
     -	if (!inflight)
     +	if (!inflight) {
      		odb_transaction_commit(transaction);
    -+		odb_transaction_release(transaction);
    ++		odb_transaction_finalize(transaction);
     +	}
      
      	release_revisions(&rev);
2:  0aff7f769e = 2:  e1903ac32f builtin/receive-pack: pass shallow file explicitly
-:  ---------- > 3:  e4950c0abe builtin/receive-pack: read unpack limit config lazily
3:  61bac2a56f ! 4:  c9b4ff73ba builtin/receive-pack: lift global state out of unpack()
    @@ Commit message
         invoke the underlying git-index-pack(1) or git-unpack-objects(1) child
         processes. In a subsequent commit, the `unpack()` logic is moved behind
         a generic ODB transaction interface to handle writing packfiles and thus
    -    can no rely on these globals.
    +    can no longer rely on these globals.
     
         Lift the global state out of `unpack()` by instead storing this state in
         a `struct unpack_opts` that gets passed to the function explicitly.
    @@ Commit message
         Signed-off-by: Justin Tobler <jltobler@gmail.com>
     
      ## builtin/receive-pack.c ##
    -@@ builtin/receive-pack.c: static void push_header_arg(struct strvec *args, struct pack_header *hdr)
    - 		     ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
    +@@ builtin/receive-pack.c: static int get_unpack_limit(struct repository *repo)
    + 	return limit;
      }
      
     +struct unpack_opts {
    @@ builtin/receive-pack.c: static void push_header_arg(struct strvec *args, struct
     +	const char *shallow_file;
     +	off_t max_input_size;
     +	int fsck_objects;
    -+	int unpack_limit;
     +	int reject_thin;
     +	int err_fd;
     +	int quiet;
    @@ builtin/receive-pack.c: static const char *unpack(struct odb_transaction *transa
      	}
      
      	odb_transaction_env(transaction, &child.env);
    - 
    --	if (ntohl(hdr.hdr_entries) < unpack_limit) {
    -+	if (ntohl(hdr.hdr_entries) < opts->unpack_limit) {
    +@@ builtin/receive-pack.c: static const char *unpack(struct odb_transaction *transaction,
    + 	if (ntohl(hdr.hdr_entries) < get_unpack_limit(the_repository)) {
      		strvec_push(&child.args, "unpack-objects");
      		push_header_arg(&child.args, &hdr);
     -		if (quiet)
    @@ builtin/receive-pack.c: static const char *unpack(struct odb_transaction *transa
     +		.fsck_msg_types = fsck_msg_types.buf,
     +		.max_input_size = max_input_size,
     +		.shallow_file = shallow_file,
    -+		.unpack_limit = unpack_limit,
     +		.reject_thin = reject_thin,
     +		.quiet = quiet,
     +	};
4:  12b83ee3bc ! 5:  7be990c2c2 builtin/receive-pack: report unpack errors via strbuf
    @@ builtin/receive-pack.c: static void update_shallow_info(struct command *commands
      }
      
     -static void report(struct command *commands, const char *unpack_status)
    -+static void report(struct command *commands, struct strbuf *unpack_status)
    ++static void report(struct command *commands, const struct strbuf *unpack_status)
      {
      	struct command *cmd;
      	struct strbuf buf = STRBUF_INIT;
    @@ builtin/receive-pack.c: static void report(struct command *commands, const char
      }
      
     -static void report_v2(struct command *commands, const char *unpack_status)
    -+static void report_v2(struct command *commands, struct strbuf *unpack_status)
    ++static void report_v2(struct command *commands, const struct strbuf *unpack_status)
      {
      	struct command *cmd;
      	struct strbuf buf = STRBUF_INIT;
    @@ builtin/receive-pack.c: int cmd_receive_pack(int argc,
     -		execute_commands(commands, unpack_status, &si, transaction,
     +		execute_commands(commands, !!unpack_status.len, &si, transaction,
      				 &push_options);
    - 		odb_transaction_release(transaction);
    + 		odb_transaction_finalize(transaction);
      		delete_tempfile(&pack_lockfile);
      		sigchain_push(SIGPIPE, SIG_IGN);
      		if (report_status_v2)
5:  8678f4cd45 = 6:  742c724943 builtin/receive-pack: explicitly pass packfile fd
6:  c390f59367 ! 7:  7743cf242a odb/transaction: add transaction interface to write packfiles
    @@ Commit message
         Signed-off-by: Justin Tobler <jltobler@gmail.com>
     
      ## builtin/receive-pack.c ##
    +@@
    + #include "gpg-interface.h"
    + #include "hex.h"
    + #include "hook.h"
    +-#include "lockfile.h"
    + #include "object.h"
    + #include "object-file.h"
    + #include "object-name.h"
    +@@
    + #include "oid-array.h"
    + #include "oidset.h"
    + #include "pack.h"
    +-#include "packfile.h"
    + #include "parse-options.h"
    + #include "pkt-line.h"
    + #include "protocol.h"
     @@ builtin/receive-pack.c: static void read_push_options(struct packet_reader *reader,
      	}
      }
    @@ builtin/receive-pack.c: static void read_push_options(struct packet_reader *read
     -		     ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
     -}
     -
    +-static int get_unpack_limit(struct repository *repo)
    +-{
    +-	static int limit = -1;
    +-
    +-	if (limit < 0) {
    +-		int receive_limit = -1;
    +-		int transfer_limit = -1;
    +-
    +-		repo_config_get_int(repo, "receive.unpacklimit",
    +-				    &receive_limit);
    +-		repo_config_get_int(repo, "transfer.unpacklimit",
    +-				    &transfer_limit);
    +-
    +-		if (receive_limit >= 0)
    +-			limit = receive_limit;
    +-		else if (transfer_limit >= 0)
    +-			limit = transfer_limit;
    +-		else
    +-			limit = 100;
    +-	}
    +-
    +-	return limit;
    +-}
    +-
     -struct unpack_opts {
     -	const char *fsck_msg_types;
     -	const char *shallow_file;
     -	off_t max_input_size;
     -	int fsck_objects;
    --	int unpack_limit;
     -	int reject_thin;
     -	int err_fd;
     -	int quiet;
    @@ builtin/receive-pack.c: static void read_push_options(struct packet_reader *read
     -
     -	odb_transaction_env(transaction, &child.env);
     -
    --	if (ntohl(hdr.hdr_entries) < opts->unpack_limit) {
    +-	if (ntohl(hdr.hdr_entries) < get_unpack_limit(the_repository)) {
     -		strvec_push(&child.args, "unpack-objects");
     -		push_header_arg(&child.args, &hdr);
     -		if (opts->quiet)
    @@ builtin/receive-pack.c: static int unpack_with_sideband(struct odb_transaction *
     @@ builtin/receive-pack.c: int cmd_receive_pack(int argc,
      		execute_commands(commands, !!unpack_status.len, &si, transaction,
      				 &push_options);
    - 		odb_transaction_release(transaction);
    + 		odb_transaction_finalize(transaction);
     -		delete_tempfile(&pack_lockfile);
      		sigchain_push(SIGPIPE, SIG_IGN);
      		if (report_status_v2)
      			report_v2(commands, &unpack_status);
     
      ## object-file.c ##
    +@@
    + #define USE_THE_REPOSITORY_VARIABLE
    + 
    + #include "git-compat-util.h"
    ++#include "config.h"
    + #include "convert.h"
    + #include "dir.h"
    + #include "environment.h"
     @@
      #include "packfile.h"
      #include "path.h"
    @@ object-file.c: static int odb_transaction_files_commit(struct odb_transaction *b
     +		     ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
     +}
     +
    ++static int get_unpack_limit(struct repository *repo)
    ++{
    ++	static int limit = -1;
    ++
    ++	if (limit < 0) {
    ++		int receive_limit = -1;
    ++		int transfer_limit = -1;
    ++
    ++		repo_config_get_int(repo, "receive.unpacklimit",
    ++				    &receive_limit);
    ++		repo_config_get_int(repo, "transfer.unpacklimit",
    ++				    &transfer_limit);
    ++
    ++		if (receive_limit >= 0)
    ++			limit = receive_limit;
    ++		else if (transfer_limit >= 0)
    ++			limit = transfer_limit;
    ++		else
    ++			limit = 100;
    ++	}
    ++
    ++	return limit;
    ++}
    ++
     +static int odb_transaction_files_write_pack(struct odb_transaction *base,
     +					    int pack_fd, struct strbuf *err_msg,
     +					    const struct odb_transaction_write_pack_opts *opts)
    @@ object-file.c: static int odb_transaction_files_commit(struct odb_transaction *b
     +
     +	odb_transaction_env(base, &child.env);
     +
    -+	if (ntohl(hdr.hdr_entries) < (unsigned int)opts->unpack_limit) {
    ++	if (ntohl(hdr.hdr_entries) < (unsigned int)get_unpack_limit(repo)) {
     +		strvec_push(&child.args, "unpack-objects");
     +		push_header_arg(&child.args, &hdr);
     +		if (opts->quiet)
    @@ object-file.c: static int odb_transaction_files_commit(struct odb_transaction *b
     +			strbuf_addstr(err_msg, "index-pack abnormal exit");
     +			return -1;
     +		}
    -+		odb_reprepare(repo->objects);
    ++
    ++		odb_source_prepare(repo->objects->sources,
    ++				   ODB_PREPARE_FLUSH_CACHES);
     +	}
     +
     +	return 0;
     +}
     +
    -+static void odb_transaction_files_release(struct odb_transaction *base)
    ++static int odb_transaction_files_finalize(struct odb_transaction *base)
     +{
     +	struct odb_transaction_files *transaction =
     +		container_of(base, struct odb_transaction_files, base);
    ++	int ret = 0;
     +
     +	for (size_t i = 0; i < transaction->pack_lockfiles_nr; i++)
    -+		delete_tempfile(&transaction->pack_lockfiles[i]);
    ++		ret |= delete_tempfile(&transaction->pack_lockfiles[i]);
    ++
     +	free(transaction->pack_lockfiles);
    ++
    ++	return ret;
     +}
     +
      static int odb_transaction_files_env(struct odb_transaction *base,
    @@ object-file.c: int odb_transaction_files_begin(struct odb_source *source,
      	transaction = xcalloc(1, sizeof(*transaction));
      	transaction->base.source = source;
      	transaction->base.commit = odb_transaction_files_commit;
    -+	transaction->base.release = odb_transaction_files_release;
    ++	transaction->base.finalize = odb_transaction_files_finalize;
      	transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
     +	transaction->base.write_pack = odb_transaction_files_write_pack;
      	transaction->base.env = odb_transaction_files_env;
    @@ odb/transaction.h
     +	 * are verified.
     +	 */
     +	const char *fsck_msg_types;
    ++
     +	/*
     +	 * Path to an alternative shallow file describing the shallow boundaries
     +	 * to honor while ingesting the pack.
     +	 */
     +	const char *shallow_file;
    ++
     +	/*
     +	 * The max size in bytes of the incoming packfile allowed. No limit is
     +	 * enforced when set to 0.
     +	 */
    ++
     +	off_t max_input_size;
    ++
     +	/*
     +	 * Whether the validity of incoming objects should be verified.
     +	 */
     +	int fsck_objects;
    -+	/*
    -+	 * The threshold for the number of incoming objects required to store
    -+	 * the objects in a packfile. This option may not be relevant to
    -+	 * backends that do not store obejcts in loose/packed formats and can be
    -+	 * ignored.
    -+	 */
    -+	int unpack_limit;
    ++
     +	/*
     +	 * Whether to reject an incoming packfile if it is "thin".
     +	 */
     +	int reject_thin;
    ++
     +	/*
     +	 * Optional file descriptor for reporting progress and errors. Set to 0
     +	 * for none.
     +	 */
     +	int err_fd;
    ++
     +	/*
     +	 * Suppresses progress reporting.
     +	 */

base-commit: 2c78326f810173a4f3aefd8021f1e07575412481
-- 
2.55.0.424.g13c7afec21