From: Bello Olamide Date: Thu, 18 Dec 2025 17:31:25 GMT Subject: Re: [Outreachy PATCH] environment: move "core.attributesFile" into repo-setting Message-ID: In-Reply-To: On Thu, 18 Dec 2025 at 09:30, Olamide Caleb Bello wrote: > > 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 > Mentored-by: Christian Couder > Mentored-by: Usman Akinyemi > Signed-off-by: Olamide Caleb Bello > --- > The link to the GitHub CI is provided below > https://github.com/cloobTech/git/actions/runs/20284228144 The link to Ayush's patches, which this patch is based on, is provided in [1]. The 'git_attributes_file' member of `struct repository` is now accessed via the `struct index_state`, istate->repo, as most of the callers in the attributes subsystem already use the `index_state`, rather than through the `struct repository *repo` as done in [1] which only knows its primary index. This ensures that the index actually owns the attributes as pointed out by Junio in the threads. [1]. https://lore.kernel.org/git/20250309153321.254844-1-ayu.chandekar@gmail.com/ > > 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(-) > > diff --git a/attr.c b/attr.c > index 4999b7e09d..9e51f8e70b 100644 > --- a/attr.c > +++ b/attr.c > @@ -879,14 +879,6 @@ const char *git_attr_system_file(void) > return system_wide; > } > > -const char *git_attr_global_file(void) > -{ > - if (!git_attributes_file) > - git_attributes_file = xdg_config_home("attributes"); > - > - return git_attributes_file; > -} > - > int git_attr_system_is_enabled(void) > { > return !git_env_bool("GIT_ATTR_NOSYSTEM", 0); > @@ -912,6 +904,8 @@ static void bootstrap_attr_stack(struct index_state *istate, > { > struct attr_stack *e; > unsigned flags = READ_ATTR_MACRO_OK; > + const char *attributes_file_path; > + struct repository *repo; > > if (*stack) > return; > @@ -926,9 +920,13 @@ static void bootstrap_attr_stack(struct index_state *istate, > push_stack(stack, e, NULL, 0); > } > > - /* home directory */ > - if (git_attr_global_file()) { > - e = read_attr_from_file(git_attr_global_file(), flags); > + if (istate && istate->repo) > + repo = istate->repo; > + else > + repo = the_repository; > + attributes_file_path = repo_settings_get_attributesfile_path(repo); > + if (attributes_file_path) { > + e = read_attr_from_file(attributes_file_path, flags); > push_stack(stack, e, NULL, 0); > } > > diff --git a/attr.h b/attr.h > index a04a521092..956ce6ba62 100644 > --- a/attr.h > +++ b/attr.h > @@ -232,9 +232,6 @@ void attr_start(void); > /* Return the system gitattributes file. */ > const char *git_attr_system_file(void); > > -/* Return the global gitattributes file, if any. */ > -const char *git_attr_global_file(void); > - > /* Return whether the system gitattributes file is enabled and should be used. */ > int git_attr_system_is_enabled(void); > > diff --git a/builtin/var.c b/builtin/var.c > index cc3a43cde2..fd577f2930 100644 > --- a/builtin/var.c > +++ b/builtin/var.c > @@ -72,7 +72,7 @@ static char *git_attr_val_system(int ident_flag UNUSED) > > static char *git_attr_val_global(int ident_flag UNUSED) > { > - char *file = xstrdup_or_null(git_attr_global_file()); > + char *file = xstrdup_or_null(repo_settings_get_attributesfile_path(the_repository)); > if (file) { > normalize_path_copy(file, file); > return file; > diff --git a/environment.c b/environment.c > index a770b5921d..ed7d8f42d9 100644 > --- a/environment.c > +++ b/environment.c > @@ -53,7 +53,6 @@ char *git_commit_encoding; > char *git_log_output_encoding; > char *apply_default_whitespace; > char *apply_default_ignorewhitespace; > -char *git_attributes_file; > int zlib_compression_level = Z_BEST_SPEED; > int pack_compression_level = Z_DEFAULT_COMPRESSION; > int fsync_object_files = -1; > @@ -363,11 +362,6 @@ static int git_default_core_config(const char *var, const char *value, > return 0; > } > > - if (!strcmp(var, "core.attributesfile")) { > - FREE_AND_NULL(git_attributes_file); > - return git_config_pathname(&git_attributes_file, var, value); > - } > - > if (!strcmp(var, "core.bare")) { > is_bare_repository_cfg = git_config_bool(var, value); > return 0; > diff --git a/environment.h b/environment.h > index 51898c99cd..3512a7072e 100644 > --- a/environment.h > +++ b/environment.h > @@ -152,7 +152,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/repo-settings.c b/repo-settings.c > index 195c24e9c0..396cf79f20 100644 > --- a/repo-settings.c > +++ b/repo-settings.c > @@ -5,6 +5,7 @@ > #include "midx.h" > #include "pack-objects.h" > #include "setup.h" > +#include "path.h" > > static void repo_cfg_bool(struct repository *r, const char *key, int *dest, > int def) > @@ -158,6 +159,7 @@ void repo_settings_clear(struct repository *r) > struct repo_settings empty = REPO_SETTINGS_INIT; > FREE_AND_NULL(r->settings.fsmonitor); > FREE_AND_NULL(r->settings.hooks_path); > + FREE_AND_NULL(r->settings.git_attributes_file); > r->settings = empty; > } > > @@ -230,3 +232,11 @@ void repo_settings_reset_shared_repository(struct repository *repo) > { > repo->settings.shared_repository_initialized = 0; > } > +const char *repo_settings_get_attributesfile_path(struct repository *repo) > +{ > + if (!repo->settings.git_attributes_file) { > + if (repo_config_get_pathname(repo, "core.attributesfile", &repo->settings.git_attributes_file)) > + repo->settings.git_attributes_file = xdg_config_home("attributes"); > + } > + return repo->settings.git_attributes_file; > +} > 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, \ > @@ -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. > + */ > +const char *repo_settings_get_attributesfile_path(struct repository *repo); > + > #endif /* REPO_SETTINGS_H */ > -- > 2.34.1 >