Re*: [PATCH v3 0/3] environment: clean up repository config handling

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> On Fri, Aug 07, 2026 at 02:11:08PM -0700, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>> > On Fri, Aug 07, 2026 at 04:59:29PM +0800, Tian Yuchen wrote:
>> > Somewhat unrelated to this patch series, but I was wondering whether you
>> > plan to drop the limitation in `repo_config_values()` that requires that
>> > the passed-in repository is `the_repository`. ...
>>
>> Hmph, that is an interesting point.  What is our plan to really
>> enable the use of repository instances other than 'the_repository'
>> here?  They of course need to be initialized with repo_init(),
>> but is that enough to sensibly use the embedded 'repo_settings'
>> and 'repo_config_values' structures?  (By the way, it is not
>> entirely clear to me why we need both and how we sift variables
>> between them.)
>
> Yeah, this split is adding to the confusion indeed. I think that we
> should make it a goal to unify those going forward.
> ...
> I've been wondering for a while whether we're operating at the wrong
> level here. Both `repo_settings` and `repo_config_values` indicates that
> we're operating in the context of a repository, but as you mention that
> may not even be the case.
>
> I don't think the approach is inherently flawed though. From my point
> of view, the best way forward is to merge those two and then generalize
> them into something like `git_config_values` or `git_settings`,
> depending on which of both variants we want to retain. We would then
> have two levels:
>
>   - One on the repository level as we have it today.
>
>   - One truly global variable, because that stuff in fact _is_ global.
>
> We'd then adapt `repo_config_values()` so that it knows to populate
> either of those variables depending on whether or not the user passes a
> valid repository, and returns a constant pointer to the respective
> structure. Callers MUST NOT modify that structure -- if they want to,
> they'll have to make a copy and pass it down the calling stack.
>
> The last part about not modifying that structure could be quite a bit
> painful though, as it would mean that we might have to adapt call chains
> to pass down a `struct git_config_values` instead of a `struct
> repository`. But arguably, that's the right thing to do anyway for at
> least some subsystems that are independent of repositories.
>
> As you say though, none of this is really related to this patch series
> at hand, and I don't think we need to resolve this discussion before we
> can merge it. I just want to make sure that we have a plan for how to
> get rid of `the_repository` instead of only shuffling stuff around.

Even after we punted on this and stopped the side discussion in
early August, there still are topics that want to move things into
repo_settings, or even worse, repo_config_values.  Until we have a
plan to fix repo_config_values() to take an arbitrary repository,
should we freeze this part of the code, perhaps like this?

----- >8 -----
Subject: repo_config_values: freeze the structure for now

The configuration and settings per repository is currently in an
unfortunate mess.  Especially given that repo_config_values() cannot
work on any repository instance other than the_repository, moving
more members into this structure is unwelcome.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 environment.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git c/environment.h w/environment.h
index 9e175e410e..03b1b35d6a 100644
--- c/environment.h
+++ w/environment.h
@@ -124,6 +124,13 @@ enum object_creation_mode {
 	OBJECT_CREATION_USES_RENAMES = 1
 };
 
+/*
+ * See https://lore.kernel.org/git/anlmwaEtwcCPse1N@pks.im/ and later
+ * discussions.  Until we figure out what to do with this and
+ * repo_settings, and allow them to be truly usable on a repository
+ * instance that is not the_repository, do *NOT* add any new members
+ * to this structure.
+ */
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;