Skip to content

[Outreachy] config: move pack size limit into repo config #2271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Joiejoie1 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[Outreachy] config: move pack size limit into repo config #2271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc18b66
repo: zero-initialize configuration values
Joiejoie1 Oct 21, 2026
ba34e82
config: move pack size limit into repo config
Joiejoie1 Oct 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion builtin/pack-objects.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5370,7 +5370,7 @@ int cmd_pack_objects(int argc,
if (!HAVE_THREADS && delta_search_threads != 1)

@gitgitgadget gitgitgadget Bot Oct 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patrick Steinhardt wrote on the Git mailing list (how to reply to this email):

On Thu, Oct 22, 2026 at 09:47:12AM +0000, Joy Gundu via GitGitGadget wrote:
> From: Joy Gundu <soniqjjj@gmail.com>
> 
> The pack size limit is currently stored in a standalone global
> variable, even though it is repository configuration.
> 
> Move the value into repo_config_values and update the configuration
> parser and pack-writing code to read it from the repository's
> configuration values.
> 
> This removes one piece of global configuration state and helps
> make Git code easier to reuse as a library component.

This reads well to me, thanks.

Patrick

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) {
Expand Down
19 changes: 3 additions & 16 deletions environment.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand All @@ -752,34 +751,22 @@ int git_default_config(const char *var, const char *value,

@gitgitgadget gitgitgadget Bot Oct 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patrick Steinhardt wrote on the Git mailing list (how to reply to this email):

On Thu, Oct 22, 2026 at 09:47:11AM +0000, Joy Gundu via GitGitGadget wrote:
> From: Joy Gundu <soniqjjj@gmail.com>

Even though the patch speaks for itself, we typically still like to a
sentence or two in the commit message itself. E.g.:

    When initializing `struct repo_config_values` we manually zero out a
    lot of its members, which is tedious. Reduce the boilerplate by first
    zeroing out the whole structure so that we only have to set those
    members that have a non-zero default value.

> Signed-off-by: Joy Gundu <soniqjjj@gmail.com>
> ---
>  environment.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/environment.c b/environment.c
> index c83cf44839..89af09aa40 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -752,34 +752,24 @@ int git_default_config(const char *var, const char *value,
>  
>  void repo_config_values_init(struct repo_config_values *cfg)
>  {
> +	memset(cfg, 0, sizeof(*cfg));
> +
>  	/* section "core" config values */
> -	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->pack_compression_level = Z_DEFAULT_COMPRESSION;
>  	cfg->precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
> -	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();
> -	cfg->use_nanosec = 0;
>  
>  	/* section "sparse" config values */
> -	cfg->sparse_expect_files_outside_of_patterns = 0;

We should probably drop this comment together with the code.

>  	/* section "branch" config values */
>  	cfg->branch_track = BRANCH_TRACK_REMOTE;

Other than that this looks good to me.

Patrick

@gitgitgadget gitgitgadget Bot Oct 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Joie Joie wrote on the Git mailing list (how to reply to this email):

Hi Patrick,

Thanks for the review and the suggested wording. I'll add an
explanation to the commit message and remove the now-unnecessary
comment.

I'll include both changes in the next version of the series.

Thanks again,
Joy

On Thu, Oct 22, 2026 at 12:30 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Thu, Oct 22, 2026 at 09:47:11AM +0000, Joy Gundu via GitGitGadget wrote:
> > From: Joy Gundu <soniqjjj@gmail.com>
>
> Even though the patch speaks for itself, we typically still like to a
> sentence or two in the commit message itself. E.g.:
>
>     When initializing `struct repo_config_values` we manually zero out a
>     lot of its members, which is tedious. Reduce the boilerplate by first
>     zeroing out the whole structure so that we only have to set those
>     members that have a non-zero default value.
>
> > Signed-off-by: Joy Gundu <soniqjjj@gmail.com>
> > ---
> >  environment.c | 14 ++------------
> >  1 file changed, 2 insertions(+), 12 deletions(-)
> >
> > diff --git a/environment.c b/environment.c
> > index c83cf44839..89af09aa40 100644
> > --- a/environment.c
> > +++ b/environment.c
> > @@ -752,34 +752,24 @@ int git_default_config(const char *var, const char *value,
> >
> >  void repo_config_values_init(struct repo_config_values *cfg)
> >  {
> > +     memset(cfg, 0, sizeof(*cfg));
> > +
> >       /* section "core" config values */
> > -     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->pack_compression_level = Z_DEFAULT_COMPRESSION;
> >       cfg->precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
> > -     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();
> > -     cfg->use_nanosec = 0;
> >
> >       /* section "sparse" config values */
> > -     cfg->sparse_expect_files_outside_of_patterns = 0;
>
> We should probably drop this comment together with the code.
>
> >       /* section "branch" config values */
> >       cfg->branch_track = BRANCH_TRACK_REMOTE;
>
> Other than that this looks good to me.
>
> Patrick

@gitgitgadget gitgitgadget Bot Oct 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patrick Steinhardt wrote on the Git mailing list (how to reply to this email):

Hi Joy,

On Fri, Oct 23, 2026 at 01:00:48PM +0100, Joie Joie wrote:
> Hi Patrick,
> 
> Thanks for the review and the suggested wording. I'll add an
> explanation to the commit message and remove the now-unnecessary
> comment.
> 
> I'll include both changes in the next version of the series.

Thanks.

One note though: we don't top-post over here, but instead write our
replies below the parts that we're replying to (like I do here).

Patrick

void repo_config_values_init(struct repo_config_values *cfg)
{
memset(cfg, 0, sizeof(*cfg));

/* section "core" config values */
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->pack_compression_level = Z_DEFAULT_COMPRESSION;
cfg->precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
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();
cfg->use_nanosec = 0;

/* section "sparse" config values */
cfg->sparse_expect_files_outside_of_patterns = 0;

/* section "branch" config values */
cfg->branch_track = BRANCH_TRACK_REMOTE;
Expand Down
3 changes: 1 addition & 2 deletions environment.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions object-file.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,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;
Expand All @@ -903,8 +905,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);
}
Expand Down
Loading