git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Jan 5, 2026, 14:23 UTC
Message-ID
<a881499d-e236-4f8e-a217-b6bce69e3e3c@gmail.com>
In-Reply-To
<aVfzMsN2ouY3UBFG@ubuntu>
Hi Olamide
On 02/01/2026 16:32, Olamide Caleb Bello wrote:
Show 10 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.

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
Show 264 quoted lines
> 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/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 <ayu.chandekar@gmail.com>
>      +    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>
>      @@ 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 */
Previous: Bello OlamideNext: Phillip Wood
Message 10 of 21 in “environment: move "core.attributesFile" into repo-setting”
  1. environment: move "core.attributesFile" into repo-settingOlamide Caleb Bello, Dec 18, 2025
  2. Bello OlamideDec 18, 2025
  3. Bello OlamideJan 2, 2026
  4. Karthik NayakJan 2, 2026
  5. Karthik NayakJan 2, 2026
  6. Bello OlamideJan 2, 2026
  7. environment: move "core.attributesFile" into repo-settingOlamide Caleb Bello, Jan 2, 2026
  8. Karthik NayakJan 5, 2026
  9. Bello OlamideJan 5, 2026
  10. Phillip WoodJan 5, 2026
  11. Phillip WoodJan 5, 2026
  12. Junio C HamanoJan 5, 2026
  13. Bello OlamideJan 6, 2026
  14. Bello OlamideJan 6, 2026
  15. Phillip WoodJan 7, 2026
  16. Phillip WoodJan 7, 2026
  17. Bello OlamideJan 7, 2026
  18. Bello OlamideJan 6, 2026
  19. Junio C HamanoJan 5, 2026
  20. Phillip WoodJan 7, 2026
  21. Bello OlamideJan 6, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.