Re: [PATCH v2 7/7] odb/transaction: add transaction interface to write packfiles

On 26/08/09 06:54PM, Junio C Hamano wrote:
> Justin Tobler <[email protected]> writes:
> 
> > In git-receive-pack(1), the incoming packfile is written to the ODB via
> > `unpack()`, which spawns git-index-pack(1) or git-unpack-objects(1)
> > directly. With pluggable object databases, an alternative backend may
> > need to handle writing packfile data differently though.
> >
> > Introduce `odb_transaction_write_pack()` as a generic interface to
> > handle writing a packfile to a transaction and use the logic from
> > `unpack()` as the "files" backend implementation. Note that a packfile
> > written via git-index-pack(1) is kept in place by a ".keep" lockfile
> > that must be retained until references are updated. To faciliate this in
> > an ODB backend agnostic manner, the "files" transaction backend takes
> > ownership of these lockfiles and removes them post-commit through its
> > release callback.
> > ...
> > +static int odb_transaction_files_write_pack(struct odb_transaction *base,
> > +					    int pack_fd, struct strbuf *err_msg,
> > +					    const struct odb_transaction_write_pack_opts *opts)
> > +{
> > + ...
> > +		odb_source_prepare(repo->objects->sources,
> > +				   ODB_PREPARE_FLUSH_CACHES);
> > +	}
> 
> This assumes that we are working with the first entry in the
> 'repo->objects->sources' linked list.  Should we not use the source
> actually associated with the current transaction (I am guessing that
> it is 'base->source' but I may be wrong)?

Currently tracking of the transaction's ODB source relies on reading the
first source in this list which is rather awkward in my opinion and
rather fragile. The ODB source specified by `base->source` here is
actually the main ODB source that the transaction is created against so
it can't be used here.

In a future series, my plan is to stop reordering the source list when
starting a transaction and instead track the source separately which
should simplify some of this. It may be a good idea for the ODB
transaction itself to track its own source regardless though and may be
something nice to add as a part of this series. I'll explore this in my
next version of the series.

Thanks,
-Justin