Re: [PATCHv3 4/4] rm: add absorb a submodules git dir before deletion

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 14, 2016, 23:55 UTC
Message-ID: <xmqqlgvivze9.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20161214224101.6211-5-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 6 quoted lines

>  			if (list.entry[i].is_submodule) {
>  				if (is_empty_dir(path)) {
> +					if (rmdir(path))
> +						die_errno("git rm: '%s'", path);
> +				} else if (file_exists(path))
> +					/* non empty directory: */

Lose colon?

> +					remove_directory_or_die(path);

... otherwise?  I.e.

				else
					???

If we are running "git rm -f <path>", then the path could be a submodule in the index and on the filesystem, it could be (1) already missing, as the user removed an empty submodule directory she is not interested in, (2) a non-directory, e.g. a file or a symbolic link, perhaps because she was trying to reorganize the superproject working tree but decided against it, or (3) something else?

(1) is perfectly OK; we end up with a result without the path, which is what "git rm -f" wanted to do anyway. I am not sure what should happen in (2), and what other corner cases there are for (3), though.

And use of file_exists(path) in the above patch may trigger a strange error message in case (2), as remove_directory_or_die() would say "path is not a directory", to which the user will say "Yes I know, I wanted you to remove it with 'git rm -f'".