Re: [PATCH v3 04/10] submodule--helper clone: check for configured submodules using helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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".



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]