Justin Tobler <jltobler@xxxxxxxxx> writes:
> diff --git a/bulk-checkin.c b/bulk-checkin.c
> index 82a73da79e8..53a20a2d92f 100644
> --- a/bulk-checkin.c
> +++ b/bulk-checkin.c
> @@ -30,11 +30,13 @@ struct bulk_checkin_packfile {
> uint32_t nr_written;
> };
>
> -static struct odb_transaction {
> +struct odb_transaction {
> + struct object_database *odb;
> +
> int nesting;
> struct tmp_objdir *objdir;
> struct bulk_checkin_packfile packfile;
> -} transaction;
> +};
Very nice to see that this singleton instance is now gone.
The singleton was really handy way to allow calls into this
subsystem to lazily initialize bulk_checkin_packfile in the call
chain starting from deflate_blob_to_pack() -> prepare_to_stream().
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?
> +int index_blob_bulk_checkin(struct odb_transaction *transaction,
> + struct object_id *oid, int fd, size_t size,
> const char *path, unsigned flags)
> {
> - int status = deflate_blob_to_pack(&transaction.packfile, oid, fd, size,
> - path, flags);
> - if (!transaction.nesting)
> - flush_bulk_checkin_packfile(&transaction.packfile);
> + int status;
> +
> + if (transaction) {
> + status = deflate_blob_to_pack(&transaction->packfile, oid, fd,
> + size, path, flags);
> + } else {
> + struct bulk_checkin_packfile state = { 0 };
> +
> + status = deflate_blob_to_pack(&state, oid, fd, size, path, flags);
> + flush_bulk_checkin_packfile(&state);
> + }
> +
> return status;
> }
OK. If we do not have a transaction (i.e. if nobody called
begin_odb_transaction() more times than end_odb_transaction()
got called), we let the underlying machinery do the right thing on
an empty state and clean up after ourselves; otherwise we follow
exactly the same code path as we used to.
> diff --git a/cache-tree.c b/cache-tree.c
> index 66ef2becbe0..d225554eedd 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -474,6 +474,7 @@ static int update_one(struct cache_tree *it,
>
> int cache_tree_update(struct index_state *istate, int flags)
> {
> + struct odb_transaction *transaction;
> int skip, i;
OK, doing the odb-transaction here would cover both this one and
write_index_as_tree and its friends, that are public interfaces into
the cache-tree subsystem.
> 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.
