Re: [PATCH] mv: allow moving nested submodules

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 18, 2016, 20:54 UTC
Message-ID: <xmqqk2ju4ozy.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1460998489-2155-1-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 10 quoted lines

>  		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) {
> +			if (mode != INDEX &&
> +			    rename(src, dst) < 0 &&
> +			    !ignore_errors)
>  				die_errno(_("renaming '%s' failed"), src);
> +

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?

Show 32 quoted lines

> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
> index 4008fae..4a2570e 100755
> --- a/t/t7001-mv.sh
> +++ b/t/t7001-mv.sh
> @@ -292,6 +292,9 @@ test_expect_success 'setup submodule' '
>  	echo content >file &&
>  	git add file &&
>  	git commit -m "added sub and file" &&
> +	mkdir -p deep/directory/hierachy &&
> +	git submodule add ./. deep/directory/hierachy/sub &&
> +	git commit -m "added another submodule" &&
>  	git branch submodule
>  '
>  
> @@ -475,4 +478,17 @@ test_expect_success 'mv -k does not accidentally destroy submodules' '
>  	git checkout .
>  '
>  
> +test_expect_success 'moving a submodule in nested directories' '
> +	(
> +		cd deep &&
> +		git mv directory ../ &&
> +		# git status would fail if the update of linking git dir to
> +		# work dir of the submodule failed.
> +		git status &&
> +		git config -f ../.gitmodules submodule.deep/directory/hierachy/sub.path >../actual &&
> +		echo "directory/hierachy/sub" >../expect
> +	) &&
> +	test_cmp actual expect
> +'
> +
>  test_done