Re: [PATCH v2 3/4] apply: remove the_repository global variable

[Patrick Steinhardt <ps@pks.im>]

On Tue, Oct 01, 2024 at 09:40:43PM +0800, shejialuo wrote:
> On Tue, Oct 01, 2024 at 02:32:30PM +0200, Patrick Steinhardt wrote:
> > On Tue, Oct 01, 2024 at 12:58:30PM +0800, shejialuo wrote:
> > > On Mon, Sep 30, 2024 at 01:06:55PM -0700, Junio C Hamano wrote:
> > > In my opinion, we should first think about how we handle the situation
> > > where we run builtins outside of the repository. The most easiest way is
> > > to pass the fallback object (aka "the_repository").
> > > 
> > > However, this seems a little strange. We are truly outside of the
> > > repository but we really rely on the "struct repository *" to do many
> > > operations. It's unrealistic to change so many interfaces which use the
> > > "struct repository *". So, we should just use the fallback idea at
> > > current.
> > 
> > I disagree with this statement. If code isn't prepare to not handle a
> > `NULL` repository we shouldn't fall back to `the_repository`, but we
> > should instead prepare the code to handle this case. This of course
> > requires us to do a ton of refactorings, but that is the idea of this
> > whole exercise to get rid of `the_repository`.
> > 
> 
> Actually, I also insist that we should refactor here. But I worry about
> the burden this would bring to John due to we may do a lot of work here.
> So, I expressed my meaning in a compromising way.
> 
> But we should face the problem directly :).

True, all of this is a long-term effort that is probably going to take
us many months, likely even years. So people working on it should take
things slow and refactor chunks that are mostly ready to be converted to
get rid of `the_repository`.

That will sometimes mean that you have to scrap the conversion you're
currently working on because you discover that it inherently relies on
`the_repository` deep down in the stack, and refactoring it would be a
huge undertaking. That definitely happened to me multiple times while
introducing `USE_THE_REPOSITORY_VARIABLE`. And every time I did discover
that, I went one level deeper to try and fix the underpinnings first.

I mostly don't want us to blur the lines by silently falling back to
`the_repository` in situations where we don't intend to. So I'd rather
go a bit slower overall and design the code such that it doesn't fall
back anymore as a way to prove that something is indeed not relying on
`the_repository` anymore. Otherwise we're going to make everyones life
harder.

Patrick