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?
> 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
> char *buf;
> unsigned long size;
> @@ -198,7 +199,8 @@ struct grep_source {
> void grep_source_init_file(struct grep_source *gs, const char *name,
> const char *path);
> void grep_source_init_oid(struct grep_source *gs, const char *name,
> - const char *path, const struct object_id *oid);
> + const char *path, const struct object_id *oid,
> + struct repository *repo);
> void grep_source_init_buf(struct grep_source *gs);
> void grep_source_clear_data(struct grep_source *gs);
> void grep_source_clear(struct grep_source *gs);
> --
> 2.33.0.rc1.237.g0d66db33f3-goog
>
