[PATCH v2 0/2] environment: move excludes_file into repo_config_values

[PATCH v2 0/2] environment: move excludes_file into repo_config_values

[Tian Yuchen]

This series continues the libification effort by migrating the global
string variable 'excludes_file' into 'struct repo_config_values'. Since
this is a dynamically allocated variable, the migration requires proper
heap memory management.

The series is structured in two commits:

 - Abstract the XDG fallback lazy-loading logic out of dir.c into a proper
getter.

 - Move the variables into the struct and introduce 'repo_config_values_clear()'.

Note on Submodules: A temporary shield 'if (repo != the_repository)' is
included in both the getter and the clear function. This prevents
uninitialized submodules from triggering the BUG() assertion.
(Inspiration: [1])

Changes since V1:

 - fix a typo in the second commit.

 - initialize excludes_file to NULL in repo_config_values_init().

 - call FREE_AND_NULL() to free attributes_file as well in
 repo_config_values_clear().

Thanks!

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>

[1] https://lore.kernel.org/git/c95a7730-7b14-4be0-a4e4-861b2f5430ea@gmail.com/

Tian Yuchen (2):
  dir: encapsulate excludes_file lazy-load
  environment: move excludes_file into repo_config_values

 dir.c         |  4 ++--
 environment.c | 32 +++++++++++++++++++++++++++++---
 environment.h | 13 ++++++++++++-
 repository.c  |  1 +
 4 files changed, 44 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] dir: encapsulate excludes_file lazy-load

[Tian Yuchen]

The global variable 'excludes_file' is used to track the path to the
global ignore file, 'core.excludesfile'. If this variable is NULL,
setup_standard_excludes() in dir.c forcefully evaluates and assigns
the XDG default path to it.

Introduce repo_excludes_file() as a getter to encapsulate this
lazy-loading logic. This prepares the variable to be safely moved
into 'struct repo_config_values' in the subsequent commit.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 dir.c         | 4 ++--
 environment.c | 7 +++++++
 environment.h | 5 +++++
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index 7a73690fbc..4f87a52b3c 100644
--- a/dir.c
+++ b/dir.c
@@ -3481,11 +3481,11 @@ static GIT_PATH_FUNC(git_path_info_exclude, "info/exclude")
 
 void setup_standard_excludes(struct dir_struct *dir)
 {
+	const char *excludes_file = repo_excludes_file(the_repository);
+
 	dir->exclude_per_dir = ".gitignore";
 
 	/* core.excludesfile defaulting to $XDG_CONFIG_HOME/git/ignore */
-	if (!excludes_file)
-		excludes_file = xdg_config_home("ignore");
 	if (excludes_file && !access_or_warn(excludes_file, R_OK, 0))
 		add_patterns_from_file_1(dir, excludes_file,
 					 dir->untracked ? &dir->internal.ss_excludes_file : NULL);
diff --git a/environment.c b/environment.c
index ba2c60103f..8efcaeafa6 100644
--- a/environment.c
+++ b/environment.c
@@ -134,6 +134,13 @@ int is_bare_repository(void)
 	return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
 }
 
