From: Bello Olamide Date: Tue, 14 Apr 2026 14:32:43 GMT Subject: Re: [PATCH v2 3/8] environment: move `zlib_compression_level` into repo_config_values Message-ID: In-Reply-To: On Tue, 14 Apr 2026 at 09:58, Karthik Nayak wrote: > > Olamide Caleb Bello writes: > > > The `zlib_compression_level` configuration is currently stored in the > > global variable `zlib_compression_level`, which makes it shared across > > repository instances within a single process. > > > > Store it instead in `repo_config_values` so the value is associated > > with the repository from which it was read. This preserves existing > > behavior while avoiding cross-repository state leakage and continues > > the effort to reduce reliance on global configuration state. > > > > Update all references to use repo_config_values(). > > > > Mentored-by: Christian Couder > > Mentored-by: Usman Akinyemi > > Signed-off-by: Olamide Caleb Bello > > --- > > builtin/index-pack.c | 3 ++- > > diff.c | 3 ++- > > environment.c | 6 +++--- > > environment.h | 2 +- > > http-push.c | 3 ++- > > object-file.c | 3 ++- > > 6 files changed, 12 insertions(+), 8 deletions(-) > > > > diff --git a/builtin/index-pack.c b/builtin/index-pack.c > > index b67fb0256c..dd82eed76f 100644 > > --- a/builtin/index-pack.c > > +++ b/builtin/index-pack.c > > @@ -1416,8 +1416,9 @@ static int write_compressed(struct hashfile *f, void *in, unsigned int size) > > git_zstream stream; > > int status; > > unsigned char outbuf[4096]; > > + struct repo_config_values *cfg = repo_config_values(the_repository); > > > > - git_deflate_init(&stream, zlib_compression_level); > > + git_deflate_init(&stream, cfg->zlib_compression_level); > > stream.next_in = in; > > stream.avail_in = size; > > > > diff --git a/diff.c b/diff.c > > index 501648a5c4..4bc0297873 100644 > > --- a/diff.c > > +++ b/diff.c > > @@ -3365,8 +3365,9 @@ static unsigned char *deflate_it(char *data, > > int bound; > > unsigned char *deflated; > > git_zstream stream; > > + struct repo_config_values *cfg = repo_config_values(the_repository); > > > > - git_deflate_init(&stream, zlib_compression_level); > > + git_deflate_init(&stream, cfg->zlib_compression_level); > > bound = git_deflate_bound(&stream, size); > > deflated = xmalloc(bound); > > stream.next_out = deflated; > > diff --git a/environment.c b/environment.c > > index 8542ac3141..5b0e88b65c 100644 > > --- a/environment.c > > +++ b/environment.c > > @@ -52,7 +52,6 @@ char *git_commit_encoding; > > char *git_log_output_encoding; > > char *apply_default_whitespace; > > char *apply_default_ignorewhitespace; > > -int zlib_compression_level = Z_BEST_SPEED; > > int pack_compression_level = Z_DEFAULT_COMPRESSION; > > int fsync_object_files = -1; > > int use_fsync = -1; > > @@ -377,7 +376,7 @@ int git_default_core_config(const char *var, const char *value, > > level = Z_DEFAULT_COMPRESSION; > > else if (level < 0 || level > Z_BEST_COMPRESSION) > > die(_("bad zlib compression level %d"), level); > > - zlib_compression_level = level; > > + cfg->zlib_compression_level = level; > > zlib_compression_seen = 1; > > return 0; > > } > > @@ -389,7 +388,7 @@ int git_default_core_config(const char *var, const char *value, > > else if (level < 0 || level > Z_BEST_COMPRESSION) > > die(_("bad zlib compression level %d"), level); > > if (!zlib_compression_seen) > > - zlib_compression_level = level; > > + cfg->zlib_compression_level = level; > > if (!pack_compression_seen) > > pack_compression_level = level; > > return 0; > > @@ -721,4 +720,5 @@ void repo_config_values_init(struct repo_config_values *cfg) > > cfg->branch_track = BRANCH_TRACK_REMOTE; > > cfg->trust_ctime = 1; > > cfg->check_stat = 1; > > + cfg->zlib_compression_level = Z_BEST_SPEED; > > } > > diff --git a/environment.h b/environment.h > > index 1d3e2e4f23..93201620af 100644 > > --- a/environment.h > > +++ b/environment.h > > @@ -93,6 +93,7 @@ struct repo_config_values { > > int apply_sparse_checkout; > > int trust_ctime; > > int check_stat; > > + int zlib_compression_level; > > Nit: applies to existing values too: > 1. Perhaps it would be nicer if these were sorted alphabetically, I > assume we'll add a lot more fields here. I’ll reorder the fields in `repo_config_values` alphabetically to keep things consistent as more entries are added. > 2. Have a comment stating the purpose of the variable? I’ll also add comments describing the purpose of each variable > > The patch looks good to me otherwise. Thanks, Karthik, for the review. Olamide Bello