Re: [PATCH 6/7] grep: add repository to OID grep sources
- From
Matheus Tavares Bernardino <matheus.bernardino@usp.br>
- Date
- Aug 11, 2021, 23:28 UTC
- Message-ID
- <CAHd-oW4ntWB7KOKxhZU4wM-Xg9MARVN2EmpxEg-rEfc4i_SUnw@mail.gmail.com>
- In-Reply-To
- <b2df2455871dc5c94ec804e9c0ce935c19da335f.1628618950.git.jonathantanmy@google.com>
On Tue, Aug 10, 2021 at 3:29 PM Jonathan Tan <jonathantanmy@google.com> 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?
Show 9 quoted lines
> 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@mail.gmail.com/ [2]: https://gitlab.com/-/snippets/1896951
Show 15 quoted lines
> 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
>