Brandon Williams <bmw...@google.com> writes:
> - /*
> - * Looking up the url in .git/config.
> - * We must not fall back to .gitmodules as we only want
> - * to process configured submodules.
> - */
> - strbuf_reset(&sb);
> - strbuf_addf(&sb, "submodule.%s.url", sub->name);
> - git_config_get_string(sb.buf, &url);
> - if (!url) {
> + /* Check if the submodule has been initialized. */
> + if (!is_submodule_initialized(ce->name)) {
> next_submodule_warn_missing(suc, out, displaypath);
> goto cleanup;
> }
> @@ -835,7 +827,7 @@ static int prepare_to_clone_next_submodule(const struct
> cache_entry *ce,
> argv_array_push(&child->args, "--depth=1");
> argv_array_pushl(&child->args, "--path", sub->path, NULL);
> argv_array_pushl(&child->args, "--name", sub->name, NULL);
> - argv_array_pushl(&child->args, "--url", url, NULL);
> + argv_array_pushl(&child->args, "--url", sub->url, NULL);
Even without this patch, we already had an instance of struct submodule
available in this function, so the query to .git/config this patch removed
was unnecessary?
I am wondering what was meant by the comment "We must not fall back to..."
that is being removed---is that because sub->url can come from .gitmodules
that is in-tree, not from .git/config? If that is the case, doesn't the
change in this hunk change behaviour from using the URL the user prefers
to using the URL the upstream suggests, overriding user's configuration?
> @@ -845,7 +837,6 @@ static int prepare_to_clone_next_submodule(const struct
> cache_entry *ce,
> argv_array_push(&child->args, suc->depth);
>
> cleanup:
> - free(url);
> strbuf_reset(&displaypath_sb);
> strbuf_reset(&sb);
