[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