RE: [RFC PATCHv2 08/17] update submodules: add depopulate_submodule
Show 12 quoted lines
> -----Original Message-----
> From: Stefan Beller [mailto:sbeller@google.com]
> Sent: Friday, December 02, 2016 7:30 PM
> To: bmwill@google.com; David Turner
> Cc: git@vger.kernel.org; sandals@crustytoothpaste.net; hvoigt@hvoigt.net;
> gitster@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.
Show 73 quoted lines
> 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 <sbeller@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?
Show 7 quoted lines
> + ret = -1;
> + goto out;
> + }
> +
> + if (!submodule_uses_gitfile(path)) {
> + /*
> + * We should be using a gitfile by now, let's double> + * check as loosing the git dir would be fatal.
Show 35 quoted lines
> + */
> + 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