On Mon, Jun 29, 2026 at 03:25:18PM -0500, Justin Tobler wrote:

On 26/06/24 01:26PM, Patrick Steinhardt wrote:

quoted

On Tue, Jun 23, 2026 at 11:19:20PM -0500, Justin Tobler wrote:

quoted

Objects received by git-receive-pack(1) are quarantined in a temporary
"incoming" directory and migrated into the object database prior to the
reference updates. The quarantine is currently managed through
`tmp_objdir` directly. In a pluggable ODB future, how exactly an object
gets written to a transaction may vary for a given ODB source. Refactor
git-receive-pack(1) to use the ODB transaction interfaces to manage the
object staging area in a more agnostic manner accordingly.

Note that the temporary directory created for git-receive-pack(1) is
eagerly created and uses a different prefix name. This behavior is

A different prefix name compared to what?

Currently the temporary directories created for ODB transactions all use
the prefix "bulk-fsync". The temp dir created by git-receive-pack(1) is
expected to have the prefix "incoming".

quoted

quoted

special cased in the "files" backend by having `odb_transaction_begin()`
callers that require this behavior provide an `ODB_TRANSACTION_RECEIVE`
flag.

Okay. I guess this is to retain existing behaviour where the temporary
directory is created lazily everywhere else. Makes me wonder whether we
should eventually change this to just unconditionally create the
directory in all cases so that we can drop this new flag.

It would be nice to not have to have a flag here, but if we want to also
keep the existing temp dir prefixes, we would also need to keep the
flags.

Fair. Makes me wonder whether we really need to keep the exact same
naming for this temporary directory. This is so deep into internals that
I'm not sure whether we really need to treat this as part of our
interface. I'm rather inclined to say it's not necessary.

In any case, I think it's fine to defer that discussion and keep this
as-is for now. But we might keep it in the back of our minds and maybe
simplify this in a subsequent patch series.

quoted

quoted

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 19eb6a1b61..ee8e03e2ab 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c

@@ -2326,7 +2323,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)
+static const char *unpack(int err_fd, struct shallow_info *si,
+			  struct odb_transaction *transaction)
 {
 	struct pack_header hdr;
 	const char *hdr_err;

It feels a bit weird that we sometimes pass the transaction as
parameter, whereas othertimes we access it via `the_repository`.

That's fair. I was trying to avoid the churn of wiring to all its
callsites, but it's probably best to be consistent. Maybe it would be
fine to just create a transaction global like we do for the reference
transaction?

My first kneejerk reaction was "no", but then I noticed that the global
variable you're talking about is local to "builtin/receive-pack.c". So
that might be an okayish solution.

Thanks!

Patrick