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

From: Jeff King <peff@peff.net>
Date: Feb 13, 2020, 18:52 UTC
Message-ID: <20200213185235.GA15252@coredump.intra.peff.net>
In-Reply-To: <xmqqh7zus6hm.fsf@gitster-ct.c.googlers.com>

On Thu, Feb 13, 2020 at 10:10:45AM -0800, Junio C Hamano wrote:

Show 9 quoted lines

> > 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.

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

-Peff