On Fri, Oct 7, 2016 at 8:06 AM, Heiko Voigt <[email protected]> wrote:
> We run a command for each sha1 change in a submodule. This is
> unnecessary since we can simply batch all sha1's we want to check into
> one command. Lets do it so we can speedup the check when many submodule
> changes are in need of checking.
>
> Signed-off-by: Heiko Voigt <[email protected]>
> ---
> submodule.c | 63
> +++++++++++++++++++++++++++++++++----------------------------
> 1 file changed, 34 insertions(+), 29 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 5044afc2f8..a05c2a34b1 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -529,27 +529,49 @@ static int append_hash_to_argv(const unsigned char
> sha1[20], void *data)
> return 0;
> }
>
> -static int submodule_needs_pushing(const char *path, const unsigned char
> sha1[20])
> +static int check_has_hash(const unsigned char sha1[20], void *data)
> {
> - if (add_submodule_odb(path) || !lookup_commit_reference(sha1))
> + int *has_hash = (int *) data;
> +
> + if (!lookup_commit_reference(sha1))
> + *has_hash = 0;
> +
> + return 0;
> +}
> +
> +static int submodule_has_hashes(const char *path, struct sha1_array *hashes)
> +{
> + int has_hash = 1;
> +
> + if (add_submodule_odb(path))
> + return 0;
> +
> + sha1_array_for_each_unique(hashes, check_has_hash, &has_hash);
> + return has_hash;
> +}
> +
> +static int submodule_needs_pushing(const char *path, struct sha1_array
> *hashes)
> +{
> + if (!submodule_has_hashes(path, hashes))
So the above is an implicit lookup already, but we did that before,
too, so it's fine.
> @@ -658,13 +665,11 @@ int find_unpushed_submodules(struct sha1_array *hashes,
> argv_array_clear(&argv);
>
> for (i = 0; i < submodules.nr; i++) {
> - struct string_list_item *item = &submodules.items[i];
> - struct collect_submodule_from_sha1s_data data;
> - data.submodule_path = item->string;
> - data.needs_pushing = needs_pushing;
> - sha1_array_for_each_unique((struct sha1_array *) item->util,
> - collect_submodules_from_sha1s,
> - &data);
> + struct string_list_item *submodule = &submodules.items[i];
> + struct sha1_array *hashes = (struct sha1_array *)
> submodule->util;
> +
> + if (submodule_needs_pushing(submodule->string, hashes))
> + string_list_insert(needs_pushing, submodule->string);
That makes sense.
Thanks!
Stefan
