On Tue, Sep 01, 2026 at 03:03:44PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
>
> > The "cache-tree" subsystem still depends on `the_repository`. Adapt it
> > to instead use repositories provided via the context, either as a new
> > parameter or the one passed in via `struct index_state`.
> >
> > Besides getting rid of `the_repository`, this also removes the last
> > dependency on registering submodule sources with the main object
> > database. When reading gitmodules from a submodule's index we implicitly
> > read that object via `the_repository`'s object database, which is of
> > course wrong. This works though because we would then register the
> > submodule's object database with the main object database, but a later
> > patch is going to get rid of that mechanism.
>
> As "the_repository, " is quite a long string, many lines bust our
> usual ~70 column limit, but other than that the changes look quite
> straight-forward.
>
> We probably should make the cache_tree_fully_valid() into file scope
> static. Your new cache_tree_fully_valid() that takes the repository
> and a cache_tree, which will be file-scope static, will gain a new
> thin wrapper around it, i.e.,
>
> int has_valid_cache_tree(struct index_state *istate)
> {
> return cache_tree_fully_valid(istate->repo, istate->cache_tree);
> }
>
> Migrate the callers outside cache_tree.c file to use it; outside the
> implementation, nobody has any business talking about a subdirectory
> of any cache-tree, or any cache-tree outside the context of the istate
> it belongs to.
Yeah, I think that's a good idea indeed. But instead of introducing a
new `has_valid_cache_tree()`, I think it makes more sense to just adapt
the current function and introduce `cache_tree_fully_valid_recursive()`
as internal function instead. That also matches the other existing
interfaces like `cache_tree_fully_valid()` better.
Thanks!
Patrick