Subject: Re: [PATCH mm/mv-file-to-no-such-dir-with-slash] mv: let 'git mv file no-such-dir/' error out on Windows, too

From: Johannes Sixt <[email-masked]>
Date: Fri, 10 Jan 2014 20:21:58 +0100

Am 09.01.2014 23:42, schrieb Junio C Hamano:
> Johannes Sixt <[email-masked]> writes:
> 
>> The previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out)
>> relies on that rename("src", "dst/") fails if directory dst does not
>> exist (note the trailing slash). This does not work as expected on Windows:
>> This rename() call is successful. Insert an explicit check for this case.
> 
> Could you care to explain "Successful how" a bit here?  Do we see
> no-such-dir mkdir'ed and then no-such-dir/file created?  Do we see
> file moved to a new file whose name is no-such-dir/?  I am guessing
> that it would be the latter, but if that is the case we would need
> at least an air-quote around "successful".

The file is renamed to no-such-dir without the slash at the end. The
updated commit message would be:

mv: let 'git mv file no-such-dir/' error out on Windows, too

The previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out)
relies on that rename("src", "dst/") fails if directory dst does not
exist (note the trailing slash). This does not work as expected on Windows:
The rename() call does not fail, but renames src to dst (without the
trailing slash). Insert an explicit check for this case to force an error.

This changes the error message from

   $ git mv file no-such-dir/
   fatal: renaming 'file' failed: Not a directory

to

   $ git mv file no-such-dir/
   fatal: destination directory does not exist, source=file, destination=no-such-dir/

Signed-off-by: Johannes Sixt <[email-masked]>

> 
>> This changes the error message from
>>
>>    $ git mv file no-such-dir/
>>    fatal: renaming 'file' failed: Not a directory
>>
>> to
>>
>>    $ git mv file no-such-dir/
>>    fatal: destination directory does not exist, source=file, destination=no-such-dir/
>>
>> Signed-off-by: Johannes Sixt <[email-masked]>
>> ---
>>  builtin/mv.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/builtin/mv.c b/builtin/mv.c
>> index 08fbc03..21c46d1 100644
>> --- a/builtin/mv.c
>> +++ b/builtin/mv.c
>> @@ -214,6 +214,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>>  			}
>>  		} else if (string_list_has_string(&src_for_dst, dst))
>>  			bad = _("multiple sources for the same target");
>> +		else if (is_dir_sep(dst[strlen(dst) - 1]))
>> +			bad = _("destination directory does not exist");
>>  		else
>>  			string_list_insert(&src_for_dst, dst);
>