On Thu, Jun 1, 2023 at 12:50 PM Derrick Stolee <derrickstolee@xxxxxxxxxx> wrote: > > On 6/1/2023 12:35 PM, Victoria Dye wrote: > > Derrick Stolee via GitGitGadget wrote: > >> From: Derrick Stolee <derrickstolee@xxxxxxxxxx> > > >> 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. I haven't really formed much of an opinion on the sea of globals in environment.h and elsewhere beyond "I sure wish we didn't have so many globals". Maybe I should have an opinion on it, but there was plenty to clean up without worrying about all of those. :-)