Re: [PATCH] repack: add config to skip updating server info

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2078 bytes --]

On Fri, Mar 11, 2022 at 11:20:41AM -0500, Taylor Blau wrote:
> On Fri, Mar 11, 2022 at 12:09:30PM +0100, Patrick Steinhardt wrote:
> > diff --git a/Documentation/config/repack.txt b/Documentation/config/repack.txt
> > index 9c413e177e..22bfc26afc 100644
> > --- a/Documentation/config/repack.txt
> > +++ b/Documentation/config/repack.txt
> > @@ -25,3 +25,6 @@ repack.writeBitmaps::
> >  	space and extra time spent on the initial repack.  This has
> >  	no effect if multiple packfiles are created.
> >  	Defaults to true on bare repos, false otherwise.
> > +
> > +repack.updateServerInfo::
> > +	If set to false, git-repack will not run git-update-server-info.
> 
> Can you clarify here what the default value of this config variable is,
> and how it interacts with repack's `-n` flag? E.g., something along the
> lines of:
> 
>     repack.updateServerInfo::
>         If set to false, linkgit:git-repack[1] will not run
>         linkgit:git-update-serve-info[1]. Defaults to true. Can be
>         overridden when true by the `-n` option of
>         linkgit:git-repack[1].
> 
> Perhaps a little verbose, but I think it leaves less ambiguity about
> what this new configuration variable is for.

Makes sense.

> > diff --git a/builtin/repack.c b/builtin/repack.c
> > index da1e364a75..3baa993da2 100644
> > --- a/builtin/repack.c
> > +++ b/builtin/repack.c
> > @@ -22,6 +22,7 @@ static int delta_base_offset = 1;
> >  static int pack_kept_objects = -1;
> >  static int write_bitmaps = -1;
> >  static int use_delta_islands;
> > +static int no_update_server_info = 0;
> 
> Not the fault of this patch, but I wonder if this would be less
> confusing if we stored `update_server_info` instead of
> `no_update_server_info`. If you have time, I think it may be worth a
> preparatory patch at the beginning to swap the two.
[snip]

I indeed first had a look at how to do this, but didn't find a negated
`OPT_BOOL()`. I had another look now though, and it seems like this is
typically solved via `OPT_NEGBIT()`.

Thanks!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]