+const char *repo_excludes_file(struct repository *repo)
+{
+	if (!excludes_file)
+		excludes_file = xdg_config_home("ignore");
+	return excludes_file;
+}
+
 int have_git_dir(void)
 {
 	return startup_info->have_repository
diff --git a/environment.h b/environment.h
index 6f18286955..52d531e4ea 100644
--- a/environment.h
+++ b/environment.h
@@ -133,6 +133,11 @@ int git_default_config(const char *, const char *,
 int git_default_core_config(const char *var, const char *value,
 			    const struct config_context *ctx, void *cb);
 
+/*
+ * TODO: This still relies on the global state.
+ */
+const char *repo_excludes_file(struct repository *repo);
+
 void repo_config_values_init(struct repo_config_values *cfg);
 
 /*
-- 
2.43.0

[PATCH v2 2/2] environment: move excludes_file into repo_config_values

[Tian Yuchen]

Continue the libification effort by moving the 'excludes_file' global
variable into 'struct repo_config_values'.

Since 'excludes_file' is a dynamically allocated string (char *), it
requires proper memory management. Introduce repo_config_values_clear()
to safely free the heap memory when repository instance is destroyed.

Note:

 - 'if (repo != the_repository)' fallback logic is temporarily added
in both the getter and the clear function. This prevents calling
repo_config_values() on uninitialized submodules, which triggers BUG().

 - 'attribute_file' is another string variable that was migrated
 earlier. Its FREE_AND_NULL() call is also added to
 repo_config_values_clear().

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 environment.c | 31 +++++++++++++++++++++++++------
 environment.h | 14 ++++++++++----
 repository.c  |  1 +
 3 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/environment.c b/environment.c
index 8efcaeafa6..b8dfa3e213 100644
--- a/environment.c
+++ b/environment.c
@@ -57,7 +57,6 @@ enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
 enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
 char *editor_program;
 char *askpass_program;
-char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
@@ -136,9 +135,13 @@ int is_bare_repository(void)
 
 const char *repo_excludes_file(struct repository *repo)
 {
-	if (!excludes_file)
-		excludes_file = xdg_config_home("ignore");
-	return excludes_file;
+	if (!repo || !repo->initialized || repo != the_repository)
+		return NULL;
+
+	if (!repo_config_values(repo)->excludes_file)
+		repo_config_values(repo)->excludes_file = xdg_config_home("ignore");
+
+	return repo_config_values(repo)->excludes_file;
 }
 
 int have_git_dir(void)
@@ -468,8 +471,8 @@ int git_default_core_config(const char *var, const char *value,
 	}
 
 	if (!strcmp(var, "core.excludesfile")) {
-		FREE_AND_NULL(excludes_file);
-		return git_config_pathname(&excludes_file, var, value);
+		FREE_AND_NULL(cfg->excludes_file);
+		return git_config_pathname(&cfg->excludes_file, var, value);
 	}
 
 	if (!strcmp(var, "core.whitespace")) {
@@ -722,6 +725,7 @@ int git_default_config(const char *var, const char *value,
 void repo_config_values_init(struct repo_config_values *cfg)
 {
 	cfg->attributes_file = NULL;
+	cfg->excludes_file = NULL;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
@@ -733,3 +737,18 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->sparse_expect_files_outside_of_patterns = 0;
 	cfg->warn_on_object_refname_ambiguity = 1;
 }
+
+void repo_config_values_clear(struct repository *repo)
+{
+	struct repo_config_values *cfg;
+
+	if (repo != the_repository)
+		return;
+
+	cfg = repo_config_values(repo);
+	if (!cfg)
+		return;
+
+	FREE_AND_NULL(cfg->attributes_file);
+	FREE_AND_NULL(cfg->excludes_file);
+}
diff --git a/environment.h b/environment.h
index 52d531e4ea..2e8352de7f 100644
--- a/environment.h
+++ b/environment.h
@@ -90,6 +90,7 @@ struct repository;
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
+	char *excludes_file;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -133,13 +134,19 @@ int git_default_config(const char *, const char *,
 int git_default_core_config(const char *var, const char *value,
 			    const struct config_context *ctx, void *cb);
 
-/*
- * TODO: This still relies on the global state.
- */
 const char *repo_excludes_file(struct repository *repo);
 
 void repo_config_values_init(struct repo_config_values *cfg);
 
+/*
+ * Frees memory allocated for dynamically loaded configuration values
+ * inside `repo_config_values`.
+ *
+ * As dynamically allocated variables are migrated into this struct,
+ * their FREE_AND_NULL() calls should be appended here.
+ */
+void repo_config_values_clear(struct repository *repo);
+
 /*
  * TODO: All the below state either explicitly or implicitly relies on
  * `the_repository`. We should eventually get rid of these and make the
@@ -213,7 +220,6 @@ extern char *git_log_output_encoding;
 
 extern char *editor_program;
 extern char *askpass_program;
-extern char *excludes_file;
 
 /*
  * The character that begins a commented line in user-editable file
diff --git a/repository.c b/repository.c
index 187dd471c4..b31f1b7852 100644
--- a/repository.c
+++ b/repository.c
@@ -388,6 +388,7 @@ void repo_clear(struct repository *repo)
 	FREE_AND_NULL(repo->parsed_objects);
 
 	repo_settings_clear(repo);
+	repo_config_values_clear(repo);
 
 	if (repo->config) {
 		git_configset_clear(repo->config);
-- 
2.43.0

Re: [PATCH v2 0/2] environment: move excludes_file into repo_config_values

[Junio C Hamano]

Tian Yuchen <cat@malon.dev> writes:

> This series continues the libification effort by migrating the global
> string variable 'excludes_file' into 'struct repo_config_values'. Since
> this is a dynamically allocated variable, the migration requires proper
> heap memory management.

This appears here:

  https://lore.kernel.org/git/20260626075037.532164-1-cat@malon.dev/

and as you can see, there is no linkage back to the previous round.

The lack of In-Reply-To and References headers unfortunately delays my
automation in marking topics with newer iterations available to be
reviewed when I come back to the keyboard, which happens overnight.

Re: [PATCH v2 2/2] environment: move excludes_file into repo_config_values

[Junio C Hamano]

Tian Yuchen <cat@malon.dev> writes:

> Continue the libification effort by moving the 'excludes_file' global
> variable into 'struct repo_config_values'.
>
> Since 'excludes_file' is a dynamically allocated string (char *), it
> requires proper memory management. Introduce repo_config_values_clear()
> to safely free the heap memory when repository instance is destroyed.
>
> Note:
>
>  - 'if (repo != the_repository)' fallback logic is temporarily added
> in both the getter and the clear function. This prevents calling
> repo_config_values() on uninitialized submodules, which triggers BUG().
>
>  - 'attribute_file' is another string variable that was migrated
>  earlier. Its FREE_AND_NULL() call is also added to
>  repo_config_values_clear().

OK.  I think the placement of the new member in repo_config_values
in this round, moving to the spot next to existing attributes_file,
makes more sense than the previous round, too.

Looking good.  Shall we mark it as ready for 'next' now?

Re: [PATCH v2 2/2] environment: move excludes_file into repo_config_values

[Junio C Hamano]

Tian Yuchen <cat@malon.dev> writes:

> Continue the libification effort by moving the 'excludes_file' global
> variable into 'struct repo_config_values'.
>
> Since 'excludes_file' is a dynamically allocated string (char *), it
> requires proper memory management. Introduce repo_config_values_clear()
> to safely free the heap memory when repository instance is destroyed.
>
> Note:
>
>  - 'if (repo != the_repository)' fallback logic is temporarily added
> in both the getter and the clear function. This prevents calling
> repo_config_values() on uninitialized submodules, which triggers BUG().

Would it be possible for the function to be called on the_repository
before it gets initialized?

> +void repo_config_values_clear(struct repository *repo)
> +{
> +	struct repo_config_values *cfg;

What I am wondering is if this check

> +	if (repo != the_repository)
> +		return;

wants to be more like

	if (!repo->initialized)
		return;

or even

	if (!repo->initialized) {
		BUG("clearing uninitialised repo config");
		return;
	}

Or perhaps not doing anything special there.

For that matter,

> +
> +	cfg = repo_config_values(repo);
> +	if (!cfg)
> +		return;

Wouldn't it be a bug to see NULL returned from the above function?
Why is it healthy to pretend as if nothing bad happened?

> +	FREE_AND_NULL(cfg->attributes_file);
> +	FREE_AND_NULL(cfg->excludes_file);
> +}


What do we want to happen when somebody does want to access (not
_clear(), but repo_config_values() itself) repo_config_values() in
today's code?  Don't we want to catch such a code as buggy?  Isn't
it the reason why repository.c:repo_config_values() check these
conditions and calls BUG() in the first place?  And if that is the
case, I find that a caller that "works around" by pretending nothing
bad happened and not calling repo_config_values(), like the above
code does, highly questionable, as it smells like sweeping problems
that you designed other parts of the code to detect with BUG() under
the rug.

In the longer run, we would want to have separate settings, which
used to be global variables but now are stored in per repository
config, available and usable in the context of each repository that
they are configured within.  If a caller wants to clear per repo
config for a repository instance by calling this function, this
function is in no place to tweak the intention of the caller by
short-circuiting the request and pretending it did what it was asked
to do.  In other words, the rest of the code not quite prepared to
deal with these global variables that turned into per repository
configuration values is *not* a problem this function should
address.  Let it be noticed by repo_config_values() function to
catch offending callers for now, and once the codebase becomes ready
to use one repo_config_values per repository, this function does not
have to change.

Re: [PATCH v2 1/2] dir: encapsulate excludes_file lazy-load

[SZEDER Gábor]

On Fri, Jun 26, 2026 at 03:50:36PM +0800, Tian Yuchen wrote:
> diff --git a/environment.c b/environment.c
> index ba2c60103f..8efcaeafa6 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -134,6 +134,13 @@ int is_bare_repository(void)
>  	return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
>  }
>  
> +const char *repo_excludes_file(struct repository *repo)
> +{
> +	if (!excludes_file)
> +		excludes_file = xdg_config_home("ignore");
> +	return excludes_file;
> +}

This function has a 'repo' parameter, which is not used in the
function at all.  This causes build failure when trying to build this
commit using DEVELOPER=1:

  environment.c: In function ‘repo_excludes_file’:
  environment.c:137:51: error: unused parameter ‘repo’ [-Werror=unused-parameter]
    137 | const char *repo_excludes_file(struct repository *repo)
        |                                ~~~~~~~~~~~~~~~~~~~^~~~
  cc1: all warnings being treated as errors
  make: *** [Makefile:2922: environment.o] Error 1

Please make sure that all commits can be built with 'make
DEVELOPER=1'.

Re: [PATCH v2 1/2] dir: encapsulate excludes_file lazy-load

[Junio C Hamano]

SZEDER Gábor <szeder.dev@gmail.com> writes:

> On Fri, Jun 26, 2026 at 03:50:36PM +0800, Tian Yuchen wrote:
>> diff --git a/environment.c b/environment.c
>> index ba2c60103f..8efcaeafa6 100644
>> --- a/environment.c
>> +++ b/environment.c
>> @@ -134,6 +134,13 @@ int is_bare_repository(void)
>>  	return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
>>  }
>>  
>> +const char *repo_excludes_file(struct repository *repo)
>> +{
>> +	if (!excludes_file)
>> +		excludes_file = xdg_config_home("ignore");
>> +	return excludes_file;
>> +}
>
> This function has a 'repo' parameter, which is not used in the
> function at all.  This causes build failure when trying to build this
> commit using DEVELOPER=1:
>
>   environment.c: In function ‘repo_excludes_file’:
>   environment.c:137:51: error: unused parameter ‘repo’ [-Werror=unused-parameter]
>     137 | const char *repo_excludes_file(struct repository *repo)
>         |                                ~~~~~~~~~~~~~~~~~~~^~~~
>   cc1: all warnings being treated as errors
>   make: *** [Makefile:2922: environment.o] Error 1
>
> Please make sure that all commits can be built with 'make
> DEVELOPER=1'.

Good point.  In this case, we can start with UNUSED in step 1/2 and
then drop the UNUSED in the second step.  I wonder how harder to read
it would become if these two patches are squashed together...

Thanks.

Re: [PATCH v2 0/2] environment: move excludes_file into repo_config_values

[Tian Yuchen]

On 6/26/26 23:42, Junio C Hamano wrote:

Sorry, there was a problem with my Thunderbird client... so the my 
emails didn't send without me noticing...

> Tian Yuchen <cat@malon.dev> writes:
> 
>> This series continues the libification effort by migrating the global
>> string variable 'excludes_file' into 'struct repo_config_values'. Since
>> this is a dynamically allocated variable, the migration requires proper
>> heap memory management.
> 
> This appears here:
> 
>    https://lore.kernel.org/git/20260626075037.532164-1-cat@malon.dev/
> 
> and as you can see, there is no linkage back to the previous round.
> 
> The lack of In-Reply-To and References headers unfortunately delays my
> automation in marking topics with newer iterations available to be
> reviewed when I come back to the keyboard, which happens overnight.

Regarding this, I forgot to bring --in-reply-to. I'll remember next time.

Thanks, yuchen

Re: [PATCH v2 2/2] environment: move excludes_file into repo_config_values

[Tian Yuchen]

On 6/27/26 03:12, Junio C Hamano wrote:
> Tian Yuchen <cat@malon.dev> writes:
> 
>> Continue the libification effort by moving the 'excludes_file' global
>> variable into 'struct repo_config_values'.
>>
>> Since 'excludes_file' is a dynamically allocated string (char *), it
>> requires proper memory management. Introduce repo_config_values_clear()
>> to safely free the heap memory when repository instance is destroyed.
>>
>> Note:
>>
>>   - 'if (repo != the_repository)' fallback logic is temporarily added
>> in both the getter and the clear function. This prevents calling
>> repo_config_values() on uninitialized submodules, which triggers BUG().
> 
> Would it be possible for the function to be called on the_repository
> before it gets initialized?
> 
>> +void repo_config_values_clear(struct repository *repo)
>> +{
>> +	struct repo_config_values *cfg;
> 
> What I am wondering is if this check
> 
>> +	if (repo != the_repository)
>> +		return;
> 
> wants to be more like
> 
> 	if (!repo->initialized)
> 		return;
> 
> or even
> 
> 	if (!repo->initialized) {
> 		BUG("clearing uninitialised repo config");
> 		return;
> 	}
> 
> Or perhaps not doing anything special there.
> 
> For that matter,
> 
>> +
>> +	cfg = repo_config_values(repo);
>> +	if (!cfg)
>> +		return;
> 
> Wouldn't it be a bug to see NULL returned from the above function?
> Why is it healthy to pretend as if nothing bad happened?
> 
>> +	FREE_AND_NULL(cfg->attributes_file);
>> +	FREE_AND_NULL(cfg->excludes_file);
>> +}
> 
> 
> What do we want to happen when somebody does want to access (not
> _clear(), but repo_config_values() itself) repo_config_values() in
> today's code?  Don't we want to catch such a code as buggy?  Isn't
> it the reason why repository.c:repo_config_values() check these
> conditions and calls BUG() in the first place?  And if that is the
> case, I find that a caller that "works around" by pretending nothing
> bad happened and not calling repo_config_values(), like the above
> code does, highly questionable, as it smells like sweeping problems
> that you designed other parts of the code to detect with BUG() under
> the rug.
> 
> In the longer run, we would want to have separate settings, which
> used to be global variables but now are stored in per repository
> config, available and usable in the context of each repository that
> they are configured within.  If a caller wants to clear per repo
> config for a repository instance by calling this function, this
> function is in no place to tweak the intention of the caller by
> short-circuiting the request and pretending it did what it was asked
> to do.  In other words, the rest of the code not quite prepared to
> deal with these global variables that turned into per repository
> configuration values is *not* a problem this function should
> address.  Let it be noticed by repo_config_values() function to
> catch offending callers for now, and once the codebase becomes ready
> to use one repo_config_values per repository, this function does not
> have to change.

Yes, indeed. I initially thought these were two solutions leading to the 
same goal, but now that you mention it, I think your point is more valid.

First, 'repo_config_values_clear()' shouldn't know whether "only 
'the_repository' is currently supported" (even if this logic is 
necessary, this function is clearly not the optimal place), so we 
shouldn't use 'repo != the_repository' to determine this.

Second, what's wrong with '!cfg' is that what it's supposed to avoid 
deviates from what it tries to avoid. 'repo_config_values(repo)' 
shouldn't validly return NULL at all. I admit this code is nonsensical.

Using 'repo->initialized' to determine this is consistent with our 
previous approach. I'll try it. Thanks for the review!

Regards, yuchen

Re: [PATCH v2 1/2] dir: encapsulate excludes_file lazy-load

[Tian Yuchen]

On 6/27/26 05:45, Junio C Hamano wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
> 
>> On Fri, Jun 26, 2026 at 03:50:36PM +0800, Tian Yuchen wrote:
>>> diff --git a/environment.c b/environment.c
>>> index ba2c60103f..8efcaeafa6 100644
>>> --- a/environment.c
>>> +++ b/environment.c
>>> @@ -134,6 +134,13 @@ int is_bare_repository(void)
>>>   	return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
>>>   }
>>>   
>>> +const char *repo_excludes_file(struct repository *repo)
>>> +{
>>> +	if (!excludes_file)
>>> +		excludes_file = xdg_config_home("ignore");
>>> +	return excludes_file;
>>> +}
>>
>> This function has a 'repo' parameter, which is not used in the
>> function at all.  This causes build failure when trying to build this
>> commit using DEVELOPER=1:
>>
>>    environment.c: In function ‘repo_excludes_file’:
>>    environment.c:137:51: error: unused parameter ‘repo’ [-Werror=unused-parameter]
>>      137 | const char *repo_excludes_file(struct repository *repo)
>>          |                                ~~~~~~~~~~~~~~~~~~~^~~~
>>    cc1: all warnings being treated as errors
>>    make: *** [Makefile:2922: environment.o] Error 1
>>
>> Please make sure that all commits can be built with 'make
>> DEVELOPER=1'.
> 
> Good point.  In this case, we can start with UNUSED in step 1/2 and
> then drop the UNUSED in the second step.  I wonder how harder to read
> it would become if these two patches are squashed together...

That makes sense. I think these two commits can be squadshed together... 
I hope so.

> 
> Thanks.

Regards, yuchen

[PATCH v4 0/1] environment: move excludes_file into repo_config_values

[Tian Yuchen]

This patch continues the libification effort by migrating the global
string variable 'excludes_file' into 'struct repo_config_values'. Since
this is a dynamically allocated variable, the migration requires proper
heap memory management.

This patch mainly does three things:

 - Abstract the XDG fallback lazy-loading logic out of dir.c into a proper
 getter.

 - Move the variables into the struct repo_config_values.

 - Introduce the memory destructor 'repo_config_values_clear()'.


Changes since V2:

 - Squash together the previous two commits into one.

 - The 'repo->initialized' check is used in both the getter and destructor.
 This eliminates redundant checks and follows the fail-fast principle. This
 is consistent with the previous global variable removal patches [1][2][3].

Note: Resending as v3 purely to fix a malformed In-Reply-To header in v2
that broke the email thread. No actual code changes since v2...

Thanks!

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>

[1] https://lore.kernel.org/git/20260610093635.139719-1-cat@malon.dev/T/#m856253610936d052a798259bfc06d598561e53c4
[2] https://lore.kernel.org/git/20260606143412.15443-1-cat@malon.dev/
[3] https://lore.kernel.org/git/20260617154929.564498-2-cat@malon.dev/T/#m8843984a6175a1a4c7e00877085c77b0c72f5803

Tian Yuchen (1):
  environment: move excludes_file into repo_config_values

 dir.c         |  4 ++--
 environment.c | 30 +++++++++++++++++++++++++++---
 environment.h | 13 ++++++++++++-
 repository.c  |  1 +
 4 files changed, 42 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH v4 1/1] environment: move excludes_file into repo_config_values

[Tian Yuchen]

The global variable 'excludes_file' is used to track the path to the
global ignore file. If this variable is NULL, setup_standard_excludes()
in dir.c forcefully evaluates and assigns the XDG default path to it.

Continue the libification effort by encapsulating this lazy-loading
fallback logic into a proper getter and moving the variable into
'struct repo_config_values'.

Since 'excludes_file' is a dynamically allocated string, it requires
proper heap memory management. Introduce repo_config_values_clear()
and wire it up in repo_clear() to safely free this memory when a
repository instance is destroyed. Also clean up the heap-allocated
'attributes_file' in this new destructor while we are at it.

Note: 'if (!repo->initialized)' is added in both the getter and the
destructor. This ensures we safely return or bypass cleaning up
uninitialized repositories without hardcoding a dependency on
'the_repository'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 dir.c         |  4 ++--
 environment.c | 30 +++++++++++++++++++++++++++---
 environment.h | 13 ++++++++++++-
 repository.c  |  1 +
 4 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/dir.c b/dir.c
index 7a73690fbc..4f87a52b3c 100644
--- a/dir.c
+++ b/dir.c
@@ -3481,11 +3481,11 @@ static GIT_PATH_FUNC(git_path_info_exclude, "info/exclude")
 
 void setup_standard_excludes(struct dir_struct *dir)
 {
+	const char *excludes_file = repo_excludes_file(the_repository);
+
 	dir->exclude_per_dir = ".gitignore";
 
 	/* core.excludesfile defaulting to $XDG_CONFIG_HOME/git/ignore */
-	if (!excludes_file)
-		excludes_file = xdg_config_home("ignore");
 	if (excludes_file && !access_or_warn(excludes_file, R_OK, 0))
 		add_patterns_from_file_1(dir, excludes_file,
 					 dir->untracked ? &dir->internal.ss_excludes_file : NULL);
diff --git a/environment.c b/environment.c
index ba2c60103f..2519c60918 100644
--- a/environment.c
+++ b/environment.c
@@ -57,7 +57,6 @@ enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
 enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
 char *editor_program;
 char *askpass_program;
-char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
@@ -134,6 +133,17 @@ int is_bare_repository(void)
 	return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
 }
 
+const char *repo_excludes_file(struct repository *repo)
+{
+	if (!repo || !repo->initialized)
+		return NULL;
+
+	if (!repo_config_values(repo)->excludes_file)
+		repo_config_values(repo)->excludes_file = xdg_config_home("ignore");
+
+	return repo_config_values(repo)->excludes_file;
+}
+
 int have_git_dir(void)
 {
 	return startup_info->have_repository
@@ -461,8 +471,8 @@ int git_default_core_config(const char *var, const char *value,
 	}
 
 	if (!strcmp(var, "core.excludesfile")) {
-		FREE_AND_NULL(excludes_file);
-		return git_config_pathname(&excludes_file, var, value);
+		FREE_AND_NULL(cfg->excludes_file);
+		return git_config_pathname(&cfg->excludes_file, var, value);
 	}
 
 	if (!strcmp(var, "core.whitespace")) {
@@ -715,6 +725,7 @@ int git_default_config(const char *var, const char *value,
 void repo_config_values_init(struct repo_config_values *cfg)
 {
 	cfg->attributes_file = NULL;
+	cfg->excludes_file = NULL;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
@@ -726,3 +737,16 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->sparse_expect_files_outside_of_patterns = 0;
 	cfg->warn_on_object_refname_ambiguity = 1;
 }
+
+void repo_config_values_clear(struct repository *repo)
+{
+	struct repo_config_values *cfg;
+
+	if (repo->initialized)
+		return;
+
+	cfg = repo_config_values(repo);
+
+	FREE_AND_NULL(cfg->attributes_file);
+	FREE_AND_NULL(cfg->excludes_file);
+}
diff --git a/environment.h b/environment.h
index 6f18286955..2e8352de7f 100644
--- a/environment.h
+++ b/environment.h
@@ -90,6 +90,7 @@ struct repository;
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
+	char *excludes_file;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -133,8 +134,19 @@ int git_default_config(const char *, const char *,
 int git_default_core_config(const char *var, const char *value,
 			    const struct config_context *ctx, void *cb);
 
+const char *repo_excludes_file(struct repository *repo);
+
 void repo_config_values_init(struct repo_config_values *cfg);
 
+/*
+ * Frees memory allocated for dynamically loaded configuration values
+ * inside `repo_config_values`.
+ *
+ * As dynamically allocated variables are migrated into this struct,
+ * their FREE_AND_NULL() calls should be appended here.
+ */
+void repo_config_values_clear(struct repository *repo);
+
 /*
  * TODO: All the below state either explicitly or implicitly relies on
  * `the_repository`. We should eventually get rid of these and make the
@@ -208,7 +220,6 @@ extern char *git_log_output_encoding;
 
 extern char *editor_program;
 extern char *askpass_program;
-extern char *excludes_file;
 
 /*
  * The character that begins a commented line in user-editable file
diff --git a/repository.c b/repository.c
index 187dd471c4..b31f1b7852 100644
--- a/repository.c
+++ b/repository.c
@@ -388,6 +388,7 @@ void repo_clear(struct repository *repo)
 	FREE_AND_NULL(repo->parsed_objects);
 
 	repo_settings_clear(repo);
+	repo_config_values_clear(repo);
 
 	if (repo->config) {
 		git_configset_clear(repo->config);
-- 
2.43.0

Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values

[Tian Yuchen]

Hi all,

Apologies again for the duplicate...

On 6/28/26 00:08, Tian Yuchen wrote:

> +const char *repo_excludes_file(struct repository *repo)
> +{
> +	if (!repo || !repo->initialized)
> +		return NULL;
> +
> +	if (!repo_config_values(repo)->excludes_file)
> +		repo_config_values(repo)->excludes_file = xdg_config_home("ignore");
> +
> +	return repo_config_values(repo)->excludes_file;
> +}

One more thing:

I deliberately didn't write a comment for the getter because it will 
probably be merged with comments from the previous several patches in 
some form in the near future... I'm not sure if it would be more 
appropriate to write a separate patch to add the corresponding comments 
then.

Regards, yuchen

Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values

[Junio C Hamano]

Tian Yuchen <cat@malon.dev> writes:

> Hi all,
>
> Apologies again for the duplicate...
>
> On 6/28/26 00:08, Tian Yuchen wrote:
>
>> +const char *repo_excludes_file(struct repository *repo)
>> +{
>> +	if (!repo || !repo->initialized)
>> +		return NULL;

I might already have said this, but I am not sure why want to be as
loose as this code.  It is not limited to this line, but I think we
saw plenty of other "We know we must get an already initialized
thing here, and the subsequent operation we perform on that thing
will cause us to die() later, so let's return silently and early
to avoid hitting die()" attempts to sweep problems under the rug.

Wouldn't we rather want to try to be more strict and say

	if (!repo || !repo->initialized)
		BUG("repo must be an initialied repository");

here?  Aren't all the callers of this function supposed to be
dealing with an already initialized repository?


>> +	if (!repo_config_values(repo)->excludes_file)
>> +		repo_config_values(repo)->excludes_file = xdg_config_home("ignore");
>> +
>> +	return repo_config_values(repo)->excludes_file;
>> +}
>
> One more thing:
>
> I deliberately didn't write a comment for the getter because it will 
> probably be merged with comments from the previous several patches in 
> some form in the near future... I'm not sure if it would be more 
> appropriate to write a separate patch to add the corresponding comments 
> then.

That's very sensible.

Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values

[Tian Yuchen]

On 6/28/26 04:47, Junio C Hamano wrote:
> Tian Yuchen <cat@malon.dev> writes:
> 
>> Hi all,
>>
>> Apologies again for the duplicate...
>>
>> On 6/28/26 00:08, Tian Yuchen wrote:
>>
>>> +const char *repo_excludes_file(struct repository *repo)
>>> +{
>>> +	if (!repo || !repo->initialized)
>>> +		return NULL;
> 
> I might already have said this, but I am not sure why want to be as
> loose as this code.  It is not limited to this line, but I think we
> saw plenty of other "We know we must get an already initialized
> thing here, and the subsequent operation we perform on that thing
> will cause us to die() later, so let's return silently and early
> to avoid hitting die()" attempts to sweep problems under the rug.
> 
> Wouldn't we rather want to try to be more strict and say
> 
> 	if (!repo || !repo->initialized)
> 		BUG("repo must be an initialied repository");
> 
> here?  Aren't all the callers of this function supposed to be
> dealing with an already initialized repository?
> 

That makes sense, but from my point of view...

'repo_config_values()' already has a check for 'repo->initialized'. If 
we're absolutely certain that the 'repo' is initialized, wouldn't it be 
better to simply remove all the checks inside the getter and leave the 
judgment to 'repo_config_values()'?

This also aligns to some extent with the previous flag getters: since an 
uninitialized 'repo' will trigger a BUG() in 'repo_config_values()', but 
"reading and writing these flags when the 'repo' is uninitialized" is 
sometimes a valid operation that's why we choose to intercept 
'repo->initialized' _before_ 'repo_config_values()' and fallback to the 
hard-coded values.

This gives the impression that _'repo_config_values()' is the function 
responsible for checking_, and the way flags are handled is an exception 
to this approach, which I think is more consistent and self-explanatory.

What do you think?


> 
>>> +	if (!repo_config_values(repo)->excludes_file)
>>> +		repo_config_values(repo)->excludes_file = xdg_config_home("ignore");
>>> +
>>> +	return repo_config_values(repo)->excludes_file;
>>> +}
>>
>> One more thing:
>>
>> I deliberately didn't write a comment for the getter because it will
>> probably be merged with comments from the previous several patches in
>> some form in the near future... I'm not sure if it would be more
>> appropriate to write a separate patch to add the corresponding comments
>> then.
> 
> That's very sensible.
> 

Thanks.

regards, yuchen

Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values

[Tian Yuchen]

On 6/28/26 11:19, Tian Yuchen wrote:

>  Let it be noticed by repo_config_values() function to
> catch offending callers for now, and once the codebase becomes ready
> to use one repo_config_values per repository, this function does not
> have to change.

And

> Wouldn't we rather want to try to be more strict and say
> 
> 	if (!repo || !repo->initialized)
> 		BUG("repo must be an initialied repository");
> 
> here?  Aren't all the callers of this function supposed to be
> dealing with an already initialized repository?

In my opinion, these two suggestions are not entirely consistent, and I 
think we need to determine the most appropriate approach.

Regards, yuchen

Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values

[Junio C Hamano]

Tian Yuchen <cat@malon.dev> writes:

>> Wouldn't we rather want to try to be more strict and say
>> 
>> 	if (!repo || !repo->initialized)
>> 		BUG("repo must be an initialied repository");
>> 
>> here?  Aren't all the callers of this function supposed to be
>> dealing with an already initialized repository?
>
> That makes sense, but from my point of view...
>
> 'repo_config_values()' already has a check for 'repo->initialized'. If 
> we're absolutely certain that the 'repo' is initialized, wouldn't it be 
> better to simply remove all the checks inside the getter and leave the 
> judgment to 'repo_config_values()'?

Yes, that was what I was getting at ;-).

Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values

[Tian Yuchen]

On 6/28/26 16:40, Junio C Hamano wrote:
> Tian Yuchen <cat@malon.dev> writes:
> 
>>> Wouldn't we rather want to try to be more strict and say
>>>
>>> 	if (!repo || !repo->initialized)
>>> 		BUG("repo must be an initialied repository");
>>>
>>> here?  Aren't all the callers of this function supposed to be
>>> dealing with an already initialized repository?
>>
>> That makes sense, but from my point of view...
>>
>> 'repo_config_values()' already has a check for 'repo->initialized'. If
>> we're absolutely certain that the 'repo' is initialized, wouldn't it be
>> better to simply remove all the checks inside the getter and leave the
>> judgment to 'repo_config_values()'?
> 
> Yes, that was what I was getting at ;-).

A lot of CI tests are failing, but that just goes to show that the 
"bugs" are being properly identified, doesn’t it?

It means there are a lot of "invalid" calls in the tests (if the way we 
define a 'valid' call, i.e. repo must be initialized, is correct)... It 
seems that code like 'if (repo != the_repository) return' or something 
similar is inevitably going to end up somewhere, even though, as you 
said, it’s "sweeping problems under the rug."

I’m not sure how to proceed from here either..

Regards, yuchen

Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values

[Christian Couder]

On Sun, Jun 28, 2026 at 2:58 PM Tian Yuchen <cat@malon.dev> wrote:
>
> On 6/28/26 16:40, Junio C Hamano wrote:
> > Tian Yuchen <cat@malon.dev> writes:
> >
> >>> Wouldn't we rather want to try to be more strict and say
> >>>
> >>>     if (!repo || !repo->initialized)
> >>>             BUG("repo must be an initialied repository");
> >>>
> >>> here?  Aren't all the callers of this function supposed to be
> >>> dealing with an already initialized repository?
> >>
> >> That makes sense, but from my point of view...
> >>
> >> 'repo_config_values()' already has a check for 'repo->initialized'. If
> >> we're absolutely certain that the 'repo' is initialized, wouldn't it be
> >> better to simply remove all the checks inside the getter and leave the
> >> judgment to 'repo_config_values()'?
> >
> > Yes, that was what I was getting at ;-).
>
> A lot of CI tests are failing, but that just goes to show that the
> "bugs" are being properly identified, doesn’t it?
>
> It means there are a lot of "invalid" calls in the tests (if the way we
> define a 'valid' call, i.e. repo must be initialized, is correct)... It
> seems that code like 'if (repo != the_repository) return' or something
> similar is inevitably going to end up somewhere, even though, as you
> said, it’s "sweeping problems under the rug."
>
> I’m not sure how to proceed from here either..

I agree that the best end state would be to have no `if (!repo ||
!repo->initialized)` check, but we shouldn't have to get there right
away. I think it's fine to proceed in several steps:

1) `if (!repo || !repo->initialized) return NULL;` allows us to remove
the global variable and use getters which will help us in the next
step.

2) `if (!repo || !repo->initialized) return BUG("repo must be an
initialized repository");` now we want to find and fix callers
(including tests) that haven't properly initialized things.

3) No `if (!repo || !repo->initialized)` check, as we are sure that
all the callers that didn't properly initialized things have been
found and fixed.

So I think 1) is fine for now as long as we properly explain in the
commit messages and in code comments (maybe using NEEDSWORK comments)
that we know there is more work to do on this in the future.

Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

> I agree that the best end state would be to have no `if (!repo ||
> !repo->initialized)` check, but we shouldn't have to get there right
> away. I think it's fine to proceed in several steps:
>
> 1) `if (!repo || !repo->initialized) return NULL;` allows us to remove
> the global variable and use getters which will help us in the next
> step.
>
> 2) `if (!repo || !repo->initialized) return BUG("repo must be an
> initialized repository");` now we want to find and fix callers
> (including tests) that haven't properly initialized things.
>
> 3) No `if (!repo || !repo->initialized)` check, as we are sure that
> all the callers that didn't properly initialized things have been
> found and fixed.
>
> So I think 1) is fine for now as long as we properly explain in the
> commit messages and in code comments (maybe using NEEDSWORK comments)
> that we know there is more work to do on this in the future.

I am OK with the progressive improvements, but if "wean us away from
global variables" topic stops at step 1 I would have to say that is
a failed experiment.  Not doing (2) means you made the system bug-to-bug
compatible from the old world where these things weren't in repo-config
but were still globals, which is code churn for nothing good to show
in the end result.  We need to get to (2) before declaring victory.

But of course, we need to start somewhere.  (1) with in-code
comments sprinkled to such places that say that we'd need to revisit
would be a good place to start.

Thanks.

Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values

[Tian Yuchen]

Hi Christian and Junio,

On 6/29/26 22:47, Junio C Hamano wrote:
> Christian Couder <christian.couder@gmail.com> writes:
> 
>> I agree that the best end state would be to have no `if (!repo ||
>> !repo->initialized)` check, but we shouldn't have to get there right
>> away. I think it's fine to proceed in several steps:
>>
>> 1) `if (!repo || !repo->initialized) return NULL;` allows us to remove
>> the global variable and use getters which will help us in the next
>> step.
>>
>> 2) `if (!repo || !repo->initialized) return BUG("repo must be an
>> initialized repository");` now we want to find and fix callers
>> (including tests) that haven't properly initialized things.
>>
>> 3) No `if (!repo || !repo->initialized)` check, as we are sure that
>> all the callers that didn't properly initialized things have been
>> found and fixed.
>>
>> So I think 1) is fine for now as long as we properly explain in the
>> commit messages and in code comments (maybe using NEEDSWORK comments)
>> that we know there is more work to do on this in the future.
> 
> I am OK with the progressive improvements, but if "wean us away from
> global variables" topic stops at step 1 I would have to say that is
> a failed experiment.  Not doing (2) means you made the system bug-to-bug
> compatible from the old world where these things weren't in repo-config
> but were still globals, which is code churn for nothing good to show
> in the end result.  We need to get to (2) before declaring victory.
> 
> But of course, we need to start somewhere.  (1) with in-code
> comments sprinkled to such places that say that we'd need to revisit
> would be a good place to start.
> 
> Thanks.

Thank you both for paving a clear way forward.

For the upcoming V5 patch, I will implement:

1. Revert to the shields ('return NULL' for the getter, and bypassing 
'repo != repository' for the _clear()).
2. Add 'NEEDSWORK' comments above them documenting that these are 
temporary changes.
3. Update the commit message to reflect this.

Once this initial migration safely lands, the next goal will be to 
investigate those failing CI tests.

Will send out V5 shortly.

Regards, yuchen

[PATCH v5 0/1] environment: move excludes_file into repo_config_values

[Tian Yuchen]

This patch continues the libification effort by migrating the global
string variable 'excludes_file' into 'struct repo_config_values'. Since
this is a dynamically allocated variable, the migration requires proper
heap memory management.

This patch mainly does three things:

 - Abstract the XDG fallback lazy-loading logic out of dir.c into a proper
 getter.

 - Move the variables into the struct repo_config_values.

 - Introduce the memory destructor 'repo_config_values_clear()'.

Changes since V4:

Defensive checks are retained in both the getter (returning NULL if
uninitialized) and the destructor (bypassing non-the_repository instances)
to maintain bug-to-bug compatibility. These are marked with 'NEEDSWORK'
comments.

Future work will track down and fix the offending callers to eventually
replace these shields with stricter BUG() assertions. [1]

THANKS!

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>

[1] https://lore.kernel.org/git/054b3fb6-0e69-473d-9778-b1b11ea82b3a@malon.dev/T/#mca0730b6735298316ec00a53d4719935d18837a6

Tian Yuchen (1):
  environment: move excludes_file into repo_config_values

 dir.c         |  4 ++--
 environment.c | 43 ++++++++++++++++++++++++++++++++++++++++---
 environment.h | 13 ++++++++++++-
 repository.c  |  1 +
 4 files changed, 55 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH v5 1/1] environment: move excludes_file into repo_config_values

[Tian Yuchen]

The global variable 'excludes_file' is used to track the path to the
global ignore file. If this variable is NULL, 'setup_standard_excludes()'
in 'dir.c' forcefully evaluates and assigns the XDG default path to it.

Continue the libification effort by encapsulating this lazy-loading
fallback logic into a proper getter and moving the variable into
'struct repo_config_values'.

Since 'excludes_file' is a dynamically allocated string, it requires
proper heap memory management. Introduce repo_config_values_clear()
and wire it up in 'repo_clear()' to safely free this memory when a
repository instance is destroyed. Also clean up the heap-allocated
'attributes_file' in this new destructor while we are at it.

Note on transition:

Defensive checks are temporarily retained in both the getter (returning
NULL if uninitialized) and the destructor (bypassing non-the_repository
instances) to maintain bug-to-bug compatibility. These are marked with
NEEDSWORK comments. Future work will track down and fix the offending
callers to eventually replace these shields with stricter BUG()
assertions.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 dir.c         |  4 ++--
 environment.c | 43 ++++++++++++++++++++++++++++++++++++++++---
 environment.h | 13 ++++++++++++-
 repository.c  |  1 +
 4 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/dir.c b/dir.c
index 7a73690fbc..4f87a52b3c 100644
--- a/dir.c
+++ b/dir.c
@@ -3481,11 +3481,11 @@ static GIT_PATH_FUNC(git_path_info_exclude, "info/exclude")
 
 void setup_standard_excludes(struct dir_struct *dir)
 {
+	const char *excludes_file = repo_excludes_file(the_repository);
+
 	dir->exclude_per_dir = ".gitignore";
 
 	/* core.excludesfile defaulting to $XDG_CONFIG_HOME/git/ignore */
-	if (!excludes_file)
-		excludes_file = xdg_config_home("ignore");
 	if (excludes_file && !access_or_warn(excludes_file, R_OK, 0))
 		add_patterns_from_file_1(dir, excludes_file,
 					 dir->untracked ? &dir->internal.ss_excludes_file : NULL);
diff --git a/environment.c b/environment.c
index ba2c60103f..5fc13d47c2 100644
--- a/environment.c
+++ b/environment.c
@@ -57,7 +57,6 @@ enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
 enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
 char *editor_program;
 char *askpass_program;
-char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
@@ -134,6 +133,24 @@ int is_bare_repository(void)
 	return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
 }
 
+const char *repo_excludes_file(struct repository *repo)
+{
+	/*
+	 * NEEDSWORK: This is a temporary shield to maintain bug-to-bug
+	 * compatibility during the libification transition.
+	 *
+	 * Once offending callers are properly fixed, this check should
+	 * be upgraded to a BUG() assertion and eventually removed entirely.
+	 */
+	if (!repo || !repo->initialized)
+		return NULL;
+
+	if (!repo_config_values(repo)->excludes_file)
+		repo_config_values(repo)->excludes_file = xdg_config_home("ignore");
+
+	return repo_config_values(repo)->excludes_file;
+}
+
 int have_git_dir(void)
 {
 	return startup_info->have_repository
@@ -461,8 +478,8 @@ int git_default_core_config(const char *var, const char *value,
 	}
 
 	if (!strcmp(var, "core.excludesfile")) {
-		FREE_AND_NULL(excludes_file);
-		return git_config_pathname(&excludes_file, var, value);
+		FREE_AND_NULL(cfg->excludes_file);
+		return git_config_pathname(&cfg->excludes_file, var, value);
 	}
 
 	if (!strcmp(var, "core.whitespace")) {
@@ -715,6 +732,7 @@ int git_default_config(const char *var, const char *value,
 void repo_config_values_init(struct repo_config_values *cfg)
 {
 	cfg->attributes_file = NULL;
+	cfg->excludes_file = NULL;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
@@ -726,3 +744,22 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->sparse_expect_files_outside_of_patterns = 0;
 	cfg->warn_on_object_refname_ambiguity = 1;
 }
+
+void repo_config_values_clear(struct repository *repo)
+{
+	struct repo_config_values *cfg;
+
+	/*
+	 * NEEDSWORK: Temporary shield to prevent temporary/uninitialized
+	 * submodules from triggering the BUG() at repository.c:59
+	 * during repo_clear(). This should be removed once submodule
+	 * lifecycle and per-repo config support are fully resolved.
+	 */
+	if (repo != the_repository)
+		return;
+
+	cfg = repo_config_values(repo);
+
+	FREE_AND_NULL(cfg->attributes_file);
+	FREE_AND_NULL(cfg->excludes_file);
+}
diff --git a/environment.h b/environment.h
index 6f18286955..2e8352de7f 100644
--- a/environment.h
+++ b/environment.h
@@ -90,6 +90,7 @@ struct repository;
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
+	char *excludes_file;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -133,8 +134,19 @@ int git_default_config(const char *, const char *,
 int git_default_core_config(const char *var, const char *value,
 			    const struct config_context *ctx, void *cb);
 
+const char *repo_excludes_file(struct repository *repo);
+
 void repo_config_values_init(struct repo_config_values *cfg);
 
+/*
+ * Frees memory allocated for dynamically loaded configuration values
+ * inside `repo_config_values`.
+ *
+ * As dynamically allocated variables are migrated into this struct,
+ * their FREE_AND_NULL() calls should be appended here.
+ */
+void repo_config_values_clear(struct repository *repo);
+
 /*
  * TODO: All the below state either explicitly or implicitly relies on
  * `the_repository`. We should eventually get rid of these and make the
@@ -208,7 +220,6 @@ extern char *git_log_output_encoding;
 
 extern char *editor_program;
 extern char *askpass_program;
-extern char *excludes_file;
 
 /*
  * The character that begins a commented line in user-editable file
diff --git a/repository.c b/repository.c
index 187dd471c4..b31f1b7852 100644
--- a/repository.c
+++ b/repository.c
@@ -388,6 +388,7 @@ void repo_clear(struct repository *repo)
 	FREE_AND_NULL(repo->parsed_objects);
 
 	repo_settings_clear(repo);
+	repo_config_values_clear(repo);
 
 	if (repo->config) {
 		git_configset_clear(repo->config);
-- 
2.43.0

[PATCH v6 0/1] environment: move excludes_file into repo_config_values

[Tian Yuchen]

This patch continues the libification effort by migrating the global
string variable 'excludes_file' into 'struct repo_config_values'. Since
this is a dynamically allocated variable, the migration requires proper
heap memory management.

This patch mainly does three things:

 - Abstract the XDG fallback lazy-loading logic out of dir.c into a proper
 getter.

 - Move the variables into the struct repo_config_values.

 - Introduce the memory destructor 'repo_config_values_clear()'.

Changes since V5:

Drop the defensive check in 'repo_excludes_file()'.

All known test errors that occur *without* defensive checks are intercepted
by the 'repo != the_repository' check when checks *are there*, rather than
by the '!repo->initialized' check. In other words, the getter actually doesn't
need any checks added, because all of its calls are safe. We just need to
keep the check and the 'NEEDSWORK' comment *on the destructor* until
'repo_config_values()' eventually supports submodules, at which point we can
remove them.

THANKS!

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>

Tian Yuchen (1):
  environment: move excludes_file into repo_config_values

 dir.c         |  4 ++--
 environment.c | 33 ++++++++++++++++++++++++++++++---
 environment.h | 13 ++++++++++++-
 repository.c  |  1 +
 4 files changed, 45 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH v6 1/1] environment: move excludes_file into repo_config_values

[Tian Yuchen]

The global variable 'excludes_file' is used to track the path to the
global ignore file. If this variable is NULL, 'setup_standard_excludes()'
in 'dir.c' forcefully evaluates and assigns the XDG default path to it.

Continue the libification effort by encapsulating this lazy-loading
fallback logic into a proper getter and moving the variable into
'struct repo_config_values'.

Since 'excludes_file' is a dynamically allocated string, it requires
proper heap memory management. Introduce repo_config_values_clear()
and wire it up in 'repo_clear()' to safely free this memory when a
repository instance is destroyed. Also clean up the heap-allocated
'attributes_file' in this new destructor while we are at it.

Note on transition:

Submodules are currently not supported by repo_config_values().
Since repo_clear() cleans up all repository instances (including
submodules), we must bypass them in repo_config_values_clear() to
prevent hitting the BUG() in repository.c:59. In the future when
submodules are supported, this check should be removed.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 dir.c         |  4 ++--
 environment.c | 33 ++++++++++++++++++++++++++++++---
 environment.h | 13 ++++++++++++-
 repository.c  |  1 +
 4 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/dir.c b/dir.c
index 7a73690fbc..4f87a52b3c 100644
--- a/dir.c
+++ b/dir.c
@@ -3481,11 +3481,11 @@ static GIT_PATH_FUNC(git_path_info_exclude, "info/exclude")
 
 void setup_standard_excludes(struct dir_struct *dir)
 {
+	const char *excludes_file = repo_excludes_file(the_repository);
+
 	dir->exclude_per_dir = ".gitignore";
 
 	/* core.excludesfile defaulting to $XDG_CONFIG_HOME/git/ignore */
-	if (!excludes_file)
-		excludes_file = xdg_config_home("ignore");
 	if (excludes_file && !access_or_warn(excludes_file, R_OK, 0))
 		add_patterns_from_file_1(dir, excludes_file,
 					 dir->untracked ? &dir->internal.ss_excludes_file : NULL);
diff --git a/environment.c b/environment.c
index ba2c60103f..f1f859dd08 100644
--- a/environment.c
+++ b/environment.c
@@ -57,7 +57,6 @@ enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
 enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
 char *editor_program;
 char *askpass_program;
-char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
@@ -134,6 +133,14 @@ int is_bare_repository(void)
 	return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
 }
 
+const char *repo_excludes_file(struct repository *repo)
+{
+	if (!repo_config_values(repo)->excludes_file)
+		repo_config_values(repo)->excludes_file = xdg_config_home("ignore");
+
+	return repo_config_values(repo)->excludes_file;
+}
+
 int have_git_dir(void)
 {
 	return startup_info->have_repository
@@ -461,8 +468,8 @@ int git_default_core_config(const char *var, const char *value,
 	}
 
 	if (!strcmp(var, "core.excludesfile")) {
-		FREE_AND_NULL(excludes_file);
-		return git_config_pathname(&excludes_file, var, value);
+		FREE_AND_NULL(cfg->excludes_file);
+		return git_config_pathname(&cfg->excludes_file, var, value);
 	}
 
 	if (!strcmp(var, "core.whitespace")) {
@@ -715,6 +722,7 @@ int git_default_config(const char *var, const char *value,
 void repo_config_values_init(struct repo_config_values *cfg)
 {
 	cfg->attributes_file = NULL;
+	cfg->excludes_file = NULL;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
@@ -726,3 +734,22 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->sparse_expect_files_outside_of_patterns = 0;
 	cfg->warn_on_object_refname_ambiguity = 1;
 }
+
+void repo_config_values_clear(struct repository *repo)
+{
+	struct repo_config_values *cfg;
+
+	/*
+	 * NEEDSWORK: Submodules are currently not supported by
+	 * repo_config_values(). Since repo_clear() cleans up all
+	 * repository instances (including submodules), we must bypass
+	 * them here to prevent hitting the BUG() in repository.c:59.
+	 */
+	if (repo != the_repository)
+		return;
+
+	cfg = repo_config_values(repo);
+
+	FREE_AND_NULL(cfg->attributes_file);
+	FREE_AND_NULL(cfg->excludes_file);
+}
diff --git a/environment.h b/environment.h
index 6f18286955..2e8352de7f 100644
--- a/environment.h
+++ b/environment.h
@@ -90,6 +90,7 @@ struct repository;
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
+	char *excludes_file;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -133,8 +134,19 @@ int git_default_config(const char *, const char *,
 int git_default_core_config(const char *var, const char *value,
 			    const struct config_context *ctx, void *cb);
 
+const char *repo_excludes_file(struct repository *repo);
+
 void repo_config_values_init(struct repo_config_values *cfg);
 
+/*
+ * Frees memory allocated for dynamically loaded configuration values
+ * inside `repo_config_values`.
+ *
+ * As dynamically allocated variables are migrated into this struct,
+ * their FREE_AND_NULL() calls should be appended here.
+ */
+void repo_config_values_clear(struct repository *repo);
+
 /*
  * TODO: All the below state either explicitly or implicitly relies on
  * `the_repository`. We should eventually get rid of these and make the
@@ -208,7 +220,6 @@ extern char *git_log_output_encoding;
 
 extern char *editor_program;
 extern char *askpass_program;
-extern char *excludes_file;
 
 /*
  * The character that begins a commented line in user-editable file
diff --git a/repository.c b/repository.c
index 187dd471c4..b31f1b7852 100644
--- a/repository.c
+++ b/repository.c
@@ -388,6 +388,7 @@ void repo_clear(struct repository *repo)
 	FREE_AND_NULL(repo->parsed_objects);
 
 	repo_settings_clear(repo);
+	repo_config_values_clear(repo);
 
 	if (repo->config) {
 		git_configset_clear(repo->config);
-- 
2.43.0

Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values

[Tian Yuchen]

On 7/1/26 00:20, Tian Yuchen wrote:
> 
> Thank you both for paving a clear way forward.
> 
> For the upcoming V5 patch, I will implement:
> 
> 1. Revert to the shields ('return NULL' for the getter, and bypassing 
> 'repo != repository' for the _clear()).
> 2. Add 'NEEDSWORK' comments above them documenting that these are 
> temporary changes.
> 3. Update the commit message to reflect this.
> 
> Once this initial migration safely lands, the next goal will be to 
> investigate those failing CI tests.
> 
> Will send out V5 shortly.
> 
> Regards, yuchen
> 

Sorry, I forgot to test the repo_excludes_file() and 
repo_config_values_clear() with/without checks separately earlier. As 
for repo_excludes_files(), I found that the check wasn't necessary, so I 
removed it in v6.

In other words, we're now only filtering out calls that aren't passing 
'the_repository' to _clear(), which is still a task to be addressed in 
the future. I think the current situation should be fairly satisfactory.

Hope this helps.

Regards, yuchen

[PATCH v7 0/9] migrate more variables into repo_config_values

[Tian Yuchen]

Hi everyone,

This patch series continues the ongoing libification effort by migrating
a batch of global configuration variables into struct repo_config_values.

What does this series do:

infrastructure & strings (commits 1-6):
Introduce 'repo_config_values_clear()' to manage the lifecycle
of heap-allocated configuration strings. This infrastructure is utilized
to migrate string variables, including 'excludes_file', 'apply' whitespace
configs, and external programs including 'editor', 'pager', 'askpass'.

enums (commits 7-9):
Migrate enumerations 'push_default', 'autorebase', and
'object_creation_mode'. Care was taken to make these types available
to the configuration structure without triggering circular header
dependencies.

RFC:

Commit 3~5. Is it really necessary to migrate _program variables?
https://lore.kernel.org/git/8e657184-ee0b-453a-9f2d-a98080d3582e@gmail.com/

Commit 6~9. Previous related discussions on 'git_branch_track'.
https://lore.kernel.org/git/CAD=f0L-mPX+KECUjXk-WBzEbTP7wCa8sB56GySQT0yh9mfUOWw@mail.gmail.com/

Note:

Since a new getter 'repo_excludes_file()' is introduced, as previously
promised, once it is finally merged into 'master', there will be a patch to
update and squash the comments.

Similarly, I've noticed that the classification and sorting of variables in
'repo_config_values' don't seem to be correct. There will also be a patch
to fix this, and I think it will form a commit series along with the comment
patch?

Changes since v6:

Only the first two commits in this patch overlap with v6. The reason the
subsequent commits were not released separately is that Christian suggested
placing the introduction of 'repo_config_values_clear()' as a standalone
commit at the very beginning.

In other words, the v6 structure has been discarded, and this series is
being released as almost a new patch.

Thanks!

Tian Yuchen (9):
  repository: introduce repo_config_values_clear()
  environment: move excludes_file into repo_config_values
  environment: move editor_program into repo_config_values
  environment: move pager_program into repo_config_values
  environment: move askpass_program into repo_config_values
  environment: migrate apply_default_whitespace and
    apply_default_ignorewhitespace
  environment: move push_default into repo_config_values
  environment: move autorebase into repo_config_values
  environment: move object_creation_mode into repo_config_values

 apply.c        | 20 +++++++-----
 branch.c       |  2 +-
 builtin/push.c |  8 ++---
 dir.c          |  4 +--
 editor.c       |  4 +--
 environment.c  | 87 +++++++++++++++++++++++++++++++++++---------------
 environment.h  | 75 +++++++++++++++++++++++++++----------------
 object-file.c  |  2 +-
 pager.c        | 17 ++++++----
 prompt.c       |  3 +-
 remote.c       |  2 +-
 repository.c   |  1 +
 12 files changed, 145 insertions(+), 80 deletions(-)

-- 
2.43.0

[PATCH v7 1/9] repository: introduce repo_config_values_clear()

[Tian Yuchen]

As part of the ongoing libification effort, dynamically allocated
global configuration variables are being moved into
'struct repo_config_values'. To prevent memory leaks, we need a
destructor to free these heap-allocated variables when a repository
instance is torn down.

Introduce 'repo_config_values_clear()' in environment.c and invoke it
from 'repo_clear()' in repository.c. As a starting point, update this
new function to handle the cleanup of 'attributes_file'.

Note:

Submodules are currently not supported by repo_config_values(), which
explicitly BUG()s out if 'repo != the_repository'. Since repo_clear()
cleans up all repository instances, we must bypass them to prevent
crashing.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 environment.c | 19 +++++++++++++++++++
 environment.h |  9 +++++++++
 repository.c  |  1 +
 3 files changed, 29 insertions(+)

diff --git a/environment.c b/environment.c
index ba2c60103f..13677484de 100644
--- a/environment.c
+++ b/environment.c
@@ -726,3 +726,22 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->sparse_expect_files_outside_of_patterns = 0;
 	cfg->warn_on_object_refname_ambiguity = 1;
 }
+
+void repo_config_values_clear(struct repository *repo)
+{
+	struct repo_config_values *cfg;
+
+	/*
+	 * NEEDSWORK: Submodules are currently not supported by
+	 * repo_config_values(), which explicitly BUG()s out if
+	 * repo != the_repository. Since repo_clear() cleans up all
+	 * repository instances, we must bypass them here to prevent
+	 * crashing.
+	 */
+	if (repo != the_repository)
+		return;
+
+	cfg = repo_config_values(repo);
+
+	FREE_AND_NULL(cfg->attributes_file);
+}
diff --git a/environment.h b/environment.h
index 6f18286955..c4a6a45704 100644
--- a/environment.h
+++ b/environment.h
@@ -135,6 +135,15 @@ int git_default_core_config(const char *var, const char *value,
 
 void repo_config_values_init(struct repo_config_values *cfg);
 
+/*
+ * Frees memory allocated for dynamically loaded configuration values
+ * inside `repo_config_values`.
+ *
+ * As dynamically allocated variables are migrated into this struct,
+ * their FREE_AND_NULL() calls should be appended here.
+ */
+void repo_config_values_clear(struct repository *repo);
+
 /*
  * TODO: All the below state either explicitly or implicitly relies on
  * `the_repository`. We should eventually get rid of these and make the
diff --git a/repository.c b/repository.c
index 187dd471c4..b31f1b7852 100644
--- a/repository.c
+++ b/repository.c
@@ -388,6 +388,7 @@ void repo_clear(struct repository *repo)
 	FREE_AND_NULL(repo->parsed_objects);
 
 	repo_settings_clear(repo);
+	repo_config_values_clear(repo);
 
 	if (repo->config) {
 		git_configset_clear(repo->config);
-- 
2.43.0

[PATCH v7 2/9] environment: move excludes_file into repo_config_values

[Tian Yuchen]

The global variable 'excludes_file' is used to track the path to the
global ignore file. If this variable is NULL,
'setup_standard_excludes()'
in 'dir.c' forcefully evaluates and assigns the XDG default path to it.

Continue the libification effort by encapsulating this lazy-loading
fallback logic into a proper getter and moving the variable into
'struct repo_config_values'.

Since 'excludes_file' is a dynamically allocated string, it requires
proper heap memory management. It is safely freed using the newly
introduced `repo_config_values_clear()` function when the repository
is torn down.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 dir.c         |  4 ++--
 environment.c | 15 ++++++++++++---
 environment.h |  4 +++-
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/dir.c b/dir.c
index 7a73690fbc..4f87a52b3c 100644
--- a/dir.c
+++ b/dir.c
@@ -3481,11 +3481,11 @@ static GIT_PATH_FUNC(git_path_info_exclude, "info/exclude")
 
 void setup_standard_excludes(struct dir_struct *dir)
 {
+	const char *excludes_file = repo_excludes_file(the_repository);
+
 	dir->exclude_per_dir = ".gitignore";
 
 	/* core.excludesfile defaulting to $XDG_CONFIG_HOME/git/ignore */
-	if (!excludes_file)
-		excludes_file = xdg_config_home("ignore");
 	if (excludes_file && !access_or_warn(excludes_file, R_OK, 0))
 		add_patterns_from_file_1(dir, excludes_file,
 					 dir->untracked ? &dir->internal.ss_excludes_file : NULL);
diff --git a/environment.c b/environment.c
index 13677484de..5950592d63 100644
--- a/environment.c
+++ b/environment.c
@@ -57,7 +57,6 @@ enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
 enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
 char *editor_program;
 char *askpass_program;
-char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
@@ -134,6 +133,14 @@ int is_bare_repository(void)
 	return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
 }
 
+const char *repo_excludes_file(struct repository *repo)
+{
+	if (!repo_config_values(repo)->excludes_file)
+		repo_config_values(repo)->excludes_file = xdg_config_home("ignore");
+
+	return repo_config_values(repo)->excludes_file;
+}
+
 int have_git_dir(void)
 {
 	return startup_info->have_repository
@@ -461,8 +468,8 @@ int git_default_core_config(const char *var, const char *value,
 	}
 
 	if (!strcmp(var, "core.excludesfile")) {
-		FREE_AND_NULL(excludes_file);
-		return git_config_pathname(&excludes_file, var, value);
+		FREE_AND_NULL(cfg->excludes_file);
+		return git_config_pathname(&cfg->excludes_file, var, value);
 	}
 
 	if (!strcmp(var, "core.whitespace")) {
@@ -715,6 +722,7 @@ int git_default_config(const char *var, const char *value,
 void repo_config_values_init(struct repo_config_values *cfg)
 {
 	cfg->attributes_file = NULL;
+	cfg->excludes_file = NULL;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
@@ -744,4 +752,5 @@ void repo_config_values_clear(struct repository *repo)
 	cfg = repo_config_values(repo);
 
 	FREE_AND_NULL(cfg->attributes_file);
+	FREE_AND_NULL(cfg->excludes_file);
 }
diff --git a/environment.h b/environment.h
index c4a6a45704..2e8352de7f 100644
--- a/environment.h
+++ b/environment.h
@@ -90,6 +90,7 @@ struct repository;
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
+	char *excludes_file;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -133,6 +134,8 @@ int git_default_config(const char *, const char *,
 int git_default_core_config(const char *var, const char *value,
 			    const struct config_context *ctx, void *cb);
 
+const char *repo_excludes_file(struct repository *repo);
+
 void repo_config_values_init(struct repo_config_values *cfg);
 
 /*
@@ -217,7 +220,6 @@ extern char *git_log_output_encoding;
 
 extern char *editor_program;
 extern char *askpass_program;
-extern char *excludes_file;
 
 /*
  * The character that begins a commented line in user-editable file
-- 
2.43.0

[PATCH v7 3/9] environment: move editor_program into repo_config_values

[Tian Yuchen]

The global variable 'editor_program' holds the path to the user's
preferred editor. Move 'editor_program' into
'struct repo_config_values' to continue the libification effort.

There have been discussions on whether external programs like
editors truly need to be configured on a per-repository basis within
the same process. While a single process might rarely invoke
different editors, this migration is necessary for two reasons:

1. Developers frequently use different toolchains for different
   projects. Per-repo configuration respects this.

2. Moving this string into 'repo_config_values' eliminates mutable
   global state. As the codebase moves toward becoming a long-running
   processes managing multiple repositories concurrently must
   not overwrite each other's program configurations.

No standalone getter function is introduced. Callers directly access
the field via 'repo_config_values()'. Heap memory is safely reclaimed
in 'repo_config_values_clear()'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 editor.c      | 4 ++--
 environment.c | 7 ++++---
 environment.h | 2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/editor.c b/editor.c
index fd174e6a03..07d264cba0 100644
--- a/editor.c
+++ b/editor.c
@@ -29,8 +29,8 @@ const char *git_editor(void)
 	const char *editor = getenv("GIT_EDITOR");
 	int terminal_is_dumb = is_terminal_dumb();
 
-	if (!editor && editor_program)
-		editor = editor_program;
+	if (!editor && repo_config_values(the_repository)->editor_program)
+		editor = repo_config_values(the_repository)->editor_program;
 	if (!editor && !terminal_is_dumb)
 		editor = getenv("VISUAL");
 	if (!editor)
diff --git a/environment.c b/environment.c
index 5950592d63..0a01f4761a 100644
--- a/environment.c
+++ b/environment.c
@@ -55,7 +55,6 @@ int fsync_object_files = -1;
 int use_fsync = -1;
 enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
 enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
-char *editor_program;
 char *askpass_program;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 enum eol core_eol = EOL_UNSET;
@@ -435,8 +434,8 @@ int git_default_core_config(const char *var, const char *value,
 	}
 
 	if (!strcmp(var, "core.editor")) {
-		FREE_AND_NULL(editor_program);
-		return git_config_string(&editor_program, var, value);
+		FREE_AND_NULL(cfg->editor_program);
+		return git_config_string(&cfg->editor_program, var, value);
 	}
 
 	if (!strcmp(var, "core.commentchar") ||
@@ -723,6 +722,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 {
 	cfg->attributes_file = NULL;
 	cfg->excludes_file = NULL;
+	cfg->editor_program = NULL;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
@@ -753,4 +753,5 @@ void repo_config_values_clear(struct repository *repo)
 
 	FREE_AND_NULL(cfg->attributes_file);
 	FREE_AND_NULL(cfg->excludes_file);
+	FREE_AND_NULL(cfg->editor_program);
 }
diff --git a/environment.h b/environment.h
index 2e8352de7f..1ec19149cb 100644
--- a/environment.h
+++ b/environment.h
@@ -91,6 +91,7 @@ struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
 	char *excludes_file;
+	char *editor_program;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -218,7 +219,6 @@ const char *get_commit_output_encoding(void);
 extern char *git_commit_encoding;
 extern char *git_log_output_encoding;
 
-extern char *editor_program;
 extern char *askpass_program;
 
 /*
-- 
2.43.0

[PATCH v7 4/9] environment: move pager_program into repo_config_values

[Tian Yuchen]

The 'pager_program' variable is currently defined as a file-scoped
static string in pager.c. Move it into 'struct repo_config_values'.

The configuration parsing logic remains strictly within pager.c to
respect subsystem boundaries. The read/write operations are simply
redirected to the repository-specific structure using
'repo_config_values()'.

Similar to the recent editor_program migration, no standalone getter
is introduced to keep the code minimal. The dynamically allocated
memory is now managed by 'repo_config_values_clear()'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 environment.c |  2 ++
 environment.h |  1 +
 pager.c       | 17 ++++++++++-------
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/environment.c b/environment.c
index 0a01f4761a..a1204fdcb2 100644
--- a/environment.c
+++ b/environment.c
@@ -723,6 +723,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->attributes_file = NULL;
 	cfg->excludes_file = NULL;
 	cfg->editor_program = NULL;
+	cfg->pager_program = NULL;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
@@ -754,4 +755,5 @@ void repo_config_values_clear(struct repository *repo)
 	FREE_AND_NULL(cfg->attributes_file);
 	FREE_AND_NULL(cfg->excludes_file);
 	FREE_AND_NULL(cfg->editor_program);
+	FREE_AND_NULL(cfg->pager_program);
 }
diff --git a/environment.h b/environment.h
index 1ec19149cb..22f6697c52 100644
--- a/environment.h
+++ b/environment.h
@@ -92,6 +92,7 @@ struct repo_config_values {
 	char *attributes_file;
 	char *excludes_file;
 	char *editor_program;
+	char *pager_program;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
diff --git a/pager.c b/pager.c
index 35b210e048..450ad053b6 100644
--- a/pager.c
+++ b/pager.c
@@ -5,6 +5,8 @@
 #include "run-command.h"
 #include "sigchain.h"
 #include "alias.h"
+#include "repository.h"
+#include "environment.h"
 
 int pager_use_color = 1;
 
@@ -13,7 +15,6 @@ int pager_use_color = 1;
 #endif
 
 static struct child_process pager_process;
-static char *pager_program;
 static int old_fd1 = -1, old_fd2 = -1;
 
 /* Is the value coming back from term_columns() just a guess? */
@@ -75,10 +76,12 @@ static void wait_for_pager_signal(int signo)
 
 static int core_pager_config(const char *var, const char *value,
 			     const struct config_context *ctx UNUSED,
-			     void *data UNUSED)
+			     void *data)
 {
+	struct repository *r = data;
+
 	if (!strcmp(var, "core.pager"))
-		return git_config_string(&pager_program, var, value);
+		return git_config_string(&repo_config_values(r)->pager_program, var, value);
 	return 0;
 }
 
@@ -91,10 +94,10 @@ const char *git_pager(struct repository *r, int stdout_is_tty)
 
 	pager = getenv("GIT_PAGER");
 	if (!pager) {
-		if (!pager_program)
+		if (!repo_config_values(r)->pager_program)
 			read_early_config(r,
-					  core_pager_config, NULL);
-		pager = pager_program;
+					  core_pager_config, r);
+		pager = repo_config_values(r)->pager_program;
 	}
 	if (!pager)
 		pager = getenv("PAGER");
@@ -303,6 +306,6 @@ int check_pager_config(struct repository *r, const char *cmd)
 	read_early_config(r, pager_command_config, &data);
 
 	if (data.value)
-		pager_program = data.value;
+		repo_config_values(r)->pager_program = data.value;
 	return data.want;
 }
-- 
2.43.0

[PATCH v7 5/9] environment: move askpass_program into repo_config_values

[Tian Yuchen]

The global variable 'askpass_program' stores the path to the program
used to prompt the user for credentials. Move it into repo_config_values
to continue the libification effort.

While it is uncommon for a single process to require different askpass
programs for different repositories, maintaining this value as a mutable
global string is a blocker for libification. Global heap-allocated
strings introduce thread-safety issues in a multi-repo environment.

Move 'askpass_program' into 'struct repo_config_values' to eliminate
this global state. The memory is now safely managed and freed via
'repo_config_values_clear()'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 environment.c | 6 ++++--
 environment.h | 1 +
 prompt.c      | 3 ++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/environment.c b/environment.c
index a1204fdcb2..3782bf68aa 100644
--- a/environment.c
+++ b/environment.c
@@ -462,8 +462,8 @@ int git_default_core_config(const char *var, const char *value,
 	}
 
 	if (!strcmp(var, "core.askpass")) {
-		FREE_AND_NULL(askpass_program);
-		return git_config_string(&askpass_program, var, value);
+		FREE_AND_NULL(cfg->askpass_program);
+		return git_config_string(&cfg->askpass_program, var, value);
 	}
 
 	if (!strcmp(var, "core.excludesfile")) {
@@ -724,6 +724,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->excludes_file = NULL;
 	cfg->editor_program = NULL;
 	cfg->pager_program = NULL;
+	cfg->askpass_program = NULL;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
@@ -756,4 +757,5 @@ void repo_config_values_clear(struct repository *repo)
 	FREE_AND_NULL(cfg->excludes_file);
 	FREE_AND_NULL(cfg->editor_program);
 	FREE_AND_NULL(cfg->pager_program);
+	FREE_AND_NULL(cfg->askpass_program);
 }
diff --git a/environment.h b/environment.h
index 22f6697c52..d55b1ba073 100644
--- a/environment.h
+++ b/environment.h
@@ -93,6 +93,7 @@ struct repo_config_values {
 	char *excludes_file;
 	char *editor_program;
 	char *pager_program;
+	char *askpass_program;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
diff --git a/prompt.c b/prompt.c
index 706fba2a50..d8d74c7e37 100644
--- a/prompt.c
+++ b/prompt.c
@@ -3,6 +3,7 @@
 #include "git-compat-util.h"
 #include "parse.h"
 #include "environment.h"
+#include "repository.h"
 #include "run-command.h"
 #include "strbuf.h"
 #include "prompt.h"
@@ -51,7 +52,7 @@ char *git_prompt(const char *prompt, int flags)
 
 		askpass = getenv("GIT_ASKPASS");
 		if (!askpass)
-			askpass = askpass_program;
+			askpass = repo_config_values(the_repository)->askpass_program;
 		if (!askpass)
 			askpass = getenv("SSH_ASKPASS");
 		if (askpass && *askpass)
-- 
2.43.0

[PATCH v7 6/9] environment: migrate apply_default_whitespace and apply_default_ignorewhitespace

[Tian Yuchen]

The global variables 'apply_default_whitespace' and
'apply_default_ignorewhitespace' are used to store the default
whitespace configuration for 'git apply'. Move these variables
into 'struct repo_config_values' to continue the libification
effort.

Dynamically allocated strings fetched via 'repo_config_get_string()'
are now tracked per-repository and safely freed in
'repo_config_values_clear()'.

As part of this transition, update 'git_apply_config()' to accept a
'struct repository *' argument rather than relying on the
'the_repository' global.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 apply.c       | 20 ++++++++++++--------
 environment.c |  6 ++++--
 environment.h |  4 ++--
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/apply.c b/apply.c
index 249248d4f2..66db9b7678 100644
--- a/apply.c
+++ b/apply.c
@@ -47,11 +47,13 @@ struct gitdiff_data {
 	int p_value;
 };
 
-static void git_apply_config(void)
+static void git_apply_config(struct repository *repo)
 {
-	repo_config_get_string(the_repository, "apply.whitespace", &apply_default_whitespace);
-	repo_config_get_string(the_repository, "apply.ignorewhitespace", &apply_default_ignorewhitespace);
-	repo_config(the_repository, git_xmerge_config, NULL);
+	repo_config_get_string(repo, "apply.whitespace",
+			       &repo_config_values(repo)->apply_default_whitespace);
+	repo_config_get_string(repo, "apply.ignorewhitespace",
+			       &repo_config_values(repo)->apply_default_ignorewhitespace);
+	repo_config(repo, git_xmerge_config, NULL);
 }
 
 static int parse_whitespace_option(struct apply_state *state, const char *option)
@@ -126,10 +128,12 @@ int init_apply_state(struct apply_state *state,
 	strset_init(&state->kept_symlinks);
 	strbuf_init(&state->root, 0);
 
-	git_apply_config();
-	if (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))
+	git_apply_config(repo);
+	if (repo_config_values(repo)->apply_default_whitespace &&
+	    parse_whitespace_option(state, repo_config_values(repo)->apply_default_whitespace))
 		return -1;
-	if (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
+	if (repo_config_values(repo)->apply_default_ignorewhitespace &&
+	    parse_ignorewhitespace_option(state, repo_config_values(repo)->apply_default_ignorewhitespace))
 		return -1;
 	return 0;
 }
@@ -192,7 +196,7 @@ int check_apply_state(struct apply_state *state, int force_apply)
 
 static void set_default_whitespace_mode(struct apply_state *state)
 {
-	if (!state->whitespace_option && !apply_default_whitespace)
+	if (!state->whitespace_option && !repo_config_values(state->repo)->apply_default_whitespace)
 		state->ws_error_action = (state->apply ? warn_on_ws_error : nowarn_ws_error);
 }
 
diff --git a/environment.c b/environment.c
index 3782bf68aa..8744790219 100644
--- a/environment.c
+++ b/environment.c
@@ -49,8 +49,6 @@ int assume_unchanged;
 int is_bare_repository_cfg = -1; /* unspecified */
 char *git_commit_encoding;
 char *git_log_output_encoding;
-char *apply_default_whitespace;
-char *apply_default_ignorewhitespace;
 int fsync_object_files = -1;
 int use_fsync = -1;
 enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
@@ -725,6 +723,8 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->editor_program = NULL;
 	cfg->pager_program = NULL;
 	cfg->askpass_program = NULL;
+	cfg->apply_default_whitespace = NULL;
+	cfg->apply_default_ignorewhitespace = NULL;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
@@ -758,4 +758,6 @@ void repo_config_values_clear(struct repository *repo)
 	FREE_AND_NULL(cfg->editor_program);
 	FREE_AND_NULL(cfg->pager_program);
 	FREE_AND_NULL(cfg->askpass_program);
+	FREE_AND_NULL(cfg->apply_default_whitespace);
+	FREE_AND_NULL(cfg->apply_default_ignorewhitespace);
 }
diff --git a/environment.h b/environment.h
index d55b1ba073..9aecd64152 100644
--- a/environment.h
+++ b/environment.h
@@ -94,6 +94,8 @@ struct repo_config_values {
 	char *editor_program;
 	char *pager_program;
 	char *askpass_program;
+	char *apply_default_whitespace;
+	char *apply_default_ignorewhitespace;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -182,8 +184,6 @@ extern int has_symlinks;
 extern int minimum_abbrev, default_abbrev;
 extern int ignore_case;
 extern int assume_unchanged;
-extern char *apply_default_whitespace;
-extern char *apply_default_ignorewhitespace;
 extern unsigned long pack_size_limit_cfg;
 
 extern int protect_hfs;
-- 
2.43.0

[PATCH v7 7/9] environment: move push_default into repo_config_values

[Tian Yuchen]

The global variable 'push_default' specifies the default behavior of
'git push' when no explicit refspec is provided. Move 'push_default'
into 'struct repo_config_values' to continue the libification effort.

While 'enum push_default_type' ideally belongs in 'remote.h', moving it
there introduces a circular dependency chain:

  remote.h -> hash.h -> repository.h -> environment.h.

Therefore, the enum definition is kept in 'environment.h' just above
'struct repo_config_values' with a NEEDSWORK comment for future cleanup.

Modify the configuration parsing in environment.c to update the
per-repository structure directly, and update caller across the
codebase to access the value via 'repo_config_values()'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 builtin/push.c |  8 ++++----
 environment.c  | 16 +++++++++-------
 environment.h  | 26 ++++++++++++++++----------
 remote.c       |  2 +-
 4 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 6021b71d66..6dc3224b60 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -88,7 +88,7 @@ static void refspec_append_mapped(struct refspec *refspec, const char *ref,
 		}
 	}
 
-	if (push_default == PUSH_DEFAULT_UPSTREAM &&
+	if (repo_config_values(the_repository)->push_default == PUSH_DEFAULT_UPSTREAM &&
 	    skip_prefix(matched->name, "refs/heads/", &branch_name)) {
 		struct branch *branch = branch_get(branch_name);
 		if (branch->merge_nr == 1 && branch->merge[0]->src) {
@@ -160,7 +160,7 @@ static NORETURN void die_push_simple(struct branch *branch,
 	 * Don't show advice for people who explicitly set
 	 * push.default.
 	 */
-	if (push_default == PUSH_DEFAULT_UNSPECIFIED)
+	if (cfg->push_default == PUSH_DEFAULT_UNSPECIFIED)
 		advice_pushdefault_maybe = _("\n"
 				 "To choose either option permanently, "
 				 "see push.default in 'git help config'.\n");
@@ -232,7 +232,7 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
 	const char *dst;
 	int same_remote;
 
-	switch (push_default) {
+	switch (repo_config_values(the_repository)->push_default) {
 	case PUSH_DEFAULT_MATCHING:
 		refspec_append(&rs, ":");
 		return;
@@ -252,7 +252,7 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
 	dst = branch->refname;
 	same_remote = !strcmp(remote->name, remote_for_branch(branch, NULL));
 
-	switch (push_default) {
+	switch (repo_config_values(the_repository)->push_default) {
 	default:
 	case PUSH_DEFAULT_UNSPECIFIED:
 	case PUSH_DEFAULT_SIMPLE:
diff --git a/environment.c b/environment.c
index 8744790219..09de2fee87 100644
--- a/environment.c
+++ b/environment.c
@@ -59,7 +59,6 @@ enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
 char *check_roundtrip_encoding;
 enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
-enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
 #endif
@@ -619,21 +618,23 @@ static int git_default_branch_config(const char *var, const char *value)
 
 static int git_default_push_config(const char *var, const char *value)
 {
+	struct repo_config_values *cfg = repo_config_values(the_repository);
+
 	if (!strcmp(var, "push.default")) {
 		if (!value)
 			return config_error_nonbool(var);
 		else if (!strcmp(value, "nothing"))
-			push_default = PUSH_DEFAULT_NOTHING;
+			cfg->push_default = PUSH_DEFAULT_NOTHING;
 		else if (!strcmp(value, "matching"))
-			push_default = PUSH_DEFAULT_MATCHING;
+			cfg->push_default = PUSH_DEFAULT_MATCHING;
 		else if (!strcmp(value, "simple"))
-			push_default = PUSH_DEFAULT_SIMPLE;
+			cfg->push_default = PUSH_DEFAULT_SIMPLE;
 		else if (!strcmp(value, "upstream"))
-			push_default = PUSH_DEFAULT_UPSTREAM;
+			cfg->push_default = PUSH_DEFAULT_UPSTREAM;
 		else if (!strcmp(value, "tracking")) /* deprecated */
-			push_default = PUSH_DEFAULT_UPSTREAM;
+			cfg->push_default = PUSH_DEFAULT_UPSTREAM;
 		else if (!strcmp(value, "current"))
-			push_default = PUSH_DEFAULT_CURRENT;
+			cfg->push_default = PUSH_DEFAULT_CURRENT;
 		else {
 			error(_("malformed value for %s: %s"), var, value);
 			return error(_("must be one of nothing, matching, simple, "
@@ -725,6 +726,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->askpass_program = NULL;
 	cfg->apply_default_whitespace = NULL;
 	cfg->apply_default_ignorewhitespace = NULL;
+	cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
diff --git a/environment.h b/environment.h
index 9aecd64152..72859b5d76 100644
--- a/environment.h
+++ b/environment.h
@@ -87,6 +87,21 @@ extern const char * const local_repo_env[];
 struct strvec;
 
 struct repository;
+
+/*
+ * NEEDSWORK: It would be better if these definitions could be moved to
+ * other more specific files, but care is needed to avoid circular
+ * inclusion issues.
+ */
+enum push_default_type {
+	PUSH_DEFAULT_NOTHING = 0,
+	PUSH_DEFAULT_MATCHING,
+	PUSH_DEFAULT_SIMPLE,
+	PUSH_DEFAULT_UPSTREAM,
+	PUSH_DEFAULT_CURRENT,
+	PUSH_DEFAULT_UNSPECIFIED
+};
+
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
@@ -96,6 +111,7 @@ struct repo_config_values {
 	char *askpass_program;
 	char *apply_default_whitespace;
 	char *apply_default_ignorewhitespace;
+	enum push_default_type push_default;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -197,16 +213,6 @@ enum rebase_setup_type {
 };
 extern enum rebase_setup_type autorebase;
 
-enum push_default_type {
-	PUSH_DEFAULT_NOTHING = 0,
-	PUSH_DEFAULT_MATCHING,
-	PUSH_DEFAULT_SIMPLE,
-	PUSH_DEFAULT_UPSTREAM,
-	PUSH_DEFAULT_CURRENT,
-	PUSH_DEFAULT_UNSPECIFIED
-};
-extern enum push_default_type push_default;
-
 enum object_creation_mode {
 	OBJECT_CREATION_USES_HARDLINKS = 0,
 	OBJECT_CREATION_USES_RENAMES = 1
diff --git a/remote.c b/remote.c
index 00723b385e..d48c01d375 100644
--- a/remote.c
+++ b/remote.c
@@ -1933,7 +1933,7 @@ static char *branch_get_push_1(struct repository *repo,
 	if (remote->mirror)
 		return tracking_for_push_dest(remote, branch->refname, err);
 
-	switch (push_default) {
+	switch (repo_config_values(repo)->push_default) {
 	case PUSH_DEFAULT_NOTHING:
 		return error_buf(err, _("push has no destination (push.default is 'nothing')"));
 
-- 
2.43.0

[PATCH v7 8/9] environment: move autorebase into repo_config_values

[Tian Yuchen]

The global variable 'autorebase' dictates whether a newly created
branch should be configured to automatically rebase by default.
Move it into 'struct repo_config_values' to continue the
libification effort.

The 'enum rebase_setup_type' definition is moved higher up in
'environment.h' so that it is visible to the repository-specific
structure. The default state AUTOREBASE_NEVER is now correctly
initialized in 'repo_config_values_init()'.

Configuration parsing in 'git_default_branch_config()' is updated to
write directly to the repository's configuration instance.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 branch.c      |  2 +-
 environment.c | 10 +++++-----
 environment.h | 16 ++++++++--------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/branch.c b/branch.c
index 243db7d0fc..e1c1f8c89d 100644
--- a/branch.c
+++ b/branch.c
@@ -61,7 +61,7 @@ static int find_tracked_branch(struct remote *remote, void *priv)
 
 static int should_setup_rebase(const char *origin)
 {
-	switch (autorebase) {
+	switch (repo_config_values(the_repository)->autorebase) {
 	case AUTOREBASE_NEVER:
 		return 0;
 	case AUTOREBASE_LOCAL:
diff --git a/environment.c b/environment.c
index 09de2fee87..7701aa3bc0 100644
--- a/environment.c
+++ b/environment.c
@@ -58,7 +58,6 @@ enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
 char *check_roundtrip_encoding;
-enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
 #endif
@@ -600,13 +599,13 @@ static int git_default_branch_config(const char *var, const char *value)
 		if (!value)
 			return config_error_nonbool(var);
 		else if (!strcmp(value, "never"))
-			autorebase = AUTOREBASE_NEVER;
+			cfg->autorebase = AUTOREBASE_NEVER;
 		else if (!strcmp(value, "local"))
-			autorebase = AUTOREBASE_LOCAL;
+			cfg->autorebase = AUTOREBASE_LOCAL;
 		else if (!strcmp(value, "remote"))
-			autorebase = AUTOREBASE_REMOTE;
+			cfg->autorebase = AUTOREBASE_REMOTE;
 		else if (!strcmp(value, "always"))
-			autorebase = AUTOREBASE_ALWAYS;
+			cfg->autorebase = AUTOREBASE_ALWAYS;
 		else
 			return error(_("malformed value for %s"), var);
 		return 0;
@@ -727,6 +726,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->apply_default_whitespace = NULL;
 	cfg->apply_default_ignorewhitespace = NULL;
 	cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
+	cfg->autorebase = AUTOREBASE_NEVER;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
diff --git a/environment.h b/environment.h
index 72859b5d76..464ff73136 100644
--- a/environment.h
+++ b/environment.h
@@ -102,6 +102,13 @@ enum push_default_type {
 	PUSH_DEFAULT_UNSPECIFIED
 };
 
+enum rebase_setup_type {
+	AUTOREBASE_NEVER = 0,
+	AUTOREBASE_LOCAL,
+	AUTOREBASE_REMOTE,
+	AUTOREBASE_ALWAYS
+};
+
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
@@ -112,6 +119,7 @@ struct repo_config_values {
 	char *apply_default_whitespace;
 	char *apply_default_ignorewhitespace;
 	enum push_default_type push_default;
+	enum rebase_setup_type autorebase;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -205,14 +213,6 @@ extern unsigned long pack_size_limit_cfg;
 extern int protect_hfs;
 extern int protect_ntfs;
 
-enum rebase_setup_type {
-	AUTOREBASE_NEVER = 0,
-	AUTOREBASE_LOCAL,
-	AUTOREBASE_REMOTE,
-	AUTOREBASE_ALWAYS
-};
-extern enum rebase_setup_type autorebase;
-
 enum object_creation_mode {
 	OBJECT_CREATION_USES_HARDLINKS = 0,
 	OBJECT_CREATION_USES_RENAMES = 1
-- 
2.43.0

[PATCH v7 9/9] environment: move object_creation_mode into repo_config_values

[Tian Yuchen]

The global variable 'object_creation_mode' controls how Git creates
object files, specifically determining whether to use hardlinks or
renames when moving temporary files into the object database. Move
it into 'struct repo_config_values' to continue the libification
effort.

Move the 'enum object_creation_mode' definition higher up in
'environment.h' to ensure it is visible to the structure. Initialize
the per-repository value to its default macro value
OBJECT_CREATION_MODE inside 'repo_config_values_init()'.

Update configuration parsing in 'git_default_core_config()' to write
directly to the repository-specific configuration structure.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 environment.c |  6 +++---
 environment.h | 12 ++++++------
 object-file.c |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/environment.c b/environment.c
index 7701aa3bc0..e50beda918 100644
--- a/environment.c
+++ b/environment.c
@@ -61,7 +61,6 @@ char *check_roundtrip_encoding;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
 #endif
-enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
 unsigned long pack_size_limit_cfg;
 
@@ -511,9 +510,9 @@ int git_default_core_config(const char *var, const char *value,
 		if (!value)
 			return config_error_nonbool(var);
 		if (!strcmp(value, "rename"))
-			object_creation_mode = OBJECT_CREATION_USES_RENAMES;
+			cfg->object_creation_mode = OBJECT_CREATION_USES_RENAMES;
 		else if (!strcmp(value, "link"))
-			object_creation_mode = OBJECT_CREATION_USES_HARDLINKS;
+			cfg->object_creation_mode = OBJECT_CREATION_USES_HARDLINKS;
 		else
 			die(_("invalid mode for object creation: %s"), value);
 		return 0;
@@ -727,6 +726,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->apply_default_ignorewhitespace = NULL;
 	cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
 	cfg->autorebase = AUTOREBASE_NEVER;
+	cfg->object_creation_mode = OBJECT_CREATION_MODE;
 	cfg->apply_sparse_checkout = 0;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 	cfg->trust_ctime = 1;
diff --git a/environment.h b/environment.h
index 464ff73136..eaa0aba7bc 100644
--- a/environment.h
+++ b/environment.h
@@ -109,6 +109,11 @@ enum rebase_setup_type {
 	AUTOREBASE_ALWAYS
 };
 
+enum object_creation_mode {
+	OBJECT_CREATION_USES_HARDLINKS = 0,
+	OBJECT_CREATION_USES_RENAMES = 1
+};
+
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
@@ -120,6 +125,7 @@ struct repo_config_values {
 	char *apply_default_ignorewhitespace;
 	enum push_default_type push_default;
 	enum rebase_setup_type autorebase;
+	enum object_creation_mode object_creation_mode;
 	int apply_sparse_checkout;
 	int trust_ctime;
 	int check_stat;
@@ -213,12 +219,6 @@ extern unsigned long pack_size_limit_cfg;
 extern int protect_hfs;
 extern int protect_ntfs;
 
-enum object_creation_mode {
-	OBJECT_CREATION_USES_HARDLINKS = 0,
-	OBJECT_CREATION_USES_RENAMES = 1
-};
-extern enum object_creation_mode object_creation_mode;
-
 extern int grafts_keep_true_parents;
 
 const char *get_log_output_encoding(void);
diff --git a/object-file.c b/object-file.c
index 9afa842da2..cbbfc8f1dc 100644
--- a/object-file.c
+++ b/object-file.c
@@ -415,7 +415,7 @@ int finalize_object_file_flags(struct repository *repo,
 retry:
 	ret = 0;
 
-	if (object_creation_mode == OBJECT_CREATION_USES_RENAMES)
+	if (repo_config_values(repo)->object_creation_mode == OBJECT_CREATION_USES_RENAMES)
 		goto try_rename;
 	else if (link(tmpfile, filename))
 		ret = errno;
-- 
2.43.0

Re: [PATCH v7 4/9] environment: move pager_program into repo_config_values

[Junio C Hamano]

Tian Yuchen <cat@malon.dev> writes:

>  	if (data.value)
> -		pager_program = data.value;
> +		repo_config_values(r)->pager_program = data.value;
>  	return data.want;
>  }

May not be a new problem, but does the old value or pager_program
leak here, if callers call this function more than once (or
pager_program gets assigned elsewhere)?