[PATCH v4 0/9] 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.

Chances since V3:
- In preparation for future `odb_transaction_write_pack()` users, the
  unpack limit takes into consideration odb_transaction_flags to augment
  configutation.
- Added additional test assertion in first patch to ensure keep file is
  generated and placed in quarantine directory.
- Removed an include statement in favor of just forward declaring a
  struct.
- Updated some commit messages.

Changes since V2:
- Added a patch to address a bug causing ".keep" files from not being
  removed.
- Started handling errors at transaction commit and finalize call sites
  instead of ignoring them. We also make sure
  `odb_transaction_finalize()` runs after every successful commit
  callsite to ensure proper cleanup.
- Updated the code handling lazy loading of unpack limit configuration
  to not longer cache the value.
- Added a patch to begin explictly tracking the ODB source used by the
  "files" transaction to avoid relying on the ordering of the ODB source
  list.
- Updated some commit messages to improve clarity.

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 (9):
  builtin/receive-pack: properly clean up keep files
  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: return temporary ODB source when set
  odb/transaction: add transaction interface to write packfiles

 builtin/add.c              |   4 +-
 builtin/receive-pack.c     | 211 ++++++++-----------------------------
 builtin/unpack-objects.c   |   2 +-
 builtin/update-index.c     |   4 +-
 cache-tree.c               |   2 +-
 fetch-pack.c               |   2 +-
 object-file.c              | 183 +++++++++++++++++++++++++++++++-
 odb.c                      |   9 +-
 odb.h                      |   6 +-
 odb/transaction.c          |  21 ++++
 odb/transaction.h          |  85 +++++++++++++++
 pack-write.c               |   7 +-
 pack.h                     |   4 +-
 read-cache.c               |   2 +-
 t/t5547-push-quarantine.sh |  22 ++++
 tmp-objdir.c               |   8 +-
 tmp-objdir.h               |   6 +-
 17 files changed, 390 insertions(+), 188 deletions(-)

