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
Patrick Steinhardt <ps@pks.im>
Date
Nov 3, 2026, 11:06 UTC
Message-ID
<aunAR67MHppLt--N@pks.im>
In-Reply-To
<87fqxnyx3s.fsf@dev.null.iotcl.com.invalid>
On Fri, Oct 30, 2026 at 05:25:11PM +0100, Toon Claes wrote:
Show 28 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > 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?
Fair, can do.
Show 12 quoted lines
> > 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?

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
Previous: Toon ClaesNext: Patrick Steinhardt
Message 4 of 10 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. Patrick SteinhardtNov 3, 2026
  5. 2/3 builtin/grep: read sparse directory trees via passed-in repositoryPatrick Steinhardt, Oct 30, 2026
  6. 3/3 builtin/grep: read ".gitmodules" via passed-in repositoryPatrick Steinhardt, Oct 30, 2026
  7. 0/3 builtin/grep: fix reading objects from wrong repositoryPatrick Steinhardt, Nov 3, 2026
  8. 1/3 builtin/grep: read submodule trees via their repositoryPatrick Steinhardt, Nov 3, 2026
  9. 2/3 builtin/grep: read sparse directory trees via passed-in repositoryPatrick Steinhardt, Nov 3, 2026
  10. 3/3 builtin/grep: read ".gitmodules" via passed-in repositoryPatrick Steinhardt, Nov 3, 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.