{"thread":{"id":"47577","subject":"[PATCH v1 0/2] Incremental rewrite of git-submodules","startedAt":"2018-01-09T18:00:59Z","lastAt":"2018-01-16T19:32:49Z","messageCount":19,"participants":["Prathamesh Chavan","Stefan Beller","Brandon Williams","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"336270","messageId":"20180109175703.4793-1-pc44800@gmail.com","threadId":"47577","inReplyTo":null,"subject":"[PATCH v1 0/2] Incremental rewrite of git-submodules","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-01-09T17:57:01Z","receivedAt":"2018-01-09T18:00:59Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"The patches [1] and [2] concerning the porting of submodule\nsubcommands: sync and deinit were updated in accoudance with\nthe changes made in one of such similar portings made earlier\nfor submodule subcommand status[3]. Following are the changes\nmade:\n\n* It was observed that the number of params increased a lot due to flags\n  like quiet, recursive, cached, etc, and keeping in mind the future\n  subcommand's ported functions as well, a single unsigned int called\n  flags was introduced to store all of these flags, instead of having\n  parameter for each one.\n\n* To accomodate the possiblity of a direct call to the functions\n  deinit_submodule() and sync_submodule(), callback functions were\n  introduced.\n\nAs before you can find this series at: \nhttps://github.com/pratham-pc/git/commits/patch-series-2\n\nAnd its build report is available at: \nhttps://travis-ci.org/pratham-pc/git/builds/\nBranch: patch-series-2\nBuild #195\n\n[1]: https://public-inbox.org/git/20170807211900.15001-6-pc44800@gmail.com/\n[2]: https://public-inbox.org/git/20170807211900.15001-7-pc44800@gmail.com/\n[3]: https://public-inbox.org/git/20171006132415.2876-4-pc44800@gmail.com/\n\nPrathamesh Chavan (2):\n  submodule: port submodule subcommand 'sync' from shell to C\n  submodule: port submodule subcommand 'deinit' from shell to C\n\n builtin/submodule--helper.c | 345 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            | 112 +-------------\n 2 files changed, 347 insertions(+), 110 deletions(-)\n\n-- \n2.14.2\n\n"},{"id":"336271","messageId":"20180109175703.4793-2-pc44800@gmail.com","threadId":"47577","inReplyTo":"20180109175703.4793-1-pc44800@gmail.com","subject":"[PATCH v1 1/2] submodule: port submodule subcommand 'sync' from shell to C","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-01-09T17:57:02Z","receivedAt":"2018-01-09T18:01:23Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"Port the submodule subcommand 'sync' from shell to C using the same\nmechanism as that used for porting submodule subcommand 'status'.\nHence, here the function cmd_sync() is ported from shell to C.\nThis is done by introducing four functions: module_sync(),\nsync_submodule(), sync_submodule_cb() and print_default_remote().\n\nThe function print_default_remote() is introduced for getting\nthe default remote as stdout.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Prathamesh Chavan <pc44800@gmail.com>\n---\n builtin/submodule--helper.c | 192 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            |  57 +------------\n 2 files changed, 193 insertions(+), 56 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex a5c4a8a69..dd7737acd 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -50,6 +50,20 @@ static char *get_default_remote(void)\n \treturn ret;\n }\n \n+static int print_default_remote(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *remote;\n+\n+\tif (argc != 1)\n+\t\tdie(_(\"submodule--helper print-default-remote takes no arguments\"));\n+\n+\tremote = get_default_remote();\n+\tif (remote)\n+\t\tprintf(\"%s\\n\", remote);\n+\n+\treturn 0;\n+}\n+\n static int starts_with_dot_slash(const char *str)\n {\n \treturn str[0] == '.' && is_dir_sep(str[1]);\n@@ -358,6 +372,25 @@ static void module_list_active(struct module_list *list)\n \t*list = active_modules;\n }\n \n+static char *get_up_path(const char *path)\n+{\n+\tint i;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tfor (i = count_slashes(path); i; i--)\n+\t\tstrbuf_addstr(&sb, \"../\");\n+\n+\t/*\n+\t * Check if 'path' ends with slash or not\n+\t * for having the same output for dir/sub_dir\n+\t * and dir/sub_dir/\n+\t */\n+\tif (!is_dir_sep(path[strlen(path) - 1]))\n+\t\tstrbuf_addstr(&sb, \"../\");\n+\n+\treturn strbuf_detach(&sb, NULL);\n+}\n+\n static int module_list(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n@@ -718,6 +751,163 @@ static int module_name(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+struct sync_cb {\n+\tconst char *prefix;\n+\tunsigned int flags;\n+};\n+\n+#define SYNC_CB_INIT { NULL, 0 }\n+\n+static void sync_submodule(const char *path, const char *prefix,\n+\t\t\t   unsigned int flags)\n+{\n+\tconst struct submodule *sub;\n+\tchar *remote_key = NULL;\n+\tchar *sub_origin_url, *super_config_url, *displaypath;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tchar *sub_config_path = NULL;\n+\n+\tif (!is_submodule_active(the_repository, path))\n+\t\treturn;\n+\n+\tsub = submodule_from_path(&null_oid, path);\n+\n+\tif (sub && sub->url) {\n+\t\tif (starts_with_dot_dot_slash(sub->url) || starts_with_dot_slash(sub->url)) {\n+\t\t\tchar *remote_url, *up_path;\n+\t\t\tchar *remote = get_default_remote();\n+\t\t\tstrbuf_addf(&sb, \"remote.%s.url\", remote);\n+\n+\t\t\tif (git_config_get_string(sb.buf, &remote_url))\n+\t\t\t\tremote_url = xgetcwd();\n+\n+\t\t\tup_path = get_up_path(path);\n+\t\t\tsub_origin_url = relative_url(remote_url, sub->url, up_path);\n+\t\t\tsuper_config_url = relative_url(remote_url, sub->url, NULL);\n+\n+\t\t\tfree(remote);\n+\t\t\tfree(up_path);\n+\t\t\tfree(remote_url);\n+\t\t} else {\n+\t\t\tsub_origin_url = xstrdup(sub->url);\n+\t\t\tsuper_config_url = xstrdup(sub->url);\n+\t\t}\n+\t} else {\n+\t\tsub_origin_url = \"\";\n+\t\tsuper_config_url = \"\";\n+\t}\n+\n+\tdisplaypath = get_submodule_displaypath(path, prefix);\n+\n+\tif (!(flags & OPT_QUIET))\n+\t\tprintf(_(\"Synchronizing submodule url for '%s'\\n\"),\n+\t\t\t displaypath);\n+\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n+\tif (git_config_set_gently(sb.buf, super_config_url))\n+\t\tdie(_(\"failed to register url for submodule path '%s'\"),\n+\t\t      displaypath);\n+\n+\tif (!is_submodule_populated_gently(path, NULL))\n+\t\tgoto cleanup;\n+\n+\tprepare_submodule_repo_env(&cp.env_array);\n+\tcp.git_cmd = 1;\n+\tcp.dir = path;\n+\targv_array_pushl(&cp.args, \"submodule--helper\",\n+\t\t\t \"print-default-remote\", NULL);\n+\n+\tstrbuf_reset(&sb);\n+\tif (capture_command(&cp, &sb, 0))\n+\t\tdie(_(\"failed to get the default remote for submodule '%s'\"),\n+\t\t      path);\n+\n+\tstrbuf_strip_suffix(&sb, \"\\n\");\n+\tremote_key = xstrfmt(\"remote.%s.url\", sb.buf);\n+\n+\tstrbuf_reset(&sb);\n+\tsubmodule_to_gitdir(&sb, path);\n+\tstrbuf_addstr(&sb, \"/config\");\n+\n+\tif (git_config_set_in_file_gently(sb.buf, remote_key, sub_origin_url))\n+\t\tdie(_(\"failed to update remote for submodule '%s'\"),\n+\t\t      path);\n+\n+\tif (flags & OPT_RECURSIVE) {\n+\t\tstruct child_process cpr = CHILD_PROCESS_INIT;\n+\n+\t\tcpr.git_cmd = 1;\n+\t\tcpr.dir = path;\n+\t\tprepare_submodule_repo_env(&cpr.env_array);\n+\n+\t\targv_array_push(&cpr.args, \"--super-prefix\");\n+\t\targv_array_pushf(&cpr.args, \"%s/\", displaypath);\n+\t\targv_array_pushl(&cpr.args, \"submodule--helper\", \"sync\",\n+\t\t\t\t \"--recursive\", NULL);\n+\n+\t\tif (flags & OPT_QUIET)\n+\t\t\targv_array_push(&cpr.args, \"--quiet\");\n+\n+\t\tif (run_command(&cpr))\n+\t\t\tdie(_(\"failed to recurse into submodule '%s'\"),\n+\t\t\t      path);\n+\t}\n+\n+cleanup:\n+\tstrbuf_release(&sb);\n+\tfree(remote_key);\n+\tfree(super_config_url);\n+\tfree(displaypath);\n+\tfree(sub_config_path);\n+\tfree(sub_origin_url);\n+}\n+\n+static void sync_submodule_cb(const struct cache_entry *list_item, void *cb_data)\n+{\n+\tstruct sync_cb *info = cb_data;\n+\tsync_submodule(list_item->name, info->prefix, info->flags);\n+\n+}\n+\n+static int module_sync(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct sync_cb info = SYNC_CB_INIT;\n+\tstruct pathspec pathspec;\n+\tstruct module_list list = MODULE_LIST_INIT;\n+\tint quiet = 0;\n+\tint recursive = 0;\n+\n+\tstruct option module_sync_options[] = {\n+\t\tOPT__QUIET(&quiet, N_(\"Suppress output of synchronizing submodule url\")),\n+\t\tOPT_BOOL(0, \"recursive\", &recursive,\n+\t\t\tN_(\"Recurse into nested submodules\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper sync [--quiet] [--recursive] [<path>]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_sync_options,\n+\t\t\t     git_submodule_helper_usage, 0);\n+\n+\tif (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)\n+\t\treturn 1;\n+\n+\tinfo.prefix = prefix;\n+\tif (quiet)\n+\t\tinfo.flags |= OPT_QUIET;\n+\tif (recursive)\n+\t\tinfo.flags |= OPT_RECURSIVE;\n+\n+\tfor_each_listed_submodule(&list, sync_submodule_cb, &info);\n+\n+\treturn 0;\n+}\n+\n static int clone_submodule(const char *path, const char *gitdir, const char *url,\n \t\t\t   const char *depth, struct string_list *reference,\n \t\t\t   int quiet, int progress)\n@@ -1498,6 +1688,8 @@ static struct cmd_struct commands[] = {\n \t{\"resolve-relative-url-test\", resolve_relative_url_test, 0},\n \t{\"init\", module_init, SUPPORT_SUPER_PREFIX},\n \t{\"status\", module_status, SUPPORT_SUPER_PREFIX},\n+\t{\"print-default-remote\", print_default_remote, 0},\n+\t{\"sync\", module_sync, SUPPORT_SUPER_PREFIX},\n \t{\"remote-branch\", resolve_remote_submodule_branch, 0},\n \t{\"push-check\", push_check, 0},\n \t{\"absorb-git-dirs\", absorb_git_dirs, SUPPORT_SUPER_PREFIX},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 156255a9e..0825cae14 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -1036,63 +1036,8 @@ cmd_sync()\n \t\t\t;;\n \t\tesac\n \tdone\n-\tcd_to_toplevel\n-\t{\n-\t\tgit submodule--helper list --prefix \"$wt_prefix\" \"$@\" ||\n-\t\techo \"#unmatched\" $?\n-\t} |\n-\twhile read -r mode sha1 stage sm_path\n-\tdo\n-\t\tdie_if_unmatched \"$mode\" \"$sha1\"\n \n-\t\t# skip inactive submodules\n-\t\tif ! git submodule--helper is-active \"$sm_path\"\n-\t\tthen\n-\t\t\tcontinue\n-\t\tfi\n-\n-\t\tname=$(git submodule--helper name \"$sm_path\")\n-\t\turl=$(git config -f .gitmodules --get submodule.\"$name\".url)\n-\n-\t\t# Possibly a url relative to parent\n-\t\tcase \"$url\" in\n-\t\t./*|../*)\n-\t\t\t# rewrite foo/bar as ../.. to find path from\n-\t\t\t# submodule work tree to superproject work tree\n-\t\t\tup_path=\"$(printf '%s\\n' \"$sm_path\" | sed \"s/[^/][^/]*/../g\")\" &&\n-\t\t\t# guarantee a trailing /\n-\t\t\tup_path=${up_path%/}/ &&\n-\t\t\t# path from submodule work tree to submodule origin repo\n-\t\t\tsub_origin_url=$(git submodule--helper resolve-relative-url \"$url\" \"$up_path\") &&\n-\t\t\t# path from superproject work tree to submodule origin repo\n-\t\t\tsuper_config_url=$(git submodule--helper resolve-relative-url \"$url\") || exit\n-\t\t\t;;\n-\t\t*)\n-\t\t\tsub_origin_url=\"$url\"\n-\t\t\tsuper_config_url=\"$url\"\n-\t\t\t;;\n-\t\tesac\n-\n-\t\tdisplaypath=$(git submodule--helper relative-path \"$prefix$sm_path\" \"$wt_prefix\")\n-\t\tsay \"$(eval_gettext \"Synchronizing submodule url for '\\$displaypath'\")\"\n-\t\tgit config submodule.\"$name\".url \"$super_config_url\"\n-\n-\t\tif test -e \"$sm_path\"/.git\n-\t\tthen\n-\t\t(\n-\t\t\tsanitize_submodule_env\n-\t\t\tcd \"$sm_path\"\n-\t\t\tremote=$(get_default_remote)\n-\t\t\tgit config remote.\"$remote\".url \"$sub_origin_url\"\n-\n-\t\t\tif test -n \"$recursive\"\n-\t\t\tthen\n-\t\t\t\tprefix=\"$prefix$sm_path/\"\n-\t\t\t\teval cmd_sync\n-\t\t\tfi\n-\t\t)\n-\t\tfi\n-\tdone\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} ${prefix:+--super-prefix \"$prefix\"} submodule--helper sync ${GIT_QUIET:+--quiet} ${recursive:+--recursive} \"$@\"\n }\n \n cmd_absorbgitdirs()\n-- \n2.14.2\n\n"},{"id":"336272","messageId":"20180109175703.4793-3-pc44800@gmail.com","threadId":"47577","inReplyTo":"20180109175703.4793-1-pc44800@gmail.com","subject":"[PATCH v1 2/2] submodule: port submodule subcommand 'deinit' from shell to C","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-01-09T17:57:03Z","receivedAt":"2018-01-09T18:01:29Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"The same mechanism is used even for porting this submodule\nsubcommand, as used in the ported subcommands till now.\nThe function cmd_deinit in split up after porting into four\nfunctions: module_deinit(), for_each_listed_submodule(),\ndeinit_submodule() and deinit_submodule_cb().\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Prathamesh Chavan <pc44800@gmail.com>\n---\n builtin/submodule--helper.c | 153 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            |  55 +---------------\n 2 files changed, 154 insertions(+), 54 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex dd7737acd..54b0e46fc 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -20,6 +20,7 @@\n #define OPT_QUIET (1 << 0)\n #define OPT_CACHED (1 << 1)\n #define OPT_RECURSIVE (1 << 2)\n+#define OPT_FORCE (1 << 3)\n \n typedef void (*each_submodule_fn)(const struct cache_entry *list_item,\n \t\t\t\t  void *cb_data);\n@@ -908,6 +909,157 @@ static int module_sync(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+struct deinit_cb {\n+\tconst char *prefix;\n+\tunsigned int flags;\n+};\n+#define DEINIT_CB_INIT { NULL, 0 }\n+\n+static void deinit_submodule(const char *path, const char *prefix,\n+\t\t\t     unsigned int flags)\n+{\n+\tconst struct submodule *sub;\n+\tchar *displaypath = NULL;\n+\tstruct child_process cp_config = CHILD_PROCESS_INIT;\n+\tstruct strbuf sb_config = STRBUF_INIT;\n+\tchar *sub_git_dir = xstrfmt(\"%s/.git\", path);\n+\tmode_t mode = 0777;\n+\n+\tsub = submodule_from_path(&null_oid, path);\n+\n+\tif (!sub || !sub->name)\n+\t\tgoto cleanup;\n+\n+\tdisplaypath = get_submodule_displaypath(path, prefix);\n+\n+\t/* remove the submodule work tree (unless the user already did it) */\n+\tif (is_directory(path)) {\n+\t\tstruct stat st;\n+\t\t/*\n+\t\t * protect submodules containing a .git directory\n+\t\t * NEEDSWORK: automatically call absorbgitdirs before\n+\t\t * warning/die.\n+\t\t */\n+\t\tif (is_directory(sub_git_dir))\n+\t\t\tdie(_(\"Submodule work tree '%s' contains a .git \"\n+\t\t\t      \"directory use 'rm -rf' if you really want \"\n+\t\t\t      \"to remove it including all of its history\"),\n+\t\t\t      displaypath);\n+\n+\t\tif (!(flags & OPT_FORCE)) {\n+\t\t\tstruct child_process cp_rm = CHILD_PROCESS_INIT;\n+\t\t\tcp_rm.git_cmd = 1;\n+\t\t\targv_array_pushl(&cp_rm.args, \"rm\", \"-qn\",\n+\t\t\t\t\t path, NULL);\n+\n+\t\t\tif (run_command(&cp_rm))\n+\t\t\t\tdie(_(\"Submodule work tree '%s' contains local \"\n+\t\t\t\t      \"modifications; use '-f' to discard them\"),\n+\t\t\t\t      displaypath);\n+\t\t}\n+\n+\t\tif (!lstat(path, &st)) {\n+\t\t\tstruct strbuf sb_rm = STRBUF_INIT;\n+\t\t\tconst char *format;\n+\n+\t\t\tstrbuf_addstr(&sb_rm, path);\n+\n+\t\t\tif (!remove_dir_recursively(&sb_rm, 0))\n+\t\t\t\tformat = _(\"Cleared directory '%s'\\n\");\n+\t\t\telse\n+\t\t\t\tformat = _(\"Could not remove submodule work tree '%s'\\n\");\n+\n+\t\t\tif (!(flags & OPT_QUIET))\n+\t\t\t\tprintf(format, displaypath);\n+\n+\t\t\tmode = st.st_mode;\n+\n+\t\t\tstrbuf_release(&sb_rm);\n+\t\t}\n+\t}\n+\n+\tif (mkdir(path, mode))\n+\t\tdie_errno(_(\"could not create empty submodule directory %s\"),\n+\t\t      displaypath);\n+\n+\tcp_config.git_cmd = 1;\n+\targv_array_pushl(&cp_config.args, \"config\", \"--get-regexp\", NULL);\n+\targv_array_pushf(&cp_config.args, \"submodule.%s\\\\.\", sub->name);\n+\n+\t/* remove the .git/config entries (unless the user already did it) */\n+\tif (!capture_command(&cp_config, &sb_config, 0) && sb_config.len) {\n+\t\tchar *sub_key = xstrfmt(\"submodule.%s\", sub->name);\n+\t\t/*\n+\t\t * remove the whole section so we have a clean state when\n+\t\t * the user later decides to init this submodule again\n+\t\t */\n+\t\tgit_config_rename_section_in_file(NULL, sub_key, NULL);\n+\t\tif (!(flags & OPT_QUIET))\n+\t\t\tprintf(_(\"Submodule '%s' (%s) unregistered for path '%s'\\n\"),\n+\t\t\t\t sub->name, sub->url, displaypath);\n+\t\tfree(sub_key);\n+\t}\n+\n+cleanup:\n+\tfree(displaypath);\n+\tfree(sub_git_dir);\n+\tstrbuf_release(&sb_config);\n+}\n+\n+static void deinit_submodule_cb(const struct cache_entry *list_item,\n+\t\t\t\tvoid *cb_data)\n+{\n+\tstruct deinit_cb *info = cb_data;\n+\tdeinit_submodule(list_item->name, info->prefix, info->flags);\n+}\n+\n+static int module_deinit(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct deinit_cb info = DEINIT_CB_INIT;\n+\tstruct pathspec pathspec;\n+\tstruct module_list list = MODULE_LIST_INIT;\n+\tint quiet = 0;\n+\tint force = 0;\n+\tint all = 0;\n+\n+\tstruct option module_deinit_options[] = {\n+\t\tOPT__QUIET(&quiet, N_(\"Suppress submodule status output\")),\n+\t\tOPT__FORCE(&force, N_(\"Remove submodule working trees even if they contain local changes\")),\n+\t\tOPT_BOOL(0, \"all\", &all, N_(\"Unregister all submodules\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule deinit [--quiet] [-f | --force] [--all | [--] [<path>...]]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_deinit_options,\n+\t\t\t     git_submodule_helper_usage, 0);\n+\n+\tif (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)\n+\t\tBUG(\"module_list_compute should not choke on empty pathspec\");\n+\n+\tinfo.prefix = prefix;\n+\tif (quiet)\n+\t\tinfo.flags |= OPT_QUIET;\n+\tif (force)\n+\t\tinfo.flags |= OPT_FORCE;\n+\n+\tif (all && argc) {\n+\t\terror(\"pathspec and --all are incompatible\");\n+\t\tusage_with_options(git_submodule_helper_usage,\n+\t\t\t\t   module_deinit_options);\n+\t}\n+\n+\tif (!argc && !all)\n+\t\tdie(_(\"Use '--all' if you really want to deinitialize all submodules\"));\n+\n+\tfor_each_listed_submodule(&list, deinit_submodule_cb, &info);\n+\n+\treturn 0;\n+}\n+\n static int clone_submodule(const char *path, const char *gitdir, const char *url,\n \t\t\t   const char *depth, struct string_list *reference,\n \t\t\t   int quiet, int progress)\n@@ -1690,6 +1842,7 @@ static struct cmd_struct commands[] = {\n \t{\"status\", module_status, SUPPORT_SUPER_PREFIX},\n \t{\"print-default-remote\", print_default_remote, 0},\n \t{\"sync\", module_sync, SUPPORT_SUPER_PREFIX},\n+\t{\"deinit\", module_deinit, 0},\n \t{\"remote-branch\", resolve_remote_submodule_branch, 0},\n \t{\"push-check\", push_check, 0},\n \t{\"absorb-git-dirs\", absorb_git_dirs, SUPPORT_SUPER_PREFIX},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 0825cae14..24914963c 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -428,60 +428,7 @@ cmd_deinit()\n \t\tshift\n \tdone\n \n-\tif test -n \"$deinit_all\" && test \"$#\" -ne 0\n-\tthen\n-\t\techo >&2 \"$(eval_gettext \"pathspec and --all are incompatible\")\"\n-\t\tusage\n-\tfi\n-\tif test $# = 0 && test -z \"$deinit_all\"\n-\tthen\n-\t\tdie \"$(eval_gettext \"Use '--all' if you really want to deinitialize all submodules\")\"\n-\tfi\n-\n-\t{\n-\t\tgit submodule--helper list --prefix \"$wt_prefix\" \"$@\" ||\n-\t\techo \"#unmatched\" $?\n-\t} |\n-\twhile read -r mode sha1 stage sm_path\n-\tdo\n-\t\tdie_if_unmatched \"$mode\" \"$sha1\"\n-\t\tname=$(git submodule--helper name \"$sm_path\") || exit\n-\n-\t\tdisplaypath=$(git submodule--helper relative-path \"$sm_path\" \"$wt_prefix\")\n-\n-\t\t# Remove the submodule work tree (unless the user already did it)\n-\t\tif test -d \"$sm_path\"\n-\t\tthen\n-\t\t\t# Protect submodules containing a .git directory\n-\t\t\tif test -d \"$sm_path/.git\"\n-\t\t\tthen\n-\t\t\t\tdie \"$(eval_gettext \"\\\n-Submodule work tree '\\$displaypath' contains a .git directory\n-(use 'rm -rf' if you really want to remove it including all of its history)\")\"\n-\t\t\tfi\n-\n-\t\t\tif test -z \"$force\"\n-\t\t\tthen\n-\t\t\t\tgit rm -qn \"$sm_path\" ||\n-\t\t\t\tdie \"$(eval_gettext \"Submodule work tree '\\$displaypath' contains local modifications; use '-f' to discard them\")\"\n-\t\t\tfi\n-\t\t\trm -rf \"$sm_path\" &&\n-\t\t\tsay \"$(eval_gettext \"Cleared directory '\\$displaypath'\")\" ||\n-\t\t\tsay \"$(eval_gettext \"Could not remove submodule work tree '\\$displaypath'\")\"\n-\t\tfi\n-\n-\t\tmkdir \"$sm_path\" || say \"$(eval_gettext \"Could not create empty submodule directory '\\$displaypath'\")\"\n-\n-\t\t# Remove the .git/config entries (unless the user already did it)\n-\t\tif test -n \"$(git config --get-regexp submodule.\"$name\\.\")\"\n-\t\tthen\n-\t\t\t# Remove the whole section so we have a clean state when\n-\t\t\t# the user later decides to init this submodule again\n-\t\t\turl=$(git config submodule.\"$name\".url)\n-\t\t\tgit config --remove-section submodule.\"$name\" 2>/dev/null &&\n-\t\t\tsay \"$(eval_gettext \"Submodule '\\$name' (\\$url) unregistered for path '\\$displaypath'\")\"\n-\t\tfi\n-\tdone\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper deinit ${GIT_QUIET:+--quiet} ${prefix:+--prefix \"$prefix\"} ${force:+--force} ${deinit_all:+--all} \"$@\"\n }\n \n is_tip_reachable () (\n-- \n2.14.2\n\n"},{"id":"336300","messageId":"CAGZ79kZs9fNOZ0wCahRrwRn8k3rHOAXCchyLmb0AaC_qCNFx4A@mail.gmail.com","threadId":"47577","inReplyTo":"20180109175703.4793-1-pc44800@gmail.com","subject":"Re: [PATCH v1 0/2] Incremental rewrite of git-submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-01-09T19:25:17Z","receivedAt":"2018-01-09T19:25:24Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jan 9, 2018 at 9:57 AM, Prathamesh Chavan <pc44800@gmail.com> wrote:\n> The patches [1] and [2] concerning the porting of submodule\n> subcommands: sync and deinit were updated in accoudance with\n> the changes made in one of such similar portings made earlier\n> for submodule subcommand status[3]. Following are the changes\n> made:\n>\n> * It was observed that the number of params increased a lot due to flags\n>   like quiet, recursive, cached, etc, and keeping in mind the future\n>   subcommand's ported functions as well, a single unsigned int called\n>   flags was introduced to store all of these flags, instead of having\n>   parameter for each one.\n>\n> * To accomodate the possiblity of a direct call to the functions\n>   deinit_submodule() and sync_submodule(), callback functions were\n>   introduced.\n>\n> As before you can find this series at:\n> https://github.com/pratham-pc/git/commits/patch-series-2\n>\n> And its build report is available at:\n> https://travis-ci.org/pratham-pc/git/builds/\n> Branch: patch-series-2\n> Build #195\n\nCool!\n\nI have reviewed both patches and found them good to apply;\n\nThanks,\nStefan\n"},{"id":"336310","messageId":"20180109200652.GE151395@google.com","threadId":"47577","inReplyTo":"20180109175703.4793-1-pc44800@gmail.com","subject":"Re: [PATCH v1 0/2] Incremental rewrite of git-submodules","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2018-01-09T20:06:52Z","receivedAt":"2018-01-09T20:07:02Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 01/09, Prathamesh Chavan wrote:\n> The patches [1] and [2] concerning the porting of submodule\n> subcommands: sync and deinit were updated in accoudance with\n> the changes made in one of such similar portings made earlier\n> for submodule subcommand status[3]. Following are the changes\n> made:\n\nThe two patches look good to me.  Thanks for continuing this work!\n\n> \n> * It was observed that the number of params increased a lot due to flags\n>   like quiet, recursive, cached, etc, and keeping in mind the future\n>   subcommand's ported functions as well, a single unsigned int called\n>   flags was introduced to store all of these flags, instead of having\n>   parameter for each one.\n\nThis is unfortunate.  The use of a flag word or using bit-fields are\nessentially equivalent so its unfortunate that the conversion to using\none or the other caused review churn.  My own preference would be to use\nbit-fields ;)  I also noticed that the flags you are using start with\nOPT_* which conflict with the parse-options namespace, sorry for not\ncatching this when a few of your older patches made it into master.\nThis isn't a big deal since no symbols look to collide so I am not\nsuggesting you change this since I would prefer to eliminate more\nunnecessary review churn on this series.\n\n> \n> * To accomodate the possiblity of a direct call to the functions\n>   deinit_submodule() and sync_submodule(), callback functions were\n>   introduced.\n> \n> As before you can find this series at: \n> https://github.com/pratham-pc/git/commits/patch-series-2\n> \n> And its build report is available at: \n> https://travis-ci.org/pratham-pc/git/builds/\n> Branch: patch-series-2\n> Build #195\n> \n> [1]: https://public-inbox.org/git/20170807211900.15001-6-pc44800@gmail.com/\n> [2]: https://public-inbox.org/git/20170807211900.15001-7-pc44800@gmail.com/\n> [3]: https://public-inbox.org/git/20171006132415.2876-4-pc44800@gmail.com/\n> \n> Prathamesh Chavan (2):\n>   submodule: port submodule subcommand 'sync' from shell to C\n>   submodule: port submodule subcommand 'deinit' from shell to C\n> \n>  builtin/submodule--helper.c | 345 ++++++++++++++++++++++++++++++++++++++++++++\n>  git-submodule.sh            | 112 +-------------\n>  2 files changed, 347 insertions(+), 110 deletions(-)\n> \n> -- \n> 2.14.2\n> \n\n-- \nBrandon Williams\n"},{"id":"336317","messageId":"xmqqpo6i4uns.fsf@gitster.mtv.corp.google.com","threadId":"47577","inReplyTo":"20180109175703.4793-2-pc44800@gmail.com","subject":"Re: [PATCH v1 1/2] submodule: port submodule subcommand 'sync' from shell to C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-09T20:57:43Z","receivedAt":"2018-01-09T20:59:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Prathamesh Chavan <pc44800@gmail.com> writes:\n\n> +static int print_default_remote(int argc, const char **argv, const char *prefix)\n> +{\n> +\tconst char *remote;\n> +\n> +\tif (argc != 1)\n> +\t\tdie(_(\"submodule--helper print-default-remote takes no arguments\"));\n> +\n> +\tremote = get_default_remote();\n> +\tif (remote)\n> +\t\tprintf(\"%s\\n\", remote);\n> +\n> +\treturn 0;\n> +}\n\nThis is called directly from main and return immediately after\nprinting, so a small leak of remote does not matter, I guess.\n\n> +static void sync_submodule(const char *path, const char *prefix,\n> +\t\t\t   unsigned int flags)\n> +{\n> +\tconst struct submodule *sub;\n> +\tchar *remote_key = NULL;\n> +\tchar *sub_origin_url, *super_config_url, *displaypath;\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tstruct child_process cp = CHILD_PROCESS_INIT;\n> +\tchar *sub_config_path = NULL;\n> +\n> +\tif (!is_submodule_active(the_repository, path))\n> +\t\treturn;\n> +\n> +\tsub = submodule_from_path(&null_oid, path);\n> +\n> +\tif (sub && sub->url) {\n> +\t\tif (starts_with_dot_dot_slash(sub->url) || starts_with_dot_slash(sub->url)) {\n\nNot a big deal, but other codepaths seem to fold this pattern into\ntwo lines, i.e.\n\n\t\tif (starts_with_dot_dot_slash(sub->url) ||\n\t\t    starts_with_dot_slash(sub->url)) {\n\n> +\t\t\tsub_origin_url = relative_url(remote_url, sub->url, up_path);\n> +\t\t\tsuper_config_url = relative_url(remote_url, sub->url, NULL);\n\nOn this side, these two are allocated memory that need to be freed.\n\n> +\t\t} else {\n> +\t\t\tsub_origin_url = xstrdup(sub->url);\n> +\t\t\tsuper_config_url = xstrdup(sub->url);\n\nThis side as well.\n\n> +\t\t}\n> +\t} else {\n> +\t\tsub_origin_url = \"\";\n> +\t\tsuper_config_url = \"\";\n\nBut not these.  You have free() of these two at the end of this\nfunction, which will break things.\n\n"},{"id":"336318","messageId":"xmqq7esq4tf6.fsf@gitster.mtv.corp.google.com","threadId":"47577","inReplyTo":"20180109175703.4793-3-pc44800@gmail.com","subject":"Re: [PATCH v1 2/2] submodule: port submodule subcommand 'deinit' from shell to C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-09T21:24:29Z","receivedAt":"2018-01-09T21:24:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Prathamesh Chavan <pc44800@gmail.com> writes:\n\n> The same mechanism is used even for porting this submodule\n> subcommand, as used in the ported subcommands till now.\n> The function cmd_deinit in split up after porting into four\n> functions: module_deinit(), for_each_listed_submodule(),\n> deinit_submodule() and deinit_submodule_cb().\n>\n> Mentored-by: Christian Couder <christian.couder@gmail.com>\n> Mentored-by: Stefan Beller <sbeller@google.com>\n> Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>\n> ---\n>  builtin/submodule--helper.c | 153 ++++++++++++++++++++++++++++++++++++++++++++\n>  git-submodule.sh            |  55 +---------------\n>  2 files changed, 154 insertions(+), 54 deletions(-)\n>\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index dd7737acd..54b0e46fc 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -20,6 +20,7 @@\n>  #define OPT_QUIET (1 << 0)\n>  #define OPT_CACHED (1 << 1)\n>  #define OPT_RECURSIVE (1 << 2)\n> +#define OPT_FORCE (1 << 3)\n>  \n>  typedef void (*each_submodule_fn)(const struct cache_entry *list_item,\n>  \t\t\t\t  void *cb_data);\n> @@ -908,6 +909,157 @@ static int module_sync(int argc, const char **argv, const char *prefix)\n>  \treturn 0;\n>  }\n>  \n> +struct deinit_cb {\n> +\tconst char *prefix;\n> +\tunsigned int flags;\n> +};\n> +#define DEINIT_CB_INIT { NULL, 0 }\n> +\n> +static void deinit_submodule(const char *path, const char *prefix,\n> +\t\t\t     unsigned int flags)\n> +{\n> +\tconst struct submodule *sub;\n> +\tchar *displaypath = NULL;\n> +\tstruct child_process cp_config = CHILD_PROCESS_INIT;\n> +\tstruct strbuf sb_config = STRBUF_INIT;\n> +\tchar *sub_git_dir = xstrfmt(\"%s/.git\", path);\n> +\tmode_t mode = 0777;\n> +\n> +\tsub = submodule_from_path(&null_oid, path);\n> +\n> +\tif (!sub || !sub->name)\n> +\t\tgoto cleanup;\n> +\n> +\tdisplaypath = get_submodule_displaypath(path, prefix);\n> +\n> +\t/* remove the submodule work tree (unless the user already did it) */\n> +\tif (is_directory(path)) {\n> +\t\tstruct stat st;\n> +\t\t/*\n> +\t\t * protect submodules containing a .git directory\n> +\t\t * NEEDSWORK: automatically call absorbgitdirs before\n> +\t\t * warning/die.\n> +\t\t */\n\nI guess that you mean \"instead of dying, automatically call absorb\nand (possibly) warn\"?  That sounds like a sensible improvement.\n\n> +\t\tif (is_directory(sub_git_dir))\n> +\t\t\tdie(_(\"Submodule work tree '%s' contains a .git \"\n> +\t\t\t      \"directory use 'rm -rf' if you really want \"\n> +\t\t\t      \"to remove it including all of its history\"),\n\nThis changes the message text by removing () around \"use ... history\",\nwhich I do not think you intended to do.\n\n> +\t\t\t      displaypath);\n> +\n> +\t\tif (!(flags & OPT_FORCE)) {\n> +\t\t\tstruct child_process cp_rm = CHILD_PROCESS_INIT;\n> +\t\t\tcp_rm.git_cmd = 1;\n> +\t\t\targv_array_pushl(&cp_rm.args, \"rm\", \"-qn\",\n> +\t\t\t\t\t path, NULL);\n> +\n> +\t\t\tif (run_command(&cp_rm))\n> +\t\t\t\tdie(_(\"Submodule work tree '%s' contains local \"\n> +\t\t\t\t      \"modifications; use '-f' to discard them\"),\n> +\t\t\t\t      displaypath);\n> +\t\t}\n> +\n> +\t\tif (!lstat(path, &st)) {\n\nWhat is this if statement doing here?  It does not make sense,\nespecially without an 'else' clause on the other side, at least to\nme.\n\nAt this point in the flow, the code has already determined that path\nis a directory above before starting to check if it has \".git/\"\nimmediately below it, or trying to run \"git rm\" in the dry run mode\nto see if it yields an error, so at this point lstat() should\nsucceed (and would say it is a directory).  I would sort-of\nunderstand it if this \"if()\" has an \"else\" clause to act on an\nerror, but that is not something the original does not do, so I am\nnot sure if it belongs to a \"rewrite to C\" patch.\n\n> +\t\t\tstruct strbuf sb_rm = STRBUF_INIT;\n> +\t\t\tconst char *format;\n> +\n> +\t\t\tstrbuf_addstr(&sb_rm, path);\n> +\n> +\t\t\tif (!remove_dir_recursively(&sb_rm, 0))\n> +\t\t\t\tformat = _(\"Cleared directory '%s'\\n\");\n> +\t\t\telse\n> +\t\t\t\tformat = _(\"Could not remove submodule work tree '%s'\\n\");\n> +\n> +\t\t\tif (!(flags & OPT_QUIET))\n> +\t\t\t\tprintf(format, displaypath);\n> +\n> +\t\t\tmode = st.st_mode;\n> +\n> +\t\t\tstrbuf_release(&sb_rm);\n> +\t\t}\n> +\t}\n\nIf the reason is \"avoid losing the original directory mode by\nremoving and recreating\", then you should be able to do much better\nby using REMOVE_DIR_KEEP_TOPLEVEL in the above \"do we still have a\ndirectory?  if so get rid of working tree contents\" thing.  And the\ncall to mkdir() below can be placed in the else clause of that\ncheck, i.e. \"the user has removed the directory as well, but there\nshould be an empty directory even for a de-initialized submodule\"\nside of this.\n\nThat of course does not have to be part of \"rewrite to C\" patch.  In\nfact, it probably should come as a follow-up improvement after the\ndust settles.\n\n> +\tif (mkdir(path, mode))\n> +\t\tdie_errno(_(\"could not create empty submodule directory %s\"),\n> +\t\t      displaypath);\n> + ...\n> +\n> +static int module_deinit(int argc, const char **argv, const char *prefix)\n> +{\n> +...\n> +\tif (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)\n> +\t\tBUG(\"module_list_compute should not choke on empty pathspec\");\n> +...\n> +\tif (all && argc) {\n> +\t\terror(\"pathspec and --all are incompatible\");\n> +\t\tusage_with_options(git_submodule_helper_usage,\n> +\t\t\t\t   module_deinit_options);\n> +\t}\n> +\n> +\tif (!argc && !all)\n> +\t\tdie(_(\"Use '--all' if you really want to deinitialize all submodules\"));\n\nShouldn't these two checks come before we call module_list_compute()?  \n\n> +\n> +\tfor_each_listed_submodule(&list, deinit_submodule_cb, &info);\n> +\n> +\treturn 0;\n> +}\n"},{"id":"336419","messageId":"CAME+mvXaL4AcK1ib2rDZKdH0eLc7te+3e9zYv8pNqNj-4cyT3Q@mail.gmail.com","threadId":"47577","inReplyTo":"xmqq7esq4tf6.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v1 2/2] submodule: port submodule subcommand 'deinit' from shell to C","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-01-10T20:22:58Z","receivedAt":"2018-01-10T20:23:04Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"On Wed, Jan 10, 2018 at 2:54 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Prathamesh Chavan <pc44800@gmail.com> writes:\n>\n>> The same mechanism is used even for porting this submodule\n>> subcommand, as used in the ported subcommands till now.\n>> The function cmd_deinit in split up after porting into four\n>> functions: module_deinit(), for_each_listed_submodule(),\n>> deinit_submodule() and deinit_submodule_cb().\n>>\n>> Mentored-by: Christian Couder <christian.couder@gmail.com>\n>> Mentored-by: Stefan Beller <sbeller@google.com>\n>> Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>\n>> ---\n>>  builtin/submodule--helper.c | 153 ++++++++++++++++++++++++++++++++++++++++++++\n>>  git-submodule.sh            |  55 +---------------\n>>  2 files changed, 154 insertions(+), 54 deletions(-)\n>>\n>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n>> index dd7737acd..54b0e46fc 100644\n>> --- a/builtin/submodule--helper.c\n>> +++ b/builtin/submodule--helper.c\n>> @@ -20,6 +20,7 @@\n>>  #define OPT_QUIET (1 << 0)\n>>  #define OPT_CACHED (1 << 1)\n>>  #define OPT_RECURSIVE (1 << 2)\n>> +#define OPT_FORCE (1 << 3)\n>>\n>>  typedef void (*each_submodule_fn)(const struct cache_entry *list_item,\n>>                                 void *cb_data);\n>> @@ -908,6 +909,157 @@ static int module_sync(int argc, const char **argv, const char *prefix)\n>>       return 0;\n>>  }\n>>\n>> +struct deinit_cb {\n>> +     const char *prefix;\n>> +     unsigned int flags;\n>> +};\n>> +#define DEINIT_CB_INIT { NULL, 0 }\n>> +\n>> +static void deinit_submodule(const char *path, const char *prefix,\n>> +                          unsigned int flags)\n>> +{\n>> +     const struct submodule *sub;\n>> +     char *displaypath = NULL;\n>> +     struct child_process cp_config = CHILD_PROCESS_INIT;\n>> +     struct strbuf sb_config = STRBUF_INIT;\n>> +     char *sub_git_dir = xstrfmt(\"%s/.git\", path);\n>> +     mode_t mode = 0777;\n>> +\n>> +     sub = submodule_from_path(&null_oid, path);\n>> +\n>> +     if (!sub || !sub->name)\n>> +             goto cleanup;\n>> +\n>> +     displaypath = get_submodule_displaypath(path, prefix);\n>> +\n>> +     /* remove the submodule work tree (unless the user already did it) */\n>> +     if (is_directory(path)) {\n>> +             struct stat st;\n>> +             /*\n>> +              * protect submodules containing a .git directory\n>> +              * NEEDSWORK: automatically call absorbgitdirs before\n>> +              * warning/die.\n>> +              */\n>\n> I guess that you mean \"instead of dying, automatically call absorb\n> and (possibly) warn\"?  That sounds like a sensible improvement.\n>\n>> +             if (is_directory(sub_git_dir))\n>> +                     die(_(\"Submodule work tree '%s' contains a .git \"\n>> +                           \"directory use 'rm -rf' if you really want \"\n>> +                           \"to remove it including all of its history\"),\n>\n> This changes the message text by removing () around \"use ... history\",\n> which I do not think you intended to do.\n>\n>> +                           displaypath);\n>> +\n>> +             if (!(flags & OPT_FORCE)) {\n>> +                     struct child_process cp_rm = CHILD_PROCESS_INIT;\n>> +                     cp_rm.git_cmd = 1;\n>> +                     argv_array_pushl(&cp_rm.args, \"rm\", \"-qn\",\n>> +                                      path, NULL);\n>> +\n>> +                     if (run_command(&cp_rm))\n>> +                             die(_(\"Submodule work tree '%s' contains local \"\n>> +                                   \"modifications; use '-f' to discard them\"),\n>> +                                   displaypath);\n>> +             }\n>> +\n>> +             if (!lstat(path, &st)) {\n>\n> What is this if statement doing here?  It does not make sense,\n> especially without an 'else' clause on the other side, at least to\n> me.\n>\n> At this point in the flow, the code has already determined that path\n> is a directory above before starting to check if it has \".git/\"\n> immediately below it, or trying to run \"git rm\" in the dry run mode\n> to see if it yields an error, so at this point lstat() should\n> succeed (and would say it is a directory).  I would sort-of\n> understand it if this \"if()\" has an \"else\" clause to act on an\n> error, but that is not something the original does not do, so I am\n> not sure if it belongs to a \"rewrite to C\" patch.\n>\n>> +                     struct strbuf sb_rm = STRBUF_INIT;\n>> +                     const char *format;\n>> +\n>> +                     strbuf_addstr(&sb_rm, path);\n>> +\n>> +                     if (!remove_dir_recursively(&sb_rm, 0))\n>> +                             format = _(\"Cleared directory '%s'\\n\");\n>> +                     else\n>> +                             format = _(\"Could not remove submodule work tree '%s'\\n\");\n>> +\n>> +                     if (!(flags & OPT_QUIET))\n>> +                             printf(format, displaypath);\n>> +\n>> +                     mode = st.st_mode;\n>> +\n>> +                     strbuf_release(&sb_rm);\n>> +             }\n>> +     }\n>\n> If the reason is \"avoid losing the original directory mode by\n> removing and recreating\", then you should be able to do much better\n> by using REMOVE_DIR_KEEP_TOPLEVEL in the above \"do we still have a\n> directory?  if so get rid of working tree contents\" thing.  And the\n> call to mkdir() below can be placed in the else clause of that\n> check, i.e. \"the user has removed the directory as well, but there\n> should be an empty directory even for a de-initialized submodule\"\n> side of this.\n>\n> That of course does not have to be part of \"rewrite to C\" patch.  In\n> fact, it probably should come as a follow-up improvement after the\n> dust settles.\n>\nFirstly, thanks a lot for taking time and reviewing the patches.\nI have a few queries about the above changes to be made in the\n\"rewrite to C\" patch.\n\nFunction lstat() was used for mainly getting the mode for the to be\ncreated new directory.\nAnd since sometimes st.st_mode may be containing garbage value, a new variable\nmode was introduced with initial value 0777.\n\nThanks for pointing out that we can introduce the flag REMOVE_DIR_KEEP_TOPLEVEL\nwhich solves the issue. And for the case where no directory exists: we\ncreate an empty\ndirectory.Since this won't be similar to what happens in the shell\nscript, this change\ncan be included in a saperate patch as an imporvement. But till the\ndust settles, does the\ncurrent patch serve the purpose? (After imporving over the other\npoints being pointed above)\n\nThanks,\nPrathamesh Chavan\n\n>> +     if (mkdir(path, mode))\n>> +             die_errno(_(\"could not create empty submodule directory %s\"),\n>> +                   displaypath);\n>> + ...\n>> +\n>> +static int module_deinit(int argc, const char **argv, const char *prefix)\n>> +{\n>> +...\n>> +     if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)\n>> +             BUG(\"module_list_compute should not choke on empty pathspec\");\n>> +...\n>> +     if (all && argc) {\n>> +             error(\"pathspec and --all are incompatible\");\n>> +             usage_with_options(git_submodule_helper_usage,\n>> +                                module_deinit_options);\n>> +     }\n>> +\n>> +     if (!argc && !all)\n>> +             die(_(\"Use '--all' if you really want to deinitialize all submodules\"));\n>\n> Shouldn't these two checks come before we call module_list_compute()?\n>\n>> +\n>> +     for_each_listed_submodule(&list, deinit_submodule_cb, &info);\n>> +\n>> +     return 0;\n>> +}\n"},{"id":"336429","messageId":"xmqqo9m1z8rf.fsf@gitster.mtv.corp.google.com","threadId":"47577","inReplyTo":"CAME+mvXaL4AcK1ib2rDZKdH0eLc7te+3e9zYv8pNqNj-4cyT3Q@mail.gmail.com","subject":"Re: [PATCH v1 2/2] submodule: port submodule subcommand 'deinit' from shell to C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-10T21:47:16Z","receivedAt":"2018-01-10T21:47:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Prathamesh Chavan <pc44800@gmail.com> writes:\n\n> Thanks for pointing out that we can introduce the flag REMOVE_DIR_KEEP_TOPLEVEL\n> which solves the issue. And for the case where no directory exists: we\n> create an empty\n> directory.Since this won't be similar to what happens in the shell\n> script, this change\n> can be included in a saperate patch as an imporvement.\n\nExactly.  The way the shell script does it is to _always_ honor\nuser's umask and recreate the directory, so before that separate\nimprovement, tweaking \"mode\" based on the returned value from an\nextra lstat() is an unneeded change of behaviour.  Just passing 0777\nand let mkdir() take the umask into account to come up with the\nfinal permission bits is more in line with the original scripted\nversion, I would think.\n\nThanks.\n\n"},{"id":"336475","messageId":"20180111201721.25930-1-pc44800@gmail.com","threadId":"47577","inReplyTo":"20180109175703.4793-1-pc44800@gmail.com","subject":"[PATCH v2 0/2] Incremental rewrite of git-submodules","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-01-11T20:17:19Z","receivedAt":"2018-01-11T20:17:40Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"Changes made to the previous version of the patch series[1]:\n\n* Since later on with certain patches, the number of bit-parameters to\n  be passed to a few functions depend on many parameters, I prefered\n  using a single flag bit.\n\n* Memory-leak of the variable 'remote' in the function:\n  print_default_remote() was avoided.\n\n* Additional condition were introduced while freeing the variables:\n  sub_origin_url and super_config_url.\n\n* print messages and comments in the deinit_submodule function were\n  corrected as suggested in previous review of this patch[2].\n\n* Call to the function lstat() for identifying the directory mode was\n  avoided and instead 0777 was used. An additional improvement is to be\n  made over this patch, but since the improvement can not directly be\n  part of the \"rewirte in C\", the patch would be floated saperately on\n  the mailing list.\n\nAs before you can find this series at:\nhttps://github.com/pratham-pc/git/commits/patch-series-2\n\nAnd its build report is available at:\nhttps://travis-ci.org/pratham-pc/git/builds/\nBranch: patch-series-2\nBuild #196\n\n[1]: https://public-inbox.org/git/20180109175703.4793-1-pc44800@gmail.com/ \n[2]: https://public-inbox.org/git/xmqq7esq4tf6.fsf@gitster.mtv.corp.google.com/\n\nPrathamesh Chavan (2):\n  submodule: port submodule subcommand 'sync' from shell to C\n  submodule: port submodule subcommand 'deinit' from shell to C\n\n builtin/submodule--helper.c | 342 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            | 112 +--------------\n 2 files changed, 344 insertions(+), 110 deletions(-)\n\n-- \n2.15.1\n\n"},{"id":"336476","messageId":"20180111201721.25930-3-pc44800@gmail.com","threadId":"47577","inReplyTo":"20180111201721.25930-1-pc44800@gmail.com","subject":"[PATCH v2 2/2] submodule: port submodule subcommand 'deinit' from shell to C","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-01-11T20:17:21Z","receivedAt":"2018-01-11T20:17:48Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"The same mechanism is used even for porting this submodule\nsubcommand, as used in the ported subcommands till now.\nThe function cmd_deinit in split up after porting into four\nfunctions: module_deinit(), for_each_listed_submodule(),\ndeinit_submodule() and deinit_submodule_cb().\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Prathamesh Chavan <pc44800@gmail.com>\n---\n builtin/submodule--helper.c | 147 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            |  55 +----------------\n 2 files changed, 148 insertions(+), 54 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex eb6f96981..b93e1d50b 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -20,6 +20,7 @@\n #define OPT_QUIET (1 << 0)\n #define OPT_CACHED (1 << 1)\n #define OPT_RECURSIVE (1 << 2)\n+#define OPT_FORCE (1 << 3)\n \n typedef void (*each_submodule_fn)(const struct cache_entry *list_item,\n \t\t\t\t  void *cb_data);\n@@ -911,6 +912,151 @@ static int module_sync(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+struct deinit_cb {\n+\tconst char *prefix;\n+\tunsigned int flags;\n+};\n+#define DEINIT_CB_INIT { NULL, 0 }\n+\n+static void deinit_submodule(const char *path, const char *prefix,\n+\t\t\t     unsigned int flags)\n+{\n+\tconst struct submodule *sub;\n+\tchar *displaypath = NULL;\n+\tstruct child_process cp_config = CHILD_PROCESS_INIT;\n+\tstruct strbuf sb_config = STRBUF_INIT;\n+\tchar *sub_git_dir = xstrfmt(\"%s/.git\", path);\n+\n+\tsub = submodule_from_path(&null_oid, path);\n+\n+\tif (!sub || !sub->name)\n+\t\tgoto cleanup;\n+\n+\tdisplaypath = get_submodule_displaypath(path, prefix);\n+\n+\t/* remove the submodule work tree (unless the user already did it) */\n+\tif (is_directory(path)) {\n+\t\tstruct strbuf sb_rm = STRBUF_INIT;\n+\t\tconst char *format;\n+\n+\t\t/*\n+\t\t * protect submodules containing a .git directory\n+\t\t * NEEDSWORK: instead of dying, automatically call\n+\t\t * absorbgitdirs and (possibly) warn.\n+\t\t */\n+\t\tif (is_directory(sub_git_dir))\n+\t\t\tdie(_(\"Submodule work tree '%s' contains a .git \"\n+\t\t\t      \"directory (use 'rm -rf' if you really want \"\n+\t\t\t      \"to remove it including all of its history)\"),\n+\t\t\t    displaypath);\n+\n+\t\tif (!(flags & OPT_FORCE)) {\n+\t\t\tstruct child_process cp_rm = CHILD_PROCESS_INIT;\n+\t\t\tcp_rm.git_cmd = 1;\n+\t\t\targv_array_pushl(&cp_rm.args, \"rm\", \"-qn\",\n+\t\t\t\t\t path, NULL);\n+\n+\t\t\tif (run_command(&cp_rm))\n+\t\t\t\tdie(_(\"Submodule work tree '%s' contains local \"\n+\t\t\t\t      \"modifications; use '-f' to discard them\"),\n+\t\t\t\t      displaypath);\n+\t\t}\n+\n+\t\tstrbuf_addstr(&sb_rm, path);\n+\n+\t\tif (!remove_dir_recursively(&sb_rm, 0))\n+\t\t\tformat = _(\"Cleared directory '%s'\\n\");\n+\t\telse\n+\t\t\tformat = _(\"Could not remove submodule work tree '%s'\\n\");\n+\n+\t\tif (!(flags & OPT_QUIET))\n+\t\t\tprintf(format, displaypath);\n+\n+\t\tstrbuf_release(&sb_rm);\n+\t}\n+\n+\tif (mkdir(path, 0777))\n+\t\tdie_errno(_(\"could not create empty submodule directory %s\"),\n+\t\t      displaypath);\n+\n+\tcp_config.git_cmd = 1;\n+\targv_array_pushl(&cp_config.args, \"config\", \"--get-regexp\", NULL);\n+\targv_array_pushf(&cp_config.args, \"submodule.%s\\\\.\", sub->name);\n+\n+\t/* remove the .git/config entries (unless the user already did it) */\n+\tif (!capture_command(&cp_config, &sb_config, 0) && sb_config.len) {\n+\t\tchar *sub_key = xstrfmt(\"submodule.%s\", sub->name);\n+\t\t/*\n+\t\t * remove the whole section so we have a clean state when\n+\t\t * the user later decides to init this submodule again\n+\t\t */\n+\t\tgit_config_rename_section_in_file(NULL, sub_key, NULL);\n+\t\tif (!(flags & OPT_QUIET))\n+\t\t\tprintf(_(\"Submodule '%s' (%s) unregistered for path '%s'\\n\"),\n+\t\t\t\t sub->name, sub->url, displaypath);\n+\t\tfree(sub_key);\n+\t}\n+\n+cleanup:\n+\tfree(displaypath);\n+\tfree(sub_git_dir);\n+\tstrbuf_release(&sb_config);\n+}\n+\n+static void deinit_submodule_cb(const struct cache_entry *list_item,\n+\t\t\t\tvoid *cb_data)\n+{\n+\tstruct deinit_cb *info = cb_data;\n+\tdeinit_submodule(list_item->name, info->prefix, info->flags);\n+}\n+\n+static int module_deinit(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct deinit_cb info = DEINIT_CB_INIT;\n+\tstruct pathspec pathspec;\n+\tstruct module_list list = MODULE_LIST_INIT;\n+\tint quiet = 0;\n+\tint force = 0;\n+\tint all = 0;\n+\n+\tstruct option module_deinit_options[] = {\n+\t\tOPT__QUIET(&quiet, N_(\"Suppress submodule status output\")),\n+\t\tOPT__FORCE(&force, N_(\"Remove submodule working trees even if they contain local changes\")),\n+\t\tOPT_BOOL(0, \"all\", &all, N_(\"Unregister all submodules\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule deinit [--quiet] [-f | --force] [--all | [--] [<path>...]]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_deinit_options,\n+\t\t\t     git_submodule_helper_usage, 0);\n+\n+\tif (all && argc) {\n+\t\terror(\"pathspec and --all are incompatible\");\n+\t\tusage_with_options(git_submodule_helper_usage,\n+\t\t\t\t   module_deinit_options);\n+\t}\n+\n+\tif (!argc && !all)\n+\t\tdie(_(\"Use '--all' if you really want to deinitialize all submodules\"));\n+\n+\tif (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)\n+\t\tBUG(\"module_list_compute should not choke on empty pathspec\");\n+\n+\tinfo.prefix = prefix;\n+\tif (quiet)\n+\t\tinfo.flags |= OPT_QUIET;\n+\tif (force)\n+\t\tinfo.flags |= OPT_FORCE;\n+\n+\tfor_each_listed_submodule(&list, deinit_submodule_cb, &info);\n+\n+\treturn 0;\n+}\n+\n static int clone_submodule(const char *path, const char *gitdir, const char *url,\n \t\t\t   const char *depth, struct string_list *reference,\n \t\t\t   int quiet, int progress)\n@@ -1693,6 +1839,7 @@ static struct cmd_struct commands[] = {\n \t{\"status\", module_status, SUPPORT_SUPER_PREFIX},\n \t{\"print-default-remote\", print_default_remote, 0},\n \t{\"sync\", module_sync, SUPPORT_SUPER_PREFIX},\n+\t{\"deinit\", module_deinit, 0},\n \t{\"remote-branch\", resolve_remote_submodule_branch, 0},\n \t{\"push-check\", push_check, 0},\n \t{\"absorb-git-dirs\", absorb_git_dirs, SUPPORT_SUPER_PREFIX},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 0825cae14..24914963c 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -428,60 +428,7 @@ cmd_deinit()\n \t\tshift\n \tdone\n \n-\tif test -n \"$deinit_all\" && test \"$#\" -ne 0\n-\tthen\n-\t\techo >&2 \"$(eval_gettext \"pathspec and --all are incompatible\")\"\n-\t\tusage\n-\tfi\n-\tif test $# = 0 && test -z \"$deinit_all\"\n-\tthen\n-\t\tdie \"$(eval_gettext \"Use '--all' if you really want to deinitialize all submodules\")\"\n-\tfi\n-\n-\t{\n-\t\tgit submodule--helper list --prefix \"$wt_prefix\" \"$@\" ||\n-\t\techo \"#unmatched\" $?\n-\t} |\n-\twhile read -r mode sha1 stage sm_path\n-\tdo\n-\t\tdie_if_unmatched \"$mode\" \"$sha1\"\n-\t\tname=$(git submodule--helper name \"$sm_path\") || exit\n-\n-\t\tdisplaypath=$(git submodule--helper relative-path \"$sm_path\" \"$wt_prefix\")\n-\n-\t\t# Remove the submodule work tree (unless the user already did it)\n-\t\tif test -d \"$sm_path\"\n-\t\tthen\n-\t\t\t# Protect submodules containing a .git directory\n-\t\t\tif test -d \"$sm_path/.git\"\n-\t\t\tthen\n-\t\t\t\tdie \"$(eval_gettext \"\\\n-Submodule work tree '\\$displaypath' contains a .git directory\n-(use 'rm -rf' if you really want to remove it including all of its history)\")\"\n-\t\t\tfi\n-\n-\t\t\tif test -z \"$force\"\n-\t\t\tthen\n-\t\t\t\tgit rm -qn \"$sm_path\" ||\n-\t\t\t\tdie \"$(eval_gettext \"Submodule work tree '\\$displaypath' contains local modifications; use '-f' to discard them\")\"\n-\t\t\tfi\n-\t\t\trm -rf \"$sm_path\" &&\n-\t\t\tsay \"$(eval_gettext \"Cleared directory '\\$displaypath'\")\" ||\n-\t\t\tsay \"$(eval_gettext \"Could not remove submodule work tree '\\$displaypath'\")\"\n-\t\tfi\n-\n-\t\tmkdir \"$sm_path\" || say \"$(eval_gettext \"Could not create empty submodule directory '\\$displaypath'\")\"\n-\n-\t\t# Remove the .git/config entries (unless the user already did it)\n-\t\tif test -n \"$(git config --get-regexp submodule.\"$name\\.\")\"\n-\t\tthen\n-\t\t\t# Remove the whole section so we have a clean state when\n-\t\t\t# the user later decides to init this submodule again\n-\t\t\turl=$(git config submodule.\"$name\".url)\n-\t\t\tgit config --remove-section submodule.\"$name\" 2>/dev/null &&\n-\t\t\tsay \"$(eval_gettext \"Submodule '\\$name' (\\$url) unregistered for path '\\$displaypath'\")\"\n-\t\tfi\n-\tdone\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper deinit ${GIT_QUIET:+--quiet} ${prefix:+--prefix \"$prefix\"} ${force:+--force} ${deinit_all:+--all} \"$@\"\n }\n \n is_tip_reachable () (\n-- \n2.15.1\n\n"},{"id":"336477","messageId":"20180111201721.25930-2-pc44800@gmail.com","threadId":"47577","inReplyTo":"20180111201721.25930-1-pc44800@gmail.com","subject":"[PATCH v2 1/2] submodule: port submodule subcommand 'sync' from shell to C","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-01-11T20:17:20Z","receivedAt":"2018-01-11T20:17:53Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"Port the submodule subcommand 'sync' from shell to C using the same\nmechanism as that used for porting submodule subcommand 'status'.\nHence, here the function cmd_sync() is ported from shell to C.\nThis is done by introducing four functions: module_sync(),\nsync_submodule(), sync_submodule_cb() and print_default_remote().\n\nThe function print_default_remote() is introduced for getting\nthe default remote as stdout.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Prathamesh Chavan <pc44800@gmail.com>\n---\n builtin/submodule--helper.c | 195 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            |  57 +------------\n 2 files changed, 196 insertions(+), 56 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex a5c4a8a69..eb6f96981 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -50,6 +50,20 @@ static char *get_default_remote(void)\n \treturn ret;\n }\n \n+static int print_default_remote(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *remote;\n+\n+\tif (argc != 1)\n+\t\tdie(_(\"submodule--helper print-default-remote takes no arguments\"));\n+\n+\tremote = get_default_remote();\n+\tif (remote)\n+\t\tprintf(\"%s\\n\", remote);\n+\n+\treturn 0;\n+}\n+\n static int starts_with_dot_slash(const char *str)\n {\n \treturn str[0] == '.' && is_dir_sep(str[1]);\n@@ -358,6 +372,25 @@ static void module_list_active(struct module_list *list)\n \t*list = active_modules;\n }\n \n+static char *get_up_path(const char *path)\n+{\n+\tint i;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tfor (i = count_slashes(path); i; i--)\n+\t\tstrbuf_addstr(&sb, \"../\");\n+\n+\t/*\n+\t * Check if 'path' ends with slash or not\n+\t * for having the same output for dir/sub_dir\n+\t * and dir/sub_dir/\n+\t */\n+\tif (!is_dir_sep(path[strlen(path) - 1]))\n+\t\tstrbuf_addstr(&sb, \"../\");\n+\n+\treturn strbuf_detach(&sb, NULL);\n+}\n+\n static int module_list(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n@@ -718,6 +751,166 @@ static int module_name(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+struct sync_cb {\n+\tconst char *prefix;\n+\tunsigned int flags;\n+};\n+\n+#define SYNC_CB_INIT { NULL, 0 }\n+\n+static void sync_submodule(const char *path, const char *prefix,\n+\t\t\t   unsigned int flags)\n+{\n+\tconst struct submodule *sub;\n+\tchar *remote_key = NULL;\n+\tchar *sub_origin_url, *super_config_url, *displaypath;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tchar *sub_config_path = NULL;\n+\n+\tif (!is_submodule_active(the_repository, path))\n+\t\treturn;\n+\n+\tsub = submodule_from_path(&null_oid, path);\n+\n+\tif (sub && sub->url) {\n+\t\tif (starts_with_dot_dot_slash(sub->url) ||\n+\t\t    starts_with_dot_slash(sub->url)) {\n+\t\t\tchar *remote_url, *up_path;\n+\t\t\tchar *remote = get_default_remote();\n+\t\t\tstrbuf_addf(&sb, \"remote.%s.url\", remote);\n+\n+\t\t\tif (git_config_get_string(sb.buf, &remote_url))\n+\t\t\t\tremote_url = xgetcwd();\n+\n+\t\t\tup_path = get_up_path(path);\n+\t\t\tsub_origin_url = relative_url(remote_url, sub->url, up_path);\n+\t\t\tsuper_config_url = relative_url(remote_url, sub->url, NULL);\n+\n+\t\t\tfree(remote);\n+\t\t\tfree(up_path);\n+\t\t\tfree(remote_url);\n+\t\t} else {\n+\t\t\tsub_origin_url = xstrdup(sub->url);\n+\t\t\tsuper_config_url = xstrdup(sub->url);\n+\t\t}\n+\t} else {\n+\t\tsub_origin_url = \"\";\n+\t\tsuper_config_url = \"\";\n+\t}\n+\n+\tdisplaypath = get_submodule_displaypath(path, prefix);\n+\n+\tif (!(flags & OPT_QUIET))\n+\t\tprintf(_(\"Synchronizing submodule url for '%s'\\n\"),\n+\t\t\t displaypath);\n+\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n+\tif (git_config_set_gently(sb.buf, super_config_url))\n+\t\tdie(_(\"failed to register url for submodule path '%s'\"),\n+\t\t      displaypath);\n+\n+\tif (!is_submodule_populated_gently(path, NULL))\n+\t\tgoto cleanup;\n+\n+\tprepare_submodule_repo_env(&cp.env_array);\n+\tcp.git_cmd = 1;\n+\tcp.dir = path;\n+\targv_array_pushl(&cp.args, \"submodule--helper\",\n+\t\t\t \"print-default-remote\", NULL);\n+\n+\tstrbuf_reset(&sb);\n+\tif (capture_command(&cp, &sb, 0))\n+\t\tdie(_(\"failed to get the default remote for submodule '%s'\"),\n+\t\t      path);\n+\n+\tstrbuf_strip_suffix(&sb, \"\\n\");\n+\tremote_key = xstrfmt(\"remote.%s.url\", sb.buf);\n+\n+\tstrbuf_reset(&sb);\n+\tsubmodule_to_gitdir(&sb, path);\n+\tstrbuf_addstr(&sb, \"/config\");\n+\n+\tif (git_config_set_in_file_gently(sb.buf, remote_key, sub_origin_url))\n+\t\tdie(_(\"failed to update remote for submodule '%s'\"),\n+\t\t      path);\n+\n+\tif (flags & OPT_RECURSIVE) {\n+\t\tstruct child_process cpr = CHILD_PROCESS_INIT;\n+\n+\t\tcpr.git_cmd = 1;\n+\t\tcpr.dir = path;\n+\t\tprepare_submodule_repo_env(&cpr.env_array);\n+\n+\t\targv_array_push(&cpr.args, \"--super-prefix\");\n+\t\targv_array_pushf(&cpr.args, \"%s/\", displaypath);\n+\t\targv_array_pushl(&cpr.args, \"submodule--helper\", \"sync\",\n+\t\t\t\t \"--recursive\", NULL);\n+\n+\t\tif (flags & OPT_QUIET)\n+\t\t\targv_array_push(&cpr.args, \"--quiet\");\n+\n+\t\tif (run_command(&cpr))\n+\t\t\tdie(_(\"failed to recurse into submodule '%s'\"),\n+\t\t\t      path);\n+\t}\n+\n+cleanup:\n+\tif (strlen(super_config_url))\n+\t\tfree(super_config_url);\n+\tif (strlen(sub_origin_url))\n+\t\tfree(sub_origin_url);\n+\tstrbuf_release(&sb);\n+\tfree(remote_key);\n+\tfree(displaypath);\n+\tfree(sub_config_path);\n+}\n+\n+static void sync_submodule_cb(const struct cache_entry *list_item, void *cb_data)\n+{\n+\tstruct sync_cb *info = cb_data;\n+\tsync_submodule(list_item->name, info->prefix, info->flags);\n+\n+}\n+\n+static int module_sync(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct sync_cb info = SYNC_CB_INIT;\n+\tstruct pathspec pathspec;\n+\tstruct module_list list = MODULE_LIST_INIT;\n+\tint quiet = 0;\n+\tint recursive = 0;\n+\n+\tstruct option module_sync_options[] = {\n+\t\tOPT__QUIET(&quiet, N_(\"Suppress output of synchronizing submodule url\")),\n+\t\tOPT_BOOL(0, \"recursive\", &recursive,\n+\t\t\tN_(\"Recurse into nested submodules\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper sync [--quiet] [--recursive] [<path>]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_sync_options,\n+\t\t\t     git_submodule_helper_usage, 0);\n+\n+\tif (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)\n+\t\treturn 1;\n+\n+\tinfo.prefix = prefix;\n+\tif (quiet)\n+\t\tinfo.flags |= OPT_QUIET;\n+\tif (recursive)\n+\t\tinfo.flags |= OPT_RECURSIVE;\n+\n+\tfor_each_listed_submodule(&list, sync_submodule_cb, &info);\n+\n+\treturn 0;\n+}\n+\n static int clone_submodule(const char *path, const char *gitdir, const char *url,\n \t\t\t   const char *depth, struct string_list *reference,\n \t\t\t   int quiet, int progress)\n@@ -1498,6 +1691,8 @@ static struct cmd_struct commands[] = {\n \t{\"resolve-relative-url-test\", resolve_relative_url_test, 0},\n \t{\"init\", module_init, SUPPORT_SUPER_PREFIX},\n \t{\"status\", module_status, SUPPORT_SUPER_PREFIX},\n+\t{\"print-default-remote\", print_default_remote, 0},\n+\t{\"sync\", module_sync, SUPPORT_SUPER_PREFIX},\n \t{\"remote-branch\", resolve_remote_submodule_branch, 0},\n \t{\"push-check\", push_check, 0},\n \t{\"absorb-git-dirs\", absorb_git_dirs, SUPPORT_SUPER_PREFIX},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 156255a9e..0825cae14 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -1036,63 +1036,8 @@ cmd_sync()\n \t\t\t;;\n \t\tesac\n \tdone\n-\tcd_to_toplevel\n-\t{\n-\t\tgit submodule--helper list --prefix \"$wt_prefix\" \"$@\" ||\n-\t\techo \"#unmatched\" $?\n-\t} |\n-\twhile read -r mode sha1 stage sm_path\n-\tdo\n-\t\tdie_if_unmatched \"$mode\" \"$sha1\"\n \n-\t\t# skip inactive submodules\n-\t\tif ! git submodule--helper is-active \"$sm_path\"\n-\t\tthen\n-\t\t\tcontinue\n-\t\tfi\n-\n-\t\tname=$(git submodule--helper name \"$sm_path\")\n-\t\turl=$(git config -f .gitmodules --get submodule.\"$name\".url)\n-\n-\t\t# Possibly a url relative to parent\n-\t\tcase \"$url\" in\n-\t\t./*|../*)\n-\t\t\t# rewrite foo/bar as ../.. to find path from\n-\t\t\t# submodule work tree to superproject work tree\n-\t\t\tup_path=\"$(printf '%s\\n' \"$sm_path\" | sed \"s/[^/][^/]*/../g\")\" &&\n-\t\t\t# guarantee a trailing /\n-\t\t\tup_path=${up_path%/}/ &&\n-\t\t\t# path from submodule work tree to submodule origin repo\n-\t\t\tsub_origin_url=$(git submodule--helper resolve-relative-url \"$url\" \"$up_path\") &&\n-\t\t\t# path from superproject work tree to submodule origin repo\n-\t\t\tsuper_config_url=$(git submodule--helper resolve-relative-url \"$url\") || exit\n-\t\t\t;;\n-\t\t*)\n-\t\t\tsub_origin_url=\"$url\"\n-\t\t\tsuper_config_url=\"$url\"\n-\t\t\t;;\n-\t\tesac\n-\n-\t\tdisplaypath=$(git submodule--helper relative-path \"$prefix$sm_path\" \"$wt_prefix\")\n-\t\tsay \"$(eval_gettext \"Synchronizing submodule url for '\\$displaypath'\")\"\n-\t\tgit config submodule.\"$name\".url \"$super_config_url\"\n-\n-\t\tif test -e \"$sm_path\"/.git\n-\t\tthen\n-\t\t(\n-\t\t\tsanitize_submodule_env\n-\t\t\tcd \"$sm_path\"\n-\t\t\tremote=$(get_default_remote)\n-\t\t\tgit config remote.\"$remote\".url \"$sub_origin_url\"\n-\n-\t\t\tif test -n \"$recursive\"\n-\t\t\tthen\n-\t\t\t\tprefix=\"$prefix$sm_path/\"\n-\t\t\t\teval cmd_sync\n-\t\t\tfi\n-\t\t)\n-\t\tfi\n-\tdone\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} ${prefix:+--super-prefix \"$prefix\"} submodule--helper sync ${GIT_QUIET:+--quiet} ${recursive:+--recursive} \"$@\"\n }\n \n cmd_absorbgitdirs()\n-- \n2.15.1\n\n"},{"id":"336480","messageId":"xmqqshbcxhla.fsf@gitster.mtv.corp.google.com","threadId":"47577","inReplyTo":"20180111201721.25930-2-pc44800@gmail.com","subject":"Re: [PATCH v2 1/2] submodule: port submodule subcommand 'sync' from shell to C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-11T20:31:45Z","receivedAt":"2018-01-11T20:31:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Prathamesh Chavan <pc44800@gmail.com> writes:\n\n> +\t\t} else {\n> +\t\t\tsub_origin_url = xstrdup(sub->url);\n> +\t\t\tsuper_config_url = xstrdup(sub->url);\n> +\t\t}\n> +\t} else {\n> +\t\tsub_origin_url = \"\";\n> +\t\tsuper_config_url = \"\";\n> +\t}\n> + ...\n> +cleanup:\n> +\tif (strlen(super_config_url))\n> +\t\tfree(super_config_url);\n> +\tif (strlen(sub_origin_url))\n> +\t\tfree(sub_origin_url);\n\nThe above is ugly and veriy likely to be wrong; imagine that\nsub->url was an empty string to begin with.\n\nDoing xstrdup(\"\") before assigning the constant to *_url would be a\nlot more sensible and maintainable solution for things like this.\n"},{"id":"336481","messageId":"xmqqlgh4xhce.fsf@gitster.mtv.corp.google.com","threadId":"47577","inReplyTo":"20180111201721.25930-1-pc44800@gmail.com","subject":"Re: [PATCH v2 0/2] Incremental rewrite of git-submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-11T20:37:05Z","receivedAt":"2018-01-11T20:37:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Prathamesh Chavan <pc44800@gmail.com> writes:\n\n> Changes made to the previous version of the patch series[1]:\n>\n> * Since later on with certain patches, the number of bit-parameters to\n>   be passed to a few functions depend on many parameters, I prefered\n>   using a single flag bit.\n\nI am not quite getting what you meant to say here.\n\n> * Memory-leak of the variable 'remote' in the function:\n>   print_default_remote() was avoided.\n\navoided how?  I am not quite getting what you meant to say here.\n\n> * Additional condition were introduced while freeing the variables:\n>   sub_origin_url and super_config_url.\n\nAs I said, I do not think the change goes into the right direction.\n\n"},{"id":"336482","messageId":"xmqqh8rsxgtw.fsf@gitster.mtv.corp.google.com","threadId":"47577","inReplyTo":"20180111201721.25930-3-pc44800@gmail.com","subject":"Re: [PATCH v2 2/2] submodule: port submodule subcommand 'deinit' from shell to C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-11T20:48:11Z","receivedAt":"2018-01-11T20:48:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Prathamesh Chavan <pc44800@gmail.com> writes:\n\n> +\t/* remove the submodule work tree (unless the user already did it) */\n> +\tif (is_directory(path)) {\n> +\t\tstruct strbuf sb_rm = STRBUF_INIT;\n> +\t\tconst char *format;\n> +\n> +\t\t/*\n> +\t\t * protect submodules containing a .git directory\n> +\t\t * NEEDSWORK: instead of dying, automatically call\n> +\t\t * absorbgitdirs and (possibly) warn.\n> +\t\t */\n> +\t\tif (is_directory(sub_git_dir))\n> +\t\t\tdie(_(\"Submodule work tree '%s' contains a .git \"\n> +\t\t\t      \"directory (use 'rm -rf' if you really want \"\n> +\t\t\t      \"to remove it including all of its history)\"),\n> +\t\t\t    displaypath);\n> +\n> +\t\tif (!(flags & OPT_FORCE)) {\n> +\t\t\tstruct child_process cp_rm = CHILD_PROCESS_INIT;\n> +\t\t\tcp_rm.git_cmd = 1;\n> +\t\t\targv_array_pushl(&cp_rm.args, \"rm\", \"-qn\",\n> +\t\t\t\t\t path, NULL);\n> +\n> +\t\t\tif (run_command(&cp_rm))\n> +\t\t\t\tdie(_(\"Submodule work tree '%s' contains local \"\n> +\t\t\t\t      \"modifications; use '-f' to discard them\"),\n> +\t\t\t\t      displaypath);\n> +\t\t}\n> +\n> +\t\tstrbuf_addstr(&sb_rm, path);\n> +\n> +\t\tif (!remove_dir_recursively(&sb_rm, 0))\n> +\t\t\tformat = _(\"Cleared directory '%s'\\n\");\n> +\t\telse\n> +\t\t\tformat = _(\"Could not remove submodule work tree '%s'\\n\");\n> +\n> +\t\tif (!(flags & OPT_QUIET))\n> +\t\t\tprintf(format, displaypath);\n> +\n> +\t\tstrbuf_release(&sb_rm);\n> +\t}\n> +\n> +\tif (mkdir(path, 0777))\n> +\t\tdie_errno(_(\"could not create empty submodule directory %s\"),\n> +\t\t      displaypath);\n\nIf path was a directory (which presumably is the normal case) and\nrecursive removal fails (i.e. when the code says \"Could not remove\"),\nthis mkdir() would also fail with EEXIST.\n\nIn such a case, the original code did not die and instead continued\nto remove the entries for the submodule from the configuration.\nThis \"rewritten\" version dies, leaving the stale configuration for\nthe submodule we failed to get rid of from the working tree.\n\nI offhand do not know which one of these error case behaviours is\nmore useful; the user needs to do something (e.g. loosening the perm\nin some paths in the submodule that prevented \"rm -rf\" from working\nwith \"chmod u+w sub/some/path\" and removing it manually) to recover\nin either case, and cleaning as much as possible by removing the\nconfiguration entries even when this mkdir() fails would probably be\na better behaviour, as long as the command as a whole exits with non\nzero status to signal an error.\n"},{"id":"336586","messageId":"20180114211529.6391-1-pc44800@gmail.com","threadId":"47577","inReplyTo":"20180111201721.25930-1-pc44800@gmail.com","subject":"[PATCH v3 0/2] Incremental rewrite of git-submodules","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-01-14T21:15:27Z","receivedAt":"2018-01-14T16:49:19Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"Changes in v3:\n\n* For the variables: super_config_url and sub_origin_url, xstrdup() was used\n  while assigning \"\" to them, before freeing.\n\n* In case of the function deinit_submodule, since the orignal code doesn't die\n  upon failure of the function mkdir(), printf was used instead of die_errno.\n\nAs before you can find this series at:\nhttps://github.com/pratham-pc/git/commits/patch-series-2\n\nAnd its build report is available at:\nhttps://travis-ci.org/pratham-pc/git/builds/\nBranch: patch-series-2\nBuild #197\n\nPrathamesh Chavan (2):\n  submodule: port submodule subcommand 'sync' from shell to C\n  submodule: port submodule subcommand 'deinit' from shell to C\n\n builtin/submodule--helper.c | 340 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            | 112 +--------------\n 2 files changed, 342 insertions(+), 110 deletions(-)\n\n-- \n2.15.1\n\n"},{"id":"336587","messageId":"20180114211529.6391-2-pc44800@gmail.com","threadId":"47577","inReplyTo":"20180114211529.6391-1-pc44800@gmail.com","subject":"[PATCH v3 1/2] submodule: port submodule subcommand 'sync' from shell to C","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-01-14T21:15:28Z","receivedAt":"2018-01-14T16:49:25Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"Port the submodule subcommand 'sync' from shell to C using the same\nmechanism as that used for porting submodule subcommand 'status'.\nHence, here the function cmd_sync() is ported from shell to C.\nThis is done by introducing four functions: module_sync(),\nsync_submodule(), sync_submodule_cb() and print_default_remote().\n\nThe function print_default_remote() is introduced for getting\nthe default remote as stdout.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Prathamesh Chavan <pc44800@gmail.com>\n---\n builtin/submodule--helper.c | 193 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            |  57 +------------\n 2 files changed, 194 insertions(+), 56 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex a5c4a8a69..745d070ea 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -50,6 +50,20 @@ static char *get_default_remote(void)\n \treturn ret;\n }\n \n+static int print_default_remote(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *remote;\n+\n+\tif (argc != 1)\n+\t\tdie(_(\"submodule--helper print-default-remote takes no arguments\"));\n+\n+\tremote = get_default_remote();\n+\tif (remote)\n+\t\tprintf(\"%s\\n\", remote);\n+\n+\treturn 0;\n+}\n+\n static int starts_with_dot_slash(const char *str)\n {\n \treturn str[0] == '.' && is_dir_sep(str[1]);\n@@ -358,6 +372,25 @@ static void module_list_active(struct module_list *list)\n \t*list = active_modules;\n }\n \n+static char *get_up_path(const char *path)\n+{\n+\tint i;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tfor (i = count_slashes(path); i; i--)\n+\t\tstrbuf_addstr(&sb, \"../\");\n+\n+\t/*\n+\t * Check if 'path' ends with slash or not\n+\t * for having the same output for dir/sub_dir\n+\t * and dir/sub_dir/\n+\t */\n+\tif (!is_dir_sep(path[strlen(path) - 1]))\n+\t\tstrbuf_addstr(&sb, \"../\");\n+\n+\treturn strbuf_detach(&sb, NULL);\n+}\n+\n static int module_list(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n@@ -718,6 +751,164 @@ static int module_name(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+struct sync_cb {\n+\tconst char *prefix;\n+\tunsigned int flags;\n+};\n+\n+#define SYNC_CB_INIT { NULL, 0 }\n+\n+static void sync_submodule(const char *path, const char *prefix,\n+\t\t\t   unsigned int flags)\n+{\n+\tconst struct submodule *sub;\n+\tchar *remote_key = NULL;\n+\tchar *sub_origin_url, *super_config_url, *displaypath;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tchar *sub_config_path = NULL;\n+\n+\tif (!is_submodule_active(the_repository, path))\n+\t\treturn;\n+\n+\tsub = submodule_from_path(&null_oid, path);\n+\n+\tif (sub && sub->url) {\n+\t\tif (starts_with_dot_dot_slash(sub->url) ||\n+\t\t    starts_with_dot_slash(sub->url)) {\n+\t\t\tchar *remote_url, *up_path;\n+\t\t\tchar *remote = get_default_remote();\n+\t\t\tstrbuf_addf(&sb, \"remote.%s.url\", remote);\n+\n+\t\t\tif (git_config_get_string(sb.buf, &remote_url))\n+\t\t\t\tremote_url = xgetcwd();\n+\n+\t\t\tup_path = get_up_path(path);\n+\t\t\tsub_origin_url = relative_url(remote_url, sub->url, up_path);\n+\t\t\tsuper_config_url = relative_url(remote_url, sub->url, NULL);\n+\n+\t\t\tfree(remote);\n+\t\t\tfree(up_path);\n+\t\t\tfree(remote_url);\n+\t\t} else {\n+\t\t\tsub_origin_url = xstrdup(sub->url);\n+\t\t\tsuper_config_url = xstrdup(sub->url);\n+\t\t}\n+\t} else {\n+\t\tsub_origin_url = xstrdup(\"\");\n+\t\tsuper_config_url = xstrdup(\"\");\n+\t}\n+\n+\tdisplaypath = get_submodule_displaypath(path, prefix);\n+\n+\tif (!(flags & OPT_QUIET))\n+\t\tprintf(_(\"Synchronizing submodule url for '%s'\\n\"),\n+\t\t\t displaypath);\n+\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n+\tif (git_config_set_gently(sb.buf, super_config_url))\n+\t\tdie(_(\"failed to register url for submodule path '%s'\"),\n+\t\t      displaypath);\n+\n+\tif (!is_submodule_populated_gently(path, NULL))\n+\t\tgoto cleanup;\n+\n+\tprepare_submodule_repo_env(&cp.env_array);\n+\tcp.git_cmd = 1;\n+\tcp.dir = path;\n+\targv_array_pushl(&cp.args, \"submodule--helper\",\n+\t\t\t \"print-default-remote\", NULL);\n+\n+\tstrbuf_reset(&sb);\n+\tif (capture_command(&cp, &sb, 0))\n+\t\tdie(_(\"failed to get the default remote for submodule '%s'\"),\n+\t\t      path);\n+\n+\tstrbuf_strip_suffix(&sb, \"\\n\");\n+\tremote_key = xstrfmt(\"remote.%s.url\", sb.buf);\n+\n+\tstrbuf_reset(&sb);\n+\tsubmodule_to_gitdir(&sb, path);\n+\tstrbuf_addstr(&sb, \"/config\");\n+\n+\tif (git_config_set_in_file_gently(sb.buf, remote_key, sub_origin_url))\n+\t\tdie(_(\"failed to update remote for submodule '%s'\"),\n+\t\t      path);\n+\n+\tif (flags & OPT_RECURSIVE) {\n+\t\tstruct child_process cpr = CHILD_PROCESS_INIT;\n+\n+\t\tcpr.git_cmd = 1;\n+\t\tcpr.dir = path;\n+\t\tprepare_submodule_repo_env(&cpr.env_array);\n+\n+\t\targv_array_push(&cpr.args, \"--super-prefix\");\n+\t\targv_array_pushf(&cpr.args, \"%s/\", displaypath);\n+\t\targv_array_pushl(&cpr.args, \"submodule--helper\", \"sync\",\n+\t\t\t\t \"--recursive\", NULL);\n+\n+\t\tif (flags & OPT_QUIET)\n+\t\t\targv_array_push(&cpr.args, \"--quiet\");\n+\n+\t\tif (run_command(&cpr))\n+\t\t\tdie(_(\"failed to recurse into submodule '%s'\"),\n+\t\t\t      path);\n+\t}\n+\n+cleanup:\n+\tfree(super_config_url);\n+\tfree(sub_origin_url);\n+\tstrbuf_release(&sb);\n+\tfree(remote_key);\n+\tfree(displaypath);\n+\tfree(sub_config_path);\n+}\n+\n+static void sync_submodule_cb(const struct cache_entry *list_item, void *cb_data)\n+{\n+\tstruct sync_cb *info = cb_data;\n+\tsync_submodule(list_item->name, info->prefix, info->flags);\n+\n+}\n+\n+static int module_sync(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct sync_cb info = SYNC_CB_INIT;\n+\tstruct pathspec pathspec;\n+\tstruct module_list list = MODULE_LIST_INIT;\n+\tint quiet = 0;\n+\tint recursive = 0;\n+\n+\tstruct option module_sync_options[] = {\n+\t\tOPT__QUIET(&quiet, N_(\"Suppress output of synchronizing submodule url\")),\n+\t\tOPT_BOOL(0, \"recursive\", &recursive,\n+\t\t\tN_(\"Recurse into nested submodules\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper sync [--quiet] [--recursive] [<path>]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_sync_options,\n+\t\t\t     git_submodule_helper_usage, 0);\n+\n+\tif (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)\n+\t\treturn 1;\n+\n+\tinfo.prefix = prefix;\n+\tif (quiet)\n+\t\tinfo.flags |= OPT_QUIET;\n+\tif (recursive)\n+\t\tinfo.flags |= OPT_RECURSIVE;\n+\n+\tfor_each_listed_submodule(&list, sync_submodule_cb, &info);\n+\n+\treturn 0;\n+}\n+\n static int clone_submodule(const char *path, const char *gitdir, const char *url,\n \t\t\t   const char *depth, struct string_list *reference,\n \t\t\t   int quiet, int progress)\n@@ -1498,6 +1689,8 @@ static struct cmd_struct commands[] = {\n \t{\"resolve-relative-url-test\", resolve_relative_url_test, 0},\n \t{\"init\", module_init, SUPPORT_SUPER_PREFIX},\n \t{\"status\", module_status, SUPPORT_SUPER_PREFIX},\n+\t{\"print-default-remote\", print_default_remote, 0},\n+\t{\"sync\", module_sync, SUPPORT_SUPER_PREFIX},\n \t{\"remote-branch\", resolve_remote_submodule_branch, 0},\n \t{\"push-check\", push_check, 0},\n \t{\"absorb-git-dirs\", absorb_git_dirs, SUPPORT_SUPER_PREFIX},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 156255a9e..0825cae14 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -1036,63 +1036,8 @@ cmd_sync()\n \t\t\t;;\n \t\tesac\n \tdone\n-\tcd_to_toplevel\n-\t{\n-\t\tgit submodule--helper list --prefix \"$wt_prefix\" \"$@\" ||\n-\t\techo \"#unmatched\" $?\n-\t} |\n-\twhile read -r mode sha1 stage sm_path\n-\tdo\n-\t\tdie_if_unmatched \"$mode\" \"$sha1\"\n \n-\t\t# skip inactive submodules\n-\t\tif ! git submodule--helper is-active \"$sm_path\"\n-\t\tthen\n-\t\t\tcontinue\n-\t\tfi\n-\n-\t\tname=$(git submodule--helper name \"$sm_path\")\n-\t\turl=$(git config -f .gitmodules --get submodule.\"$name\".url)\n-\n-\t\t# Possibly a url relative to parent\n-\t\tcase \"$url\" in\n-\t\t./*|../*)\n-\t\t\t# rewrite foo/bar as ../.. to find path from\n-\t\t\t# submodule work tree to superproject work tree\n-\t\t\tup_path=\"$(printf '%s\\n' \"$sm_path\" | sed \"s/[^/][^/]*/../g\")\" &&\n-\t\t\t# guarantee a trailing /\n-\t\t\tup_path=${up_path%/}/ &&\n-\t\t\t# path from submodule work tree to submodule origin repo\n-\t\t\tsub_origin_url=$(git submodule--helper resolve-relative-url \"$url\" \"$up_path\") &&\n-\t\t\t# path from superproject work tree to submodule origin repo\n-\t\t\tsuper_config_url=$(git submodule--helper resolve-relative-url \"$url\") || exit\n-\t\t\t;;\n-\t\t*)\n-\t\t\tsub_origin_url=\"$url\"\n-\t\t\tsuper_config_url=\"$url\"\n-\t\t\t;;\n-\t\tesac\n-\n-\t\tdisplaypath=$(git submodule--helper relative-path \"$prefix$sm_path\" \"$wt_prefix\")\n-\t\tsay \"$(eval_gettext \"Synchronizing submodule url for '\\$displaypath'\")\"\n-\t\tgit config submodule.\"$name\".url \"$super_config_url\"\n-\n-\t\tif test -e \"$sm_path\"/.git\n-\t\tthen\n-\t\t(\n-\t\t\tsanitize_submodule_env\n-\t\t\tcd \"$sm_path\"\n-\t\t\tremote=$(get_default_remote)\n-\t\t\tgit config remote.\"$remote\".url \"$sub_origin_url\"\n-\n-\t\t\tif test -n \"$recursive\"\n-\t\t\tthen\n-\t\t\t\tprefix=\"$prefix$sm_path/\"\n-\t\t\t\teval cmd_sync\n-\t\t\tfi\n-\t\t)\n-\t\tfi\n-\tdone\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} ${prefix:+--super-prefix \"$prefix\"} submodule--helper sync ${GIT_QUIET:+--quiet} ${recursive:+--recursive} \"$@\"\n }\n \n cmd_absorbgitdirs()\n-- \n2.15.1\n\n"},{"id":"336588","messageId":"20180114211529.6391-3-pc44800@gmail.com","threadId":"47577","inReplyTo":"20180114211529.6391-1-pc44800@gmail.com","subject":"[PATCH v3 2/2] submodule: port submodule subcommand 'deinit' from shell to C","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-01-14T21:15:29Z","receivedAt":"2018-01-14T16:49:28Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"The same mechanism is used even for porting this submodule\nsubcommand, as used in the ported subcommands till now.\nThe function cmd_deinit in split up after porting into four\nfunctions: module_deinit(), for_each_listed_submodule(),\ndeinit_submodule() and deinit_submodule_cb().\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Prathamesh Chavan <pc44800@gmail.com>\n---\n builtin/submodule--helper.c | 147 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            |  55 +----------------\n 2 files changed, 148 insertions(+), 54 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 745d070ea..b1daca995 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -20,6 +20,7 @@\n #define OPT_QUIET (1 << 0)\n #define OPT_CACHED (1 << 1)\n #define OPT_RECURSIVE (1 << 2)\n+#define OPT_FORCE (1 << 3)\n \n typedef void (*each_submodule_fn)(const struct cache_entry *list_item,\n \t\t\t\t  void *cb_data);\n@@ -909,6 +910,151 @@ static int module_sync(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+struct deinit_cb {\n+\tconst char *prefix;\n+\tunsigned int flags;\n+};\n+#define DEINIT_CB_INIT { NULL, 0 }\n+\n+static void deinit_submodule(const char *path, const char *prefix,\n+\t\t\t     unsigned int flags)\n+{\n+\tconst struct submodule *sub;\n+\tchar *displaypath = NULL;\n+\tstruct child_process cp_config = CHILD_PROCESS_INIT;\n+\tstruct strbuf sb_config = STRBUF_INIT;\n+\tchar *sub_git_dir = xstrfmt(\"%s/.git\", path);\n+\n+\tsub = submodule_from_path(&null_oid, path);\n+\n+\tif (!sub || !sub->name)\n+\t\tgoto cleanup;\n+\n+\tdisplaypath = get_submodule_displaypath(path, prefix);\n+\n+\t/* remove the submodule work tree (unless the user already did it) */\n+\tif (is_directory(path)) {\n+\t\tstruct strbuf sb_rm = STRBUF_INIT;\n+\t\tconst char *format;\n+\n+\t\t/*\n+\t\t * protect submodules containing a .git directory\n+\t\t * NEEDSWORK: instead of dying, automatically call\n+\t\t * absorbgitdirs and (possibly) warn.\n+\t\t */\n+\t\tif (is_directory(sub_git_dir))\n+\t\t\tdie(_(\"Submodule work tree '%s' contains a .git \"\n+\t\t\t      \"directory (use 'rm -rf' if you really want \"\n+\t\t\t      \"to remove it including all of its history)\"),\n+\t\t\t    displaypath);\n+\n+\t\tif (!(flags & OPT_FORCE)) {\n+\t\t\tstruct child_process cp_rm = CHILD_PROCESS_INIT;\n+\t\t\tcp_rm.git_cmd = 1;\n+\t\t\targv_array_pushl(&cp_rm.args, \"rm\", \"-qn\",\n+\t\t\t\t\t path, NULL);\n+\n+\t\t\tif (run_command(&cp_rm))\n+\t\t\t\tdie(_(\"Submodule work tree '%s' contains local \"\n+\t\t\t\t      \"modifications; use '-f' to discard them\"),\n+\t\t\t\t      displaypath);\n+\t\t}\n+\n+\t\tstrbuf_addstr(&sb_rm, path);\n+\n+\t\tif (!remove_dir_recursively(&sb_rm, 0))\n+\t\t\tformat = _(\"Cleared directory '%s'\\n\");\n+\t\telse\n+\t\t\tformat = _(\"Could not remove submodule work tree '%s'\\n\");\n+\n+\t\tif (!(flags & OPT_QUIET))\n+\t\t\tprintf(format, displaypath);\n+\n+\t\tstrbuf_release(&sb_rm);\n+\t}\n+\n+\tif (mkdir(path, 0777))\n+\t\tprintf(_(\"could not create empty submodule directory %s\"),\n+\t\t      displaypath);\n+\n+\tcp_config.git_cmd = 1;\n+\targv_array_pushl(&cp_config.args, \"config\", \"--get-regexp\", NULL);\n+\targv_array_pushf(&cp_config.args, \"submodule.%s\\\\.\", sub->name);\n+\n+\t/* remove the .git/config entries (unless the user already did it) */\n+\tif (!capture_command(&cp_config, &sb_config, 0) && sb_config.len) {\n+\t\tchar *sub_key = xstrfmt(\"submodule.%s\", sub->name);\n+\t\t/*\n+\t\t * remove the whole section so we have a clean state when\n+\t\t * the user later decides to init this submodule again\n+\t\t */\n+\t\tgit_config_rename_section_in_file(NULL, sub_key, NULL);\n+\t\tif (!(flags & OPT_QUIET))\n+\t\t\tprintf(_(\"Submodule '%s' (%s) unregistered for path '%s'\\n\"),\n+\t\t\t\t sub->name, sub->url, displaypath);\n+\t\tfree(sub_key);\n+\t}\n+\n+cleanup:\n+\tfree(displaypath);\n+\tfree(sub_git_dir);\n+\tstrbuf_release(&sb_config);\n+}\n+\n+static void deinit_submodule_cb(const struct cache_entry *list_item,\n+\t\t\t\tvoid *cb_data)\n+{\n+\tstruct deinit_cb *info = cb_data;\n+\tdeinit_submodule(list_item->name, info->prefix, info->flags);\n+}\n+\n+static int module_deinit(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct deinit_cb info = DEINIT_CB_INIT;\n+\tstruct pathspec pathspec;\n+\tstruct module_list list = MODULE_LIST_INIT;\n+\tint quiet = 0;\n+\tint force = 0;\n+\tint all = 0;\n+\n+\tstruct option module_deinit_options[] = {\n+\t\tOPT__QUIET(&quiet, N_(\"Suppress submodule status output\")),\n+\t\tOPT__FORCE(&force, N_(\"Remove submodule working trees even if they contain local changes\")),\n+\t\tOPT_BOOL(0, \"all\", &all, N_(\"Unregister all submodules\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule deinit [--quiet] [-f | --force] [--all | [--] [<path>...]]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_deinit_options,\n+\t\t\t     git_submodule_helper_usage, 0);\n+\n+\tif (all && argc) {\n+\t\terror(\"pathspec and --all are incompatible\");\n+\t\tusage_with_options(git_submodule_helper_usage,\n+\t\t\t\t   module_deinit_options);\n+\t}\n+\n+\tif (!argc && !all)\n+\t\tdie(_(\"Use '--all' if you really want to deinitialize all submodules\"));\n+\n+\tif (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)\n+\t\tBUG(\"module_list_compute should not choke on empty pathspec\");\n+\n+\tinfo.prefix = prefix;\n+\tif (quiet)\n+\t\tinfo.flags |= OPT_QUIET;\n+\tif (force)\n+\t\tinfo.flags |= OPT_FORCE;\n+\n+\tfor_each_listed_submodule(&list, deinit_submodule_cb, &info);\n+\n+\treturn 0;\n+}\n+\n static int clone_submodule(const char *path, const char *gitdir, const char *url,\n \t\t\t   const char *depth, struct string_list *reference,\n \t\t\t   int quiet, int progress)\n@@ -1691,6 +1837,7 @@ static struct cmd_struct commands[] = {\n \t{\"status\", module_status, SUPPORT_SUPER_PREFIX},\n \t{\"print-default-remote\", print_default_remote, 0},\n \t{\"sync\", module_sync, SUPPORT_SUPER_PREFIX},\n+\t{\"deinit\", module_deinit, 0},\n \t{\"remote-branch\", resolve_remote_submodule_branch, 0},\n \t{\"push-check\", push_check, 0},\n \t{\"absorb-git-dirs\", absorb_git_dirs, SUPPORT_SUPER_PREFIX},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 0825cae14..24914963c 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -428,60 +428,7 @@ cmd_deinit()\n \t\tshift\n \tdone\n \n-\tif test -n \"$deinit_all\" && test \"$#\" -ne 0\n-\tthen\n-\t\techo >&2 \"$(eval_gettext \"pathspec and --all are incompatible\")\"\n-\t\tusage\n-\tfi\n-\tif test $# = 0 && test -z \"$deinit_all\"\n-\tthen\n-\t\tdie \"$(eval_gettext \"Use '--all' if you really want to deinitialize all submodules\")\"\n-\tfi\n-\n-\t{\n-\t\tgit submodule--helper list --prefix \"$wt_prefix\" \"$@\" ||\n-\t\techo \"#unmatched\" $?\n-\t} |\n-\twhile read -r mode sha1 stage sm_path\n-\tdo\n-\t\tdie_if_unmatched \"$mode\" \"$sha1\"\n-\t\tname=$(git submodule--helper name \"$sm_path\") || exit\n-\n-\t\tdisplaypath=$(git submodule--helper relative-path \"$sm_path\" \"$wt_prefix\")\n-\n-\t\t# Remove the submodule work tree (unless the user already did it)\n-\t\tif test -d \"$sm_path\"\n-\t\tthen\n-\t\t\t# Protect submodules containing a .git directory\n-\t\t\tif test -d \"$sm_path/.git\"\n-\t\t\tthen\n-\t\t\t\tdie \"$(eval_gettext \"\\\n-Submodule work tree '\\$displaypath' contains a .git directory\n-(use 'rm -rf' if you really want to remove it including all of its history)\")\"\n-\t\t\tfi\n-\n-\t\t\tif test -z \"$force\"\n-\t\t\tthen\n-\t\t\t\tgit rm -qn \"$sm_path\" ||\n-\t\t\t\tdie \"$(eval_gettext \"Submodule work tree '\\$displaypath' contains local modifications; use '-f' to discard them\")\"\n-\t\t\tfi\n-\t\t\trm -rf \"$sm_path\" &&\n-\t\t\tsay \"$(eval_gettext \"Cleared directory '\\$displaypath'\")\" ||\n-\t\t\tsay \"$(eval_gettext \"Could not remove submodule work tree '\\$displaypath'\")\"\n-\t\tfi\n-\n-\t\tmkdir \"$sm_path\" || say \"$(eval_gettext \"Could not create empty submodule directory '\\$displaypath'\")\"\n-\n-\t\t# Remove the .git/config entries (unless the user already did it)\n-\t\tif test -n \"$(git config --get-regexp submodule.\"$name\\.\")\"\n-\t\tthen\n-\t\t\t# Remove the whole section so we have a clean state when\n-\t\t\t# the user later decides to init this submodule again\n-\t\t\turl=$(git config submodule.\"$name\".url)\n-\t\t\tgit config --remove-section submodule.\"$name\" 2>/dev/null &&\n-\t\t\tsay \"$(eval_gettext \"Submodule '\\$name' (\\$url) unregistered for path '\\$displaypath'\")\"\n-\t\tfi\n-\tdone\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper deinit ${GIT_QUIET:+--quiet} ${prefix:+--prefix \"$prefix\"} ${force:+--force} ${deinit_all:+--all} \"$@\"\n }\n \n is_tip_reachable () (\n-- \n2.15.1\n\n"},{"id":"336671","messageId":"xmqq4lnlvbty.fsf@gitster.mtv.corp.google.com","threadId":"47577","inReplyTo":"20180114211529.6391-1-pc44800@gmail.com","subject":"Re: [PATCH v3 0/2] Incremental rewrite of git-submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-16T19:32:41Z","receivedAt":"2018-01-16T19:32:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Prathamesh Chavan <pc44800@gmail.com> writes:\n\n> Changes in v3:\n>\n> * For the variables: super_config_url and sub_origin_url, xstrdup() was used\n>   while assigning \"\" to them, before freeing.\n>\n> * In case of the function deinit_submodule, since the orignal code doesn't die\n>   upon failure of the function mkdir(), printf was used instead of die_errno.\n>\n> As before you can find this series at:\n> https://github.com/pratham-pc/git/commits/patch-series-2\n>\n> And its build report is available at:\n> https://travis-ci.org/pratham-pc/git/builds/\n> Branch: patch-series-2\n> Build #197\n>\n> Prathamesh Chavan (2):\n>   submodule: port submodule subcommand 'sync' from shell to C\n>   submodule: port submodule subcommand 'deinit' from shell to C\n>\n>  builtin/submodule--helper.c | 340 ++++++++++++++++++++++++++++++++++++++++++++\n>  git-submodule.sh            | 112 +--------------\n>  2 files changed, 342 insertions(+), 110 deletions(-)\n\nLooks sensible.  Thanks.\n\n"}]}