From: Phillip Wood <phillip.wood123@gmail.com>
Date: Thu, 22 Jan 2026 14:40:00 GMT
Subject: Re: [Outreachy PATCH v3 1/3] environment: stop storing `core.attributesFile` globally
Message-ID: <60dfb907-c2b8-4fd4-b975-742f7ec18721@gmail.com>
In-Reply-To: <1aa41da8334296e4c1063b81fc40ec3b1dcdcb7b.1768681947.git.belkid98@gmail.com>

Hi Olamide

On 17/01/2026 20:59, Olamide Caleb Bello wrote:
> The config value is parsed in git_default_core_config(), loaded eagerly
> and stored in the global variable `git_attributes_file`.
> Storing this value in a global variable can lead to unexpected
> behaviours when more than one Git repository run in the same Git process.

It would maybe be helpful to explain what the unexpected behavior is and 
how it is caused.

> diff --git a/environment.h b/environment.h
> index 51898c99cd..aea73ff25b 100644
> --- a/environment.h
> +++ b/environment.h
> @@ -84,6 +84,12 @@ extern const char * const local_repo_env[];
>   
>   struct strvec;
>   
> +/* Config values parsed by git_default_config() */
> +struct repo_config_values {
> +	/* core config values */
> +	char *attributes_file_path;

The variable we're converting is called "attributes_file", do we really 
need to add a "_path" suffix?

Apart from that everything here looks good

Thanks

Phillip

> +};
> +
>   /*
>    * Wrapper of getenv() that returns a strdup value. This value is kept
>    * in argv to be freed later.
> @@ -107,6 +113,8 @@ const char *strip_namespace(const char *namespaced_ref);
>   int git_default_config(const char *, const char *,
>   		       const struct config_context *, void *);
>   
> +void repo_config_values_init(struct repo_config_values *cfg);
> +
>   /*
>    * TODO: All the below state either explicitly or implicitly relies on
>    * `the_repository`. We should eventually get rid of these and make the
> @@ -152,7 +160,6 @@ extern int assume_unchanged;
>   extern int warn_on_object_refname_ambiguity;
>   extern char *apply_default_whitespace;
>   extern char *apply_default_ignorewhitespace;
> -extern char *git_attributes_file;
>   extern int zlib_compression_level;
>   extern int pack_compression_level;
>   extern unsigned long pack_size_limit_cfg;
> diff --git a/repository.c b/repository.c
> index c7e75215ac..d308cd78bf 100644
> --- a/repository.c
> +++ b/repository.c
> @@ -57,6 +57,7 @@ void initialize_repository(struct repository *repo)
>   	ALLOC_ARRAY(repo->index, 1);
>   	index_state_init(repo->index, repo);
>   	repo->check_deprecated_config = true;
> +	repo_config_values_init(&repo->config_values);
>   
>   	/*
>   	 * When a command runs inside a repository, it learns what
> diff --git a/repository.h b/repository.h
> index 6063c4b846..638a142577 100644
> --- a/repository.h
> +++ b/repository.h
> @@ -3,6 +3,7 @@
>   
>   #include "strmap.h"
>   #include "repo-settings.h"
> +#include "environment.h"
>   
>   struct config_set;
>   struct git_hash_algo;
> @@ -148,6 +149,9 @@ struct repository {
>   	/* Repository's compatibility hash algorithm. */
>   	const struct git_hash_algo *compat_hash_algo;
>   
> +	/* Repository's config values parsed by git_default_config() */
> +	struct repo_config_values config_values;
> +
>   	/* Repository's reference storage format, as serialized on disk. */
>   	enum ref_storage_format ref_storage_format;
>