Re: [PATCH 3/3] environment: reorder variables in repo_config_values structure
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 5, 2026, 21:47 UTC
- Message-ID
- <xmqqo6fgnssx.fsf@gitster.g>
- In-Reply-To
- <20260805115342.3939931-4-cat@malon.dev>
Tian Yuchen <cat@malon.dev> writes:
Show 5 quoted lines
> Reorder the fields in struct repo_config_values and its initialization > function to follow the order of configuration sections. > > Keeping the declaration and initialization order aligned makes the > structure easier to review and maintain.
Really?
Do you have some automated tool to make sure these initialization assignments in the environment.c file and declaration in the environment.h file match the order in Documentation/config/*.adoc or something else? Have you designated some list as the authoritative source of truth to check these against? Without such a list to check the code against and a mechanism to enforce the ordering, I find it hard to agree with such a claim that this makes it easier to maintain.
It is typical to list the structure members in the order of stricter to looser alignment requirement of their types. I do not know how strictly it is followed for "struct repo_config_values", but by spreading pointer valued members more widely with smaller enums in between, the change certainly is making the overall structure size larger by requiring more padding between the members with different alignment requirements. Not that we would have 100s of instances of these structures.
Show 112 quoted lines
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
> Signed-off-by: Tian Yuchen <cat@malon.dev>
> ---
> environment.c | 31 +++++++++++++++++++++----------
> environment.h | 20 +++++++++++++-------
> 2 files changed, 34 insertions(+), 17 deletions(-)
>
> diff --git a/environment.c b/environment.c
> index f5628b6758..918d8b50b8 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -745,31 +745,42 @@ int git_default_config(const char *var, const char *value,
>
> void repo_config_values_init(struct repo_config_values *cfg)
> {
> + /* core */
> cfg->attributes_file = NULL;
> cfg->excludes_file = NULL;
> cfg->editor_program = NULL;
> cfg->pager_program = NULL;
> cfg->askpass_program = NULL;
> - cfg->apply_default_whitespace = NULL;
> - cfg->apply_default_ignorewhitespace = NULL;
> - cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
> - cfg->autorebase = AUTOREBASE_NEVER;
> cfg->object_creation_mode = OBJECT_CREATION_MODE;
> cfg->apply_sparse_checkout = 0;
> + cfg->trust_ctime = 1;
> + cfg->check_stat = 1;
> + cfg->zlib_compression_level = Z_BEST_SPEED;
> + cfg->precomposed_unicode = -1;
> + cfg->core_sparse_checkout_cone = 0;
> + cfg->warn_on_object_refname_ambiguity = 1;
> cfg->protect_hfs = PROTECT_HFS_DEFAULT;
> cfg->protect_ntfs = PROTECT_NTFS_DEFAULT;
> cfg->ignore_case = 0;
> cfg->trust_executable_bit = 1;
> cfg->has_symlinks = platform_has_symlinks();
> +
> + /* apply */
> + cfg->apply_default_whitespace = NULL;
> + cfg->apply_default_ignorewhitespace = NULL;
> +
> + /* branch */
> + cfg->autorebase = AUTOREBASE_NEVER;
> cfg->branch_track = BRANCH_TRACK_REMOTE;
> - cfg->trust_ctime = 1;
> - cfg->check_stat = 1;
> - cfg->zlib_compression_level = Z_BEST_SPEED;
> +
> + /* pack */
> cfg->pack_compression_level = Z_DEFAULT_COMPRESSION;
> - cfg->precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
> - cfg->core_sparse_checkout_cone = 0;
> +
> + /* push */
> + cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
> +
> + /* sparse */
> cfg->sparse_expect_files_outside_of_patterns = 0;
> - cfg->warn_on_object_refname_ambiguity = 1;
> }
>
> void repo_config_values_clear(struct repo_config_values *cfg)
> diff --git a/environment.h b/environment.h
> index 30678257b5..52ed13c0fc 100644
> --- a/environment.h
> +++ b/environment.h
> @@ -121,16 +121,11 @@ struct repo_config_values {
> char *editor_program;
> char *pager_program;
> char *askpass_program;
> - char *apply_default_whitespace;
> - char *apply_default_ignorewhitespace;
> - enum push_default_type push_default;
> - enum rebase_setup_type autorebase;
> enum object_creation_mode object_creation_mode;
> int apply_sparse_checkout;
> int trust_ctime;
> int check_stat;
> int zlib_compression_level;
> - int pack_compression_level;
> int precomposed_unicode;
> int core_sparse_checkout_cone;
> int warn_on_object_refname_ambiguity;
> @@ -140,11 +135,22 @@ struct repo_config_values {
> int trust_executable_bit;
> int has_symlinks;
>
> - /* section "sparse" config values */
> - int sparse_expect_files_outside_of_patterns;
> + /* section "apply" config values */
> + char *apply_default_whitespace;
> + char *apply_default_ignorewhitespace;
>
> /* section "branch" config values */
> + enum rebase_setup_type autorebase;
> enum branch_track branch_track;
> +
> + /* section "pack" config values */
> + int pack_compression_level;
> +
> + /* section "push" config values */
> + enum push_default_type push_default;
> +
> + /* section "sparse" config values */
> + int sparse_expect_files_outside_of_patterns;
> };
>
> struct repo_config_values *repo_config_values(struct repository *repo);