Git development
 help / color / mirror / Atom feed
From: Justin Tobler <jltobler@gmail.com>
To: git@vger.kernel.org
Cc: ps@pks.im, gitster@pobox.com, Justin Tobler <jltobler@gmail.com>
Subject: [PATCH v3 1/9] builtin/receive-pack: properly clean up keep files
Date: Tue, 11 Aug 2026 12:54:07 -0500	[thread overview]
Message-ID: <20260811175415.2044235-2-jltobler@gmail.com> (raw)
In-Reply-To: <20260811175415.2044235-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 to hold the
pack 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 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 | 14 ++++++++++++++
 5 files changed, 29 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..68dcf08cf3 100644
--- a/pack.h
+++ b/pack.h
@@ -3,6 +3,7 @@
 
 #include "object.h"
 #include "csum-file.h"
+#include "odb/source.h"
 
 struct packed_git;
 struct pack_window;
@@ -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..400a597606 100755
--- a/t/t5547-push-quarantine.sh
+++ b/t/t5547-push-quarantine.sh
@@ -70,4 +70,18 @@ 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 &&
+	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


  reply	other threads:[~2026-08-11 17:54 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-06 21:38 [PATCH 0/6] builtin/receive-pack: support pluggable packfile writes Justin Tobler
2026-08-06 21:38 ` [PATCH 1/6] odb/transaction: add transaction release interface Justin Tobler
2026-08-07  7:03   ` Patrick Steinhardt
2026-08-07 15:11     ` Justin Tobler
2026-08-06 21:38 ` [PATCH 2/6] builtin/receive-pack: pass shallow file explicitly Justin Tobler
2026-08-07  7:03   ` Patrick Steinhardt
2026-08-06 21:38 ` [PATCH 3/6] builtin/receive-pack: lift global state out of unpack() Justin Tobler
2026-08-07  7:03   ` Patrick Steinhardt
2026-08-07 15:33     ` Justin Tobler
2026-08-06 21:38 ` [PATCH 4/6] builtin/receive-pack: report unpack errors via strbuf Justin Tobler
2026-08-07  7:03   ` Patrick Steinhardt
2026-08-07 15:36     ` Justin Tobler
2026-08-09 19:00       ` Justin Tobler
2026-08-10  5:15         ` Patrick Steinhardt
2026-08-06 21:38 ` [PATCH 5/6] builtin/receive-pack: explicitly pass packfile fd Justin Tobler
2026-08-06 21:38 ` [PATCH 6/6] odb/transaction: add transaction interface to write packfiles Justin Tobler
2026-08-07  7:03   ` Patrick Steinhardt
2026-08-07 16:01     ` Justin Tobler
2026-08-09 19:00 ` [PATCH v2 0/7] builtin/receive-pack: support pluggable packfile writes Justin Tobler
2026-08-09 19:01   ` [PATCH v2 1/7] odb/transaction: add transaction finalize interface Justin Tobler
2026-08-10  3:38     ` Junio C Hamano
2026-08-10 19:10       ` Justin Tobler
2026-08-09 19:01   ` [PATCH v2 2/7] builtin/receive-pack: pass shallow file explicitly Justin Tobler
2026-08-09 19:01   ` [PATCH v2 3/7] builtin/receive-pack: read unpack limit config lazily Justin Tobler
2026-08-10  5:15     ` Patrick Steinhardt
2026-08-10 15:42       ` Justin Tobler
2026-08-10 17:54     ` Junio C Hamano
2026-08-10 19:16       ` Justin Tobler
2026-08-09 19:01   ` [PATCH v2 4/7] builtin/receive-pack: lift global state out of unpack() Justin Tobler
2026-08-09 19:01   ` [PATCH v2 5/7] builtin/receive-pack: report unpack errors via strbuf Justin Tobler
2026-08-09 19:01   ` [PATCH v2 6/7] builtin/receive-pack: explicitly pass packfile fd Justin Tobler
2026-08-09 19:01   ` [PATCH v2 7/7] odb/transaction: add transaction interface to write packfiles Justin Tobler
2026-08-10  1:54     ` Junio C Hamano
2026-08-10 19:29       ` Justin Tobler
2026-08-10  4:02     ` Junio C Hamano
2026-08-10 19:54       ` Justin Tobler
2026-08-11 17:54   ` [PATCH v3 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
2026-08-11 17:54     ` Justin Tobler [this message]
2026-08-11 17:54     ` [PATCH v3 2/9] odb/transaction: add transaction finalize interface Justin Tobler
2026-08-11 17:54     ` [PATCH v3 3/9] builtin/receive-pack: pass shallow file explicitly Justin Tobler
2026-08-11 17:54     ` [PATCH v3 4/9] builtin/receive-pack: read unpack limit config lazily Justin Tobler
2026-08-11 17:54     ` [PATCH v3 5/9] builtin/receive-pack: lift global state out of unpack() Justin Tobler
2026-08-11 17:54     ` [PATCH v3 6/9] builtin/receive-pack: report unpack errors via strbuf Justin Tobler
2026-08-11 17:54     ` [PATCH v3 7/9] builtin/receive-pack: explicitly pass packfile fd Justin Tobler
2026-08-11 17:54     ` [PATCH v3 8/9] odb: return temporary ODB source when set Justin Tobler
2026-08-11 17:54     ` [PATCH v3 9/9] odb/transaction: add transaction interface to write packfiles Justin Tobler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260811175415.2044235-2-jltobler@gmail.com \
    --to=jltobler@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox