Re: [PATCH 1/2] mingw: verify that paths are not mistaken for remote nicknames

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

 



On Mon, May 29, 2017 at 10:25 PM, Johannes Sixt <j6t@xxxxxxxx> wrote:
> Am 26.05.2017 um 05:35 schrieb Junio C Hamano:
>> When asked to open/fopen a path, e.g. "a/b:/c", which does not exist
>> on the filesystem, Windows (correctly) fails to open it but sets
>> EINVAL to errno because the pathname has characters that cannot be
>> stored in its filesystem.
>>
>> As this is an expected failure, teach is_missing_file_error() helper
>> about this case.
>>
>> This is RFC, as there may be a case where we get EINVAL from
>> open/fopen for reasons other than "the filesystem does not like this
>> pathname" that may be worth reporting to the user, and this change
>> is sweeping such an error under the rug.
>>
>> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
>> ---
>>   wrapper.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/wrapper.c b/wrapper.c
>> index f1c87ec7ea..74aa3b7803 100644
>> --- a/wrapper.c
>> +++ b/wrapper.c
>> @@ -434,6 +434,10 @@ static void warn_on_inaccessible(const char *path)
>>    * see if the errno indicates a missing file that we can safely ignore.
>>    */
>>   static int is_missing_file_error(int errno_) {
>> +#ifdef GIT_WINDOWS_NATIVE
>> +     if (errno_ == EINVAL)
>> +             return 1;
>> +#endif
>>       return (errno_ == ENOENT || errno_ == ENOTDIR);
>>   }
>
> I would prefer to catch the case in the compatibility layer. Here is
> a two patch series that would replace your 12/13 and 13/13.
>
> ---- 8< ----
> From: Johannes Schindelin <Johannes.Schindelin@xxxxxx>
> Subject: mingw: verify that paths are not mistaken for remote nicknames
>
> This added test case simply verifies that users will not be bothered
> with bogus complaints à la
>
>         warning: unable to access '.git/remotes/D:\repo': Invalid argument
>
> when fetching from a Windows path (in this case, D:\repo).
>
> [j6t: mark the new test as test_expect_failure]
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> Signed-off-by: Johannes Sixt <j6t@xxxxxxxx>
> ---
>  t/t5580-clone-push-unc.sh | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/t/t5580-clone-push-unc.sh b/t/t5580-clone-push-unc.sh
> index b195f71ea9..fd719a209e 100755
> --- a/t/t5580-clone-push-unc.sh
> +++ b/t/t5580-clone-push-unc.sh
> @@ -1,13 +1,19 @@
>  #!/bin/sh
>
> -test_description='various UNC path tests (Windows-only)'
> +test_description='various Windows-only path tests'
>  . ./test-lib.sh
>
>  if ! test_have_prereq MINGW; then
> -       skip_all='skipping UNC path tests, requires Windows'
> +       skip_all='skipping Windows-only path tests'
>         test_done
>  fi
>
> +test_expect_failure 'remote nick cannot contain backslashes' '
> +       BACKSLASHED="$(pwd | tr / \\\\)" &&
> +       git ls-remote "$BACKSLASHED" >out 2>err &&
> +       ! grep "unable to access" err
> +'

Doesn't this need test_i18ngrep?:

    wrapper.c:581:  warning_errno(_("unable to access '%s'"), path);
    wrapper.c:602:          die_errno(_("unable to access '%s'"), path);

Or is it one of these, which isn't translated:

    http-push.c:1531:               error("unable to access '%s': %s",
url, curl_errorstr);
    remote-curl.c:319:              die("unable to access '%s': %s",
url.buf, curl_errorstr);


> +
>  UNCPATH="$(pwd)"
>  case "$UNCPATH" in
>  [A-Z]:*)
> --
> 2.13.0.55.g17b7d13330




[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]