[PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Aug 20, 2026, 23:49 UTC
- 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 <jltobler@gmail.com>
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 packfilesbase-commit: 2c78326f810173a4f3aefd8021f1e07575412481
-- 2.55.0.424.g13c7afec21