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

From: Justin Tobler <jltobler@gmail.com>
Date: Jan 29, 2026, 19:41 UTC
Message-ID: <aXu1fWWdP3dYKdHf@denethor>
In-Reply-To: <aXtDZyFoSQahkxBa@pks.im>

On 26/01/29 12:24PM, Patrick Steinhardt wrote:

Show 8 quoted lines

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

Show 6 quoted lines

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

Show 14 quoted lines

> 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