Re: [PATCH v4 4/4] t0060: verify that basename() and dirname() work as expected

[Ramsay Jones <ramsay@ramsayjones.plus.com>]

On 13/01/16 05:47, Torsten Bögershausen wrote:
> On 01/12/2016 08:57 AM, Johannes Schindelin wrote:
> 
[snip]

>> +
>> +static struct test_data basename_data[] = {
>> +	/* --- POSIX type paths --- */
>> +	{ NULL,              "."    },
>> +	{ "",                "."    },
>> +	{ ".",               "."    },
>> +	{ "..",              ".."   },
>> +	{ "/",               "/"    },
>> +#if defined(__CYGWIN__) && !defined(NO_LIBGEN_H)
> Why the !defined(NO_LIBGEN_H)

These tests were derived from a standalone test program which
I was using to test my implementation of gitbasename() *and*
to document the differences between it and the system versions
of the <libgen.h> basename(). (I didn't bother with the GNU version
of basename, which is somewhat strange).

This particular section documents what is almost certainly a bug
in the cygwin basename() and also documents my choice of 'fix'.
(ie. in my implementation I chose to return '/' for '//', which
is one of the possible options that POSIX allows.)

> 
> Shouldn't CYGWIN always behave the same ?
> And, in general, shouldn't all Windows version behave the same ?

Hmm, cygwin is not really a 'Windows version', so ... (We have been
caught out before by cygwin 'supporting' UNC paths, so support for
'//' is open to question. Also, some git programs on cygwin kinda
sorta support dos paths ...)

> (This would mean, that we always use ../compat/basename.c for all kind
> of Windows Git implementattion. Would there be a drawback ?)

And maybe not just Windows ...

> 
> 
> The main problem is, that t0060 fails under Mac OS (with mac ports installed):
> expecting success: test-path-utils dirname
> error: FAIL: dirname(//) => '/' != '//'

Yep, not surprised. Again that test file was developed and tested on
only the five platforms available to me at the time, namely: Linux (both
32 and 64bit), Windows XP 32-bit (MSVC), MinGW 32-bit and Cygwin 32-bit.

POSIX says, in part [1]:

    If the string pointed to by path consists entirely of the '/' character,
    basename() shall return a pointer to the string "/". If the string pointed
    to by path is exactly "//", it is implementation-defined whether '/' or "//"
    is returned.

[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/basename.html

So we should expect other systems to differ, even if they support POSIX. (and maybe
not just this test case.)

> 
> not ok 2 - dirname
> #       test-path-utils dirname
> 
> To my understanding the treatment of a path name like "//"
> is defined as "undefined":
> If /string/ is "//", it is implementation-defined whether steps 3 to 6 are skipped or processed.
> http://pubs.opengroup.org/onlinepubs/009695399/utilities/basename.html
> 
> What I understand is that a path like "//XX/YY/ZZ" can be handled in 3 different ways:
> a) Same as "/XX/YY/ZZ", silently turning "//" into "/"
> b) Same as "\\XX\YY\ZZ", using UNC names under Windows. # Note: this reads as "\\\\XX\\YY\\ZZ" in a C program
> c) As invalid
> 
> 
> Does it make sense to use the compat/basename.c for all Git implementations ?
> (Or at least for CYGWIN, Mac OS, MSYS, MSVC)
> 

Maybe, I hadn't got that far yet ... :-D

ATB,
Ramsay Jones