Subject: Re: [PATCH v2 00/20] Introduce `USE_THE_REPOSITORY_VARIABLE` macro

From: Patrick Steinhardt <[email-masked]>
Date: Fri, 14 Jun 2024 07:28:39 +0200

On Thu, Jun 13, 2024 at 05:17:08PM -0700, Junio C Hamano wrote:
> Ramsay Jones <[email-masked]> writes:
> 
> > On 14/06/2024 00:18, Junio C Hamano wrote:
> >> On Thu, Jun 13, 2024 at 4:15 PM Ramsay Jones
> >> <[email-masked]> wrote:
> >>>
> >>> Hmm, odd; isn't the declaration of 'the_repository' from
> >>> the "repository.h" header file visible at this point?
> >> 
> >> No. The declaration is guarded with USE_THE_REPOSITORY_VARIABLE CPP macro
> >> in the header, and repository.c does not define it.
> >> 
> >
> > Ah, OK. I haven't been following too closely and didn't
> > notice that the declaration in the header file was now
> > conditional. :(
> >
> > But that does beg the question - why is repository.c not
> > defining the USE_THE_REPOSITORY_VARIABLE?
> 
> I think the goal of the series is to eventually get to the point
> where nobody uses the_repository variable.  If repository.c, which
> consists of a set of service routines that work on a repository
> instance, defined it, showing willingness to implicitly rely on
> the_repository through things like get_oid_hex() (which would rely
> on the_repository->hash_algo), that would go the opposite direction,
> so everything, other than the definition of the_repository variable
> itself that allows other files that still do rely implicitly on the
> variable to link with it, in repository.c would actively want to
> refuse to use services only available to those who define USE_THE_*
> macro.

Exactly, that's why it doesn't declare `USE_THE_REPOSITORY_VARIABLE`.
The macro doesn't only guard use of `the_repository`, but does also
guards other functions that implicitly relies on it, and we do not want
to use these in "repository.c". So even though the added `extern`
declaration is somewhat ugly, I think it is preferable over defining the
macro.

Patrick