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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 14, 2024, 15:54 UTC
Message-ID: <xmqq5xubikp5.fsf@gitster.g>
In-Reply-To: <ZmvVBwLEb0qofY2m@tanuki>

Patrick Steinhardt <ps@pks.im> writes:

Show 21 quoted lines

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

Slightly off-topic, but in retrospect, the approach of marking "this file is done" taken by the very initial version of the "no more implicit access to the_index" series may have been easier to grok easier to grok for reviewers casually looking at it from the sideline, than marking "this file still needs work". When we gradually deprecated the convenience API that implicitly access the_index instance, we marked with NO_THE_INDEX_COMPATIBILITY_MACROS the files that no longer require it (in other words, we are done with the file wrt the transition). We later flipped the polarity and gave USE_THE_INDEX_COMPATIBILITY_MACROS to those that are not marked with NO_THE_INDEX_COMPATIBILITY_MACROS as we progressed and the source files that still used the convenience API has become minorities.

Anyway, I think now people understood what this series aims to achieve and how it does so, hopefully.

Thanks.