Re: [GSoC][PATCH 6/6 v2] submodule: port submodule subcommand 'deinit' from shell to C

2017-06-27 Thread Christian Couder
On Tue, Jun 27, 2017 at 1:11 AM, Prathamesh Chavan  wrote:

> +static void deinit_submodule(const struct cache_entry *list_item,
> +void *cb_data)
> +{
> +   struct deinit_cb *info = cb_data;
> +   const struct submodule *sub;
> +   char *displaypath = NULL;
> +   struct child_process cp_config = CHILD_PROCESS_INIT;
> +   struct strbuf sb_config = STRBUF_INIT;
> +   char *sm_path = xstrdup(list_item->name);
> +   char *sub_git_dir = xstrfmt("%s/.git", sm_path);
> +
> +   sub = submodule_from_path(null_sha1, sm_path);
> +
> +   if (!sub->name)

In the previous patch "!sub" is used before "!sub->url", so we might
want to check "!sub" here too.

> +   goto cleanup;
> +
> +   displaypath = get_submodule_displaypath(sm_path, info->prefix);
> +
> +   /* remove the submodule work tree (unless the user already did it) */
> +   if (is_directory(sm_path)) {
> +   struct child_process cp = CHILD_PROCESS_INIT;
> +
> +   /* protect submodules containing a .git directory */
> +   if (is_git_directory(sub_git_dir))
> +   die(_("Submodule work tree '%s' contains a .git "
> + "directory use 'rm -rf' if you really want "
> + "to remove it including all of its history"),
> + displaypath);
> +
> +   if (!info->force) {
> +   struct child_process cp_rm = CHILD_PROCESS_INIT;
> +   cp_rm.git_cmd = 1;
> +   argv_array_pushl(&cp_rm.args, "rm", "-qn", sm_path,
> +NULL);
> +
> +   /* list_item->name is changed by cmd_rm() below */

It looks like cmd_rm() is not used anymore below, so this comment
could go and the sm_path variable might not be needed any more.

> +   if (run_command(&cp_rm))
> +   die(_("Submodule work tree '%s' contains 
> local "
> + "modifications; use '-f' to discard 
> them"),
> + displaypath);
> +   }
> +
> +   cp.use_shell = 1;

Do we really need a shell here?

> +   argv_array_pushl(&cp.args, "rm", "-rf", sm_path, NULL);
> +   if (!run_command(&cp)) {
> +   if (!info->quiet)
> +   printf(_("Cleared directory '%s'\n"),
> +displaypath);
> +   } else {
> +   if (!info->quiet)
> +   printf(_("Could not remove submodule work 
> tree '%s'\n"),
> +displaypath);
> +   }
> +   }
> +
> +   if (mkdir(sm_path, 0700))

Are you sure about the 0700 mode?
Shouldn't this depend on the shared repository settings?

> +   die(_("could not create empty submodule directory %s"),
> + displaypath);


[GSoC][PATCH 6/6 v2] submodule: port submodule subcommand 'deinit' from shell to C

2017-06-26 Thread Prathamesh Chavan
The same mechanism is used even for porting this submodule
subcommand, as used in the ported subcommands till now.
The function cmd_deinit in split up after porting into three
functions: module_deinit, for_each_submodule_list and
deinit_submodule.

Mentored-by: Christian Couder 
Mentored-by: Stefan Beller 
Signed-off-by: Prathamesh Chavan 
---
 builtin/submodule--helper.c | 140 
 git-submodule.sh|  55 +
 2 files changed, 141 insertions(+), 54 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4e3322846..17942529b 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -751,6 +751,145 @@ static int module_status(int argc, const char **argv, 
const char *prefix)
return 0;
 }
 
+struct deinit_cb {
+   const char *prefix;
+   unsigned int quiet: 1;
+   unsigned int force: 1;
+   unsigned int all: 1;
+};
+#define DEINIT_CB_INIT { NULL, 0, 0, 0 }
+
+static void deinit_submodule(const struct cache_entry *list_item,
+void *cb_data)
+{
+   struct deinit_cb *info = cb_data;
+   const struct submodule *sub;
+   char *displaypath = NULL;
+   struct child_process cp_config = CHILD_PROCESS_INIT;
+   struct strbuf sb_config = STRBUF_INIT;
+   char *sm_path = xstrdup(list_item->name);
+   char *sub_git_dir = xstrfmt("%s/.git", sm_path);
+
+   sub = submodule_from_path(null_sha1, sm_path);
+
+   if (!sub->name)
+   goto cleanup;
+
+   displaypath = get_submodule_displaypath(sm_path, info->prefix);
+
+   /* remove the submodule work tree (unless the user already did it) */
+   if (is_directory(sm_path)) {
+   struct child_process cp = CHILD_PROCESS_INIT;
+
+   /* protect submodules containing a .git directory */
+   if (is_git_directory(sub_git_dir))
+   die(_("Submodule work tree '%s' contains a .git "
+ "directory use 'rm -rf' if you really want "
+ "to remove it including all of its history"),
+ displaypath);
+
+   if (!info->force) {
+   struct child_process cp_rm = CHILD_PROCESS_INIT;
+   cp_rm.git_cmd = 1;
+   argv_array_pushl(&cp_rm.args, "rm", "-qn", sm_path,
+NULL);
+
+   /* list_item->name is changed by cmd_rm() below */
+   if (run_command(&cp_rm))
+   die(_("Submodule work tree '%s' contains local "
+ "modifications; use '-f' to discard 
them"),
+ displaypath);
+   }
+
+   cp.use_shell = 1;
+   argv_array_pushl(&cp.args, "rm", "-rf", sm_path, NULL);
+   if (!run_command(&cp)) {
+   if (!info->quiet)
+   printf(_("Cleared directory '%s'\n"),
+displaypath);
+   } else {
+   if (!info->quiet)
+   printf(_("Could not remove submodule work tree 
'%s'\n"),
+displaypath);
+   }
+   }
+
+   if (mkdir(sm_path, 0700))
+   die(_("could not create empty submodule directory %s"),
+ displaypath);
+
+   cp_config.git_cmd = 1;
+   argv_array_pushl(&cp_config.args, "config", "--get-regexp", NULL);
+   argv_array_pushf(&cp_config.args, "submodule.%s\\.", sub->name);
+
+   /* remove the .git/config entries (unless the user already did it) */
+   if (!capture_command(&cp_config, &sb_config, 0) && sb_config.len) {
+   char *sub_key = xstrfmt("submodule.%s", sub->name);
+   /*
+* remove the whole section so we have a clean state when
+* the user later decides to init this submodule again
+*/
+   git_config_rename_section_in_file(NULL, sub_key, NULL);
+   if (!info->quiet)
+   printf(_("Submodule '%s' (%s) unregistered for path 
'%s'\n"),
+sub->name, sub->url, displaypath);
+   free(sub_key);
+   }
+
+cleanup:
+   free(displaypath);
+   free(sub_git_dir);
+   free(sm_path);
+   strbuf_release(&sb_config);
+}
+
+static int module_deinit(int argc, const char **argv, const char *prefix)
+{
+   struct deinit_cb info = DEINIT_CB_INIT;
+   struct pathspec pathspec;
+   struct module_list list = MODULE_LIST_INIT;
+   int quiet = 0;
+   int force = 0;
+   int all = 0;
+
+   struct option module_deinit_options[] = {
+   OPT__QUIET(&quiet, N_("Suppress submodule status output")),
+   OPT__