Re: [PATCHv2 4/5] ok_to_remove_submodule: absorb the submodule git dir
From: Junio C Hamano <hidden>
Date: 2016-12-14 18:48:27
Stefan Beller [off-list ref] writes:
quoted hunk
It is a major reason to say no, when deciding if a submodule can be deleted, if the git directory of the submodule being contained in the submodule's working directory. Migrate the git directory into the superproject instead of failing, and proceed with the other checks. Signed-off-by: Stefan Beller <redacted> --- submodule.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)diff --git a/submodule.c b/submodule.c index 2d13744b06..e42efa2337 100644 --- a/submodule.c +++ b/submodule.c@@ -1026,11 +1026,22 @@ int ok_to_remove_submodule(const char *path, unsigned flags) struct strbuf buf = STRBUF_INIT; int ok_to_remove = 1; + /* Is it there? */ if (!file_exists(path) || is_empty_dir(path)) return 1; - if (!submodule_uses_gitfile(path)) - return 0; + /* Does it have a .git directory? */ + if (!submodule_uses_gitfile(path)) { + absorb_git_dir_into_superproject("", path, + ABSORB_GITDIR_RECURSE_SUBMODULES); + + /* + * We should be using a gitfile by now. Let's double + * check as losing the git dir would be fatal. + */ + if (!submodule_uses_gitfile(path)) + return 0; + }
It feels a bit funny for a function that answers yes/no question to actually have a side effect, but probably it is OK. It feels dirty, but I dunno. A brief reading of the above makes us wonder what should happen if the absorb_git_dir_into_superproject() call fails, but a separate "submodule_uses_gitfile()" is needed to see if it failed? Perhaps the function needs to tell the caller if it succeeded?
argv_array_pushl(&cp.args, "status", "--porcelain", "--ignore-submodules=none", NULL);