Re: [PATCH v3] cache-tree.c: remove implicit dependency on the_repository

Derrick Stolee [off-list ref] 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@gmail.com/ (local)

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.