Re: [PATCHv2 4/5] ok_to_remove_submodule: absorb the submodule git dir

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Stefan Beller <sbeller@xxxxxxxxxx> writes:

> 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 <sbeller@xxxxxxxxxx>
> ---
>  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);



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]