Range-diff against v3:
 1:  58569303f9 !  1:  13a57feea7 builtin/receive-pack: properly clean up keep files
    @@ Commit message
         builtin/receive-pack: properly clean up keep files
     
         When git-receive-pack(1) stores an incoming packfile with
    -    git-index-pack(1), a ".keep" file is written alongside it to hold the
    -    pack in place until the references have been updated, and is removed
    -    afterwards. The path used to remove it is derived via
    +    git-index-pack(1), a ".keep" file is written alongside it in the
    +    transaction quarantine directory and also gets migrated to the main ODB
    +    when the ODB transaction is committed. This keep lockfile ensures the
    +    packfile remains in place until the references have been updated and is
    +    removed afterwards. The path used to remove it is derived via
         `index_pack_lockfile()` from the repository's primary object directory.
     
         In bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB
    @@ Commit message
         parent process would set the temporary directory set as an alternate
         only. By using ODB transactions, the ODB source list is also reordered
         for the parent process which results in `index_pack_lockfile()` deriving
    -    the ".keep" path relative to the temporary directory instead the actual
    -    main ODB source path. Consequently, this prevents the ".keep" file from
    -    being properly removed after being migrated into the main ODB source
    -    post-commit.
    +    the ".keep" path relative to the temporary directory instead of the
    +    actual main ODB source path. Consequently, this prevents the ".keep"
    +    file from being properly removed after being migrated into the main ODB
    +    source post-commit.
     
         Update `index_pack_lockfile()` to operate on an ODB source explicitly
         provided to it and update call sites accordingly to pass the expected
    @@ pack-write.c: char *index_pack_lockfile(struct repository *r, int ip_out, int *i
     
      ## pack.h ##
     @@
    - 
    - #include "object.h"
    - #include "csum-file.h"
    -+#include "odb/source.h"
    - 
      struct packed_git;
      struct pack_window;
    + struct repository;
    ++struct odb_source;
    + 
    + /*
    +  * Packed object header
     @@ pack.h: off_t write_pack_header(struct hashfile *f, uint32_t);
      void fixup_pack_header_footer(const struct git_hash_algo *, int,
      			      unsigned char *, const char *, uint32_t,
    @@ t/t5547-push-quarantine.sh: test_expect_success 'updating a ref from quarantine
     +	git init --bare keep.git &&
     +
     +	git -C keep.git config set receive.unpackLimit 0 &&
    ++
    ++	# While incoming objects are still quarantined, validate that the keep
    ++	# lockfile does indeed exist.
    ++	test_hook -C keep.git pre-receive <<-\EOF &&
    ++	keep="$(ls "$GIT_QUARANTINE_PATH"/pack/pack-*.keep)" &&
    ++	test -f "$keep"
    ++	EOF
    ++
     +	test_commit foo &&
     +	git push keep.git HEAD &&
     +	pack="$(ls keep.git/objects/pack/pack-*.pack)" &&
 2:  dba9696866 !  2:  49254af71c odb/transaction: add transaction finalize interface
    @@ Commit message
         after the reference updates are finished.
     
         All other callers commit a transaction and immediately finalize it with
    -    no work in between and cannot meaningfully recover should either step
    -    fail, so introduce an `odb_transaction_commit_and_finalize_or_die()`
    -    helper that performs both and dies on error. Call sites are updated
    +    no work in between and cannot meaningfully recover should either fail,
    +    so introduce an `odb_transaction_commit_and_finalize_or_die()` helper
    +    that performs both and dies on error. Call sites are updated
         accordingly.
     
         Signed-off-by: Justin Tobler <jltobler@gmail.com>
 3:  09bc00a070 =  3:  882cf06bc3 builtin/receive-pack: pass shallow file explicitly
 4:  2586ea4041 =  4:  8deec37a09 builtin/receive-pack: read unpack limit config lazily
 5:  adf325095e =  5:  92d56134f0 builtin/receive-pack: lift global state out of unpack()
 6:  29f407bf36 =  6:  d614b10715 builtin/receive-pack: report unpack errors via strbuf
 7:  b85f5e868c =  7:  bc5839ad8e builtin/receive-pack: explicitly pass packfile fd
 8:  620eafe035 =  8:  13540b91b8 odb: return temporary ODB source when set
 9:  2e75a8bd6c !  9:  62d46d5c07 odb/transaction: add transaction interface to write packfiles
    @@ object-file.c
      #include "setup.h"
      #include "strvec.h"
      #include "tempfile.h"
    -@@ object-file.c: struct odb_transaction_files {
    +@@ object-file.c: struct transaction_packfile {
    + 
    + struct odb_transaction_files {
    + 	struct odb_transaction base;
    ++	enum odb_transaction_flags flags;
    + 
    + 	struct tmp_objdir *objdir;
      	struct odb_source *quarantine;
      	struct transaction_packfile packfile;
      	const char *prefix;
    @@ object-file.c: static int odb_transaction_files_commit(struct odb_transaction *b
     +		     ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
     +}
     +
    -+static unsigned int get_unpack_limit(struct repository *repo)
    ++static unsigned int get_unpack_limit(struct repository *repo,
    ++				     enum odb_transaction_flags flags)
     +{
    -+	unsigned int limit = 100;
    ++	unsigned int limit = 0;
     +
    -+	repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
    -+	repo_config_get_uint(repo, "receive.unpacklimit", &limit);
    ++	if (flags & ODB_TRANSACTION_RECEIVE) {
    ++		limit = 100;
    ++		repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
    ++		repo_config_get_uint(repo, "receive.unpacklimit", &limit);
    ++	}
     +
     +	return limit;
     +}
    @@ object-file.c: static int odb_transaction_files_commit(struct odb_transaction *b
     +
     +	odb_transaction_env(base, &child.env);
     +
    -+	if (ntohl(hdr.hdr_entries) < get_unpack_limit(repo)) {
    ++	if (ntohl(hdr.hdr_entries) < get_unpack_limit(repo, transaction->flags)) {
     +		strvec_push(&child.args, "unpack-objects");
     +		push_header_arg(&child.args, &hdr);
     +		if (opts->quiet)
    @@ object-file.c: int odb_transaction_files_begin(struct odb_source *source,
      	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;
    ++	transaction->flags = flags;
      
      	transaction->prefix = "bulk-fsync";
    + 	if (flags & ODB_TRANSACTION_RECEIVE) {
     
      ## odb/transaction.c ##
     @@ odb/transaction.c: int odb_transaction_write_object_stream(struct odb_transaction *transaction,

base-commit: 2c78326f810173a4f3aefd8021f1e07575412481
-- 
2.55.0.424.g13c7afec21