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

On 7/26/26 01:02, Junio C Hamano wrote:
> 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...

I was also wondering if doing this was appropriate... Since that's the 
case, let's not migrate the _abbrev variable for now. I'll expand this 
series, migrate some other variables and resend it when ready.

Regards, yuchen