Subject: Re: [PATCH v3 8/8] builtin/checkout-index: stop using `the_repository`

From: Junio C Hamano <[email-masked]>
Date: Fri, 07 Mar 2025 06:15:31 -0800

Usman Akinyemi <[email-masked]> writes:

>> Hmph, if we are passing anything down to these code paths, I would
>> have expected that it would be an instance of "struct index_state".
>>
>> Do these two helper functions need anything other than that from the
>> repository instance?
> No, they do not. They could possibly do in the future and is there any
> reason why we might want to pass the "struct index_state" instead of
> the whole "struct repository" ?

You are asking a wrong question.

When there are two things, A and B, where B is a piece of data with
a smaller scope that is contained within A, unless your function
needs to access parts of A that is impossible to access if you fed
it only B, limiting the interface to the smallest piece necessary
(in this case, B) is always the better design.  A potential caller
that only has B and not A can still call your function that takes B
if you designed the API that way.  If you insist that the caller
pass A (and infer which B to use as part of A), a potential caller
that only has B cannot call your function.

So, with that understanding of a general principle in mind, you need
a stronger justification to pass the larger object A that goes
against the best practice, saying "Even though we do not have to
pass A because only B is necessary, we pass A because ...".  And you
just said there is no such reason why you need a repository and for
these functions an index_state is sufficient.

There are many functions that only need "struct index_state"
instance and obtains one from their callers.  They do not have a
repository object, other than the one that is referred to from the
index_state (to keep track of which repository it originated from).

But this pointer is not designed to round-trip.  There can be, and
there indeed are, code paths that use multiple index_state instances
associated with one repository.  But a repository instance has only
one ".index" member.  It means if you pass a repository, you are
making it impossible for your potential callers that has secondary
index_state instances.