Re: [PATCH v2 6/8] maintenance: add ability to pass config options

From: Robert Coup <robert@coup.net.nz>
Date: Feb 28, 2022, 12:02 UTC
Message-ID: <CACf-nVeh9p1ba+tPSFQvqDBq3SaNtnhCQMibd__EED0w0Btozg@mail.gmail.com>
In-Reply-To: <xmqqsfs7fn6v.fsf@gitster.g>

Hi Junio,

On Fri, 25 Feb 2022 at 06:57, Junio C Hamano <gitster@pobox.com> wrote:

>
> Style.  SP on both sides of '<'.

Thanks

> It is unclear if it is generally a good idea to pass hardcoded set
> of configuration variables to begin with, but provided if it makes
> sense [*], the implementation seems OK.

My RFC didn't do any repacking, and you (rightly) raised it at https://lore.kernel.org/git/xmqqk0eecpl9.fsf@gitster.g/

Show 10 quoted lines

>> To note:
>>
>>  1. This will produce duplicated objects between the existing and newly
>>     fetched packs, but gc will clean them up.
>
> ... it is not smart enough to stell them to exclude what we _ought_
> to have by telling them what the _old_ filter spec was.  That's OK
> for a starting point, I guess.  Hopefully, at the end of this
> operation, we should garbage collect the duplicated objects by
> default (with an option to turn it off)?

This was my best stab at (I think) safely doing it — it respects users who have disabled auto-gc/repacking/maintenance, and doesn't add yet another config variable. But we could also:

1. do nothing — the user's object DB will contain duplicate objects
until some repacking happens. This could be up to twice the size it
"should" be.
2. print a message to suggest running `git gc` / `git maintenance`

I'm keen on some input from Derrick or someone deeply familiar with the maintenance/GC bits (particularly wrt incremental repacking) whether what I'm doing would cause issues for people with complex setups I haven't thought of.

Show 8 quoted lines

>         Side note.  And this is a big *IF*, as we can see in all
>         other helper functions in run-commands.h, nobody has such a
>         privision.  If supporting such a "feature" makes sense, we
>         probably would need to do so with a common interface that
>         can be used across run_command() API, not with an ad-hoc
>         interface that is only usable with run_auto_maintenance(),
>         which may look somewhat similar to how we have a common way
>         to pass set of environment variables.

Ævar pointed out GIT_CONFIG_PARAMETERS has an API to wrap it's use, so I can adapt the implementation to use that.

Thanks

Rob :)