{"thread":{"id":"65869","subject":"[PATCH v1 0/2] environment: move excludes_file into repo_config_values","startedAt":"2026-06-25T16:19:02Z","lastAt":"2026-06-25T20:40:11Z","messageCount":4,"participants":["Tian Yuchen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"546421","messageId":"20260625161845.7543-1-cat@malon.dev","threadId":"65869","inReplyTo":null,"subject":"[PATCH v1 0/2] environment: move excludes_file into repo_config_values","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-06-25T16:18:43Z","receivedAt":"2026-06-25T16:19:02Z","isPatch":true,"body":"This series continues the libification effort by migrating the global\nstring variable 'excludes_file' into 'struct repo_config_values'. Since\nthis is a dynamically allocated variable, the migration requires proper\nheap memory management.\n\nThe series is structured in two commits:\n\n - Abstract the XDG fallback lazy-loading logic out of dir.c into a proper\ngetter.\n\n - Move the variable into the struct and introducs 'repo_config_values_clear()'.\n\nNote on Submodules: A temporary shield 'if (repo != the_repository)' is\nincluded in both the getter and the clear function. This prevents\nuninitialized submodules from triggering the BUG() assertion.\n(Inspiration: [1])\n\nThanks.\n\nMentored-by: Christian Couder \nMentored-by: Ayush Chandekar \nMentored-by: Olamide Caleb Bello \nSigned-off-by: Tian Yuchen \n\n[1] https://lore.kernel.org/git/c95a7730-7b14-4be0-a4e4-861b2f5430ea@gmail.com/\n\nTian Yuchen (2):\n dir: encapsulate excludes_file lazy-load\n environment: move excludes_file into repo_config_values\n\n dir.c | 4 ++--\n environment.c | 29 ++++++++++++++++++++++++++---\n environment.h | 14 +++++++++++++-\n repository.c | 1 +\n 4 files changed, 42 insertions(+), 6 deletions(-)\n\n-- \n2.43.0\n\n"},{"id":"546422","messageId":"20260625161845.7543-3-cat@malon.dev","threadId":"65869","inReplyTo":"20260625161845.7543-1-cat@malon.dev","subject":"[PATCH v1 2/2] environment: move excludes_file into repo_config_values","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-06-25T16:18:45Z","receivedAt":"2026-06-25T16:19:04Z","isPatch":true,"body":"Continue the libification effor by moving the 'excludes_file' global\nvariable into 'struct repo_config_values'.\n\nSince 'excludes_file' is a dynamically allocated string (char *), it\nrequires proper memory management. Introduce repo_config_values_clear()\nto safely free the heap memory when repository instance is destroyed.\n\nNote: 'if (repo != the_repository)' fallback logic is temporarily added\nin both the getter and the clear function. This prevents calling\nrepo_config_values() on uninitialized submodules, which triggers BUG().\n\nMentored-by: Christian Couder \nMentored-by: Ayush Chandekar \nMentored-by: Olamide Caleb Bello \nSigned-off-by: Tian Yuchen \n---\n environment.c | 28 ++++++++++++++++++++++------\n environment.h | 15 +++++++++++----\n repository.c | 1 +\n 3 files changed, 34 insertions(+), 10 deletions(-)\n\ndiff --git a/environment.c b/environment.c\nindex 8efcaeafa6..e1c7ed178a 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -57,7 +57,6 @@ enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;\n enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;\n char *editor_program;\n char *askpass_program;\n-char *excludes_file;\n enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;\n enum eol core_eol = EOL_UNSET;\n int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;\n@@ -136,9 +135,13 @@ int is_bare_repository(void)\n \n const char *repo_excludes_file(struct repository *repo)\n {\n-\tif (!excludes_file)\n-\t\texcludes_file = xdg_config_home(\"ignore\");\n-\treturn excludes_file;\n+\tif (!repo || !repo->initialized || repo != the_repository)\n+\t\treturn NULL;\n+\n+\tif (!repo_config_values(repo)->excludes_file)\n+\t\trepo_config_values(repo)->excludes_file = xdg_config_home(\"ignore\");\n+\n+\treturn repo_config_values(repo)->excludes_file;\n }\n \n int have_git_dir(void)\n@@ -468,8 +471,8 @@ int git_default_core_config(const char *var, const char *value,\n \t}\n \n \tif (!strcmp(var, \"core.excludesfile\")) {\n-\t\tFREE_AND_NULL(excludes_file);\n-\t\treturn git_config_pathname(&excludes_file, var, value);\n+\t\tFREE_AND_NULL(cfg->excludes_file);\n+\t\treturn git_config_pathname(&cfg->excludes_file, var, value);\n \t}\n \n \tif (!strcmp(var, \"core.whitespace\")) {\n@@ -733,3 +736,16 @@ void repo_config_values_init(struct repo_config_values *cfg)\n \tcfg->sparse_expect_files_outside_of_patterns = 0;\n \tcfg->warn_on_object_refname_ambiguity = 1;\n }\n+\n+void repo_config_values_clear(struct repository *repo)\n+{\n+\tstruct repo_config_values *cfg;\n+\n+\tif (repo != the_repository)\n+\t\treturn;\n+\n+\tcfg = repo_config_values(repo);\n+\tif (!cfg)\n+\t\treturn;\n+\tFREE_AND_NULL(cfg->excludes_file);\n+}\ndiff --git a/environment.h b/environment.h\nindex 52d531e4ea..2839913551 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -98,6 +98,7 @@ struct repo_config_values {\n \tint precomposed_unicode;\n \tint core_sparse_checkout_cone;\n \tint warn_on_object_refname_ambiguity;\n+\tchar *excludes_file;\n \n \t/* section \"sparse\" config values */\n \tint sparse_expect_files_outside_of_patterns;\n@@ -133,13 +134,20 @@ int git_default_config(const char *, const char *,\n int git_default_core_config(const char *var, const char *value,\n \t\t\t const struct config_context *ctx, void *cb);\n \n-/*\n- * TODO: This still relies on the global state.\n- */\n const char *repo_excludes_file(struct repository *repo);\n \n void repo_config_values_init(struct repo_config_values *cfg);\n \n+/*\n+ * Frees memory allocated for dynamically loaded configuration values\n+ * inside `repo_config_values`.\n+ *\n+ * Note: `excludes_file` is currently the only heap-allocated field in\n+ * this struct. As other dynamically allocated variables are migrated,\n+ * their FREE_AND_NULL() calls should be appended here.\n+ */\n+void repo_config_values_clear(struct repository *repo);\n+\n /*\n * TODO: All the below state either explicitly or implicitly relies on\n * `the_repository`. We should eventually get rid of these and make the\n@@ -213,7 +221,6 @@ extern char *git_log_output_encoding;\n \n extern char *editor_program;\n extern char *askpass_program;\n-extern char *excludes_file;\n \n /*\n * The character that begins a commented line in user-editable file\ndiff --git a/repository.c b/repository.c\nindex 187dd471c4..b31f1b7852 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -388,6 +388,7 @@ void repo_clear(struct repository *repo)\n \tFREE_AND_NULL(repo->parsed_objects);\n \n \trepo_settings_clear(repo);\n+\trepo_config_values_clear(repo);\n \n \tif (repo->config) {\n \t\tgit_configset_clear(repo->config);\n-- \n2.43.0\n\n"},{"id":"546423","messageId":"20260625161845.7543-2-cat@malon.dev","threadId":"65869","inReplyTo":"20260625161845.7543-1-cat@malon.dev","subject":"[PATCH v1 1/2] dir: encapsulate excludes_file lazy-load","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-06-25T16:18:44Z","receivedAt":"2026-06-25T16:19:06Z","isPatch":true,"body":"The global variable 'excludes_file' is used to track the path to the\nglobal ignore file, 'core.excludesfile'. If this variable is NULL,\nsetup_standard_excludes() in dir.c forcefully evaluates and assigns\nthe XDG default path to it.\n\nIntroduce repo_excludes_file() as a getter to encapsulate this\nlazy-loading logic. This prepares the variable to be safely moved\ninto 'struct repo_config_values' in the subsequent commit.\n\nMentored-by: Christian Couder \nMentored-by: Ayush Chandekar \nMentored-by: Olamide Caleb Bello \nSigned-off-by: Tian Yuchen \n---\n dir.c | 4 ++--\n environment.c | 7 +++++++\n environment.h | 5 +++++\n 3 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 7a73690fbc..4f87a52b3c 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -3481,11 +3481,11 @@ static GIT_PATH_FUNC(git_path_info_exclude, \"info/exclude\")\n \n void setup_standard_excludes(struct dir_struct *dir)\n {\n+\tconst char *excludes_file = repo_excludes_file(the_repository);\n+\n \tdir->exclude_per_dir = \".gitignore\";\n \n \t/* core.excludesfile defaulting to $XDG_CONFIG_HOME/git/ignore */\n-\tif (!excludes_file)\n-\t\texcludes_file = xdg_config_home(\"ignore\");\n \tif (excludes_file && !access_or_warn(excludes_file, R_OK, 0))\n \t\tadd_patterns_from_file_1(dir, excludes_file,\n \t\t\t\t\t dir->untracked ? &dir->internal.ss_excludes_file : NULL);\ndiff --git a/environment.c b/environment.c\nindex ba2c60103f..8efcaeafa6 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -134,6 +134,13 @@ int is_bare_repository(void)\n \treturn is_bare_repository_cfg && !repo_get_work_tree(the_repository);\n }\n \n+const char *repo_excludes_file(struct repository *repo)\n+{\n+\tif (!excludes_file)\n+\t\texcludes_file = xdg_config_home(\"ignore\");\n+\treturn excludes_file;\n+}\n+\n int have_git_dir(void)\n {\n \treturn startup_info->have_repository\ndiff --git a/environment.h b/environment.h\nindex 6f18286955..52d531e4ea 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -133,6 +133,11 @@ int git_default_config(const char *, const char *,\n int git_default_core_config(const char *var, const char *value,\n \t\t\t const struct config_context *ctx, void *cb);\n \n+/*\n+ * TODO: This still relies on the global state.\n+ */\n+const char *repo_excludes_file(struct repository *repo);\n+\n void repo_config_values_init(struct repo_config_values *cfg);\n \n /*\n-- \n2.43.0\n\n"},{"id":"546432","messageId":"xmqqwlvme4lz.fsf@gitster.g","threadId":"65869","inReplyTo":"20260625161845.7543-3-cat@malon.dev","subject":"Re: [PATCH v1 2/2] environment: move excludes_file into repo_config_values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-25T20:40:08Z","receivedAt":"2026-06-25T20:40:11Z","isPatch":true,"body":"Tian Yuchen writes:\n\n> Continue the libification effor by moving the 'excludes_file' global\n\n\"effort\"?\n\n> variable into 'struct repo_config_values'.\n>\n> Since 'excludes_file' is a dynamically allocated string (char *), it\n> requires proper memory management. Introduce repo_config_values_clear()\n> to safely free the heap memory when repository instance is destroyed.\n>\n> Note: 'if (repo != the_repository)' fallback logic is temporarily added\n> in both the getter and the clear function. This prevents calling\n> repo_config_values() on uninitialized submodules, which triggers BUG().\n>\n> Mentored-by: Christian Couder \n> Mentored-by: Ayush Chandekar \n> Mentored-by: Olamide Caleb Bello \n> Signed-off-by: Tian Yuchen \n> ---\n> environment.c | 28 ++++++++++++++++++++++------\n> environment.h | 15 +++++++++++----\n> repository.c | 1 +\n> 3 files changed, 34 insertions(+), 10 deletions(-)\n> ...\n> @@ -733,3 +736,16 @@ void repo_config_values_init(struct repo_config_values *cfg)\n> \tcfg->sparse_expect_files_outside_of_patterns = 0;\n> \tcfg->warn_on_object_refname_ambiguity = 1;\n\nShouldn't cfg->excludes_file be explicitly initialized to NULL here\nfor completeness? There are other explicit but redundant 0 assignment\nto the members of this struct in the same function.\n\n> }\n> +\n> +void repo_config_values_clear(struct repository *repo)\n> +{\n> +\tstruct repo_config_values *cfg;\n> +\n> +\tif (repo != the_repository)\n> +\t\treturn;\n> +\n> +\tcfg = repo_config_values(repo);\n> +\tif (!cfg)\n> +\t\treturn;\n> +\tFREE_AND_NULL(cfg->excludes_file);\n> +}\n> diff --git a/environment.h b/environment.h\n> index 52d531e4ea..2839913551 100644\n> --- a/environment.h\n> +++ b/environment.h\n> @@ -98,6 +98,7 @@ struct repo_config_values {\n> \tint precomposed_unicode;\n> \tint core_sparse_checkout_cone;\n> \tint warn_on_object_refname_ambiguity;\n> +\tchar *excludes_file;\n> \n> \t/* section \"sparse\" config values */\n> \tint sparse_expect_files_outside_of_patterns;\n> @@ -133,13 +134,20 @@ int git_default_config(const char *, const char *,\n> int git_default_core_config(const char *var, const char *value,\n> \t\t\t const struct config_context *ctx, void *cb);\n> \n> -/*\n> - * TODO: This still relies on the global state.\n> - */\n> const char *repo_excludes_file(struct repository *repo);\n\nGood.\n\n> +/*\n> + * Frees memory allocated for dynamically loaded configuration values\n> + * inside `repo_config_values`.\n> + *\n> + * Note: `excludes_file` is currently the only heap-allocated field in\n> + * this struct. As other dynamically allocated variables are migrated,\n> + * their FREE_AND_NULL() calls should be appended here.\n\nIsn't attributes_file also heap-allocated member in this struct as well?\n"}]}