From: Patrick Steinhardt Date: Tue, 02 Jun 2026 08:12:51 GMT Subject: Re: [PATCH v4 3/8] environment: move `zlib_compression_level` into `struct repo_config_values` Message-ID: In-Reply-To: On Tue, Jun 02, 2026 at 09:07:32AM +0900, Junio C Hamano wrote: > Olamide Caleb Bello writes: > > > @@ -906,6 +906,7 @@ static int start_loose_object_common(struct odb_source *source, > > const struct git_hash_algo *algo = source->odb->repo->hash_algo; > > const struct git_hash_algo *compat = source->odb->repo->compat_hash_algo; > > int fd; > > + struct repo_config_values *cfg = repo_config_values(the_repository); > > Would source->odb->repo have properly initialized repo_config_values > structure at this point? Shouldn't we be using it for this call, > instead of the_repository? I think as an intermediate step it's okay-ish to use `the_repository`, as it doesn't make the status quo any worse. But ideally, we'd have a follow-up patch series that converts "object-file.c" to drop the dependency on `the_repository` completely, which will be easier after this patch series here has landed as there will only be a handful more config options to migrate: - `pack_compression_level` and `zlib_compression_level` get migrated in this series. - `object_creation_mode` still needs migration. - `pack_size_limit_cfg` still needs migration. Other than that we really only need to use the correct repo in a small set of functions. Overall, I think it's sensible to always use `the_repository` at the callsites in a patch series like this so that it's obvious that there is no change in behaviour. So every patch series that gets rid of global state in a subsystem X will basically bubble up the global state into the next-higher level, and it's then the duty of the next patch series to address that next-higher level. The only exception of course is subsystems that already got rid of `the_repository` -- we really shouldn't reintroduce the use there. Patrick