# [Outreachy PATCH] environment: move "core.attributesFile" into repo-setting 21 messages from 2025-12-18 to 2026-01-07. Participants: Olamide Caleb Bello, Bello Olamide, Karthik Nayak, Phillip Wood, Junio C Hamano. Thread: https://gitlist.dev/t/64647 ## Olamide Caleb Bello, 2025-12-18 08:30 Subject: [Outreachy PATCH] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/aUO7jQQAERTe5xYc%40ubuntu ``` 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 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 ``` ## Bello Olamide, 2025-12-18 17:31 Subject: Re: [Outreachy PATCH] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/CAD%3Df0L8hCyUK6OJXrz7%3DKNVTJ_cjkX3pt6N-ZiH%2B58PHR21%3D3g%40mail.gmail.com 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 > ``` ## Bello Olamide, 2026-01-02 08:01 Subject: Re: [Outreachy PATCH] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/CAD%3Df0L88QW_tL2iKg8ru3mU7t-vmY%3Dp61S33GN%2B6tSQBMQAjqw%40mail.gmail.com 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 Hello. Please I am replying to this as no reviews have been done on this patch. Thanks [...] ``` ## Karthik Nayak, 2026-01-02 08:48 Subject: Re: [Outreachy PATCH] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZRDFdZJWsq5JOckRgfF2V0Whv-jCxbpgeRi80NOs0oTDQ%40mail.gmail.com In-Reply-To: ``` Olamide Caleb Bello writes: > 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 > > 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] > 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`. > @@ -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. > + */ > +const char *repo_settings_get_attributesfile_path(struct repository *repo); > + > #endif /* REPO_SETTINGS_H */ > -- > 2.34.1 ``` ## Karthik Nayak, 2026-01-02 08:49 Subject: Re: [Outreachy PATCH] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZQ8zQKk7Jmu_Pwm-VbCY9u3WP7oC51JBVUSXEi9pk_UfA%40mail.gmail.com In-Reply-To: ``` Bello Olamide writes: > 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 > > Hello. > Please I am replying to this as no reviews have been done on this patch. > Thanks Thanks for the bump, I think reviews are slowed down due to holidays. You should see a uptick henceforth :) ``` ## Bello Olamide, 2026-01-02 11:26 Subject: Re: [Outreachy PATCH] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/CAD%3Df0L8K%2BOu6Kg5gUEqQpNzbSi-FHMsovOKtJN2hzjFYHywiPQ%40mail.gmail.com In-Reply-To: ``` On Fri, 2 Jan 2026 at 09:48, Karthik Nayak wrote: > > Olamide Caleb Bello writes: > > > 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 > > > > 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] > > > 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`. > > > @@ -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. > Thank you for the review Karthik. I will send an updated version with the changes. Bello. ``` ## Olamide Caleb Bello, 2026-01-02 16:32 Subject: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/aVfzMsN2ouY3UBFG%40ubuntu In-Reply-To: ``` 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/git/git/actions/runs/20661817110 Changes in v2: -------------- - Renamed the variable in the repo-settings struct to `attributes_file_path` - Modified the comment section of the accessor function declaration to indicate the core.attributesFile is read via repo config. Range diff vs v1: ----------------- 1: 4975f77cde ! 1: fc1dbec892 environment: move "core.attributesfile" into repo-setting @@ Commit message Note that in `bootstrap_attr_stack()`, the `index_state` is used only if it exists, else we default to `the_repository`. - Reported-by: Ayush Chandekar + Based-on-patch-by: Ayush Chandekar Mentored-by: Christian Couder Mentored-by: Usman Akinyemi Signed-off-by: Olamide Caleb Bello @@ attr.c: static void bootstrap_attr_stack(struct index_state *istate, if (*stack) return; @@ attr.c: static void bootstrap_attr_stack(struct index_state *istate, - push_stack(stack, e, NULL, 0); } -- /* home directory */ + /* home directory */ - if (git_attr_global_file()) { - e = read_attr_from_file(git_attr_global_file(), flags); + if (istate && istate->repo) @@ repo-settings.c: 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); ++ FREE_AND_NULL(r->settings.attributes_file_path); r->settings = empty; } @@ repo-settings.c: void repo_settings_reset_shared_repository(struct repository *r } +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"); ++ if (!repo->settings.attributes_file_path) { ++ if (repo_config_get_pathname(repo, "core.attributesfile", &repo->settings.attributes_file_path)) ++ repo->settings.attributes_file_path = xdg_config_home("attributes"); + } -+ return repo->settings.git_attributes_file; ++ return repo->settings.attributes_file_path; +} ## repo-settings.h ## @@ repo-settings.h: struct repo_settings { unsigned long big_file_threshold; char *hooks_path; -+ char *git_attributes_file; ++ char *attributes_file_path; }; #define REPO_SETTINGS_INIT { \ .shared_repository = -1, \ @@ repo-settings.h: int repo_settings_get_shared_repository(struct repository *repo +/* + * Read the value for "core.attributesfile". + * Defaults to xdg_config_home("attributes") if the core.attributesfile -+ * isn't available. ++ * which is set via repo config isn't available. + */ +const char *repo_settings_get_attributesfile_path(struct repository *repo); + attr.c | 19 +++++++++---------- 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(+), 21 deletions(-) diff --git a/attr.c b/attr.c index 4999b7e09d..b081400c18 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; @@ -927,8 +921,13 @@ static void bootstrap_attr_stack(struct index_state *istate, } /* 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..cc53a3cd3b 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.attributes_file_path); 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.attributes_file_path) { + if (repo_config_get_pathname(repo, "core.attributesfile", &repo->settings.attributes_file_path)) + repo->settings.attributes_file_path = xdg_config_home("attributes"); + } + return repo->settings.attributes_file_path; +} diff --git a/repo-settings.h b/repo-settings.h index d477885561..1209e1db83 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 *attributes_file_path; }; #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 + * which is set via repo config isn't available. + */ +const char *repo_settings_get_attributesfile_path(struct repository *repo); + #endif /* REPO_SETTINGS_H */ -- 2.52.0.210.g7f9f2609ac.dirty ``` ## Karthik Nayak, 2026-01-05 11:09 Subject: Re: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZTOKvEQaMxymi%2BmRcqyNy4bZ4JbK2HPtq6CeewjHMo_%3Dg%40mail.gmail.com In-Reply-To: ``` Olamide Caleb Bello writes: [snip] > @@ -927,8 +921,13 @@ static void bootstrap_attr_stack(struct index_state *istate, > } > > /* 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); > } > For my own understanding, when can `istate` be NULL? [snip] ``` ## Bello Olamide, 2026-01-05 11:39 Subject: Re: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/CAD%3Df0L-ge9FfNh04Nu05eg9Q6t_gtLaPi2%3DjiT1LXOjF20OO2Q%40mail.gmail.com In-Reply-To: ``` On Mon, 5 Jan 2026 at 12:09, Karthik Nayak wrote: > > Olamide Caleb Bello writes: > [snip] > > > @@ -927,8 +921,13 @@ static void bootstrap_attr_stack(struct index_state *istate, > > } > > > > /* 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); > > } > > > > For my own understanding, when can `istate` be NULL? > Thank you for your question Karthik. So it was stated in a comment in `apply.c:read_old_data():2340` that `git apply without --index/cached should never look at the index because the target file may not be in the index yet and we may not be in a Git repository.` So NULL is passed to convert_to_git() in place of `istate`. So when we do `git apply patch.file` `istate` is NULL, but when we do `git apply --cached patch.file`, `istate` is not NULL. Bello ``` ## Phillip Wood, 2026-01-05 14:23 Subject: Re: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/a881499d-e236-4f8e-a217-b6bce69e3e3c%40gmail.com In-Reply-To: ``` Hi Olamide On 02/01/2026 16:32, 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. This changes when the config setting gets parsed which unfortunately regresses the user experience when the setting is invalid. If I run 'git -c core.attributesFile=~does-not-exist rebase -i' with git built from master it fails immediately with "fatal: failed to expand user dir in: '~does-not-exist'". With this patch applied it prompts me to edit the todo list and then fails when it tries to checkout the commit we're rebasing onto. Because "git rebase" expects reset_head() to return an error rather die if the checkout fails it is left in a strange state where only practical course of action for the user is to run "git rebase --abort". It is quite common that moving from parsing config settings eagerly by calling repo_config() at startup to parsing them lazily via 'stuct repo_settings' causes regressions like this. We really should find a way to address that before moving more settings into 'struct repo_settings' Thanks Phillip > 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/git/git/actions/runs/20661817110 > > Changes in v2: > -------------- > - Renamed the variable in the repo-settings struct to `attributes_file_path` > - Modified the comment section of the accessor function declaration to > indicate the core.attributesFile is read via repo config. > > Range diff vs v1: > ----------------- > 1: 4975f77cde ! 1: fc1dbec892 environment: move "core.attributesfile" into repo-setting > @@ Commit message > Note that in `bootstrap_attr_stack()`, the `index_state` is used only > if it exists, else we default to `the_repository`. > > - Reported-by: Ayush Chandekar > + Based-on-patch-by: Ayush Chandekar > Mentored-by: Christian Couder > Mentored-by: Usman Akinyemi > Signed-off-by: Olamide Caleb Bello > @@ attr.c: static void bootstrap_attr_stack(struct index_state *istate, > if (*stack) > return; > @@ attr.c: static void bootstrap_attr_stack(struct index_state *istate, > - push_stack(stack, e, NULL, 0); > } > > -- /* home directory */ > + /* home directory */ > - if (git_attr_global_file()) { > - e = read_attr_from_file(git_attr_global_file(), flags); > + if (istate && istate->repo) > @@ repo-settings.c: 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); > ++ FREE_AND_NULL(r->settings.attributes_file_path); > r->settings = empty; > } > > @@ repo-settings.c: void repo_settings_reset_shared_repository(struct repository *r > } > +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"); > ++ if (!repo->settings.attributes_file_path) { > ++ if (repo_config_get_pathname(repo, "core.attributesfile", &repo->settings.attributes_file_path)) > ++ repo->settings.attributes_file_path = xdg_config_home("attributes"); > + } > -+ return repo->settings.git_attributes_file; > ++ return repo->settings.attributes_file_path; > +} > > ## repo-settings.h ## > @@ repo-settings.h: struct repo_settings { > unsigned long big_file_threshold; > > char *hooks_path; > -+ char *git_attributes_file; > ++ char *attributes_file_path; > }; > #define REPO_SETTINGS_INIT { \ > .shared_repository = -1, \ > @@ repo-settings.h: int repo_settings_get_shared_repository(struct repository *repo > +/* > + * Read the value for "core.attributesfile". > + * Defaults to xdg_config_home("attributes") if the core.attributesfile > -+ * isn't available. > ++ * which is set via repo config isn't available. > + */ > +const char *repo_settings_get_attributesfile_path(struct repository *repo); > + > > attr.c | 19 +++++++++---------- > 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(+), 21 deletions(-) > > diff --git a/attr.c b/attr.c > index 4999b7e09d..b081400c18 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; > @@ -927,8 +921,13 @@ static void bootstrap_attr_stack(struct index_state *istate, > } > > /* 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..cc53a3cd3b 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.attributes_file_path); > 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.attributes_file_path) { > + if (repo_config_get_pathname(repo, "core.attributesfile", &repo->settings.attributes_file_path)) > + repo->settings.attributes_file_path = xdg_config_home("attributes"); > + } > + return repo->settings.attributes_file_path; > +} > diff --git a/repo-settings.h b/repo-settings.h > index d477885561..1209e1db83 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 *attributes_file_path; > }; > #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 > + * which is set via repo config isn't available. > + */ > +const char *repo_settings_get_attributesfile_path(struct repository *repo); > + > #endif /* REPO_SETTINGS_H */ ``` ## Phillip Wood, 2026-01-05 15:00 Subject: Re: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: <3947f777-e08a-4c17-81e3-c4711fe666a0@gmail.com> URL: https://gitlist.dev/e/3947f777-e08a-4c17-81e3-c4711fe666a0%40gmail.com In-Reply-To: ``` On 05/01/2026 14:23, Phillip Wood wrote: > > It is quite common that moving from parsing config settings eagerly by > calling repo_config() at startup to parsing them lazily via 'stuct > repo_settings' causes regressions like this. We really should find a way > to address that before moving more settings into 'struct repo_settings' See https://lore.kernel.org/git/d61c966b-61ae-4ba9-b983-c8dab6e2c292@gmail.com for some discussion about a possible solution. Thanks Phillip ``` ## Junio C Hamano, 2026-01-05 22:24 Subject: Re: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/xmqq1pk3lmu3.fsf%40gitster.g In-Reply-To: ``` Phillip Wood writes: > If I run 'git -c core.attributesFile=~does-not-exist rebase -i' with git > built from master it fails immediately with "fatal: failed to expand > user dir in: '~does-not-exist'". Hmph, if you call any behaviour change a "regression", this may certainly count as one, but I do not necessarily think the above is a good behaviour. Think about a use case where attributes are not used at all, e.g., "git -c core.attributesFile=~does-not-matter cat-file -t HEAD"; would it make sense to barf when your configuration file has an invalid definition for what you are *not* using? So if the change makes it stop barfing, it can even be argued that this is an improvement. > It is quite common that moving from parsing config settings eagerly by > calling repo_config() at startup to parsing them lazily via 'stuct > repo_settings' causes regressions like this. We really should find a way > to address that before moving more settings into 'struct repo_settings' Very true. If we know the set of things we parse early and have a way to say "this command only X, Y, and Z matters (but not W)", then the above cat-file example can omit the attributesFile from the "we care" set. I think overusing repo_settings is a disease. Moving a singleton global to per repository (by adding to struct repository) is one thing and it is very welcome. But changing the way configuration variables are parsed (e.g., what used to be parsed by only those who care about is now parsed by everybody, or vice versa) needs to be handled carefully. ``` ## Junio C Hamano, 2026-01-05 22:28 Subject: Re: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/xmqqwm1vk83a.fsf%40gitster.g In-Reply-To: <3947f777-e08a-4c17-81e3-c4711fe666a0@gmail.com> ``` Phillip Wood writes: > On 05/01/2026 14:23, Phillip Wood wrote: >> >> It is quite common that moving from parsing config settings eagerly by >> calling repo_config() at startup to parsing them lazily via 'stuct >> repo_settings' causes regressions like this. We really should find a way >> to address that before moving more settings into 'struct repo_settings' > > See > https://lore.kernel.org/git/d61c966b-61ae-4ba9-b983-c8dab6e2c292@gmail.com > for some discussion about a possible solution. Nice, but I suspect it would be an improvement already without passing repository instance via git_default_config() and instead have the code use the_repository; it is even possible not to have any repository when the callchain executes. ``` ## Bello Olamide, 2026-01-06 08:08 Subject: Re: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/CAD%3Df0L-hv1ZYGDyHRCYu3BqgrbvutS%2BJVn0D3kBq-wq--qgY7A%40mail.gmail.com In-Reply-To: ``` On Mon, 5 Jan 2026 at 15:23, Phillip Wood wrote: > > Hi Olamide > > On 02/01/2026 16:32, 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. > > This changes when the config setting gets parsed which unfortunately > regresses the user experience when the setting is invalid. > > If I run 'git -c core.attributesFile=~does-not-exist rebase -i' with git > built from master it fails immediately with "fatal: failed to expand > user dir in: '~does-not-exist'". With this patch applied it prompts me > to edit the todo list and then fails when it tries to checkout the > commit we're rebasing onto. Because "git rebase" expects reset_head() to > return an error rather die if the checkout fails it is left in a strange > state where only practical course of action for the user is to run "git > rebase --abort". Yes I tried this and I experienced the same behaviour. > > It is quite common that moving from parsing config settings eagerly by > calling repo_config() at startup to parsing them lazily via 'stuct > repo_settings' causes regressions like this. We really should find a way > to address that before moving more settings into 'struct repo_settings' > Yes, I came across an initial discussion about `prepare_repo_settings()` and the issues about the appropriate place to call it but it seemed there was no resolution then. ``` ## Bello Olamide, 2026-01-06 08:09 Subject: Re: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/CAD%3Df0L8aoddeekws0vemTuWL7vb1eJv0kRhAGvEUTVG%2B17qtDw%40mail.gmail.com In-Reply-To: <3947f777-e08a-4c17-81e3-c4711fe666a0@gmail.com> ``` On Mon, 5 Jan 2026 at 16:00, Phillip Wood wrote: > > On 05/01/2026 14:23, Phillip Wood wrote: > > > > It is quite common that moving from parsing config settings eagerly by > > calling repo_config() at startup to parsing them lazily via 'stuct > > repo_settings' causes regressions like this. We really should find a way > > to address that before moving more settings into 'struct repo_settings' > > See > https://lore.kernel.org/git/d61c966b-61ae-4ba9-b983-c8dab6e2c292@gmail.com > for some discussion about a possible solution. Yes, thank you. ``` ## Bello Olamide, 2026-01-06 09:33 Subject: Re: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/CAD%3Df0L9BEPSQivgpM7qURT%2BWFDY-%2BYs_M6Knv8hE0JDw4Wjj5A%40mail.gmail.com In-Reply-To: ``` On Mon, 5 Jan 2026 at 23:28, Junio C Hamano wrote: > > Phillip Wood writes: > > > On 05/01/2026 14:23, Phillip Wood wrote: > >> > >> It is quite common that moving from parsing config settings eagerly by > >> calling repo_config() at startup to parsing them lazily via 'stuct > >> repo_settings' causes regressions like this. We really should find a way > >> to address that before moving more settings into 'struct repo_settings' > > > > See > > https://lore.kernel.org/git/d61c966b-61ae-4ba9-b983-c8dab6e2c292@gmail.com > > for some discussion about a possible solution. > > Nice, but I suspect it would be an improvement already without > passing repository instance via git_default_config() and instead > have the code use the_repository; it is even possible not to have > any repository when the callchain executes. Thank you Junio. Okay, should I move the variable into the repo struct or repo-settings struct as the case may be, then initialize it in git_default_config()? For example ` if (!strcmp(var, "core.attributesfile")) { FREE_AND_NULL(the_repository->settings.git_attributes_file); return git_config_pathname(&the_repository->settings.git_attributes_file, var, value); } ` Does this sound reasonable? Also the `git_default_core_config()` is used to initialize the variables to store the settings in the "core" section of the config file. So how about other functions like `git _default_branch_config()`, `git _default-sparse_config()`. Do I use the same approach above? Thanks ``` ## Bello Olamide, 2026-01-06 13:44 Subject: Re: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/CAD%3Df0L9H5Q%3DzW02nr11OSBNgFH3UMLwVjVjn3zhgZ2rjwE85WA%40mail.gmail.com In-Reply-To: ``` On Tue, 6 Jan 2026 at 10:33, Bello Olamide wrote: > > On Mon, 5 Jan 2026 at 23:28, Junio C Hamano wrote: > > > > Phillip Wood writes: > > > > > On 05/01/2026 14:23, Phillip Wood wrote: > > >> > > >> It is quite common that moving from parsing config settings eagerly by > > >> calling repo_config() at startup to parsing them lazily via 'stuct > > >> repo_settings' causes regressions like this. We really should find a way > > >> to address that before moving more settings into 'struct repo_settings' > > > > > > See > > > https://lore.kernel.org/git/d61c966b-61ae-4ba9-b983-c8dab6e2c292@gmail.com > > > for some discussion about a possible solution. > > > > Nice, but I suspect it would be an improvement already without > > passing repository instance via git_default_config() and instead > > have the code use the_repository; it is even possible not to have > > any repository when the callchain executes. But won't this be a temporary solution since the goal is to prevent the use of `the_repository`? ``` ## Phillip Wood, 2026-01-07 10:17 Subject: Re: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: <34175f3d-0814-47c6-9945-7c19f2c60ca4@gmail.com> URL: https://gitlist.dev/e/34175f3d-0814-47c6-9945-7c19f2c60ca4%40gmail.com In-Reply-To: ``` On 05/01/2026 22:24, Junio C Hamano wrote: > Phillip Wood writes: > >> If I run 'git -c core.attributesFile=~does-not-exist rebase -i' with git >> built from master it fails immediately with "fatal: failed to expand >> user dir in: '~does-not-exist'". > > Hmph, if you call any behaviour change a "regression", this may > certainly count as one, but I do not necessarily think the above is > a good behaviour. One can argue that it depends on the command, but a as far as "git rebase" is concerned this change in behavior is a regression. It is not just rebase that is affected, for example "git merge" in a partial clone now downloads all the blobs it wants before erroring out which isn't terrible but it is hard to argue that's an improvement. I suspect "git cherry-pick" and "git revert" are also negatively impacted by this change. > Think about a use case where attributes are not used at all, e.g., > "git -c core.attributesFile=~does-not-matter cat-file -t HEAD"; > would it make sense to barf when your configuration file has an > invalid definition for what you are *not* using? So if the change > makes it stop barfing, it can even be argued that this is an > improvement. For some commands, but the fact that other commands now die when they're not expecting to and so end up in a strange state is a regression. Given how widespread the use of attributes is it would be hard to audit all the affected code paths and adjust them to the new behavior. >> It is quite common that moving from parsing config settings eagerly by >> calling repo_config() at startup to parsing them lazily via 'stuct >> repo_settings' causes regressions like this. We really should find a way >> to address that before moving more settings into 'struct repo_settings' > > Very true. If we know the set of things we parse early and have a > way to say "this command only X, Y, and Z matters (but not W)", then > the above cat-file example can omit the attributesFile from the "we > care" set. That would be nice, but it would be painful to implement for commands like cat-file which sometimes need to read core.attributesFile and sometimes don't depending on which options they're passed as we typically read the config before parsing the command line options. We'd also need to be careful to update the list when adding new features which depended on config variables that were not previously used by that command. > I think overusing repo_settings is a disease. Moving a singleton > global to per repository (by adding to struct repository) is one > thing and it is very welcome. But changing the way configuration > variables are parsed (e.g., what used to be parsed by only those who > care about is now parsed by everybody, or vice versa) needs to be > handled carefully. Indeed Thanks Phillip ``` ## Phillip Wood, 2026-01-07 10:26 Subject: Re: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: <922629dc-828c-4bdf-939c-b38b7b59e8e8@gmail.com> URL: https://gitlist.dev/e/922629dc-828c-4bdf-939c-b38b7b59e8e8%40gmail.com In-Reply-To: ``` On 06/01/2026 13:44, Bello Olamide wrote: > On Tue, 6 Jan 2026 at 10:33, Bello Olamide wrote: >> >> On Mon, 5 Jan 2026 at 23:28, Junio C Hamano wrote: >>> >>> Phillip Wood writes: >>> >>>> On 05/01/2026 14:23, Phillip Wood wrote: >>>>> >>>>> It is quite common that moving from parsing config settings eagerly by >>>>> calling repo_config() at startup to parsing them lazily via 'stuct >>>>> repo_settings' causes regressions like this. We really should find a way >>>>> to address that before moving more settings into 'struct repo_settings' >>>> >>>> See >>>> https://lore.kernel.org/git/d61c966b-61ae-4ba9-b983-c8dab6e2c292@gmail.com >>>> for some discussion about a possible solution. >>> >>> Nice, but I suspect it would be an improvement already without >>> passing repository instance via git_default_config() and instead >>> have the code use the_repository; it is even possible not to have >>> any repository when the callchain executes. > > But won't this be a temporary solution since the goal is to prevent the use of > `the_repository`? Yes but it would be a good start as passing a repository down to git_default_config() will be quite invasive. It would certainly be better if we can find a solution that uses the repository passed to command when it is non-NULL. Unfortunately commands like "git diff --no-index" are passed a NULL repository but we have chosen to store our config in a `struct repository` and so we need some kind of fake repository for those commands. If we stored our config in a separate struct we wouldn't need to fake a repository but then we'd have to pass the config round separately to the repository which is a pain. Perhaps git_default_config() could use `the_repository` when it's given a NULL pointer for the callback data. Thanks Phillip ``` ## Phillip Wood, 2026-01-07 14:18 Subject: Re: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: <8899016f-eeef-404b-8da6-ff3a90e81cea@gmail.com> URL: https://gitlist.dev/e/8899016f-eeef-404b-8da6-ff3a90e81cea%40gmail.com In-Reply-To: <922629dc-828c-4bdf-939c-b38b7b59e8e8@gmail.com> ``` On 07/01/2026 10:26, Phillip Wood wrote: > On 06/01/2026 13:44, Bello Olamide wrote: >> >> But won't this be a temporary solution since the goal is to prevent >> the use of >> `the_repository`? > > Yes but it would be a good start as passing a repository down to > git_default_config() will be quite invasive. To expand on this the first steps could be (i) create a new struct to hold the config settings from git_default_config() (ii) add that struct as a member of `struct repository` (iii) one-by-one, for each setting parsed by git_default_config() add a new member to the config struct, store the parsed value in `the_repository` and adjust any code that uses the variable. Then later we can tackle the intrusive change to pass a `struct repository` down to git_default_config() and store the settings in that rather than `the_repository`. If we add a local variable to git_default_config() in step (iii) above then getting it to use the repository passed down the call chain will simply be a matter of doing something like - struct repository *r = the_repository; + struct repository *r = cb ? cb : the_repository; Thanks Phillip ``` ## Bello Olamide, 2026-01-07 15:33 Subject: Re: [Outreachy PATCH v2] environment: move "core.attributesFile" into repo-setting Message-ID: URL: https://gitlist.dev/e/CAD%3Df0L9in5tjLNUcM86uzd_caTaZYre9RRygmizO4G%3DS_DBFRQ%40mail.gmail.com In-Reply-To: <8899016f-eeef-404b-8da6-ff3a90e81cea@gmail.com> ``` On Wed, 7 Jan 2026 at 15:19, Phillip Wood wrote: > > On 07/01/2026 10:26, Phillip Wood wrote: > > On 06/01/2026 13:44, Bello Olamide wrote: > >> > >> But won't this be a temporary solution since the goal is to prevent > >> the use of > >> `the_repository`? > > > > Yes but it would be a good start as passing a repository down to > > git_default_config() will be quite invasive. > > To expand on this the first steps could be > (i) create a new struct to hold the config settings from > git_default_config() > (ii) add that struct as a member of `struct repository` > (iii) one-by-one, for each setting parsed by git_default_config() add a > new member to the config struct, store the parsed value in > `the_repository` and adjust any code that uses the variable. > > Then later we can tackle the intrusive change to pass a `struct > repository` down to git_default_config() and store the settings in that > rather than `the_repository`. If we add a local variable to > git_default_config() in step (iii) above then getting it to use the > repository passed down the call chain will simply be a matter of doing > something like > > - struct repository *r = the_repository; > + struct repository *r = cb ? cb : the_repository; > > Thanks > > Phillip > Thanks for the detailed proposal, which makes a lot of sense. I’ll align my changes with this staged approach. Bello ```