{"thread":{"id":"63081","subject":"[PATCH] environment: move access to \"core.attributesfile\" into repo settings","startedAt":"2025-03-09T15:33:34Z","lastAt":"2025-12-09T09:23:07Z","messageCount":18,"participants":["Ayush Chandekar","Patrick Steinhardt","Junio C Hamano","Karthik Nayak","shejialuo","Olamide Caleb Bello","Bello Olamide"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"513851","messageId":"20250309153321.254844-1-ayu.chandekar@gmail.com","threadId":"63081","inReplyTo":null,"subject":"[PATCH] environment: move access to \"core.attributesfile\" into repo settings","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-03-09T15:33:21Z","receivedAt":"2025-03-09T15:33:34Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"Stop relying on global state to access the \"core.attributesfile\"\nconfiguration. Instead, store the value in `struct repo_settings` and\nretrieve it via `repo_settings_get_attributes_file_path()`.\n\nThis prevents incorrect values from being used when a user or tool is\nhandling multiple repositories in the same process, each with different\nattribute configurations. It also improves repository isolation and helps\nprogress towards libification by avoiding unnecessary global state.\n\nSigned-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n---\n attr.c               | 27 ++++++++++++---------------\n attr.h               |  7 +++++--\n builtin/check-attr.c |  2 +-\n builtin/var.c        |  9 +++++++--\n config.c             |  5 -----\n environment.c        |  1 -\n environment.h        |  1 -\n repo-settings.c      | 11 +++++++++++\n repo-settings.h      |  3 +++\n 9 files changed, 39 insertions(+), 27 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 0bd2750528..aec4b42245 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -879,12 +879,9 @@ const char *git_attr_system_file(void)\n \treturn system_wide;\n }\n \n-const char *git_attr_global_file(void)\n+const char *git_attr_global_file(struct repository *repo)\n {\n-\tif (!git_attributes_file)\n-\t\tgit_attributes_file = xdg_config_home(\"attributes\");\n-\n-\treturn git_attributes_file;\n+\treturn repo_settings_get_attributesfile_path(repo);\n }\n \n int git_attr_system_is_enabled(void)\n@@ -906,7 +903,7 @@ static void push_stack(struct attr_stack **attr_stack_p,\n \t}\n }\n \n-static void bootstrap_attr_stack(struct index_state *istate,\n+static void bootstrap_attr_stack(struct repository *repo, struct index_state *istate,\n \t\t\t\t const struct object_id *tree_oid,\n \t\t\t\t struct attr_stack **stack)\n {\n@@ -927,8 +924,8 @@ static void bootstrap_attr_stack(struct index_state *istate,\n \t}\n \n \t/* home directory */\n-\tif (git_attr_global_file()) {\n-\t\te = read_attr_from_file(git_attr_global_file(), flags);\n+\tif (git_attr_global_file(repo)) {\n+\t\te = read_attr_from_file(git_attr_global_file(repo), flags);\n \t\tpush_stack(stack, e, NULL, 0);\n \t}\n \n@@ -946,7 +943,7 @@ static void bootstrap_attr_stack(struct index_state *istate,\n \tpush_stack(stack, e, NULL, 0);\n }\n \n-static void prepare_attr_stack(struct index_state *istate,\n+static void prepare_attr_stack(struct repository *repo, struct index_state *istate,\n \t\t\t       const struct object_id *tree_oid,\n \t\t\t       const char *path, int dirlen,\n \t\t\t       struct attr_stack **stack)\n@@ -969,7 +966,7 @@ static void prepare_attr_stack(struct index_state *istate,\n \t * .gitattributes in deeper directories to shallower ones,\n \t * and finally use the built-in set as the default.\n \t */\n-\tbootstrap_attr_stack(istate, tree_oid, stack);\n+\tbootstrap_attr_stack(repo, istate, tree_oid, stack);\n \n \t/*\n \t * Pop the \"info\" one that is always at the top of the stack.\n@@ -1143,7 +1140,7 @@ static void determine_macros(struct all_attrs_item *all_attrs,\n  * If check->check_nr is non-zero, only attributes in check[] are collected.\n  * Otherwise all attributes are collected.\n  */\n-static void collect_some_attrs(struct index_state *istate,\n+static void collect_some_attrs(struct repository *repo, struct index_state *istate,\n \t\t\t       const struct object_id *tree_oid,\n \t\t\t       const char *path, struct attr_check *check)\n {\n@@ -1164,7 +1161,7 @@ static void collect_some_attrs(struct index_state *istate,\n \t\tdirlen = 0;\n \t}\n \n-\tprepare_attr_stack(istate, tree_oid, path, dirlen, &check->stack);\n+\tprepare_attr_stack(repo, istate, tree_oid, path, dirlen, &check->stack);\n \tall_attrs_init(&g_attr_hashmap, check);\n \tdetermine_macros(check->all_attrs, check->stack);\n \n@@ -1310,7 +1307,7 @@ void git_check_attr(struct index_state *istate,\n \tint i;\n \tconst struct object_id *tree_oid = default_attr_source();\n \n-\tcollect_some_attrs(istate, tree_oid, path, check);\n+\tcollect_some_attrs(the_repository, istate, tree_oid, path, check);\n \n \tfor (i = 0; i < check->nr; i++) {\n \t\tunsigned int n = check->items[i].attr->attr_nr;\n@@ -1321,14 +1318,14 @@ void git_check_attr(struct index_state *istate,\n \t}\n }\n \n-void git_all_attrs(struct index_state *istate,\n+void git_all_attrs(struct repository *repo, struct index_state *istate,\n \t\t   const char *path, struct attr_check *check)\n {\n \tint i;\n \tconst struct object_id *tree_oid = default_attr_source();\n \n \tattr_check_reset(check);\n-\tcollect_some_attrs(istate, tree_oid, path, check);\n+\tcollect_some_attrs(repo, istate, tree_oid, path, check);\n \n \tfor (i = 0; i < check->all_attrs_nr; i++) {\n \t\tconst char *name = check->all_attrs[i].attr->name;\ndiff --git a/attr.h b/attr.h\nindex a04a521092..c4f26b8f58 100644\n--- a/attr.h\n+++ b/attr.h\n@@ -213,11 +213,13 @@ void git_check_attr(struct index_state *istate,\n \t\t    const char *path,\n \t\t    struct attr_check *check);\n \n+struct repository;\n+\n /*\n  * Retrieve all attributes that apply to the specified path.\n  * check holds the attributes and their values.\n  */\n-void git_all_attrs(struct index_state *istate,\n+void git_all_attrs(struct repository *repo, struct index_state *istate,\n \t\t   const char *path, struct attr_check *check);\n \n enum git_attr_direction {\n@@ -233,7 +235,7 @@ void attr_start(void);\n const char *git_attr_system_file(void);\n \n /* Return the global gitattributes file, if any. */\n-const char *git_attr_global_file(void);\n+const char *git_attr_global_file(struct repository *repo);\n \n /* Return whether the system gitattributes file is enabled and should be used. */\n int git_attr_system_is_enabled(void);\n@@ -283,4 +285,5 @@ struct match_attr {\n struct match_attr *parse_attr_line(const char *line, const char *src,\n \t\t\t\t   int lineno, unsigned flags);\n \n+\n #endif /* ATTR_H */\ndiff --git a/builtin/check-attr.c b/builtin/check-attr.c\nindex 7cf275b893..1b8a89dfb2 100644\n--- a/builtin/check-attr.c\n+++ b/builtin/check-attr.c\n@@ -70,7 +70,7 @@ static void check_attr(const char *prefix, struct attr_check *check,\n \t\tprefix_path(prefix, prefix ? strlen(prefix) : 0, file);\n \n \tif (collect_all) {\n-\t\tgit_all_attrs(the_repository->index, full_path, check);\n+\t\tgit_all_attrs(the_repository, the_repository->index, full_path, check);\n \t} else {\n \t\tgit_check_attr(the_repository->index, full_path, check);\n \t}\ndiff --git a/builtin/var.c b/builtin/var.c\nindex ada642a9fe..3d635c235e 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -69,9 +69,9 @@ static char *git_attr_val_system(int ident_flag UNUSED)\n \treturn NULL;\n }\n \n-static char *git_attr_val_global(int ident_flag UNUSED)\n+static char *repo_git_attr_val_global(struct repository *repo, int ident_flag UNUSED)\n {\n-\tchar *file = xstrdup_or_null(git_attr_global_file());\n+\tchar *file = xstrdup_or_null(git_attr_global_file(repo));\n \tif (file) {\n \t\tnormalize_path_copy(file, file);\n \t\treturn file;\n@@ -79,6 +79,11 @@ static char *git_attr_val_global(int ident_flag UNUSED)\n \treturn NULL;\n }\n \n+static char *git_attr_val_global(int ident_flag)\n+{\n+\treturn repo_git_attr_val_global(the_repository, ident_flag);\n+}\n+\n static char *git_config_val_system(int ident_flag UNUSED)\n {\n \tif (git_config_system()) {\ndiff --git a/config.c b/config.c\nindex 36f76fafe5..d483f1418c 100644\n--- a/config.c\n+++ b/config.c\n@@ -1432,11 +1432,6 @@ static int git_default_core_config(const char *var, const char *value,\n \t\treturn 0;\n \t}\n \n-\tif (!strcmp(var, \"core.attributesfile\")) {\n-\t\tFREE_AND_NULL(git_attributes_file);\n-\t\treturn git_config_pathname(&git_attributes_file, var, value);\n-\t}\n-\n \tif (!strcmp(var, \"core.hookspath\")) {\n \t\tFREE_AND_NULL(git_hooks_path);\n \t\treturn git_config_pathname(&git_hooks_path, var, value);\ndiff --git a/environment.c b/environment.c\nindex e5b361bb5d..e1da5a69b7 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -42,7 +42,6 @@ char *git_commit_encoding;\n char *git_log_output_encoding;\n char *apply_default_whitespace;\n char *apply_default_ignorewhitespace;\n-char *git_attributes_file;\n char *git_hooks_path;\n int zlib_compression_level = Z_BEST_SPEED;\n int pack_compression_level = Z_DEFAULT_COMPRESSION;\ndiff --git a/environment.h b/environment.h\nindex 2f43340f0b..9dc6bc0f3f 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -159,7 +159,6 @@ extern int assume_unchanged;\n extern int warn_on_object_refname_ambiguity;\n extern char *apply_default_whitespace;\n extern char *apply_default_ignorewhitespace;\n-extern char *git_attributes_file;\n extern char *git_hooks_path;\n extern int zlib_compression_level;\n extern int pack_compression_level;\ndiff --git a/repo-settings.c b/repo-settings.c\nindex 9d16d5399e..420ca72f5f 100644\n--- a/repo-settings.c\n+++ b/repo-settings.c\n@@ -4,6 +4,7 @@\n #include \"repository.h\"\n #include \"midx.h\"\n #include \"pack-objects.h\"\n+#include \"path.h\"\n \n static void repo_cfg_bool(struct repository *r, const char *key, int *dest,\n \t\t\t  int def)\n@@ -167,3 +168,13 @@ int repo_settings_get_warn_ambiguous_refs(struct repository *repo)\n \t\t\t      &repo->settings.warn_ambiguous_refs, 1);\n \treturn repo->settings.warn_ambiguous_refs;\n }\n+\n+const char *repo_settings_get_attributesfile_path(struct repository *repo)\n+{\n+\tif (!repo->settings.git_attributes_file) {\n+\t\tif (repo_config_get_pathname(repo, \"core.attributesfile\", &repo->settings.git_attributes_file)) {\n+\t\t\trepo->settings.git_attributes_file = xdg_config_home(\"attributes\");\n+\t\t}\n+\t}\n+\treturn repo->settings.git_attributes_file;\n+}\n\\ No newline at end of file\ndiff --git a/repo-settings.h b/repo-settings.h\nindex 93ea0c3274..2a5ca5f07d 100644\n--- a/repo-settings.h\n+++ b/repo-settings.h\n@@ -61,6 +61,7 @@ struct repo_settings {\n \tsize_t delta_base_cache_limit;\n \tsize_t packed_git_window_size;\n \tsize_t packed_git_limit;\n+\tchar *git_attributes_file;\n };\n #define REPO_SETTINGS_INIT { \\\n \t.index_version = -1, \\\n@@ -78,5 +79,7 @@ void prepare_repo_settings(struct repository *r);\n enum log_refs_config repo_settings_get_log_all_ref_updates(struct repository *repo);\n /* Read the value for \"core.warnAmbiguousRefs\". */\n int repo_settings_get_warn_ambiguous_refs(struct repository *repo);\n+/* Read the value for \"core.attributesfile\". */\n+const char *repo_settings_get_attributesfile_path(struct repository *repo);\n \n #endif /* REPO_SETTINGS_H */\n-- \n2.48.GIT\n\n"},{"id":"513870","messageId":"Z86PUkJ1sbSH2VTU@pks.im","threadId":"63081","inReplyTo":"20250309153321.254844-1-ayu.chandekar@gmail.com","subject":"Re: [PATCH] environment: move access to \"core.attributesfile\" into repo settings","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-10T07:05:54Z","receivedAt":"2025-03-10T07:06:04Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Mar 09, 2025 at 09:03:21PM +0530, Ayush Chandekar wrote:\n> Stop relying on global state to access the \"core.attributesfile\"\n> configuration. Instead, store the value in `struct repo_settings` and\n> retrieve it via `repo_settings_get_attributes_file_path()`.\n> \n> This prevents incorrect values from being used when a user or tool is\n> handling multiple repositories in the same process, each with different\n> attribute configurations. It also improves repository isolation and helps\n> progress towards libification by avoiding unnecessary global state.\n\nWe typically switch the order around a bit in our commit messages: we\nfirst explain what the actual problem is, and then we say how we fix it.\n\n> diff --git a/attr.c b/attr.c\n> index 0bd2750528..aec4b42245 100644\n> --- a/attr.c\n> +++ b/attr.c\n> @@ -879,12 +879,9 @@ const char *git_attr_system_file(void)\n>  \treturn system_wide;\n>  }\n>  \n> -const char *git_attr_global_file(void)\n> +const char *git_attr_global_file(struct repository *repo)\n>  {\n> -\tif (!git_attributes_file)\n> -\t\tgit_attributes_file = xdg_config_home(\"attributes\");\n> -\n> -\treturn git_attributes_file;\n> +\treturn repo_settings_get_attributesfile_path(repo);\n>  }\n>  \n>  int git_attr_system_is_enabled(void)\n\nHm. I wonder what the actual merit of this function is after the\nrefactoring. Right now there isn't really any as it is a direct wrapper\nof `repo_settings_get_attributesfile_path()`.\n\n> diff --git a/attr.h b/attr.h\n> index a04a521092..c4f26b8f58 100644\n> --- a/attr.h\n> +++ b/attr.h\n> @@ -213,11 +213,13 @@ void git_check_attr(struct index_state *istate,\n>  \t\t    const char *path,\n>  \t\t    struct attr_check *check);\n>  \n> +struct repository;\n> +\n>  /*\n>   * Retrieve all attributes that apply to the specified path.\n>   * check holds the attributes and their values.\n>   */\n> -void git_all_attrs(struct index_state *istate,\n> +void git_all_attrs(struct repository *repo, struct index_state *istate,\n>  \t\t   const char *path, struct attr_check *check);\n>  \n>  enum git_attr_direction {\n> @@ -233,7 +235,7 @@ void attr_start(void);\n>  const char *git_attr_system_file(void);\n>  \n>  /* Return the global gitattributes file, if any. */\n> -const char *git_attr_global_file(void);\n> +const char *git_attr_global_file(struct repository *repo);\n>  \n>  /* Return whether the system gitattributes file is enabled and should be used. */\n>  int git_attr_system_is_enabled(void);\n\nI think it would make sense to split out this change into a separate\ncommit. The first commit would move the config into \"repo-settings.c\",\nthe second commit would adapt functions and their callers as necessary.\n\n> @@ -283,4 +285,5 @@ struct match_attr {\n>  struct match_attr *parse_attr_line(const char *line, const char *src,\n>  \t\t\t\t   int lineno, unsigned flags);\n>  \n> +\n>  #endif /* ATTR_H */\n\nExtraneous newline.\n\n> diff --git a/builtin/check-attr.c b/builtin/check-attr.c\n> index 7cf275b893..1b8a89dfb2 100644\n> --- a/builtin/check-attr.c\n> +++ b/builtin/check-attr.c\n> @@ -70,7 +70,7 @@ static void check_attr(const char *prefix, struct attr_check *check,\n>  \t\tprefix_path(prefix, prefix ? strlen(prefix) : 0, file);\n>  \n>  \tif (collect_all) {\n> -\t\tgit_all_attrs(the_repository->index, full_path, check);\n> +\t\tgit_all_attrs(the_repository, the_repository->index, full_path, check);\n>  \t} else {\n>  \t\tgit_check_attr(the_repository->index, full_path, check);\n>  \t}\n> diff --git a/builtin/var.c b/builtin/var.c\n> index ada642a9fe..3d635c235e 100644\n> --- a/builtin/var.c\n> +++ b/builtin/var.c\n> @@ -69,9 +69,9 @@ static char *git_attr_val_system(int ident_flag UNUSED)\n>  \treturn NULL;\n>  }\n>  \n> -static char *git_attr_val_global(int ident_flag UNUSED)\n> +static char *repo_git_attr_val_global(struct repository *repo, int ident_flag UNUSED)\n>  {\n> -\tchar *file = xstrdup_or_null(git_attr_global_file());\n> +\tchar *file = xstrdup_or_null(git_attr_global_file(repo));\n>  \tif (file) {\n>  \t\tnormalize_path_copy(file, file);\n>  \t\treturn file;\n> @@ -79,6 +79,11 @@ static char *git_attr_val_global(int ident_flag UNUSED)\n>  \treturn NULL;\n>  }\n>  \n> +static char *git_attr_val_global(int ident_flag)\n> +{\n> +\treturn repo_git_attr_val_global(the_repository, ident_flag);\n> +}\n> +\n>  static char *git_config_val_system(int ident_flag UNUSED)\n>  {\n>  \tif (git_config_system()) {\n\nI think we should just retain `git_attr_val_global()` and plug in\n`the_repository`. The extra change here doesn't add anything, and\n\"builtin/var.c\" being a builtin means is not reused anywhere else,\neither.\n\n> diff --git a/repo-settings.c b/repo-settings.c\n> index 9d16d5399e..420ca72f5f 100644\n> --- a/repo-settings.c\n> +++ b/repo-settings.c\n> @@ -167,3 +168,13 @@ int repo_settings_get_warn_ambiguous_refs(struct repository *repo)\n>  \t\t\t      &repo->settings.warn_ambiguous_refs, 1);\n>  \treturn repo->settings.warn_ambiguous_refs;\n>  }\n> +\n> +const char *repo_settings_get_attributesfile_path(struct repository *repo)\n> +{\n> +\tif (!repo->settings.git_attributes_file) {\n> +\t\tif (repo_config_get_pathname(repo, \"core.attributesfile\", &repo->settings.git_attributes_file)) {\n> +\t\t\trepo->settings.git_attributes_file = xdg_config_home(\"attributes\");\n> +\t\t}\n> +\t}\n\nWe don't use curly braces around one-line statements.\n\n> +\treturn repo->settings.git_attributes_file;\n> +}\n\nOne thing I'm missing is the code to `free()` the allocated memory in\n`repo_settings_clear()`.\n\n> \\ No newline at end of file\n\nNit: missing newline at the end of the file.\n\nThanks!\n\nPatrick\n"},{"id":"513888","messageId":"CAE7as+bm1+aMz3SpiYeZWD9PUHNjzOYNgKm_FnEPzJesSFcodA@mail.gmail.com","threadId":"63081","inReplyTo":"Z86PUkJ1sbSH2VTU@pks.im","subject":"Re: [PATCH] environment: move access to \"core.attributesfile\" into repo settings","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-03-10T09:07:24Z","receivedAt":"2025-03-10T09:07:36Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"Hey,\nthanks for reviewing the patch!\n\n> We typically switch the order around a bit in our commit messages: we\n> first explain what the actual problem is, and then we say how we fix it.\n\nGot it.\n\n\n> Hm. I wonder what the actual merit of this function is after the\n> refactoring. Right now there isn't really any as it is a direct wrapper\n> of `repo_settings_get_attributesfile_path()`.\n\nI can remove the function and replace all the instances with\n`repo_settings_get_attributesfile_path()`. What do you think?\n\n> I think it would make sense to split out this change into a separate\n> commit. The first commit would move the config into \"repo-settings.c\",\n> the second commit would adapt functions and their callers as necessary.\n\nAlright.\n\n> Extraneous newline.\nApologies. Will fix it.\n\n> I think we should just retain `git_attr_val_global()` and plug in\n> `the_repository`. The extra change here doesn't add anything, and\n> \"builtin/var.c\" being a builtin means is not reused anywhere else,\n> either.\n\nMakes sense. I will drop `repo_git_attr_val_globa()` and keep\n`git_attr_val_global()` with `the_repository`.\n\n\n> We don't use curly braces around one-line statements.\nWill fix it.\n\n> One thing I'm missing is the code to `free()` the allocated memory in\n> `repo_settings_clear()`.\nOh right.\n\n> > \\ No newline at end of file\n\nGot it.\n\nThanks,\nAyush:)\n"},{"id":"513898","messageId":"20250310151048.69825-1-ayu.chandekar@gmail.com","threadId":"63081","inReplyTo":"20250309153321.254844-1-ayu.chandekar@gmail.com","subject":"[GSOC PATCH v2 0/2] Stop depending on `the_repository` for core.attributesfile","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-03-10T15:10:46Z","receivedAt":"2025-03-10T15:11:11Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"This series moves access to the \"core.attributesfile\" configuration into\n`repo_settings`, eliminating the dependency on the global `the_repository`\ninstance. It also updates the relevant attribute-related code paths to use\na repository-scoped accessor. This is a part of the ongoing effort towards\nlibification of git.\n\nAyush Chandekar (2):\n  environment: move access to \"core.attributesfile\" into repo settings\n  attr: use `repo_settings_get_attributesfile_path()` and update callers\n\n attr.c               | 28 ++++++++++------------------\n attr.h               |  7 +++----\n builtin/check-attr.c |  2 +-\n builtin/var.c        |  2 +-\n config.c             |  5 -----\n environment.c        |  1 -\n environment.h        |  1 -\n repo-settings.c      | 11 +++++++++++\n repo-settings.h      |  3 +++\n 9 files changed, 29 insertions(+), 31 deletions(-)\n\n-- \n2.48.GIT\n\n"},{"id":"513899","messageId":"20250310151048.69825-2-ayu.chandekar@gmail.com","threadId":"63081","inReplyTo":"20250310151048.69825-1-ayu.chandekar@gmail.com","subject":"[GSOC PATCH v2 1/2] environment: move access to \"core.attributesfile\" into repo settings","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-03-10T15:10:47Z","receivedAt":"2025-03-10T15:11:14Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"When handling multiple repositories within the same process, relying on\nglobal state for accessing the \"core.attributesfile\" configuration can\nlead to incorrect values being used. It also makes it harder to isolate\nrepositories and hinders the libification of git.\n\nStore the \"core.attributesfile\" configuration in the `repo_settings`\ninstead of relying on the global state. Add a new function\n`repo_settings_get_attributesfile_path()` to retrieve this setting in a\nrepository-scoped manner.\n\nSigned-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n---\n config.c        |  5 -----\n environment.c   |  1 -\n environment.h   |  1 -\n repo-settings.c | 11 +++++++++++\n repo-settings.h |  3 +++\n 5 files changed, 14 insertions(+), 7 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 658569af08..b52ad3e3ad 100644\n--- a/config.c\n+++ b/config.c\n@@ -1432,11 +1432,6 @@ static int git_default_core_config(const char *var, const char *value,\n \t\treturn 0;\n \t}\n \n-\tif (!strcmp(var, \"core.attributesfile\")) {\n-\t\tFREE_AND_NULL(git_attributes_file);\n-\t\treturn git_config_pathname(&git_attributes_file, var, value);\n-\t}\n-\n \tif (!strcmp(var, \"core.bare\")) {\n \t\tis_bare_repository_cfg = git_config_bool(var, value);\n \t\treturn 0;\ndiff --git a/environment.c b/environment.c\nindex 9e4c7781be..d7bf911ec5 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -42,7 +42,6 @@ char *git_commit_encoding;\n char *git_log_output_encoding;\n char *apply_default_whitespace;\n char *apply_default_ignorewhitespace;\n-char *git_attributes_file;\n int zlib_compression_level = Z_BEST_SPEED;\n int pack_compression_level = Z_DEFAULT_COMPRESSION;\n int fsync_object_files = -1;\ndiff --git a/environment.h b/environment.h\nindex 45e690f203..b7860eed3a 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -149,7 +149,6 @@ extern int assume_unchanged;\n extern int warn_on_object_refname_ambiguity;\n extern char *apply_default_whitespace;\n extern char *apply_default_ignorewhitespace;\n-extern char *git_attributes_file;\n extern int zlib_compression_level;\n extern int pack_compression_level;\n extern size_t packed_git_window_size;\ndiff --git a/repo-settings.c b/repo-settings.c\nindex 67e9cfd2e6..17e60aa0d6 100644\n--- a/repo-settings.c\n+++ b/repo-settings.c\n@@ -5,6 +5,7 @@\n #include \"midx.h\"\n #include \"pack-objects.h\"\n #include \"setup.h\"\n+#include \"path.h\"\n \n static void repo_cfg_bool(struct repository *r, const char *key, int *dest,\n \t\t\t  int def)\n@@ -148,6 +149,7 @@ void repo_settings_clear(struct repository *r)\n \tstruct repo_settings empty = REPO_SETTINGS_INIT;\n \tFREE_AND_NULL(r->settings.fsmonitor);\n \tFREE_AND_NULL(r->settings.hooks_path);\n+\tFREE_AND_NULL(r->settings.git_attributes_file);\n \tr->settings = empty;\n }\n \n@@ -207,3 +209,12 @@ void repo_settings_reset_shared_repository(struct repository *repo)\n {\n \trepo->settings.shared_repository_initialized = 0;\n }\n+\n+const char *repo_settings_get_attributesfile_path(struct repository *repo)\n+{\n+\tif (!repo->settings.git_attributes_file) {\n+\t\tif (repo_config_get_pathname(repo, \"core.attributesfile\", &repo->settings.git_attributes_file))\n+\t\t\trepo->settings.git_attributes_file = xdg_config_home(\"attributes\");\n+\t}\n+\treturn repo->settings.git_attributes_file;\n+}\ndiff --git a/repo-settings.h b/repo-settings.h\nindex ddc11967e0..58dadd9dae 100644\n--- a/repo-settings.h\n+++ b/repo-settings.h\n@@ -66,6 +66,7 @@ struct repo_settings {\n \tsize_t packed_git_limit;\n \n \tchar *hooks_path;\n+\tchar *git_attributes_file;\n };\n #define REPO_SETTINGS_INIT { \\\n \t.shared_repository = -1, \\\n@@ -92,5 +93,7 @@ const char *repo_settings_get_hooks_path(struct repository *repo);\n int repo_settings_get_shared_repository(struct repository *repo);\n void repo_settings_set_shared_repository(struct repository *repo, int value);\n void repo_settings_reset_shared_repository(struct repository *repo);\n+/* Read the value for \"core.attributesfile\". */\n+const char *repo_settings_get_attributesfile_path(struct repository *repo);\n \n #endif /* REPO_SETTINGS_H */\n-- \n2.48.GIT\n\n"},{"id":"513900","messageId":"20250310151048.69825-3-ayu.chandekar@gmail.com","threadId":"63081","inReplyTo":"20250310151048.69825-1-ayu.chandekar@gmail.com","subject":"[GSOC PATCH v2 2/2] attr: use `repo_settings_get_attributesfile_path()` and update callers","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-03-10T15:10:48Z","receivedAt":"2025-03-10T15:11:17Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"Update attribute-related functions to retrieve the \"core.attributesfile\"\nconfiguration via the new repository-scoped accessor\n`repo_settings_get_attributesfile_path()`. This improves behaviour in\nmulti-repository contexts and aligns with the goal of minimizing\nreliance on global state.\n\nSigned-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n---\n attr.c               | 28 ++++++++++------------------\n attr.h               |  7 +++----\n builtin/check-attr.c |  2 +-\n builtin/var.c        |  2 +-\n 4 files changed, 15 insertions(+), 24 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 0bd2750528..8f28463e8c 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -879,14 +879,6 @@ const char *git_attr_system_file(void)\n \treturn system_wide;\n }\n \n-const char *git_attr_global_file(void)\n-{\n-\tif (!git_attributes_file)\n-\t\tgit_attributes_file = xdg_config_home(\"attributes\");\n-\n-\treturn git_attributes_file;\n-}\n-\n int git_attr_system_is_enabled(void)\n {\n \treturn !git_env_bool(\"GIT_ATTR_NOSYSTEM\", 0);\n@@ -906,7 +898,7 @@ static void push_stack(struct attr_stack **attr_stack_p,\n \t}\n }\n \n-static void bootstrap_attr_stack(struct index_state *istate,\n+static void bootstrap_attr_stack(struct repository *repo, struct index_state *istate,\n \t\t\t\t const struct object_id *tree_oid,\n \t\t\t\t struct attr_stack **stack)\n {\n@@ -927,8 +919,8 @@ static void bootstrap_attr_stack(struct index_state *istate,\n \t}\n \n \t/* home directory */\n-\tif (git_attr_global_file()) {\n-\t\te = read_attr_from_file(git_attr_global_file(), flags);\n+\tif (repo_settings_get_attributesfile_path(repo)) {\n+\t\te = read_attr_from_file(repo_settings_get_attributesfile_path(repo), flags);\n \t\tpush_stack(stack, e, NULL, 0);\n \t}\n \n@@ -946,7 +938,7 @@ static void bootstrap_attr_stack(struct index_state *istate,\n \tpush_stack(stack, e, NULL, 0);\n }\n \n-static void prepare_attr_stack(struct index_state *istate,\n+static void prepare_attr_stack(struct repository *repo, struct index_state *istate,\n \t\t\t       const struct object_id *tree_oid,\n \t\t\t       const char *path, int dirlen,\n \t\t\t       struct attr_stack **stack)\n@@ -969,7 +961,7 @@ static void prepare_attr_stack(struct index_state *istate,\n \t * .gitattributes in deeper directories to shallower ones,\n \t * and finally use the built-in set as the default.\n \t */\n-\tbootstrap_attr_stack(istate, tree_oid, stack);\n+\tbootstrap_attr_stack(repo, istate, tree_oid, stack);\n \n \t/*\n \t * Pop the \"info\" one that is always at the top of the stack.\n@@ -1143,7 +1135,7 @@ static void determine_macros(struct all_attrs_item *all_attrs,\n  * If check->check_nr is non-zero, only attributes in check[] are collected.\n  * Otherwise all attributes are collected.\n  */\n-static void collect_some_attrs(struct index_state *istate,\n+static void collect_some_attrs(struct repository *repo, struct index_state *istate,\n \t\t\t       const struct object_id *tree_oid,\n \t\t\t       const char *path, struct attr_check *check)\n {\n@@ -1164,7 +1156,7 @@ static void collect_some_attrs(struct index_state *istate,\n \t\tdirlen = 0;\n \t}\n \n-\tprepare_attr_stack(istate, tree_oid, path, dirlen, &check->stack);\n+\tprepare_attr_stack(repo, istate, tree_oid, path, dirlen, &check->stack);\n \tall_attrs_init(&g_attr_hashmap, check);\n \tdetermine_macros(check->all_attrs, check->stack);\n \n@@ -1310,7 +1302,7 @@ void git_check_attr(struct index_state *istate,\n \tint i;\n \tconst struct object_id *tree_oid = default_attr_source();\n \n-\tcollect_some_attrs(istate, tree_oid, path, check);\n+\tcollect_some_attrs(the_repository, istate, tree_oid, path, check);\n \n \tfor (i = 0; i < check->nr; i++) {\n \t\tunsigned int n = check->items[i].attr->attr_nr;\n@@ -1321,14 +1313,14 @@ void git_check_attr(struct index_state *istate,\n \t}\n }\n \n-void git_all_attrs(struct index_state *istate,\n+void git_all_attrs(struct repository *repo, struct index_state *istate,\n \t\t   const char *path, struct attr_check *check)\n {\n \tint i;\n \tconst struct object_id *tree_oid = default_attr_source();\n \n \tattr_check_reset(check);\n-\tcollect_some_attrs(istate, tree_oid, path, check);\n+\tcollect_some_attrs(repo, istate, tree_oid, path, check);\n \n \tfor (i = 0; i < check->all_attrs_nr; i++) {\n \t\tconst char *name = check->all_attrs[i].attr->name;\ndiff --git a/attr.h b/attr.h\nindex a04a521092..1ff058bef7 100644\n--- a/attr.h\n+++ b/attr.h\n@@ -213,11 +213,13 @@ void git_check_attr(struct index_state *istate,\n \t\t    const char *path,\n \t\t    struct attr_check *check);\n \n+struct repository;\n+\n /*\n  * Retrieve all attributes that apply to the specified path.\n  * check holds the attributes and their values.\n  */\n-void git_all_attrs(struct index_state *istate,\n+void git_all_attrs(struct repository *repo, struct index_state *istate,\n \t\t   const char *path, struct attr_check *check);\n \n enum git_attr_direction {\n@@ -232,9 +234,6 @@ void attr_start(void);\n /* Return the system gitattributes file. */\n const char *git_attr_system_file(void);\n \n-/* Return the global gitattributes file, if any. */\n-const char *git_attr_global_file(void);\n-\n /* Return whether the system gitattributes file is enabled and should be used. */\n int git_attr_system_is_enabled(void);\n \ndiff --git a/builtin/check-attr.c b/builtin/check-attr.c\nindex 7cf275b893..1b8a89dfb2 100644\n--- a/builtin/check-attr.c\n+++ b/builtin/check-attr.c\n@@ -70,7 +70,7 @@ static void check_attr(const char *prefix, struct attr_check *check,\n \t\tprefix_path(prefix, prefix ? strlen(prefix) : 0, file);\n \n \tif (collect_all) {\n-\t\tgit_all_attrs(the_repository->index, full_path, check);\n+\t\tgit_all_attrs(the_repository, the_repository->index, full_path, check);\n \t} else {\n \t\tgit_check_attr(the_repository->index, full_path, check);\n \t}\ndiff --git a/builtin/var.c b/builtin/var.c\nindex ada642a9fe..8fbf5430a4 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -71,7 +71,7 @@ static char *git_attr_val_system(int ident_flag UNUSED)\n \n static char *git_attr_val_global(int ident_flag UNUSED)\n {\n-\tchar *file = xstrdup_or_null(git_attr_global_file());\n+\tchar *file = xstrdup_or_null(repo_settings_get_attributesfile_path(the_repository));\n \tif (file) {\n \t\tnormalize_path_copy(file, file);\n \t\treturn file;\n-- \n2.48.GIT\n\n"},{"id":"513919","messageId":"xmqqwmcw97z2.fsf@gitster.g","threadId":"63081","inReplyTo":"Z86PUkJ1sbSH2VTU@pks.im","subject":"Re: [PATCH] environment: move access to \"core.attributesfile\" into repo settings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-10T16:16:33Z","receivedAt":"2025-03-10T16:16:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Sun, Mar 09, 2025 at 09:03:21PM +0530, Ayush Chandekar wrote:\n>> Stop relying on global state to access the \"core.attributesfile\"\n>> configuration. Instead, store the value in `struct repo_settings` and\n>> retrieve it via `repo_settings_get_attributes_file_path()`.\n>> \n>> This prevents incorrect values from being used when a user or tool is\n>> handling multiple repositories in the same process, each with different\n>> attribute configurations. It also improves repository isolation and helps\n>> progress towards libification by avoiding unnecessary global state.\n>\n> We typically switch the order around a bit in our commit messages: we\n> first explain what the actual problem is, and then we say how we fix it.\n\nA good suggestion.\n\n>>  int git_attr_system_is_enabled(void)\n>\n> Hm. I wonder what the actual merit of this function is after the\n> refactoring. Right now there isn't really any as it is a direct wrapper\n> of `repo_settings_get_attributesfile_path()`.\n\nAnother thing that felt awkward about this change is actually\nlarger.  The current attributes globals are built around the notion\nthat the functions involved work on a single set of attributes at a\ntime.  Even in a single repository, when you are checking new contents\ninto the object database and when you are checking objects out of\nthe object database, you'd need to switch the direction manually,\nwhich means you always have two sets of attributes active that you\ncan switch between (one is from the working tree and the other one\nis from the index, if I am recalling correctly).\n\nBut step back and think.  What does it mean to make them belong to a\nrepository instance?  Whose index and working tree does the attribute\nset that belongs to a repository that is not the_repository come from?\n\nSo it seems to me that removing the reliance on globals is a good\nthing, and introducing some abstraction is a good step forward, but\nit is dubious if the abstracted \"set of attributes\" should be part\nof a repository instance.  This is a bit similar to the_index\nsituation, where we can have more than one in-core index instance\nfor a given repository we are working with, an index_state knows its\nrepository, but a repository instance only has a pointer to its\nprimary index_state instance and does not know other index_state\nobjects.  The right way to deal with in-core index is to pass\nindex_state, not repository, through the call chain for this reason,\nand I suspect that we are better off to make sure that we do the\nsame for the attributes, i.e. pass pointer to an attribute set\ninstance through the callchain, not a repository.\n\n\n\n"},{"id":"513925","messageId":"CAE7as+aSRuo9sFxSX8M66HB3EOH+_OwugAnAJfN800_6GiDqBQ@mail.gmail.com","threadId":"63081","inReplyTo":"xmqqwmcw97z2.fsf@gitster.g","subject":"Re: [PATCH] environment: move access to \"core.attributesfile\" into repo settings","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-03-10T17:21:11Z","receivedAt":"2025-03-10T17:21:22Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"> Another thing that felt awkward about this change is actually\n> larger.  The current attributes globals are built around the notion\n> that the functions involved work on a single set of attributes at a\n> time.  Even in a single repository, when you are checking new contents\n> into the object database and when you are checking objects out of\n> the object database, you'd need to switch the direction manually,\n> which means you always have two sets of attributes active that you\n> can switch between (one is from the working tree and the other one\n> is from the index, if I am recalling correctly).\n>\n> But step back and think.  What does it mean to make them belong to a\n> repository instance?  Whose index and working tree does the attribute\n> set that belongs to a repository that is not the_repository come from?\n\nI'm trying my best to wrap my head around this. I definitely don't fully\nunderstand your review yet since I'm still quite new to the codebase.\nI get what you mean when you said which repo does it belong to.\nBut in the long term, isn’t our goal to get rid of the_repository anyway?\nSo at some point, wouldn't we need to either attach attributes to a\nrepository or have the attribute set know about its repository?\n\nThanks,\nAyush:)\n"},{"id":"513931","messageId":"xmqqcyeo7kno.fsf@gitster.g","threadId":"63081","inReplyTo":"CAE7as+aSRuo9sFxSX8M66HB3EOH+_OwugAnAJfN800_6GiDqBQ@mail.gmail.com","subject":"Re: [PATCH] environment: move access to \"core.attributesfile\" into repo settings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-10T19:25:31Z","receivedAt":"2025-03-10T19:25:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ayush Chandekar <ayu.chandekar@gmail.com> writes:\n\n> But in the long term, isn’t our goal to get rid of the_repository anyway?\n> So at some point, wouldn't we need to either attach attributes to a\n> repository or have the attribute set know about its repository?\n\nMy point is that it may not help further the cause of removing the\nassumption that certain operations only work on the_repository and\nnot on an arbitrary \"struct repository\" instance, to muck with the\nattribute subsystem.  If it turns out that attribute data should not\nbelong to a repository instance, then it would not help to have the\nglobals moved to members of \"struct repository\" and pass a\nrepository instance down the code paths.  Rather, it may turn out\nthat we are better off passing a separate structure that is *NOT* a\n\"struct repository\" that represents the set(s) of attributes down\nthe same code paths.\n\n"},{"id":"513944","messageId":"CAOLa=ZSqgzLv=X9=7kFFeA+w_PsPwYz6iJyeqW=i4yrrszURBg@mail.gmail.com","threadId":"63081","inReplyTo":"20250310151048.69825-2-ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH v2 1/2] environment: move access to \"core.attributesfile\" into repo settings","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-10T21:11:06Z","receivedAt":"2025-03-10T21:11:08Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Ayush Chandekar <ayu.chandekar@gmail.com> writes:\n\n[snip]\n\n> diff --git a/repo-settings.h b/repo-settings.h\n> index ddc11967e0..58dadd9dae 100644\n> --- a/repo-settings.h\n> +++ b/repo-settings.h\n> @@ -66,6 +66,7 @@ struct repo_settings {\n>  \tsize_t packed_git_limit;\n>\n>  \tchar *hooks_path;\n> +\tchar *git_attributes_file;\n>  };\n>  #define REPO_SETTINGS_INIT { \\\n>  \t.shared_repository = -1, \\\n> @@ -92,5 +93,7 @@ const char *repo_settings_get_hooks_path(struct repository *repo);\n>  int repo_settings_get_shared_repository(struct repository *repo);\n>  void repo_settings_set_shared_repository(struct repository *repo, int value);\n>  void repo_settings_reset_shared_repository(struct repository *repo);\n> +/* Read the value for \"core.attributesfile\". */\n\nNit: Shouldn't we also mention that we default to\n`xdg_config_home(\"attributes\")` if the 'core.attributesfile' value isn't\navailable?\n\n> +const char *repo_settings_get_attributesfile_path(struct repository *repo);\n>\n>  #endif /* REPO_SETTINGS_H */\n> --\n> 2.48.GIT\n"},{"id":"513945","messageId":"CAOLa=ZT=zGTF2DLEy9VjXhcUN3wEi7_R=8O6nV-TtBXKT=ENXg@mail.gmail.com","threadId":"63081","inReplyTo":"20250310151048.69825-3-ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH v2 2/2] attr: use `repo_settings_get_attributesfile_path()` and update callers","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-10T21:17:06Z","receivedAt":"2025-03-10T21:17:08Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Ayush Chandekar <ayu.chandekar@gmail.com> writes:\n\n> Update attribute-related functions to retrieve the \"core.attributesfile\"\n> configuration via the new repository-scoped accessor\n> `repo_settings_get_attributesfile_path()`. This improves behaviour in\n> multi-repository contexts and aligns with the goal of minimizing\n> reliance on global state.\n>\n\nWe should also talk about the modifications made to pass around the\nrepository struct.\n\n> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n> ---\n>  attr.c               | 28 ++++++++++------------------\n>  attr.h               |  7 +++----\n>  builtin/check-attr.c |  2 +-\n>  builtin/var.c        |  2 +-\n>  4 files changed, 15 insertions(+), 24 deletions(-)\n>\n> diff --git a/attr.c b/attr.c\n> index 0bd2750528..8f28463e8c 100644\n> --- a/attr.c\n> +++ b/attr.c\n> @@ -879,14 +879,6 @@ const char *git_attr_system_file(void)\n>  \treturn system_wide;\n>  }\n>\n> -const char *git_attr_global_file(void)\n> -{\n> -\tif (!git_attributes_file)\n> -\t\tgit_attributes_file = xdg_config_home(\"attributes\");\n> -\n> -\treturn git_attributes_file;\n> -}\n> -\n>  int git_attr_system_is_enabled(void)\n>  {\n>  \treturn !git_env_bool(\"GIT_ATTR_NOSYSTEM\", 0);\n> @@ -906,7 +898,7 @@ static void push_stack(struct attr_stack **attr_stack_p,\n>  \t}\n>  }\n>\n> -static void bootstrap_attr_stack(struct index_state *istate,\n> +static void bootstrap_attr_stack(struct repository *repo, struct index_state *istate,\n\nNit: here and other places, can this be a 'const'?\n\n>  \t\t\t\t const struct object_id *tree_oid,\n>  \t\t\t\t struct attr_stack **stack)\n>  {\n> @@ -927,8 +919,8 @@ static void bootstrap_attr_stack(struct index_state *istate,\n>  \t}\n>\n>  \t/* home directory */\n> -\tif (git_attr_global_file()) {\n> -\t\te = read_attr_from_file(git_attr_global_file(), flags);\n> +\tif (repo_settings_get_attributesfile_path(repo)) {\n> +\t\te = read_attr_from_file(repo_settings_get_attributesfile_path(repo), flags);\n>  \t\tpush_stack(stack, e, NULL, 0);\n>  \t}\n>\n> @@ -946,7 +938,7 @@ static void bootstrap_attr_stack(struct index_state *istate,\n>  \tpush_stack(stack, e, NULL, 0);\n>  }\n>\n> -static void prepare_attr_stack(struct index_state *istate,\n> +static void prepare_attr_stack(struct repository *repo, struct index_state *istate,\n>  \t\t\t       const struct object_id *tree_oid,\n>  \t\t\t       const char *path, int dirlen,\n>  \t\t\t       struct attr_stack **stack)\n> @@ -969,7 +961,7 @@ static void prepare_attr_stack(struct index_state *istate,\n>  \t * .gitattributes in deeper directories to shallower ones,\n>  \t * and finally use the built-in set as the default.\n>  \t */\n> -\tbootstrap_attr_stack(istate, tree_oid, stack);\n> +\tbootstrap_attr_stack(repo, istate, tree_oid, stack);\n>\n>  \t/*\n>  \t * Pop the \"info\" one that is always at the top of the stack.\n> @@ -1143,7 +1135,7 @@ static void determine_macros(struct all_attrs_item *all_attrs,\n>   * If check->check_nr is non-zero, only attributes in check[] are collected.\n>   * Otherwise all attributes are collected.\n>   */\n> -static void collect_some_attrs(struct index_state *istate,\n> +static void collect_some_attrs(struct repository *repo, struct index_state *istate,\n>  \t\t\t       const struct object_id *tree_oid,\n>  \t\t\t       const char *path, struct attr_check *check)\n>  {\n> @@ -1164,7 +1156,7 @@ static void collect_some_attrs(struct index_state *istate,\n>  \t\tdirlen = 0;\n>  \t}\n>\n> -\tprepare_attr_stack(istate, tree_oid, path, dirlen, &check->stack);\n> +\tprepare_attr_stack(repo, istate, tree_oid, path, dirlen, &check->stack);\n>  \tall_attrs_init(&g_attr_hashmap, check);\n>  \tdetermine_macros(check->all_attrs, check->stack);\n>\n> @@ -1310,7 +1302,7 @@ void git_check_attr(struct index_state *istate,\n>  \tint i;\n>  \tconst struct object_id *tree_oid = default_attr_source();\n>\n> -\tcollect_some_attrs(istate, tree_oid, path, check);\n> +\tcollect_some_attrs(the_repository, istate, tree_oid, path, check);\n>\n\nThe other places in the same file we pass around the 'repository'\nstruct, but here we use 'the_repository'. Even below, we modify an\nexternal function's signature to avail the 'repository' struct.\n\nCan't we modify 'git_check_attr()' to also receive a 'repository'? If\nnot, perhaps it would be much simpler to simply pass 'the_repository'\neverywhere and cleanup this file in another follow up series?\n\n>  \tfor (i = 0; i < check->nr; i++) {\n>  \t\tunsigned int n = check->items[i].attr->attr_nr;\n> @@ -1321,14 +1313,14 @@ void git_check_attr(struct index_state *istate,\n>  \t}\n>  }\n>\n> -void git_all_attrs(struct index_state *istate,\n> +void git_all_attrs(struct repository *repo, struct index_state *istate,\n>  \t\t   const char *path, struct attr_check *check)\n>  {\n>  \tint i;\n>  \tconst struct object_id *tree_oid = default_attr_source();\n>\n>  \tattr_check_reset(check);\n> -\tcollect_some_attrs(istate, tree_oid, path, check);\n> +\tcollect_some_attrs(repo, istate, tree_oid, path, check);\n>\n>  \tfor (i = 0; i < check->all_attrs_nr; i++) {\n>  \t\tconst char *name = check->all_attrs[i].attr->name;\n> diff --git a/attr.h b/attr.h\n> index a04a521092..1ff058bef7 100644\n> --- a/attr.h\n> +++ b/attr.h\n> @@ -213,11 +213,13 @@ void git_check_attr(struct index_state *istate,\n>  \t\t    const char *path,\n>  \t\t    struct attr_check *check);\n>\n> +struct repository;\n> +\n>  /*\n>   * Retrieve all attributes that apply to the specified path.\n>   * check holds the attributes and their values.\n>   */\n> -void git_all_attrs(struct index_state *istate,\n> +void git_all_attrs(struct repository *repo, struct index_state *istate,\n>  \t\t   const char *path, struct attr_check *check);\n>\n>  enum git_attr_direction {\n> @@ -232,9 +234,6 @@ void attr_start(void);\n>  /* Return the system gitattributes file. */\n>  const char *git_attr_system_file(void);\n>\n> -/* Return the global gitattributes file, if any. */\n> -const char *git_attr_global_file(void);\n> -\n>  /* Return whether the system gitattributes file is enabled and should be used. */\n>  int git_attr_system_is_enabled(void);\n>\n> diff --git a/builtin/check-attr.c b/builtin/check-attr.c\n> index 7cf275b893..1b8a89dfb2 100644\n> --- a/builtin/check-attr.c\n> +++ b/builtin/check-attr.c\n> @@ -70,7 +70,7 @@ static void check_attr(const char *prefix, struct attr_check *check,\n>  \t\tprefix_path(prefix, prefix ? strlen(prefix) : 0, file);\n>\n>  \tif (collect_all) {\n> -\t\tgit_all_attrs(the_repository->index, full_path, check);\n> +\t\tgit_all_attrs(the_repository, the_repository->index, full_path, check);\n>  \t} else {\n>  \t\tgit_check_attr(the_repository->index, full_path, check);\n>  \t}\n> diff --git a/builtin/var.c b/builtin/var.c\n> index ada642a9fe..8fbf5430a4 100644\n> --- a/builtin/var.c\n> +++ b/builtin/var.c\n> @@ -71,7 +71,7 @@ static char *git_attr_val_system(int ident_flag UNUSED)\n>\n>  static char *git_attr_val_global(int ident_flag UNUSED)\n>  {\n> -\tchar *file = xstrdup_or_null(git_attr_global_file());\n> +\tchar *file = xstrdup_or_null(repo_settings_get_attributesfile_path(the_repository));\n>  \tif (file) {\n>  \t\tnormalize_path_copy(file, file);\n>  \t\treturn file;\n> --\n> 2.48.GIT\n"},{"id":"513954","messageId":"xmqqy0xc4h7b.fsf@gitster.g","threadId":"63081","inReplyTo":"CAOLa=ZT=zGTF2DLEy9VjXhcUN3wEi7_R=8O6nV-TtBXKT=ENXg@mail.gmail.com","subject":"Re: [GSOC PATCH v2 2/2] attr: use `repo_settings_get_attributesfile_path()` and update callers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-10T23:08:24Z","receivedAt":"2025-03-10T23:08:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n> Ayush Chandekar <ayu.chandekar@gmail.com> writes:\n>\n>> Update attribute-related functions to retrieve the \"core.attributesfile\"\n>> configuration via the new repository-scoped accessor\n>> `repo_settings_get_attributesfile_path()`. This improves behaviour in\n>> multi-repository contexts and aligns with the goal of minimizing\n>> reliance on global state.\n>>\n>\n> We should also talk about the modifications made to pass around the\n> repository struct.\n\nYes.  We first should justify if it makes sense to cram attribute\nset into the repository object and pass it around in the first\nplace.  Many index-state related functions do not pass repository\naround because they work on index-state, so index-state is passed\naround instead.  Perhaps the attribute subsystem should be the same\nway, in that their globals should belong to its own abstraction that\nis smaller scale than a repository object (it is permissible to have\nsuch an attribute-set object know about which repository instance it\nis related to, though).\n"},{"id":"513979","messageId":"Z9BLMLXJ7Desl-n6@ArchLinux","threadId":"63081","inReplyTo":"20250310151048.69825-3-ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH v2 2/2] attr: use `repo_settings_get_attributesfile_path()` and update callers","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2025-03-11T14:39:44Z","receivedAt":"2025-03-11T14:39:35Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Mon, Mar 10, 2025 at 08:40:48PM +0530, Ayush Chandekar wrote:\n> Update attribute-related functions to retrieve the \"core.attributesfile\"\n> configuration via the new repository-scoped accessor\n> `repo_settings_get_attributesfile_path()`. This improves behaviour in\n> multi-repository contexts and aligns with the goal of minimizing\n> reliance on global state.\n> \n> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n> ---\n>  attr.c               | 28 ++++++++++------------------\n>  attr.h               |  7 +++----\n>  builtin/check-attr.c |  2 +-\n>  builtin/var.c        |  2 +-\n>  4 files changed, 15 insertions(+), 24 deletions(-)\n> \n> diff --git a/attr.c b/attr.c\n> index 0bd2750528..8f28463e8c 100644\n> --- a/attr.c\n> +++ b/attr.c\n> @@ -879,14 +879,6 @@ const char *git_attr_system_file(void)\n>  \treturn system_wide;\n>  }\n>  \n> -const char *git_attr_global_file(void)\n> -{\n> -\tif (!git_attributes_file)\n> -\t\tgit_attributes_file = xdg_config_home(\"attributes\");\n> -\n> -\treturn git_attributes_file;\n> -}\n> -\n>  int git_attr_system_is_enabled(void)\n>  {\n>  \treturn !git_env_bool(\"GIT_ATTR_NOSYSTEM\", 0);\n> @@ -906,7 +898,7 @@ static void push_stack(struct attr_stack **attr_stack_p,\n>  \t}\n>  }\n>  \n> -static void bootstrap_attr_stack(struct index_state *istate,\n> +static void bootstrap_attr_stack(struct repository *repo, struct index_state *istate,\n\nI have scanned the definition of the \"struct index_state\", there is a\n\"struct repository *repo\" member in this data structure. This makes me\nthink why do we need to pass the \"struct repository *repo\" in the first\nplace. A design question, should we just use `istate->repo` directly?\n\n>  \t\t\t\t const struct object_id *tree_oid,\n>  \t\t\t\t struct attr_stack **stack)\n>  {\n> @@ -927,8 +919,8 @@ static void bootstrap_attr_stack(struct index_state *istate,\n>  \t}\n>  \n>  \t/* home directory */\n> -\tif (git_attr_global_file()) {\n> -\t\te = read_attr_from_file(git_attr_global_file(), flags);\n> +\tif (repo_settings_get_attributesfile_path(repo)) {\n> +\t\te = read_attr_from_file(repo_settings_get_attributesfile_path(repo), flags);\n>  \t\tpush_stack(stack, e, NULL, 0);\n>  \t}\n>  \n> @@ -946,7 +938,7 @@ static void bootstrap_attr_stack(struct index_state *istate,\n>  \tpush_stack(stack, e, NULL, 0);\n>  }\n>  \n> -static void prepare_attr_stack(struct index_state *istate,\n> +static void prepare_attr_stack(struct repository *repo, struct index_state *istate,\n>  \t\t\t       const struct object_id *tree_oid,\n>  \t\t\t       const char *path, int dirlen,\n>  \t\t\t       struct attr_stack **stack)\n\nIf we use \"istate->repo\", we don't even need to change this function.\n\n> @@ -969,7 +961,7 @@ static void prepare_attr_stack(struct index_state *istate,\n>  \t * .gitattributes in deeper directories to shallower ones,\n>  \t * and finally use the built-in set as the default.\n>  \t */\n> -\tbootstrap_attr_stack(istate, tree_oid, stack);\n> +\tbootstrap_attr_stack(repo, istate, tree_oid, stack);\n>  \n>  \t/*\n>  \t * Pop the \"info\" one that is always at the top of the stack.\n\n[snip]\n\nThanks,\nJialuo\n"},{"id":"513988","messageId":"xmqqcyen4i09.fsf@gitster.g","threadId":"63081","inReplyTo":"Z9BLMLXJ7Desl-n6@ArchLinux","subject":"Re: [GSOC PATCH v2 2/2] attr: use `repo_settings_get_attributesfile_path()` and update callers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-11T17:03:18Z","receivedAt":"2025-03-11T17:03:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"shejialuo <shejialuo@gmail.com> writes:\n\n>> -static void bootstrap_attr_stack(struct index_state *istate,\n>> +static void bootstrap_attr_stack(struct repository *repo, struct index_state *istate,\n>\n> I have scanned the definition of the \"struct index_state\", there is a\n> \"struct repository *repo\" member in this data structure. This makes me\n> think why do we need to pass the \"struct repository *repo\" in the first\n> place. A design question, should we just use `istate->repo` directly?\n\nGood thing to notice.\n\nAs the attribute system is all about giving extra information on the\npaths that appear in the index and in the working tree, it may make\nsense for the API to go from the index state which is about the\nindex and the working tree to access the attributes, rather than\nfrom the repository structure, which controls a lot wider concept\nand moving anything and everything there will easily and quickly\nmake it a messy kitchen sink.\n\n"},{"id":"513992","messageId":"CAE7as+ZROO1GiEhXYga5Nqmrs5Xr=k9zsAiP2y0xzuny1ws+UQ@mail.gmail.com","threadId":"63081","inReplyTo":"Z9BLMLXJ7Desl-n6@ArchLinux","subject":"Re: [GSOC PATCH v2 2/2] attr: use `repo_settings_get_attributesfile_path()` and update callers","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-03-11T17:20:50Z","receivedAt":"2025-03-11T17:21:02Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"> If we use \"istate->repo\", we don't even need to change this function.\n\nOh you're absolutely right about that. I actually just got to learn more\nabout `index_state` from another thread where Junio brought it up.\n\nBut now with his suggestion that attributes may not belong in the\nrepository struct at all, I'm a bit unsure how to move the patch forward.\n\nOne thing I did take away from this is that we shouldn't be cramming\nenvironment variables into the repository struct. This discussion has\ndefinitely helped me think more clearly about the design, and I think\nit'll guide me take better decisions going forward.\n\nI’d really appreciate your thoughts on how you think we should\napproach this from here.\n\nThanks,\nAyush:)\n"},{"id":"513994","messageId":"CAE7as+YBnOd3jTYVzmHNjei0gjhMwsV3XGk1Y7Vi45CvzJTo4A@mail.gmail.com","threadId":"63081","inReplyTo":"CAOLa=ZT=zGTF2DLEy9VjXhcUN3wEi7_R=8O6nV-TtBXKT=ENXg@mail.gmail.com","subject":"Re: [GSOC PATCH v2 2/2] attr: use `repo_settings_get_attributesfile_path()` and update callers","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-03-11T17:41:43Z","receivedAt":"2025-03-11T17:41:54Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"> Can't we modify 'git_check_attr()' to also receive a 'repository'? If\n> not, perhaps it would be much simpler to simply pass 'the_repository'\n> everywhere and cleanup this file in another follow up series?\n>\n\nRight, that was one of the things I considered too. But since `git_check_attr()`\nis used in a lot of widely used code paths that don't currently pass a\nstruct repository, it felt like threading repo through all of them would\ncreate a much larger change that I intended for this patch series.\n\nThat's why I decided to stick with `the_repository` for now, and perhaps revisit\nthe cleanup in a follow-up series once the proposed changes are\naccepted by the community.\n"},{"id":"531856","messageId":"20251208201132.40186-1-belkid98@gmail.com","threadId":"63081","inReplyTo":"CAE7as+ZROO1GiEhXYga5Nqmrs5Xr=k9zsAiP2y0xzuny1ws+UQ@mail.gmail.com","subject":"Outreachy intern: Request for the completion of this series","fromName":"Olamide Caleb Bello","fromEmail":"belkid98@gmail.com","sentAt":"2025-12-08T20:11:32Z","receivedAt":"2025-12-08T20:11:39Z","isPatch":false,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"\nHello Ayush,\nMy name is Bello and I am an intern for the ongoing round of the Outreachy\nprogram for the project \"Refactor in order to reduce Git's global state\".\n\nI would like to commend and appreciate you on your previous works done with\nregards to the project. They provided enough guide for me in my bid to continue\nwhere you stopped.\nI referenced this patch in my proposal as a part of the\npatches I would like to complete to kick start my internship.\nPlease let me know if you will be okay with me completing this patch and\nsubmitting for review.\n\nThanks\nBello.\n\n"},{"id":"531897","messageId":"CAD=f0L8JMUxKBDFj+=v+HvEQ-TFM2ZxkZbkGSfhx4=3-PD=nCg@mail.gmail.com","threadId":"63081","inReplyTo":"CAE7as+Zka6J+du+V5zsHzc4eiH+Mzx=dQUnjuJz_Mhnk2RzOPA@mail.gmail.com","subject":"Re: Outreachy intern: Request for the completion of this series","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2025-12-09T09:23:08Z","receivedAt":"2025-12-09T09:23:07Z","isPatch":false,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Tue, 9 Dec 2025 at 01:30, Ayush Chandekar <ayu.chandekar@gmail.com> wrote:\n>\n>\n>\n> On Tue, Dec 9, 2025 at 1:41 AM Olamide Caleb Bello <belkid98@gmail.com> wrote:\n> >\n> >\n> > Hello Ayush,\n> > My name is Bello and I am an intern for the ongoing round of the Outreachy\n> > program for the project \"Refactor in order to reduce Git's global state\".\n> >\n> > I would like to commend and appreciate you on your previous works done with\n> > regards to the project. They provided enough guide for me in my bid to continue\n> > where you stopped.\n> > I referenced this patch in my proposal as a part of the\n> > patches I would like to complete to kick start my internship.\n> > Please let me know if you will be okay with me completing this patch and\n> > submitting for review.\n> >\n> > Thanks\n> > Bello.\n>\n> Hey Bello,\n>\n> Thanks a lot for reaching out, and congratulations on your Outreachy internship!\n>\n> You are absolutely welcome to complete the patch. I'm glad my previous work was helpful, and I wish you all the best!\n>\n> Ayush:)\n\nThank you\n\nBello.\n"}]}