Re: [PATCH 2/3] replace-objects: create wrapper around setting

From: Derrick Stolee <derrickstolee@github.com>
Date: Jun 1, 2023, 19:50 UTC
Message-ID: <72fb4420-2492-e644-58cc-b9b3dbfb8037@github.com>
In-Reply-To: <49ea603b-ebbd-4a14-e0dd-07078e56de0a@github.com>

On 6/1/2023 12:35 PM, Victoria Dye wrote:

> Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <derrickstolee@github.com>

Show 26 quoted lines

>> diff --git a/replace-object.h b/replace-object.h
>> index 7786d4152b0..b141075023e 100644
>> --- a/replace-object.h
>> +++ b/replace-object.h
>> @@ -27,6 +27,19 @@ void prepare_replace_object(struct repository *r);
>>  const struct object_id *do_lookup_replace_object(struct repository *r,
>>  						 const struct object_id *oid);
>>  
>> +
>> +/*
>> + * Some commands disable replace-refs unconditionally, and otherwise each
>> + * repository could alter the core.useReplaceRefs config value.
>> + *
>> + * Return 1 if and only if all of the following are true:
>> + *
>> + *  a. disable_replace_refs() has not been called.
>> + *  b. GIT_NO_REPLACE_OBJECTS is unset or zero.
>> + *  c. the given repository does not have core.useReplaceRefs=false.
>> + */
>> +int replace_refs_enabled(struct repository *r);
> 
> Since the purpose of this function is to access global state, would
> 'environment.[c|h]' be a more appropriate place for it (and
> 'disable_replace_refs()', for that matter)? There's also some precedent;
> 'set_shared_repository()' and 'get_shared_repository()' have a very similar
> design to what you've added here.

 
That's an interesting idea that I had not considered. My vague sense
is that it is worth isolating the functionality to this header instead
of lumping it into the giant environment.h header, but I've CC'd
Elijah (who is leading a lot of this header organization stuff) to see
if he has an opinion on this matter.

Thanks, -Stolee