Re: [PATCH] config: move pack size limit into repo config

[Patrick Steinhardt <ps@pks.im>]

On Tue, Oct 20, 2026 at 06:56:27PM +0000, Joy via GitGitGadget wrote:
> From: Joy Gundu <soniqjjj@gmail.com>
> 
> Store the pack.packsizelimit value in repository configuration
> rather than keeping it in a standalone global variable for the repository.
> 
> Update the configuration parser and pack-writing code to read
> the value from repo_config_values rather than a global variable.
> 
> This change removes one piece of global configuration state from Git
> and helps make Git code easier to reuse as a library component.

The commit message typically starts with an observation, followed by an
explanation what's bad about the observed status quo and then finally
explains how that state is improved. The order you have here does not
match that typical flow.

> diff --git a/environment.c b/environment.c
> index c83cf44839..e350d6949e 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -728,7 +727,7 @@ int git_default_config(const char *var, const char *value,
>  	}
>  
>  	if (!strcmp(var, "pack.packsizelimit")) {
> -		pack_size_limit_cfg = git_config_ulong(var, value, ctx->kvi);
> +		cfg->pack_size_limit = git_config_ulong(var, value, ctx->kvi);
>  		return 0;
>  	}
>  
> @@ -768,6 +767,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
>  	cfg->check_stat = 1;
>  	cfg->zlib_compression_level = Z_BEST_SPEED;
>  	cfg->pack_compression_level = Z_DEFAULT_COMPRESSION;
> +	cfg->pack_size_limit = 0;
>  	cfg->precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
>  	cfg->core_sparse_checkout_cone = 0;
>  	cfg->warn_on_object_refname_ambiguity = 1;

Hmm, quite annoying that we need to explicitly zero out all these
fields. Maybe we should adapt `repo_config_values_init()` to first
`memset(cfg, 0, sizeof(*cfg))`? That would allow us to remove a lot of
assignments. If you want to do that, it would be great to do it as a
preparatory patch in the same series.

Other than that the diff looks exactly the same as a local patch I had
already written myself, so thanks for allowing me to worry about one
less thing :)

Patrick