All of lore.kernel.org
 help / color / mirror / Atom feed
From: Justin Tobler <jltobler@gmail.com>
To: git@vger.kernel.org
Cc: Justin Tobler <jltobler@gmail.com>
Subject: [PATCH 2/3] object-file: explicitly write ".keep" lockfile
Date: Thu, 15 Oct 2026 21:29:00 -0500	[thread overview]
Message-ID: <20261016022901.3838402-3-jltobler@gmail.com> (raw)
In-Reply-To: <20261016022901.3838402-1-jltobler@gmail.com>

The `odb_transaction_write_pack()` interface for the "files" backend
relies on git-index-pack(1) to write the specified packfile into an ODB
transaction as a packfile. To prevent a concurrent housekeeping
operation from deleting the newly written packfile prior to the
references being updated, a ".keep" lockfile is also generated by
git-index-pack(1) via the `--keep` option. Consequently, the ".keep"
lockfile is written into the ODB transaction alongside the packfile and
later migrated together to the primary ODB during transaction commit.

In a subsequent commit, writing the lockfile is deferred until
transaction commit and thus cannot rely on git-index-pack(1) to write
it. In preparation for this, update `odb_transaction_files_write_pack()`
to explicitly generate the same ".keep" lockfile itself.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c | 99 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 79 insertions(+), 20 deletions(-)

diff --git a/object-file.c b/object-file.c
index 210984f825..ebe46d35de 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1256,6 +1256,43 @@ int read_loose_object(struct repository *repo,
 	return ret;
 }
 
+static int write_pack_lock(struct odb_source *source, const char *pack_name,
+			   const char *keep_msg)
+{
+	struct strbuf lock_path = STRBUF_INIT;
+	struct strbuf contents = STRBUF_INIT;
+	int ret = -1;
+	int fd;
+
+	strbuf_addf(&lock_path, "%s/pack/pack-%s.keep",
+		    source->path, pack_name);
+	strbuf_addf(&contents, "%s\n", keep_msg);
+
+	fd = safe_create_file_with_leading_directories(source->odb->repo,
+						       lock_path.buf);
+	if (fd < 0)
+		goto out;
+
+	if (write_in_full(fd, contents.buf, contents.len) < 0) {
+		close(fd);
+		unlink(lock_path.buf);
+		goto out;
+	}
+
+	if (close(fd)) {
+		unlink(lock_path.buf);
+		goto out;
+	}
+
+	ret = 0;
+
+out:
+	strbuf_release(&lock_path);
+	strbuf_release(&contents);
+
+	return ret;
+}
+
 static int odb_transaction_files_commit(struct odb_transaction *base)
 {
 	struct odb_transaction_files *transaction =
@@ -1389,18 +1426,17 @@ static int odb_transaction_files_write_pack(struct odb_transaction *base,
 			return -1;
 		}
 	} else {
+		struct strbuf final_path = STRBUF_INIT;
+		struct strbuf pack_name = STRBUF_INIT;
+		struct strbuf keep_msg = STRBUF_INIT;
 		char hostname[HOST_NAME_MAX + 1];
-		char *lockfile;
+		int ret;
 
 		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");
@@ -1424,31 +1460,54 @@ static int odb_transaction_files_write_pack(struct odb_transaction *base,
 			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);
-		}
+		ret = read_index_pack_report(repo, child.out, &pack_name, NULL);
+
 		close(child.out);
 
 		status = finish_command(&child);
 		if (status) {
+			strbuf_release(&pack_name);
 			strbuf_addstr(err_msg, "index-pack abnormal exit");
 			return -1;
 		}
 
+		if (ret) {
+			strbuf_release(&pack_name);
+			strbuf_addstr(err_msg, "index-pack failed to report pack name");
+			return -1;
+		}
+
+		strbuf_addf(&keep_msg, "receive-pack %"PRIuMAX" on %s",
+			    (uintmax_t)getpid(), hostname);
+
+		if (write_pack_lock(transaction->quarantine, pack_name.buf, keep_msg.buf)) {
+			strbuf_release(&pack_name);
+			strbuf_release(&keep_msg);
+			strbuf_addstr(err_msg, "lockfile write failed");
+			return -1;
+		}
+
+		ALLOC_GROW(transaction->pack_lockfiles,
+			   transaction->pack_lockfiles_nr + 1,
+			   transaction->pack_lockfiles_alloc);
+
+		/*
+		 * 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.
+		 */
+		strbuf_addf(&final_path, "%s/pack/pack-%s.keep",
+			    base->source->path, pack_name.buf);
+		transaction->pack_lockfiles[transaction->pack_lockfiles_nr++] =
+			register_tempfile(final_path.buf);
+
 		odb_source_prepare(transaction->quarantine,
 				   ODB_PREPARE_FLUSH_CACHES);
+
+		strbuf_release(&final_path);
+		strbuf_release(&pack_name);
+		strbuf_release(&keep_msg);
 	}
 
 	return 0;
-- 
2.55.0.424.g13c7afec21


  parent reply	other threads:[~2026-10-16  2:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-16  2:28 [PATCH 0/3] object-file: fix ".keep" lockfile ownership in ODB transactions Justin Tobler
2026-10-16  2:28 ` [PATCH 1/3] pack-write: lift and expose index-pack report parsing Justin Tobler
2026-10-16  6:58   ` Patrick Steinhardt
2026-10-16 17:30     ` Justin Tobler
2026-10-16  2:29 ` Justin Tobler [this message]
2026-10-16  6:58   ` [PATCH 2/3] object-file: explicitly write ".keep" lockfile Patrick Steinhardt
2026-10-16 18:37     ` Justin Tobler
2026-10-16  2:29 ` [PATCH 3/3] object-file: write ".keep" lockfiles during transaction commit Justin Tobler
2026-10-16  6:58   ` Patrick Steinhardt
2026-10-16 19:22     ` 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=20261016022901.3838402-3-jltobler@gmail.com \
    --to=jltobler@gmail.com \
    --cc=git@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.