Subject: Re: [PATCH 17/19] environment: move compression level into repo settings

From: Patrick Steinhardt <[email-masked]>
Date: Wed, 16 Jul 2025 14:56:10 +0200

On Tue, Jul 15, 2025 at 06:12:18PM +0200, Patrick Steinhardt wrote:
> On Tue, Jul 15, 2025 at 04:51:32PM +0100, Phillip Wood wrote:
> > On 15/07/2025 12:27, Patrick Steinhardt wrote:
> > > On Fri, Jul 11, 2025 at 11:55:27AM -0700, Junio C Hamano wrote:
> > > > Phillip Wood <[email-masked]> writes:
> [snip]
> > >    - Not all configuration is equal. It may be perfectly fine to ignore
> > >      some configuration, but other configuration may very much be mission
> > >      critical. And whether or not configuration is important isn't really
> > >      something we can decide, as it will depend on the specific use case.
> > > 
> > > So I'm afraid that there just isn't a perfect solution here. Does it
> > > make sense to die due to a config key that isn't even used by a specific
> > > command? Maybe. And if not, which config keys _should_ make us die in
> > > case they are invalid?
> > > 
> > > The overall situation right now is a proper mess: we have config parsing
> > > cluttered everywhere, and the behaviour is just plain inconsistent. Some
> > > parsing is delayed, some isn't.
> > 
> > Indeed. My objection here was that we were delaying the parsing when it
> > wasn't delayed before. Is it feasible to call prepare_repo_settings() in
> > repo_config()? That would at least avoid the problem that moving config
> > settings into `struct repo_settings` changes when the settings are parsed
> > unless the command calls prepare_repo_settings() at start up. As far as I
> > remember `git config` uses config_with_options() so that would not be
> > adversely affected by such a change.
> 
> Hm, yeah, I think adding it to `repo_config()` might be a viable
> approach. I'll give it a try tomorrow and see what breaks :)

The answer is "quite a lot". I'm now 15 patches deep to try and fix
this and am nowhere close to a working state yet. The single biggest
issue is `core.shared_repository`, which is used in a ton of places and
which causes all kinds of pain.

I think I'll stop working on this for now, and would rather like to drop
the last three patches from this series so that we can move forward with
it.

Patrick