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

[Justin Tobler <jltobler@gmail.com>]

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