patch, 3 partsbuiltin/grep: fix reading objects from wrong repository

Hi,

this small patch series fixes the bug reported in [1], where doing `git grep --recurse-submodules` with nested submodule trees fails to read the trees. The only patch needed here is the first one, the other patches are only belt-and-suspenders as I've been checking whether we use the wrong repository anywhere else.

At the end of this patch series we only use `the_repository` in `cmd_grep()` itself. Ideally we'd adapt "builtin/grep.c" to get rid of `the_repository` altogether, but unfortunately it's a bit on the involved side as we also have to worry about the case where git-grep(1) is executed outside of a repository with "--no-index". So I skipped that refactoring for now.

The patch is based on v2.56.0 so that it can be easily backported.

Thanks!

Patrick

[1]: <CAMtVYEjWd2XwJpXdk3PV2jbNdoOaqS3x+08EMu5byy55RxfRmA@mail.gmail.com>

---
Patrick Steinhardt (3):
      builtin/grep: read submodule trees via their repository
      builtin/grep: read sparse directory trees via passed-in repository
      builtin/grep: read ".gitmodules" via passed-in repository

 builtin/grep.c                     |  6 +++---
 t/t7814-grep-recurse-submodules.sh | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)

--- base-commit: a018953688f1b10bddf91bff8747068f5f4746a4 change-id: 20261030-pks-grep-recurse-submodule-20177cc7794c

The function `grep_tree()` searches for a specific string recursively in a tree. It can recurse in two different ways:

  - It may either see a subtree, in which case it recurses into itself.

  - Or it may see a submodule, and in that case it recurses via
    `grep_submodule()` which then again calls `grep_tree()`.

In the latter case we continue grepping in the submodule repository's object graph. Historically this used to work by linking that submodule's object directory into the main object database. But in 93cd344e34f (builtin/grep: stop registering submodule ODB as source, 2026-09-11) we stopped doing that and instead expect to read submodule objects via the submodule repository.

This logic is broken though: while we indeed know to read blobs via the submodule's object database, we didn't adapt trees in the same way. The result is that Git fails to read the object:

    $ git grep --recurse-submodules -e hello HEAD
    fatal: unable to read tree (935cff11a11f8734cf4e0d40955d1e0e5c12038e)

This went unnoticed because our tests only cover submodules that contain files at the top level.

Fix this by reading nested trees via `opt->repo`, which is set to the submodule's repository via `grep_submodule()`.

Reported-by: Root <rootminus0x1@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/grep.c                     |  2 +-
 t/t7814-grep-recurse-submodules.sh | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

Patrick Steinhardt <ps@pks.im> writes:

Should we mention this is a regression in 2.56?

Derp... What simple (in hindsight) fix.

Small nit, shall we use test_when_finished here?

Verified the test fails without the fix above.

All changes look good.

The other two patches make sense to me as well. Seems worthwhile to include them even if they aren't directly covered with a test.

On Fri, Oct 30, 2026 at 05:25:11PM +0100, Toon Claes wrote:

Fair, can do.

Normally I would, but in this case it would have felt weird to have "rm -rf sub super && test_when_finished "rm -rf sub super". We have to delete the old repos because they exist already, so arguably the correct location to add the `test_when_finished` would have been that preceding test. But I'd propose that this can be done at a later time?

Patrick

When iterating through the index in `grep_cache()` we may encounter sparse directory entries. In that case we read the tree object they point to so that we can recurse into it via `grep_tree()`. We do so via `the_repository` though, even though `grep_cache()` may be called for a submodule via `grep_submodule()`.

This issue is similar to the one fixed in the preceding commit, where `grep_tree()` didn't use the correct repository. But in this case here the issue cannot be triggered as we call `repo_read_gitmodules()` on the submodule repository, and that function will cause us to read its index. But because we only ever set `command_requires_full_index` for the top-level repository we will convert the submodule's index into a full index, and consequently we will never see sparse directory entries. This is also documented as such in a NEEDSWORK comment in `grep_submodule()`.

Regardless of that the code is still wrong, and it may very easily turn into a bug once we start to handle sparse indices for submodules. Adapt it to use the correct repository to preempt such a future bug.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/grep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

When grepping through a list of objects with "--recurse-submodules", we call `gitmodules_config_oid()` to load gitmodules for the specific revision. But instead of passing `opt->repo` we pass `the_repository` to that function, which is somewhat fishy.

This is again not a bug because `grep_objects()` is only ever called by `cmd_grep()`, and consequently we know that `opt->repo` is always going to be set to `the_repository`. But it's a code smell regardless of that, so adapt the code to use `opt->repo` instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/grep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi,

this small patch series fixes the bug reported in [1], where doing `git grep --recurse-submodules` with nested submodule trees fails to read the trees. The only patch needed here is the first one, the other patches are only belt-and-suspenders as I've been checking whether we use the wrong repository anywhere else.

