Re: [PATCH 2/5] object.c: lookup_unknown_object() accept 'r' as parameter

[Taylor Blau <me@ttaylorr.com>]

On Thu, Feb 13, 2020 at 01:52:35PM -0500, Jeff King wrote:
> On Thu, Feb 13, 2020 at 10:10:45AM -0800, Junio C Hamano wrote:
>
> > > Right, but my suggestion was that this advice doesn't apply to this
> > > particular instance since I don't expect that we'd ever passing
> > > something other than 'the_repository'.
> > >
> > > Specifically, I was worried that we'd get bitten by re-assigning 'r' in
> > > the middle of the function and then end up in some odd broken state.
> >
> > "git fsck" works only in a single, "the", repository, so I guess you
> > are right to be worried about unnecessary complexity here.
>
> I think the end-game for this whole repository transition would be to
> get rid of the_repository, though. I.e., I'd envision the progression
> something like this:
>
>   1. Teach all of the library code to take (and operate on) "struct
>      repository".
>
>   2. Teach static local functions like this to pass in the_repository.
>
>   3. Teach top-level commands like cmd_fsck() to pass the_repository to
>      all of those static local helpers.
>
>   4. Teach top-level commands to get a real repository pointer, either
>      from the git.c wrapper (when RUN_SETUP is used) or by calling
>      setup_git_repository() themselves.
>
>   5. Grep for the_repository and drop it everywhere.
>
> Here we're at step 2 now, but declaring "r" makes moving to step 3 just
> a little easier. And I think the existence of steps 4 and 5 implies that
> it would eventually be worth going through step 3.

Ah, the transition to step 3 justifies this, I think. I wasn't aware
that steps 3+ existed. If they didn't, I'd stand by my original advice,
but given that they do, the approach here makes more sense long-term.

> Of course I just wrote those steps down for the first time, so maybe
> nobody else shares my vision. ;)

Thanks for writing it down. I'm sure that it has been loosely discussed
over a while, but this is the first time that I've seen it all in one
place.

> -Peff

Thanks,
Taylor