[PATCH v4 1/9] builtin/receive-pack: properly clean up keep files
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Aug 19, 2026, 21:53 UTC
- Message-ID
- <20260819215311.3880274-2-jltobler@gmail.com>
- In-Reply-To
- <20260819215311.3880274-1-jltobler@gmail.com>
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 <jltobler@gmail.com> --- builtin/receive-pack.c | 8 +++++++- fetch-pack.c | 2 +- pack-write.c | 7 ++++--- pack.h | 4 +++- t/t5547-push-quarantine.sh | 22 ++++++++++++++++++++++ 5 files changed, 37 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..3da253cc1a 100755 --- a/t/t5547-push-quarantine.sh +++ b/t/t5547-push-quarantine.sh @@ -70,4 +70,26 @@ 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 does indeed exist. + test_hook -C keep.git pre-receive <<-\EOF && + keep="$(ls "$GIT_QUARANTINE_PATH"/pack/pack-*.keep)" && + test -f "$keep" + EOF + + test_commit foo && + git push keep.git HEAD && + pack="$(ls keep.git/objects/pack/pack-*.pack)" && + keep="${pack%.pack}.keep" && + + test_path_is_file "$pack" && + test_path_is_missing "$keep" +' + test_done
-- 2.55.0.424.g13c7afec21