Re: [Outreachy PATCH v3 1/3] environment: stop storing `core.attributesFile` globally
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Jan 22, 2026, 14:40 UTC
- 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.
Show 12 quoted lines
> 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
Show 56 quoted lines
> +};
> +
> /*
> * 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;
>