git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [GSoC][PATCH 5/8] submodule: port submodule subcommand 'sync' from shell to C

From
Prathamesh Chavan <pc44800@gmail.com>
Date
Jul 20, 2017, 19:36 UTC
Message-ID
<CAME+mvV9-spX0DrToZdtydhZ5cXA5A7R7JCca4f87Rn3tdLUpw@mail.gmail.com>
In-Reply-To
<CAGZ79kZrVQZH3ASoZavEeCSUBei8By1xs3fAQaEbVSCMOXoYxg@mail.gmail.com>

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.

Show 13 quoted lines
>> +       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?
Show 11 quoted lines
>
>> +
>> +       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?

Show 29 quoted lines
>> +
>> +       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

Previous: Stefan BellerNext: Stefan Beller
Message 12 of 18 in “Update: Week 9”
  1. Prathamesh ChavanJul 18, 2017
  2. [GSoC][PATCH 1/8] submodule--helper: introduce get_submodule_displaypath()Prathamesh Chavan, Jul 18, 2017
  3. [GSoC][PATCH 2/8] submodule--helper: introduce for_each_submodule_list()Prathamesh Chavan, Jul 18, 2017
  4. [GSoC][PATCH 3/8] submodule: port set_name_rev() from shell to CPrathamesh Chavan, Jul 18, 2017
  5. [GSoC][PATCH 4/8] submodule: port submodule subcommand 'status' from shell to CPrathamesh Chavan, Jul 18, 2017
  6. Stefan BellerJul 18, 2017
  7. Junio C HamanoJul 18, 2017
  8. Stefan BellerJul 18, 2017
  9. Junio C HamanoJul 18, 2017
  10. [GSoC][PATCH 5/8] submodule: port submodule subcommand 'sync' from shell to CPrathamesh Chavan, Jul 18, 2017
  11. Stefan BellerJul 18, 2017
  12. Prathamesh ChavanJul 20, 2017
  13. Stefan BellerJul 20, 2017
  14. [GSoC][PATCH 6/8] submodule: port submodule subcommand 'deinit' from shell to CPrathamesh Chavan, Jul 18, 2017
  15. Christian CouderJul 19, 2017
  16. [GSoC][PATCH 7/8] diff: change scope of the function count_lines()Prathamesh Chavan, Jul 18, 2017
  17. [GSoC][PATCH 8/8] submodule: port submodule subcommand 'summary' from shell to CPrathamesh Chavan, Jul 18, 2017
  18. Christian CouderJul 19, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.