Re: [PATCH v1 0/3] environment: migrate more global variables, pt.2

Tian Yuchen <[email protected]> writes:

> Hi all,
>
> This series moves:
>
>  - (1/3) minimum_abbrev and default_abbrev 
>  - (2/3) pack_size_limit_cfg
>  - (3/3) assume_unchanged
>
> into repo_config_values to continue the libification effort. 
>
> Note: in commit 1/3, we need (repo != the_repository) checks in the
> getters, because some subsystems where the readers of _abbrev
> configurations live forbid the use of 'the_repository' and only accept
> 'repo' [1]. We have to explicitly intercept those intances that are
> not 'the_repository'.

Sorry but I am not sure I follow.  If a repository that is not
the_repository is not yet allowed, shouldn't the caller be flagged
for passing a random repository that is not the_repository as not
conforming to the API (yet) with:

        if (repo != the_repository)
                BUG(...);

rather than papering over the issue with an unconditional

        repo = the_repository;

override?

If the API that deals with this 'abbrev' setting needs to call
another API that only superficially takes any 'repo' parameter
without supporting anything other than the_repository, isn't that a
sign that the other API needs to be extended to work with any 'repo'
before the 'abbrev' part of the system can use it, simply because the
former is not ready?  Futzing with the 'abbrev' part of the system in
such a state piles on more unfinished work that will need to be fixed
later without achieving anything, except for the superficial "now
this part too can take a 'repo' parameter, even though it does not
support anything but the_repository", no?

Puzzled...