Justin Tobler <jltobler@xxxxxxxxx> writes: > The bulk-checkin subsystem depends on `the_repository`. Adapt functions > and call sites to access the repository through `struct odb_transaction` > instead. The `USE_THE_REPOSITORY_VARIBALE` is still required as the > `pack_compression_level` and `pack_size_limit_cfg` globals are still > used. Also we grab the details of the new packfile the bulk-checkin machinery is building out of the transaction, which made some redundant parameters functions take go away, ... > -static void finish_tmp_packfile(struct strbuf *basename, > - const char *pack_tmp_name, > - struct pack_idx_entry **written_list, > - uint32_t nr_written, > - struct pack_idx_option *pack_idx_opts, > +static void finish_tmp_packfile(struct odb_transaction *transaction, > + struct strbuf *basename, > unsigned char hash[]) ... like this one. Very nice. > @@ -117,7 +118,8 @@ static void flush_batch_fsync(struct odb_transaction *transaction) > * to ensure that the data in each new object file is durable before > * the final name is visible. > */ > - strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX", repo_get_object_directory(the_repository)); > + strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX", > + transaction->odb->sources->path); This is doing a lot more than a simple "the_repository" -> "odb->repo" replacement. How much confidence do we have that the internal detail of repo_get_object_directory() will stay the same and our developers in the future would spot that this open coded copy needs to be updated if they have to change it? > -static int deflate_blob_to_pack(struct bulk_checkin_packfile *state, > - struct object_id *result_oid, > - int fd, size_t size, > - const char *path, unsigned flags) > +int index_blob_bulk_checkin(struct odb_transaction *transaction, > + struct object_id *result_oid, > + int fd, size_t size, > + const char *path, unsigned flags) Ahh, OK, with the simplification to always take transaction, there is no need to have the deflate_blob_to_pack() function, which had only one caller, as an internal implementation detail anymore. This change could have been in the previous step and it would have been less surprising; the above "Ahh, OK" is my reaction to the surprise ;-)
