Re: [PATCH v2 3/8] environment: move `zlib_compression_level` into repo_config_values
- From
Bello Olamide <belkid98@gmail.com>
- Date
- Apr 14, 2026, 14:32 UTC
- Message-ID
- <CAD=f0L-ckZUpcvjB02V=ebDgSQ2XeNG8pwHfUqOgeoJF67tLGA@mail.gmail.com>
- In-Reply-To
- <CAOLa=ZRexa+uYj=F2++=vijBb760MgjdTwq3REPpxcwk02caHg@mail.gmail.com>
On Tue, 14 Apr 2026 at 09:58, Karthik Nayak <karthik.188@gmail.com> wrote:
Show 105 quoted lines
>
> Olamide Caleb Bello <belkid98@gmail.com> 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 <christian.couder@gmail.com>
> > Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
> > Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
> > ---
> > 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