Re: [PATCH v2 0/6] global: drop `the_index` variable

[Patrick Steinhardt <ps@pks.im>]

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

On Thu, Apr 18, 2024 at 12:36:32PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > this is the second version of my patch series that aims to drop
> > `the_index`.
> >
> > Changes compared to v1:
> >
> >   - This version goes a bit further now and completely drops the
> >     static `the_index` variable, as well. The repository's index gets
> >     allocated dynamically now, like all the other sub-structures like
> >     the ODB.
> >
> >   - This also allows this series to remove `initialize_the_repository()`
> >     now. Instead, callers call `initialize_repository()` now.
> >
> > There is still quite an ugly hack in `initialize_repository()` which
> > requires us to treat `the_repository` specially. This is because
> > `the_hash_algo` maps to `the_repository->hash_algo`, and we rely on it
> > being initialized to SHA1. So we need call `repo_set_hash_algo()` on
> > `the_repository`. On the other hand, we cannot set the hash algo on
> > repos which are not `the_repository`, because that breaks stuff, as
> > well.
> >
> > I'm currently prepping another patch series that builds on top of this
> > series and cleans up this mess. It surfaces several bugs that got masked
> > by our setup, like for example `git rev-parse --short=` not working
> > correctly with SHA256 because we always truncate to the maximum length
> > of SHA1.
> 
> I'll take a look, and may even comment on them later, but let me
> otherwise place this series on the back burner, not because I am in
> love with the_index (I am not) and not because I think the "index"
> member in the repository struct is a mistake (I suspect it is, but I
> am not convinced either way), but because I'd prefer to see our tree
> to be quiescent when we apply a tree-wide patch like [2/6], but
> we'll be in -rc period soonish, during which time we'd want to be
> able to concentrate on fixing regressions without having to worry
> about being able to reapply such tree-wide changes to keep 'next'
> and 'seen' building.

Makes sense, thanks!

Patrick

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