[PATCH 0/4] odb: support ODB source specific transaction handling

[PATCH 0/4] odb: support ODB source specific transaction handling

[Justin Tobler]

Greetings,

ODB transactions provide a mechanism for writing new objects in the
repository that are in a pending state until they are committed. With
pluggable ODBs, each source will likely need to handle transactions
differently. This patch series prepares ODB transaction handling to be
defined for each ODB source by creating a generic odb_transaction
structure that stores a callback for committing the transaction.

For now, `struct odb_transaction_loose` is the only transaction
implementation and what is always returned when creating a transaction.
Additionally, this series lifts transaction logic that is common to all
backends out of the implementation layer into
`odb_transaction_{begin,commit}()` layer.

Thanks,
-Justin

Justin Tobler (4):
  odb: store ODB source in `struct odb_transaction`
  object-file: rename transaction functions
  odb: prepare `struct odb_transaction` to support more sources
  odb: transparently handle common transaction behavior

 object-file.c | 77 +++++++++++++++++++++++++--------------------------
 object-file.h | 10 ++-----
 odb.c         | 22 +++++++++++++--
 odb.h         | 17 ++++++++++++
 4 files changed, 77 insertions(+), 49 deletions(-)


base-commit: ea717645d199f6f1b66058886475db3e8c9330e9
-- 
2.52.0.373.g68cb7f9e92

[PATCH 1/4] odb: store ODB source in `struct odb_transaction`

[Justin Tobler]

Each `struct odb_transaction` currently stores a reference to the
`struct object_database`. Since transactions are handled per object
source, instead store a reference to the source.

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

diff --git a/object-file.c b/object-file.c
index e7e4c3348f..196509b252 100644
--- a/object-file.c
+++ b/object-file.c
@@ -711,7 +711,7 @@ struct transaction_packfile {
 };
 
 struct odb_transaction {
-	struct object_database *odb;
+	struct odb_source *source;
 
 	struct tmp_objdir *objdir;
 	struct transaction_packfile packfile;
@@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
 	if (!transaction || transaction->objdir)
 		return;
 
-	transaction->objdir = tmp_objdir_create(transaction->odb->repo, "bulk-fsync");
+	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
 	if (transaction->objdir)
 		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
 }
@@ -772,7 +772,7 @@ static void flush_loose_object_transaction(struct odb_transaction *transaction)
 	 * the final name is visible.
 	 */
 	strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
-		    repo_get_object_directory(transaction->odb->repo));
+		    repo_get_object_directory(transaction->source->odb->repo));
 	temp = xmks_tempfile(temp_path.buf);
 	fsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));
 	delete_tempfile(&temp);
