Subject: Re: [PATCH 08/16] update submodules: add depopulate_submodule

From: Brandon Williams <[email-masked]>
Date: Tue, 15 Nov 2016 15:44:03 -0800

On 11/15, Stefan Beller wrote:
> Implement the functionality needed to enable work tree manipulating
> commands to that a deleted submodule should not only affect the index

"to that a deleted"  did you mean "so that a deleted"

> (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).
> 
> 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).

So if a submodule uses a .git directory then it will be ignored during
the checkout?  All other submodules will actually be removed? Couldn't
you end up in an undesirable state with a checkout effecting one
submodule but not another?

> diff --git a/cache.h b/cache.h
> index a50a61a..65c47e4 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);
>  
> +void remove_subtree_or_die(const char *path);
> +
>  #endif /* CACHE_H */

Should probably place an explicit 'extern' in the function prototype.

> +int depopulate_submodule(const char *path)
> +{
> +	int ret = 0;
> +	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)) {
> +		warning(_("cannot remove submodule '%s' because it (or one of "
> +			  "its nested submodules) uses a .git directory"),
> +			  path);
> +		ret = -1;
> +		goto out;
> +	}
> +
> +	remove_subtree_or_die(path);
> +
> +out:
> +	free(dot_git);
> +	return ret;
> +}

-- 
Brandon Williams