Re: [Outreachy PATCH] environment: move "core.attributesFile" into repo-setting

From: Karthik Nayak <karthik.188@gmail.com>
Date: Jan 2, 2026, 08:48 UTC
Message-ID: <CAOLa=ZRDFdZJWsq5JOckRgfF2V0Whv-jCxbpgeRi80NOs0oTDQ@mail.gmail.com>
In-Reply-To: <aUO7jQQAERTe5xYc@ubuntu>

Olamide Caleb Bello <belkid98@gmail.com> writes:

Show 36 quoted lines

> When handling multiple repositories within the same process, relying on
> global state for accessing the "core.attributesFile" configuration can
> lead to incorrect values being used. It also makes it harder to isolate
> repositories and hinders the libification of git.
> The functions `bootstrap_attr_stack()` and `git_attr_val_system()`
> retrieve "core.attributesFile" via `git_attr_global_file()`
> which reads from global state `git_attributes_file`.
>
> Move the "core.attributesFile" configuration into the
> `struct repo_settings` instead of relying on the global state.
> A new function `repo_settings_get_attributesfile_path()` is added
> and used to retrieve this setting in a repository-scoped manner.
> The functions to retrieve "core.attributesFile" are replaced with
> the new accessor function `repo_settings_get_attributesfile_path()`
> This improves multi-repository behaviour and aligns with the goal of
> libifying of Git.
>
> Note that in `bootstrap_attr_stack()`, the `index_state` is used only
> if it exists, else we default to `the_repository`.
>
> Based-on-patch-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
> Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
> ---
> The link to the GitHub CI is provided below
> https://github.com/cloobTech/git/actions/runs/20284228144
>
>  attr.c          | 20 +++++++++-----------
>  attr.h          |  3 ---
>  builtin/var.c   |  2 +-
>  environment.c   |  6 ------
>  environment.h   |  1 -
>  repo-settings.c | 10 ++++++++++
>  repo-settings.h |  8 ++++++++
>  7 files changed, 28 insertions(+), 22 deletions(-)

The change is very welcome. Apart from some small comments below, the patch looks good.

[snip]

Show 12 quoted lines

> diff --git a/repo-settings.h b/repo-settings.h
> index d477885561..362f355267 100644
> --- a/repo-settings.h
> +++ b/repo-settings.h
> @@ -68,6 +68,7 @@ struct repo_settings {
>  	unsigned long big_file_threshold;
>
>  	char *hooks_path;
> +	char *git_attributes_file;
>  };
>  #define REPO_SETTINGS_INIT { \
>  	.shared_repository = -1, \

It would make more sense to rename this variable to `attributes_file_path`, that would better denote what is actually stored here and syncs better with `repo_settings_get_attributesfile_path`.

Show 8 quoted lines

> @@ -99,4 +100,11 @@ int repo_settings_get_shared_repository(struct repository *repo);
>  void repo_settings_set_shared_repository(struct repository *repo, int value);
>  void repo_settings_reset_shared_repository(struct repository *repo);
>
> +/*
> + * Read the value for "core.attributesfile".
> + * Defaults to xdg_config_home("attributes") if the core.attributesfile
> + * isn't available.

While it is obvious, it would be nice to point out that `core.attributesfile` is set via config.

Show 6 quoted lines

> + */
> +const char *repo_settings_get_attributesfile_path(struct repository *repo);
> +
>  #endif /* REPO_SETTINGS_H */
> --
> 2.34.1