@@ -1344,7 +1344,7 @@ static int already_written(struct odb_transaction *transaction,
 			   struct object_id *oid)
 {
 	/* The object may already exist in the repository */
-	if (odb_has_object(transaction->odb, oid,
+	if (odb_has_object(transaction->source->odb, oid,
 			   HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))
 		return 1;
 
@@ -1365,7 +1365,7 @@ static void prepare_packfile_transaction(struct odb_transaction *transaction,
 	if (!(flags & INDEX_WRITE_OBJECT) || state->f)
 		return;
 
-	state->f = create_tmp_packfile(transaction->odb->repo,
+	state->f = create_tmp_packfile(transaction->source->odb->repo,
 				       &state->pack_tmp_name);
 	reset_pack_idx_option(&state->pack_idx_opts);
 
@@ -1469,7 +1469,7 @@ static int stream_blob_to_pack(struct transaction_packfile *state,
 static void flush_packfile_transaction(struct odb_transaction *transaction)
 {
 	struct transaction_packfile *state = &transaction->packfile;
-	struct repository *repo = transaction->odb->repo;
+	struct repository *repo = transaction->source->odb->repo;
 	unsigned char hash[GIT_MAX_RAWSZ];
 	struct strbuf packname = STRBUF_INIT;
 	char *idx_tmp_name = NULL;
@@ -1494,7 +1494,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)
 	}
 
 	strbuf_addf(&packname, "%s/pack/pack-%s.",
-		    repo_get_object_directory(transaction->odb->repo),
+		    repo_get_object_directory(transaction->source->odb->repo),
 		    hash_to_hex_algop(hash, repo->hash_algo));
 
 	stage_tmp_packfiles(repo, &packname, state->pack_tmp_name,
@@ -1553,7 +1553,7 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,
 
 	header_len = format_object_header((char *)obuf, sizeof(obuf),
 					  OBJ_BLOB, size);
-	transaction->odb->repo->hash_algo->init_fn(&ctx);
+	transaction->source->odb->repo->hash_algo->init_fn(&ctx);
 	git_hash_update(&ctx, obuf, header_len);
 
 	/* Note: idx is non-NULL when we are writing */
@@ -1993,7 +1993,7 @@ struct odb_transaction *object_file_transaction_begin(struct odb_source *source)
 		return NULL;
 
 	CALLOC_ARRAY(odb->transaction, 1);
-	odb->transaction->odb = odb;
+	odb->transaction->source = source;
 
 	return odb->transaction;
 }
@@ -2006,11 +2006,11 @@ void object_file_transaction_commit(struct odb_transaction *transaction)
 	/*
 	 * Ensure the transaction ending matches the pending transaction.
 	 */
-	ASSERT(transaction == transaction->odb->transaction);
+	ASSERT(transaction == transaction->source->odb->transaction);
 
 	flush_loose_object_transaction(transaction);
 	flush_packfile_transaction(transaction);
-	transaction->odb->transaction = NULL;
+	transaction->source->odb->transaction = NULL;
 	free(transaction);
 }
 
-- 
2.52.0.373.g68cb7f9e92

Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`

[Patrick Steinhardt]

On Wed, Jan 28, 2026 at 05:45:16PM -0600, Justin Tobler wrote:
> Each `struct odb_transaction` currently stores a reference to the
> `struct object_database`. Since transactions are handled per object
> source, instead store a reference to the source.

Makes sense.

> diff --git a/object-file.c b/object-file.c
> index e7e4c3348f..196509b252 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
>  	if (!transaction || transaction->objdir)
>  		return;
>  
> -	transaction->objdir = tmp_objdir_create(transaction->odb->repo, "bulk-fsync");
> +	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
>  	if (transaction->objdir)
>  		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
>  }

This makes me wonder whether we should first refactor the `tmp_objdir`
subsystem to receive a source instead of a repository as input.
Otherwise we "pretend" that the transaction is on the source level, but
we ultimately still end up creating the temporary directory in the
repository's object directory unconditionally.

It wouldn't really change anything right now as we only ever write
objects via the primary object source anyway, so the end result would be
the same. But it just feels like a good first step to me to fix this
conceptual inconsistency, and it shouldn't be too involved either as
`tmp_objdir_create()` only has three callsites.

Patrick

Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Wed, Jan 28, 2026 at 05:45:16PM -0600, Justin Tobler wrote:
>> Each `struct odb_transaction` currently stores a reference to the
>> `struct object_database`. Since transactions are handled per object
>> source, instead store a reference to the source.
>
> Makes sense.
>
>> diff --git a/object-file.c b/object-file.c
>> index e7e4c3348f..196509b252 100644
>> --- a/object-file.c
>> +++ b/object-file.c
>> @@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
>>  	if (!transaction || transaction->objdir)
>>  		return;
>>  
>> -	transaction->objdir = tmp_objdir_create(transaction->odb->repo, "bulk-fsync");
>> +	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
>>  	if (transaction->objdir)
>>  		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
>>  }
>
> This makes me wonder whether we should first refactor the `tmp_objdir`
> subsystem to receive a source instead of a repository as input.
> Otherwise we "pretend" that the transaction is on the source level, but
> we ultimately still end up creating the temporary directory in the
> repository's object directory unconditionally.
>
> It wouldn't really change anything right now as we only ever write
> objects via the primary object source anyway, so the end result would be
> the same. But it just feels like a good first step to me to fix this
> conceptual inconsistency, and it shouldn't be too involved either as
> `tmp_objdir_create()` only has three callsites.

I agree with your "not really change anything right now" comment,
but a new odb source that will be invented in the future may not
even be file based, and a generic-sounding tmp_objdir_create() that
creates a temporary directory on the filesystem may not even be an
appropriate abstraction.

If we have two or more odb sources both are filesystem based, on the
other hand, I do not think it is particulary bad if these two odb
sources belonging to the same repository took a temporary directory
out of that repository.  As long as one temporary object directory
taken by one odb source is not used to commit the transaction into
the other odb source, it would be fine, no?

Thanks.

Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`

[Justin Tobler]

On 26/01/29 11:25AM, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > This makes me wonder whether we should first refactor the `tmp_objdir`
> > subsystem to receive a source instead of a repository as input.
> > Otherwise we "pretend" that the transaction is on the source level, but
> > we ultimately still end up creating the temporary directory in the
> > repository's object directory unconditionally.
> >
> > It wouldn't really change anything right now as we only ever write
> > objects via the primary object source anyway, so the end result would be
> > the same. But it just feels like a good first step to me to fix this
> > conceptual inconsistency, and it shouldn't be too involved either as
> > `tmp_objdir_create()` only has three callsites.
> 
> I agree with your "not really change anything right now" comment,
> but a new odb source that will be invented in the future may not
> even be file based, and a generic-sounding tmp_objdir_create() that
> creates a temporary directory on the filesystem may not even be an
> appropriate abstraction.

Yup, the tmp-objdir system is really only useful in the context of file
based ODB sources. I figure future ODB sources will have to have a
different mechanism to temporarly store objects.

Interestingly, it looks like there are only three users of odb-tmpdir:
remerge-diffs, git-recieve-pack, and ODB transactions. All of these
use-cases seems like a reasonble fit to create an ODB transaction
instead of managing the tmpdir directly. In the case of remerge-diffs
the transaction would need to always be aborted. If this is done, then a
tmpdir could become an internal detail of the ODB transaction for the
files backend.

> If we have two or more odb sources both are filesystem based, on the
> other hand, I do not think it is particulary bad if these two odb
> sources belonging to the same repository took a temporary directory
> out of that repository.  As long as one temporary object directory
> taken by one odb source is not used to commit the transaction into
> the other odb source, it would be fine, no?

I believe that we only support having a single tmpdir per Git process
via `the_tmp_objdir` global anyway. So maybe on second thought
configuring per source doesn't make much sense especially if only the
"files" ODB source would make use of it. If we do go down the route of
merging the tmpdir with the "files" ODB transaction implementation, we
could probably just pass the concrete "files" ODB source to it instead.

-Justin

Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> Interestingly, it looks like there are only three users of odb-tmpdir:
> remerge-diffs, git-recieve-pack, and ODB transactions. All of these
> use-cases seems like a reasonble fit to create an ODB transaction
> instead of managing the tmpdir directly. In the case of remerge-diffs
> the transaction would need to always be aborted. If this is done, then a
> tmpdir could become an internal detail of the ODB transaction for the
> files backend.

;-)  I agree 100%.

"Prepare to create objects that may be undone in the end", "Now make
these objects we created so far as parmanent part of the object
store", "Reject those objects we created so far as the transaction
created them is being aborted" are requests at the right abstraction
level.  "Give me a temporary object directory" is not.

Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`

[Justin Tobler]

On 26/01/29 12:28PM, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > Interestingly, it looks like there are only three users of odb-tmpdir:
> > remerge-diffs, git-recieve-pack, and ODB transactions. All of these
> > use-cases seems like a reasonble fit to create an ODB transaction
> > instead of managing the tmpdir directly. In the case of remerge-diffs
> > the transaction would need to always be aborted. If this is done, then a
> > tmpdir could become an internal detail of the ODB transaction for the
> > files backend.
> 
> ;-)  I agree 100%.
> 
> "Prepare to create objects that may be undone in the end", "Now make
> these objects we created so far as parmanent part of the object
> store", "Reject those objects we created so far as the transaction
> created them is being aborted" are requests at the right abstraction
> level.  "Give me a temporary object directory" is not.

Ok, I'll go ahead and leave the tmp-objdir stuff alone in this patch
series and look into extending the ODB transaction usage to replace
existing tmpdir callsites in a followup series.

Thanks,
-Justin

Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`

[Justin Tobler]

On 26/01/29 12:24PM, Patrick Steinhardt wrote:
> On Wed, Jan 28, 2026 at 05:45:16PM -0600, Justin Tobler wrote:
> > diff --git a/object-file.c b/object-file.c
> > index e7e4c3348f..196509b252 100644
> > --- a/object-file.c
> > +++ b/object-file.c
> > @@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
> >  	if (!transaction || transaction->objdir)
> >  		return;
> >  
> > -	transaction->objdir = tmp_objdir_create(transaction->odb->repo, "bulk-fsync");
> > +	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
> >  	if (transaction->objdir)
> >  		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
> >  }
> 
> This makes me wonder whether we should first refactor the `tmp_objdir`
> subsystem to receive a source instead of a repository as input.
> Otherwise we "pretend" that the transaction is on the source level, but
> we ultimately still end up creating the temporary directory in the
> repository's object directory unconditionally.
> 
> It wouldn't really change anything right now as we only ever write
> objects via the primary object source anyway, so the end result would be
> the same. But it just feels like a good first step to me to fix this
> conceptual inconsistency, and it shouldn't be too involved either as
> `tmp_objdir_create()` only has three callsites.

Ya this seems reasonable and I agree that it makes sense to do as a
first step. In the next version I'll make this change in a prepatory
commit.

-Justin

[PATCH 2/4] object-file: rename transaction functions

[Justin Tobler]

In a subsequent commit, ODB transactions are made more generic to
facilitate each ODB source providing its own transaction handling.
Rename `object_file_transaction_{begin,commit}()` to
`odb_transaction_loose_{begin,commit}()` to better match the future
source specific transaction implementation.

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

diff --git a/object-file.c b/object-file.c
index 196509b252..790be25f08 100644
--- a/object-file.c
+++ b/object-file.c
@@ -723,7 +723,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
 	 * We lazily create the temporary object directory
 	 * the first time an object might be added, since
 	 * callers may not know whether any objects will be
-	 * added at the time they call object_file_transaction_begin.
+	 * added at the time they call odb_transaction_loose_begin.
 	 */
 	if (!transaction || transaction->objdir)
 		return;
@@ -1985,7 +1985,7 @@ int read_loose_object(struct repository *repo,
 	return ret;
 }
 
-struct odb_transaction *object_file_transaction_begin(struct odb_source *source)
+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
 {
 	struct object_database *odb = source->odb;
 
@@ -1998,7 +1998,7 @@ struct odb_transaction *object_file_transaction_begin(struct odb_source *source)
 	return odb->transaction;
 }
 
-void object_file_transaction_commit(struct odb_transaction *transaction)
+void odb_transaction_loose_commit(struct odb_transaction *transaction)
 {
 	if (!transaction)
 		return;
diff --git a/object-file.h b/object-file.h
index 1229d5f675..03f0474656 100644
--- a/object-file.h
+++ b/object-file.h
@@ -202,16 +202,16 @@ struct odb_transaction;
 
 /*
  * Tell the object database to optimize for adding
- * multiple objects. object_file_transaction_commit must be called
+ * multiple objects. odb_transaction_loose_commit must be called
  * to make new objects visible. If a transaction is already
  * pending, NULL is returned.
  */
-struct odb_transaction *object_file_transaction_begin(struct odb_source *source);
+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);
 
 /*
  * Tell the object database to make any objects from the
  * current transaction visible.
  */
-void object_file_transaction_commit(struct odb_transaction *transaction);
+void odb_transaction_loose_commit(struct odb_transaction *transaction);
 
 #endif /* OBJECT_FILE_H */
diff --git a/odb.c b/odb.c
index ac70b6a099..90dcbca821 100644
--- a/odb.c
+++ b/odb.c
@@ -1153,10 +1153,10 @@ void odb_reprepare(struct object_database *o)
 
 struct odb_transaction *odb_transaction_begin(struct object_database *odb)
 {
-	return object_file_transaction_begin(odb->sources);
+	return odb_transaction_loose_begin(odb->sources);
 }
 
 void odb_transaction_commit(struct odb_transaction *transaction)
 {
-	object_file_transaction_commit(transaction);
+	odb_transaction_loose_commit(transaction);
 }
-- 
2.52.0.373.g68cb7f9e92

[PATCH 3/4] odb: prepare `struct odb_transaction` to support more sources

[Justin Tobler]

Each ODB transaction should be specific to the ODB source it pertains
to. Update `struct odb_transaction` to store a commit callback specific
to its object source type. For now `struct odb_transaction_loose` is the
only transaction type and what is always returned when starting a
transaction.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c | 80 ++++++++++++++++++++++++++++-----------------------
 object-file.h |  6 ----
 odb.c         |  5 +++-
 odb.h         | 17 +++++++++++
 4 files changed, 65 insertions(+), 43 deletions(-)

diff --git a/object-file.c b/object-file.c
index 790be25f08..e4739fc0cc 100644
--- a/object-file.c
+++ b/object-file.c
@@ -710,15 +710,17 @@ struct transaction_packfile {
 	uint32_t nr_written;
 };
 
-struct odb_transaction {
-	struct odb_source *source;
+struct odb_transaction_loose {
+	struct odb_transaction base;
 
 	struct tmp_objdir *objdir;
 	struct transaction_packfile packfile;
 };
 
-static void prepare_loose_object_transaction(struct odb_transaction *transaction)
+static void prepare_loose_object_transaction(struct odb_transaction *base)
 {
+	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
+
 	/*
 	 * We lazily create the temporary object directory
 	 * the first time an object might be added, since
@@ -728,14 +730,16 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
 	if (!transaction || transaction->objdir)
 		return;
 
-	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
+	transaction->objdir = tmp_objdir_create(base->source->odb->repo, "bulk-fsync");
 	if (transaction->objdir)
 		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
 }
 
-static void fsync_loose_object_transaction(struct odb_transaction *transaction,
+static void fsync_loose_object_transaction(struct odb_transaction *base,
 					   int fd, const char *filename)
 {
+	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
+
 	/*
 	 * If we have an active ODB transaction, we issue a call that
 	 * cleans the filesystem page cache but avoids a hardware flush
@@ -754,7 +758,7 @@ static void fsync_loose_object_transaction(struct odb_transaction *transaction,
 /*
  * Cleanup after batch-mode fsync_object_files.
  */
-static void flush_loose_object_transaction(struct odb_transaction *transaction)
+static void flush_loose_object_transaction(struct odb_transaction_loose *transaction)
 {
 	struct strbuf temp_path = STRBUF_INIT;
 	struct tempfile *temp;
@@ -772,7 +776,7 @@ static void flush_loose_object_transaction(struct odb_transaction *transaction)
 	 * the final name is visible.
 	 */
 	strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
-		    repo_get_object_directory(transaction->source->odb->repo));
+		    repo_get_object_directory(transaction->base.source->odb->repo));
 	temp = xmks_tempfile(temp_path.buf);
 	fsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));
 	delete_tempfile(&temp);
@@ -1340,11 +1344,11 @@ static int index_core(struct index_state *istate,
 	return ret;
 }
 
-static int already_written(struct odb_transaction *transaction,
+static int already_written(struct odb_transaction_loose *transaction,
 			   struct object_id *oid)
 {
 	/* The object may already exist in the repository */
-	if (odb_has_object(transaction->source->odb, oid,
+	if (odb_has_object(transaction->base.source->odb, oid,
 			   HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))
 		return 1;
 
@@ -1358,14 +1362,14 @@ static int already_written(struct odb_transaction *transaction,
 }
 
 /* Lazily create backing packfile for the state */
-static void prepare_packfile_transaction(struct odb_transaction *transaction,
+static void prepare_packfile_transaction(struct odb_transaction_loose *transaction,
 					 unsigned flags)
 {
 	struct transaction_packfile *state = &transaction->packfile;
 	if (!(flags & INDEX_WRITE_OBJECT) || state->f)
 		return;
 
-	state->f = create_tmp_packfile(transaction->source->odb->repo,
+	state->f = create_tmp_packfile(transaction->base.source->odb->repo,
 				       &state->pack_tmp_name);
 	reset_pack_idx_option(&state->pack_idx_opts);
 
@@ -1466,10 +1470,10 @@ static int stream_blob_to_pack(struct transaction_packfile *state,
 	return 0;
 }
 
-static void flush_packfile_transaction(struct odb_transaction *transaction)
+static void flush_packfile_transaction(struct odb_transaction_loose *transaction)
 {
 	struct transaction_packfile *state = &transaction->packfile;
-	struct repository *repo = transaction->source->odb->repo;
+	struct repository *repo = transaction->base.source->odb->repo;
 	unsigned char hash[GIT_MAX_RAWSZ];
 	struct strbuf packname = STRBUF_INIT;
 	char *idx_tmp_name = NULL;
@@ -1494,7 +1498,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)
 	}
 
 	strbuf_addf(&packname, "%s/pack/pack-%s.",
-		    repo_get_object_directory(transaction->source->odb->repo),
+		    repo_get_object_directory(transaction->base.source->odb->repo),
 		    hash_to_hex_algop(hash, repo->hash_algo));
 
 	stage_tmp_packfiles(repo, &packname, state->pack_tmp_name,
@@ -1534,7 +1538,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)
  * binary blobs, they generally do not want to get any conversion, and
  * callers should avoid this code path when filters are requested.
  */
-static int index_blob_packfile_transaction(struct odb_transaction *transaction,
+static int index_blob_packfile_transaction(struct odb_transaction_loose *transaction,
 					   struct object_id *result_oid, int fd,
 					   size_t size, const char *path,
 					   unsigned flags)
@@ -1553,7 +1557,7 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,
 
 	header_len = format_object_header((char *)obuf, sizeof(obuf),
 					  OBJ_BLOB, size);
-	transaction->source->odb->repo->hash_algo->init_fn(&ctx);
+	transaction->base.source->odb->repo->hash_algo->init_fn(&ctx);
 	git_hash_update(&ctx, obuf, header_len);
 
 	/* Note: idx is non-NULL when we are writing */
@@ -1629,10 +1633,11 @@ int index_fd(struct index_state *istate, struct object_id *oid,
 		ret = index_core(istate, oid, fd, xsize_t(st->st_size),
 				 type, path, flags);
 	} else {
+		struct object_database *odb = the_repository->objects;
 		struct odb_transaction *transaction;
 
-		transaction = odb_transaction_begin(the_repository->objects);
-		ret = index_blob_packfile_transaction(the_repository->objects->transaction,
+		transaction = odb_transaction_begin(odb);
+		ret = index_blob_packfile_transaction((struct odb_transaction_loose *)odb->transaction,
 						      oid, fd,
 						      xsize_t(st->st_size),
 						      path, flags);
@@ -1985,35 +1990,38 @@ int read_loose_object(struct repository *repo,
 	return ret;
 }
 
-struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
+static void odb_transaction_loose_commit(struct odb_transaction *base)
 {
-	struct object_database *odb = source->odb;
-
-	if (odb->transaction)
-		return NULL;
-
-	CALLOC_ARRAY(odb->transaction, 1);
-	odb->transaction->source = source;
-
-	return odb->transaction;
-}
-
-void odb_transaction_loose_commit(struct odb_transaction *transaction)
-{
-	if (!transaction)
-		return;
+	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
 
 	/*
 	 * Ensure the transaction ending matches the pending transaction.
 	 */
-	ASSERT(transaction == transaction->source->odb->transaction);
+	ASSERT(base == base->source->odb->transaction);
 
 	flush_loose_object_transaction(transaction);
 	flush_packfile_transaction(transaction);
-	transaction->source->odb->transaction = NULL;
+	base->source->odb->transaction = NULL;
 	free(transaction);
 }
 
+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
+{
+	struct odb_transaction_loose *transaction;
+	struct object_database *odb = source->odb;
+
+	if (odb->transaction)
+		return NULL;
+
+	transaction = xcalloc(1, sizeof(*transaction));
+	transaction->base.source = source;
+	transaction->base.commit = odb_transaction_loose_commit;
+
+	odb->transaction = &transaction->base;
+
+	return &transaction->base;
+}
+
 struct odb_source_loose *odb_source_loose_new(struct odb_source *source)
 {
 	struct odb_source_loose *loose;
diff --git a/object-file.h b/object-file.h
index 03f0474656..1bd355b7f4 100644
--- a/object-file.h
+++ b/object-file.h
@@ -208,10 +208,4 @@ struct odb_transaction;
  */
 struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);
 
-/*
- * Tell the object database to make any objects from the
- * current transaction visible.
- */
-void odb_transaction_loose_commit(struct odb_transaction *transaction);
-
 #endif /* OBJECT_FILE_H */
diff --git a/odb.c b/odb.c
index 90dcbca821..5b112f2464 100644
--- a/odb.c
+++ b/odb.c
@@ -1158,5 +1158,8 @@ struct odb_transaction *odb_transaction_begin(struct object_database *odb)
 
 void odb_transaction_commit(struct odb_transaction *transaction)
 {
-	odb_transaction_loose_commit(transaction);
+	if (!transaction)
+		return;
+
+	transaction->commit(transaction);
 }
diff --git a/odb.h b/odb.h
index bab07755f4..83d3a37805 100644
--- a/odb.h
+++ b/odb.h
@@ -77,7 +77,24 @@ struct odb_source {
 struct packed_git;
 struct packfile_store;
 struct cached_object_entry;
+
+/*
+ * A transaction may be started for an object database prior to writing new
+ * objects via odb_transaction_begin(). These objects are not committed until
+ * odb_transaction_commit() is invoked. Only a single transaction may be pending
+ * at a time.
+ *
+ * Each ODB source is expected to implement its own transaction handling.
+ */
 struct odb_transaction;
+typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);
+struct odb_transaction {
+	/* The ODB source the transaction is opened against. */
+	struct odb_source *source;
+
+	/* The ODB source specific callback invoked to commit a transaction. */
+	odb_transaction_commit_fn commit;
+};
 
 /*
  * The object database encapsulates access to objects in a repository. It
-- 
2.52.0.373.g68cb7f9e92

Re: [PATCH 3/4] odb: prepare `struct odb_transaction` to support more sources

[Patrick Steinhardt]

On Wed, Jan 28, 2026 at 05:45:18PM -0600, Justin Tobler wrote:

The bit about supporting "more sources" in the subject reads a bit weird
to me. We still only handle a single source in a transaction, not
multiple ones. I guess what you rather want to say is that we handle
"generic" sources? How about:

    odb: prepare `struct odb_source` to become generic

> Each ODB transaction should be specific to the ODB source it pertains
> to.

This is a claim that should probably be backed up a bit. I myself
obviously agree with it, but I think it should be noted _why_ we want to
have this in the first place.

The patch itself looks as expected to me, as we split up `struct
odb_transaction` into two structures:

  - `struct odb_transaction` continues to exist, but is now the generic
    part that simply contains the source and a function pointer.

  - `struct odb_transaction_loose` is the backend-specific
    implementation.

One question though: is this tansaction really specific to loose
objects? We also seem to be handling packfiles there in
`prepare_packfile_transaction()`, so it rather feels like this is
specific to the whole "files" backend. I might be misunderstanding
though.

> --- a/object-file.h
> +++ b/object-file.h
> @@ -208,10 +208,4 @@ struct odb_transaction;
>   */
>  struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);

Right, we still need this function to be public right now. But
eventually it'll be converted into a function pointer part of the
`struct odb_source`.

> -/*
> - * Tell the object database to make any objects from the
> - * current transaction visible.
> - */
> -void odb_transaction_loose_commit(struct odb_transaction *transaction);

But this doesn't have to be public anymore as we have it available to us
via `tranasction->commit()`. Makes sense.

Patrick

Re: [PATCH 3/4] odb: prepare `struct odb_transaction` to support more sources

[Justin Tobler]

On 26/01/29 12:24PM, Patrick Steinhardt wrote:
> On Wed, Jan 28, 2026 at 05:45:18PM -0600, Justin Tobler wrote:
> 
> The bit about supporting "more sources" in the subject reads a bit weird
> to me. We still only handle a single source in a transaction, not
> multiple ones. I guess what you rather want to say is that we handle
> "generic" sources? How about:
> 
>     odb: prepare `struct odb_source` to become generic

The intent was that with generic transactions we it becomes possible to
support additional ODB sources each with their own transaction
implementation, but I agree the current wording is not great. I'll
update in the next version.

> > Each ODB transaction should be specific to the ODB source it pertains
> > to.
> 
> This is a claim that should probably be backed up a bit. I myself
> obviously agree with it, but I think it should be noted _why_ we want to
> have this in the first place.

Ya that's fair. I'll update the commit message in the next version to
properly explain the intent here.

> The patch itself looks as expected to me, as we split up `struct
> odb_transaction` into two structures:
> 
>   - `struct odb_transaction` continues to exist, but is now the generic
>     part that simply contains the source and a function pointer.
> 
>   - `struct odb_transaction_loose` is the backend-specific
>     implementation.
> 
> One question though: is this tansaction really specific to loose
> objects? We also seem to be handling packfiles there in
> `prepare_packfile_transaction()`, so it rather feels like this is
> specific to the whole "files" backend. I might be misunderstanding
> though.

The current transaction backend is primarily used to facilitate bulk
writing of what would otherwise be loose objects into a packfile. I
would like to eventually expand the use of ODB transactions though to
cover areas like git-recieve-pack(1) which currently uses tmp-objdir
directly. So it probably makes sense to call this odb_transaction_files.
Will update in the next version.

-Justin

[PATCH 4/4] odb: transparently handle common transaction behavior

[Justin Tobler]

A new ODB transaction is created and returned via
`odb_transaction_begin()` and stored in the ODB. Only a single
transaction may be pending at a time. If the ODB already has a
transaction, the function is expected to return NULL. Similarly, when
committing a transaction via `odb_transaction_commit()` the transaction
being committed must match the pending transaction and upon commit reset
the ODB transaction to NULL.

These behaviors apply regardless of the ODB transaction implementation.
Move the corresponding logic into `odb_transaction_{begin,commit}()`
accordingly.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c |  9 ---------
 odb.c         | 17 ++++++++++++++++-
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/object-file.c b/object-file.c
index e4739fc0cc..0def5efa91 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1994,15 +1994,8 @@ static void odb_transaction_loose_commit(struct odb_transaction *base)
 {
 	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
 
-	/*
-	 * Ensure the transaction ending matches the pending transaction.
-	 */
-	ASSERT(base == base->source->odb->transaction);
-
 	flush_loose_object_transaction(transaction);
 	flush_packfile_transaction(transaction);
-	base->source->odb->transaction = NULL;
-	free(transaction);
 }
 
 struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
@@ -2017,8 +2010,6 @@ struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
 	transaction->base.source = source;
 	transaction->base.commit = odb_transaction_loose_commit;
 
-	odb->transaction = &transaction->base;
-
 	return &transaction->base;
 }
 
diff --git a/odb.c b/odb.c
index 5b112f2464..332860735e 100644
--- a/odb.c
+++ b/odb.c
@@ -1153,7 +1153,15 @@ void odb_reprepare(struct object_database *o)
 
 struct odb_transaction *odb_transaction_begin(struct object_database *odb)
 {
-	return odb_transaction_loose_begin(odb->sources);
+	struct odb_transaction *transaction;
+
+	if (odb->transaction)
+		return NULL;
+
+	transaction = odb_transaction_loose_begin(odb->sources);
+	odb->transaction = transaction;
+
+	return transaction;
 }
 
 void odb_transaction_commit(struct odb_transaction *transaction)
@@ -1161,5 +1169,12 @@ void odb_transaction_commit(struct odb_transaction *transaction)
 	if (!transaction)
 		return;
 
+	/*
+	 * Ensure the transaction ending matches the pending transaction.
+	 */
+	ASSERT(transaction == transaction->source->odb->transaction);
+
 	transaction->commit(transaction);
+	transaction->source->odb->transaction = NULL;
+	free(transaction);
 }
-- 
2.52.0.373.g68cb7f9e92

Re: [PATCH 4/4] odb: transparently handle common transaction behavior

[Patrick Steinhardt]

On Wed, Jan 28, 2026 at 05:45:19PM -0600, Justin Tobler wrote:
> A new ODB transaction is created and returned via
> `odb_transaction_begin()` and stored in the ODB. Only a single
> transaction may be pending at a time. If the ODB already has a
> transaction, the function is expected to return NULL. Similarly, when
> committing a transaction via `odb_transaction_commit()` the transaction
> being committed must match the pending transaction and upon commit reset
> the ODB transaction to NULL.
> 
> These behaviors apply regardless of the ODB transaction implementation.
> Move the corresponding logic into `odb_transaction_{begin,commit}()`
> accordingly.

Besides deduplicating the logic, it also makes the ODB source only
care about itself, whereas the ODB is responsible for managing the
transaction's lifecycle. Which I think is a good thing.

> diff --git a/odb.c b/odb.c
> index 5b112f2464..332860735e 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -1153,7 +1153,15 @@ void odb_reprepare(struct object_database *o)
>  
>  struct odb_transaction *odb_transaction_begin(struct object_database *odb)
>  {
> -	return odb_transaction_loose_begin(odb->sources);
> +	struct odb_transaction *transaction;
> +
> +	if (odb->transaction)
> +		return NULL;
> +
> +	transaction = odb_transaction_loose_begin(odb->sources);
> +	odb->transaction = transaction;
> +
> +	return transaction;
>  }
>  
>  void odb_transaction_commit(struct odb_transaction *transaction)

Nit: the `transaction` variable seems somewhat pointless to me.

Patrick

[PATCH v2 0/4] odb: support ODB source specific transaction handling

[Justin Tobler]

Greetings,

ODB transactions provide a mechanism for writing new objects in the
repository that are in a pending state until they are committed. With
pluggable ODBs, each source will likely need to handle transactions
differently. This patch series prepares ODB transaction handling to be
defined for each ODB source by creating a generic odb_transaction
structure that stores a callback for committing the transaction.

For now, `struct odb_transaction_files` is the only transaction
implementation and what is always returned when creating a transaction.
Additionally, this series lifts transaction logic that is common to all
backends out of the implementation layer into
`odb_transaction_{begin,commit}()` layer.

Changes since V1:

- Renamed the files transaction type to be `odb_transaction_files`.
  Associated functions were also renamed accordingly.
- Updated commit messages.
- The `tmp_objdir` subsystem has been left unchange in the version. In a
  followup series, I plan to couple tmpdirs directly to ODB files
  transactions.

Thanks,
-Justin

Justin Tobler (4):
  odb: store ODB source in `struct odb_transaction`
  object-file: rename transaction functions
  odb: prepare `struct odb_transaction` to become generic
  odb: transparently handle common transaction behavior

 object-file.c | 77 +++++++++++++++++++++++++--------------------------
 object-file.h | 10 ++-----
 odb.c         | 19 +++++++++++--
 odb.h         | 17 ++++++++++++
 4 files changed, 74 insertions(+), 49 deletions(-)

Range-diff against v1:
1:  525c96f5f2 = 1:  525c96f5f2 odb: store ODB source in `struct odb_transaction`
2:  5ee1085453 ! 2:  02528f49fb object-file: rename transaction functions
    @@ Commit message
         In a subsequent commit, ODB transactions are made more generic to
         facilitate each ODB source providing its own transaction handling.
         Rename `object_file_transaction_{begin,commit}()` to
    -    `odb_transaction_loose_{begin,commit}()` to better match the future
    +    `odb_transaction_files_{begin,commit}()` to better match the future
         source specific transaction implementation.
     
         Signed-off-by: Justin Tobler <jltobler@gmail.com>
    @@ object-file.c: static void prepare_loose_object_transaction(struct odb_transacti
      	 * the first time an object might be added, since
      	 * callers may not know whether any objects will be
     -	 * added at the time they call object_file_transaction_begin.
    -+	 * added at the time they call odb_transaction_loose_begin.
    ++	 * added at the time they call odb_transaction_files_begin.
      	 */
      	if (!transaction || transaction->objdir)
      		return;
    @@ object-file.c: int read_loose_object(struct repository *repo,
      }
      
     -struct odb_transaction *object_file_transaction_begin(struct odb_source *source)
    -+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
    ++struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
      {
      	struct object_database *odb = source->odb;
      
    @@ object-file.c: struct odb_transaction *object_file_transaction_begin(struct odb_
      }
      
     -void object_file_transaction_commit(struct odb_transaction *transaction)
    -+void odb_transaction_loose_commit(struct odb_transaction *transaction)
    ++void odb_transaction_files_commit(struct odb_transaction *transaction)
      {
      	if (!transaction)
      		return;
    @@ object-file.h: struct odb_transaction;
      /*
       * Tell the object database to optimize for adding
     - * multiple objects. object_file_transaction_commit must be called
    -+ * multiple objects. odb_transaction_loose_commit must be called
    ++ * multiple objects. odb_transaction_files_commit must be called
       * to make new objects visible. If a transaction is already
       * pending, NULL is returned.
       */
     -struct odb_transaction *object_file_transaction_begin(struct odb_source *source);
    -+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);
    ++struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);
      
      /*
       * Tell the object database to make any objects from the
       * current transaction visible.
       */
     -void object_file_transaction_commit(struct odb_transaction *transaction);
    -+void odb_transaction_loose_commit(struct odb_transaction *transaction);
    ++void odb_transaction_files_commit(struct odb_transaction *transaction);
      
      #endif /* OBJECT_FILE_H */
     
    @@ odb.c: void odb_reprepare(struct object_database *o)
      struct odb_transaction *odb_transaction_begin(struct object_database *odb)
      {
     -	return object_file_transaction_begin(odb->sources);
    -+	return odb_transaction_loose_begin(odb->sources);
    ++	return odb_transaction_files_begin(odb->sources);
      }
      
      void odb_transaction_commit(struct odb_transaction *transaction)
      {
     -	object_file_transaction_commit(transaction);
    -+	odb_transaction_loose_commit(transaction);
    ++	odb_transaction_files_commit(transaction);
      }
3:  a06bfd8c55 ! 3:  081c82428b odb: prepare `struct odb_transaction` to support more sources
    @@ Metadata
     Author: Justin Tobler <jltobler@gmail.com>
     
      ## Commit message ##
    -    odb: prepare `struct odb_transaction` to support more sources
    +    odb: prepare `struct odb_transaction` to become generic
     
    -    Each ODB transaction should be specific to the ODB source it pertains
    -    to. Update `struct odb_transaction` to store a commit callback specific
    -    to its object source type. For now `struct odb_transaction_loose` is the
    +    An ODB transaction handles how objects are stored temporarily and
    +    eventually committed. Due to object storage being implemented
    +    differently for a given ODB source, the ODB transactions must be
    +    implemented in a manner specific to the source the objects are being
    +    written to. To provide generic transactions, `struct odb_transaction` is
    +    updated to store a commit callback that can be configured to support a
    +    specific ODB source. For now `struct odb_transaction_files` is the
         only transaction type and what is always returned when starting a
         transaction.
     
    @@ object-file.c: struct transaction_packfile {
      
     -struct odb_transaction {
     -	struct odb_source *source;
    -+struct odb_transaction_loose {
    ++struct odb_transaction_files {
     +	struct odb_transaction base;
      
      	struct tmp_objdir *objdir;
    @@ object-file.c: struct transaction_packfile {
     -static void prepare_loose_object_transaction(struct odb_transaction *transaction)
     +static void prepare_loose_object_transaction(struct odb_transaction *base)
      {
    -+	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
    ++	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
     +
      	/*
      	 * We lazily create the temporary object directory
    @@ object-file.c: static void prepare_loose_object_transaction(struct odb_transacti
     +static void fsync_loose_object_transaction(struct odb_transaction *base,
      					   int fd, const char *filename)
      {
    -+	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
    ++	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
     +
      	/*
      	 * If we have an active ODB transaction, we issue a call that
    @@ object-file.c: static void fsync_loose_object_transaction(struct odb_transaction
       * Cleanup after batch-mode fsync_object_files.
       */
     -static void flush_loose_object_transaction(struct odb_transaction *transaction)
    -+static void flush_loose_object_transaction(struct odb_transaction_loose *transaction)
    ++static void flush_loose_object_transaction(struct odb_transaction_files *transaction)
      {
      	struct strbuf temp_path = STRBUF_INIT;
      	struct tempfile *temp;
    @@ object-file.c: static int index_core(struct index_state *istate,
      }
      
     -static int already_written(struct odb_transaction *transaction,
    -+static int already_written(struct odb_transaction_loose *transaction,
    ++static int already_written(struct odb_transaction_files *transaction,
      			   struct object_id *oid)
      {
      	/* The object may already exist in the repository */
    @@ object-file.c: static int already_written(struct odb_transaction *transaction,
      
      /* Lazily create backing packfile for the state */
     -static void prepare_packfile_transaction(struct odb_transaction *transaction,
    -+static void prepare_packfile_transaction(struct odb_transaction_loose *transaction,
    ++static void prepare_packfile_transaction(struct odb_transaction_files *transaction,
      					 unsigned flags)
      {
      	struct transaction_packfile *state = &transaction->packfile;
    @@ object-file.c: static int stream_blob_to_pack(struct transaction_packfile *state
      }
      
     -static void flush_packfile_transaction(struct odb_transaction *transaction)
    -+static void flush_packfile_transaction(struct odb_transaction_loose *transaction)
    ++static void flush_packfile_transaction(struct odb_transaction_files *transaction)
      {
      	struct transaction_packfile *state = &transaction->packfile;
     -	struct repository *repo = transaction->source->odb->repo;
    @@ object-file.c: static void flush_packfile_transaction(struct odb_transaction *tr
       * callers should avoid this code path when filters are requested.
       */
     -static int index_blob_packfile_transaction(struct odb_transaction *transaction,
    -+static int index_blob_packfile_transaction(struct odb_transaction_loose *transaction,
    ++static int index_blob_packfile_transaction(struct odb_transaction_files *transaction,
      					   struct object_id *result_oid, int fd,
      					   size_t size, const char *path,
      					   unsigned flags)
    @@ object-file.c: int index_fd(struct index_state *istate, struct object_id *oid,
     -		transaction = odb_transaction_begin(the_repository->objects);
     -		ret = index_blob_packfile_transaction(the_repository->objects->transaction,
     +		transaction = odb_transaction_begin(odb);
    -+		ret = index_blob_packfile_transaction((struct odb_transaction_loose *)odb->transaction,
    ++		ret = index_blob_packfile_transaction((struct odb_transaction_files *)odb->transaction,
      						      oid, fd,
      						      xsize_t(st->st_size),
      						      path, flags);
    @@ object-file.c: int read_loose_object(struct repository *repo,
      	return ret;
      }
      
    --struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
    -+static void odb_transaction_loose_commit(struct odb_transaction *base)
    +-struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
    ++static void odb_transaction_files_commit(struct odb_transaction *base)
      {
     -	struct object_database *odb = source->odb;
     -
    @@ object-file.c: int read_loose_object(struct repository *repo,
     -	return odb->transaction;
     -}
     -
    --void odb_transaction_loose_commit(struct odb_transaction *transaction)
    +-void odb_transaction_files_commit(struct odb_transaction *transaction)
     -{
     -	if (!transaction)
     -		return;
    -+	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
    ++	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
      
      	/*
      	 * Ensure the transaction ending matches the pending transaction.
    @@ object-file.c: int read_loose_object(struct repository *repo,
      	free(transaction);
      }
      
    -+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
    ++struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
     +{
    -+	struct odb_transaction_loose *transaction;
    ++	struct odb_transaction_files *transaction;
     +	struct object_database *odb = source->odb;
     +
     +	if (odb->transaction)
    @@ object-file.c: int read_loose_object(struct repository *repo,
     +
     +	transaction = xcalloc(1, sizeof(*transaction));
     +	transaction->base.source = source;
    -+	transaction->base.commit = odb_transaction_loose_commit;
    ++	transaction->base.commit = odb_transaction_files_commit;
     +
     +	odb->transaction = &transaction->base;
     +
    @@ object-file.c: int read_loose_object(struct repository *repo,
      ## object-file.h ##
     @@ object-file.h: struct odb_transaction;
       */
    - struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);
    + struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);
      
     -/*
     - * Tell the object database to make any objects from the
     - * current transaction visible.
     - */
    --void odb_transaction_loose_commit(struct odb_transaction *transaction);
    +-void odb_transaction_files_commit(struct odb_transaction *transaction);
     -
      #endif /* OBJECT_FILE_H */
     
    @@ odb.c: struct odb_transaction *odb_transaction_begin(struct object_database *odb
      
      void odb_transaction_commit(struct odb_transaction *transaction)
      {
    --	odb_transaction_loose_commit(transaction);
    +-	odb_transaction_files_commit(transaction);
     +	if (!transaction)
     +		return;
     +
4:  476ed1235c ! 4:  ed8164a3a3 odb: transparently handle common transaction behavior
    @@ Commit message
         Signed-off-by: Justin Tobler <jltobler@gmail.com>
     
      ## object-file.c ##
    -@@ object-file.c: static void odb_transaction_loose_commit(struct odb_transaction *base)
    +@@ object-file.c: static void odb_transaction_files_commit(struct odb_transaction *base)
      {
    - 	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
    + 	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
      
     -	/*
     -	 * Ensure the transaction ending matches the pending transaction.
    @@ object-file.c: static void odb_transaction_loose_commit(struct odb_transaction *
     -	free(transaction);
      }
      
    - struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
    -@@ object-file.c: struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
    + struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
    +@@ object-file.c: struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
      	transaction->base.source = source;
    - 	transaction->base.commit = odb_transaction_loose_commit;
    + 	transaction->base.commit = odb_transaction_files_commit;
      
     -	odb->transaction = &transaction->base;
     -
    @@ odb.c: void odb_reprepare(struct object_database *o)
      
      struct odb_transaction *odb_transaction_begin(struct object_database *odb)
      {
    --	return odb_transaction_loose_begin(odb->sources);
    -+	struct odb_transaction *transaction;
    -+
    +-	return odb_transaction_files_begin(odb->sources);
     +	if (odb->transaction)
     +		return NULL;
     +
    -+	transaction = odb_transaction_loose_begin(odb->sources);
    -+	odb->transaction = transaction;
    ++	odb->transaction = odb_transaction_files_begin(odb->sources);
     +
    -+	return transaction;
    ++	return odb->transaction;
      }
      
      void odb_transaction_commit(struct odb_transaction *transaction)

base-commit: ea717645d199f6f1b66058886475db3e8c9330e9
-- 
2.52.0.373.g68cb7f9e92

[PATCH v2 1/4] odb: store ODB source in `struct odb_transaction`

[Justin Tobler]

Each `struct odb_transaction` currently stores a reference to the
`struct object_database`. Since transactions are handled per object
source, instead store a reference to the source.

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

diff --git a/object-file.c b/object-file.c
index e7e4c3348f..196509b252 100644
--- a/object-file.c
+++ b/object-file.c
@@ -711,7 +711,7 @@ struct transaction_packfile {
 };
 
 struct odb_transaction {
-	struct object_database *odb;
+	struct odb_source *source;
 
 	struct tmp_objdir *objdir;
 	struct transaction_packfile packfile;
@@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
 	if (!transaction || transaction->objdir)
 		return;
 
-	transaction->objdir = tmp_objdir_create(transaction->odb->repo, "bulk-fsync");
+	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
 	if (transaction->objdir)
 		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
 }
@@ -772,7 +772,7 @@ static void flush_loose_object_transaction(struct odb_transaction *transaction)
 	 * the final name is visible.
 	 */
 	strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
-		    repo_get_object_directory(transaction->odb->repo));
+		    repo_get_object_directory(transaction->source->odb->repo));
 	temp = xmks_tempfile(temp_path.buf);
 	fsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));
 	delete_tempfile(&temp);
@@ -1344,7 +1344,7 @@ static int already_written(struct odb_transaction *transaction,
 			   struct object_id *oid)
 {
 	/* The object may already exist in the repository */
-	if (odb_has_object(transaction->odb, oid,
+	if (odb_has_object(transaction->source->odb, oid,
 			   HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))
 		return 1;
 
@@ -1365,7 +1365,7 @@ static void prepare_packfile_transaction(struct odb_transaction *transaction,
 	if (!(flags & INDEX_WRITE_OBJECT) || state->f)
 		return;
 
-	state->f = create_tmp_packfile(transaction->odb->repo,
+	state->f = create_tmp_packfile(transaction->source->odb->repo,
 				       &state->pack_tmp_name);
 	reset_pack_idx_option(&state->pack_idx_opts);
 
@@ -1469,7 +1469,7 @@ static int stream_blob_to_pack(struct transaction_packfile *state,
 static void flush_packfile_transaction(struct odb_transaction *transaction)
 {
 	struct transaction_packfile *state = &transaction->packfile;
-	struct repository *repo = transaction->odb->repo;
+	struct repository *repo = transaction->source->odb->repo;
 	unsigned char hash[GIT_MAX_RAWSZ];
 	struct strbuf packname = STRBUF_INIT;
 	char *idx_tmp_name = NULL;
@@ -1494,7 +1494,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)
 	}
 
 	strbuf_addf(&packname, "%s/pack/pack-%s.",
-		    repo_get_object_directory(transaction->odb->repo),
+		    repo_get_object_directory(transaction->source->odb->repo),
 		    hash_to_hex_algop(hash, repo->hash_algo));
 
 	stage_tmp_packfiles(repo, &packname, state->pack_tmp_name,
@@ -1553,7 +1553,7 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,
 
 	header_len = format_object_header((char *)obuf, sizeof(obuf),
 					  OBJ_BLOB, size);
-	transaction->odb->repo->hash_algo->init_fn(&ctx);
+	transaction->source->odb->repo->hash_algo->init_fn(&ctx);
 	git_hash_update(&ctx, obuf, header_len);
 
 	/* Note: idx is non-NULL when we are writing */
@@ -1993,7 +1993,7 @@ struct odb_transaction *object_file_transaction_begin(struct odb_source *source)
 		return NULL;
 
 	CALLOC_ARRAY(odb->transaction, 1);
-	odb->transaction->odb = odb;
+	odb->transaction->source = source;
 
 	return odb->transaction;
 }
@@ -2006,11 +2006,11 @@ void object_file_transaction_commit(struct odb_transaction *transaction)
 	/*
 	 * Ensure the transaction ending matches the pending transaction.
 	 */
-	ASSERT(transaction == transaction->odb->transaction);
+	ASSERT(transaction == transaction->source->odb->transaction);
 
 	flush_loose_object_transaction(transaction);
 	flush_packfile_transaction(transaction);
-	transaction->odb->transaction = NULL;
+	transaction->source->odb->transaction = NULL;
 	free(transaction);
 }
 
-- 
2.52.0.373.g68cb7f9e92

[PATCH v2 2/4] object-file: rename transaction functions

[Justin Tobler]

In a subsequent commit, ODB transactions are made more generic to
facilitate each ODB source providing its own transaction handling.
Rename `object_file_transaction_{begin,commit}()` to
`odb_transaction_files_{begin,commit}()` to better match the future
source specific transaction implementation.

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

diff --git a/object-file.c b/object-file.c
index 196509b252..7b34a2b274 100644
--- a/object-file.c
+++ b/object-file.c
@@ -723,7 +723,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
 	 * We lazily create the temporary object directory
 	 * the first time an object might be added, since
 	 * callers may not know whether any objects will be
-	 * added at the time they call object_file_transaction_begin.
+	 * added at the time they call odb_transaction_files_begin.
 	 */
 	if (!transaction || transaction->objdir)
 		return;
@@ -1985,7 +1985,7 @@ int read_loose_object(struct repository *repo,
 	return ret;
 }
 
-struct odb_transaction *object_file_transaction_begin(struct odb_source *source)
+struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
 {
 	struct object_database *odb = source->odb;
 
@@ -1998,7 +1998,7 @@ struct odb_transaction *object_file_transaction_begin(struct odb_source *source)
 	return odb->transaction;
 }
 
-void object_file_transaction_commit(struct odb_transaction *transaction)
+void odb_transaction_files_commit(struct odb_transaction *transaction)
 {
 	if (!transaction)
 		return;
diff --git a/object-file.h b/object-file.h
index 1229d5f675..b4a3341a89 100644
--- a/object-file.h
+++ b/object-file.h
@@ -202,16 +202,16 @@ struct odb_transaction;
 
 /*
  * Tell the object database to optimize for adding
- * multiple objects. object_file_transaction_commit must be called
+ * multiple objects. odb_transaction_files_commit must be called
  * to make new objects visible. If a transaction is already
  * pending, NULL is returned.
  */
-struct odb_transaction *object_file_transaction_begin(struct odb_source *source);
+struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);
 
 /*
  * Tell the object database to make any objects from the
  * current transaction visible.
  */
-void object_file_transaction_commit(struct odb_transaction *transaction);
+void odb_transaction_files_commit(struct odb_transaction *transaction);
 
 #endif /* OBJECT_FILE_H */
diff --git a/odb.c b/odb.c
index ac70b6a099..a5e6fd01a9 100644
--- a/odb.c
+++ b/odb.c
@@ -1153,10 +1153,10 @@ void odb_reprepare(struct object_database *o)
 
 struct odb_transaction *odb_transaction_begin(struct object_database *odb)
 {
-	return object_file_transaction_begin(odb->sources);
+	return odb_transaction_files_begin(odb->sources);
 }
 
 void odb_transaction_commit(struct odb_transaction *transaction)
 {
-	object_file_transaction_commit(transaction);
+	odb_transaction_files_commit(transaction);
 }
-- 
2.52.0.373.g68cb7f9e92

[PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

[Justin Tobler]

An ODB transaction handles how objects are stored temporarily and
eventually committed. Due to object storage being implemented
differently for a given ODB source, the ODB transactions must be
implemented in a manner specific to the source the objects are being
written to. To provide generic transactions, `struct odb_transaction` is
updated to store a commit callback that can be configured to support a
specific ODB source. For now `struct odb_transaction_files` is the
only transaction type and what is always returned when starting a
transaction.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c | 80 ++++++++++++++++++++++++++++-----------------------
 object-file.h |  6 ----
 odb.c         |  5 +++-
 odb.h         | 17 +++++++++++
 4 files changed, 65 insertions(+), 43 deletions(-)

diff --git a/object-file.c b/object-file.c
index 7b34a2b274..d7e153c1b9 100644
--- a/object-file.c
+++ b/object-file.c
@@ -710,15 +710,17 @@ struct transaction_packfile {
 	uint32_t nr_written;
 };
 
-struct odb_transaction {
-	struct odb_source *source;
+struct odb_transaction_files {
+	struct odb_transaction base;
 
 	struct tmp_objdir *objdir;
 	struct transaction_packfile packfile;
 };
 
-static void prepare_loose_object_transaction(struct odb_transaction *transaction)
+static void prepare_loose_object_transaction(struct odb_transaction *base)
 {
+	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
+
 	/*
 	 * We lazily create the temporary object directory
 	 * the first time an object might be added, since
@@ -728,14 +730,16 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
 	if (!transaction || transaction->objdir)
 		return;
 
-	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
+	transaction->objdir = tmp_objdir_create(base->source->odb->repo, "bulk-fsync");
 	if (transaction->objdir)
 		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
 }
 
-static void fsync_loose_object_transaction(struct odb_transaction *transaction,
+static void fsync_loose_object_transaction(struct odb_transaction *base,
 					   int fd, const char *filename)
 {
+	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
+
 	/*
 	 * If we have an active ODB transaction, we issue a call that
 	 * cleans the filesystem page cache but avoids a hardware flush
@@ -754,7 +758,7 @@ static void fsync_loose_object_transaction(struct odb_transaction *transaction,
 /*
  * Cleanup after batch-mode fsync_object_files.
  */
-static void flush_loose_object_transaction(struct odb_transaction *transaction)
+static void flush_loose_object_transaction(struct odb_transaction_files *transaction)
 {
 	struct strbuf temp_path = STRBUF_INIT;
 	struct tempfile *temp;
@@ -772,7 +776,7 @@ static void flush_loose_object_transaction(struct odb_transaction *transaction)
 	 * the final name is visible.
 	 */
 	strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
-		    repo_get_object_directory(transaction->source->odb->repo));
+		    repo_get_object_directory(transaction->base.source->odb->repo));
 	temp = xmks_tempfile(temp_path.buf);
 	fsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));
 	delete_tempfile(&temp);
@@ -1340,11 +1344,11 @@ static int index_core(struct index_state *istate,
 	return ret;
 }
 
-static int already_written(struct odb_transaction *transaction,
+static int already_written(struct odb_transaction_files *transaction,
 			   struct object_id *oid)
 {
 	/* The object may already exist in the repository */
-	if (odb_has_object(transaction->source->odb, oid,
+	if (odb_has_object(transaction->base.source->odb, oid,
 			   HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))
 		return 1;
 
@@ -1358,14 +1362,14 @@ static int already_written(struct odb_transaction *transaction,
 }
 
 /* Lazily create backing packfile for the state */
-static void prepare_packfile_transaction(struct odb_transaction *transaction,
+static void prepare_packfile_transaction(struct odb_transaction_files *transaction,
 					 unsigned flags)
 {
 	struct transaction_packfile *state = &transaction->packfile;
 	if (!(flags & INDEX_WRITE_OBJECT) || state->f)
 		return;
 
-	state->f = create_tmp_packfile(transaction->source->odb->repo,
+	state->f = create_tmp_packfile(transaction->base.source->odb->repo,
 				       &state->pack_tmp_name);
 	reset_pack_idx_option(&state->pack_idx_opts);
 
@@ -1466,10 +1470,10 @@ static int stream_blob_to_pack(struct transaction_packfile *state,
 	return 0;
 }
 
-static void flush_packfile_transaction(struct odb_transaction *transaction)
+static void flush_packfile_transaction(struct odb_transaction_files *transaction)
 {
 	struct transaction_packfile *state = &transaction->packfile;
-	struct repository *repo = transaction->source->odb->repo;
+	struct repository *repo = transaction->base.source->odb->repo;
 	unsigned char hash[GIT_MAX_RAWSZ];
 	struct strbuf packname = STRBUF_INIT;
 	char *idx_tmp_name = NULL;
@@ -1494,7 +1498,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)
 	}
 
 	strbuf_addf(&packname, "%s/pack/pack-%s.",
-		    repo_get_object_directory(transaction->source->odb->repo),
+		    repo_get_object_directory(transaction->base.source->odb->repo),
 		    hash_to_hex_algop(hash, repo->hash_algo));
 
 	stage_tmp_packfiles(repo, &packname, state->pack_tmp_name,
@@ -1534,7 +1538,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)
  * binary blobs, they generally do not want to get any conversion, and
  * callers should avoid this code path when filters are requested.
  */
-static int index_blob_packfile_transaction(struct odb_transaction *transaction,
+static int index_blob_packfile_transaction(struct odb_transaction_files *transaction,
 					   struct object_id *result_oid, int fd,
 					   size_t size, const char *path,
 					   unsigned flags)
@@ -1553,7 +1557,7 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,
 
 	header_len = format_object_header((char *)obuf, sizeof(obuf),
 					  OBJ_BLOB, size);
-	transaction->source->odb->repo->hash_algo->init_fn(&ctx);
+	transaction->base.source->odb->repo->hash_algo->init_fn(&ctx);
 	git_hash_update(&ctx, obuf, header_len);
 
 	/* Note: idx is non-NULL when we are writing */
@@ -1629,10 +1633,11 @@ int index_fd(struct index_state *istate, struct object_id *oid,
 		ret = index_core(istate, oid, fd, xsize_t(st->st_size),
 				 type, path, flags);
 	} else {
+		struct object_database *odb = the_repository->objects;
 		struct odb_transaction *transaction;
 
-		transaction = odb_transaction_begin(the_repository->objects);
-		ret = index_blob_packfile_transaction(the_repository->objects->transaction,
+		transaction = odb_transaction_begin(odb);
+		ret = index_blob_packfile_transaction((struct odb_transaction_files *)odb->transaction,
 						      oid, fd,
 						      xsize_t(st->st_size),
 						      path, flags);
@@ -1985,35 +1990,38 @@ int read_loose_object(struct repository *repo,
 	return ret;
 }
 
-struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
+static void odb_transaction_files_commit(struct odb_transaction *base)
 {
-	struct object_database *odb = source->odb;
-
-	if (odb->transaction)
-		return NULL;
-
-	CALLOC_ARRAY(odb->transaction, 1);
-	odb->transaction->source = source;
-
-	return odb->transaction;
-}
-
-void odb_transaction_files_commit(struct odb_transaction *transaction)
-{
-	if (!transaction)
-		return;
+	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
 
 	/*
 	 * Ensure the transaction ending matches the pending transaction.
 	 */
-	ASSERT(transaction == transaction->source->odb->transaction);
+	ASSERT(base == base->source->odb->transaction);
 
 	flush_loose_object_transaction(transaction);
 	flush_packfile_transaction(transaction);
-	transaction->source->odb->transaction = NULL;
+	base->source->odb->transaction = NULL;
 	free(transaction);
 }
 
+struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
+{
+	struct odb_transaction_files *transaction;
+	struct object_database *odb = source->odb;
+
+	if (odb->transaction)
+		return NULL;
+
+	transaction = xcalloc(1, sizeof(*transaction));
+	transaction->base.source = source;
+	transaction->base.commit = odb_transaction_files_commit;
+
+	odb->transaction = &transaction->base;
+
+	return &transaction->base;
+}
+
 struct odb_source_loose *odb_source_loose_new(struct odb_source *source)
 {
 	struct odb_source_loose *loose;
diff --git a/object-file.h b/object-file.h
index b4a3341a89..a62d0de394 100644
--- a/object-file.h
+++ b/object-file.h
@@ -208,10 +208,4 @@ struct odb_transaction;
  */
 struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);
 
-/*
- * Tell the object database to make any objects from the
- * current transaction visible.
- */
-void odb_transaction_files_commit(struct odb_transaction *transaction);
-
 #endif /* OBJECT_FILE_H */
diff --git a/odb.c b/odb.c
index a5e6fd01a9..349b4218a5 100644
--- a/odb.c
+++ b/odb.c
@@ -1158,5 +1158,8 @@ struct odb_transaction *odb_transaction_begin(struct object_database *odb)
 
 void odb_transaction_commit(struct odb_transaction *transaction)
 {
-	odb_transaction_files_commit(transaction);
+	if (!transaction)
+		return;
+
+	transaction->commit(transaction);
 }
diff --git a/odb.h b/odb.h
index bab07755f4..83d3a37805 100644
--- a/odb.h
+++ b/odb.h
@@ -77,7 +77,24 @@ struct odb_source {
 struct packed_git;
 struct packfile_store;
 struct cached_object_entry;
+
+/*
+ * A transaction may be started for an object database prior to writing new
+ * objects via odb_transaction_begin(). These objects are not committed until
+ * odb_transaction_commit() is invoked. Only a single transaction may be pending
+ * at a time.
+ *
+ * Each ODB source is expected to implement its own transaction handling.
+ */
 struct odb_transaction;
+typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);
+struct odb_transaction {
+	/* The ODB source the transaction is opened against. */
+	struct odb_source *source;
+
+	/* The ODB source specific callback invoked to commit a transaction. */
+	odb_transaction_commit_fn commit;
+};
 
 /*
  * The object database encapsulates access to objects in a repository. It
-- 
2.52.0.373.g68cb7f9e92

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

[Toon Claes]

Justin Tobler <jltobler@gmail.com> writes:

> An ODB transaction handles how objects are stored temporarily and
> eventually committed. Due to object storage being implemented
> differently for a given ODB source, the ODB transactions must be
> implemented in a manner specific to the source the objects are being
> written to. To provide generic transactions, `struct odb_transaction` is
> updated to store a commit callback that can be configured to support a
> specific ODB source. For now `struct odb_transaction_files` is the
> only transaction type and what is always returned when starting a
> transaction.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  object-file.c | 80 ++++++++++++++++++++++++++++-----------------------
>  object-file.h |  6 ----
>  odb.c         |  5 +++-
>  odb.h         | 17 +++++++++++
>  4 files changed, 65 insertions(+), 43 deletions(-)
>
> diff --git a/object-file.c b/object-file.c
> index 7b34a2b274..d7e153c1b9 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -710,15 +710,17 @@ struct transaction_packfile {
>  	uint32_t nr_written;
>  };
>  
> -struct odb_transaction {
> -	struct odb_source *source;
> +struct odb_transaction_files {
> +	struct odb_transaction base;
>  
>  	struct tmp_objdir *objdir;
>  	struct transaction_packfile packfile;
>  };
>  
> -static void prepare_loose_object_transaction(struct odb_transaction *transaction)
> +static void prepare_loose_object_transaction(struct odb_transaction *base)
>  {
> +	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;

So you're assuming `struct odb_transaction` is the first field in
`struct odb_transaction_files`?

I think it would be safer to do this instead:

+	struct odb_transaction_files *transaction =
+		container_of(base, struct odb_transaction_files, base);

(this also can be applied in a few other places in this patch)

-- 
Cheers,
Toon

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

[Justin Tobler]

On 26/02/03 04:54PM, Toon Claes wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> > -static void prepare_loose_object_transaction(struct odb_transaction *transaction)
> > +static void prepare_loose_object_transaction(struct odb_transaction *base)
> >  {
> > +	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
> 
> So you're assuming `struct odb_transaction` is the first field in
> `struct odb_transaction_files`?

Yes, `struct odb_transaction_files` contains a reference to `struct
odb_transaction` as its first member which is the "base" transaction
type. This way we can easily cast to the containing "concrete" type.

> I think it would be safer to do this instead:
> 
> +	struct odb_transaction_files *transaction =
> +		container_of(base, struct odb_transaction_files, base);
> 
> (this also can be applied in a few other places in this patch)

So long as `struct odb_transaction` remains the first member of `struct
odb_transaction_files`, using `container_of()` is functionally the same
thing as casting. This is also how we do things for `odb_read_stream`
and `odb_loose_read_stream` so I don't think it is too big of a deal
either way.

I've made the change locally, but will hold off from sending another
version unless folks feel strongly about this change or there is other
feedback.

Thanks,
-Justin

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> So long as `struct odb_transaction` remains the first member of `struct
> odb_transaction_files`, using `container_of()` is functionally the same
> thing as casting. This is also how we do things for `odb_read_stream`
> and `odb_loose_read_stream` so I don't think it is too big of a deal
> either way.

It would be a good theme to clean them all up, together with
existing ones, in a separate topic.  Code hygiene matters.

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

[Patrick Steinhardt]

On Tue, Feb 03, 2026 at 02:54:08PM -0800, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > So long as `struct odb_transaction` remains the first member of `struct
> > odb_transaction_files`, using `container_of()` is functionally the same
> > thing as casting. This is also how we do things for `odb_read_stream`
> > and `odb_loose_read_stream` so I don't think it is too big of a deal
> > either way.
> 
> It would be a good theme to clean them all up, together with
> existing ones, in a separate topic.  Code hygiene matters.

Fair indeed. Justin, will you handle this cleanup once this topic here
is merged or shall I do it?

Patrick

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

[Justin Tobler]

On 26/02/04 07:26AM, Patrick Steinhardt wrote:
> On Tue, Feb 03, 2026 at 02:54:08PM -0800, Junio C Hamano wrote:
> > Justin Tobler <jltobler@gmail.com> writes:
> > 
> > > So long as `struct odb_transaction` remains the first member of `struct
> > > odb_transaction_files`, using `container_of()` is functionally the same
> > > thing as casting. This is also how we do things for `odb_read_stream`
> > > and `odb_loose_read_stream` so I don't think it is too big of a deal
> > > either way.
> > 
> > It would be a good theme to clean them all up, together with
> > existing ones, in a separate topic.  Code hygiene matters.
> 
> Fair indeed. Justin, will you handle this cleanup once this topic here
> is merged or shall I do it?

I've already have a patch that addresses `odb_transaction_files`
and `odb_loose_read_stream`. I'll check to see if there are any others I
notice and send a followup series after this one gets merged.

-Justin

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1098 bytes --]

Justin Tobler <jltobler@gmail.com> writes:

[snip]

> +
> +/*
> + * A transaction may be started for an object database prior to writing new
> + * objects via odb_transaction_begin(). These objects are not committed until
> + * odb_transaction_commit() is invoked. Only a single transaction may be pending
> + * at a time.
> + *
> + * Each ODB source is expected to implement its own transaction handling.
> + */
>  struct odb_transaction;

Nit: Wouldn't it be nicer to just the below `struct odb_transaction`
here and drop this line?

> +typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);
> +struct odb_transaction {
> +	/* The ODB source the transaction is opened against. */
> +	struct odb_source *source;
> +
> +	/* The ODB source specific callback invoked to commit a transaction. */
> +	odb_transaction_commit_fn commit;
> +};
>
>  /*
>   * The object database encapsulates access to objects in a repository. It
> --
> 2.52.0.373.g68cb7f9e92

Just a question in general, is the idea to eventually also add support
for {prepare, rollback} within odb transactions?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

[Justin Tobler]

On 26/02/04 02:31AM, Karthik Nayak wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> [snip]
> 
> > +
> > +/*
> > + * A transaction may be started for an object database prior to writing new
> > + * objects via odb_transaction_begin(). These objects are not committed until
> > + * odb_transaction_commit() is invoked. Only a single transaction may be pending
> > + * at a time.
> > + *
> > + * Each ODB source is expected to implement its own transaction handling.
> > + */
> >  struct odb_transaction;
> 
> Nit: Wouldn't it be nicer to just the below `struct odb_transaction`
> here and drop this line?

I assume you mean drop the typedef in favor of defining it directly in
the struct and thus removing the need for the forward declation. I kind
of like having a typedef for the function callback, but I don't feel too
strongly either way. I've ammended locally, but will hold off from
sending another version unless there is other feedback.

> > +typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);
> > +struct odb_transaction {
> > +	/* The ODB source the transaction is opened against. */
> > +	struct odb_source *source;
> > +
> > +	/* The ODB source specific callback invoked to commit a transaction. */
> > +	odb_transaction_commit_fn commit;
> > +};
> >
> >  /*
> >   * The object database encapsulates access to objects in a repository. It
> > --
> > 2.52.0.373.g68cb7f9e92
> 
> Just a question in general, is the idea to eventually also add support
> for {prepare, rollback} within odb transactions?

I'm not quite sure yet about "prepare", but certainly an "abort" or
"rollback" will be introduced in a followup series. This will be useful
as we expand ODB transaction usage to other operations that require the
ability to remove temporary objects such as remerge-diffs.

Thanks,
-Justin

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1938 bytes --]

Justin Tobler <jltobler@gmail.com> writes:

> On 26/02/04 02:31AM, Karthik Nayak wrote:
>> Justin Tobler <jltobler@gmail.com> writes:
>>
>> [snip]
>>
>> > +
>> > +/*
>> > + * A transaction may be started for an object database prior to writing new
>> > + * objects via odb_transaction_begin(). These objects are not committed until
>> > + * odb_transaction_commit() is invoked. Only a single transaction may be pending
>> > + * at a time.
>> > + *
>> > + * Each ODB source is expected to implement its own transaction handling.
>> > + */
>> >  struct odb_transaction;
>>
>> Nit: Wouldn't it be nicer to just the below `struct odb_transaction`
>> here and drop this line?
>
> I assume you mean drop the typedef in favor of defining it directly in
> the struct and thus removing the need for the forward declation. I kind
> of like having a typedef for the function callback, but I don't feel too
> strongly either way. I've ammended locally, but will hold off from
> sending another version unless there is other feedback.
>

All good.

>> > +typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);
>> > +struct odb_transaction {
>> > +	/* The ODB source the transaction is opened against. */
>> > +	struct odb_source *source;
>> > +
>> > +	/* The ODB source specific callback invoked to commit a transaction. */
>> > +	odb_transaction_commit_fn commit;
>> > +};
>> >
>> >  /*
>> >   * The object database encapsulates access to objects in a repository. It
>> > --
>> > 2.52.0.373.g68cb7f9e92
>>
>> Just a question in general, is the idea to eventually also add support
>> for {prepare, rollback} within odb transactions?
>
> I'm not quite sure yet about "prepare", but certainly an "abort" or
> "rollback" will be introduced in a followup series. This will be useful
> as we expand ODB transaction usage to other operations that require the
> ability to remove temporary objects such as remerge-diffs.
>

Alright.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v2 4/4] odb: transparently handle common transaction behavior

[Justin Tobler]

A new ODB transaction is created and returned via
`odb_transaction_begin()` and stored in the ODB. Only a single
transaction may be pending at a time. If the ODB already has a
transaction, the function is expected to return NULL. Similarly, when
committing a transaction via `odb_transaction_commit()` the transaction
being committed must match the pending transaction and upon commit reset
the ODB transaction to NULL.

These behaviors apply regardless of the ODB transaction implementation.
Move the corresponding logic into `odb_transaction_{begin,commit}()`
accordingly.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c |  9 ---------
 odb.c         | 14 +++++++++++++-
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/object-file.c b/object-file.c
index d7e153c1b9..1b62996ef0 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1994,15 +1994,8 @@ static void odb_transaction_files_commit(struct odb_transaction *base)
 {
 	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
 
-	/*
-	 * Ensure the transaction ending matches the pending transaction.
-	 */
-	ASSERT(base == base->source->odb->transaction);
-
 	flush_loose_object_transaction(transaction);
 	flush_packfile_transaction(transaction);
-	base->source->odb->transaction = NULL;
-	free(transaction);
 }
 
 struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
@@ -2017,8 +2010,6 @@ struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
 	transaction->base.source = source;
 	transaction->base.commit = odb_transaction_files_commit;
 
-	odb->transaction = &transaction->base;
-
 	return &transaction->base;
 }
 
diff --git a/odb.c b/odb.c
index 349b4218a5..1679cc0465 100644
--- a/odb.c
+++ b/odb.c
@@ -1153,7 +1153,12 @@ void odb_reprepare(struct object_database *o)
 
 struct odb_transaction *odb_transaction_begin(struct object_database *odb)
 {
-	return odb_transaction_files_begin(odb->sources);
+	if (odb->transaction)
+		return NULL;
+
+	odb->transaction = odb_transaction_files_begin(odb->sources);
+
+	return odb->transaction;
 }
 
 void odb_transaction_commit(struct odb_transaction *transaction)
@@ -1161,5 +1166,12 @@ void odb_transaction_commit(struct odb_transaction *transaction)
 	if (!transaction)
 		return;
 
+	/*
+	 * Ensure the transaction ending matches the pending transaction.
+	 */
+	ASSERT(transaction == transaction->source->odb->transaction);
+
 	transaction->commit(transaction);
+	transaction->source->odb->transaction = NULL;
+	free(transaction);
 }
-- 
2.52.0.373.g68cb7f9e92

Re: [PATCH v2 4/4] odb: transparently handle common transaction behavior

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 839 bytes --]

Justin Tobler <jltobler@gmail.com> writes:

> A new ODB transaction is created and returned via
> `odb_transaction_begin()` and stored in the ODB. Only a single
> transaction may be pending at a time. If the ODB already has a
> transaction, the function is expected to return NULL. Similarly, when
> committing a transaction via `odb_transaction_commit()` the transaction
> being committed must match the pending transaction and upon commit reset
> the ODB transaction to NULL.
>

But isn't this merely a limitation of the current implementation of the
files transactions? Couldn't a potential ODB source support parallel
transactions where this might no longer hold?

> These behaviors apply regardless of the ODB transaction implementation.
> Move the corresponding logic into `odb_transaction_{begin,commit}()`
> accordingly.
>

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v2 4/4] odb: transparently handle common transaction behavior

[Justin Tobler]

On 26/02/04 10:34AM, Karthik Nayak wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > A new ODB transaction is created and returned via
> > `odb_transaction_begin()` and stored in the ODB. Only a single
> > transaction may be pending at a time. If the ODB already has a
> > transaction, the function is expected to return NULL. Similarly, when
> > committing a transaction via `odb_transaction_commit()` the transaction
> > being committed must match the pending transaction and upon commit reset
> > the ODB transaction to NULL.
> 
> But isn't this merely a limitation of the current implementation of the
> files transactions? Couldn't a potential ODB source support parallel
> transactions where this might no longer hold?

Just to clarify, this limitation exists per Git process. For the time
being, we only support writing objects to a single ODB source so a
single transaction for object writes seems reasonable for now.
Furthermore, the current "files" transaction backend relies on the
tmp_odjdir subsystem which means only a single temp odjdir may exist for
a Git process to write objects to. 

-Justin

Re: [PATCH v2 4/4] odb: transparently handle common transaction behavior

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1236 bytes --]

Justin Tobler <jltobler@gmail.com> writes:

> On 26/02/04 10:34AM, Karthik Nayak wrote:
>> Justin Tobler <jltobler@gmail.com> writes:
>>
>> > A new ODB transaction is created and returned via
>> > `odb_transaction_begin()` and stored in the ODB. Only a single
>> > transaction may be pending at a time. If the ODB already has a
>> > transaction, the function is expected to return NULL. Similarly, when
>> > committing a transaction via `odb_transaction_commit()` the transaction
>> > being committed must match the pending transaction and upon commit reset
>> > the ODB transaction to NULL.
>>
>> But isn't this merely a limitation of the current implementation of the
>> files transactions? Couldn't a potential ODB source support parallel
>> transactions where this might no longer hold?
>
> Just to clarify, this limitation exists per Git process.

I missed this, makes sense now.

> For the time
> being, we only support writing objects to a single ODB source so a
> single transaction for object writes seems reasonable for now.
> Furthermore, the current "files" transaction backend relies on the
> tmp_odjdir subsystem which means only a single temp odjdir may exist for
> a Git process to write objects to.
>
> -Justin

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v2 0/4] odb: support ODB source specific transaction handling

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> Changes since V1:
>
> - Renamed the files transaction type to be `odb_transaction_files`.
>   Associated functions were also renamed accordingly.
> - Updated commit messages.
> - The `tmp_objdir` subsystem has been left unchange in the version. In a
>   followup series, I plan to couple tmpdirs directly to ODB files
>   transactions.

Ah, I was wondering why the changes are so small (it looked
essentially s/loose/files/ and nothing else to me), but if we are
leaving the tmp_objdir stuff out of the scope (which is sensible),
then there aren't much left to do relative to the previous
iteration.

Looking good.  Will replace.

Thanks.

Re: [PATCH v2 0/4] odb: support ODB source specific transaction handling

[Patrick Steinhardt]

On Mon, Feb 02, 2026 at 05:16:11PM -0800, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > Changes since V1:
> >
> > - Renamed the files transaction type to be `odb_transaction_files`.
> >   Associated functions were also renamed accordingly.
> > - Updated commit messages.
> > - The `tmp_objdir` subsystem has been left unchange in the version. In a
> >   followup series, I plan to couple tmpdirs directly to ODB files
> >   transactions.
> 
> Ah, I was wondering why the changes are so small (it looked
> essentially s/loose/files/ and nothing else to me), but if we are
> leaving the tmp_objdir stuff out of the scope (which is sensible),
> then there aren't much left to do relative to the previous
> iteration.

Yeah, agreed. As long as there is a subsequent patch series that fixes
the `tmp_objdir` thing I'm happy. It'd be a no-op refactoring for now
anyway given that we always write via the temporary object directory,
but it's a good cleanup to do on top.

Anyway, I'm happy with the status quo of this patch series, thanks!

Patrick