[PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes

Justin Tobler <[email protected]>
Newsgroups org.kernel.vger.git
Message-ID <[email protected]>
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 V4:
- Added an additional test assertion in the frist patch to ensure keep
  files are also migrated to the main ODB prior to being removed when
  the transaction is finalized.
- Updated a commit message.

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 |  31 ++++++
 tmp-objdir.c               |   8 +-
 tmp-objdir.h               |   6 +-
 17 files changed, 399 insertions(+), 188 deletions(-)

Range-diff against v4:
 1:  13a57feea7 !  1:  1bae015e8c builtin/receive-pack: properly clean up keep files
    @@ t/t5547-push-quarantine.sh: test_expect_success 'updating a ref from quarantine
     +
     +	git -C keep.git config set receive.unpackLimit 0 &&
     +
    -+	# While incoming objects are still quarantined, validate that the keep
    -+	# lockfile does indeed exist.
    ++	# While incoming objects are still quarantined, validate that the
    ++	# ".keep" lockfile is present in the quarantine directory.
     +	test_hook -C keep.git pre-receive <<-\EOF &&
     +	keep="$(ls "$GIT_QUARANTINE_PATH"/pack/pack-*.keep)" &&
     +	test -f "$keep"
     +	EOF
     +
    ++	# After quarantined objects are migrated, validate that the ".keep"
    ++	# lockfile is migrated and present in the main ODB.
    ++	test_hook -C keep.git reference-transaction <<-\EOF &&
    ++	keep="$(ls objects/pack/pack-*.keep)" &&
    ++	test -f "$keep"
    ++	EOF
    ++
     +	test_commit foo &&
     +	git push keep.git HEAD &&
    ++
    ++	# Once the operation is complete, validate that the ".keep" lockfile has
    ++	# been removed.
     +	pack="$(ls keep.git/objects/pack/pack-*.pack)" &&
     +	keep="${pack%.pack}.keep" &&
    -+
     +	test_path_is_file "$pack" &&
     +	test_path_is_missing "$keep"
     +'
 2:  49254af71c !  2:  a2a10966a8 odb/transaction: add transaction finalize interface
    @@ Commit message
         `odb_transaction_finalize()` call site in git-receive-pack(1) is made
         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 fail,
    -    so introduce an `odb_transaction_commit_and_finalize_or_die()` helper
    -    that performs both and dies on error. Call sites are updated
    -    accordingly.
    +    All other callers commit a transaction and immediately finalize it
    +    without any work happening in between those two operations.
    +    Consequently, they cannot meaningfully recover in case either of them
    +    would fail, and spelling out these two separate steps with proper error
    +    handling would be quite repetitive and pointless. Introduce a helper
    +    `odb_transaction_commit_and_finalize_or_die()` for those call sites and
    +    update them accordingly.
     
         Signed-off-by: Justin Tobler <[email protected]>
     
 3:  882cf06bc3 =  3:  063b1830a1 builtin/receive-pack: pass shallow file explicitly
 4:  8deec37a09 =  4:  04c42ebefd builtin/receive-pack: read unpack limit config lazily
 5:  92d56134f0 =  5:  f4a633a212 builtin/receive-pack: lift global state out of unpack()
 6:  d614b10715 =  6:  9b89af0bd8 builtin/receive-pack: report unpack errors via strbuf
 7:  bc5839ad8e =  7:  edb54e79f6 builtin/receive-pack: explicitly pass packfile fd
 8:  13540b91b8 =  8:  452affa42f odb: return temporary ODB source when set
 9:  62d46d5c07 =  9:  dae4b96bc3 odb/transaction: add transaction interface to write packfiles

base-commit: 2c78326f810173a4f3aefd8021f1e07575412481
-- 
2.55.0.424.g13c7afec21
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.