git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/3] builtin/grep: read submodule trees via their repository

From
Toon Claes <toon@iotcl.com>
Date
Oct 30, 2026, 16:25 UTC
Message-ID
<87fqxnyx3s.fsf@dev.null.iotcl.com.invalid>
In-Reply-To
<20261030-pks-grep-recurse-submodule-v1-1-d4eb5c87d8fb@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 24 quoted lines
> 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.
Should we mention this is a regression in 2.56?
Show 20 quoted lines
> 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(-)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index b045f8a488d..22fb365eeac 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -653,7 +653,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
>  			void *data;
>  			size_t size;
>  
> -			data = odb_read_object(the_repository->objects,
> +			data = odb_read_object(repo->objects,
Derp... What simple (in hindsight) fix.
Show 13 quoted lines
>  					       &entry.oid, &type, &size);
>  			if (!data)
>  				die(_("unable to read tree (%s)"),
> diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
> index 3d149d34c15..f51c5181805 100755
> --- a/t/t7814-grep-recurse-submodules.sh
> +++ b/t/t7814-grep-recurse-submodules.sh
> @@ -631,4 +631,27 @@ test_expect_success 'check scope of core.useReplaceRefs' '
>  	git -C base grep --cached --recurse-submodules C
>  '
>  
> +test_expect_success 'grep --recurse-submodules with tree-ish and nested directory' '
> +	rm -rf sub super &&
Small nit, shall we use test_when_finished here?
Show 21 quoted lines
> +	git init sub &&
> +	mkdir sub/dir &&
> +	echo hello >sub/top &&
> +	echo hello >sub/dir/nested &&
> +	git -C sub add top dir/nested &&
> +	git -C sub commit --message "sub" &&
> +
> +	test_config_global protocol.file.allow always &&
> +	git init super &&
> +	git -C super submodule add "$(pwd)/sub" sub &&
> +	git -C super commit --message "super" &&
> +
> +	cat >expect <<-\EOF &&
> +	HEAD:sub/dir/nested:hello
> +	HEAD:sub/top:hello
> +	EOF
> +	git -C super grep --recurse-submodules -e hello HEAD >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_done
Verified the test fails without the fix above.
All changes look good.
> -- 
> 2.56.0.341.g393f52fd96.dirty

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.

-- 
Laters,
Toon
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 3 of 5 in “builtin/grep: fix reading objects from wrong repository”
  1. 0/3 builtin/grep: fix reading objects from wrong repositoryPatrick Steinhardt, Oct 30, 2026
  2. 1/3 builtin/grep: read submodule trees via their repositoryPatrick Steinhardt, Oct 30, 2026
  3. Toon ClaesOct 30, 2026
  4. 2/3 builtin/grep: read sparse directory trees via passed-in repositoryPatrick Steinhardt, Oct 30, 2026
  5. 3/3 builtin/grep: read ".gitmodules" via passed-in repositoryPatrick Steinhardt, Oct 30, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.