Re: [PATCH 2/7] grep: use submodule-ODB-as-alternate lazy-addition
- From
Emily Shaffer <emilyshaffer@google.com>
- Date
- Aug 11, 2021, 21:36 UTC
- Message-ID
- <YRRC528p06ByShE5@google.com>
- In-Reply-To
- <31e9b914c4bb500a98fbb14674c93a0d8ece47a2.1628618950.git.jonathantanmy@google.com>
On Tue, Aug 10, 2021 at 11:28:40AM -0700, Jonathan Tan wrote:
Show 24 quoted lines
> > In the parent commit, Git was taught to add submodule ODBs as alternates > lazily, but grep does not use this because it computes the path to add > directly, not going through add_submodule_odb(). Add an equivalent to > add_submodule_odb() that takes the exact ODB path and teach grep to use > it. > > Signed-off-by: Jonathan Tan <jonathantanmy@google.com> > --- > builtin/grep.c | 2 +- > submodule.c | 5 +++++ > submodule.h | 1 + > 3 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/builtin/grep.c b/builtin/grep.c > index 7d2f8e5adb..87bcb934a2 100644 > --- a/builtin/grep.c > +++ b/builtin/grep.c > @@ -450,7 +450,7 @@ static int grep_submodule(struct grep_opt *opt, > * store is no longer global and instead is a member of the repository > * object. > */ > - add_to_alternates_memory(subrepo.objects->odb->path); > + add_submodule_odb_by_path(subrepo.objects->odb->path);
I had wondered whether we can entirely drop add_to_alternates_memory() but I see on second reading that that's still used by register_all_submodule_odb...(). I wonder if it can become static in submodule.c to prevent users from dodging the envvar+BUG()?
Show 27 quoted lines
> obj_read_unlock();
>
> memcpy(&subopt, opt, sizeof(subopt));
> diff --git a/submodule.c b/submodule.c
> index 8fde90e906..8de1aecaeb 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -187,6 +187,11 @@ int add_submodule_odb(const char *path)
> return ret;
> }
>
> +void add_submodule_odb_by_path(const char *path)
> +{
> + string_list_insert(&added_submodule_odb_paths, xstrdup(path));
> +}
> +
> int register_all_submodule_odb_as_alternates(void)
> {
> int i;
> diff --git a/submodule.h b/submodule.h
> index c252784bc2..17a06cc43b 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -104,6 +104,7 @@ int bad_to_remove_submodule(const char *path, unsigned flags);
> * the_repository.
> */
> int add_submodule_odb(const char *path);Curious whether add_submodule_odb() can be reduced by calling add_submodule_odb_by_path() internally.
Show 7 quoted lines
> +void add_submodule_odb_by_path(const char *path); > int register_all_submodule_odb_as_alternates(void); > > /* > -- > 2.33.0.rc1.237.g0d66db33f3-goog >
These are all pretty minor comments; with or without them,
Reviewed-by: Emily Shaffer <emilyshaffer@google.com>