Re: [PATCH v1 1/2] submodule: port submodule subcommand 'sync' from shell to C

[Junio C Hamano]

Prathamesh Chavan  writes:

> +static int print_default_remote(int argc, const char **argv, const char 
> *prefix)
> +{
> + const char *remote;
> +
> + if (argc != 1)
> + die(_("submodule--helper print-default-remote takes no 
> arguments"));
> +
> + remote = get_default_remote();
> + if (remote)
> + printf("%s\n", remote);
> +
> + return 0;
> +}

This is called directly from main and return immediately after
printing, so a small leak of remote does not matter, I guess.

> +static void sync_submodule(const char *path, const char *prefix,
> +unsigned int flags)
> +{
> + const struct submodule *sub;
> + char *remote_key = NULL;
> + char *sub_origin_url, *super_config_url, *displaypath;
> + struct strbuf sb = STRBUF_INIT;
> + struct child_process cp = CHILD_PROCESS_INIT;
> + char *sub_config_path = NULL;
> +
> + if (!is_submodule_active(the_repository, path))
> + return;
> +
> + sub = submodule_from_path(_oid, path);
> +
> + if (sub && sub->url) {
> + if (starts_with_dot_dot_slash(sub->url) || 
> starts_with_dot_slash(sub->url)) {

Not a big deal, but other codepaths seem to fold this pattern into
two lines, i.e.

if (starts_with_dot_dot_slash(sub->url) ||
starts_with_dot_slash(sub->url)) {

> + sub_origin_url = relative_url(remote_url, sub->url, 
> up_path);
> + super_config_url = relative_url(remote_url, sub->url, 
> NULL);

On this side, these two are allocated memory that need to be freed.

> + } else {
> + sub_origin_url = xstrdup(sub->url);
> + super_config_url = xstrdup(sub->url);

This side as well.

> + }
> + } else {
> + sub_origin_url = "";
> + super_config_url = "";

But not these.  You have free() of these two at the end of this
function, which will break things.

[PATCH v1 1/2] submodule: port submodule subcommand 'sync' from shell to C

[Prathamesh Chavan]

Port the submodule subcommand 'sync' from shell to C using the same
mechanism as that used for porting submodule subcommand 'status'.
Hence, here the function cmd_sync() is ported from shell to C.
This is done by introducing four functions: module_sync(),
sync_submodule(), sync_submodule_cb() and print_default_remote().

The function print_default_remote() is introduced for getting
the default remote as stdout.

Mentored-by: Christian Couder 
Mentored-by: Stefan Beller 
Signed-off-by: Prathamesh Chavan 
---
 builtin/submodule--helper.c | 192 
 git-submodule.sh|  57 +
 2 files changed, 193 insertions(+), 56 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index a5c4a8a69..dd7737acd 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -50,6 +50,20 @@ static char *get_default_remote(void)
return ret;
 }
 
+static int print_default_remote(int argc, const char **argv, const char 
*prefix)
+{
+   const char *remote;
+
+   if (argc != 1)
+   die(_("submodule--helper print-default-remote takes no 
arguments"));
+
+   remote = get_default_remote();
+   if (remote)
+   printf("%s\n", remote);
+
+   return 0;
+}
+
 static int starts_with_dot_slash(const char *str)
 {
return str[0] == '.' && is_dir_sep(str[1]);
@@ -358,6 +372,25 @@ static void module_list_active(struct module_list *list)
*list = active_modules;
 }
 
+static char *get_up_path(const char *path)
+{
+   int i;
+   struct strbuf sb = STRBUF_INIT;
+
+   for (i = count_slashes(path); i; i--)
+   strbuf_addstr(, "../");
+
+   /*
+* Check if 'path' ends with slash or not
+* for having the same output for dir/sub_dir
+* and dir/sub_dir/
+*/
+   if (!is_dir_sep(path[strlen(path) - 1]))
+   strbuf_addstr(, "../");
+
+   return strbuf_detach(, NULL);
+}
+
 static int module_list(int argc, const char **argv, const char *prefix)
 {
int i;
@@ -718,6 +751,163 @@ static int module_name(int argc, const char **argv, const 
char *prefix)
return 0;
 }
 
+struct sync_cb {
+   const char *prefix;
+   unsigned int flags;
+};
+
+#define SYNC_CB_INIT { NULL, 0 }
+
+static void sync_submodule(const char *path, const char *prefix,
+  unsigned int flags)
+{
+   const struct submodule *sub;
+   char *remote_key = NULL;
+   char *sub_origin_url, *super_config_url, *displaypath;
+   struct strbuf sb = STRBUF_INIT;
+   struct child_process cp = CHILD_PROCESS_INIT;
+   char *sub_config_path = NULL;
+
+   if (!is_submodule_active(the_repository, path))
+   return;
+
+   sub = submodule_from_path(_oid, path);
+
+   if (sub && sub->url) {
+   if (starts_with_dot_dot_slash(sub->url) || 
starts_with_dot_slash(sub->url)) {
+   char *remote_url, *up_path;
+   char *remote = get_default_remote();
+   strbuf_addf(, "remote.%s.url", remote);
+
+   if (git_config_get_string(sb.buf, _url))
+   remote_url = xgetcwd();
+
+   up_path = get_up_path(path);
+   sub_origin_url = relative_url(remote_url, sub->url, 
up_path);
+   super_config_url = relative_url(remote_url, sub->url, 
NULL);
+
+   free(remote);
+   free(up_path);
+   free(remote_url);
+   } else {
+   sub_origin_url = xstrdup(sub->url);
+   super_config_url = xstrdup(sub->url);
+   }
+   } else {
+   sub_origin_url = "";
+   super_config_url = "";
+   }
+
+   displaypath = get_submodule_displaypath(path, prefix);
+
+   if (!(flags & OPT_QUIET))
+   printf(_("Synchronizing submodule url for '%s'\n"),
+displaypath);
+
+   strbuf_reset();
+   strbuf_addf(, "submodule.%s.url", sub->name);
+   if (git_config_set_gently(sb.buf, super_config_url))
+   die(_("failed to register url for submodule path '%s'"),
+ displaypath);
+
+   if (!is_submodule_populated_gently(path, NULL))
+   goto cleanup;
+
+   prepare_submodule_repo_env(_array);
+   cp.git_cmd = 1;
+   cp.dir = path;
+   argv_array_pushl(, "submodule--helper",
+"print-default-remote", NULL);
+
+   strbuf_reset();
+   if (capture_command(, , 0))
+   die(_("failed to get the default remote for submodule '%s'"),
+ path);
+
+   strbuf_strip_suffix(, "\n");
+   remote_key = xstrfmt("remote.%s.url", sb.buf);
+
+   strbuf_reset();
+