Re: [PATCH] mv: allow moving nested submodules

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 18, 2016, 21:13 UTC
Message-ID: <xmqqfuui4o4e.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqk2ju4ozy.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 10 quoted lines

> If ignore-errors is set and rename fails, this would fall through
> and try to touch this codepath...
>
>>  			if (submodule_gitfile[i]) {
>>  				if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
>>  					connect_work_tree_and_git_dir(dst, submodule_gitfile[i]);
>
> ... but I am not sure if this thing is prepared to cope with such a
> case?  src should have been moved to dst but if rename() failed we
> wouldn't see what we expect at dst, or would we?

In other words, I was wondering if this part should read more like this:

 builtin/mv.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index aeae855..37ed0fc 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -252,9 +252,14 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 		int pos;
 		if (show_only || verbose)
 			printf(_("Renaming %s to %s\n"), src, dst);
-		if (!show_only && mode != INDEX) {
-			if (rename(src, dst) < 0 && !ignore_errors)
+		if (show_only)
+			;
+		else {
+			if (mode != INDEX && rename(src, dst) < 0) {
+				if (ignore_errors)
+					continue;
 				die_errno(_("renaming '%s' failed"), src);
+			}
 			if (submodule_gitfile[i]) {
 				if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
 					connect_work_tree_and_git_dir(dst, submodule_gitfile[i]);