On Thu, Jun 13, 2024 at 05:17:08PM -0700, Junio C Hamano wrote: > Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx> writes: > > > On 14/06/2024 00:18, Junio C Hamano wrote: > >> On Thu, Jun 13, 2024 at 4:15 PM Ramsay Jones > >> <ramsay@xxxxxxxxxxxxxxxxxxxx> 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
Attachment:
signature.asc
Description: PGP signature