On 25/08/20 05:15PM, Junio C Hamano wrote: > I somehow expected that odb would know what repository it was > instanciated to work with, or in the worst case where in-core odb is > in theory sharable among multiple in-core repositories, at least > begin_odb_transaction() would take <repository, odb> pair and the > transaction would know for which repository the transaction is > working for. The `struct odb_transaction` maintains a pointer to its parent `struct object_database`. This ODB also has a pointer to the `struct repository`. Thus, from an ongoing transaction we should be able to get the repository we want. For `begin_odb_transaction()` we should only have to provide `struct object_database` and from that the transaction can access the corresponding repository. In the general cases, this is exactly what we do, but this doesn't work for `index_blob_bulk_checkin()` and its accompanying internal functions as they may be invoked from outside of a transaction. > Do we need bulk_checkin_packfile as a separate structure and pass it > around, or would these internal functions be better off passing an > instance of odb_transaction around and learn the repository from > odb->repo? In its current form, the bulk-checkin mechanism for packfiles may be used outside of transactions. For example when calling `index_blob_bulk_checkin()` without a transaction, the single object is written to the packfile moved into the primary ODB. With a transaction, multiple objects may be written to a single packfile. This whole setup is rather awkward though. Thinking about this more, we should probably just require `index_blob_bulk_checkin()` be provided a transaction. Callers will need to ensure a transaction is running so that a `struct bulk_checkin_packfile` gets set up, but this shouldn't be a big deal. With this we could easily just propagate the transaction for all these function as you suggested. I'll do this in the next version. Thanks! -Justin
