> On Tue, Aug 10, 2021 at 3:29 PM Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote:
> >
> > Record the repository whenever an OID grep source is created, and teach
> > the worker threads to explicitly provide the repository when accessing
> > objects.
>
> This patch fixes the second NEEDSWORK comment at grep_submodule(),
> right? Maybe this comment could be replaced with a mention that
> add_submodule_odb_by_path() is called for testing purposes, and it
> should no longer produce a real addition to the alternates list?
Good catch. I'll update the comment.
> > diff --git a/grep.h b/grep.h
> > index f4a3090f1c..c5234f9b38 100644
> > --- a/grep.h
> > +++ b/grep.h
> > @@ -187,6 +187,7 @@ struct grep_source {
> > GREP_SOURCE_BUF,
> > } type;
> > void *identifier;
> > + struct repository *repo; /* if GREP_SOURCE_OID */
>
> Hmm, the grep threads now have two `struct repository` references, one
> in `struct grep_source` and one in `struct grep_opt` (see
> builtin/grep.c:run()). The one from `struct grep_opt` will always be
> `the_repository` (in the worker threads). I wonder if, in the long
> run, we could instruct the worker threads to use the new reference
> from `struct grep_source` throughout grep.c, and perhaps even remove
> the one from `struct grep_opt`.
>
> This would solve the issue I mentioned in [1], about git grep
> currently ignoring the textconv attributes from submodules, and
> instead applying the ones from the superproject in the submodules'
> files. (Here there is an example of this bug: [2] .)
>
> It would also allow grep to use the textconv cache from each
> submodule, instead of saving/reading everything from the
> superproject's textconv cache.
>
> While this doesn't happen, though, could we perhaps add a comment
> somewhere to avoid any confusion regarding the two different
> repository pointers that the worker threads hold?
>
> [1]: https://lore.kernel.org/git/CAHd-oW5iEQarYVxEXoTG-ua2zdoybTrSjCBKtO0YT292fm0NQQ@xxxxxxxxxxxxxx/
> [2]: https://gitlab.com/-/snippets/1896951
Ah...another good catch. Thanks also for a link to your email - I'll
mention it when I add the comment you suggested.
