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

From: Patrick Steinhardt <ps@pks.im>
Date: Oct 1, 2024, 14:09 UTC
Message-ID: <ZvwCj4J_HRiSF_S0@pks.im>
In-Reply-To: <Zvv723-OwvEr0qMV@ArchLinux>

On Tue, Oct 01, 2024 at 09:40:43PM +0800, shejialuo wrote:

Show 25 quoted lines

> 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