From: Justin Tobler Date: Thu, 20 Aug 2026 23:49:31 GMT Subject: [PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes Message-ID: <20260820234940.894624-1-jltobler@gmail.com> In-Reply-To: <20260819215311.3880274-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 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 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