On 6/2/2023 9:47 PM, Elijah Newren wrote:

On Thu, Jun 1, 2023 at 12:50 PM Derrick Stolee [off-list ref] wrote:

quoted

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

quoted

Derrick Stolee via GitGitGadget wrote:

quoted

From: Derrick Stolee <redacted>

quoted

quoted

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

Thanks for chiming in (even with "I haven't thought about it too much").

Thinking back on this with some time since the initial question, I think
the grouping "global state" into environment.h isn't the right goal.
Using replace-object.h collects all _functionality related to the feature_
in a single place, and it just so happens to include some global state due
to the needs of the feature.

I plan to keep these methods in replace-object.h. With that, we have only
20 files that include that, as opposed to 141 including environment.h.

Thanks,
-Stolee