> It claimed that grep would still need some explicit handling, but that is
> not the call to repo_read_gitmodules (applying this patch on top of
> ff6f1f564c4 still keep the test suite happy, specifically
> t7814-grep-recurse-submodules, which contains a test
> "grep history with moved submoules")
Firstly, spelling of "remove" and "superfluous" in the commit title.
I don't think the "grep history with moved submodules" test exercises
much. That test only tests the superproject > submodule case, but we
need a superproject > submodule > sub-submodule case, because what is
being removed is a call to repo_read_gitmodules() on a repository
("struct repository submodule") that has a superproject ("struct
repository *superproject"). In other words, we need a submodule that has
its own gitmodules.
Alternatively, it would be fine if someone could point out where the
.gitmodules file is lazily loaded when grep_submodule() is invoked. I
couldn't find it, although I wasn't looking very hard. I did look at the
invocation of repo_submodule_init() (right before the removed lines),
which indeed calls repo_read_gitmodules() indirectly through
submodule_from_path(), but that is called on the superproject, whereas
what is being removed is a call on the submodule.
> The special handling is the call to gitmodules_config_oid which was added
> already in 74ed43711f (grep: enable recurse-submodules to work on
> <tree> objects, 2016-12-16), but then was still named
> gitmodules_config_sha1.
If you're stating that gitmodules_config_oid() is where the .gitmodules
file is lazily loaded, it doesn't seem to be that way, because that
function works only on the_repository (at least on 'master' and 'next').
> This is a resend of origin/sb/grep-submodule-cleanup,
> and I think picking ff6f1f564c4 as the base for the series would
> also be appropriate.
Any particular reason why you suggest that commit (which is more than a
year old)? It seems that basing this on 'master' is fine.
