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

From: Junio C Hamano <[email-masked]>
Date: Thu, 18 Apr 2024 12:36:32 -0700

Patrick Steinhardt <[email-masked]> 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.

Thanks.