From 7874b2c7e1889a28482d50905b5d2ba7e480bfb8 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Thu, 20 Aug 2026 18:49:32 -0500 Subject: [PATCH 1/9] 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 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 transactions, 2026-07-10), git-receive-pack(1) started using the ODB transaction interfaces instead of managing a temporary directory directly. When starting an ODB transaction, the sources list is reordered to insert the newly created transaction source first as the primary to ensure writes are routed to it accordingly. Prior to using ODB transactions, git-receive-pack(1) would only set the temporary directory as the primary source for the child git-index-pack(1) and git-unpack-objects(1) processes it spawned and the 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 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 ODB source. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 8 +++++++- fetch-pack.c | 2 +- pack-write.c | 7 ++++--- pack.h | 4 +++- t/t5547-push-quarantine.sh | 31 +++++++++++++++++++++++++++++++ 5 files changed, 46 insertions(+), 6 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 86933d8d7e..d74b787148 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -2412,7 +2412,13 @@ static const char *unpack(int err_fd, struct shallow_info *si, if (status) return "index-pack fork failed"; - lockfile = index_pack_lockfile(the_repository, child.out, NULL); + /* + * The lockfile filepath is expected to be the final location of + * the ".keep" file after being migrated to the main ODB source. + * This ensures the lockfile can be found and removed later + * after the ODB transaction has been committed. + */ + lockfile = index_pack_lockfile(transaction->source, child.out, NULL); if (lockfile) { pack_lockfile = register_tempfile(lockfile); free(lockfile); diff --git a/fetch-pack.c b/fetch-pack.c index 922a9b2581..6df5813b33 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1075,7 +1075,7 @@ static int get_pack(struct fetch_pack_args *args, die(_("fetch-pack: unable to fork off %s"), cmd_name); if (do_keep && (pack_lockfiles || fsck_objects)) { int is_well_formed; - char *pack_lockfile = index_pack_lockfile(the_repository, + char *pack_lockfile = index_pack_lockfile(the_repository->objects->sources, cmd.out, &is_well_formed); diff --git a/pack-write.c b/pack-write.c index 24033a9101..85674e4b72 100644 --- a/pack-write.c +++ b/pack-write.c @@ -469,10 +469,11 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo, fsync_component_or_die(FSYNC_COMPONENT_PACK, pack_fd, pack_name); } -char *index_pack_lockfile(struct repository *r, int ip_out, int *is_well_formed) +char *index_pack_lockfile(struct odb_source *source, int ip_out, + int *is_well_formed) { char packname[GIT_MAX_HEXSZ + 6]; - const int len = r->hash_algo->hexsz + 6; + const int len = source->odb->repo->hash_algo->hexsz + 6; /* * The first thing we expect from index-pack's output @@ -489,7 +490,7 @@ char *index_pack_lockfile(struct repository *r, int ip_out, int *is_well_formed) packname[len-1] = 0; if (skip_prefix(packname, "keep\t", &name)) return xstrfmt("%s/pack/pack-%s.keep", - repo_get_object_directory(r), name); + source->path, name); return NULL; } if (is_well_formed) diff --git a/pack.h b/pack.h index 1cde92082b..ada506b5c5 100644 --- a/pack.h +++ b/pack.h @@ -7,6 +7,7 @@ struct packed_git; struct pack_window; struct repository; +struct odb_source; /* * Packed object header @@ -105,7 +106,8 @@ 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, unsigned char *, off_t); -char *index_pack_lockfile(struct repository *r, int fd, int *is_well_formed); +char *index_pack_lockfile(struct odb_source *source, int fd, + int *is_well_formed); struct ref; diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh index 0798ddab02..1b7097179e 100755 --- a/t/t5547-push-quarantine.sh +++ b/t/t5547-push-quarantine.sh @@ -70,4 +70,35 @@ test_expect_success 'updating a ref from quarantine is forbidden' ' git -C update.git fsck ' +test_expect_success '.keep file is removed after push' ' + test_when_finished rm -rf keep.git && + 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 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" +' + test_done From ecdda043e0e200a18fabf99b7e793fc33367e6c4 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Thu, 20 Aug 2026 18:49:33 -0500 Subject: [PATCH 2/9] odb/transaction: add transaction finalize interface When committing an ODB transaction via `odb_transaction_commit()`, the staged objects are made visible and the underlying transaction is freed at the same time. Coupling these two steps does not leave room for any post-commit transaction operations to be introduced though. Such a capability is useful if an ODB transaction backend needs to hold on to lockfiles after transaction commit until references are updated, as is the case with the existing "files" backend in git-receive-pack(1). Stop freeing the transaction in `odb_transaction_commit()` and introduce `odb_transaction_finalize()` to explicitly clean up the transaction accordingly. Note that the finalize interface also provides an optional callback for any backend-specific deferred cleanup. In a subsequent commit, the "files" transaction backend will use this to remove ".keep" files generated for packfiles received via git-receive-pack(1) after references have been updated. In preparation for this, the `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 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 Signed-off-by: Junio C Hamano --- builtin/add.c | 4 ++-- builtin/receive-pack.c | 1 + builtin/unpack-objects.c | 2 +- builtin/update-index.c | 4 ++-- cache-tree.c | 2 +- object-file.c | 2 +- odb/transaction.c | 14 ++++++++++++++ odb/transaction.h | 23 +++++++++++++++++++++++ read-cache.c | 2 +- 9 files changed, 46 insertions(+), 8 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index 60ffbede2b..ad418a5952 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -393,7 +393,7 @@ int cmd_add(int argc, char *seen = NULL; char *ps_matched = NULL; struct lock_file lock_file = LOCK_INIT; - struct odb_transaction *transaction; + struct odb_transaction *transaction = NULL; repo_config(repo, add_config, NULL); @@ -600,7 +600,7 @@ int cmd_add(int argc, if (chmod_arg && pathspec.nr) exit_status |= chmod_pathspec(repo, &pathspec, chmod_arg[0], show_only); - odb_transaction_commit(transaction); + odb_transaction_commit_and_finalize_or_die(transaction); finish: if (write_locked_index(repo->index, &lock_file, diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index d74b787148..ed1edcbe93 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -2720,6 +2720,7 @@ int cmd_receive_pack(int argc, use_keepalive = KEEPALIVE_ALWAYS; execute_commands(commands, unpack_status, &si, transaction, &push_options); + odb_transaction_finalize(transaction); delete_tempfile(&pack_lockfile); sigchain_push(SIGPIPE, SIG_IGN); if (report_status_v2) diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 4263edfbec..d6a2d616d9 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -603,7 +603,7 @@ static void unpack_all(void) unpack_one(i); display_progress(progress, i + 1); } - odb_transaction_commit(transaction); + odb_transaction_commit_and_finalize_or_die(transaction); stop_progress(&progress); if (delta_list) diff --git a/builtin/update-index.c b/builtin/update-index.c index 241abd4332..b25d4ecb10 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -1156,7 +1156,7 @@ int cmd_update_index(int argc, * a transaction. */ if (transaction && verbose) { - odb_transaction_commit(transaction); + odb_transaction_commit_and_finalize_or_die(transaction); transaction = NULL; } @@ -1224,7 +1224,7 @@ int cmd_update_index(int argc, /* * By now we have added all of the new objects */ - odb_transaction_commit(transaction); + odb_transaction_commit_and_finalize_or_die(transaction); if (split_index > 0) { if (repo_config_get_split_index(the_repository) == 0) diff --git a/cache-tree.c b/cache-tree.c index d92f513286..a220372a42 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -538,7 +538,7 @@ int cache_tree_update(struct index_state *istate, int flags) i = update_one(istate->cache_tree, istate->cache, istate->cache_nr, "", 0, &skip, flags); if (!inflight) - odb_transaction_commit(transaction); + odb_transaction_commit_and_finalize_or_die(transaction); trace2_region_leave("cache_tree", "update", istate->repo); trace_performance_leave("cache_tree_update"); if (i < 0) diff --git a/object-file.c b/object-file.c index ec35c318bc..4d03c167d5 100644 --- a/object-file.c +++ b/object-file.c @@ -965,7 +965,7 @@ int index_fd(struct index_state *istate, struct object_id *oid, xsize_t(st->st_size), oid); if (!inflight) - odb_transaction_commit(transaction); + odb_transaction_commit_and_finalize_or_die(transaction); } else { ret = hash_blob_stream(&stream, the_repository->hash_algo, oid, diff --git a/odb/transaction.c b/odb/transaction.c index dab7da6a9a..9e9a982778 100644 --- a/odb/transaction.c +++ b/odb/transaction.c @@ -33,6 +33,20 @@ int odb_transaction_commit(struct odb_transaction *transaction) ret = transaction->commit(transaction); transaction->source->odb->transaction = NULL; + + return ret; +} + +int odb_transaction_finalize(struct odb_transaction *transaction) +{ + int ret = 0; + + if (!transaction) + return 0; + + if (transaction->finalize) + ret = transaction->finalize(transaction); + free(transaction); return ret; diff --git a/odb/transaction.h b/odb/transaction.h index 4cb2eafcbf..6ed39b3d0e 100644 --- a/odb/transaction.h +++ b/odb/transaction.h @@ -22,6 +22,13 @@ struct odb_transaction { */ int (*commit)(struct odb_transaction *transaction); + /* + * Optional ODB source specific callback invoked when the transaction + * needs to perform any deferred cleanup after objects have been + * committed. Returns 0 on success, a negative error code otherwise. + */ + int (*finalize)(struct odb_transaction *transaction); + /* * This callback is expected to write the given object stream into * the ODB transaction. Note that for now, only blobs support streaming. @@ -75,6 +82,22 @@ static inline void odb_transaction_begin_or_die(struct object_database *odb, */ int odb_transaction_commit(struct odb_transaction *transaction); +/* + * Finalizes an ODB transaction, performing any deferred cleanup and freeing it. + * Must be called for every successfully started transaction. Note that, if the + * specified transaction is NULL, the function is a no-op. Returns 0 on success, + * a negative error code otherwise. + */ +int odb_transaction_finalize(struct odb_transaction *transaction); + +static inline void odb_transaction_commit_and_finalize_or_die(struct odb_transaction *transaction) +{ + if (odb_transaction_commit(transaction)) + die(_("failed to commit ODB transaction")); + if (odb_transaction_finalize(transaction)) + die(_("failed to finalize ODB transaction")); +} + /* * Writes the object in the provided stream into the transaction. The resulting * object ID is written into the out pointer. Returns 0 on success, a negative diff --git a/read-cache.c b/read-cache.c index 6c449f393d..0cd0ef85ec 100644 --- a/read-cache.c +++ b/read-cache.c @@ -4049,7 +4049,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix, odb_transaction_begin_or_die(repo->objects, &transaction, 0); run_diff_files(&rev, DIFF_RACY_IS_MODIFIED); if (!inflight) - odb_transaction_commit(transaction); + odb_transaction_commit_and_finalize_or_die(transaction); release_revisions(&rev); return !!data.add_errors; From 727b99cdb11395ad6932bdb3fb4923a98c23c803 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Thu, 20 Aug 2026 18:49:34 -0500 Subject: [PATCH 3/9] builtin/receive-pack: pass shallow file explicitly If shallow information is provided during `unpack()`, a temporary shallow file is created and stored in global state. In a subsequent commit, the `unpack()` logic is moved behind a generic ODB transaction interface to handle writing packfiles and thus can no longer rely on such global state. Lift the setup of the temporary shallow file out of `unpack()` and wire it through to its call sites explicitly. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index ed1edcbe93..135105deae 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -86,7 +86,6 @@ static const char *head_name; static void *head_name_to_free; static int sent_capabilities; static int shallow_update; -static const char *alt_shallow_file; static struct strbuf push_cert = STRBUF_INIT; static struct object_id push_cert_oid; static struct signature_check sigcheck; @@ -2334,8 +2333,8 @@ static void push_header_arg(struct strvec *args, struct pack_header *hdr) ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries)); } -static const char *unpack(int err_fd, struct shallow_info *si, - struct odb_transaction *transaction) +static const char *unpack(struct odb_transaction *transaction, + const char *shallow_file, int err_fd) { struct pack_header hdr; const char *hdr_err; @@ -2354,10 +2353,9 @@ static const char *unpack(int err_fd, struct shallow_info *si, return hdr_err; } - if (si->nr_ours || si->nr_theirs) { - alt_shallow_file = setup_temporary_shallow(si->shallow); + if (shallow_file) { strvec_push(&child.args, "--shallow-file"); - strvec_push(&child.args, alt_shallow_file); + strvec_push(&child.args, shallow_file); } odb_transaction_env(transaction, &child.env); @@ -2433,14 +2431,14 @@ static const char *unpack(int err_fd, struct shallow_info *si, return NULL; } -static const char *unpack_with_sideband(struct shallow_info *si, - struct odb_transaction *transaction) +static const char *unpack_with_sideband(struct odb_transaction *transaction, + const char *shallow_file) { struct async muxer; const char *ret; if (!use_sideband) - return unpack(0, si, transaction); + return unpack(transaction, shallow_file, 0); use_keepalive = KEEPALIVE_AFTER_NUL; memset(&muxer, 0, sizeof(muxer)); @@ -2449,13 +2447,14 @@ static const char *unpack_with_sideband(struct shallow_info *si, if (start_async(&muxer)) return NULL; - ret = unpack(muxer.in, si, transaction); + ret = unpack(transaction, shallow_file, muxer.in); finish_async(&muxer); return ret; } -static void prepare_shallow_update(struct shallow_info *si) +static void prepare_shallow_update(struct shallow_info *si, + const char *shallow_file) { int i, j, k, bitmap_size = DIV_ROUND_UP(si->ref->nr, 32); @@ -2495,12 +2494,13 @@ static void prepare_shallow_update(struct shallow_info *si) * command. check_connected() will be done with * true .git/shallow though. */ - setenv(GIT_SHALLOW_FILE_ENVIRONMENT, alt_shallow_file, 1); + setenv(GIT_SHALLOW_FILE_ENVIRONMENT, shallow_file, 1); } static void update_shallow_info(struct command *commands, struct shallow_info *si, - struct oid_array *ref) + struct oid_array *ref, + const char *shallow_file) { struct command *cmd; int *ref_status; @@ -2519,7 +2519,7 @@ static void update_shallow_info(struct command *commands, si->ref = ref; if (shallow_update) { - prepare_shallow_update(si); + prepare_shallow_update(si, shallow_file); return; } @@ -2711,11 +2711,17 @@ int cmd_receive_pack(int argc, if (!si.nr_ours && !si.nr_theirs) shallow_update = 0; if (!delete_only(commands)) { + const char *alt_shallow_file = NULL; + + if (si.nr_ours || si.nr_theirs) + alt_shallow_file = setup_temporary_shallow(si.shallow); + if (odb_transaction_begin(the_repository->objects, &transaction, ODB_TRANSACTION_RECEIVE)) unpack_status = "unable to start object transaction"; else - unpack_status = unpack_with_sideband(&si, transaction); - update_shallow_info(commands, &si, &ref); + unpack_status = unpack_with_sideband(transaction, alt_shallow_file); + + update_shallow_info(commands, &si, &ref, alt_shallow_file); } use_keepalive = KEEPALIVE_ALWAYS; execute_commands(commands, unpack_status, &si, transaction, From c59466d4cfcc4b0cf364bebc32ede9da53abf9a3 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Thu, 20 Aug 2026 18:49:35 -0500 Subject: [PATCH 4/9] builtin/receive-pack: read unpack limit config lazily In git-receive-pack(1), the `receive.unpackLimit` and `transfer.unpackLimit` configuration decides whether an incoming packfile should be exploded into loose objects or kept as a packfile on-disk. In a subsequent commit, the logic to write the incoming packfile is made ODB backend agnostic and moved behind a pluggable ODB transaction interface. Consequently, whether to explode a packfile is a detail of how a particular backend stores objects and should not be a part of the generic interface itself. In preparation for this, instead resolve the unpack limit lazily inside `unpack()` by reading the configuration directly. The now-unused unpack limit globals are dropped accordingly. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 135105deae..971dc3f52e 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -62,12 +62,9 @@ static enum deny_action deny_delete_current = DENY_UNCONFIGURED; static int receive_fsck_objects = -1; static int transfer_fsck_objects = -1; static struct strbuf fsck_msg_types = STRBUF_INIT; -static int receive_unpack_limit = -1; -static int transfer_unpack_limit = -1; static int advertise_atomic_push = 1; static int advertise_push_options; static int advertise_sid; -static int unpack_limit = 100; static off_t max_input_size; static int report_status; static int report_status_v2; @@ -157,16 +154,6 @@ static int receive_pack_config(const char *var, const char *value, return 0; } - if (strcmp(var, "receive.unpacklimit") == 0) { - receive_unpack_limit = git_config_int(var, value, ctx->kvi); - return 0; - } - - if (strcmp(var, "transfer.unpacklimit") == 0) { - transfer_unpack_limit = git_config_int(var, value, ctx->kvi); - return 0; - } - if (strcmp(var, "receive.fsck.skiplist") == 0) { char *path; @@ -2333,6 +2320,16 @@ static void push_header_arg(struct strvec *args, struct pack_header *hdr) ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries)); } +static unsigned int get_unpack_limit(struct repository *repo) +{ + unsigned int limit = 100; + + repo_config_get_uint(repo, "transfer.unpacklimit", &limit); + repo_config_get_uint(repo, "receive.unpacklimit", &limit); + + return limit; +} + static const char *unpack(struct odb_transaction *transaction, const char *shallow_file, int err_fd) { @@ -2360,7 +2357,7 @@ static const char *unpack(struct odb_transaction *transaction, odb_transaction_env(transaction, &child.env); - if (ntohl(hdr.hdr_entries) < unpack_limit) { + if (ntohl(hdr.hdr_entries) < get_unpack_limit(the_repository)) { strvec_push(&child.args, "unpack-objects"); push_header_arg(&child.args, &hdr); if (quiet) @@ -2658,11 +2655,6 @@ int cmd_receive_pack(int argc, if (cert_nonce_seed) push_cert_nonce = prepare_push_cert_nonce(service_dir, time(NULL)); - if (0 <= receive_unpack_limit) - unpack_limit = receive_unpack_limit; - else if (0 <= transfer_unpack_limit) - unpack_limit = transfer_unpack_limit; - switch (determine_protocol_version_server()) { case protocol_v2: /* From 255f3a7a5318b7c2dc97c15845af3e4ec5f51d9a Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Thu, 20 Aug 2026 18:49:36 -0500 Subject: [PATCH 5/9] builtin/receive-pack: lift global state out of unpack() In git-receive-pack(1), writing the packfile to the transaction is handled via `unpack()` which relies on global variables to decide how to invoke the underlying git-index-pack(1) or git-unpack-objects(1) child processes. In a subsequent commit, the `unpack()` logic is moved behind a generic ODB transaction interface to handle writing packfiles and thus can no longer rely on these globals. Lift the global state out of `unpack()` by instead storing this state in a `struct unpack_opts` that gets passed to the function explicitly. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 63 +++++++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 22 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 971dc3f52e..f062b93b8d 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -2330,18 +2330,24 @@ static unsigned int get_unpack_limit(struct repository *repo) return limit; } +struct unpack_opts { + const char *fsck_msg_types; + const char *shallow_file; + off_t max_input_size; + int fsck_objects; + int reject_thin; + int err_fd; + int quiet; +}; + static const char *unpack(struct odb_transaction *transaction, - const char *shallow_file, int err_fd) + const struct unpack_opts *opts) { struct pack_header hdr; const char *hdr_err; int status; struct child_process child = CHILD_PROCESS_INIT; - int fsck_objects = (receive_fsck_objects >= 0 - ? receive_fsck_objects - : transfer_fsck_objects >= 0 - ? transfer_fsck_objects - : 0); + int err_fd = opts->err_fd; hdr_err = parse_pack_header(&hdr); if (hdr_err) { @@ -2350,9 +2356,9 @@ static const char *unpack(struct odb_transaction *transaction, return hdr_err; } - if (shallow_file) { + if (opts->shallow_file) { strvec_push(&child.args, "--shallow-file"); - strvec_push(&child.args, shallow_file); + strvec_push(&child.args, opts->shallow_file); } odb_transaction_env(transaction, &child.env); @@ -2360,14 +2366,14 @@ static const char *unpack(struct odb_transaction *transaction, if (ntohl(hdr.hdr_entries) < get_unpack_limit(the_repository)) { strvec_push(&child.args, "unpack-objects"); push_header_arg(&child.args, &hdr); - if (quiet) + if (opts->quiet) strvec_push(&child.args, "-q"); - if (fsck_objects) + if (opts->fsck_objects) strvec_pushf(&child.args, "--strict%s", - fsck_msg_types.buf); - if (max_input_size) + opts->fsck_msg_types); + if (opts->max_input_size) strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX, - (uintmax_t)max_input_size); + (uintmax_t)opts->max_input_size); child.no_stdout = 1; child.err = err_fd; child.git_cmd = 1; @@ -2388,18 +2394,18 @@ static const char *unpack(struct odb_transaction *transaction, (uintmax_t)getpid(), hostname); - if (!quiet && err_fd) + if (!opts->quiet && err_fd) strvec_push(&child.args, "--show-resolving-progress"); - if (use_sideband) + if (err_fd) strvec_push(&child.args, "--report-end-of-input"); - if (fsck_objects) + if (opts->fsck_objects) strvec_pushf(&child.args, "--strict%s", - fsck_msg_types.buf); - if (!reject_thin) + opts->fsck_msg_types); + if (!opts->reject_thin) strvec_push(&child.args, "--fix-thin"); - if (max_input_size) + if (opts->max_input_size) strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX, - (uintmax_t)max_input_size); + (uintmax_t)opts->max_input_size); child.out = -1; child.err = err_fd; child.git_cmd = 1; @@ -2431,11 +2437,23 @@ static const char *unpack(struct odb_transaction *transaction, static const char *unpack_with_sideband(struct odb_transaction *transaction, const char *shallow_file) { + struct unpack_opts opts = { + .fsck_objects = (receive_fsck_objects >= 0 + ? receive_fsck_objects + : transfer_fsck_objects >= 0 + ? transfer_fsck_objects + : 0), + .fsck_msg_types = fsck_msg_types.buf, + .max_input_size = max_input_size, + .shallow_file = shallow_file, + .reject_thin = reject_thin, + .quiet = quiet, + }; struct async muxer; const char *ret; if (!use_sideband) - return unpack(transaction, shallow_file, 0); + return unpack(transaction, &opts); use_keepalive = KEEPALIVE_AFTER_NUL; memset(&muxer, 0, sizeof(muxer)); @@ -2444,7 +2462,8 @@ static const char *unpack_with_sideband(struct odb_transaction *transaction, if (start_async(&muxer)) return NULL; - ret = unpack(transaction, shallow_file, muxer.in); + opts.err_fd = muxer.in; + ret = unpack(transaction, &opts); finish_async(&muxer); return ret; From 429dd07aa0bc2082d93edc6dea15da426e8807cb Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Thu, 20 Aug 2026 18:49:37 -0500 Subject: [PATCH 6/9] builtin/receive-pack: report unpack errors via strbuf When writing packfiles via `unpack()`, error messages are returned directly by the function. In preparation for `unpack()` logic being moved behind a generic ODB transaction interface, update the function to instead write any error messages to a caller provided strbuf and return a negative value on error. Call sites are updated to use the error strbuf accordingly. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 63 ++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index f062b93b8d..6df872697b 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -2015,7 +2015,7 @@ cleanup: } static void execute_commands(struct command *commands, - const char *unpacker_error, + int unpacker_error, struct shallow_info *si, struct odb_transaction *transaction, const struct string_list *push_options) @@ -2340,8 +2340,8 @@ struct unpack_opts { int quiet; }; -static const char *unpack(struct odb_transaction *transaction, - const struct unpack_opts *opts) +static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg, + const struct unpack_opts *opts) { struct pack_header hdr; const char *hdr_err; @@ -2353,7 +2353,8 @@ static const char *unpack(struct odb_transaction *transaction, if (hdr_err) { if (err_fd > 0) close(err_fd); - return hdr_err; + strbuf_addstr(err_msg, hdr_err); + return -1; } if (opts->shallow_file) { @@ -2378,8 +2379,10 @@ static const char *unpack(struct odb_transaction *transaction, child.err = err_fd; child.git_cmd = 1; status = run_command(&child); - if (status) - return "unpack-objects abnormal exit"; + if (status) { + strbuf_addstr(err_msg, "unpack-objects abnormal exit"); + return -1; + } } else { char hostname[HOST_NAME_MAX + 1]; char *lockfile; @@ -2410,8 +2413,10 @@ static const char *unpack(struct odb_transaction *transaction, child.err = err_fd; child.git_cmd = 1; status = start_command(&child); - if (status) - return "index-pack fork failed"; + if (status) { + strbuf_addstr(err_msg, "index-pack fork failed"); + return -1; + } /* * The lockfile filepath is expected to be the final location of @@ -2427,15 +2432,18 @@ static const char *unpack(struct odb_transaction *transaction, close(child.out); status = finish_command(&child); - if (status) - return "index-pack abnormal exit"; + if (status) { + strbuf_addstr(err_msg, "index-pack abnormal exit"); + return -1; + } odb_reprepare(the_repository->objects); } - return NULL; + return 0; } -static const char *unpack_with_sideband(struct odb_transaction *transaction, - const char *shallow_file) +static int unpack_with_sideband(struct odb_transaction *transaction, + const char *shallow_file, + struct strbuf *err_msg) { struct unpack_opts opts = { .fsck_objects = (receive_fsck_objects >= 0 @@ -2450,20 +2458,20 @@ static const char *unpack_with_sideband(struct odb_transaction *transaction, .quiet = quiet, }; struct async muxer; - const char *ret; + int ret; if (!use_sideband) - return unpack(transaction, &opts); + return unpack(transaction, err_msg, &opts); use_keepalive = KEEPALIVE_AFTER_NUL; memset(&muxer, 0, sizeof(muxer)); muxer.proc = copy_to_sideband; muxer.in = -1; if (start_async(&muxer)) - return NULL; + return 0; opts.err_fd = muxer.in; - ret = unpack(transaction, &opts); + ret = unpack(transaction, err_msg, &opts); finish_async(&muxer); return ret; @@ -2552,13 +2560,13 @@ static void update_shallow_info(struct command *commands, free(ref_status); } -static void report(struct command *commands, const char *unpack_status) +static void report(struct command *commands, const struct strbuf *unpack_status) { struct command *cmd; struct strbuf buf = STRBUF_INIT; packet_buf_write(&buf, "unpack %s\n", - unpack_status ? unpack_status : "ok"); + unpack_status->len ? unpack_status->buf : "ok"); for (cmd = commands; cmd; cmd = cmd->next) { if (!cmd->error_string) packet_buf_write(&buf, "ok %s\n", @@ -2576,14 +2584,14 @@ static void report(struct command *commands, const char *unpack_status) strbuf_release(&buf); } -static void report_v2(struct command *commands, const char *unpack_status) +static void report_v2(struct command *commands, const struct strbuf *unpack_status) { struct command *cmd; struct strbuf buf = STRBUF_INIT; struct ref_push_report *report; packet_buf_write(&buf, "unpack %s\n", - unpack_status ? unpack_status : "ok"); + unpack_status->len ? unpack_status->buf : "ok"); for (cmd = commands; cmd; cmd = cmd->next) { int count = 0; @@ -2707,8 +2715,8 @@ int cmd_receive_pack(int argc, PACKET_READ_DIE_ON_ERR_PACKET); if ((commands = read_head_info(&reader, &shallow))) { - const char *unpack_status = NULL; struct string_list push_options = STRING_LIST_INIT_DUP; + struct strbuf unpack_status = STRBUF_INIT; if (use_push_options) read_push_options(&reader, &push_options); @@ -2728,22 +2736,22 @@ int cmd_receive_pack(int argc, alt_shallow_file = setup_temporary_shallow(si.shallow); if (odb_transaction_begin(the_repository->objects, &transaction, ODB_TRANSACTION_RECEIVE)) - unpack_status = "unable to start object transaction"; + strbuf_addstr(&unpack_status, "unable to start object transaction"); else - unpack_status = unpack_with_sideband(transaction, alt_shallow_file); + unpack_with_sideband(transaction, alt_shallow_file, &unpack_status); update_shallow_info(commands, &si, &ref, alt_shallow_file); } use_keepalive = KEEPALIVE_ALWAYS; - execute_commands(commands, unpack_status, &si, transaction, + execute_commands(commands, !!unpack_status.len, &si, transaction, &push_options); odb_transaction_finalize(transaction); delete_tempfile(&pack_lockfile); sigchain_push(SIGPIPE, SIG_IGN); if (report_status_v2) - report_v2(commands, unpack_status); + report_v2(commands, &unpack_status); else if (report_status) - report(commands, unpack_status); + report(commands, &unpack_status); sigchain_pop(SIGPIPE); run_receive_hook(commands, "post-receive", 1, NULL, &push_options); @@ -2768,6 +2776,7 @@ int cmd_receive_pack(int argc, if (auto_update_server_info) update_server_info(the_repository, 0); clear_shallow_info(&si); + strbuf_release(&unpack_status); } if (use_sideband) packet_flush(1); From 8e84d34da2bf58fe25ffcd56cb15c60232fb43de Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Thu, 20 Aug 2026 18:49:38 -0500 Subject: [PATCH 7/9] builtin/receive-pack: explicitly pass packfile fd When processing the incoming packfile in git-receive-pack(1), `unpack()` assumes it should always read it from stdin. In preparation for `unpack()` logic being moved behind a generic ODB transaction interface, update the function signature to take the an explicit fd provided by callers to read the incoming packfile from instead. Call sites are updated accordingly. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 6df872697b..b369466783 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -2292,9 +2292,9 @@ static void read_push_options(struct packet_reader *reader, } } -static const char *parse_pack_header(struct pack_header *hdr) +static const char *parse_pack_header(struct pack_header *hdr, int pack_fd) { - switch (read_pack_header(0, hdr)) { + switch (read_pack_header(pack_fd, hdr)) { case PH_ERROR_EOF: return "eof before pack header was fully read"; @@ -2340,8 +2340,8 @@ struct unpack_opts { int quiet; }; -static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg, - const struct unpack_opts *opts) +static int unpack(struct odb_transaction *transaction, int pack_fd, + struct strbuf *err_msg, const struct unpack_opts *opts) { struct pack_header hdr; const char *hdr_err; @@ -2349,7 +2349,7 @@ static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg, struct child_process child = CHILD_PROCESS_INIT; int err_fd = opts->err_fd; - hdr_err = parse_pack_header(&hdr); + hdr_err = parse_pack_header(&hdr, pack_fd); if (hdr_err) { if (err_fd > 0) close(err_fd); @@ -2376,6 +2376,7 @@ static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg, strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX, (uintmax_t)opts->max_input_size); child.no_stdout = 1; + child.in = pack_fd; child.err = err_fd; child.git_cmd = 1; status = run_command(&child); @@ -2410,6 +2411,7 @@ static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg, strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX, (uintmax_t)opts->max_input_size); child.out = -1; + child.in = pack_fd; child.err = err_fd; child.git_cmd = 1; status = start_command(&child); @@ -2461,7 +2463,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction, int ret; if (!use_sideband) - return unpack(transaction, err_msg, &opts); + return unpack(transaction, 0, err_msg, &opts); use_keepalive = KEEPALIVE_AFTER_NUL; memset(&muxer, 0, sizeof(muxer)); @@ -2471,7 +2473,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction, return 0; opts.err_fd = muxer.in; - ret = unpack(transaction, err_msg, &opts); + ret = unpack(transaction, 0, err_msg, &opts); finish_async(&muxer); return ret; From 40932d0a7e35fa31b1ac237ccc77a0a346ce887d Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Thu, 20 Aug 2026 18:49:39 -0500 Subject: [PATCH 8/9] odb: return temporary ODB source when set When invoked, `odb_set_temporary_primary_source()` installs a temporary object directory as the new primary ODB source. A caller that wants to operate on the ODB source of the open transaction must assume that it is the first entry in the ODB source list which is a bit awkward and fragile. Instead, return the newly installed source directly and report the previous primary source via a new `prev_source` out parameter. Propagate the installed source through `tmp_objdir_replace_primary_odb()` and start storing it in the "files" ODB transaction so a subsequent commit can easily access it without relying on the ODB source list ordering. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- object-file.c | 3 ++- odb.c | 9 +++++++-- odb.h | 6 ++++-- tmp-objdir.c | 8 +++++--- tmp-objdir.h | 6 ++++-- 5 files changed, 22 insertions(+), 10 deletions(-) diff --git a/object-file.c b/object-file.c index 4d03c167d5..db63587f6d 100644 --- a/object-file.c +++ b/object-file.c @@ -485,6 +485,7 @@ struct odb_transaction_files { struct odb_transaction base; struct tmp_objdir *objdir; + struct odb_source *quarantine; struct transaction_packfile packfile; const char *prefix; }; @@ -507,7 +508,7 @@ int odb_transaction_files_prepare(struct odb_transaction *base) if (!transaction->objdir) return error(_("unable to create temporary object directory")); - tmp_objdir_replace_primary_odb(transaction->objdir, 0); + transaction->quarantine = tmp_objdir_replace_primary_odb(transaction->objdir, 0); return 0; } diff --git a/odb.c b/odb.c index caf1d0f542..8afcb6b637 100644 --- a/odb.c +++ b/odb.c @@ -226,7 +226,8 @@ struct odb_source *odb_add_to_alternates_memory(struct object_database *odb, } struct odb_source *odb_set_temporary_primary_source(struct object_database *odb, - const char *dir, int will_destroy) + const char *dir, int will_destroy, + struct odb_source **prev_source) { struct odb_source *source; @@ -250,7 +251,11 @@ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb, source->will_destroy = will_destroy; source->next = odb->sources; odb->sources = source; - return source->next; + + if (prev_source) + *prev_source = source->next; + + return source; } void odb_restore_primary_source(struct object_database *odb, diff --git a/odb.h b/odb.h index fca67e8253..bdfcb9509a 100644 --- a/odb.h +++ b/odb.h @@ -199,10 +199,12 @@ struct odb_source *odb_find_source_or_die(struct object_database *odb, const cha /* * Replace the current writable object directory with the specified temporary - * object directory; returns the former primary source. + * object directory and return the newly installed primary source. The former + * primary source is reported via `prev_source` when non-NULL. */ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb, - const char *dir, int will_destroy); + const char *dir, int will_destroy, + struct odb_source **prev_source); /* * Restore the primary source that was previously replaced by diff --git a/tmp-objdir.c b/tmp-objdir.c index d199d39e7c..e633d97e0e 100644 --- a/tmp-objdir.c +++ b/tmp-objdir.c @@ -327,11 +327,13 @@ void tmp_objdir_add_as_alternate(const struct tmp_objdir *t) odb_add_to_alternates_memory(t->repo->objects, t->path.buf); } -void tmp_objdir_replace_primary_odb(struct tmp_objdir *t, int will_destroy) +struct odb_source *tmp_objdir_replace_primary_odb(struct tmp_objdir *t, + int will_destroy) { if (t->prev_source) BUG("the primary object database is already replaced"); - t->prev_source = odb_set_temporary_primary_source(t->repo->objects, - t->path.buf, will_destroy); t->will_destroy = will_destroy; + + return odb_set_temporary_primary_source(t->repo->objects, t->path.buf, + will_destroy, &t->prev_source); } diff --git a/tmp-objdir.h b/tmp-objdir.h index ccf800faa7..81eb927413 100644 --- a/tmp-objdir.h +++ b/tmp-objdir.h @@ -64,8 +64,10 @@ void tmp_objdir_add_as_alternate(const struct tmp_objdir *); /* * Replaces the writable object store in the current process with the temporary * object directory and makes the former main object store an alternate. - * If will_destroy is nonzero, the object directory may not be migrated. + * If will_destroy is nonzero, the object directory may not be migrated. Returns + * the newly installed primary source. */ -void tmp_objdir_replace_primary_odb(struct tmp_objdir *, int will_destroy); +struct odb_source *tmp_objdir_replace_primary_odb(struct tmp_objdir *, + int will_destroy); #endif /* TMP_OBJDIR_H */ From 2154d88f3aef51dda4cd75216dc5749bdafc8852 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Thu, 20 Aug 2026 18:49:40 -0500 Subject: [PATCH 9/9] odb/transaction: add transaction interface to write packfiles In git-receive-pack(1), the incoming packfile is written to the ODB via `unpack()`, which spawns git-index-pack(1) or git-unpack-objects(1) directly. With pluggable object databases, an alternative backend may need to handle writing packfile data differently though. Introduce `odb_transaction_write_pack()` as a generic interface to handle writing a packfile to a transaction and use the logic from `unpack()` as the "files" backend implementation. Note that when storing the objects as a packfile, git-index-pack(1) also writes a ".keep" lockfile next to it to prevent a concurrent repack from removing the new pack prior to reference updates being performed. The "files" transaction backend is responsible for managing these ".keep" files and removes them post-commit once the transaction is finalized. Call sites in git-receive-pack(1) are updated accordingly. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 160 +----------------------------------- object-file.c | 178 +++++++++++++++++++++++++++++++++++++++++ odb/transaction.c | 7 ++ odb/transaction.h | 62 ++++++++++++++ 4 files changed, 250 insertions(+), 157 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index b369466783..e6e54ba55f 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -15,7 +15,6 @@ #include "gpg-interface.h" #include "hex.h" #include "hook.h" -#include "lockfile.h" #include "object.h" #include "object-file.h" #include "object-name.h" @@ -23,7 +22,6 @@ #include "oid-array.h" #include "oidset.h" #include "pack.h" -#include "packfile.h" #include "parse-options.h" #include "pkt-line.h" #include "protocol.h" @@ -2292,162 +2290,11 @@ static void read_push_options(struct packet_reader *reader, } } -static const char *parse_pack_header(struct pack_header *hdr, int pack_fd) -{ - switch (read_pack_header(pack_fd, hdr)) { - case PH_ERROR_EOF: - return "eof before pack header was fully read"; - - case PH_ERROR_PACK_SIGNATURE: - return "protocol error (pack signature mismatch detected)"; - - case PH_ERROR_PROTOCOL: - return "protocol error (pack version unsupported)"; - - default: - return "unknown error in parse_pack_header"; - - case 0: - return NULL; - } -} - -static struct tempfile *pack_lockfile; - -static void push_header_arg(struct strvec *args, struct pack_header *hdr) -{ - strvec_pushf(args, "--pack_header=%"PRIu32",%"PRIu32, - ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries)); -} - -static unsigned int get_unpack_limit(struct repository *repo) -{ - unsigned int limit = 100; - - repo_config_get_uint(repo, "transfer.unpacklimit", &limit); - repo_config_get_uint(repo, "receive.unpacklimit", &limit); - - return limit; -} - -struct unpack_opts { - const char *fsck_msg_types; - const char *shallow_file; - off_t max_input_size; - int fsck_objects; - int reject_thin; - int err_fd; - int quiet; -}; - -static int unpack(struct odb_transaction *transaction, int pack_fd, - struct strbuf *err_msg, const struct unpack_opts *opts) -{ - struct pack_header hdr; - const char *hdr_err; - int status; - struct child_process child = CHILD_PROCESS_INIT; - int err_fd = opts->err_fd; - - hdr_err = parse_pack_header(&hdr, pack_fd); - if (hdr_err) { - if (err_fd > 0) - close(err_fd); - strbuf_addstr(err_msg, hdr_err); - return -1; - } - - if (opts->shallow_file) { - strvec_push(&child.args, "--shallow-file"); - strvec_push(&child.args, opts->shallow_file); - } - - odb_transaction_env(transaction, &child.env); - - if (ntohl(hdr.hdr_entries) < get_unpack_limit(the_repository)) { - strvec_push(&child.args, "unpack-objects"); - push_header_arg(&child.args, &hdr); - if (opts->quiet) - strvec_push(&child.args, "-q"); - if (opts->fsck_objects) - strvec_pushf(&child.args, "--strict%s", - opts->fsck_msg_types); - if (opts->max_input_size) - strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX, - (uintmax_t)opts->max_input_size); - child.no_stdout = 1; - child.in = pack_fd; - child.err = err_fd; - child.git_cmd = 1; - status = run_command(&child); - if (status) { - strbuf_addstr(err_msg, "unpack-objects abnormal exit"); - return -1; - } - } else { - char hostname[HOST_NAME_MAX + 1]; - char *lockfile; - - strvec_pushl(&child.args, "index-pack", "--stdin", NULL); - push_header_arg(&child.args, &hdr); - - if (xgethostname(hostname, sizeof(hostname))) - xsnprintf(hostname, sizeof(hostname), "localhost"); - strvec_pushf(&child.args, - "--keep=receive-pack %"PRIuMAX" on %s", - (uintmax_t)getpid(), - hostname); - - if (!opts->quiet && err_fd) - strvec_push(&child.args, "--show-resolving-progress"); - if (err_fd) - strvec_push(&child.args, "--report-end-of-input"); - if (opts->fsck_objects) - strvec_pushf(&child.args, "--strict%s", - opts->fsck_msg_types); - if (!opts->reject_thin) - strvec_push(&child.args, "--fix-thin"); - if (opts->max_input_size) - strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX, - (uintmax_t)opts->max_input_size); - child.out = -1; - child.in = pack_fd; - child.err = err_fd; - child.git_cmd = 1; - status = start_command(&child); - if (status) { - strbuf_addstr(err_msg, "index-pack fork failed"); - return -1; - } - - /* - * The lockfile filepath is expected to be the final location of - * the ".keep" file after being migrated to the main ODB source. - * This ensures the lockfile can be found and removed later - * after the ODB transaction has been committed. - */ - lockfile = index_pack_lockfile(transaction->source, child.out, NULL); - if (lockfile) { - pack_lockfile = register_tempfile(lockfile); - free(lockfile); - } - close(child.out); - - status = finish_command(&child); - if (status) { - strbuf_addstr(err_msg, "index-pack abnormal exit"); - return -1; - } - odb_reprepare(the_repository->objects); - } - return 0; -} - static int unpack_with_sideband(struct odb_transaction *transaction, const char *shallow_file, struct strbuf *err_msg) { - struct unpack_opts opts = { + struct odb_transaction_write_pack_opts opts = { .fsck_objects = (receive_fsck_objects >= 0 ? receive_fsck_objects : transfer_fsck_objects >= 0 @@ -2463,7 +2310,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction, int ret; if (!use_sideband) - return unpack(transaction, 0, err_msg, &opts); + return odb_transaction_write_pack(transaction, 0, err_msg, &opts); use_keepalive = KEEPALIVE_AFTER_NUL; memset(&muxer, 0, sizeof(muxer)); @@ -2473,7 +2320,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction, return 0; opts.err_fd = muxer.in; - ret = unpack(transaction, 0, err_msg, &opts); + ret = odb_transaction_write_pack(transaction, 0, err_msg, &opts); finish_async(&muxer); return ret; @@ -2748,7 +2595,6 @@ int cmd_receive_pack(int argc, execute_commands(commands, !!unpack_status.len, &si, transaction, &push_options); odb_transaction_finalize(transaction); - delete_tempfile(&pack_lockfile); sigchain_push(SIGPIPE, SIG_IGN); if (report_status_v2) report_v2(commands, &unpack_status); diff --git a/object-file.c b/object-file.c index db63587f6d..265c5f7a3c 100644 --- a/object-file.c +++ b/object-file.c @@ -10,6 +10,7 @@ #define USE_THE_REPOSITORY_VARIABLE #include "git-compat-util.h" +#include "config.h" #include "convert.h" #include "dir.h" #include "environment.h" @@ -26,6 +27,7 @@ #include "packfile.h" #include "path.h" #include "read-cache-ll.h" +#include "run-command.h" #include "setup.h" #include "strvec.h" #include "tempfile.h" @@ -483,11 +485,16 @@ 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; + + struct tempfile **pack_lockfiles; + size_t pack_lockfiles_nr; + size_t pack_lockfiles_alloc; }; int odb_transaction_files_prepare(struct odb_transaction *base) @@ -1291,6 +1298,174 @@ static int odb_transaction_files_commit(struct odb_transaction *base) return 0; } +static const char *parse_pack_header(struct pack_header *hdr, int pack_fd) +{ + switch (read_pack_header(pack_fd, hdr)) { + case PH_ERROR_EOF: + return "eof before pack header was fully read"; + + case PH_ERROR_PACK_SIGNATURE: + return "protocol error (pack signature mismatch detected)"; + + case PH_ERROR_PROTOCOL: + return "protocol error (pack version unsupported)"; + + default: + return "unknown error in parse_pack_header"; + + case 0: + return NULL; + } +} + +static void push_header_arg(struct strvec *args, struct pack_header *hdr) +{ + strvec_pushf(args, "--pack_header=%"PRIu32",%"PRIu32, + ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries)); +} + +static unsigned int get_unpack_limit(struct repository *repo, + enum odb_transaction_flags flags) +{ + unsigned int limit = 0; + + 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; +} + +static int odb_transaction_files_write_pack(struct odb_transaction *base, + int pack_fd, struct strbuf *err_msg, + const struct odb_transaction_write_pack_opts *opts) +{ + struct odb_transaction_files *transaction = + container_of(base, struct odb_transaction_files, base); + struct repository *repo = base->source->odb->repo; + struct child_process child = CHILD_PROCESS_INIT; + struct pack_header hdr; + const char *hdr_err; + int err_fd = opts->err_fd; + int status; + + hdr_err = parse_pack_header(&hdr, pack_fd); + if (hdr_err) { + if (err_fd > 0) + close(err_fd); + strbuf_addstr(err_msg, hdr_err); + return -1; + } + + if (opts->shallow_file) { + strvec_push(&child.args, "--shallow-file"); + strvec_push(&child.args, opts->shallow_file); + } + + odb_transaction_env(base, &child.env); + + 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) + strvec_push(&child.args, "-q"); + if (opts->fsck_objects) + strvec_pushf(&child.args, "--strict%s", + opts->fsck_msg_types); + if (opts->max_input_size) + strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX, + (uintmax_t)opts->max_input_size); + child.no_stdout = 1; + child.in = pack_fd; + child.err = err_fd; + child.git_cmd = 1; + status = run_command(&child); + if (status) { + strbuf_addstr(err_msg, "unpack-objects abnormal exit"); + return -1; + } + } else { + char hostname[HOST_NAME_MAX + 1]; + char *lockfile; + + strvec_pushl(&child.args, "index-pack", "--stdin", NULL); + push_header_arg(&child.args, &hdr); + + if (xgethostname(hostname, sizeof(hostname))) + xsnprintf(hostname, sizeof(hostname), "localhost"); + strvec_pushf(&child.args, + "--keep=receive-pack %"PRIuMAX" on %s", + (uintmax_t)getpid(), + hostname); + + if (!opts->quiet && err_fd) + strvec_push(&child.args, "--show-resolving-progress"); + if (err_fd) + strvec_push(&child.args, "--report-end-of-input"); + if (opts->fsck_objects) + strvec_pushf(&child.args, "--strict%s", + opts->fsck_msg_types); + if (!opts->reject_thin) + strvec_push(&child.args, "--fix-thin"); + if (opts->max_input_size) + strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX, + (uintmax_t)opts->max_input_size); + child.out = -1; + child.in = pack_fd; + child.err = err_fd; + child.git_cmd = 1; + status = start_command(&child); + if (status) { + strbuf_addstr(err_msg, "index-pack fork failed"); + return -1; + } + + /* + * The lockfile filepath is expected to be the final location of + * the ".keep" file after being migrated to the main ODB source. + * This ensures the lockfile can be found and removed later + * after the ODB transaction has been committed. + */ + lockfile = index_pack_lockfile(base->source, child.out, NULL); + if (lockfile) { + ALLOC_GROW(transaction->pack_lockfiles, + transaction->pack_lockfiles_nr + 1, + transaction->pack_lockfiles_alloc); + transaction->pack_lockfiles[transaction->pack_lockfiles_nr++] = + register_tempfile(lockfile); + free(lockfile); + } + close(child.out); + + status = finish_command(&child); + if (status) { + strbuf_addstr(err_msg, "index-pack abnormal exit"); + return -1; + } + + odb_source_prepare(transaction->quarantine, + ODB_PREPARE_FLUSH_CACHES); + } + + return 0; +} + +static int odb_transaction_files_finalize(struct odb_transaction *base) +{ + struct odb_transaction_files *transaction = + container_of(base, struct odb_transaction_files, base); + int ret = 0; + + for (size_t i = 0; i < transaction->pack_lockfiles_nr; i++) + ret |= delete_tempfile(&transaction->pack_lockfiles[i]); + + free(transaction->pack_lockfiles); + + return ret; +} + static int odb_transaction_files_env(struct odb_transaction *base, struct strvec *env) { @@ -1314,8 +1489,11 @@ int odb_transaction_files_begin(struct odb_source *source, transaction = xcalloc(1, sizeof(*transaction)); transaction->base.source = source; transaction->base.commit = odb_transaction_files_commit; + transaction->base.finalize = odb_transaction_files_finalize; 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) { diff --git a/odb/transaction.c b/odb/transaction.c index 9e9a982778..c9144e6cd6 100644 --- a/odb/transaction.c +++ b/odb/transaction.c @@ -59,6 +59,13 @@ int odb_transaction_write_object_stream(struct odb_transaction *transaction, return transaction->write_object_stream(transaction, stream, len, oid); } +int odb_transaction_write_pack(struct odb_transaction *transaction, int pack_fd, + struct strbuf *err_msg, + const struct odb_transaction_write_pack_opts *opts) +{ + return transaction->write_pack(transaction, pack_fd, err_msg, opts); +} + int odb_transaction_env(struct odb_transaction *transaction, struct strvec *env) { if (!transaction) diff --git a/odb/transaction.h b/odb/transaction.h index 6ed39b3d0e..8cb06c1191 100644 --- a/odb/transaction.h +++ b/odb/transaction.h @@ -4,6 +4,50 @@ #include "gettext.h" #include "odb.h" +/* + * Options controlling how odb_transaction_write_pack() ingests a packfile. + */ +struct odb_transaction_write_pack_opts { + /* + * Optional fsck severity configuration to apply when incoming objects + * are verified. + */ + const char *fsck_msg_types; + + /* + * Path to an alternative shallow file describing the shallow boundaries + * to honor while ingesting the pack. + */ + const char *shallow_file; + + /* + * The max size in bytes of the incoming packfile allowed. No limit is + * enforced when set to 0. + */ + off_t max_input_size; + + /* + * Whether the validity of incoming objects should be verified. + */ + int fsck_objects; + + /* + * Whether to reject an incoming packfile if it is "thin". + */ + int reject_thin; + + /* + * Optional file descriptor for reporting progress and errors. Set to 0 + * for none. + */ + int err_fd; + + /* + * Suppresses progress reporting. + */ + int quiet; +}; + /* * A transaction may be started for an object database prior to writing new * objects via odb_transaction_begin(). These objects are not committed until @@ -40,6 +84,15 @@ struct odb_transaction { int (*write_object_stream)(struct odb_transaction *transaction, struct odb_write_stream *stream, size_t len, struct object_id *oid); + /* + * This callback is expected to ingest the packfile readable via + * `pack_fd` into the transaction. Returns 0 on success, a negative + * error code otherwise. On failure, a human-readable description is + * appended to `err_msg`. + */ + int (*write_pack)(struct odb_transaction *transaction, int pack_fd, + struct strbuf *err_msg, + const struct odb_transaction_write_pack_opts *opts); /* * This callback is expected to populate the provided strvec with the @@ -107,6 +160,15 @@ int odb_transaction_write_object_stream(struct odb_transaction *transaction, struct odb_write_stream *stream, size_t len, struct object_id *oid); +/* + * Ingests the packfile readable via `pack_fd` into the transaction. Returns 0 + * on success, a negative error code otherwise. On failure, a human-readable + * description is appended to `err_msg`. + */ +int odb_transaction_write_pack(struct odb_transaction *transaction, int pack_fd, + struct strbuf *err_msg, + const struct odb_transaction_write_pack_opts *opts); + /* * Populates the provided strvec with the environment variables that a child * process should inherit so that its object writes participate in the