Thread (5 messages) 5 messages, 3 authors, 2024-10-01

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

From: Patrick Steinhardt <hidden>
Date: 2024-10-01 14:09:32

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:
quoted
On Tue, Oct 01, 2024 at 12:58:30PM +0800, shejialuo wrote:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help