Subject: Re: [PATCH 1/3] repository: create disable_replace_refs()

From: Victoria Dye <[email-masked]>
Date: Thu, 1 Jun 2023 09:35:25 -0700

Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <[email-masked]>
> 
> Several builtins depend on being able to disable the replace references
> so we actually operate on each object individually. These currently do
> so by directly mutating the 'read_replace_refs' global.
> 
> A future change will move this global into a different place, so it will
> be necessary to change all of these lines. However, we can simplify that
> transition by abstracting the purpose of these global assignments with a
> method call.
> 
> We will never scope this to an in-memory repository as we want to make
> sure that we never use replace refs throughout the life of the process
> if this method is called.
> 

Although unfortunate (it would be nice to remove the global), this makes
sense. Disabling replace refs needs to be process-wide, and manually
propagating a repository setting to other repositories would be awkward
and prone to error.

All of my questions on this patch ("why were the 'disable_replace_refs()'
calls added later in the function than the original 'read_replace_refs =
0'?" and "why was '#include "environment.h"' added in 'repo-settings.c'?")
were asked [1] and answered [2] already. Beyond those two points, this patch
looks good!

[1] https://lore.kernel.org/git/[email-masked]/
[2] https://lore.kernel.org/git/[email-masked]/

> +/*
> + * Some commands override config and environment settings for using
> + * replace references. Use this method to disable the setting and ensure
> + * those other settings will not override this choice. This applies
> + * globally to all in-process repositories.
> + */
> +void disable_replace_refs(void);
> +

Thanks for including the function documentation. It concisely explains the
purpose of 'disable_replace_refs()' and helps clarify how replace refs are
treated in Git. 

>  #endif /* REPLACE_OBJECT_H */