{"thread":{"id":"66122","subject":"[PATCH 0/3] environment: clean up repository config handling","startedAt":"2026-08-05T11:54:11Z","lastAt":"2026-09-11T06:22:42Z","messageCount":27,"participants":["Tian Yuchen","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"549679","messageId":"20260805115342.3939931-1-cat@malon.dev","threadId":"66122","inReplyTo":null,"subject":"[PATCH 0/3] environment: clean up repository config handling","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-05T11:53:38Z","receivedAt":"2026-08-05T11:54:11Z","isPatch":true,"body":"Hi all,\n\nThis series contains several cleanup patches for repository configuration\nhandling.\n\nNo functional changes are intended. The patches make the related code\nmore consistent and easier to maintain by improving documentation,\nformatting, and the organization of repo_config_values.\n\nRFC:\n\nIf there are other small cleanups in this area that would be useful to\ninclude, suggestions are welcome.\n\nRegards, yuchen\n\nTian Yuchen (3):\n  environment: simplify repository config getters\n  environment: clarify repository config getter documentation\n  environment: reorder variables in repo_config_values structure\n\n environment.c | 49 ++++++++++++++++++++++++++++++-------------------\n environment.h | 31 ++++++++++++++++---------------\n 2 files changed, 46 insertions(+), 34 deletions(-)\n\n-- \n2.43.0\n\n"},{"id":"549680","messageId":"20260805115342.3939931-3-cat@malon.dev","threadId":"66122","inReplyTo":"20260805115342.3939931-1-cat@malon.dev","subject":"[PATCH 2/3] environment: clarify repository config getter documentation","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-05T11:53:40Z","receivedAt":"2026-08-05T11:54:13Z","isPatch":true,"body":"Update the comment above repository config getters to describe their\ncommon behavior.\n\nThe getters handle repositories that are not fully initialized by\nreturning the corresponding default values.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\nMentored-by: Olamide Caleb Bello <belkid98@gmail.com>\nSigned-off-by: Tian Yuchen <cat@malon.dev>\n---\n environment.h | 11 +++--------\n 1 file changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/environment.h b/environment.h\nindex e7ec5b0437..30678257b5 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -175,18 +175,13 @@ 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- * Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.\n- * They check `repo->initialized` to prevent calling `repo_config_values()`\n- * before the repository setup is fully complete or in non-git environments.\n+ * Getters for configuration variables in `struct repo_config_values`.\n+ * These functions handle uninitialized repositories or non-git\n+ * environments by returning appropriate default values.\n  */\n int repo_protect_hfs(struct repository *repo);\n int repo_protect_ntfs(struct repository *repo);\n \n-/*\n- * Getter for the `ignore_case` field of `struct repo_config_values`.\n- * It checks `repo->initialized` to prevent calling repo_config_values()`\n- * before the repository setup is fully complete or in non-git environments.\n- */\n int repo_ignore_case(struct repository *repo);\n \n int repo_trust_executable_bit(struct repository *repo);\n-- \n2.43.0\n\n"},{"id":"549681","messageId":"20260805115342.3939931-2-cat@malon.dev","threadId":"66122","inReplyTo":"20260805115342.3939931-1-cat@malon.dev","subject":"[PATCH 1/3] environment: simplify repository config getters","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-05T11:53:39Z","receivedAt":"2026-08-05T11:54:15Z","isPatch":true,"body":"Drop unnecessary parentheses and NULL checks in repository config\ngetters.\n\nThese getters are only used with non-NULL repositories, so the\nextra checks do not match their current callers.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\nMentored-by: Olamide Caleb Bello <belkid98@gmail.com>\nSigned-off-by: Tian Yuchen <cat@malon.dev>\n---\n environment.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/environment.c b/environment.c\nindex 76ee65e62b..f5628b6758 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -119,23 +119,23 @@ int is_bare_repository(struct repository *repo)\n \n int repo_protect_ntfs(struct repository *repo)\n {\n-\treturn (repo && repo->initialized) ?\n-\t\trepo_config_values(repo)->protect_ntfs :\n-\t\tPROTECT_NTFS_DEFAULT;\n+\treturn repo->initialized\n+\t\t? repo_config_values(repo)->protect_ntfs\n+\t\t: PROTECT_NTFS_DEFAULT;\n }\n \n int repo_protect_hfs(struct repository *repo)\n {\n-\treturn (repo && repo->initialized) ?\n-\t\trepo_config_values(repo)->protect_hfs :\n-\t\tPROTECT_HFS_DEFAULT;\n+\treturn repo->initialized\n+\t\t? repo_config_values(repo)->protect_hfs\n+\t\t: PROTECT_HFS_DEFAULT;\n }\n \n int repo_ignore_case(struct repository *repo)\n {\n-\treturn (repo && repo->initialized) ?\n-\t\trepo_config_values(repo)->ignore_case :\n-\t\t0;\n+\treturn repo->initialized\n+\t\t? repo_config_values(repo)->ignore_case\n+\t\t: 0;\n }\n \n int repo_trust_executable_bit(struct repository *repo)\n-- \n2.43.0\n\n"},{"id":"549682","messageId":"20260805115342.3939931-4-cat@malon.dev","threadId":"66122","inReplyTo":"20260805115342.3939931-1-cat@malon.dev","subject":"[PATCH 3/3] environment: reorder variables in repo_config_values structure","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-05T11:53:41Z","receivedAt":"2026-08-05T11:54:16Z","isPatch":true,"body":"Reorder the fields in struct repo_config_values and its initialization\nfunction to follow the order of configuration sections.\n\nKeeping the declaration and initialization order aligned makes the\nstructure easier to review and maintain.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\nMentored-by: Olamide Caleb Bello <belkid98@gmail.com>\nSigned-off-by: Tian Yuchen <cat@malon.dev>\n---\n environment.c | 31 +++++++++++++++++++++----------\n environment.h | 20 +++++++++++++-------\n 2 files changed, 34 insertions(+), 17 deletions(-)\n\ndiff --git a/environment.c b/environment.c\nindex f5628b6758..918d8b50b8 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -745,31 +745,42 @@ int git_default_config(const char *var, const char *value,\n \n void repo_config_values_init(struct repo_config_values *cfg)\n {\n+\t/* core */\n \tcfg->attributes_file = NULL;\n \tcfg->excludes_file = NULL;\n \tcfg->editor_program = NULL;\n \tcfg->pager_program = NULL;\n \tcfg->askpass_program = NULL;\n-\tcfg->apply_default_whitespace = NULL;\n-\tcfg->apply_default_ignorewhitespace = NULL;\n-\tcfg->push_default = PUSH_DEFAULT_UNSPECIFIED;\n-\tcfg->autorebase = AUTOREBASE_NEVER;\n \tcfg->object_creation_mode = OBJECT_CREATION_MODE;\n \tcfg->apply_sparse_checkout = 0;\n+\tcfg->trust_ctime = 1;\n+\tcfg->check_stat = 1;\n+\tcfg->zlib_compression_level = Z_BEST_SPEED;\n+\tcfg->precomposed_unicode = -1;\n+\tcfg->core_sparse_checkout_cone = 0;\n+\tcfg->warn_on_object_refname_ambiguity = 1;\n \tcfg->protect_hfs = PROTECT_HFS_DEFAULT;\n \tcfg->protect_ntfs = PROTECT_NTFS_DEFAULT;\n \tcfg->ignore_case = 0;\n \tcfg->trust_executable_bit = 1;\n \tcfg->has_symlinks = platform_has_symlinks();\n+\n+\t/* apply */\n+\tcfg->apply_default_whitespace = NULL;\n+\tcfg->apply_default_ignorewhitespace = NULL;\n+\n+\t/* branch */\n+\tcfg->autorebase = AUTOREBASE_NEVER;\n \tcfg->branch_track = BRANCH_TRACK_REMOTE;\n-\tcfg->trust_ctime = 1;\n-\tcfg->check_stat = 1;\n-\tcfg->zlib_compression_level = Z_BEST_SPEED;\n+\n+\t/* pack */\n \tcfg->pack_compression_level = Z_DEFAULT_COMPRESSION;\n-\tcfg->precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */\n-\tcfg->core_sparse_checkout_cone = 0;\n+\n+\t/* push */\n+\tcfg->push_default = PUSH_DEFAULT_UNSPECIFIED;\n+\n+\t/* sparse */\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 repo_config_values *cfg)\ndiff --git a/environment.h b/environment.h\nindex 30678257b5..52ed13c0fc 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -121,16 +121,11 @@ struct repo_config_values {\n \tchar *editor_program;\n \tchar *pager_program;\n \tchar *askpass_program;\n-\tchar *apply_default_whitespace;\n-\tchar *apply_default_ignorewhitespace;\n-\tenum push_default_type push_default;\n-\tenum rebase_setup_type autorebase;\n \tenum object_creation_mode object_creation_mode;\n \tint apply_sparse_checkout;\n \tint trust_ctime;\n \tint check_stat;\n \tint zlib_compression_level;\n-\tint pack_compression_level;\n \tint precomposed_unicode;\n \tint core_sparse_checkout_cone;\n \tint warn_on_object_refname_ambiguity;\n@@ -140,11 +135,22 @@ struct repo_config_values {\n \tint trust_executable_bit;\n \tint has_symlinks;\n \n-\t/* section \"sparse\" config values */\n-\tint sparse_expect_files_outside_of_patterns;\n+\t/* section \"apply\" config values */\n+\tchar *apply_default_whitespace;\n+\tchar *apply_default_ignorewhitespace;\n \n \t/* section \"branch\" config values */\n+\tenum rebase_setup_type autorebase;\n \tenum branch_track branch_track;\n+\n+\t/* section \"pack\" config values */\n+\tint pack_compression_level;\n+\n+\t/* section \"push\" config values */\n+\tenum push_default_type push_default;\n+\n+\t/* section \"sparse\" config values */\n+\tint sparse_expect_files_outside_of_patterns;\n };\n \n struct repo_config_values *repo_config_values(struct repository *repo);\n-- \n2.43.0\n\n"},{"id":"549779","messageId":"xmqqtsp8nt7i.fsf@gitster.g","threadId":"66122","inReplyTo":"20260805115342.3939931-3-cat@malon.dev","subject":"Re: [PATCH 2/3] environment: clarify repository config getter documentation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-05T21:38:41Z","receivedAt":"2026-08-05T21:38:44Z","isPatch":true,"body":"Tian Yuchen <cat@malon.dev> writes:\n\n> Update the comment above repository config getters to describe their\n> common behavior.\n>\n> The getters handle repositories that are not fully initialized by\n> returning the corresponding default values.\n>\n> Mentored-by: Christian Couder <christian.couder@gmail.com>\n> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>\n> Signed-off-by: Tian Yuchen <cat@malon.dev>\n> ---\n>  environment.h | 11 +++--------\n>  1 file changed, 3 insertions(+), 8 deletions(-)\n>\n> diff --git a/environment.h b/environment.h\n> index e7ec5b0437..30678257b5 100644\n> --- a/environment.h\n> +++ b/environment.h\n> @@ -175,18 +175,13 @@ 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> - * Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.\n> - * They check `repo->initialized` to prevent calling `repo_config_values()`\n> - * before the repository setup is fully complete or in non-git environments.\n> + * Getters for configuration variables in `struct repo_config_values`.\n> + * These functions handle uninitialized repositories or non-git\n> + * environments by returning appropriate default values.\n>   */\n>  int repo_protect_hfs(struct repository *repo);\n>  int repo_protect_ntfs(struct repository *repo);\n>  \n> -/*\n\nTwo puzzlements.\n\n * Is the above comment block meant to apply to repo_ignore_case()\n   in addition to repo_protect_ntfs() and repo_protect_hfs()?  If\n   so, the blank line before repo_ignore_case() is a bit misleading.\n\n * The phrase \"uninitialized repositories or non-Git environments\"\n   strongly hints that I can pass NULL to indicate that we are\n   running in a non-Git environment.  However, the change in\n   [PATCH 1/3] we just saw means I would get a segfault if I did so,\n   does it not?\n\n> - * Getter for the `ignore_case` field of `struct repo_config_values`.\n> - * It checks `repo->initialized` to prevent calling repo_config_values()`\n> - * before the repository setup is fully complete or in non-git environments.\n> - */\n>  int repo_ignore_case(struct repository *repo);\n>  \n>  int repo_trust_executable_bit(struct repository *repo);\n"},{"id":"549780","messageId":"xmqqo6fgnssx.fsf@gitster.g","threadId":"66122","inReplyTo":"20260805115342.3939931-4-cat@malon.dev","subject":"Re: [PATCH 3/3] environment: reorder variables in repo_config_values structure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-05T21:47:26Z","receivedAt":"2026-08-05T21:47:29Z","isPatch":true,"body":"Tian Yuchen <cat@malon.dev> writes:\n\n> Reorder the fields in struct repo_config_values and its initialization\n> function to follow the order of configuration sections.\n>\n> Keeping the declaration and initialization order aligned makes the\n> structure easier to review and maintain.\n\nReally?\n\nDo you have some automated tool to make sure these initialization\nassignments in the environment.c file and declaration in the\nenvironment.h file match the order in Documentation/config/*.adoc or\nsomething else?  Have you designated some list as the authoritative\nsource of truth to check these against?  Without such a list to\ncheck the code against and a mechanism to enforce the ordering, I\nfind it hard to agree with such a claim that this makes it easier to\nmaintain.\n\nIt is typical to list the structure members in the order of stricter\nto looser alignment requirement of their types.  I do not know how\nstrictly it is followed for \"struct repo_config_values\", but by\nspreading pointer valued members more widely with smaller enums in\nbetween, the change certainly is making the overall structure size\nlarger by requiring more padding between the members with different\nalignment requirements.  Not that we would have 100s of instances of\nthese structures.\n\n> Mentored-by: Christian Couder <christian.couder@gmail.com>\n> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>\n> Signed-off-by: Tian Yuchen <cat@malon.dev>\n> ---\n>  environment.c | 31 +++++++++++++++++++++----------\n>  environment.h | 20 +++++++++++++-------\n>  2 files changed, 34 insertions(+), 17 deletions(-)\n>\n> diff --git a/environment.c b/environment.c\n> index f5628b6758..918d8b50b8 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -745,31 +745,42 @@ int git_default_config(const char *var, const char *value,\n>  \n>  void repo_config_values_init(struct repo_config_values *cfg)\n>  {\n> +\t/* core */\n>  \tcfg->attributes_file = NULL;\n>  \tcfg->excludes_file = NULL;\n>  \tcfg->editor_program = NULL;\n>  \tcfg->pager_program = NULL;\n>  \tcfg->askpass_program = NULL;\n> -\tcfg->apply_default_whitespace = NULL;\n> -\tcfg->apply_default_ignorewhitespace = NULL;\n> -\tcfg->push_default = PUSH_DEFAULT_UNSPECIFIED;\n> -\tcfg->autorebase = AUTOREBASE_NEVER;\n>  \tcfg->object_creation_mode = OBJECT_CREATION_MODE;\n>  \tcfg->apply_sparse_checkout = 0;\n> +\tcfg->trust_ctime = 1;\n> +\tcfg->check_stat = 1;\n> +\tcfg->zlib_compression_level = Z_BEST_SPEED;\n> +\tcfg->precomposed_unicode = -1;\n> +\tcfg->core_sparse_checkout_cone = 0;\n> +\tcfg->warn_on_object_refname_ambiguity = 1;\n>  \tcfg->protect_hfs = PROTECT_HFS_DEFAULT;\n>  \tcfg->protect_ntfs = PROTECT_NTFS_DEFAULT;\n>  \tcfg->ignore_case = 0;\n>  \tcfg->trust_executable_bit = 1;\n>  \tcfg->has_symlinks = platform_has_symlinks();\n> +\n> +\t/* apply */\n> +\tcfg->apply_default_whitespace = NULL;\n> +\tcfg->apply_default_ignorewhitespace = NULL;\n> +\n> +\t/* branch */\n> +\tcfg->autorebase = AUTOREBASE_NEVER;\n>  \tcfg->branch_track = BRANCH_TRACK_REMOTE;\n> -\tcfg->trust_ctime = 1;\n> -\tcfg->check_stat = 1;\n> -\tcfg->zlib_compression_level = Z_BEST_SPEED;\n> +\n> +\t/* pack */\n>  \tcfg->pack_compression_level = Z_DEFAULT_COMPRESSION;\n> -\tcfg->precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */\n> -\tcfg->core_sparse_checkout_cone = 0;\n> +\n> +\t/* push */\n> +\tcfg->push_default = PUSH_DEFAULT_UNSPECIFIED;\n> +\n> +\t/* sparse */\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 repo_config_values *cfg)\n> diff --git a/environment.h b/environment.h\n> index 30678257b5..52ed13c0fc 100644\n> --- a/environment.h\n> +++ b/environment.h\n> @@ -121,16 +121,11 @@ struct repo_config_values {\n>  \tchar *editor_program;\n>  \tchar *pager_program;\n>  \tchar *askpass_program;\n> -\tchar *apply_default_whitespace;\n> -\tchar *apply_default_ignorewhitespace;\n> -\tenum push_default_type push_default;\n> -\tenum rebase_setup_type autorebase;\n>  \tenum object_creation_mode object_creation_mode;\n>  \tint apply_sparse_checkout;\n>  \tint trust_ctime;\n>  \tint check_stat;\n>  \tint zlib_compression_level;\n> -\tint pack_compression_level;\n>  \tint precomposed_unicode;\n>  \tint core_sparse_checkout_cone;\n>  \tint warn_on_object_refname_ambiguity;\n> @@ -140,11 +135,22 @@ struct repo_config_values {\n>  \tint trust_executable_bit;\n>  \tint has_symlinks;\n>  \n> -\t/* section \"sparse\" config values */\n> -\tint sparse_expect_files_outside_of_patterns;\n> +\t/* section \"apply\" config values */\n> +\tchar *apply_default_whitespace;\n> +\tchar *apply_default_ignorewhitespace;\n>  \n>  \t/* section \"branch\" config values */\n> +\tenum rebase_setup_type autorebase;\n>  \tenum branch_track branch_track;\n> +\n> +\t/* section \"pack\" config values */\n> +\tint pack_compression_level;\n> +\n> +\t/* section \"push\" config values */\n> +\tenum push_default_type push_default;\n> +\n> +\t/* section \"sparse\" config values */\n> +\tint sparse_expect_files_outside_of_patterns;\n>  };\n>  \n>  struct repo_config_values *repo_config_values(struct repository *repo);\n"},{"id":"549781","messageId":"xmqqjyq4nsqb.fsf@gitster.g","threadId":"66122","inReplyTo":"20260805115342.3939931-2-cat@malon.dev","subject":"Re: [PATCH 1/3] environment: simplify repository config getters","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-05T21:49:00Z","receivedAt":"2026-08-05T21:49:03Z","isPatch":true,"body":"Tian Yuchen <cat@malon.dev> writes:\n\n> Drop unnecessary parentheses and NULL checks in repository config\n> getters.\n>\n> These getters are only used with non-NULL repositories, so the\n> extra checks do not match their current callers.\n\nOK.  If repo MUST always be non-NULL, even when we haven't fully\ninitialized them, then not punting on repo==NULL case like the\noriginal code is definitely an improvement.\n\nLooking good.\n\n>\n> Mentored-by: Christian Couder <christian.couder@gmail.com>\n> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>\n> Signed-off-by: Tian Yuchen <cat@malon.dev>\n> ---\n>  environment.c | 18 +++++++++---------\n>  1 file changed, 9 insertions(+), 9 deletions(-)\n>\n> diff --git a/environment.c b/environment.c\n> index 76ee65e62b..f5628b6758 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -119,23 +119,23 @@ int is_bare_repository(struct repository *repo)\n>  \n>  int repo_protect_ntfs(struct repository *repo)\n>  {\n> -\treturn (repo && repo->initialized) ?\n> -\t\trepo_config_values(repo)->protect_ntfs :\n> -\t\tPROTECT_NTFS_DEFAULT;\n> +\treturn repo->initialized\n> +\t\t? repo_config_values(repo)->protect_ntfs\n> +\t\t: PROTECT_NTFS_DEFAULT;\n>  }\n>  \n>  int repo_protect_hfs(struct repository *repo)\n>  {\n> -\treturn (repo && repo->initialized) ?\n> -\t\trepo_config_values(repo)->protect_hfs :\n> -\t\tPROTECT_HFS_DEFAULT;\n> +\treturn repo->initialized\n> +\t\t? repo_config_values(repo)->protect_hfs\n> +\t\t: PROTECT_HFS_DEFAULT;\n>  }\n>  \n>  int repo_ignore_case(struct repository *repo)\n>  {\n> -\treturn (repo && repo->initialized) ?\n> -\t\trepo_config_values(repo)->ignore_case :\n> -\t\t0;\n> +\treturn repo->initialized\n> +\t\t? repo_config_values(repo)->ignore_case\n> +\t\t: 0;\n>  }\n>  \n>  int repo_trust_executable_bit(struct repository *repo)\n"},{"id":"549809","messageId":"dbcbb042-5c50-4569-9b18-3edcc7b1ef4b@malon.dev","threadId":"66122","inReplyTo":"xmqqo6fgnssx.fsf@gitster.g","subject":"Re: [PATCH 3/3] environment: reorder variables in repo_config_values structure","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-06T08:44:03Z","receivedAt":"2026-08-06T08:44:12Z","isPatch":true,"body":"On 8/6/26 05:47, Junio C Hamano wrote:\n> Tian Yuchen <cat@malon.dev> writes:\n> \n>> Reorder the fields in struct repo_config_values and its initialization\n>> function to follow the order of configuration sections.\n>>\n>> Keeping the declaration and initialization order aligned makes the\n>> structure easier to review and maintain.\n> \n> Really?\n> \n> Do you have some automated tool to make sure these initialization\n> assignments in the environment.c file and declaration in the\n> environment.h file match the order in Documentation/config/*.adoc or\n> something else?  Have you designated some list as the authoritative\n> source of truth to check these against?  Without such a list to\n> check the code against and a mechanism to enforce the ordering, I\n> find it hard to agree with such a claim that this makes it easier to\n> maintain.\n\nI see.\n\n> \n> It is typical to list the structure members in the order of stricter\n> to looser alignment requirement of their types.  I do not know how\n> strictly it is followed for \"struct repo_config_values\", but by\n> spreading pointer valued members more widely with smaller enums in\n> between, the change certainly is making the overall structure size\n> larger by requiring more padding between the members with different\n> alignment requirements.  Not that we would have 100s of instances of\n> these structures.\n> \n\nOh, I overlooked the size issue. Thanks for pointing out.\n\n\nI think I will drop this commit. However, the original comments:\n\nstruct repo_config_values {\n\t/* section \"core\" config values */\n\tchar *attributes_file;\n\tchar *excludes_file;\n\tchar *editor_program;\n\tchar *pager_program;\n\tchar *askpass_program;\n\tchar *apply_default_whitespace;\n\tchar *apply_default_ignorewhitespace;\n\tenum push_default_type push_default;\n\tenum rebase_setup_type autorebase;\n\tenum object_creation_mode object_creation_mode;\n\tint apply_sparse_checkout;\n\tint trust_ctime;\n\tint check_stat;\n\tint zlib_compression_level;\n\tint pack_compression_level;\n\tint precomposed_unicode;\n\tint core_sparse_checkout_cone;\n\tint warn_on_object_refname_ambiguity;\n\tint protect_hfs;\n\tint protect_ntfs;\n\tint ignore_case;\n\tint trust_executable_bit;\n\tint has_symlinks;\n\n\t/* section \"sparse\" config values */\n\tint sparse_expect_files_outside_of_patterns;\n\n\t/* section \"branch\" config values */\n\tenum branch_track branch_track;\n};\n\nstill do not accurately reflect the grouping of the members, right? Can \nwe remove them directly instead?\n\n\nThanks, yuchen\n\n>> Mentored-by: Christian Couder <christian.couder@gmail.com>\n>> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n>> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>\n>> Signed-off-by: Tian Yuchen <cat@malon.dev>\n>> ---\n>>   environment.c | 31 +++++++++++++++++++++----------\n>>   environment.h | 20 +++++++++++++-------\n>>   2 files changed, 34 insertions(+), 17 deletions(-)\n>>\n>> diff --git a/environment.c b/environment.c\n>> index f5628b6758..918d8b50b8 100644\n>> --- a/environment.c\n>> +++ b/environment.c\n>> @@ -745,31 +745,42 @@ int git_default_config(const char *var, const char *value,\n>>   \n>>   void repo_config_values_init(struct repo_config_values *cfg)\n>>   {\n>> +\t/* core */\n>>   \tcfg->attributes_file = NULL;\n>>   \tcfg->excludes_file = NULL;\n>>   \tcfg->editor_program = NULL;\n>>   \tcfg->pager_program = NULL;\n>>   \tcfg->askpass_program = NULL;\n>> -\tcfg->apply_default_whitespace = NULL;\n>> -\tcfg->apply_default_ignorewhitespace = NULL;\n>> -\tcfg->push_default = PUSH_DEFAULT_UNSPECIFIED;\n>> -\tcfg->autorebase = AUTOREBASE_NEVER;\n>>   \tcfg->object_creation_mode = OBJECT_CREATION_MODE;\n>>   \tcfg->apply_sparse_checkout = 0;\n>> +\tcfg->trust_ctime = 1;\n>> +\tcfg->check_stat = 1;\n>> +\tcfg->zlib_compression_level = Z_BEST_SPEED;\n>> +\tcfg->precomposed_unicode = -1;\n>> +\tcfg->core_sparse_checkout_cone = 0;\n>> +\tcfg->warn_on_object_refname_ambiguity = 1;\n>>   \tcfg->protect_hfs = PROTECT_HFS_DEFAULT;\n>>   \tcfg->protect_ntfs = PROTECT_NTFS_DEFAULT;\n>>   \tcfg->ignore_case = 0;\n>>   \tcfg->trust_executable_bit = 1;\n>>   \tcfg->has_symlinks = platform_has_symlinks();\n>> +\n>> +\t/* apply */\n>> +\tcfg->apply_default_whitespace = NULL;\n>> +\tcfg->apply_default_ignorewhitespace = NULL;\n>> +\n>> +\t/* branch */\n>> +\tcfg->autorebase = AUTOREBASE_NEVER;\n>>   \tcfg->branch_track = BRANCH_TRACK_REMOTE;\n>> -\tcfg->trust_ctime = 1;\n>> -\tcfg->check_stat = 1;\n>> -\tcfg->zlib_compression_level = Z_BEST_SPEED;\n>> +\n>> +\t/* pack */\n>>   \tcfg->pack_compression_level = Z_DEFAULT_COMPRESSION;\n>> -\tcfg->precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */\n>> -\tcfg->core_sparse_checkout_cone = 0;\n>> +\n>> +\t/* push */\n>> +\tcfg->push_default = PUSH_DEFAULT_UNSPECIFIED;\n>> +\n>> +\t/* sparse */\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 repo_config_values *cfg)\n>> diff --git a/environment.h b/environment.h\n>> index 30678257b5..52ed13c0fc 100644\n>> --- a/environment.h\n>> +++ b/environment.h\n>> @@ -121,16 +121,11 @@ struct repo_config_values {\n>>   \tchar *editor_program;\n>>   \tchar *pager_program;\n>>   \tchar *askpass_program;\n>> -\tchar *apply_default_whitespace;\n>> -\tchar *apply_default_ignorewhitespace;\n>> -\tenum push_default_type push_default;\n>> -\tenum rebase_setup_type autorebase;\n>>   \tenum object_creation_mode object_creation_mode;\n>>   \tint apply_sparse_checkout;\n>>   \tint trust_ctime;\n>>   \tint check_stat;\n>>   \tint zlib_compression_level;\n>> -\tint pack_compression_level;\n>>   \tint precomposed_unicode;\n>>   \tint core_sparse_checkout_cone;\n>>   \tint warn_on_object_refname_ambiguity;\n>> @@ -140,11 +135,22 @@ struct repo_config_values {\n>>   \tint trust_executable_bit;\n>>   \tint has_symlinks;\n>>   \n>> -\t/* section \"sparse\" config values */\n>> -\tint sparse_expect_files_outside_of_patterns;\n>> +\t/* section \"apply\" config values */\n>> +\tchar *apply_default_whitespace;\n>> +\tchar *apply_default_ignorewhitespace;\n>>   \n>>   \t/* section \"branch\" config values */\n>> +\tenum rebase_setup_type autorebase;\n>>   \tenum branch_track branch_track;\n>> +\n>> +\t/* section \"pack\" config values */\n>> +\tint pack_compression_level;\n>> +\n>> +\t/* section \"push\" config values */\n>> +\tenum push_default_type push_default;\n>> +\n>> +\t/* section \"sparse\" config values */\n>> +\tint sparse_expect_files_outside_of_patterns;\n>>   };\n>>   \n>>   struct repo_config_values *repo_config_values(struct repository *repo);\n\n"},{"id":"549810","messageId":"c39c51d7-07bf-42f7-8b26-47dd9ef0e5b3@malon.dev","threadId":"66122","inReplyTo":"xmqqtsp8nt7i.fsf@gitster.g","subject":"Re: [PATCH 2/3] environment: clarify repository config getter documentation","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-06T08:49:07Z","receivedAt":"2026-08-06T08:49:17Z","isPatch":true,"body":"On 8/6/26 05:38, Junio C Hamano wrote:\n> Tian Yuchen <cat@malon.dev> writes:\n> \n>> Update the comment above repository config getters to describe their\n>> common behavior.\n>>\n>> The getters handle repositories that are not fully initialized by\n>> returning the corresponding default values.\n>>\n>> Mentored-by: Christian Couder <christian.couder@gmail.com>\n>> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n>> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>\n>> Signed-off-by: Tian Yuchen <cat@malon.dev>\n>> ---\n>>   environment.h | 11 +++--------\n>>   1 file changed, 3 insertions(+), 8 deletions(-)\n>>\n>> diff --git a/environment.h b/environment.h\n>> index e7ec5b0437..30678257b5 100644\n>> --- a/environment.h\n>> +++ b/environment.h\n>> @@ -175,18 +175,13 @@ 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>> - * Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.\n>> - * They check `repo->initialized` to prevent calling `repo_config_values()`\n>> - * before the repository setup is fully complete or in non-git environments.\n>> + * Getters for configuration variables in `struct repo_config_values`.\n>> + * These functions handle uninitialized repositories or non-git\n>> + * environments by returning appropriate default values.\n>>    */\n>>   int repo_protect_hfs(struct repository *repo);\n>>   int repo_protect_ntfs(struct repository *repo);\n>>   \n>> -/*\n> \n> Two puzzlements.\n> \n>   * Is the above comment block meant to apply to repo_ignore_case()\n>     in addition to repo_protect_ntfs() and repo_protect_hfs()?  If\n>     so, the blank line before repo_ignore_case() is a bit misleading.\n> \n\nNot really, they are meant to apply to all getters below. I will remove \nthe blank lines.\n\n>   * The phrase \"uninitialized repositories or non-Git environments\"\n>     strongly hints that I can pass NULL to indicate that we are\n>     running in a non-Git environment.  However, the change in\n>     [PATCH 1/3] we just saw means I would get a segfault if I did so,\n>     does it not?\n> \n\nThis is a mistake. I meant \"these getters can handle repositories, even \nwhen they are not fully initailzed\" but not \"these getters can handle \nwhatever we pass in\". So I will change it in the next reroll.\n\n>> - * Getter for the `ignore_case` field of `struct repo_config_values`.\n>> - * It checks `repo->initialized` to prevent calling repo_config_values()`\n>> - * before the repository setup is fully complete or in non-git environments.\n>> - */\n>>   int repo_ignore_case(struct repository *repo);\n>>   \n>>   int repo_trust_executable_bit(struct repository *repo);\n\nThanks! yuchen\n\n"},{"id":"549812","messageId":"20260806092557.3951208-1-cat@malon.dev","threadId":"66122","inReplyTo":"20260805115342.3939931-1-cat@malon.dev","subject":"[PATCH v2 0/3] environment: clean up repository config handling","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-06T09:25:54Z","receivedAt":"2026-08-06T09:26:21Z","isPatch":true,"body":"Hi all,\n\nThis series contains several cleanup patches for repository configuration\nhandling.\n\nNo functional changes are intended. The patches make the related code\nmore consistent and easier to maintain by improving documentation,\nformatting, and the organization of repo_config_values.\n\nRFC:\nIf there are other small cleanups in this area that would be useful to\ninclude, suggestions are welcome.\n\nRegards, yuchen\n\nChanges since v1:\n\n - in commit 2/3, drop several blank lines to group the getters under the\n comment. Note that the comment does not apply to repo_excludes_file.\n\n - in commit 3/3, do not change the order of the members. Instead, drop\n the original comments that do not accurately categorize them.\n\nTian Yuchen (3):\n  environment: simplify repository config getters\n  environment: clarify repository config getter documentation\n  environment: remove inaccurate repo_config_values comments\n\n environment.c | 18 +++++++++---------\n environment.h | 19 +++----------------\n 2 files changed, 12 insertions(+), 25 deletions(-)\n\n-- \n2.43.0\n\n"},{"id":"549813","messageId":"20260806092557.3951208-2-cat@malon.dev","threadId":"66122","inReplyTo":"20260806092557.3951208-1-cat@malon.dev","subject":"[PATCH v2 1/3] environment: simplify repository config getters","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-06T09:25:55Z","receivedAt":"2026-08-06T09:26:26Z","isPatch":true,"body":"Drop unnecessary parentheses and NULL checks in repository config\ngetters.\n\nThese getters are only used with non-NULL repositories, so the\nextra checks do not match their current callers.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\nMentored-by: Olamide Caleb Bello <belkid98@gmail.com>\nSigned-off-by: Tian Yuchen <cat@malon.dev>\n---\n environment.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/environment.c b/environment.c\nindex 76ee65e62b..f5628b6758 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -119,23 +119,23 @@ int is_bare_repository(struct repository *repo)\n \n int repo_protect_ntfs(struct repository *repo)\n {\n-\treturn (repo && repo->initialized) ?\n-\t\trepo_config_values(repo)->protect_ntfs :\n-\t\tPROTECT_NTFS_DEFAULT;\n+\treturn repo->initialized\n+\t\t? repo_config_values(repo)->protect_ntfs\n+\t\t: PROTECT_NTFS_DEFAULT;\n }\n \n int repo_protect_hfs(struct repository *repo)\n {\n-\treturn (repo && repo->initialized) ?\n-\t\trepo_config_values(repo)->protect_hfs :\n-\t\tPROTECT_HFS_DEFAULT;\n+\treturn repo->initialized\n+\t\t? repo_config_values(repo)->protect_hfs\n+\t\t: PROTECT_HFS_DEFAULT;\n }\n \n int repo_ignore_case(struct repository *repo)\n {\n-\treturn (repo && repo->initialized) ?\n-\t\trepo_config_values(repo)->ignore_case :\n-\t\t0;\n+\treturn repo->initialized\n+\t\t? repo_config_values(repo)->ignore_case\n+\t\t: 0;\n }\n \n int repo_trust_executable_bit(struct repository *repo)\n-- \n2.43.0\n\n"},{"id":"549814","messageId":"20260806092557.3951208-3-cat@malon.dev","threadId":"66122","inReplyTo":"20260806092557.3951208-1-cat@malon.dev","subject":"[PATCH v2 2/3] environment: clarify repository config getter documentation","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-06T09:25:56Z","receivedAt":"2026-08-06T09:26:29Z","isPatch":true,"body":"Update the comment above repository config getters to describe their\ncommon behavior.\n\nThe getters handle repositories that are not fully initialized by\nreturning the corresponding default values.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\nMentored-by: Olamide Caleb Bello <belkid98@gmail.com>\nSigned-off-by: Tian Yuchen <cat@malon.dev>\n---\n environment.h | 14 +++-----------\n 1 file changed, 3 insertions(+), 11 deletions(-)\n\ndiff --git a/environment.h b/environment.h\nindex e7ec5b0437..1a58b553b5 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -175,22 +175,14 @@ 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- * Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.\n- * They check `repo->initialized` to prevent calling `repo_config_values()`\n- * before the repository setup is fully complete or in non-git environments.\n+ * Getters for configuration variables in `struct repo_config_values`.\n+ * These functions handle repositories that are not fully initialized\n+ * by returning appropriate default values.\n  */\n int repo_protect_hfs(struct repository *repo);\n int repo_protect_ntfs(struct repository *repo);\n-\n-/*\n- * Getter for the `ignore_case` field of `struct repo_config_values`.\n- * It checks `repo->initialized` to prevent calling repo_config_values()`\n- * before the repository setup is fully complete or in non-git environments.\n- */\n int repo_ignore_case(struct repository *repo);\n-\n int repo_trust_executable_bit(struct repository *repo);\n-\n int repo_has_symlinks(struct repository *repo);\n \n const char *repo_excludes_file(struct repository *repo);\n-- \n2.43.0\n\n"},{"id":"549815","messageId":"20260806092557.3951208-4-cat@malon.dev","threadId":"66122","inReplyTo":"20260806092557.3951208-1-cat@malon.dev","subject":"[PATCH v2 3/3] environment: remove inaccurate repo_config_values comments","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-06T09:25:57Z","receivedAt":"2026-08-06T09:26:32Z","isPatch":true,"body":"The section comments in struct repo_config_values do not accurately\ndescribe all members grouped under them. Remove them rather than implying\na relationship that does not exist.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\nMentored-by: Olamide Caleb Bello <belkid98@gmail.com>\nSigned-off-by: Tian Yuchen <cat@malon.dev>\n---\n environment.h | 5 -----\n 1 file changed, 5 deletions(-)\n\ndiff --git a/environment.h b/environment.h\nindex 1a58b553b5..ab52330159 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -115,7 +115,6 @@ enum object_creation_mode {\n };\n \n struct repo_config_values {\n-\t/* section \"core\" config values */\n \tchar *attributes_file;\n \tchar *excludes_file;\n \tchar *editor_program;\n@@ -139,11 +138,7 @@ struct repo_config_values {\n \tint ignore_case;\n \tint trust_executable_bit;\n \tint has_symlinks;\n-\n-\t/* section \"sparse\" config values */\n \tint sparse_expect_files_outside_of_patterns;\n-\n-\t/* section \"branch\" config values */\n \tenum branch_track branch_track;\n };\n \n-- \n2.43.0\n\n"},{"id":"549863","messageId":"xmqq5x1nmc90.fsf@gitster.g","threadId":"66122","inReplyTo":"dbcbb042-5c50-4569-9b18-3edcc7b1ef4b@malon.dev","subject":"Re: [PATCH 3/3] environment: reorder variables in repo_config_values structure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-06T16:42:35Z","receivedAt":"2026-08-06T16:42:38Z","isPatch":true,"body":"Tian Yuchen <cat@malon.dev> writes:\n\n> On 8/6/26 05:47, Junio C Hamano wrote:\n>> Tian Yuchen <cat@malon.dev> writes:\n>> \n>>> Reorder the fields in struct repo_config_values and its initialization\n>>> function to follow the order of configuration sections.\n>>>\n>>> Keeping the declaration and initialization order aligned makes the\n>>> structure easier to review and maintain.\n>> \n>> Really?\n>> \n>> Do you have some automated tool to make sure these initialization\n>> assignments in the environment.c file and declaration in the\n>> environment.h file match the order in Documentation/config/*.adoc or\n>> something else?  Have you designated some list as the authoritative\n>> source of truth to check these against?  Without such a list to\n>> check the code against and a mechanism to enforce the ordering, I\n>> find it hard to agree with such a claim that this makes it easier to\n>> maintain.\n>\n> I see.\n>\n>> \n>> It is typical to list the structure members in the order of stricter\n>> to looser alignment requirement of their types.  I do not know how\n>> strictly it is followed for \"struct repo_config_values\", but by\n>> spreading pointer valued members more widely with smaller enums in\n>> between, the change certainly is making the overall structure size\n>> larger by requiring more padding between the members with different\n>> alignment requirements.  Not that we would have 100s of instances of\n>> these structures.\n>> \n>\n> Oh, I overlooked the size issue. Thanks for pointing out.\n\nI didn't mean to \"point out\" any size issue.  As I said, it is not\nlike we have hundreds of these, so padding bloat here and there\nwould not matter and if we get a readability boost by reordering\ninto a sensible order, that by itself could be a win.\n\n"},{"id":"549864","messageId":"xmqqv79nkxc3.fsf@gitster.g","threadId":"66122","inReplyTo":"20260806092557.3951208-2-cat@malon.dev","subject":"Re: [PATCH v2 1/3] environment: simplify repository config getters","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-06T16:50:04Z","receivedAt":"2026-08-06T16:50:10Z","isPatch":true,"body":"Tian Yuchen <cat@malon.dev> writes:\n\n> Drop unnecessary parentheses and NULL checks in repository config\n> getters.\n>\n> These getters are only used with non-NULL repositories, so the\n> extra checks do not match their current callers.\n\nYou would need to explain why it is sensible to enforce on future\ncallers the same rule that current callers honor, or why it is\nunlikely that we will gain any more callers in the future (which\nwould justify catering only to current callers).\n\n> Mentored-by: Christian Couder <christian.couder@gmail.com>\n> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>\n> Signed-off-by: Tian Yuchen <cat@malon.dev>\n> ---\n>  environment.c | 18 +++++++++---------\n>  1 file changed, 9 insertions(+), 9 deletions(-)\n>\n> diff --git a/environment.c b/environment.c\n> index 76ee65e62b..f5628b6758 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -119,23 +119,23 @@ int is_bare_repository(struct repository *repo)\n>  \n>  int repo_protect_ntfs(struct repository *repo)\n>  {\n> -\treturn (repo && repo->initialized) ?\n> -\t\trepo_config_values(repo)->protect_ntfs :\n> -\t\tPROTECT_NTFS_DEFAULT;\n> +\treturn repo->initialized\n> +\t\t? repo_config_values(repo)->protect_ntfs\n> +\t\t: PROTECT_NTFS_DEFAULT;\n>  }\n>  \n>  int repo_protect_hfs(struct repository *repo)\n>  {\n> -\treturn (repo && repo->initialized) ?\n> -\t\trepo_config_values(repo)->protect_hfs :\n> -\t\tPROTECT_HFS_DEFAULT;\n> +\treturn repo->initialized\n> +\t\t? repo_config_values(repo)->protect_hfs\n> +\t\t: PROTECT_HFS_DEFAULT;\n>  }\n>  \n>  int repo_ignore_case(struct repository *repo)\n>  {\n> -\treturn (repo && repo->initialized) ?\n> -\t\trepo_config_values(repo)->ignore_case :\n> -\t\t0;\n> +\treturn repo->initialized\n> +\t\t? repo_config_values(repo)->ignore_case\n> +\t\t: 0;\n>  }\n>  \n>  int repo_trust_executable_bit(struct repository *repo)\n"},{"id":"549865","messageId":"xmqqpkzvkx54.fsf@gitster.g","threadId":"66122","inReplyTo":"20260806092557.3951208-3-cat@malon.dev","subject":"Re: [PATCH v2 2/3] environment: clarify repository config getter documentation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-06T16:54:15Z","receivedAt":"2026-08-06T16:54:17Z","isPatch":true,"body":"Tian Yuchen <cat@malon.dev> writes:\n\n> Update the comment above repository config getters to describe their\n> common behavior.\n>\n> The getters handle repositories that are not fully initialized by\n> returning the corresponding default values.\n>\n> Mentored-by: Christian Couder <christian.couder@gmail.com>\n> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>\n> Signed-off-by: Tian Yuchen <cat@malon.dev>\n> ---\n>  environment.h | 14 +++-----------\n>  1 file changed, 3 insertions(+), 11 deletions(-)\n>\n> diff --git a/environment.h b/environment.h\n> index e7ec5b0437..1a58b553b5 100644\n> --- a/environment.h\n> +++ b/environment.h\n> @@ -175,22 +175,14 @@ 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> - * Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.\n> - * They check `repo->initialized` to prevent calling `repo_config_values()`\n> - * before the repository setup is fully complete or in non-git environments.\n> + * Getters for configuration variables in `struct repo_config_values`.\n> + * These functions handle repositories that are not fully initialized\n> + * by returning appropriate default values.\n>   */\n\nDo we also want to mention that calling them when the caller is\noutside a repository is an error, or is it obvious enough?\n\n>  int repo_protect_hfs(struct repository *repo);\n>  int repo_protect_ntfs(struct repository *repo);\n> -\n> -/*\n> - * Getter for the `ignore_case` field of `struct repo_config_values`.\n> - * It checks `repo->initialized` to prevent calling repo_config_values()`\n> - * before the repository setup is fully complete or in non-git environments.\n> - */\n>  int repo_ignore_case(struct repository *repo);\n> -\n>  int repo_trust_executable_bit(struct repository *repo);\n> -\n>  int repo_has_symlinks(struct repository *repo);\n>  \n>  const char *repo_excludes_file(struct repository *repo);\n"},{"id":"549960","messageId":"dc22396e-21b8-442f-a93d-f49e7af5e99a@malon.dev","threadId":"66122","inReplyTo":"xmqq5x1nmc90.fsf@gitster.g","subject":"Re: [PATCH 3/3] environment: reorder variables in repo_config_values structure","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-07T08:26:15Z","receivedAt":"2026-08-07T08:26:31Z","isPatch":true,"body":"On 8/7/26 00:42, Junio C Hamano wrote:\n> Tian Yuchen <cat@malon.dev> writes:\n> \n>> On 8/6/26 05:47, Junio C Hamano wrote:\n>>> Tian Yuchen <cat@malon.dev> writes:\n>>>\n>>>> Reorder the fields in struct repo_config_values and its initialization\n>>>> function to follow the order of configuration sections.\n>>>>\n>>>> Keeping the declaration and initialization order aligned makes the\n>>>> structure easier to review and maintain.\n>>>\n>>> Really?\n>>>\n>>> Do you have some automated tool to make sure these initialization\n>>> assignments in the environment.c file and declaration in the\n>>> environment.h file match the order in Documentation/config/*.adoc or\n>>> something else?  Have you designated some list as the authoritative\n>>> source of truth to check these against?  Without such a list to\n>>> check the code against and a mechanism to enforce the ordering, I\n>>> find it hard to agree with such a claim that this makes it easier to\n>>> maintain.\n>>\n>> I see.\n>>\n>>>\n>>> It is typical to list the structure members in the order of stricter\n>>> to looser alignment requirement of their types.  I do not know how\n>>> strictly it is followed for \"struct repo_config_values\", but by\n>>> spreading pointer valued members more widely with smaller enums in\n>>> between, the change certainly is making the overall structure size\n>>> larger by requiring more padding between the members with different\n>>> alignment requirements.  Not that we would have 100s of instances of\n>>> these structures.\n>>>\n>>\n>> Oh, I overlooked the size issue. Thanks for pointing out.\n> \n> I didn't mean to \"point out\" any size issue.  As I said, it is not\n> like we have hundreds of these, so padding bloat here and there\n> would not matter and if we get a readability boost by reordering\n> into a sensible order, that by itself could be a win.\n> \n\nOkay.\n\nAs you said before, the boost on readability seems to be limited. \nReordering by config section is not a strong maintenance rule without an \nauthoritative source. So let's don't reorder them anyways.\n\nThanks, yuchen\n"},{"id":"549961","messageId":"46b0a9fd-ce30-4110-bd9f-b315ab4a09ce@malon.dev","threadId":"66122","inReplyTo":"xmqqv79nkxc3.fsf@gitster.g","subject":"Re: [PATCH v2 1/3] environment: simplify repository config getters","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-07T08:30:30Z","receivedAt":"2026-08-07T08:30:42Z","isPatch":true,"body":"On 8/7/26 00:50, Junio C Hamano wrote:\n> Tian Yuchen <cat@malon.dev> writes:\n> \n>> Drop unnecessary parentheses and NULL checks in repository config\n>> getters.\n>>\n>> These getters are only used with non-NULL repositories, so the\n>> extra checks do not match their current callers.\n> \n> You would need to explain why it is sensible to enforce on future\n> callers the same rule that current callers honor, or why it is\n> unlikely that we will gain any more callers in the future (which\n> would justify catering only to current callers).\n> \n>> Mentored-by: Christian Couder <christian.couder@gmail.com>\n>> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\n>> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>\n>> Signed-off-by: Tian Yuchen <cat@malon.dev>\n>> ---\n>>   environment.c | 18 +++++++++---------\n>>   1 file changed, 9 insertions(+), 9 deletions(-)\n>>\n>> diff --git a/environment.c b/environment.c\n>> index 76ee65e62b..f5628b6758 100644\n>> --- a/environment.c\n>> +++ b/environment.c\n>> @@ -119,23 +119,23 @@ int is_bare_repository(struct repository *repo)\n>>   \n>>   int repo_protect_ntfs(struct repository *repo)\n>>   {\n>> -\treturn (repo && repo->initialized) ?\n>> -\t\trepo_config_values(repo)->protect_ntfs :\n>> -\t\tPROTECT_NTFS_DEFAULT;\n>> +\treturn repo->initialized\n>> +\t\t? repo_config_values(repo)->protect_ntfs\n>> +\t\t: PROTECT_NTFS_DEFAULT;\n>>   }\n>>   \n>>   int repo_protect_hfs(struct repository *repo)\n>>   {\n>> -\treturn (repo && repo->initialized) ?\n>> -\t\trepo_config_values(repo)->protect_hfs :\n>> -\t\tPROTECT_HFS_DEFAULT;\n>> +\treturn repo->initialized\n>> +\t\t? repo_config_values(repo)->protect_hfs\n>> +\t\t: PROTECT_HFS_DEFAULT;\n>>   }\n>>   \n>>   int repo_ignore_case(struct repository *repo)\n>>   {\n>> -\treturn (repo && repo->initialized) ?\n>> -\t\trepo_config_values(repo)->ignore_case :\n>> -\t\t0;\n>> +\treturn repo->initialized\n>> +\t\t? repo_config_values(repo)->ignore_case\n>> +\t\t: 0;\n>>   }\n>>   \n>>   int repo_trust_executable_bit(struct repository *repo)\n\nI see, will change the commit message then ;)\n\nThanks, yuchen\n"},{"id":"549962","messageId":"20260807085932.3958759-1-cat@malon.dev","threadId":"66122","inReplyTo":"20260805115342.3939931-1-cat@malon.dev","subject":"[PATCH v3 0/3] environment: clean up repository config handling","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-07T08:59:29Z","receivedAt":"2026-08-07T08:59:46Z","isPatch":true,"body":"Hi all,\n\nThis series contains several cleanup patches for repository configuration\nhandling.\n\nNo functional changes are intended. The patches make the related code\nmore consistent and easier to maintain by improving documentation,\nformatting, and the organization of repo_config_values.\n\nRFC:\nIf there are other small cleanups in this area that would be useful to\ninclude, suggestions are welcome.\n\nRegards, yuchen\n\nChanges since v2:\n\n - in the commit message of patch 1/3, explain why NULL repository is\n not allowed.\n\n - in patch 2/3, mention in the comment that NULL repository shouldn't\n be passed in.\n\nTian Yuchen (3):\n  environment: drop redundant NULL checks in config getters\n  environment: clarify repository config getter documentation\n  environment: remove inaccurate repo_config_values comments\n\n environment.c | 18 +++++++++---------\n environment.h | 20 ++++----------------\n 2 files changed, 13 insertions(+), 25 deletions(-)\n\n-- \n2.43.0\n\n"},{"id":"549963","messageId":"20260807085932.3958759-2-cat@malon.dev","threadId":"66122","inReplyTo":"20260807085932.3958759-1-cat@malon.dev","subject":"[PATCH v3 1/3] environment: drop redundant NULL checks in config getters","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-07T08:59:30Z","receivedAt":"2026-08-07T08:59:48Z","isPatch":true,"body":"These repository config getters require a valid repository pointer.\nWhile an uninitialized repository is a valid state and is handled by\nreturning default values, passing NULL is a programming error.\n\nDrop the NULL checks so that invalid callers are not silently accepted.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\nMentored-by: Olamide Caleb Bello <belkid98@gmail.com>\nSigned-off-by: Tian Yuchen <cat@malon.dev>\n---\n environment.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/environment.c b/environment.c\nindex 76ee65e62b..f5628b6758 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -119,23 +119,23 @@ int is_bare_repository(struct repository *repo)\n \n int repo_protect_ntfs(struct repository *repo)\n {\n-\treturn (repo && repo->initialized) ?\n-\t\trepo_config_values(repo)->protect_ntfs :\n-\t\tPROTECT_NTFS_DEFAULT;\n+\treturn repo->initialized\n+\t\t? repo_config_values(repo)->protect_ntfs\n+\t\t: PROTECT_NTFS_DEFAULT;\n }\n \n int repo_protect_hfs(struct repository *repo)\n {\n-\treturn (repo && repo->initialized) ?\n-\t\trepo_config_values(repo)->protect_hfs :\n-\t\tPROTECT_HFS_DEFAULT;\n+\treturn repo->initialized\n+\t\t? repo_config_values(repo)->protect_hfs\n+\t\t: PROTECT_HFS_DEFAULT;\n }\n \n int repo_ignore_case(struct repository *repo)\n {\n-\treturn (repo && repo->initialized) ?\n-\t\trepo_config_values(repo)->ignore_case :\n-\t\t0;\n+\treturn repo->initialized\n+\t\t? repo_config_values(repo)->ignore_case\n+\t\t: 0;\n }\n \n int repo_trust_executable_bit(struct repository *repo)\n-- \n2.43.0\n\n"},{"id":"549964","messageId":"20260807085932.3958759-3-cat@malon.dev","threadId":"66122","inReplyTo":"20260807085932.3958759-1-cat@malon.dev","subject":"[PATCH v3 2/3] environment: clarify repository config getter documentation","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-07T08:59:31Z","receivedAt":"2026-08-07T08:59:51Z","isPatch":true,"body":"Update the comment above repository config getters to describe their\ncommon behavior.\n\nThe getters handle repositories that are not fully initialized by\nreturning the corresponding default values.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\nMentored-by: Olamide Caleb Bello <belkid98@gmail.com>\nSigned-off-by: Tian Yuchen <cat@malon.dev>\n---\n environment.h | 15 ++++-----------\n 1 file changed, 4 insertions(+), 11 deletions(-)\n\ndiff --git a/environment.h b/environment.h\nindex e7ec5b0437..6f864c1635 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -175,22 +175,15 @@ 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- * Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.\n- * They check `repo->initialized` to prevent calling `repo_config_values()`\n- * before the repository setup is fully complete or in non-git environments.\n+ * Getters for configuration variables in `struct repo_config_values`.\n+ * These functions require a non-NULL repository pointer and handle\n+ * repositories that are not fully initialized by returning appropriate\n+ * default values.\n  */\n int repo_protect_hfs(struct repository *repo);\n int repo_protect_ntfs(struct repository *repo);\n-\n-/*\n- * Getter for the `ignore_case` field of `struct repo_config_values`.\n- * It checks `repo->initialized` to prevent calling repo_config_values()`\n- * before the repository setup is fully complete or in non-git environments.\n- */\n int repo_ignore_case(struct repository *repo);\n-\n int repo_trust_executable_bit(struct repository *repo);\n-\n int repo_has_symlinks(struct repository *repo);\n \n const char *repo_excludes_file(struct repository *repo);\n-- \n2.43.0\n\n"},{"id":"549965","messageId":"20260807085932.3958759-4-cat@malon.dev","threadId":"66122","inReplyTo":"20260807085932.3958759-1-cat@malon.dev","subject":"[PATCH v3 3/3] environment: remove inaccurate repo_config_values comments","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-08-07T08:59:32Z","receivedAt":"2026-08-07T08:59:54Z","isPatch":true,"body":"The section comments in struct repo_config_values do not accurately\ndescribe all members grouped under them. Remove them rather than implying\na relationship that does not exist.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>\nMentored-by: Olamide Caleb Bello <belkid98@gmail.com>\nSigned-off-by: Tian Yuchen <cat@malon.dev>\n---\n environment.h | 5 -----\n 1 file changed, 5 deletions(-)\n\ndiff --git a/environment.h b/environment.h\nindex 6f864c1635..67fd387d35 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -115,7 +115,6 @@ enum object_creation_mode {\n };\n \n struct repo_config_values {\n-\t/* section \"core\" config values */\n \tchar *attributes_file;\n \tchar *excludes_file;\n \tchar *editor_program;\n@@ -139,11 +138,7 @@ struct repo_config_values {\n \tint ignore_case;\n \tint trust_executable_bit;\n \tint has_symlinks;\n-\n-\t/* section \"sparse\" config values */\n \tint sparse_expect_files_outside_of_patterns;\n-\n-\t/* section \"branch\" config values */\n \tenum branch_track branch_track;\n };\n \n-- \n2.43.0\n\n"},{"id":"549981","messageId":"anW7wHfUxYj9cj0P@pks.im","threadId":"66122","inReplyTo":"20260807085932.3958759-1-cat@malon.dev","subject":"Re: [PATCH v3 0/3] environment: clean up repository config handling","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-07T11:04:32Z","receivedAt":"2026-08-07T11:04:39Z","isPatch":true,"body":"On Fri, Aug 07, 2026 at 04:59:29PM +0800, Tian Yuchen wrote:\n> Hi all,\n> \n> This series contains several cleanup patches for repository configuration\n> handling.\n> \n> No functional changes are intended. The patches make the related code\n> more consistent and easier to maintain by improving documentation,\n> formatting, and the organization of repo_config_values.\n> \n> RFC:\n> If there are other small cleanups in this area that would be useful to\n> include, suggestions are welcome.\n\nSomewhat unrelated to this patch series, but I was wondering whether you\nplan to drop the limitation in `repo_config_values()` that requires that\nthe passed-in repository is `the_repository`. This limitation is\nstarting to create problems as more and more of our infrastructure is\nmigrating into `struct repo_config_values`, so using a different repo\nthan `the_repository` is starting to become harder and harder in our\ncodebase.\n\nThanks!\n\nPatrick\n"},{"id":"550052","messageId":"xmqq1pc9eivn.fsf@gitster.g","threadId":"66122","inReplyTo":"anW7wHfUxYj9cj0P@pks.im","subject":"Re: [PATCH v3 0/3] environment: clean up repository config handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-07T21:11:08Z","receivedAt":"2026-08-07T21:11:11Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Fri, Aug 07, 2026 at 04:59:29PM +0800, Tian Yuchen wrote:\n>> Hi all,\n>> \n>> This series contains several cleanup patches for repository configuration\n>> handling.\n>> \n>> No functional changes are intended. The patches make the related code\n>> more consistent and easier to maintain by improving documentation,\n>> formatting, and the organization of repo_config_values.\n>> \n>> RFC:\n>> If there are other small cleanups in this area that would be useful to\n>> include, suggestions are welcome.\n>\n> Somewhat unrelated to this patch series, but I was wondering whether you\n> plan to drop the limitation in `repo_config_values()` that requires that\n> the passed-in repository is `the_repository`. This limitation is\n> starting to create problems as more and more of our infrastructure is\n> migrating into `struct repo_config_values`, so using a different repo\n> than `the_repository` is starting to become harder and harder in our\n> codebase.\n>\n> Thanks!\n>\n> Patrick\n\nHmph, that is an interesting point.  What is our plan to really\nenable the use of repository instances other than 'the_repository'\nhere?  They of course need to be initialized with repo_init(),\nbut is that enough to sensibly use the embedded 'repo_settings'\nand 'repo_config_values' structures?  (By the way, it is not\nentirely clear to me why we need both and how we sift variables\nbetween them.)  Some code paths need to work outside a repository\nand still need to know about per-user or per-system settings.\nWe were perfectly happy reading from global variables when we had\nthe majority of them there.  It is my understanding that they are\nnow found in 'repo_config_values' or 'repo_settings' associated\nwith 'the_repository', which I think is something we cannot\nreally avoid doing.  Unless we try to get rid of 'the_repository'\nand instead have free-standing 'repo_settings' and\n'repo_config_values' structures that are not tied to any\nrepository instance, we are back to depending on a set of global\nvariables. 😞\n\nIn any case, all of that has little to do with this series, I\nsuspect, unless we are redesigning these configurations and\nsettings in such a way that they are not necessarily tied to\nany repository instance.  While I do not know the exact details,\nI can imagine a hierarchical system where system- and\nuser-wide sets of setting values are known independently of any\nrepository, only to be overridden by per-repository settings\nusing a last-one-wins strategy at lookup time.\n"},{"id":"550155","messageId":"anlmwaEtwcCPse1N@pks.im","threadId":"66122","inReplyTo":"xmqq1pc9eivn.fsf@gitster.g","subject":"Re: [PATCH v3 0/3] environment: clean up repository config handling","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-10T05:50:57Z","receivedAt":"2026-08-10T05:51:04Z","isPatch":true,"body":"On Fri, Aug 07, 2026 at 02:11:08PM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > On Fri, Aug 07, 2026 at 04:59:29PM +0800, Tian Yuchen wrote:\n> >> Hi all,\n> >> \n> >> This series contains several cleanup patches for repository configuration\n> >> handling.\n> >> \n> >> No functional changes are intended. The patches make the related code\n> >> more consistent and easier to maintain by improving documentation,\n> >> formatting, and the organization of repo_config_values.\n> >> \n> >> RFC:\n> >> If there are other small cleanups in this area that would be useful to\n> >> include, suggestions are welcome.\n> >\n> > Somewhat unrelated to this patch series, but I was wondering whether you\n> > plan to drop the limitation in `repo_config_values()` that requires that\n> > the passed-in repository is `the_repository`. This limitation is\n> > starting to create problems as more and more of our infrastructure is\n> > migrating into `struct repo_config_values`, so using a different repo\n> > than `the_repository` is starting to become harder and harder in our\n> > codebase.\n> >\n> > Thanks!\n> >\n> > Patrick\n> \n> Hmph, that is an interesting point.  What is our plan to really\n> enable the use of repository instances other than 'the_repository'\n> here?  They of course need to be initialized with repo_init(),\n> but is that enough to sensibly use the embedded 'repo_settings'\n> and 'repo_config_values' structures?  (By the way, it is not\n> entirely clear to me why we need both and how we sift variables\n> between them.)\n\nYeah, this split is adding to the confusion indeed. I think that we\nshould make it a goal to unify those going forward.\n\n[snip]\n> In any case, all of that has little to do with this series, I\n> suspect, unless we are redesigning these configurations and\n> settings in such a way that they are not necessarily tied to\n> any repository instance.  While I do not know the exact details,\n> I can imagine a hierarchical system where system- and\n> user-wide sets of setting values are known independently of any\n> repository, only to be overridden by per-repository settings\n> using a last-one-wins strategy at lookup time.\n\nI've been wondering for a while whether we're operating at the wrong\nlevel here. Both `repo_settings` and `repo_config_values` indicates that\nwe're operating in the context of a repository, but as you mention that\nmay not even be the case.\n\nI don't think the approach is inherently flawed though. From my point\nof view, the best way forward is to merge those two and then generalize\nthem into something like `git_config_values` or `git_settings`,\ndepending on which of both variants we want to retain. We would then\nhave two levels:\n\n  - One on the repository level as we have it today.\n\n  - One truly global variable, because that stuff in fact _is_ global.\n\nWe'd then adapt `repo_config_values()` so that it knows to populate\neither of those variables depending on whether or not the user passes a\nvalid repository, and returns a constant pointer to the respective\nstructure. Callers MUST NOT modify that structure -- if they want to,\nthey'll have to make a copy and pass it down the calling stack.\n\nThe last part about not modifying that structure could be quite a bit\npainful though, as it would mean that we might have to adapt call chains\nto pass down a `struct git_config_values` instead of a `struct\nrepository`. But arguably, that's the right thing to do anyway for at\nleast some subsystems that are independent of repositories.\n\nAs you say though, none of this is really related to this patch series\nat hand, and I don't think we need to resolve this discussion before we\ncan merge it. I just want to make sure that we have a plan for how to\nget rid of `the_repository` instead of only shuffling stuff around.\n\nPatrick\n"},{"id":"551311","messageId":"xmqq5x0wfyf9.fsf@gitster.g","threadId":"66122","inReplyTo":"anlmwaEtwcCPse1N@pks.im","subject":"Re: [PATCH v3 0/3] environment: clean up repository config handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-26T19:56:42Z","receivedAt":"2026-08-26T19:56:45Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Fri, Aug 07, 2026 at 02:11:08PM -0700, Junio C Hamano wrote:\n>> Patrick Steinhardt <ps@pks.im> writes:\n>> \n>> > On Fri, Aug 07, 2026 at 04:59:29PM +0800, Tian Yuchen wrote:\n>> >> Hi all,\n>> >> \n>> >> This series contains several cleanup patches for repository configuration\n>> >> handling.\n>> >> \n>> >> No functional changes are intended. The patches make the related code\n>> >> more consistent and easier to maintain by improving documentation,\n>> >> formatting, and the organization of repo_config_values.\n>> >> \n>> >> RFC:\n>> >> If there are other small cleanups in this area that would be useful to\n>> >> include, suggestions are welcome.\n>> >\n>> > Somewhat unrelated to this patch series, but I was wondering whether you\n>> > plan to drop the limitation in `repo_config_values()` that requires that\n>> > the passed-in repository is `the_repository`. This limitation is\n>> > starting to create problems as more and more of our infrastructure is\n>> > migrating into `struct repo_config_values`, so using a different repo\n>> > than `the_repository` is starting to become harder and harder in our\n>> > codebase.\n>> >\n>> > Thanks!\n>> >\n>> > Patrick\n>> \n>> Hmph, that is an interesting point.  What is our plan to really\n>> ...\n> Yeah, this split is adding to the confusion indeed. I think that we\n> should make it a goal to unify those going forward.\n> ...\n> The last part about not modifying that structure could be quite a bit\n> painful though, as it would mean that we might have to adapt call chains\n> to pass down a `struct git_config_values` instead of a `struct\n> repository`. But arguably, that's the right thing to do anyway for at\n> least some subsystems that are independent of repositories.\n>\n> As you say though, none of this is really related to this patch series\n> at hand, and I don't think we need to resolve this discussion before we\n> can merge it. I just want to make sure that we have a plan for how to\n> get rid of `the_repository` instead of only shuffling stuff around.\n\nWell, after this sort-of offtopic exchange, the thread went dark.\n\nIs anybody interested in reviewing these patches and move the topic\nforward?\n\nThanks.\n"},{"id":"552516","messageId":"aqOeHlPWer60LcoO@pks.im","threadId":"66122","inReplyTo":"20260807085932.3958759-2-cat@malon.dev","subject":"Re: [PATCH v3 1/3] environment: drop redundant NULL checks in config getters","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-11T06:22:22Z","receivedAt":"2026-09-11T06:22:42Z","isPatch":true,"body":"On Fri, Aug 07, 2026 at 04:59:30PM +0800, Tian Yuchen wrote:\n> These repository config getters require a valid repository pointer.\n> While an uninitialized repository is a valid state and is handled by\n> returning default values, passing NULL is a programming error.\n> \n> Drop the NULL checks so that invalid callers are not silently accepted.\n\nI'm not quite convinced that having these checks in the first place is a\ngood idea. The single biggest problem is that we silently ignore the\nsettings in case the repository just happens to be uninitialized, and we\nwouldn't ever notice.\n\nOn top of that, we even fall back to the wrong value: if we don't have a\nrepository, we shouldn't fall back to the default values. Instead,\nshouldn't we fall back to the global- or system-level configuration?\n\nI'm not convinced that this design is correct. What I think we should be\ndoing is:\n\n  - Have the functions accept an optional repository.\n\n  - If a repository is passed, then we verify that it is initialized.\n    If not, we BUG.\n\n  - If we haven't yet read the configuration for that repository, then\n    we automatically do it so that we can also pass a repository other\n    than `the_repository`.\n\n  - If no repository is passed, then we populate a global variable that\n    contains the system- and global-level configuration and return that\n    value instead.\n\nThat'd work both in the context where we have a repository and where we\ndon't have one, and we'd detect the edge case where we have a repository\nthat is uninitialized.\n\n> diff --git a/environment.c b/environment.c\n> index 76ee65e62b..f5628b6758 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -119,23 +119,23 @@ int is_bare_repository(struct repository *repo)\n>  \n>  int repo_protect_ntfs(struct repository *repo)\n>  {\n> -\treturn (repo && repo->initialized) ?\n> -\t\trepo_config_values(repo)->protect_ntfs :\n> -\t\tPROTECT_NTFS_DEFAULT;\n> +\treturn repo->initialized\n> +\t\t? repo_config_values(repo)->protect_ntfs\n> +\t\t: PROTECT_NTFS_DEFAULT;\n>  }\n\nSo I think if we want to lose these checks, we should lose both of them\nand require the repository to be initialized. But I feel like this whole\nsubsystem needs a bit of a redesign before we can continue iterating on\nit.\n\nPatrick\n"}]}