[PATCH 0/6] receive-pack: use ODB transactions to stage object writes

[PATCH 0/6] receive-pack: use ODB transactions to stage object writes

[Justin Tobler]

Greetings,

This patch series replaces direct usage of the `tmp_objdir` interfaces
in git-receive-pack(1) to instead use the `odb_transaction` interfaces
to create/manage a staging area to write objects to. The purpose of this
change is to get git-receive-pack(1) one step closer to being ODB
backend agnostic. For now, the object writes themselves are still
"files" backend specific due to being handled by the git-index-pack(1)
and git-unpack-objects(1) child processes. This will be tackled in a
separate series though.

Thanks,
-Justin

Justin Tobler (6):
  object-file: rename files transaction prepare function
  object-file: propagate files transaction errors
  odb/transaction: propagate begin errors
  odb/transaction: propagate commit errors
  odb/transaction: add transaction env interface
  builtin/receive-pack: stage incoming objects via ODB transactions

 builtin/add.c            |  2 +-
 builtin/receive-pack.c   | 46 ++++++++++--------------
 builtin/unpack-objects.c |  2 +-
 builtin/update-index.c   |  2 +-
 cache-tree.c             |  2 +-
 object-file.c            | 77 +++++++++++++++++++++++++++++++---------
 object-file.h            |  7 ++--
 odb/source-files.c       |  9 ++---
 odb/source-inmemory.c    |  3 +-
 odb/source-loose.c       |  3 +-
 odb/source.h             |  9 +++--
 odb/transaction.c        | 38 +++++++++++++++-----
 odb/transaction.h        | 49 +++++++++++++++++++++----
 read-cache.c             |  2 +-
 14 files changed, 173 insertions(+), 78 deletions(-)


base-commit: ab776a62a78576513ee121424adb19597fbb7613
-- 
2.54.0.105.g59ff4886a5

[PATCH 1/6] object-file: rename files transaction prepare function

[Justin Tobler]

The "files" ODB transaction backend lazily creates a temporary object
directory when the first loose object is written to the transaction via
`prepare_loose_object_transaction()`. In a subsequent commit, the
temporary directory is used to also write packfiles to.

Rename the function to `odb_transaction_files_prepare()` accordingly.

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

diff --git a/object-file.c b/object-file.c
index e3d92bbda2..a3eb8d71dd 100644
--- a/object-file.c
+++ b/object-file.c
@@ -499,7 +499,7 @@ struct odb_transaction_files {
 	struct transaction_packfile packfile;
 };
 
