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 <jltobler@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
main
Justin Tobler 2026-08-20 18:49:32 -05:00 committed by Junio C Hamano
parent 11c6700f10
commit 7874b2c7e1
5 changed files with 46 additions and 6 deletions

View File

@ -2412,7 +2412,13 @@ static const char *unpack(int err_fd, struct shallow_info *si,
if (status) if (status)
return "index-pack fork failed"; 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) { if (lockfile) {
pack_lockfile = register_tempfile(lockfile); pack_lockfile = register_tempfile(lockfile);
free(lockfile); free(lockfile);

View File

@ -1075,7 +1075,7 @@ static int get_pack(struct fetch_pack_args *args,
die(_("fetch-pack: unable to fork off %s"), cmd_name); die(_("fetch-pack: unable to fork off %s"), cmd_name);
if (do_keep && (pack_lockfiles || fsck_objects)) { if (do_keep && (pack_lockfiles || fsck_objects)) {
int is_well_formed; int is_well_formed;
char *pack_lockfile = index_pack_lockfile(the_repository, char *pack_lockfile = index_pack_lockfile(the_repository->objects->sources,
cmd.out, cmd.out,
&is_well_formed); &is_well_formed);



View File

@ -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); 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]; 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 * 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; packname[len-1] = 0;
if (skip_prefix(packname, "keep\t", &name)) if (skip_prefix(packname, "keep\t", &name))
return xstrfmt("%s/pack/pack-%s.keep", return xstrfmt("%s/pack/pack-%s.keep",
repo_get_object_directory(r), name); source->path, name);
return NULL; return NULL;
} }
if (is_well_formed) if (is_well_formed)

4
pack.h
View File

@ -7,6 +7,7 @@
struct packed_git; struct packed_git;
struct pack_window; struct pack_window;
struct repository; struct repository;
struct odb_source;


/* /*
* Packed object header * 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, void fixup_pack_header_footer(const struct git_hash_algo *, int,
unsigned char *, const char *, uint32_t, unsigned char *, const char *, uint32_t,
unsigned char *, off_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; struct ref;



View File

@ -70,4 +70,35 @@ test_expect_success 'updating a ref from quarantine is forbidden' '
git -C update.git fsck 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 test_done