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

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

[Joy via GitGitGadget]

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.

Signed-off-by: Joy Gundu <soniqjjj@gmail.com>
---
    config: move pack size limit into repo config

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2271%2FJoiejoie1%2Foutreachy%2Fglobal-state-mini-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2271/Joiejoie1/outreachy/global-state-mini-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2271

 builtin/pack-objects.c | 2 +-
 environment.c          | 4 ++--
 environment.h          | 3 +--
 object-file.c          | 6 ++++--
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index af9390a46b..1c9044060d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -5380,7 +5380,7 @@ int cmd_pack_objects(int argc,
 	if (!HAVE_THREADS && delta_search_threads != 1)
 		warning(_("no threads support, ignoring --threads"));
 	if (!pack_to_stdout && !pack_size_limit)
-		pack_size_limit = pack_size_limit_cfg;
+		pack_size_limit = cfg->pack_size_limit;
 	if (pack_to_stdout && pack_size_limit)
 		die(_("--max-pack-size cannot be used to build a pack for transfer"));
 	if (pack_size_limit && pack_size_limit < 1024*1024) {
diff --git a/environment.c b/environment.c
index c83cf44839..e350d6949e 100644
--- a/environment.c
+++ b/environment.c
@@ -57,7 +57,6 @@ char *check_roundtrip_encoding;
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
 #endif
 int grafts_keep_true_parents;
-unsigned long pack_size_limit_cfg;
 
 #ifndef PROTECT_HFS_DEFAULT
 #define PROTECT_HFS_DEFAULT 0
@@ -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;
diff --git a/environment.h b/environment.h
index b336459e90..070314f7ef 100644
--- a/environment.h
+++ b/environment.h
@@ -132,6 +132,7 @@ struct repo_config_values {
 	int check_stat;
 	int zlib_compression_level;
 	int pack_compression_level;
+	unsigned long pack_size_limit;
 	int precomposed_unicode;
 	int core_sparse_checkout_cone;
 	int warn_on_object_refname_ambiguity;
@@ -235,8 +236,6 @@ int have_git_dir(void);
 /* Environment bits from configuration mechanism */
 extern int minimum_abbrev, default_abbrev;
 extern int assume_unchanged;
-extern unsigned long pack_size_limit_cfg;
-
 extern int grafts_keep_true_parents;
 
 const char *get_log_output_encoding(void);
diff --git a/object-file.c b/object-file.c
index 210984f825..3178bd6e65 100644
--- a/object-file.c
+++ b/object-file.c
@@ -885,6 +885,8 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas
 								 struct odb_transaction_files,
 								 base);
 	struct transaction_packfile *state = &transaction->packfile;
+	struct repo_config_values *cfg =
+		repo_config_values(the_repository);
 	struct git_hash_ctx ctx;
 	unsigned char obuf[16384];
 	unsigned header_len;
@@ -906,8 +908,8 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas
 	 * the difference between the inflated and on-disk size is limited
 	 * to zlib compression and is sufficient for this check.
 	 */
-	if (state->nr_written && pack_size_limit_cfg &&
-	    pack_size_limit_cfg < state->offset + stream->size) {
+	if (state->nr_written && cfg->pack_size_limit &&
+	    cfg->pack_size_limit < state->offset + stream->size) {
 		flush_packfile_transaction(transaction);
 		odb_reprepare(transaction->base.source->odb);
 	}

base-commit: c51408889c3b372814bc20b9431c3b74d0d268eb
-- 
gitgitgadget

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

[Kristoffer Haugsbakk]

On Tue, Oct 20, 2026, at 20:56, Joy via GitGitGadget wrote:
> From: Joy Gundu <soniqjjj@gmail.com>
>
> [...]
>
> This change removes one piece of global configuration state from Git
> and helps make Git code easier to reuse as a library component.

It's nice to see a paragraph explaining the
overall project in plain terms. Not just
"we are getting rid of globals" or "this is
libification".

>
> Signed-off-by: Joy Gundu <soniqjjj@gmail.com>
> [...]

Sent from mobile

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

[Patrick Steinhardt]

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

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

[Joie Joie]

Hi Patrick,

Thanks for the feedback! I’ll take a closer look at zero-initializing
repo_config_values and see if it makes sense to include it as a
preparatory patch.

I’ll also keep your advice on commit message structure in mind for
future patches.

Thanks again for taking the time to review my work!

Best,
Joy


On Wed, Oct 21, 2026 at 9:09 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> 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