-static void prepare_loose_object_transaction(struct odb_transaction *base)
+static void odb_transaction_files_prepare(struct odb_transaction *base)
 {
 	struct odb_transaction_files *transaction =
 		container_of_or_null(base, struct odb_transaction_files, base);
@@ -761,7 +761,7 @@ int write_loose_object(struct odb_source_loose *loose,
 	static struct strbuf filename = STRBUF_INIT;
 
 	if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
-		prepare_loose_object_transaction(loose->base.odb->transaction);
+		odb_transaction_files_prepare(loose->base.odb->transaction);
 
 	odb_loose_path(loose, &filename, oid);
 
@@ -825,7 +825,7 @@ int odb_source_loose_write_stream(struct odb_source_loose *loose,
 	int hdrlen;
 
 	if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
-		prepare_loose_object_transaction(loose->base.odb->transaction);
+		odb_transaction_files_prepare(loose->base.odb->transaction);
 
 	/* Since oid is not determined, save tmp file to odb path. */
 	strbuf_addf(&filename, "%s/", loose->base.path);
-- 
2.54.0.105.g59ff4886a5

[PATCH 2/6] object-file: propagate files transaction errors

[Justin Tobler]

The "files" transaction backend may encounter errors related to managing
the temporary directory used to stage objects, but silently ignores
these errors. Instead return errors encountered in the
`odb_transaction_files_{prepare,begin,commit}()` interfaces to allow
callers to handle as needed.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c      | 41 ++++++++++++++++++++++++++++-------------
 object-file.h      |  5 +++--
 odb/source-files.c |  6 +-----
 odb/transaction.h  |  2 +-
 4 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/object-file.c b/object-file.c
index a3eb8d71dd..18c2df75fb 100644
--- a/object-file.c
+++ b/object-file.c
@@ -499,7 +499,7 @@ struct odb_transaction_files {
 	struct transaction_packfile packfile;
 };
 
-static void odb_transaction_files_prepare(struct odb_transaction *base)
+static int odb_transaction_files_prepare(struct odb_transaction *base)
 {
 	struct odb_transaction_files *transaction =
 		container_of_or_null(base, struct odb_transaction_files, base);
@@ -511,11 +511,15 @@ static void odb_transaction_files_prepare(struct odb_transaction *base)
 	 * added at the time they call odb_transaction_files_begin.
 	 */
 	if (!transaction || transaction->objdir)
-		return;
+		return 0;
 
 	transaction->objdir = tmp_objdir_create(base->source->odb->repo, "bulk-fsync");
-	if (transaction->objdir)
-		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
+	if (!transaction->objdir)
+		return -1;
+
+	tmp_objdir_replace_primary_odb(transaction->objdir, 0);
+
+	return 0;
 }
 
 static void fsync_loose_object_transaction(struct odb_transaction *base,
@@ -542,13 +546,13 @@ static void fsync_loose_object_transaction(struct odb_transaction *base,
 /*
  * Cleanup after batch-mode fsync_object_files.
  */
-static void flush_loose_object_transaction(struct odb_transaction_files *transaction)
+static int flush_loose_object_transaction(struct odb_transaction_files *transaction)
 {
 	struct strbuf temp_path = STRBUF_INIT;
 	struct tempfile *temp;
 
 	if (!transaction->objdir)
-		return;
+		return 0;
 
 	/*
 	 * Issue a full hardware flush against a temporary file to ensure
@@ -570,8 +574,12 @@ static void flush_loose_object_transaction(struct odb_transaction_files *transac
 	 * Make the object files visible in the primary ODB after their data is
 	 * fully durable.
 	 */
-	tmp_objdir_migrate(transaction->objdir);
+	if (tmp_objdir_migrate(transaction->objdir))
+		return -1;
+
 	transaction->objdir = NULL;
+
+	return 0;
 }
 
 /* Finalize a file on disk, and close it. */
@@ -1670,27 +1678,34 @@ int read_loose_object(struct repository *repo,
 	return ret;
 }
 
-static void odb_transaction_files_commit(struct odb_transaction *base)
+static int odb_transaction_files_commit(struct odb_transaction *base)
 {
 	struct odb_transaction_files *transaction =
 		container_of(base, struct odb_transaction_files, base);
 
-	flush_loose_object_transaction(transaction);
+	if (flush_loose_object_transaction(transaction))
+		return -1;
 	flush_packfile_transaction(transaction);
+
+	return 0;
 }
 
-struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
+int odb_transaction_files_begin(struct odb_source *source,
+				struct odb_transaction **out)
 {
 	struct odb_transaction_files *transaction;
 	struct object_database *odb = source->odb;
 
-	if (odb->transaction)
-		return NULL;
+	if (odb->transaction) {
+		*out = NULL;
+		return 0;
+	}
 
 	transaction = xcalloc(1, sizeof(*transaction));
 	transaction->base.source = source;
 	transaction->base.commit = odb_transaction_files_commit;
 	transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
+	*out = &transaction->base;
 
-	return &transaction->base;
+	return 0;
 }
diff --git a/object-file.h b/object-file.h
index 528c4e6e69..ac927fec07 100644
--- a/object-file.h
+++ b/object-file.h
@@ -195,8 +195,9 @@ struct odb_transaction;
  * Tell the object database to optimize for adding
  * multiple objects. odb_transaction_files_commit must be called
  * to make new objects visible. If a transaction is already
- * pending, NULL is returned.
+ * pending, out is set to NULL.
  */
-struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);
+int odb_transaction_files_begin(struct odb_source *source,
+				struct odb_transaction **out);
 
 #endif /* OBJECT_FILE_H */
diff --git a/odb/source-files.c b/odb/source-files.c
index 5bdd042922..2545bd81d4 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -182,11 +182,7 @@ static int odb_source_files_write_object_stream(struct odb_source *source,
 static int odb_source_files_begin_transaction(struct odb_source *source,
 					      struct odb_transaction **out)
 {
-	struct odb_transaction *tx = odb_transaction_files_begin(source);
-	if (!tx)
-		return -1;
-	*out = tx;
-	return 0;
+	return odb_transaction_files_begin(source, out);
 }
 
 static int odb_source_files_read_alternates(struct odb_source *source,
diff --git a/odb/transaction.h b/odb/transaction.h
index 854fda06f5..f4c1ebfaaa 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -17,7 +17,7 @@ struct odb_transaction {
 	struct odb_source *source;
 
 	/* The ODB source specific callback invoked to commit a transaction. */
-	void (*commit)(struct odb_transaction *transaction);
+	int (*commit)(struct odb_transaction *transaction);
 
 	/*
 	 * This callback is expected to write the given object stream into
-- 
2.54.0.105.g59ff4886a5

[PATCH 3/6] odb/transaction: propagate begin errors

[Justin Tobler]

When `odb_transaction_begin()` is invoked, the function returns the
transaction pointer directly. There is no way for the backend to
signal that it failed to set up its state, such as when creating the
temporary object directory backing the transaction.

In a subsequent commit, git-receive-pack(1) starts using ODB
transactions and needs to be able to report such failures rather
than silently ignore them. Refactor `odb_transaction_begin()` to
return an int error code and write the resulting transaction into an
out parameter. Also introduce `odb_transaction_begin_or_die()` as a
convenience for callsites that do not need to handle errors
explicitly.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 builtin/add.c            |  2 +-
 builtin/unpack-objects.c |  2 +-
 builtin/update-index.c   |  2 +-
 cache-tree.c             |  2 +-
 object-file.c            |  3 ++-
 odb/transaction.c        | 16 +++++++++++-----
 odb/transaction.h        | 19 +++++++++++++++----
 read-cache.c             |  2 +-
 8 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index c859f66519..3d5d9cfdb9 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -581,7 +581,7 @@ int cmd_add(int argc,
 		string_list_clear(&only_match_skip_worktree, 0);
 	}
 
-	transaction = odb_transaction_begin(repo->objects);
+	odb_transaction_begin_or_die(repo->objects, &transaction);
 
 	ps_matched = xcalloc(pathspec.nr, 1);
 	if (add_renormalize)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index f3849bb654..d0136cdd99 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -598,7 +598,7 @@ static void unpack_all(void)
 		progress = start_progress(the_repository,
 					  _("Unpacking objects"), nr_objects);
 	CALLOC_ARRAY(obj_list, nr_objects);
-	transaction = odb_transaction_begin(the_repository->objects);
+	odb_transaction_begin_or_die(the_repository->objects, &transaction);
 	for (i = 0; i < nr_objects; i++) {
 		unpack_one(i);
 		display_progress(progress, i + 1);
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 3d6646c318..17f3ea284c 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1124,7 +1124,7 @@ int cmd_update_index(int argc,
 	 * Allow the object layer to optimize adding multiple objects in
 	 * a batch.
 	 */
-	transaction = odb_transaction_begin(the_repository->objects);
+	odb_transaction_begin_or_die(the_repository->objects, &transaction);
 	while (ctx.argc) {
 		if (parseopt_state != PARSE_OPT_DONE)
 			parseopt_state = parse_options_step(&ctx, options,
diff --git a/cache-tree.c b/cache-tree.c
index 184f7e2635..1a7dfed9cf 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -490,7 +490,7 @@ int cache_tree_update(struct index_state *istate, int flags)
 
 	trace_performance_enter();
 	trace2_region_enter("cache_tree", "update", istate->repo);
-	transaction = odb_transaction_begin(the_repository->objects);
+	odb_transaction_begin_or_die(the_repository->objects, &transaction);
 	i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
 		       "", 0, &skip, flags);
 	odb_transaction_commit(transaction);
diff --git a/object-file.c b/object-file.c
index 18c2df75fb..696f05dc2d 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1389,8 +1389,9 @@ int index_fd(struct index_state *istate, struct object_id *oid,
 
 		if (flags & INDEX_WRITE_OBJECT) {
 			struct object_database *odb = the_repository->objects;
-			struct odb_transaction *transaction = odb_transaction_begin(odb);
+			struct odb_transaction *transaction;
 
+			odb_transaction_begin_or_die(odb, &transaction);
 			ret = odb_transaction_write_object_stream(odb->transaction,
 								  &stream,
 								  xsize_t(st->st_size),
diff --git a/odb/transaction.c b/odb/transaction.c
index b16e07aebf..d3de01db50 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -2,14 +2,20 @@
 #include "odb/source.h"
 #include "odb/transaction.h"
 
-struct odb_transaction *odb_transaction_begin(struct object_database *odb)
+int odb_transaction_begin(struct object_database *odb,
+			  struct odb_transaction **out)
 {
-	if (odb->transaction)
-		return NULL;
+	int ret;
 
-	odb_source_begin_transaction(odb->sources, &odb->transaction);
+	if (odb->transaction) {
+		*out = NULL;
+		return 0;
+	}
 
-	return odb->transaction;
+	ret = odb_source_begin_transaction(odb->sources, out);
+	odb->transaction = *out;
+
+	return ret;
 }
 
 void odb_transaction_commit(struct odb_transaction *transaction)
diff --git a/odb/transaction.h b/odb/transaction.h
index f4c1ebfaaa..cd6d50f2e5 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -1,6 +1,8 @@
 #ifndef ODB_TRANSACTION_H
 #define ODB_TRANSACTION_H
 
+#include "git-compat-util.h"
+#include "gettext.h"
 #include "odb.h"
 #include "odb/source.h"
 
@@ -33,11 +35,20 @@ struct odb_transaction {
 };
 
 /*
- * Starts an ODB transaction. Subsequent objects are written to the transaction
- * and not committed until odb_transaction_commit() is invoked on the
- * transaction. If the ODB already has a pending transaction, NULL is returned.
+ * Starts an ODB transaction and returns it via `out`. Subsequent objects are
+ * written to the transaction and not committed until odb_transaction_commit()
+ * is invoked on the transaction. Returns 0 on success and a negative value on
+ * error. If the ODB already has a pending transaction, `out` is set to NULL.
  */
-struct odb_transaction *odb_transaction_begin(struct object_database *odb);
+int odb_transaction_begin(struct object_database *odb,
+			  struct odb_transaction **out);
+
+static inline void odb_transaction_begin_or_die(struct object_database *odb,
+						struct odb_transaction **out)
+{
+	if (odb_transaction_begin(odb, out))
+		die(_("failed to start ODB transaction"));
+}
 
 /*
  * Commits an ODB transaction making the written objects visible. If the
diff --git a/read-cache.c b/read-cache.c
index 21ca58beea..db0bfa60fe 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -4042,7 +4042,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
 	 * This function is invoked from commands other than 'add', which
 	 * may not have their own transaction active.
 	 */
-	transaction = odb_transaction_begin(repo->objects);
+	odb_transaction_begin_or_die(repo->objects, &transaction);
 	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
 	odb_transaction_commit(transaction);
 
-- 
2.54.0.105.g59ff4886a5

[PATCH 4/6] odb/transaction: propagate commit errors

[Justin Tobler]

When `odb_transaction_commit()` is invoked, the return value of the
backend commit callback is silently discarded. A backend has no way
to signal that committing failed, such as when the "files" backend
cannot migrate its temporary object directory into the permanent
ODB.

In a subsequent commit, git-receive-pack(1) starts using ODB transaction
to stage objects and consequently cares about such failures so it can
handle the error appropriately. Change the commit callback signature to
return an int error code and have `odb_transaction_commit()` forward it
accordingly.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 odb/transaction.c | 13 ++++++++++---
 odb/transaction.h |  2 +-
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/odb/transaction.c b/odb/transaction.c
index d3de01db50..b20d6a16f8 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -18,19 +18,26 @@ int odb_transaction_begin(struct object_database *odb,
 	return ret;
 }
 
-void odb_transaction_commit(struct odb_transaction *transaction)
+int odb_transaction_commit(struct odb_transaction *transaction)
 {
+	int ret;
+
 	if (!transaction)
-		return;
+		return 0;
 
 	/*
 	 * Ensure the transaction ending matches the pending transaction.
 	 */
 	ASSERT(transaction == transaction->source->odb->transaction);
 
-	transaction->commit(transaction);
+	ret = transaction->commit(transaction);
+	if (ret)
+		return ret;
+
 	transaction->source->odb->transaction = NULL;
 	free(transaction);
+
+	return 0;
 }
 
 int odb_transaction_write_object_stream(struct odb_transaction *transaction,
diff --git a/odb/transaction.h b/odb/transaction.h
index cd6d50f2e5..7898770071 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -54,7 +54,7 @@ static inline void odb_transaction_begin_or_die(struct object_database *odb,
  * Commits an ODB transaction making the written objects visible. If the
  * specified transaction is NULL, the function is a no-op.
  */
-void odb_transaction_commit(struct odb_transaction *transaction);
+int odb_transaction_commit(struct odb_transaction *transaction);
 
 /*
  * Writes the object in the provided stream into the transaction. The resulting
-- 
2.54.0.105.g59ff4886a5

[PATCH 5/6] odb/transaction: add transaction env interface

[Justin Tobler]

The ODB transaction backend is responsible for creating/managing its own
staging area for writing objects. Other child processes spawned by Git
may need to access to uncommitted objects or write new objects in the
staging area though.

Introduce `odb_transaction_env()` which is expected to provide the set
of environment variables needed by a child process to access the
transaction staging area.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c     | 11 +++++++++++
 odb/transaction.c |  8 ++++++++
 odb/transaction.h | 19 +++++++++++++++++++
 3 files changed, 38 insertions(+)

diff --git a/object-file.c b/object-file.c
index 696f05dc2d..14064d188a 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1691,6 +1691,16 @@ static int odb_transaction_files_commit(struct odb_transaction *base)
 	return 0;
 }
 
+static const char **odb_transaction_files_env(struct odb_transaction *base)
+{
+	struct odb_transaction_files *transaction =
+		container_of(base, struct odb_transaction_files, base);
+
+	odb_transaction_files_prepare(&transaction->base);
+
+	return tmp_objdir_env(transaction->objdir);
+}
+
 int odb_transaction_files_begin(struct odb_source *source,
 				struct odb_transaction **out)
 {
@@ -1706,6 +1716,7 @@ int odb_transaction_files_begin(struct odb_source *source,
 	transaction->base.source = source;
 	transaction->base.commit = odb_transaction_files_commit;
 	transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
+	transaction->base.env = odb_transaction_files_env;
 	*out = &transaction->base;
 
 	return 0;
diff --git a/odb/transaction.c b/odb/transaction.c
index b20d6a16f8..20d3f43f54 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -46,3 +46,11 @@ int odb_transaction_write_object_stream(struct odb_transaction *transaction,
 {
 	return transaction->write_object_stream(transaction, stream, len, oid);
 }
+
+const char **odb_transaction_env(struct odb_transaction *transaction)
+{
+	if (!transaction)
+		return NULL;
+
+	return transaction->env(transaction);
+}
diff --git a/odb/transaction.h b/odb/transaction.h
index 7898770071..536458297b 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -32,6 +32,16 @@ struct odb_transaction {
 	int (*write_object_stream)(struct odb_transaction *transaction,
 				   struct odb_write_stream *stream, size_t len,
 				   struct object_id *oid);
+
+	/*
+	 * This callback is expected to return a NULL-terminated array of
+	 * environment variables that a child process should inherit so
+	 * that its object writes participate in the transaction. The
+	 * returned array is owned by the backend and remains valid until
+	 * the transaction ends. May return NULL when the backend does not
+	 * need to expose any state to child processes.
+	 */
+	const char **(*env)(struct odb_transaction *transaction);
 };
 
 /*
@@ -65,4 +75,13 @@ int odb_transaction_write_object_stream(struct odb_transaction *transaction,
 					struct odb_write_stream *stream,
 					size_t len, struct object_id *oid);
 
+/*
+ * Returns a NULL-terminated array of environment variables that a child
+ * process should inherit so that its object writes participate in the
+ * transaction, suitable for passing via child_process.env. Returns NULL if
+ * the transaction is NULL or the backend does not expose any state to child
+ * processes.
+ */
+const char **odb_transaction_env(struct odb_transaction *transaction);
+
 #endif
-- 
2.54.0.105.g59ff4886a5

[PATCH 6/6] builtin/receive-pack: stage incoming objects via ODB transactions

[Justin Tobler]

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
special cased in the "files" backend by having `odb_transaction_begin()`
callers that require this behavior provide an `ODB_TRANSACTION_RECEIVE`
flag.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 builtin/add.c            |  2 +-
 builtin/receive-pack.c   | 46 ++++++++++++++++------------------------
 builtin/unpack-objects.c |  2 +-
 builtin/update-index.c   |  2 +-
 cache-tree.c             |  2 +-
 object-file.c            | 22 ++++++++++++++++---
 object-file.h            |  4 +++-
 odb/source-files.c       |  5 +++--
 odb/source-inmemory.c    |  3 ++-
 odb/source-loose.c       |  3 ++-
 odb/source.h             |  9 +++++---
 odb/transaction.c        |  5 +++--
 odb/transaction.h        | 13 ++++++++----
 read-cache.c             |  2 +-
 14 files changed, 70 insertions(+), 50 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 3d5d9cfdb9..60ffbede2b 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -581,7 +581,7 @@ int cmd_add(int argc,
 		string_list_clear(&only_match_skip_worktree, 0);
 	}
 
-	odb_transaction_begin_or_die(repo->objects, &transaction);
+	odb_transaction_begin_or_die(repo->objects, &transaction, 0);
 
 	ps_matched = xcalloc(pathspec.nr, 1);
 	if (add_renormalize)
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
@@ -112,8 +112,6 @@ static enum {
 } use_keepalive;
 static int keepalive_in_sec = 5;
 
-static struct tmp_objdir *tmp_objdir;
-
 static struct proc_receive_ref {
 	unsigned int want_add:1,
 		     want_delete:1,
@@ -959,8 +957,8 @@ static int run_receive_hook(struct command *commands,
 		strvec_push(&opt.env, "GIT_PUSH_OPTION_COUNT");
 	}
 
-	if (tmp_objdir)
-		strvec_pushv(&opt.env, tmp_objdir_env(tmp_objdir));
+	if (the_repository->objects->transaction)
+		strvec_pushv(&opt.env, odb_transaction_env(the_repository->objects->transaction));
 
 	prepare_push_cert_sha1(&opt);
 
@@ -1363,7 +1361,7 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 		    !delayed_reachability_test(si, i))
 			oid_array_append(&extra, &si->shallow->oid[i]);
 
-	opt.env = tmp_objdir_env(tmp_objdir);
+	opt.env = odb_transaction_env(the_repository->objects->transaction);
 	setup_alternate_shallow(&shallow_lock, &opt.shallow_file, &extra);
 	if (check_connected(command_singleton_iterator, cmd, &opt)) {
 		rollback_shallow_file(the_repository, &shallow_lock);
@@ -1802,7 +1800,7 @@ static void set_connectivity_errors(struct command *commands,
 			/* to be checked in update_shallow_ref() */
 			continue;
 
-		opt.env = tmp_objdir_env(tmp_objdir);
+		opt.env = odb_transaction_env(the_repository->objects->transaction);
 		if (!check_connected(command_singleton_iterator, &singleton,
 				     &opt))
 			continue;
@@ -2057,7 +2055,7 @@ static void execute_commands(struct command *commands,
 		data.si = si;
 		opt.err_fd = err_fd;
 		opt.progress = err_fd && !quiet;
-		opt.env = tmp_objdir_env(tmp_objdir);
+		opt.env = odb_transaction_env(the_repository->objects->transaction);
 		opt.exclude_hidden_refs_section = "receive";
 
 		if (check_connected(iterate_receive_command_list, &data, &opt))
@@ -2106,14 +2104,13 @@ static void execute_commands(struct command *commands,
 	 * Now we'll start writing out refs, which means the objects need
 	 * to be in their final positions so that other processes can see them.
 	 */
-	if (tmp_objdir_migrate(tmp_objdir) < 0) {
+	if (odb_transaction_commit(the_repository->objects->transaction)) {
 		for (cmd = commands; cmd; cmd = cmd->next) {
 			if (!cmd->error_string)
 				cmd->error_string = "unable to migrate objects to permanent storage";
 		}
 		return;
 	}
-	tmp_objdir = NULL;
 
 	check_aliased_updates(commands);
 
@@ -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;
@@ -2351,20 +2349,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)
 		strvec_push(&child.args, alt_shallow_file);
 	}
 
-	tmp_objdir = tmp_objdir_create(the_repository, "incoming");
-	if (!tmp_objdir) {
-		if (err_fd > 0)
-			close(err_fd);
-		return "unable to create temporary object directory";
-	}
-	strvec_pushv(&child.env, tmp_objdir_env(tmp_objdir));
-
-	/*
-	 * Normally we just pass the tmp_objdir environment to the child
-	 * processes that do the heavy lifting, but we may need to see these
-	 * objects ourselves to set up shallow information.
-	 */
-	tmp_objdir_add_as_alternate(tmp_objdir);
+	strvec_pushv(&child.env, odb_transaction_env(transaction));
 
 	if (ntohl(hdr.hdr_entries) < unpack_limit) {
 		strvec_push(&child.args, "unpack-objects");
@@ -2431,13 +2416,14 @@ static const char *unpack(int err_fd, struct shallow_info *si)
 	return NULL;
 }
 
-static const char *unpack_with_sideband(struct shallow_info *si)
+static const char *unpack_with_sideband(struct shallow_info *si,
+					struct odb_transaction *transaction)
 {
 	struct async muxer;
 	const char *ret;
 
 	if (!use_sideband)
-		return unpack(0, si);
+		return unpack(0, si, transaction);
 
 	use_keepalive = KEEPALIVE_AFTER_NUL;
 	memset(&muxer, 0, sizeof(muxer));
@@ -2446,7 +2432,7 @@ static const char *unpack_with_sideband(struct shallow_info *si)
 	if (start_async(&muxer))
 		return NULL;
 
-	ret = unpack(muxer.in, si);
+	ret = unpack(muxer.in, si, transaction);
 
 	finish_async(&muxer);
 	return ret;
@@ -2623,6 +2609,7 @@ int cmd_receive_pack(int argc,
 	struct oid_array ref = OID_ARRAY_INIT;
 	struct shallow_info si;
 	struct packet_reader reader;
+	struct odb_transaction *transaction = NULL;
 
 	struct option options[] = {
 		OPT__QUIET(&quiet, N_("quiet")),
@@ -2707,7 +2694,10 @@ int cmd_receive_pack(int argc,
 		if (!si.nr_ours && !si.nr_theirs)
 			shallow_update = 0;
 		if (!delete_only(commands)) {
-			unpack_status = unpack_with_sideband(&si);
+			if (odb_transaction_begin(the_repository->objects, &transaction, ODB_TRANSACTION_RECEIVE))
+				unpack_status = "unable to start ODB transaction";
+			else
+				unpack_status = unpack_with_sideband(&si, transaction);
 			update_shallow_info(commands, &si, &ref);
 		}
 		use_keepalive = KEEPALIVE_ALWAYS;
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index d0136cdd99..c3d0fc7507 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -598,7 +598,7 @@ static void unpack_all(void)
 		progress = start_progress(the_repository,
 					  _("Unpacking objects"), nr_objects);
 	CALLOC_ARRAY(obj_list, nr_objects);
-	odb_transaction_begin_or_die(the_repository->objects, &transaction);
+	odb_transaction_begin_or_die(the_repository->objects, &transaction, 0);
 	for (i = 0; i < nr_objects; i++) {
 		unpack_one(i);
 		display_progress(progress, i + 1);
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 17f3ea284c..bf6ea60ef4 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1124,7 +1124,7 @@ int cmd_update_index(int argc,
 	 * Allow the object layer to optimize adding multiple objects in
 	 * a batch.
 	 */
-	odb_transaction_begin_or_die(the_repository->objects, &transaction);
+	odb_transaction_begin_or_die(the_repository->objects, &transaction, 0);
 	while (ctx.argc) {
 		if (parseopt_state != PARSE_OPT_DONE)
 			parseopt_state = parse_options_step(&ctx, options,
diff --git a/cache-tree.c b/cache-tree.c
index 1a7dfed9cf..ed05acc4c7 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -490,7 +490,7 @@ int cache_tree_update(struct index_state *istate, int flags)
 
 	trace_performance_enter();
 	trace2_region_enter("cache_tree", "update", istate->repo);
-	odb_transaction_begin_or_die(the_repository->objects, &transaction);
+	odb_transaction_begin_or_die(the_repository->objects, &transaction, 0);
 	i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
 		       "", 0, &skip, flags);
 	odb_transaction_commit(transaction);
diff --git a/object-file.c b/object-file.c
index 14064d188a..e7958753ec 100644
--- a/object-file.c
+++ b/object-file.c
@@ -497,6 +497,7 @@ struct odb_transaction_files {
 
 	struct tmp_objdir *objdir;
 	struct transaction_packfile packfile;
+	const char *prefix;
 };
 
 static int odb_transaction_files_prepare(struct odb_transaction *base)
@@ -513,7 +514,7 @@ static int odb_transaction_files_prepare(struct odb_transaction *base)
 	if (!transaction || transaction->objdir)
 		return 0;
 
-	transaction->objdir = tmp_objdir_create(base->source->odb->repo, "bulk-fsync");
+	transaction->objdir = tmp_objdir_create(base->source->odb->repo, transaction->prefix);
 	if (!transaction->objdir)
 		return -1;
 
@@ -1391,7 +1392,7 @@ int index_fd(struct index_state *istate, struct object_id *oid,
 			struct object_database *odb = the_repository->objects;
 			struct odb_transaction *transaction;
 
-			odb_transaction_begin_or_die(odb, &transaction);
+			odb_transaction_begin_or_die(odb, &transaction, 0);
 			ret = odb_transaction_write_object_stream(odb->transaction,
 								  &stream,
 								  xsize_t(st->st_size),
@@ -1702,7 +1703,8 @@ static const char **odb_transaction_files_env(struct odb_transaction *base)
 }
 
 int odb_transaction_files_begin(struct odb_source *source,
-				struct odb_transaction **out)
+				struct odb_transaction **out,
+				enum odb_transaction_flags flags)
 {
 	struct odb_transaction_files *transaction;
 	struct object_database *odb = source->odb;
@@ -1717,6 +1719,20 @@ int odb_transaction_files_begin(struct odb_source *source,
 	transaction->base.commit = odb_transaction_files_commit;
 	transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
 	transaction->base.env = odb_transaction_files_env;
+
+	transaction->prefix = "bulk-fsync";
+	if (flags & ODB_TRANSACTION_RECEIVE) {
+		/*
+		 * ODB transactions for git-receive-pack(1) eagerly create a
+		 * temporary directory and use a different prefix.
+		 */
+		transaction->prefix = "incoming";
+		if (odb_transaction_files_prepare(&transaction->base)) {
+			free(transaction);
+			return -1;
+		}
+	}
+
 	*out = &transaction->base;
 
 	return 0;
diff --git a/object-file.h b/object-file.h
index ac927fec07..fe098d54cb 100644
--- a/object-file.h
+++ b/object-file.h
@@ -5,6 +5,7 @@
 #include "object.h"
 #include "odb.h"
 #include "odb/source-loose.h"
+#include "odb/transaction.h"
 
 /* The maximum size for an object header. */
 #define MAX_HEADER_LEN 32
@@ -198,6 +199,7 @@ struct odb_transaction;
  * pending, out is set to NULL.
  */
 int odb_transaction_files_begin(struct odb_source *source,
-				struct odb_transaction **out);
+				struct odb_transaction **out,
+				enum odb_transaction_flags flags);
 
 #endif /* OBJECT_FILE_H */
diff --git a/odb/source-files.c b/odb/source-files.c
index 2545bd81d4..534f48aad9 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -180,9 +180,10 @@ static int odb_source_files_write_object_stream(struct odb_source *source,
 }
 
 static int odb_source_files_begin_transaction(struct odb_source *source,
-					      struct odb_transaction **out)
+					      struct odb_transaction **out,
+					      enum odb_transaction_flags flags)
 {
-	return odb_transaction_files_begin(source, out);
+	return odb_transaction_files_begin(source, out, flags);
 }
 
 static int odb_source_files_read_alternates(struct odb_source *source,
diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
index e004566d76..9644d9d474 100644
--- a/odb/source-inmemory.c
+++ b/odb/source-inmemory.c
@@ -304,7 +304,8 @@ static int odb_source_inmemory_freshen_object(struct odb_source *source,
 }
 
 static int odb_source_inmemory_begin_transaction(struct odb_source *source UNUSED,
-						 struct odb_transaction **out UNUSED)
+						 struct odb_transaction **out UNUSED,
+						 enum odb_transaction_flags flags UNUSED)
 {
 	return error("in-memory source does not support transactions");
 }
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 66e6bb8d3f..57c91986b4 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -638,7 +638,8 @@ static int odb_source_loose_write_object_stream(struct odb_source *source,
 }
 
 static int odb_source_loose_begin_transaction(struct odb_source *source UNUSED,
-					      struct odb_transaction **out UNUSED)
+					      struct odb_transaction **out UNUSED,
+					      enum odb_transaction_flags flags UNUSED)
 {
 	/* TODO: this is a known omission that we'll want to address eventually. */
 	return error("loose source does not support transactions");
diff --git a/odb/source.h b/odb/source.h
index 2192a101b8..3790d03ff2 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -3,6 +3,7 @@
 
 #include "object.h"
 #include "odb.h"
+#include "odb/transaction.h"
 
 enum odb_source_type {
 	/*
@@ -228,7 +229,8 @@ struct odb_source {
 	 * negative error code otherwise.
 	 */
 	int (*begin_transaction)(struct odb_source *source,
-				 struct odb_transaction **out);
+				 struct odb_transaction **out,
+				 enum odb_transaction_flags flags);
 
 	/*
 	 * This callback is expected to read the list of alternate object
@@ -467,9 +469,10 @@ static inline int odb_source_write_alternate(struct odb_source *source,
  * Returns 0 on success, a negative error code otherwise.
  */
 static inline int odb_source_begin_transaction(struct odb_source *source,
-					       struct odb_transaction **out)
+					       struct odb_transaction **out,
+					       enum odb_transaction_flags flags)
 {
-	return source->begin_transaction(source, out);
+	return source->begin_transaction(source, out, flags);
 }
 
 #endif
diff --git a/odb/transaction.c b/odb/transaction.c
index 20d3f43f54..34c212020c 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -3,7 +3,8 @@
 #include "odb/transaction.h"
 
 int odb_transaction_begin(struct object_database *odb,
-			  struct odb_transaction **out)
+			  struct odb_transaction **out,
+			  enum odb_transaction_flags flags)
 {
 	int ret;
 
@@ -12,7 +13,7 @@ int odb_transaction_begin(struct object_database *odb,
 		return 0;
 	}
 
-	ret = odb_source_begin_transaction(odb->sources, out);
+	ret = odb_source_begin_transaction(odb->sources, out, flags);
 	odb->transaction = *out;
 
 	return ret;
diff --git a/odb/transaction.h b/odb/transaction.h
index 536458297b..78392ff13d 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -4,7 +4,6 @@
 #include "git-compat-util.h"
 #include "gettext.h"
 #include "odb.h"
-#include "odb/source.h"
 
 /*
  * A transaction may be started for an object database prior to writing new
@@ -44,6 +43,10 @@ struct odb_transaction {
 	const char **(*env)(struct odb_transaction *transaction);
 };
 
+enum odb_transaction_flags {
+	ODB_TRANSACTION_RECEIVE = (1 << 0),
+};
+
 /*
  * Starts an ODB transaction and returns it via `out`. Subsequent objects are
  * written to the transaction and not committed until odb_transaction_commit()
@@ -51,12 +54,14 @@ struct odb_transaction {
  * error. If the ODB already has a pending transaction, `out` is set to NULL.
  */
 int odb_transaction_begin(struct object_database *odb,
-			  struct odb_transaction **out);
+			  struct odb_transaction **out,
+			  enum odb_transaction_flags flags);
 
 static inline void odb_transaction_begin_or_die(struct object_database *odb,
-						struct odb_transaction **out)
+						struct odb_transaction **out,
+						enum odb_transaction_flags flags)
 {
-	if (odb_transaction_begin(odb, out))
+	if (odb_transaction_begin(odb, out, flags))
 		die(_("failed to start ODB transaction"));
 }
 
diff --git a/read-cache.c b/read-cache.c
index db0bfa60fe..35bfb25576 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -4042,7 +4042,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
 	 * This function is invoked from commands other than 'add', which
 	 * may not have their own transaction active.
 	 */
-	odb_transaction_begin_or_die(repo->objects, &transaction);
+	odb_transaction_begin_or_die(repo->objects, &transaction, 0);
 	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
 	odb_transaction_commit(transaction);
 
-- 
2.54.0.105.g59ff4886a5

Re: [PATCH 2/6] object-file: propagate files transaction errors

[Patrick Steinhardt]

On Tue, Jun 23, 2026 at 11:19:16PM -0500, Justin Tobler wrote:
> The "files" transaction backend may encounter errors related to managing
> the temporary directory used to stage objects, but silently ignores
> these errors. Instead return errors encountered in the
> `odb_transaction_files_{prepare,begin,commit}()` interfaces to allow
> callers to handle as needed.

Missing a then? "to handle as needed" -> "to handle them as needed"

Makes sense. It always felt a bit off that those functions didn't have a
way to signal errors to the caller.

> diff --git a/object-file.c b/object-file.c
> index a3eb8d71dd..18c2df75fb 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -499,7 +499,7 @@ struct odb_transaction_files {
>  	struct transaction_packfile packfile;
>  };
>  
> -static void odb_transaction_files_prepare(struct odb_transaction *base)
> +static int odb_transaction_files_prepare(struct odb_transaction *base)
>  {
>  	struct odb_transaction_files *transaction =
>  		container_of_or_null(base, struct odb_transaction_files, base);

By the way, is there any reason why those functions are still hosted in
"object-file.c" instead of in "odb/source-files.c"? I should probably
know, but I forgot.

> @@ -511,11 +511,15 @@ static void odb_transaction_files_prepare(struct odb_transaction *base)
>  	 * added at the time they call odb_transaction_files_begin.
>  	 */
>  	if (!transaction || transaction->objdir)
> -		return;
> +		return 0;
>  
>  	transaction->objdir = tmp_objdir_create(base->source->odb->repo, "bulk-fsync");
> -	if (transaction->objdir)
> -		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
> +	if (!transaction->objdir)
> +		return -1;

Huh. So previously we just didn't handle this error at all and just
continued to tag along? Did that result in anything sensible or was this
just YOLOing it?

> @@ -542,13 +546,13 @@ static void fsync_loose_object_transaction(struct odb_transaction *base,
>  /*
>   * Cleanup after batch-mode fsync_object_files.
>   */
> -static void flush_loose_object_transaction(struct odb_transaction_files *transaction)
> +static int flush_loose_object_transaction(struct odb_transaction_files *transaction)

Feels like this function should've been renamed in the preceding commit,
as well.

>  {
>  	struct strbuf temp_path = STRBUF_INIT;
>  	struct tempfile *temp;
>  
>  	if (!transaction->objdir)
> -		return;
> +		return 0;
>  
>  	/*
>  	 * Issue a full hardware flush against a temporary file to ensure
> @@ -570,8 +574,12 @@ static void flush_loose_object_transaction(struct odb_transaction_files *transac

There is a call to `xmks_tempfile()` hidden that can fail, but that
failure is already handled in that function itself by dying.

>  	 * Make the object files visible in the primary ODB after their data is
>  	 * fully durable.
>  	 */
> -	tmp_objdir_migrate(transaction->objdir);
> +	if (tmp_objdir_migrate(transaction->objdir))
> +		return -1;

Feels like another case of YOLOing it. The migration could have failed,
but we just ignored that failure and never told the user about it. The
result may be silent corruption, I assume?

> @@ -1670,27 +1678,34 @@ int read_loose_object(struct repository *repo,
>  	return ret;
>  }
>  
> -static void odb_transaction_files_commit(struct odb_transaction *base)
> +static int odb_transaction_files_commit(struct odb_transaction *base)
>  {
>  	struct odb_transaction_files *transaction =
>  		container_of(base, struct odb_transaction_files, base);
>  
> -	flush_loose_object_transaction(transaction);
> +	if (flush_loose_object_transaction(transaction))
> +		return -1;
>  	flush_packfile_transaction(transaction);
> +
> +	return 0;
>  }
>  
> -struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
> +int odb_transaction_files_begin(struct odb_source *source,
> +				struct odb_transaction **out)
>  {
>  	struct odb_transaction_files *transaction;
>  	struct object_database *odb = source->odb;
>  
> -	if (odb->transaction)
> -		return NULL;
> +	if (odb->transaction) {
> +		*out = NULL;
> +		return 0;
> +	}
>  
>  	transaction = xcalloc(1, sizeof(*transaction));
>  	transaction->base.source = source;
>  	transaction->base.commit = odb_transaction_files_commit;
>  	transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
> +	*out = &transaction->base;
>  
> -	return &transaction->base;
> +	return 0;
>  }

It's still somewhat fishy that we have this ODB-level transaction, but
that's a preexisting issue and thus outside the scope of this patch
series. Ideally though, it would be possible for there to be multiple
transactions, and it would be the caller's responsibility for juggling
these transactions. Just as it happens with reference transactions.

> diff --git a/odb/transaction.h b/odb/transaction.h
> index 854fda06f5..f4c1ebfaaa 100644
> --- a/odb/transaction.h
> +++ b/odb/transaction.h
> @@ -17,7 +17,7 @@ struct odb_transaction {
>  	struct odb_source *source;
>  
>  	/* The ODB source specific callback invoked to commit a transaction. */
> -	void (*commit)(struct odb_transaction *transaction);
> +	int (*commit)(struct odb_transaction *transaction);

We might want to document the returned error code here.

Patrick

Re: [PATCH 3/6] odb/transaction: propagate begin errors

[Patrick Steinhardt]

On Tue, Jun 23, 2026 at 11:19:17PM -0500, Justin Tobler wrote:
> diff --git a/odb/transaction.c b/odb/transaction.c
> index b16e07aebf..d3de01db50 100644
> --- a/odb/transaction.c
> +++ b/odb/transaction.c
> @@ -2,14 +2,20 @@
>  #include "odb/source.h"
>  #include "odb/transaction.h"
>  
> -struct odb_transaction *odb_transaction_begin(struct object_database *odb)
> +int odb_transaction_begin(struct object_database *odb,
> +			  struct odb_transaction **out)
>  {
> -	if (odb->transaction)
> -		return NULL;
> +	int ret;
>  
> -	odb_source_begin_transaction(odb->sources, &odb->transaction);
> +	if (odb->transaction) {
> +		*out = NULL;
> +		return 0;
> +	}

Hm. So we may return successful, but not set the `out` pointer to a
transaction. And...

> diff --git a/odb/transaction.h b/odb/transaction.h
> index f4c1ebfaaa..cd6d50f2e5 100644
> --- a/odb/transaction.h
> +++ b/odb/transaction.h
> @@ -33,11 +35,20 @@ struct odb_transaction {
>  };
>  
>  /*
> - * Starts an ODB transaction. Subsequent objects are written to the transaction
> - * and not committed until odb_transaction_commit() is invoked on the
> - * transaction. If the ODB already has a pending transaction, NULL is returned.
> + * Starts an ODB transaction and returns it via `out`. Subsequent objects are
> + * written to the transaction and not committed until odb_transaction_commit()
> + * is invoked on the transaction. Returns 0 on success and a negative value on
> + * error. If the ODB already has a pending transaction, `out` is set to NULL.
>   */
> -struct odb_transaction *odb_transaction_begin(struct object_database *odb);
> +int odb_transaction_begin(struct object_database *odb,
> +			  struct odb_transaction **out);
> +
> +static inline void odb_transaction_begin_or_die(struct object_database *odb,
> +						struct odb_transaction **out)
> +{
> +	if (odb_transaction_begin(odb, out))
> +		die(_("failed to start ODB transaction"));
> +}

... we don't special-case that here, either. So a caller may invoke the
function, not die, but it might still not have a valid transaction. That
feels wrong to me.

Patrick

Re: [PATCH 4/6] odb/transaction: propagate commit errors

[Patrick Steinhardt]

On Tue, Jun 23, 2026 at 11:19:18PM -0500, Justin Tobler wrote:
> diff --git a/odb/transaction.h b/odb/transaction.h
> index cd6d50f2e5..7898770071 100644
> --- a/odb/transaction.h
> +++ b/odb/transaction.h
> @@ -54,7 +54,7 @@ static inline void odb_transaction_begin_or_die(struct object_database *odb,
>   * Commits an ODB transaction making the written objects visible. If the
>   * specified transaction is NULL, the function is a no-op.
>   */
> -void odb_transaction_commit(struct odb_transaction *transaction);
> +int odb_transaction_commit(struct odb_transaction *transaction);

Should the function comment be amended, as well? We should definitely
point out that calling this with a NULL transaction also returns
success.

Patrick

Re: [PATCH 5/6] odb/transaction: add transaction env interface

[Patrick Steinhardt]

On Tue, Jun 23, 2026 at 11:19:19PM -0500, Justin Tobler wrote:
> The ODB transaction backend is responsible for creating/managing its own
> staging area for writing objects. Other child processes spawned by Git
> may need to access to uncommitted objects or write new objects in the

s/may need to access to/may need access to/

> staging area though.
> 
> Introduce `odb_transaction_env()` which is expected to provide the set
> of environment variables needed by a child process to access the
> transaction staging area.

Possessive s is missing, I think.

> diff --git a/object-file.c b/object-file.c
> index 696f05dc2d..14064d188a 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1691,6 +1691,16 @@ static int odb_transaction_files_commit(struct odb_transaction *base)
>  	return 0;
>  }
>  
> +static const char **odb_transaction_files_env(struct odb_transaction *base)
> +{
> +	struct odb_transaction_files *transaction =
> +		container_of(base, struct odb_transaction_files, base);
> +
> +	odb_transaction_files_prepare(&transaction->base);
> +
> +	return tmp_objdir_env(transaction->objdir);
> +}
> +
>  int odb_transaction_files_begin(struct odb_source *source,
>  				struct odb_transaction **out)
>  {

Makes sense. Transactions may have a different way to quarantine the
write than using a quarantine directory. So making this functionality
pluggable so that backends may expose a separate set of environment
variables feels sensible.

> diff --git a/odb/transaction.h b/odb/transaction.h
> index 7898770071..536458297b 100644
> --- a/odb/transaction.h
> +++ b/odb/transaction.h
> @@ -32,6 +32,16 @@ struct odb_transaction {
>  	int (*write_object_stream)(struct odb_transaction *transaction,
>  				   struct odb_write_stream *stream, size_t len,
>  				   struct object_id *oid);
> +
> +	/*
> +	 * This callback is expected to return a NULL-terminated array of
> +	 * environment variables that a child process should inherit so
> +	 * that its object writes participate in the transaction. The
> +	 * returned array is owned by the backend and remains valid until
> +	 * the transaction ends. May return NULL when the backend does not
> +	 * need to expose any state to child processes.
> +	 */
> +	const char **(*env)(struct odb_transaction *transaction);

Would it make more sense to adapt this function so that:

  - It receives a `struct strvec` as input that the environment
    variables are to be amended to.

  - It returns a normal error code to indicate errors?

Patrick

Re: [PATCH 6/6] builtin/receive-pack: stage incoming objects via ODB transactions

[Patrick Steinhardt]

On Tue, Jun 23, 2026 at 11:19:20PM -0500, Justin Tobler wrote:
> 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?

> 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 might've also made sense to split this commit up into two: one to
introduce the flag parameter, and then one to do the changes to
git-receive-pack(1).

> 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
> @@ -112,8 +112,6 @@ static enum {
>  } use_keepalive;
>  static int keepalive_in_sec = 5;
>  
> -static struct tmp_objdir *tmp_objdir;
> -
>  static struct proc_receive_ref {
>  	unsigned int want_add:1,
>  		     want_delete:1,

I assume the goal is that we convert all other users of the tmp-objdir
subsystem to also use transactions eventually, so that this becomes an
implementation detail fo the files transaction?

> @@ -2106,14 +2104,13 @@ static void execute_commands(struct command *commands,
>  	 * Now we'll start writing out refs, which means the objects need
>  	 * to be in their final positions so that other processes can see them.
>  	 */
> -	if (tmp_objdir_migrate(tmp_objdir) < 0) {
> +	if (odb_transaction_commit(the_repository->objects->transaction)) {
>  		for (cmd = commands; cmd; cmd = cmd->next) {
>  			if (!cmd->error_string)
>  				cmd->error_string = "unable to migrate objects to permanent storage";
>  		}
>  		return;
>  	}
> -	tmp_objdir = NULL;

We don't need to unset the transaction because that's what
`odb_transaction_commit()` already does for us, I assume?

> @@ -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`.

> @@ -2351,20 +2349,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)
>  		strvec_push(&child.args, alt_shallow_file);
>  	}
>  
> -	tmp_objdir = tmp_objdir_create(the_repository, "incoming");
> -	if (!tmp_objdir) {
> -		if (err_fd > 0)
> -			close(err_fd);
> -		return "unable to create temporary object directory";
> -	}
> -	strvec_pushv(&child.env, tmp_objdir_env(tmp_objdir));
> -
> -	/*
> -	 * Normally we just pass the tmp_objdir environment to the child
> -	 * processes that do the heavy lifting, but we may need to see these
> -	 * objects ourselves to set up shallow information.
> -	 */
> -	tmp_objdir_add_as_alternate(tmp_objdir);
> +	strvec_pushv(&child.env, odb_transaction_env(transaction));

Interesting, this here seems like a change in behaviour. Previously we
added the transactions as an alternate, but now we only propagate it via
the environment. I didn't see this mentioned in the commit message.

> @@ -2707,7 +2694,10 @@ int cmd_receive_pack(int argc,
>  		if (!si.nr_ours && !si.nr_theirs)
>  			shallow_update = 0;
>  		if (!delete_only(commands)) {
> -			unpack_status = unpack_with_sideband(&si);
> +			if (odb_transaction_begin(the_repository->objects, &transaction, ODB_TRANSACTION_RECEIVE))
> +				unpack_status = "unable to start ODB transaction";

s/ODB/object/

This may be visible to the user, and "ODB" may mean nothing to them.

> diff --git a/object-file.c b/object-file.c
> index 14064d188a..e7958753ec 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1702,7 +1703,8 @@ static const char **odb_transaction_files_env(struct odb_transaction *base)
>  }
>  
>  int odb_transaction_files_begin(struct odb_source *source,
> -				struct odb_transaction **out)
> +				struct odb_transaction **out,
> +				enum odb_transaction_flags flags)
>  {
>  	struct odb_transaction_files *transaction;
>  	struct object_database *odb = source->odb;
> @@ -1717,6 +1719,20 @@ int odb_transaction_files_begin(struct odb_source *source,
>  	transaction->base.commit = odb_transaction_files_commit;
>  	transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
>  	transaction->base.env = odb_transaction_files_env;
> +
> +	transaction->prefix = "bulk-fsync";
> +	if (flags & ODB_TRANSACTION_RECEIVE) {
> +		/*
> +		 * ODB transactions for git-receive-pack(1) eagerly create a
> +		 * temporary directory and use a different prefix.
> +		 */
> +		transaction->prefix = "incoming";
> +		if (odb_transaction_files_prepare(&transaction->base)) {
> +			free(transaction);
> +			return -1;
> +		}
> +	}
> +

Okay, makes sense. I really wonder whether we need to insist this much
on the exact name used by this, but better be safe than sorry for now I
guess.

And as mentioned before, I also wonder whether it really makes sense to
have the lazy creation of the tmp-objdir. Maybe add a NEEDSWORK item
here that mentions we want to investigate whether this is even needed at
all?

> diff --git a/odb/transaction.h b/odb/transaction.h
> index 536458297b..78392ff13d 100644
> --- a/odb/transaction.h
> +++ b/odb/transaction.h
> @@ -44,6 +43,10 @@ struct odb_transaction {
>  	const char **(*env)(struct odb_transaction *transaction);
>  };
>  
> +enum odb_transaction_flags {
> +	ODB_TRANSACTION_RECEIVE = (1 << 0),
> +};

It's not clear at all what this flag does based on its name, so we
should have documentation for it.

Patrick

Re: [PATCH 0/6] receive-pack: use ODB transactions to stage object writes

[Patrick Steinhardt]

On Tue, Jun 23, 2026 at 11:19:14PM -0500, Justin Tobler wrote:
> Greetings,
> 
> This patch series replaces direct usage of the `tmp_objdir` interfaces
> in git-receive-pack(1) to instead use the `odb_transaction` interfaces
> to create/manage a staging area to write objects to. The purpose of this
> change is to get git-receive-pack(1) one step closer to being ODB
> backend agnostic. For now, the object writes themselves are still
> "files" backend specific due to being handled by the git-index-pack(1)
> and git-unpack-objects(1) child processes. This will be tackled in a
> separate series though.

Thanks, this was a pleasant read. I've got a bunch of comments, but
overall I really like the direction of this patch series.

Patrick