> -----Original Message-----
> From: Stefan Beller [mailto:sbel...@google.com]
> Sent: Friday, December 02, 2016 7:30 PM
> To: bmw...@google.com; David Turner
> Cc: git@vger.kernel.org; sand...@crustytoothpaste.net; hvo...@hvoigt.net;
> gits...@pobox.com; Stefan Beller
> Subject: [RFC PATCHv2 08/17] update submodules: add depopulate_submodule
> 
> Implement the functionality needed to enable work tree manipulating
> commands so that a deleted submodule should not only affect the index
> (leaving all the files of the submodule in the work tree) but also to
> remove the work tree of the superproject (including any untracked files).

"including any untracked files" bothers me, I think.  Checkout is not usually 
willing to overwrite untracked files; it seems odd to me that it would be 
willing to do so in the submodule case.  I would be OK if they were both 
untracked and gitignored, I think.

> To do so, we need an equivalent of "rm -rf", which is already found in
> entry.c, so expose that and for clarity add a suffix "_or_dir" to it.
> 
> That will only work properly when the submodule uses a gitfile instead of
> a .git directory and no untracked files are present. Otherwise the removal
> will fail with a warning (which is just what happened until now).
> 
> Signed-off-by: Stefan Beller <sbel...@google.com>
> ---
>  cache.h     |  2 ++
>  entry.c     |  5 +++++
>  submodule.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  submodule.h |  1 +
>  4 files changed, 54 insertions(+)
> 
> diff --git a/cache.h b/cache.h
> index a50a61a197..b645ca2f9a 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -2018,4 +2018,6 @@ void sleep_millisec(int millisec);
>   */
>  void safe_create_dir(const char *dir, int share);
> 
> +extern void remove_directory_or_die(struct strbuf *path);
> +
>  #endif /* CACHE_H */
> diff --git a/entry.c b/entry.c
> index c6eea240b6..02c4ac9f22 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -73,6 +73,11 @@ static void remove_subtree(struct strbuf *path)
>               die_errno("cannot rmdir '%s'", path->buf);  }
> 
> +void remove_directory_or_die(struct strbuf *path) {
> +     remove_subtree(path);
> +}
> +
>  static int create_file(const char *path, unsigned int mode)  {
>       mode = (mode & 0100) ? 0777 : 0666;
> diff --git a/submodule.c b/submodule.c
> index 62e9ef3872..7bb64d6c69 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -324,6 +324,52 @@ void prepare_submodule_repo_env(struct argv_array
> *out)
>       argv_array_push(out, "GIT_DIR=.git");
>  }
> 
> +int depopulate_submodule(const char *path) {
> +     int ret = 0;
> +     struct strbuf pathbuf = STRBUF_INIT;
> +     char *dot_git = xstrfmt("%s/.git", path);
> +
> +     /* Is it populated? */
> +     if (!resolve_gitdir(dot_git))
> +             goto out;
> +
> +     /* Does it have a .git directory? */
> +     if (!submodule_uses_gitfile(path)) {
> +             struct child_process cp = CHILD_PROCESS_INIT;
> +
> +             prepare_submodule_repo_env(&cp.env_array);
> +             argv_array_pushl(&cp.args, "submodule--helper",
> +                              "embed-git-dirs", path, NULL);
> +             cp.git_cmd = 1;
> +             if (run_command(&cp)) {
> +                     warning(_("Cannot remove submodule '%s'\n"
> +                               "because it (or one of its nested submodules)
> has a git \n"
> +                               "directory in the working tree, which could 
> not
> be embedded\n"
> +                               "the superprojects git directory
> automatically."), path);

What if instead it couldn't run the command because you're out of file 
descriptors or pids or memory or something?

I think this message should be in submodule--helper --embed-git-dirs instead, 
and we should just pass it through here.  Or, perhaps, instead of shelling out 
here, we should just call the functions directly?

> +                     ret = -1;
> +                     goto out;
> +             }
> +
> +             if (!submodule_uses_gitfile(path)) {
> +                     /*
> +                      * We should be using a gitfile by now, let's double

Comma splice.  

> +                      * check as loosing the git dir would be fatal.

s/loosing/losing/

> +                      */
> +                     die("BUG: \"git submodule--helper embed git-dirs '%s'\"
> "
> +                         "did not embed the git-dirs recursively for '%s'",
> +                         path, path);
> +             }
> +     }
> +
> +     strbuf_addstr(&pathbuf, path);
> +     remove_directory_or_die(&pathbuf);
> +out:
> +     strbuf_release(&pathbuf);
> +     free(dot_git);
> +     return ret;
> +}
> +
>  /* Helper function to display the submodule header line prior to the full
>   * summary output. If it can locate the submodule objects directory it
> will
>   * attempt to lookup both the left and right commits and put them into
> the diff --git a/submodule.h b/submodule.h index 7d890e0464..d8bb1d4baf
> 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -63,6 +63,7 @@ extern void set_config_update_recurse_submodules(int
> value);
>   */
>  extern int submodule_is_interesting(const char *path);  extern int
> submodules_interesting_for_update(void);
> +extern int depopulate_submodule(const char *path);
>  extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
> extern int fetch_populated_submodules(const struct argv_array *options,
>                              const char *prefix, int command_line_option,
> --
> 2.11.0.rc2.28.g2673dad

Reply via email to