On 03/14, Stefan Beller wrote:
> On Tue, Mar 14, 2017 at 11:06 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > Brandon Williams <bmwill@xxxxxxxxxx> 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?
>
> Yes. We want to check for the submodule being "initialized", i.e.
> having a url in .git/config. (and the struct submodule reads in both .git/config
> and .gitmodules and overlays them with a given precedence order)
>
> > 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?
>
> The mentioned precedence makes sure to have the right order:
>
> /* Overlay the parsed .gitmodules file with .git/config */
> gitmodules_config();
> git_config(submodule_config, NULL);
>
> such that the sub->url is correct as a URL, but not correct as a boolean
> indicator if the submodule is "initialized".
Thanks for clarifying this.
--
Brandon Williams