From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 29 Jan 2026 11:24:29 GMT
Subject: Re: [PATCH 4/4] odb: transparently handle common transaction behavior
Message-ID: <aXtDbTuATmSkWRx_@pks.im>
In-Reply-To: <20260128234519.2721179-5-jltobler@gmail.com>

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