On 25/08/22 09:37AM, Junio C Hamano wrote:
> Justin Tobler <jltobler@xxxxxxxxx> writes:
> We now need to be a lot more careful to make sure that everybody has
> access to a valid bulk_checkin_packfile struct, which makes the
> implementation of index_blob_bulk_checkin() a bit awkward and we
> need to invent one bulk_checkin_packfile instance right there.
> Luckily it goes away in the next step, I guess?
Ya, now that we cannot rely on the transaction singleton to provide a
bulk_checkin_packfile, we have create one if there is no transaction. I
completely agree that this is awkward which is why the next step
requires index_blob_bulk_checkin() callers to ensure a transaction is
created ahead of time.
If it is preferable, I can combine these two patches together. I kept
them separate as I thought it would better explain the steps, but maybe
that isn't the best way to structure the patch as we are largely undoing
the change in the next patch anyway.
> > diff --git a/odb.h b/odb.h
> > index 3dfc66d75a3..a89b2143909 100644
> > --- a/odb.h
> > +++ b/odb.h
> > @@ -84,6 +84,7 @@ struct odb_source {
> >
> > struct packed_git;
> > struct cached_object_entry;
> > +struct odb_transaction;
> >
> > /*
> > * The object database encapsulates access to objects in a repository. It
> > @@ -94,6 +95,13 @@ struct object_database {
> > /* Repository that owns this database. */
> > struct repository *repo;
> >
> > + /*
> > + * State of current current object database transaction. Only one
> > + * transaction may be pending at a time. Is NULL when no transaction is
> > + * configured.
> > + */
> > + struct odb_transaction *transaction;
>
> Once dust from this topic settles, we may want to rename the
> bulk-checkin.[ch] to have "odb" somewhere in its name, perhaps? I
> usually do not like renaming file for the sake of renaming to make
> the result look "pretty" (people may use "consistent naming" ),
> though. I dunno.
I'm hoping for an eventual state where the transactional interfaces,
{begin,end}_odb_transaction(), are moved directly into odb.{h,c}. The
current implementation of transaction handling is specific to the
current object database source. In a pluggable object database future
where we could have different types of object database sources,
transaction handling will likely have to implemented separately.
With this in mind, we could move the current transaction implementation
into something like object-files.c. This is already where the vast
majority of its call sites are and would enable us to further simply the
interface we expose.
I plan do submit a followup series to do this. :)
-Justin
