Derrick Stolee <stolee@xxxxxxxxx> writes: > With your additional comments, I think it is clear that the "fourth > option" I mentioned earlier [1] is the way to go: > > Finally, there is yet a fourth option: use istate->repo instead. In > 1fd9ae51 (repository: add repo reference to index_state), I added a > 'repo' member to struct index_state. This is intended for methods to > access a repository directly from the index. > > [1] https://lore.kernel.org/git/f187df01-8e59-ac74-01e1-586a7a63fd4e@xxxxxxxxx/ Thanks. I wasn't following the earlier discussion closely, as the topic seemed to be in the hands of good reviewers ;-) > So in this sense, we should always use istate->repo, but we might > still need the following guard in some places: > > if (!istate->repo) > istate->repo = the_repository; > > in case there are situations where the index is loaded before > the_repository is loaded. I have hit this in testing, but don't fully > understand the cases where this can happen. As a longer term goal, it may not be a bad idea to make sure that anybody who passes an istate should not have to pass a repo. I do not think of a reason why, other than historical inertia, to do so in post 1fd9ae51 world. > The way it would change this patch is to drop the 'struct repository *r' > pointers and changes to the method signatures. Instead, keep the > methods only taking a 'struct index_state *istate' and use istate->repo > everywhere. Yes, and that would result in a patch that touches very limited small parts of cache-tree.c without needing to touch any caller, I would presume. Thanks.
