Re: [PATCH 0/5] global: drop external `the_index` variable

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2284 bytes --]

On Mon, Apr 15, 2024 at 10:50:48AM -0700, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
> > Maybe I've got the wrong end of the stick but my impression is that it
> > is the use of "the_repository" in library code (i.e. the files outside
> > builtin/) that causes most of the pain. With that in mind would be we
> > better focusing contributor and reviewer effort on eliminating
> > "the_repository" from those files instead? It would need to be done in
> > carefully in stages but would bring real benefits.
> 
> I am afraid that it would take a much larger effort.
> 
> I have a suspicion that many of the users of the_index do not have
> to even need that the index_state they work on is connected to any
> instance of a repository object (in other words, I tend to think
> that the value of having a pointer to an index_state in an instance
> of a repository structure is dubious), so in a sense, this rewrite
> of code that use the_index to use the_repository may be going in a
> wrong direction.

I wouldn't say it's going in a wrong direction. It simply makes explicit
what already is implicit and thus simplifies our ability to reason about
the code.

At least it does that for me -- if others disagree then I will rethink
my approach. But the third patch demonstrates that the current approach
also causes weird inconsistencies in how we track the state of both
`the_index` and `the_repository` in tandem.

> In other words, these functions may eventually want to take a pointer
> to an index_state structure as their parameters, without having to
> deal with the whole repository structure, but this rewrite assumes
> they would eventually want to all work with a repository structure
> when the_repository dependency is lifted.  I'll need to see the
> codepaths involved more carefully and think about it.

Yes, that's something that we will have to do eventually. I simply think
that the easiest way to approach this is bottom-up by moving the level
at which we inject the index one level up at a time. And with every
level that we move up we should figure out the intent of the code so
that we can decide whether it should use a local copy, the index of the
repository or something else.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]