At the end of this patch series we only use `the_repository` in `cmd_grep()` itself. Ideally we'd adapt "builtin/grep.c" to get rid of `the_repository` altogether, but unfortunately it's a bit on the involved side as we also have to worry about the case where git-grep(1) is executed outside of a repository with "--no-index". So I skipped that refactoring for now.

The patch is based on v2.56.0 so that it can be easily backported.

Changes in v2:
  - Mention that this was released as part of Git 2.56.0.
  - Link to v1: https://patch.msgid.link/20261030-pks-grep-recurse-submodule-v1-0-d4eb5c87d8fb@pks.im

Thanks!

Patrick

[1]: <CAMtVYEjWd2XwJpXdk3PV2jbNdoOaqS3x+08EMu5byy55RxfRmA@mail.gmail.com>

---
Patrick Steinhardt (3):
      builtin/grep: read submodule trees via their repository
      builtin/grep: read sparse directory trees via passed-in repository
      builtin/grep: read ".gitmodules" via passed-in repository

 builtin/grep.c                     |  6 +++---
 t/t7814-grep-recurse-submodules.sh | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)

Range-diff versus v1:

1:  e9ffa0cdee5 ! 1:  46d165815b5 builtin/grep: read submodule trees via their repository
    @@ Commit message
         In the latter case we continue grepping in the submodule repository's
         object graph. Historically this used to work by linking that submodule's
         object directory into the main object database. But in 93cd344e34f
    -    (builtin/grep: stop registering submodule ODB as source, 2026-09-11) we
    -    stopped doing that and instead expect to read submodule objects via the
    -    submodule repository.
    +    (builtin/grep: stop registering submodule ODB as source, 2026-09-11),
    +    which was released as part of Git 2.56, we stopped doing that and
    +    instead expect to read submodule objects via the submodule repository.
     
         This logic is broken though: while we indeed know to read blobs via the
         submodule's object database, we didn't adapt trees in the same way. The
2:  dfe45977b15 = 2:  f7516e29f68 builtin/grep: read sparse directory trees via passed-in repository
3:  4b914d9d07d = 3:  dc19e6589d4 builtin/grep: read ".gitmodules" via passed-in repository

--- base-commit: a018953688f1b10bddf91bff8747068f5f4746a4 change-id: 20261030-pks-grep-recurse-submodule-20177cc7794c

The function `grep_tree()` searches for a specific string recursively in a tree. It can recurse in two different ways:

  - It may either see a subtree, in which case it recurses into itself.

  - Or it may see a submodule, and in that case it recurses via
    `grep_submodule()` which then again calls `grep_tree()`.

In the latter case we continue grepping in the submodule repository's object graph. Historically this used to work by linking that submodule's object directory into the main object database. But in 93cd344e34f (builtin/grep: stop registering submodule ODB as source, 2026-09-11), which was released as part of Git 2.56, we stopped doing that and instead expect to read submodule objects via the submodule repository.

This logic is broken though: while we indeed know to read blobs via the submodule's object database, we didn't adapt trees in the same way. The result is that Git fails to read the object:

    $ git grep --recurse-submodules -e hello HEAD
    fatal: unable to read tree (935cff11a11f8734cf4e0d40955d1e0e5c12038e)

This went unnoticed because our tests only cover submodules that contain files at the top level.

Fix this by reading nested trees via `opt->repo`, which is set to the submodule's repository via `grep_submodule()`.

Reported-by: Root <rootminus0x1@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/grep.c                     |  2 +-
 t/t7814-grep-recurse-submodules.sh | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

When iterating through the index in `grep_cache()` we may encounter sparse directory entries. In that case we read the tree object they point to so that we can recurse into it via `grep_tree()`. We do so via `the_repository` though, even though `grep_cache()` may be called for a submodule via `grep_submodule()`.

This issue is similar to the one fixed in the preceding commit, where `grep_tree()` didn't use the correct repository. But in this case here the issue cannot be triggered as we call `repo_read_gitmodules()` on the submodule repository, and that function will cause us to read its index. But because we only ever set `command_requires_full_index` for the top-level repository we will convert the submodule's index into a full index, and consequently we will never see sparse directory entries. This is also documented as such in a NEEDSWORK comment in `grep_submodule()`.

Regardless of that the code is still wrong, and it may very easily turn into a bug once we start to handle sparse indices for submodules. Adapt it to use the correct repository to preempt such a future bug.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/grep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

When grepping through a list of objects with "--recurse-submodules", we call `gitmodules_config_oid()` to load gitmodules for the specific revision. But instead of passing `opt->repo` we pass `the_repository` to that function, which is somewhat fishy.

This is again not a bug because `grep_objects()` is only ever called by `cmd_grep()`, and consequently we know that `opt->repo` is always going to be set to `the_repository`. But it's a code smell regardless of that, so adapt the code to use `opt->repo` instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/grep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)