[PATCH v2 0/7] builtin/receive-pack: support pluggable packfile writes
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Aug 9, 2026, 19:00 UTC
- Message-ID
- <20260809190106.1565882-1-jltobler@gmail.com>
- In-Reply-To
- <20260806213859.816157-1-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