{"thread":{"id":"64647","subject":"[Outreachy PATCH] environment: move \"core.attributesFile\" into repo-setting","startedAt":"2025-12-18T08:30:09Z","lastAt":"2026-01-07T15:33:16Z","messageCount":21,"participants":["Olamide Caleb Bello","Bello Olamide","Karthik Nayak","Phillip Wood","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"532446","messageId":"aUO7jQQAERTe5xYc@ubuntu","threadId":"64647","inReplyTo":null,"subject":"[Outreachy PATCH] environment: move \"core.attributesFile\" into repo-setting","fromName":"Olamide Caleb Bello","fromEmail":"belkid98@gmail.com","sentAt":"2025-12-18T08:30:05Z","receivedAt":"2025-12-18T08:30:09Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?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.\nThe functions `bootstrap_attr_stack()` and `git_attr_val_system()`\nretrieve \"core.attributesFile\" via `git_attr_global_file()`\nwhich reads from global state `git_attributes_file`.\n\nMove the \"core.attributesFile\" configuration into the\n`struct repo_settings` instead of relying on the global state.\nA new function `repo_settings_get_attributesfile_path()` is added\nand used to retrieve this setting in a repository-scoped manner.\nThe functions to retrieve \"core.attributesFile\" are replaced with\nthe new accessor function `repo_settings_get_attributesfile_path()`\nThis improves multi-repository behaviour and aligns with the goal of\nlibifying of Git.\n\nNote that in `bootstrap_attr_stack()`, the `index_state` is used only\nif it exists, else we default to `the_repository`.\n\nBased-on-patch-by: Ayush Chandekar <ayu.chandekar@gmail.com>\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\nSigned-off-by: Olamide Caleb Bello <belkid98@gmail.com>\n---\nThe link to the GitHub CI is provided below\nhttps://github.com/cloobTech/git/actions/runs/20284228144\n\n attr.c          | 20 +++++++++-----------\n attr.h          |  3 ---\n builtin/var.c   |  2 +-\n environment.c   |  6 ------\n environment.h   |  1 -\n repo-settings.c | 10 ++++++++++\n repo-settings.h |  8 ++++++++\n 7 files changed, 28 insertions(+), 22 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 4999b7e09d..9e51f8e70b 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@@ -912,6 +904,8 @@ static void bootstrap_attr_stack(struct index_state *istate,\n {\n \tstruct attr_stack *e;\n \tunsigned flags = READ_ATTR_MACRO_OK;\n+\tconst char *attributes_file_path;\n+\tstruct repository *repo;\n \n \tif (*stack)\n \t\treturn;\n@@ -926,9 +920,13 @@ static void bootstrap_attr_stack(struct index_state *istate,\n \t\tpush_stack(stack, e, NULL, 0);\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 (istate && istate->repo)\n+\t\trepo = istate->repo;\n+\telse\n+\t\trepo = the_repository;\n+\tattributes_file_path = repo_settings_get_attributesfile_path(repo);\n+\tif (attributes_file_path) {\n+\t\te = read_attr_from_file(attributes_file_path, flags);\n \t\tpush_stack(stack, e, NULL, 0);\n \t}\n \ndiff --git a/attr.h b/attr.h\nindex a04a521092..956ce6ba62 100644\n--- a/attr.h\n+++ b/attr.h\n@@ -232,9 +232,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/var.c b/builtin/var.c\nindex cc3a43cde2..fd577f2930 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -72,7 +72,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;\ndiff --git a/environment.c b/environment.c\nindex a770b5921d..ed7d8f42d9 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -53,7 +53,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;\n@@ -363,11 +362,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.h b/environment.h\nindex 51898c99cd..3512a7072e 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -152,7 +152,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 unsigned long pack_size_limit_cfg;\ndiff --git a/repo-settings.c b/repo-settings.c\nindex 195c24e9c0..396cf79f20 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@@ -158,6 +159,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@@ -230,3 +232,11 @@ void repo_settings_reset_shared_repository(struct repository *repo)\n {\n \trepo->settings.shared_repository_initialized = 0;\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 d477885561..362f355267 100644\n--- a/repo-settings.h\n+++ b/repo-settings.h\n@@ -68,6 +68,7 @@ struct repo_settings {\n \tunsigned long big_file_threshold;\n \n \tchar *hooks_path;\n+\tchar *git_attributes_file;\n };\n #define REPO_SETTINGS_INIT { \\\n \t.shared_repository = -1, \\\n@@ -99,4 +100,11 @@ 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 \n+/*\n+ * Read the value for \"core.attributesfile\".\n+ * Defaults to xdg_config_home(\"attributes\") if the core.attributesfile\n+ * isn't available.\n+ */\n+const char *repo_settings_get_attributesfile_path(struct repository *repo);\n+\n #endif /* REPO_SETTINGS_H */\n-- \n2.34.1\n\n"},{"id":"532491","messageId":"CAD=f0L8hCyUK6OJXrz7=KNVTJ_cjkX3pt6N-ZiH+58PHR21=3g@mail.gmail.com","threadId":"64647","inReplyTo":"aUO7jQQAERTe5xYc@ubuntu","subject":"Re: [Outreachy PATCH] environment: move \"core.attributesFile\" into repo-setting","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2025-12-18T17:31:25Z","receivedAt":"2025-12-18T17:31:24Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Thu, 18 Dec 2025 at 09:30, Olamide Caleb Bello <belkid98@gmail.com> wrote:\n>\n> When handling multiple repositories within the same process, relying on\n> global state for accessing the \"core.attributesFile\" configuration can\n> lead to incorrect values being used. It also makes it harder to isolate\n> repositories and hinders the libification of git.\n> The functions `bootstrap_attr_stack()` and `git_attr_val_system()`\n> retrieve \"core.attributesFile\" via `git_attr_global_file()`\n> which reads from global state `git_attributes_file`.\n>\n> Move the \"core.attributesFile\" configuration into the\n> `struct repo_settings` instead of relying on the global state.\n> A new function `repo_settings_get_attributesfile_path()` is added\n> and used to retrieve this setting in a repository-scoped manner.\n> The functions to retrieve \"core.attributesFile\" are replaced with\n> the new accessor function `repo_settings_get_attributesfile_path()`\n> This improves multi-repository behaviour and aligns with the goal of\n> libifying of Git.\n>\n> Note that in `bootstrap_attr_stack()`, the `index_state` is used only\n> if it exists, else we default to `the_repository`.\n>\n> Based-on-patch-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n> Mentored-by: Christian Couder <christian.couder@gmail.com>\n> Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>\n> ---\n> The link to the GitHub CI is provided below\n> https://github.com/cloobTech/git/actions/runs/20284228144\n\nThe link to Ayush's patches, which this patch is based on, is provided in\n[1].\nThe 'git_attributes_file' member of `struct repository` is now\naccessed via the `struct index_state`,\nistate->repo, as most of the callers in the attributes subsystem\nalready use the `index_state`,\nrather than through the `struct repository *repo` as done in [1] which\nonly knows its primary index.\nThis ensures that the index actually owns the attributes as pointed\nout by Junio in the threads.\n\n[1]. https://lore.kernel.org/git/20250309153321.254844-1-ayu.chandekar@gmail.com/\n\n>\n>  attr.c          | 20 +++++++++-----------\n>  attr.h          |  3 ---\n>  builtin/var.c   |  2 +-\n>  environment.c   |  6 ------\n>  environment.h   |  1 -\n>  repo-settings.c | 10 ++++++++++\n>  repo-settings.h |  8 ++++++++\n>  7 files changed, 28 insertions(+), 22 deletions(-)\n>\n> diff --git a/attr.c b/attr.c\n> index 4999b7e09d..9e51f8e70b 100644\n> --- a/attr.c\n> +++ b/attr.c\n> @@ -879,14 +879,6 @@ const char *git_attr_system_file(void)\n>         return system_wide;\n>  }\n>\n> -const char *git_attr_global_file(void)\n> -{\n> -       if (!git_attributes_file)\n> -               git_attributes_file = xdg_config_home(\"attributes\");\n> -\n> -       return git_attributes_file;\n> -}\n> -\n>  int git_attr_system_is_enabled(void)\n>  {\n>         return !git_env_bool(\"GIT_ATTR_NOSYSTEM\", 0);\n> @@ -912,6 +904,8 @@ static void bootstrap_attr_stack(struct index_state *istate,\n>  {\n>         struct attr_stack *e;\n>         unsigned flags = READ_ATTR_MACRO_OK;\n> +       const char *attributes_file_path;\n> +       struct repository *repo;\n>\n>         if (*stack)\n>                 return;\n> @@ -926,9 +920,13 @@ static void bootstrap_attr_stack(struct index_state *istate,\n>                 push_stack(stack, e, NULL, 0);\n>         }\n>\n> -       /* home directory */\n> -       if (git_attr_global_file()) {\n> -               e = read_attr_from_file(git_attr_global_file(), flags);\n> +       if (istate && istate->repo)\n> +               repo = istate->repo;\n> +       else\n> +               repo = the_repository;\n> +       attributes_file_path = repo_settings_get_attributesfile_path(repo);\n> +       if (attributes_file_path) {\n> +               e = read_attr_from_file(attributes_file_path, flags);\n>                 push_stack(stack, e, NULL, 0);\n>         }\n>\n> diff --git a/attr.h b/attr.h\n> index a04a521092..956ce6ba62 100644\n> --- a/attr.h\n> +++ b/attr.h\n> @@ -232,9 +232,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/var.c b/builtin/var.c\n> index cc3a43cde2..fd577f2930 100644\n> --- a/builtin/var.c\n> +++ b/builtin/var.c\n> @@ -72,7 +72,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> -       char *file = xstrdup_or_null(git_attr_global_file());\n> +       char *file = xstrdup_or_null(repo_settings_get_attributesfile_path(the_repository));\n>         if (file) {\n>                 normalize_path_copy(file, file);\n>                 return file;\n> diff --git a/environment.c b/environment.c\n> index a770b5921d..ed7d8f42d9 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -53,7 +53,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;\n> @@ -363,11 +362,6 @@ static int git_default_core_config(const char *var, const char *value,\n>                 return 0;\n>         }\n>\n> -       if (!strcmp(var, \"core.attributesfile\")) {\n> -               FREE_AND_NULL(git_attributes_file);\n> -               return git_config_pathname(&git_attributes_file, var, value);\n> -       }\n> -\n>         if (!strcmp(var, \"core.bare\")) {\n>                 is_bare_repository_cfg = git_config_bool(var, value);\n>                 return 0;\n> diff --git a/environment.h b/environment.h\n> index 51898c99cd..3512a7072e 100644\n> --- a/environment.h\n> +++ b/environment.h\n> @@ -152,7 +152,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 unsigned long pack_size_limit_cfg;\n> diff --git a/repo-settings.c b/repo-settings.c\n> index 195c24e9c0..396cf79f20 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>                           int def)\n> @@ -158,6 +159,7 @@ void repo_settings_clear(struct repository *r)\n>         struct repo_settings empty = REPO_SETTINGS_INIT;\n>         FREE_AND_NULL(r->settings.fsmonitor);\n>         FREE_AND_NULL(r->settings.hooks_path);\n> +       FREE_AND_NULL(r->settings.git_attributes_file);\n>         r->settings = empty;\n>  }\n>\n> @@ -230,3 +232,11 @@ void repo_settings_reset_shared_repository(struct repository *repo)\n>  {\n>         repo->settings.shared_repository_initialized = 0;\n>  }\n> +const char *repo_settings_get_attributesfile_path(struct repository *repo)\n> +{\n> +       if (!repo->settings.git_attributes_file) {\n> +               if (repo_config_get_pathname(repo, \"core.attributesfile\", &repo->settings.git_attributes_file))\n> +                       repo->settings.git_attributes_file = xdg_config_home(\"attributes\");\n> +       }\n> +       return repo->settings.git_attributes_file;\n> +}\n> diff --git a/repo-settings.h b/repo-settings.h\n> index d477885561..362f355267 100644\n> --- a/repo-settings.h\n> +++ b/repo-settings.h\n> @@ -68,6 +68,7 @@ struct repo_settings {\n>         unsigned long big_file_threshold;\n>\n>         char *hooks_path;\n> +       char *git_attributes_file;\n>  };\n>  #define REPO_SETTINGS_INIT { \\\n>         .shared_repository = -1, \\\n> @@ -99,4 +100,11 @@ 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>\n> +/*\n> + * Read the value for \"core.attributesfile\".\n> + * Defaults to xdg_config_home(\"attributes\") if the core.attributesfile\n> + * isn't available.\n> + */\n> +const char *repo_settings_get_attributesfile_path(struct repository *repo);\n> +\n>  #endif /* REPO_SETTINGS_H */\n> --\n> 2.34.1\n>\n"},{"id":"532906","messageId":"CAD=f0L88QW_tL2iKg8ru3mU7t-vmY=p61S33GN+6tSQBMQAjqw@mail.gmail.com","threadId":"64647","inReplyTo":"aUO7jQQAERTe5xYc@ubuntu","subject":"Re: [Outreachy PATCH] environment: move \"core.attributesFile\" into repo-setting","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2026-01-02T08:01:57Z","receivedAt":"2026-01-02T08:01:57Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Thu, 18 Dec 2025 at 09:30, Olamide Caleb Bello <belkid98@gmail.com> wrote:\n>\n> When handling multiple repositories within the same process, relying on\n> global state for accessing the \"core.attributesFile\" configuration can\n> lead to incorrect values being used. It also makes it harder to isolate\n> repositories and hinders the libification of git.\n> The functions `bootstrap_attr_stack()` and `git_attr_val_system()`\n> retrieve \"core.attributesFile\" via `git_attr_global_file()`\n> which reads from global state `git_attributes_file`.\n>\n> Move the \"core.attributesFile\" configuration into the\n> `struct repo_settings` instead of relying on the global state.\n> A new function `repo_settings_get_attributesfile_path()` is added\n> and used to retrieve this setting in a repository-scoped manner.\n> The functions to retrieve \"core.attributesFile\" are replaced with\n> the new accessor function `repo_settings_get_attributesfile_path()`\n> This improves multi-repository behaviour and aligns with the goal of\n> libifying of Git.\n>\n> Note that in `bootstrap_attr_stack()`, the `index_state` is used only\n> if it exists, else we default to `the_repository`.\n>\n> Based-on-patch-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n> Mentored-by: Christian Couder <christian.couder@gmail.com>\n> Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>\n\nHello.\nPlease I am replying to this as no reviews have been done on this patch.\nThanks\n[...]\n"},{"id":"532907","messageId":"CAOLa=ZRDFdZJWsq5JOckRgfF2V0Whv-jCxbpgeRi80NOs0oTDQ@mail.gmail.com","threadId":"64647","inReplyTo":"aUO7jQQAERTe5xYc@ubuntu","subject":"Re: [Outreachy PATCH] environment: move \"core.attributesFile\" into repo-setting","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-02T08:48:25Z","receivedAt":"2026-01-02T08:48:27Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Olamide Caleb Bello <belkid98@gmail.com> writes:\n\n> When handling multiple repositories within the same process, relying on\n> global state for accessing the \"core.attributesFile\" configuration can\n> lead to incorrect values being used. It also makes it harder to isolate\n> repositories and hinders the libification of git.\n> The functions `bootstrap_attr_stack()` and `git_attr_val_system()`\n> retrieve \"core.attributesFile\" via `git_attr_global_file()`\n> which reads from global state `git_attributes_file`.\n>\n> Move the \"core.attributesFile\" configuration into the\n> `struct repo_settings` instead of relying on the global state.\n> A new function `repo_settings_get_attributesfile_path()` is added\n> and used to retrieve this setting in a repository-scoped manner.\n> The functions to retrieve \"core.attributesFile\" are replaced with\n> the new accessor function `repo_settings_get_attributesfile_path()`\n> This improves multi-repository behaviour and aligns with the goal of\n> libifying of Git.\n>\n> Note that in `bootstrap_attr_stack()`, the `index_state` is used only\n> if it exists, else we default to `the_repository`.\n>\n> Based-on-patch-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n> Mentored-by: Christian Couder <christian.couder@gmail.com>\n> Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>\n> ---\n> The link to the GitHub CI is provided below\n> https://github.com/cloobTech/git/actions/runs/20284228144\n>\n>  attr.c          | 20 +++++++++-----------\n>  attr.h          |  3 ---\n>  builtin/var.c   |  2 +-\n>  environment.c   |  6 ------\n>  environment.h   |  1 -\n>  repo-settings.c | 10 ++++++++++\n>  repo-settings.h |  8 ++++++++\n>  7 files changed, 28 insertions(+), 22 deletions(-)\n\nThe change is very welcome. Apart from some small comments below, the\npatch looks good.\n\n[snip]\n\n> diff --git a/repo-settings.h b/repo-settings.h\n> index d477885561..362f355267 100644\n> --- a/repo-settings.h\n> +++ b/repo-settings.h\n> @@ -68,6 +68,7 @@ struct repo_settings {\n>  \tunsigned long big_file_threshold;\n>\n>  \tchar *hooks_path;\n> +\tchar *git_attributes_file;\n>  };\n>  #define REPO_SETTINGS_INIT { \\\n>  \t.shared_repository = -1, \\\n\nIt would make more sense to rename this variable to\n`attributes_file_path`, that would better denote what is actually stored\nhere and syncs better with `repo_settings_get_attributesfile_path`.\n\n> @@ -99,4 +100,11 @@ 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>\n> +/*\n> + * Read the value for \"core.attributesfile\".\n> + * Defaults to xdg_config_home(\"attributes\") if the core.attributesfile\n> + * isn't available.\n\nWhile it is obvious, it would be nice to point out that\n`core.attributesfile` is set via config.\n\n> + */\n> +const char *repo_settings_get_attributesfile_path(struct repository *repo);\n> +\n>  #endif /* REPO_SETTINGS_H */\n> --\n> 2.34.1\n"},{"id":"532908","messageId":"CAOLa=ZQ8zQKk7Jmu_Pwm-VbCY9u3WP7oC51JBVUSXEi9pk_UfA@mail.gmail.com","threadId":"64647","inReplyTo":"CAD=f0L88QW_tL2iKg8ru3mU7t-vmY=p61S33GN+6tSQBMQAjqw@mail.gmail.com","subject":"Re: [Outreachy PATCH] environment: move \"core.attributesFile\" into repo-setting","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-02T08:49:14Z","receivedAt":"2026-01-02T08:49:16Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Bello Olamide <belkid98@gmail.com> writes:\n\n> On Thu, 18 Dec 2025 at 09:30, Olamide Caleb Bello <belkid98@gmail.com> wrote:\n>>\n>> When handling multiple repositories within the same process, relying on\n>> global state for accessing the \"core.attributesFile\" configuration can\n>> lead to incorrect values being used. It also makes it harder to isolate\n>> repositories and hinders the libification of git.\n>> The functions `bootstrap_attr_stack()` and `git_attr_val_system()`\n>> retrieve \"core.attributesFile\" via `git_attr_global_file()`\n>> which reads from global state `git_attributes_file`.\n>>\n>> Move the \"core.attributesFile\" configuration into the\n>> `struct repo_settings` instead of relying on the global state.\n>> A new function `repo_settings_get_attributesfile_path()` is added\n>> and used to retrieve this setting in a repository-scoped manner.\n>> The functions to retrieve \"core.attributesFile\" are replaced with\n>> the new accessor function `repo_settings_get_attributesfile_path()`\n>> This improves multi-repository behaviour and aligns with the goal of\n>> libifying of Git.\n>>\n>> Note that in `bootstrap_attr_stack()`, the `index_state` is used only\n>> if it exists, else we default to `the_repository`.\n>>\n>> Based-on-patch-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n>> Mentored-by: Christian Couder <christian.couder@gmail.com>\n>> Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n>> Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>\n>\n> Hello.\n> Please I am replying to this as no reviews have been done on this patch.\n> Thanks\n\nThanks for the bump, I think reviews are slowed down due to holidays.\nYou should see a uptick henceforth :)\n"},{"id":"532918","messageId":"CAD=f0L8K+Ou6Kg5gUEqQpNzbSi-FHMsovOKtJN2hzjFYHywiPQ@mail.gmail.com","threadId":"64647","inReplyTo":"CAOLa=ZRDFdZJWsq5JOckRgfF2V0Whv-jCxbpgeRi80NOs0oTDQ@mail.gmail.com","subject":"Re: [Outreachy PATCH] environment: move \"core.attributesFile\" into repo-setting","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2026-01-02T11:26:04Z","receivedAt":"2026-01-02T11:26:03Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Fri, 2 Jan 2026 at 09:48, Karthik Nayak <karthik.188@gmail.com> wrote:\n>\n> Olamide Caleb Bello <belkid98@gmail.com> writes:\n>\n> > When handling multiple repositories within the same process, relying on\n> > global state for accessing the \"core.attributesFile\" configuration can\n> > lead to incorrect values being used. It also makes it harder to isolate\n> > repositories and hinders the libification of git.\n> > The functions `bootstrap_attr_stack()` and `git_attr_val_system()`\n> > retrieve \"core.attributesFile\" via `git_attr_global_file()`\n> > which reads from global state `git_attributes_file`.\n> >\n> > Move the \"core.attributesFile\" configuration into the\n> > `struct repo_settings` instead of relying on the global state.\n> > A new function `repo_settings_get_attributesfile_path()` is added\n> > and used to retrieve this setting in a repository-scoped manner.\n> > The functions to retrieve \"core.attributesFile\" are replaced with\n> > the new accessor function `repo_settings_get_attributesfile_path()`\n> > This improves multi-repository behaviour and aligns with the goal of\n> > libifying of Git.\n> >\n> > Note that in `bootstrap_attr_stack()`, the `index_state` is used only\n> > if it exists, else we default to `the_repository`.\n> >\n> > Based-on-patch-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n> > Mentored-by: Christian Couder <christian.couder@gmail.com>\n> > Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> > Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>\n> > ---\n> > The link to the GitHub CI is provided below\n> > https://github.com/cloobTech/git/actions/runs/20284228144\n> >\n> >  attr.c          | 20 +++++++++-----------\n> >  attr.h          |  3 ---\n> >  builtin/var.c   |  2 +-\n> >  environment.c   |  6 ------\n> >  environment.h   |  1 -\n> >  repo-settings.c | 10 ++++++++++\n> >  repo-settings.h |  8 ++++++++\n> >  7 files changed, 28 insertions(+), 22 deletions(-)\n>\n> The change is very welcome. Apart from some small comments below, the\n> patch looks good.\n>\n> [snip]\n>\n> > diff --git a/repo-settings.h b/repo-settings.h\n> > index d477885561..362f355267 100644\n> > --- a/repo-settings.h\n> > +++ b/repo-settings.h\n> > @@ -68,6 +68,7 @@ struct repo_settings {\n> >       unsigned long big_file_threshold;\n> >\n> >       char *hooks_path;\n> > +     char *git_attributes_file;\n> >  };\n> >  #define REPO_SETTINGS_INIT { \\\n> >       .shared_repository = -1, \\\n>\n> It would make more sense to rename this variable to\n> `attributes_file_path`, that would better denote what is actually stored\n> here and syncs better with `repo_settings_get_attributesfile_path`.\n>\n> > @@ -99,4 +100,11 @@ 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> >\n> > +/*\n> > + * Read the value for \"core.attributesfile\".\n> > + * Defaults to xdg_config_home(\"attributes\") if the core.attributesfile\n> > + * isn't available.\n>\n> While it is obvious, it would be nice to point out that\n> `core.attributesfile` is set via config.\n>\nThank you for the review Karthik.\nI will send an updated version with the changes.\n\nBello.\n"},{"id":"532922","messageId":"aVfzMsN2ouY3UBFG@ubuntu","threadId":"64647","inReplyTo":"aUO7jQQAERTe5xYc@ubuntu","subject":"[Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Olamide Caleb Bello","fromEmail":"belkid98@gmail.com","sentAt":"2026-01-02T16:32:50Z","receivedAt":"2026-01-02T16:33:00Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?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.\nThe functions `bootstrap_attr_stack()` and `git_attr_val_system()`\nretrieve \"core.attributesFile\" via `git_attr_global_file()`\nwhich reads from global state `git_attributes_file`.\n\nMove the \"core.attributesFile\" configuration into the\n`struct repo_settings` instead of relying on the global state.\nA new function `repo_settings_get_attributesfile_path()` is added\nand used to retrieve this setting in a repository-scoped manner.\nThe functions to retrieve \"core.attributesFile\" are replaced with\nthe new accessor function `repo_settings_get_attributesfile_path()`\nThis improves multi-repository behaviour and aligns with the goal of\nlibifying of Git.\n\nNote that in `bootstrap_attr_stack()`, the `index_state` is used only\nif it exists, else we default to `the_repository`.\n\nBased-on-patch-by: Ayush Chandekar <ayu.chandekar@gmail.com>\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\nSigned-off-by: Olamide Caleb Bello <belkid98@gmail.com>\n---\nThe link to the GitHub CI is provided below\nhttps://github.com/git/git/actions/runs/20661817110\n\nChanges in v2:\n--------------\n- Renamed the variable in the repo-settings struct to `attributes_file_path`\n- Modified the comment section of the accessor function declaration to\n  indicate the core.attributesFile is read via repo config.\n\nRange diff vs v1:\n-----------------\n1:  4975f77cde ! 1:  fc1dbec892 environment: move \"core.attributesfile\" into repo-setting\n    @@ Commit message\n         Note that in `bootstrap_attr_stack()`, the `index_state` is used only\n         if it exists, else we default to `the_repository`.\n     \n    -    Reported-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n    +    Based-on-patch-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n         Mentored-by: Christian Couder <christian.couder@gmail.com>\n         Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n         Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>\n    @@ attr.c: static void bootstrap_attr_stack(struct index_state *istate,\n      \tif (*stack)\n      \t\treturn;\n     @@ attr.c: static void bootstrap_attr_stack(struct index_state *istate,\n    - \t\tpush_stack(stack, e, NULL, 0);\n      \t}\n      \n    --\t/* home directory */\n    + \t/* home directory */\n     -\tif (git_attr_global_file()) {\n     -\t\te = read_attr_from_file(git_attr_global_file(), flags);\n     +\tif (istate && istate->repo)\n    @@ repo-settings.c: 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    ++\tFREE_AND_NULL(r->settings.attributes_file_path);\n      \tr->settings = empty;\n      }\n      \n    @@ repo-settings.c: void repo_settings_reset_shared_repository(struct repository *r\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    ++\tif (!repo->settings.attributes_file_path) {\n    ++\t\tif (repo_config_get_pathname(repo, \"core.attributesfile\", &repo->settings.attributes_file_path))\n    ++\t\t\trepo->settings.attributes_file_path = xdg_config_home(\"attributes\");\n     +\t}\n    -+\treturn repo->settings.git_attributes_file;\n    ++\treturn repo->settings.attributes_file_path;\n     +}\n     \n      ## repo-settings.h ##\n    @@ repo-settings.h: struct repo_settings {\n      \tunsigned long big_file_threshold;\n      \n      \tchar *hooks_path;\n    -+\tchar *git_attributes_file;\n    ++\tchar *attributes_file_path;\n      };\n      #define REPO_SETTINGS_INIT { \\\n      \t.shared_repository = -1, \\\n    @@ repo-settings.h: int repo_settings_get_shared_repository(struct repository *repo\n     +/*\n     + * Read the value for \"core.attributesfile\".\n     + * Defaults to xdg_config_home(\"attributes\") if the core.attributesfile\n    -+ * isn't available.\n    ++ * which is set via repo config isn't available.\n     + */\n     +const char *repo_settings_get_attributesfile_path(struct repository *repo);\n     +\n\n attr.c          | 19 +++++++++----------\n attr.h          |  3 ---\n builtin/var.c   |  2 +-\n environment.c   |  6 ------\n environment.h   |  1 -\n repo-settings.c | 10 ++++++++++\n repo-settings.h |  8 ++++++++\n 7 files changed, 28 insertions(+), 21 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 4999b7e09d..b081400c18 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@@ -912,6 +904,8 @@ static void bootstrap_attr_stack(struct index_state *istate,\n {\n \tstruct attr_stack *e;\n \tunsigned flags = READ_ATTR_MACRO_OK;\n+\tconst char *attributes_file_path;\n+\tstruct repository *repo;\n \n \tif (*stack)\n \t\treturn;\n@@ -927,8 +921,13 @@ 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 (istate && istate->repo)\n+\t\trepo = istate->repo;\n+\telse\n+\t\trepo = the_repository;\n+\tattributes_file_path = repo_settings_get_attributesfile_path(repo);\n+\tif (attributes_file_path) {\n+\t\te = read_attr_from_file(attributes_file_path, flags);\n \t\tpush_stack(stack, e, NULL, 0);\n \t}\n \ndiff --git a/attr.h b/attr.h\nindex a04a521092..956ce6ba62 100644\n--- a/attr.h\n+++ b/attr.h\n@@ -232,9 +232,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/var.c b/builtin/var.c\nindex cc3a43cde2..fd577f2930 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -72,7 +72,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;\ndiff --git a/environment.c b/environment.c\nindex a770b5921d..ed7d8f42d9 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -53,7 +53,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;\n@@ -363,11 +362,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.h b/environment.h\nindex 51898c99cd..3512a7072e 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -152,7 +152,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 unsigned long pack_size_limit_cfg;\ndiff --git a/repo-settings.c b/repo-settings.c\nindex 195c24e9c0..cc53a3cd3b 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@@ -158,6 +159,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.attributes_file_path);\n \tr->settings = empty;\n }\n \n@@ -230,3 +232,11 @@ void repo_settings_reset_shared_repository(struct repository *repo)\n {\n \trepo->settings.shared_repository_initialized = 0;\n }\n+const char *repo_settings_get_attributesfile_path(struct repository *repo)\n+{\n+\tif (!repo->settings.attributes_file_path) {\n+\t\tif (repo_config_get_pathname(repo, \"core.attributesfile\", &repo->settings.attributes_file_path))\n+\t\t\trepo->settings.attributes_file_path = xdg_config_home(\"attributes\");\n+\t}\n+\treturn repo->settings.attributes_file_path;\n+}\ndiff --git a/repo-settings.h b/repo-settings.h\nindex d477885561..1209e1db83 100644\n--- a/repo-settings.h\n+++ b/repo-settings.h\n@@ -68,6 +68,7 @@ struct repo_settings {\n \tunsigned long big_file_threshold;\n \n \tchar *hooks_path;\n+\tchar *attributes_file_path;\n };\n #define REPO_SETTINGS_INIT { \\\n \t.shared_repository = -1, \\\n@@ -99,4 +100,11 @@ 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 \n+/*\n+ * Read the value for \"core.attributesfile\".\n+ * Defaults to xdg_config_home(\"attributes\") if the core.attributesfile\n+ * which is set via repo config isn't available.\n+ */\n+const char *repo_settings_get_attributesfile_path(struct repository *repo);\n+\n #endif /* REPO_SETTINGS_H */\n-- \n2.52.0.210.g7f9f2609ac.dirty\n\n"},{"id":"533026","messageId":"CAOLa=ZTOKvEQaMxymi+mRcqyNy4bZ4JbK2HPtq6CeewjHMo_=g@mail.gmail.com","threadId":"64647","inReplyTo":"aVfzMsN2ouY3UBFG@ubuntu","subject":"Re: [Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-05T11:09:43Z","receivedAt":"2026-01-05T11:09:45Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Olamide Caleb Bello <belkid98@gmail.com> writes:\n[snip]\n\n> @@ -927,8 +921,13 @@ 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 (istate && istate->repo)\n> +\t\trepo = istate->repo;\n> +\telse\n> +\t\trepo = the_repository;\n> +\tattributes_file_path = repo_settings_get_attributesfile_path(repo);\n> +\tif (attributes_file_path) {\n> +\t\te = read_attr_from_file(attributes_file_path, flags);\n>  \t\tpush_stack(stack, e, NULL, 0);\n>  \t}\n>\n\nFor my own understanding, when can `istate` be NULL?\n\n[snip]\n"},{"id":"533028","messageId":"CAD=f0L-ge9FfNh04Nu05eg9Q6t_gtLaPi2=jiT1LXOjF20OO2Q@mail.gmail.com","threadId":"64647","inReplyTo":"CAOLa=ZTOKvEQaMxymi+mRcqyNy4bZ4JbK2HPtq6CeewjHMo_=g@mail.gmail.com","subject":"Re: [Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2026-01-05T11:39:19Z","receivedAt":"2026-01-05T11:39:18Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Mon, 5 Jan 2026 at 12:09, Karthik Nayak <karthik.188@gmail.com> wrote:\n>\n> Olamide Caleb Bello <belkid98@gmail.com> writes:\n> [snip]\n>\n> > @@ -927,8 +921,13 @@ static void bootstrap_attr_stack(struct index_state *istate,\n> >       }\n> >\n> >       /* home directory */\n> > -     if (git_attr_global_file()) {\n> > -             e = read_attr_from_file(git_attr_global_file(), flags);\n> > +     if (istate && istate->repo)\n> > +             repo = istate->repo;\n> > +     else\n> > +             repo = the_repository;\n> > +     attributes_file_path = repo_settings_get_attributesfile_path(repo);\n> > +     if (attributes_file_path) {\n> > +             e = read_attr_from_file(attributes_file_path, flags);\n> >               push_stack(stack, e, NULL, 0);\n> >       }\n> >\n>\n> For my own understanding, when can `istate` be NULL?\n>\n\nThank you for your question Karthik.\nSo it was stated in a comment in `apply.c:read_old_data():2340` that\n`git apply without --index/cached\nshould never look at the index because the target file may not be in\nthe index yet\nand we may not be in a Git repository.`\nSo NULL is passed to convert_to_git() in place of `istate`.\n\nSo when we do\n`git apply patch.file` `istate` is NULL, but when we do\n`git apply --cached patch.file`, `istate` is not NULL.\n\nBello\n"},{"id":"533050","messageId":"a881499d-e236-4f8e-a217-b6bce69e3e3c@gmail.com","threadId":"64647","inReplyTo":"aVfzMsN2ouY3UBFG@ubuntu","subject":"Re: [Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-01-05T14:23:26Z","receivedAt":"2026-01-05T14:23:29Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Olamide\n\nOn 02/01/2026 16:32, Olamide Caleb Bello wrote:\n> When handling multiple repositories within the same process, relying on\n> global state for accessing the \"core.attributesFile\" configuration can\n> lead to incorrect values being used. It also makes it harder to isolate\n> repositories and hinders the libification of git.\n> The functions `bootstrap_attr_stack()` and `git_attr_val_system()`\n> retrieve \"core.attributesFile\" via `git_attr_global_file()`\n> which reads from global state `git_attributes_file`.\n> \n> Move the \"core.attributesFile\" configuration into the\n> `struct repo_settings` instead of relying on the global state.\n\nThis changes when the config setting gets parsed which unfortunately \nregresses the user experience when the setting is invalid.\n\nIf I run 'git -c core.attributesFile=~does-not-exist rebase -i' with git \nbuilt from master it fails immediately with \"fatal: failed to expand \nuser dir in: '~does-not-exist'\". With this patch applied it prompts me \nto edit the todo list and then fails when it tries to checkout the \ncommit we're rebasing onto. Because \"git rebase\" expects reset_head() to \nreturn an error rather die if the checkout fails it is left in a strange \nstate where only practical course of action for the user is to run \"git \nrebase --abort\".\n\nIt is quite common that moving from parsing config settings eagerly by \ncalling repo_config() at startup to parsing them lazily via 'stuct \nrepo_settings' causes regressions like this. We really should find a way \nto address that before moving more settings into 'struct repo_settings'\n\nThanks\n\nPhillip\n\n\n> A new function `repo_settings_get_attributesfile_path()` is added\n> and used to retrieve this setting in a repository-scoped manner.\n> The functions to retrieve \"core.attributesFile\" are replaced with\n> the new accessor function `repo_settings_get_attributesfile_path()`\n> This improves multi-repository behaviour and aligns with the goal of\n> libifying of Git.\n> \n> Note that in `bootstrap_attr_stack()`, the `index_state` is used only\n> if it exists, else we default to `the_repository`.\n> \n> Based-on-patch-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n> Mentored-by: Christian Couder <christian.couder@gmail.com>\n> Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>\n> ---\n> The link to the GitHub CI is provided below\n> https://github.com/git/git/actions/runs/20661817110\n> \n> Changes in v2:\n> --------------\n> - Renamed the variable in the repo-settings struct to `attributes_file_path`\n> - Modified the comment section of the accessor function declaration to\n>    indicate the core.attributesFile is read via repo config.\n> \n> Range diff vs v1:\n> -----------------\n> 1:  4975f77cde ! 1:  fc1dbec892 environment: move \"core.attributesfile\" into repo-setting\n>      @@ Commit message\n>           Note that in `bootstrap_attr_stack()`, the `index_state` is used only\n>           if it exists, else we default to `the_repository`.\n>       \n>      -    Reported-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n>      +    Based-on-patch-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n>           Mentored-by: Christian Couder <christian.couder@gmail.com>\n>           Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n>           Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>\n>      @@ attr.c: static void bootstrap_attr_stack(struct index_state *istate,\n>        \tif (*stack)\n>        \t\treturn;\n>       @@ attr.c: static void bootstrap_attr_stack(struct index_state *istate,\n>      - \t\tpush_stack(stack, e, NULL, 0);\n>        \t}\n>        \n>      --\t/* home directory */\n>      + \t/* home directory */\n>       -\tif (git_attr_global_file()) {\n>       -\t\te = read_attr_from_file(git_attr_global_file(), flags);\n>       +\tif (istate && istate->repo)\n>      @@ repo-settings.c: 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>      ++\tFREE_AND_NULL(r->settings.attributes_file_path);\n>        \tr->settings = empty;\n>        }\n>        \n>      @@ repo-settings.c: void repo_settings_reset_shared_repository(struct repository *r\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>      ++\tif (!repo->settings.attributes_file_path) {\n>      ++\t\tif (repo_config_get_pathname(repo, \"core.attributesfile\", &repo->settings.attributes_file_path))\n>      ++\t\t\trepo->settings.attributes_file_path = xdg_config_home(\"attributes\");\n>       +\t}\n>      -+\treturn repo->settings.git_attributes_file;\n>      ++\treturn repo->settings.attributes_file_path;\n>       +}\n>       \n>        ## repo-settings.h ##\n>      @@ repo-settings.h: struct repo_settings {\n>        \tunsigned long big_file_threshold;\n>        \n>        \tchar *hooks_path;\n>      -+\tchar *git_attributes_file;\n>      ++\tchar *attributes_file_path;\n>        };\n>        #define REPO_SETTINGS_INIT { \\\n>        \t.shared_repository = -1, \\\n>      @@ repo-settings.h: int repo_settings_get_shared_repository(struct repository *repo\n>       +/*\n>       + * Read the value for \"core.attributesfile\".\n>       + * Defaults to xdg_config_home(\"attributes\") if the core.attributesfile\n>      -+ * isn't available.\n>      ++ * which is set via repo config isn't available.\n>       + */\n>       +const char *repo_settings_get_attributesfile_path(struct repository *repo);\n>       +\n> \n>   attr.c          | 19 +++++++++----------\n>   attr.h          |  3 ---\n>   builtin/var.c   |  2 +-\n>   environment.c   |  6 ------\n>   environment.h   |  1 -\n>   repo-settings.c | 10 ++++++++++\n>   repo-settings.h |  8 ++++++++\n>   7 files changed, 28 insertions(+), 21 deletions(-)\n> \n> diff --git a/attr.c b/attr.c\n> index 4999b7e09d..b081400c18 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> @@ -912,6 +904,8 @@ static void bootstrap_attr_stack(struct index_state *istate,\n>   {\n>   \tstruct attr_stack *e;\n>   \tunsigned flags = READ_ATTR_MACRO_OK;\n> +\tconst char *attributes_file_path;\n> +\tstruct repository *repo;\n>   \n>   \tif (*stack)\n>   \t\treturn;\n> @@ -927,8 +921,13 @@ 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 (istate && istate->repo)\n> +\t\trepo = istate->repo;\n> +\telse\n> +\t\trepo = the_repository;\n> +\tattributes_file_path = repo_settings_get_attributesfile_path(repo);\n> +\tif (attributes_file_path) {\n> +\t\te = read_attr_from_file(attributes_file_path, flags);\n>   \t\tpush_stack(stack, e, NULL, 0);\n>   \t}\n>   \n> diff --git a/attr.h b/attr.h\n> index a04a521092..956ce6ba62 100644\n> --- a/attr.h\n> +++ b/attr.h\n> @@ -232,9 +232,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/var.c b/builtin/var.c\n> index cc3a43cde2..fd577f2930 100644\n> --- a/builtin/var.c\n> +++ b/builtin/var.c\n> @@ -72,7 +72,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> diff --git a/environment.c b/environment.c\n> index a770b5921d..ed7d8f42d9 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -53,7 +53,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;\n> @@ -363,11 +362,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;\n> diff --git a/environment.h b/environment.h\n> index 51898c99cd..3512a7072e 100644\n> --- a/environment.h\n> +++ b/environment.h\n> @@ -152,7 +152,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 unsigned long pack_size_limit_cfg;\n> diff --git a/repo-settings.c b/repo-settings.c\n> index 195c24e9c0..cc53a3cd3b 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> @@ -158,6 +159,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.attributes_file_path);\n>   \tr->settings = empty;\n>   }\n>   \n> @@ -230,3 +232,11 @@ void repo_settings_reset_shared_repository(struct repository *repo)\n>   {\n>   \trepo->settings.shared_repository_initialized = 0;\n>   }\n> +const char *repo_settings_get_attributesfile_path(struct repository *repo)\n> +{\n> +\tif (!repo->settings.attributes_file_path) {\n> +\t\tif (repo_config_get_pathname(repo, \"core.attributesfile\", &repo->settings.attributes_file_path))\n> +\t\t\trepo->settings.attributes_file_path = xdg_config_home(\"attributes\");\n> +\t}\n> +\treturn repo->settings.attributes_file_path;\n> +}\n> diff --git a/repo-settings.h b/repo-settings.h\n> index d477885561..1209e1db83 100644\n> --- a/repo-settings.h\n> +++ b/repo-settings.h\n> @@ -68,6 +68,7 @@ struct repo_settings {\n>   \tunsigned long big_file_threshold;\n>   \n>   \tchar *hooks_path;\n> +\tchar *attributes_file_path;\n>   };\n>   #define REPO_SETTINGS_INIT { \\\n>   \t.shared_repository = -1, \\\n> @@ -99,4 +100,11 @@ 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>   \n> +/*\n> + * Read the value for \"core.attributesfile\".\n> + * Defaults to xdg_config_home(\"attributes\") if the core.attributesfile\n> + * which is set via repo config isn't available.\n> + */\n> +const char *repo_settings_get_attributesfile_path(struct repository *repo);\n> +\n>   #endif /* REPO_SETTINGS_H */\n\n"},{"id":"533055","messageId":"3947f777-e08a-4c17-81e3-c4711fe666a0@gmail.com","threadId":"64647","inReplyTo":"a881499d-e236-4f8e-a217-b6bce69e3e3c@gmail.com","subject":"Re: [Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-01-05T15:00:28Z","receivedAt":"2026-01-05T15:00:32Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 05/01/2026 14:23, Phillip Wood wrote:\n> \n> It is quite common that moving from parsing config settings eagerly by \n> calling repo_config() at startup to parsing them lazily via 'stuct \n> repo_settings' causes regressions like this. We really should find a way \n> to address that before moving more settings into 'struct repo_settings'\n\nSee \nhttps://lore.kernel.org/git/d61c966b-61ae-4ba9-b983-c8dab6e2c292@gmail.com \nfor some discussion about a possible solution.\n\nThanks\n\nPhillip\n\n"},{"id":"533086","messageId":"xmqq1pk3lmu3.fsf@gitster.g","threadId":"64647","inReplyTo":"a881499d-e236-4f8e-a217-b6bce69e3e3c@gmail.com","subject":"Re: [Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-05T22:24:36Z","receivedAt":"2026-01-05T22:24:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> If I run 'git -c core.attributesFile=~does-not-exist rebase -i' with git \n> built from master it fails immediately with \"fatal: failed to expand \n> user dir in: '~does-not-exist'\".\n\nHmph, if you call any behaviour change a \"regression\", this may\ncertainly count as one, but I do not necessarily think the above is\na good behaviour.\n\nThink about a use case where attributes are not used at all, e.g.,\n\"git -c core.attributesFile=~does-not-matter cat-file -t HEAD\";\nwould it make sense to barf when your configuration file has an\ninvalid definition for what you are *not* using?  So if the change\nmakes it stop barfing, it can even be argued that this is an\nimprovement.\n\n> It is quite common that moving from parsing config settings eagerly by \n> calling repo_config() at startup to parsing them lazily via 'stuct \n> repo_settings' causes regressions like this. We really should find a way \n> to address that before moving more settings into 'struct repo_settings'\n\nVery true.  If we know the set of things we parse early and have a\nway to say \"this command only X, Y, and Z matters (but not W)\", then\nthe above cat-file example can omit the attributesFile from the \"we\ncare\" set.\n\nI think overusing repo_settings is a disease.  Moving a singleton\nglobal to per repository (by adding to struct repository) is one\nthing and it is very welcome.  But changing the way configuration\nvariables are parsed (e.g., what used to be parsed by only those who\ncare about is now parsed by everybody, or vice versa) needs to be\nhandled carefully.\n"},{"id":"533087","messageId":"xmqqwm1vk83a.fsf@gitster.g","threadId":"64647","inReplyTo":"3947f777-e08a-4c17-81e3-c4711fe666a0@gmail.com","subject":"Re: [Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-05T22:28:25Z","receivedAt":"2026-01-05T22:28:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> On 05/01/2026 14:23, Phillip Wood wrote:\n>> \n>> It is quite common that moving from parsing config settings eagerly by \n>> calling repo_config() at startup to parsing them lazily via 'stuct \n>> repo_settings' causes regressions like this. We really should find a way \n>> to address that before moving more settings into 'struct repo_settings'\n>\n> See \n> https://lore.kernel.org/git/d61c966b-61ae-4ba9-b983-c8dab6e2c292@gmail.com \n> for some discussion about a possible solution.\n\nNice, but I suspect it would be an improvement already without\npassing repository instance via git_default_config() and instead\nhave the code use the_repository; it is even possible not to have\nany repository when the callchain executes.\n"},{"id":"533112","messageId":"CAD=f0L-hv1ZYGDyHRCYu3BqgrbvutS+JVn0D3kBq-wq--qgY7A@mail.gmail.com","threadId":"64647","inReplyTo":"a881499d-e236-4f8e-a217-b6bce69e3e3c@gmail.com","subject":"Re: [Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2026-01-06T08:08:10Z","receivedAt":"2026-01-06T08:08:09Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Mon, 5 Jan 2026 at 15:23, Phillip Wood <phillip.wood123@gmail.com> wrote:\n>\n> Hi Olamide\n>\n> On 02/01/2026 16:32, Olamide Caleb Bello wrote:\n> > When handling multiple repositories within the same process, relying on\n> > global state for accessing the \"core.attributesFile\" configuration can\n> > lead to incorrect values being used. It also makes it harder to isolate\n> > repositories and hinders the libification of git.\n> > The functions `bootstrap_attr_stack()` and `git_attr_val_system()`\n> > retrieve \"core.attributesFile\" via `git_attr_global_file()`\n> > which reads from global state `git_attributes_file`.\n> >\n> > Move the \"core.attributesFile\" configuration into the\n> > `struct repo_settings` instead of relying on the global state.\n>\n> This changes when the config setting gets parsed which unfortunately\n> regresses the user experience when the setting is invalid.\n>\n> If I run 'git -c core.attributesFile=~does-not-exist rebase -i' with git\n> built from master it fails immediately with \"fatal: failed to expand\n> user dir in: '~does-not-exist'\". With this patch applied it prompts me\n> to edit the todo list and then fails when it tries to checkout the\n> commit we're rebasing onto. Because \"git rebase\" expects reset_head() to\n> return an error rather die if the checkout fails it is left in a strange\n> state where only practical course of action for the user is to run \"git\n> rebase --abort\".\n\nYes I tried this and I experienced the same behaviour.\n\n>\n> It is quite common that moving from parsing config settings eagerly by\n> calling repo_config() at startup to parsing them lazily via 'stuct\n> repo_settings' causes regressions like this. We really should find a way\n> to address that before moving more settings into 'struct repo_settings'\n>\n\nYes, I came across an initial discussion about `prepare_repo_settings()`\nand the issues about the appropriate place to call it but it seemed there was\nno resolution then.\n"},{"id":"533113","messageId":"CAD=f0L8aoddeekws0vemTuWL7vb1eJv0kRhAGvEUTVG+17qtDw@mail.gmail.com","threadId":"64647","inReplyTo":"3947f777-e08a-4c17-81e3-c4711fe666a0@gmail.com","subject":"Re: [Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2026-01-06T08:09:07Z","receivedAt":"2026-01-06T08:09:06Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Mon, 5 Jan 2026 at 16:00, Phillip Wood <phillip.wood123@gmail.com> wrote:\n>\n> On 05/01/2026 14:23, Phillip Wood wrote:\n> >\n> > It is quite common that moving from parsing config settings eagerly by\n> > calling repo_config() at startup to parsing them lazily via 'stuct\n> > repo_settings' causes regressions like this. We really should find a way\n> > to address that before moving more settings into 'struct repo_settings'\n>\n> See\n> https://lore.kernel.org/git/d61c966b-61ae-4ba9-b983-c8dab6e2c292@gmail.com\n> for some discussion about a possible solution.\n\nYes, thank you.\n"},{"id":"533115","messageId":"CAD=f0L9BEPSQivgpM7qURT+WFDY-+Ys_M6Knv8hE0JDw4Wjj5A@mail.gmail.com","threadId":"64647","inReplyTo":"xmqqwm1vk83a.fsf@gitster.g","subject":"Re: [Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2026-01-06T09:33:20Z","receivedAt":"2026-01-06T09:33:19Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Mon, 5 Jan 2026 at 23:28, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n>\n> > On 05/01/2026 14:23, Phillip Wood wrote:\n> >>\n> >> It is quite common that moving from parsing config settings eagerly by\n> >> calling repo_config() at startup to parsing them lazily via 'stuct\n> >> repo_settings' causes regressions like this. We really should find a way\n> >> to address that before moving more settings into 'struct repo_settings'\n> >\n> > See\n> > https://lore.kernel.org/git/d61c966b-61ae-4ba9-b983-c8dab6e2c292@gmail.com\n> > for some discussion about a possible solution.\n>\n> Nice, but I suspect it would be an improvement already without\n> passing repository instance via git_default_config() and instead\n> have the code use the_repository; it is even possible not to have\n> any repository when the callchain executes.\n\nThank you Junio.\nOkay, should I move the variable into the repo struct or repo-settings struct\nas the case may be, then initialize it in git_default_config()?\n\nFor example\n`\n         if (!strcmp(var, \"core.attributesfile\")) {\n              FREE_AND_NULL(the_repository->settings.git_attributes_file);\n              return\ngit_config_pathname(&the_repository->settings.git_attributes_file,\nvar, value);\n            }\n`\nDoes this sound reasonable?\n\nAlso the `git_default_core_config()` is used to initialize the\nvariables to store the settings\nin the \"core\" section of the config file.\nSo how about other functions like `git _default_branch_config()`,\n`git _default-sparse_config()`.\nDo I use the same approach above?\n\nThanks\n"},{"id":"533144","messageId":"CAD=f0L9H5Q=zW02nr11OSBNgFH3UMLwVjVjn3zhgZ2rjwE85WA@mail.gmail.com","threadId":"64647","inReplyTo":"CAD=f0L9BEPSQivgpM7qURT+WFDY-+Ys_M6Knv8hE0JDw4Wjj5A@mail.gmail.com","subject":"Re: [Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2026-01-06T13:44:56Z","receivedAt":"2026-01-06T13:44:57Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Tue, 6 Jan 2026 at 10:33, Bello Olamide <belkid98@gmail.com> wrote:\n>\n> On Mon, 5 Jan 2026 at 23:28, Junio C Hamano <gitster@pobox.com> wrote:\n> >\n> > Phillip Wood <phillip.wood123@gmail.com> writes:\n> >\n> > > On 05/01/2026 14:23, Phillip Wood wrote:\n> > >>\n> > >> It is quite common that moving from parsing config settings eagerly by\n> > >> calling repo_config() at startup to parsing them lazily via 'stuct\n> > >> repo_settings' causes regressions like this. We really should find a way\n> > >> to address that before moving more settings into 'struct repo_settings'\n> > >\n> > > See\n> > > https://lore.kernel.org/git/d61c966b-61ae-4ba9-b983-c8dab6e2c292@gmail.com\n> > > for some discussion about a possible solution.\n> >\n> > Nice, but I suspect it would be an improvement already without\n> > passing repository instance via git_default_config() and instead\n> > have the code use the_repository; it is even possible not to have\n> > any repository when the callchain executes.\n\nBut won't this be a temporary solution since the goal is to prevent the use of\n`the_repository`?\n"},{"id":"533206","messageId":"34175f3d-0814-47c6-9945-7c19f2c60ca4@gmail.com","threadId":"64647","inReplyTo":"xmqq1pk3lmu3.fsf@gitster.g","subject":"Re: [Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-01-07T10:17:39Z","receivedAt":"2026-01-07T10:17:49Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 05/01/2026 22:24, Junio C Hamano wrote:\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n> \n>> If I run 'git -c core.attributesFile=~does-not-exist rebase -i' with git\n>> built from master it fails immediately with \"fatal: failed to expand\n>> user dir in: '~does-not-exist'\".\n> \n> Hmph, if you call any behaviour change a \"regression\", this may\n> certainly count as one, but I do not necessarily think the above is\n> a good behaviour.\n\nOne can argue that it depends on the command, but a as far as \"git \nrebase\" is concerned this change in behavior is a regression. It is not \njust rebase that is affected, for example \"git merge\" in a partial clone \nnow downloads all the blobs it wants before erroring out which isn't \nterrible but it is hard to argue that's an improvement. I suspect \"git \ncherry-pick\" and \"git revert\" are also negatively impacted by this change.\n\n> Think about a use case where attributes are not used at all, e.g.,\n> \"git -c core.attributesFile=~does-not-matter cat-file -t HEAD\";\n> would it make sense to barf when your configuration file has an\n> invalid definition for what you are *not* using?  So if the change\n> makes it stop barfing, it can even be argued that this is an\n> improvement.\n\nFor some commands, but the fact that other commands now die when they're \nnot expecting to and so end up in a strange state is a regression. Given \nhow widespread the use of attributes is it would be hard to audit all \nthe affected code paths and adjust them to the new behavior.\n\n>> It is quite common that moving from parsing config settings eagerly by\n>> calling repo_config() at startup to parsing them lazily via 'stuct\n>> repo_settings' causes regressions like this. We really should find a way\n>> to address that before moving more settings into 'struct repo_settings'\n> \n> Very true.  If we know the set of things we parse early and have a\n> way to say \"this command only X, Y, and Z matters (but not W)\", then\n> the above cat-file example can omit the attributesFile from the \"we\n> care\" set.\n\nThat would be nice, but it would be painful to implement for commands \nlike cat-file which sometimes need to read core.attributesFile and \nsometimes don't depending on which options they're passed as we \ntypically read the config before parsing the command line options. We'd \nalso need to be careful to update the list when adding new features \nwhich depended on config variables that were not previously used by that \ncommand.\n\n> I think overusing repo_settings is a disease.  Moving a singleton\n> global to per repository (by adding to struct repository) is one\n> thing and it is very welcome.  But changing the way configuration\n> variables are parsed (e.g., what used to be parsed by only those who\n> care about is now parsed by everybody, or vice versa) needs to be\n> handled carefully.\n\nIndeed\n\nThanks\n\nPhillip\n\n"},{"id":"533207","messageId":"922629dc-828c-4bdf-939c-b38b7b59e8e8@gmail.com","threadId":"64647","inReplyTo":"CAD=f0L9H5Q=zW02nr11OSBNgFH3UMLwVjVjn3zhgZ2rjwE85WA@mail.gmail.com","subject":"Re: [Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-01-07T10:26:12Z","receivedAt":"2026-01-07T10:26:22Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 06/01/2026 13:44, Bello Olamide wrote:\n> On Tue, 6 Jan 2026 at 10:33, Bello Olamide <belkid98@gmail.com> wrote:\n>>\n>> On Mon, 5 Jan 2026 at 23:28, Junio C Hamano <gitster@pobox.com> wrote:\n>>>\n>>> Phillip Wood <phillip.wood123@gmail.com> writes:\n>>>\n>>>> On 05/01/2026 14:23, Phillip Wood wrote:\n>>>>>\n>>>>> It is quite common that moving from parsing config settings eagerly by\n>>>>> calling repo_config() at startup to parsing them lazily via 'stuct\n>>>>> repo_settings' causes regressions like this. We really should find a way\n>>>>> to address that before moving more settings into 'struct repo_settings'\n>>>>\n>>>> See\n>>>> https://lore.kernel.org/git/d61c966b-61ae-4ba9-b983-c8dab6e2c292@gmail.com\n>>>> for some discussion about a possible solution.\n>>>\n>>> Nice, but I suspect it would be an improvement already without\n>>> passing repository instance via git_default_config() and instead\n>>> have the code use the_repository; it is even possible not to have\n>>> any repository when the callchain executes.\n> \n> But won't this be a temporary solution since the goal is to prevent the use of\n> `the_repository`?\n\nYes but it would be a good start as passing a repository down to \ngit_default_config() will be quite invasive. It would certainly be \nbetter if we can find a solution that uses the repository passed to \ncommand when it is non-NULL. Unfortunately commands like \"git diff \n--no-index\" are passed a NULL repository but we have chosen to store our \nconfig in a `struct repository` and so we need some kind of fake \nrepository for those commands. If we stored our config in a separate \nstruct we wouldn't need to fake a repository but then we'd have to pass \nthe config round separately to the repository which is a pain. Perhaps \ngit_default_config() could use `the_repository` when it's given a NULL \npointer for the callback data.\n\nThanks\n\nPhillip\n\n"},{"id":"533222","messageId":"8899016f-eeef-404b-8da6-ff3a90e81cea@gmail.com","threadId":"64647","inReplyTo":"922629dc-828c-4bdf-939c-b38b7b59e8e8@gmail.com","subject":"Re: [Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-01-07T14:18:57Z","receivedAt":"2026-01-07T14:19:07Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 07/01/2026 10:26, Phillip Wood wrote:\n> On 06/01/2026 13:44, Bello Olamide wrote:\n>>\n>> But won't this be a temporary solution since the goal is to prevent \n>> the use of\n>> `the_repository`?\n> \n> Yes but it would be a good start as passing a repository down to \n> git_default_config() will be quite invasive.\n\nTo expand on this the first steps could be\n   (i) create a new struct to hold the config settings from\n       git_default_config()\n  (ii) add that struct as a member of `struct repository`\n(iii) one-by-one, for each setting parsed by git_default_config() add a\n       new member to the config struct, store the parsed value in\n       `the_repository` and adjust any code that uses the variable.\n\nThen later we can tackle the intrusive change to pass a `struct \nrepository` down to git_default_config() and store the settings in that \nrather than `the_repository`. If we add a local variable to \ngit_default_config() in step (iii) above then getting it to use the \nrepository passed down the call chain will simply be a matter of doing \nsomething like\n\n-\tstruct repository *r = the_repository;\n+\tstruct repository *r = cb ? cb : the_repository;\n\nThanks\n\nPhillip\n\n"},{"id":"533226","messageId":"CAD=f0L9in5tjLNUcM86uzd_caTaZYre9RRygmizO4G=S_DBFRQ@mail.gmail.com","threadId":"64647","inReplyTo":"8899016f-eeef-404b-8da6-ff3a90e81cea@gmail.com","subject":"Re: [Outreachy PATCH v2] environment: move \"core.attributesFile\" into repo-setting","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2026-01-07T15:33:03Z","receivedAt":"2026-01-07T15:33:16Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Wed, 7 Jan 2026 at 15:19, Phillip Wood <phillip.wood123@gmail.com> wrote:\n>\n> On 07/01/2026 10:26, Phillip Wood wrote:\n> > On 06/01/2026 13:44, Bello Olamide wrote:\n> >>\n> >> But won't this be a temporary solution since the goal is to prevent\n> >> the use of\n> >> `the_repository`?\n> >\n> > Yes but it would be a good start as passing a repository down to\n> > git_default_config() will be quite invasive.\n>\n> To expand on this the first steps could be\n>    (i) create a new struct to hold the config settings from\n>        git_default_config()\n>   (ii) add that struct as a member of `struct repository`\n> (iii) one-by-one, for each setting parsed by git_default_config() add a\n>        new member to the config struct, store the parsed value in\n>        `the_repository` and adjust any code that uses the variable.\n>\n> Then later we can tackle the intrusive change to pass a `struct\n> repository` down to git_default_config() and store the settings in that\n> rather than `the_repository`. If we add a local variable to\n> git_default_config() in step (iii) above then getting it to use the\n> repository passed down the call chain will simply be a matter of doing\n> something like\n>\n> -       struct repository *r = the_repository;\n> +       struct repository *r = cb ? cb : the_repository;\n>\n> Thanks\n>\n> Phillip\n>\n\nThanks for the detailed proposal, which makes a lot of sense.\nI’ll align my changes with this staged approach.\n\nBello\n"}]}