| Message ID | 31e9b914c4bb500a98fbb14674c93a0d8ece47a2.1628618950.git.jonathantanmy@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | In grep, no adding submodule ODB as alternates | expand |
On Tue, Aug 10, 2021 at 11:28:40AM -0700, Jonathan Tan wrote: > > 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()? > 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. > +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>
> 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()? You mean make add_to_alternates_memory() static? I'm not sure how we can do that - alternates is a concern that extends beyond submodules. > Curious whether add_submodule_odb() can be reduced by calling > add_submodule_odb_by_path() internally. That's a reasonable idea, but I don't think it works in this case - in particular, add_submodule_odb_by_path() dups its argument whereas add_submodule_odb() already has an allocated string that it is prepared to give up ownership of. Also, add_submodule_odb_by_path() is only one line, so it won't save us much.
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); 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); +void add_submodule_odb_by_path(const char *path); int register_all_submodule_odb_as_alternates(void); /*
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(-)