Firstly, thanks for reviewing my patches. I have even checked out the
other reviews
and improvised the other patches according to reviews as well.
I had a few doubts about this one though.
>> + const struct submodule *sub;
>> + char *sub_key, *remote_key;
>> + char *sub_origin_url, *super_config_url, *displaypath;
>> + struct strbuf sb = STRBUF_INIT;
>> + struct child_process cp = CHILD_PROCESS_INIT;
>> +
>> + if (!is_submodule_active(the_repository, list_item->name))
>> + return;
>
> We can use the_repository here, as we also use child processes to
> recurse, such that we always operate on the_repository as the
> superproject.
>
Sorry, but can you explain this a bit more?
>
>> +
>> + sub = submodule_from_path(null_sha1, list_item->name);
>> +
>> + if (!sub || !sub->url)
>> + die(_("no url found for submodule path '%s' in .gitmodules"),
>> + list_item->name);
>
> We do not die in the shell script when the url is missing in the
> .gitmodules file.
>
Then to have a faithful conversion, IMO, deleting the above lines
would be the correct way?
>> +
>> + prepare_submodule_repo_env(&cp.env_array);
>> + cp.git_cmd = 1;
>> + cp.dir = list_item->name;
>> + argv_array_pushl(&cp.args, "submodule--helper",
>> + "print-default-remote", NULL);
>> + if (capture_command(&cp, &sb, 0))
>> + die(_("failed to get the default remote for submodule '%s'"),
>> + list_item->name);
>> +
>> + strbuf_strip_suffix(&sb, "\n");
>> + remote_key = xstrfmt("remote.%s.url", sb.buf);
>> + strbuf_release(&sb);
>> +
>> + child_process_init(&cp);
>> + prepare_submodule_repo_env(&cp.env_array);
>> + cp.git_cmd = 1;
>> + cp.dir = list_item->name;
>> + argv_array_pushl(&cp.args, "config", remote_key, sub_origin_url, NULL);
>> + if (run_command(&cp))
>> + die(_("failed to update remote for submodule '%s'"),
>> + list_item->name);
>
> While it is a strict conversion from the shell script, we could also
> try to do this in-process:
> 1) we'd find out the submodules git dir using submodule_to_gitdir
> 2) construct the path the the config file as "%s/.gitconfig"
> 3) using git_config_set_in_file (which presumably takes file name,
> key and value) the value can be set
Thanks for pointing that out. That surely reduced a child_process.
Although the path of the config file for the case of submodules
would be constructed by "%s/config".
Thanks,
Prathamesh Chavan
