Re: [PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-19 Thread Jiang Xin
2013/9/18 Johannes Sixt :
> Am 17.09.2013 10:24, schrieb Jiang Xin:
>> I have checked the behavior of UNC path on Windows (msysGit):
>>
>> * I can cd to a UNC path:
>>
>> cd //server1/share1/path
>>
>> * can cd to other share:
>>
>> cd ../../share2/path
>>
>> * and can cd to other server's share:
>>
>> cd ../../../server2/share/path
>>
>> That means relative_path(path1, path2) support UNC paths out of the box.
>> We only need to check both path1 and path2 are UNC paths, or both not.
>
> Your tests are flawed. You issued the commands in bash, which (or rather
> MSYS) does everything for you that you need to make it work. But in
> reality it does not, because the system cannot apply .. to //server/share:
>
> $ git ls-remote //srv/public/../repos/his/setups.git
> fatal: '//srv/public/../repos/his/setups.git' does not appear to be a
> git repository
> fatal: Could not read from remote repository.
>
> Please make sure you have the correct access rights
> and the repository exists.
>
> even though the repository (and //srv/public, let me assure) exists:
>
> $ git ls-remote //srv/repos/his/setups.git
> bea489b0611a72c41f133343fdccbd3e2b9f80b5HEAD
> ...

After see this link (provided by Torsten):




I find the following commands could work:

$ git ls-remote //srv/repos/his/setups.git
$ git ls-remote //srv/repos/his/../his/setups.git
$ git ls-remote //?/UNC/srv/repos/his/setups.git
$ git ls-remote //?/UNC/srv/repos/../repos/his/setups.git

But no luck for this one:

$ git ls-remote //srv/repos/../repos/his/setups.git

I trace it using gdb, and find it failed in "stat()/mingw_stat()" call
of function "enter_repo" in path.c. But I can not find out why
"git ls-remote //?/UNC/srv/repos/../repos/his/setups.git" could
work (success in shell, failed in gdb).

> Please let me suggest not to scratch where there is no itch. ;) Your
> round v2 was good enough.
>
> If you really want to check UNC paths, then you must compare two path
> components after the the double-slash, not just one.
>

I have already try this (honor two path components after //)
during the reroll for patch v3. But I am not satisfied with it,
and it seems like over-engineered: Rename "have_same_root"
to "get_common_root_prefix_width", and it return -1 for no
same_root found, otherwize return the length of root_prefix_width.

Since restored the default behavior of setup.c in commit ("Use
simpler relative_path when set_git_dir"), function "relative_path"
are only used in "quote.c" and "builtin/clean.c", and two paths
provided to "relative_path" are always (I can not find exception)
relative paths. So no itch exist I think.

-- 
Jiang Xin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-18 Thread Torsten Bögershausen
On 2013-09-17 21.32, Johannes Sixt wrote:
> Am 17.09.2013 10:24, schrieb Jiang Xin:
>> I have checked the behavior of UNC path on Windows (msysGit):
>>
>> * I can cd to a UNC path:
>>
>> cd //server1/share1/path
>>
>> * can cd to other share:
>>
>> cd ../../share2/path
>>
>> * and can cd to other server's share:
>>
>> cd ../../../server2/share/path
>>
>> That means relative_path(path1, path2) support UNC paths out of the box.
>> We only need to check both path1 and path2 are UNC paths, or both not.
> 
> Your tests are flawed. You issued the commands in bash, which (or rather
> MSYS) does everything for you that you need to make it work. But in
> reality it does not, because the system cannot apply .. to //server/share:
> 
> $ git ls-remote //srv/public/../repos/his/setups.git
> fatal: '//srv/public/../repos/his/setups.git' does not appear to be a
> git repository
> fatal: Could not read from remote repository.
> 
> Please make sure you have the correct access rights
> and the repository exists.
> 
> even though the repository (and //srv/public, let me assure) exists:
> 
> $ git ls-remote //srv/repos/his/setups.git
> bea489b0611a72c41f133343fdccbd3e2b9f80b5HEAD
> ...
> 
> The situation does not change with your latest round (v3).
> 
> Please let me suggest not to scratch where there is no itch. ;) Your
> round v2 was good enough.
> 
> If you really want to check UNC paths, then you must compare two path
> components after the the double-slash, not just one.
> 
> Furthermore, you should audit all code that references
> is_absolute_path(), relative_path(), normalize_path_copy(), and possibly
> a few others whether the functions or call sites need improvement.
> That's worth a separate patch.
> 
> -- Hannes

I tend to agree here.
The V2 patch fixed a regression.
This should be one commit on its own:
Documentation/SubmittingPatches:
(1) Make separate commits for logically separate changes.

Fixing a bug is a good thing, thanks for working on this,

The support for UNC paths is a new feature, and this deserves a seperate commit.
/Torsten





--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-17 Thread Johannes Sixt
Am 17.09.2013 10:24, schrieb Jiang Xin:
> I have checked the behavior of UNC path on Windows (msysGit):
> 
> * I can cd to a UNC path:
> 
> cd //server1/share1/path
> 
> * can cd to other share:
> 
> cd ../../share2/path
> 
> * and can cd to other server's share:
> 
> cd ../../../server2/share/path
> 
> That means relative_path(path1, path2) support UNC paths out of the box.
> We only need to check both path1 and path2 are UNC paths, or both not.

Your tests are flawed. You issued the commands in bash, which (or rather
MSYS) does everything for you that you need to make it work. But in
reality it does not, because the system cannot apply .. to //server/share:

$ git ls-remote //srv/public/../repos/his/setups.git
fatal: '//srv/public/../repos/his/setups.git' does not appear to be a
git repository
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

even though the repository (and //srv/public, let me assure) exists:

$ git ls-remote //srv/repos/his/setups.git
bea489b0611a72c41f133343fdccbd3e2b9f80b5HEAD
...

The situation does not change with your latest round (v3).

Please let me suggest not to scratch where there is no itch. ;) Your
round v2 was good enough.

If you really want to check UNC paths, then you must compare two path
components after the the double-slash, not just one.

Furthermore, you should audit all code that references
is_absolute_path(), relative_path(), normalize_path_copy(), and possibly
a few others whether the functions or call sites need improvement.
That's worth a separate patch.

-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-17 Thread Jiang Xin
2013/9/13 Junio C Hamano :
> Jiang Xin  writes:
>
>> 2013/9/13 Junio C Hamano :
>>>
>>> For systems that need POSIX escape hatch for Apollo Domain ;-), we
>>> would need a bit more work.  When both path1 and path2 begin with a
>>> double-dash, we would need to check if they match up to the next
>>> slash, so that
>>>
>>>  - //host1/usr/src and //host1/usr/lib share the same root and the
>>>former can be made to ../src relative to the latter;
>>>
>>>  - //host1/usr/src and //host2/usr/lib are of separate roots.
>>>
>>> or something.
>>
>> But how could we know which platform supports network pathnames and
>> needs such implementation.
>
> Near the end of
>
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_12
>
> is this:
>
> If a pathname begins with two successive  characters, the
> first component following the leading  characters may be
> interpreted in an implementation-defined manner, although more than
> two leading  characters shall be treated as a single 
> character.
>
> Two points to note are
>
>  (1) Only paths that begin with exactly two slashes are special.
>
>  (2) As it is "implementation-defined", we are not even allowed to
>  treat that //host1/usr/src and //host1/usr/lib as sharing "the
>  same root", and make the former to ../src relative to the
>  latter.
>
> So in the strictest sense, we do not have to bother. As long as we
> make sure we do not molest anything that begins with exactly two
> slashes.
>

I have checked the behavior of UNC path on Windows (msysGit):

* I can cd to a UNC path:

cd //server1/share1/path

* can cd to other share:

cd ../../share2/path

* and can cd to other server's share:

cd ../../../server2/share/path

That means relative_path(path1, path2) support UNC paths out of the box.
We only need to check both path1 and path2 are UNC paths, or both not.

So, funciton “have_same_root" will write like this:


+static int have_same_root(const char *path1, const char *path2)
+{
+   int is_abs1, is_abs2;
+
+   is_abs1 = is_absolute_path(path1);
+   is_abs2 = is_absolute_path(path2);
+   if (is_abs1 && is_abs2) {
+   if (is_unc_path(path1) ^ is_unc_path(path2))
+   return 0;
+   return tolower(path1[0]) == tolower(path2[0]);
+   } else {
+   return !is_abs1 && !is_abs2;
+   }
+}


-- 
Jiang Xin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-13 Thread Torsten Bögershausen
On 13.09.13 06:55, Jiang Xin wrote:
> 2013/9/13 Junio C Hamano :
>> For systems that need POSIX escape hatch for Apollo Domain ;-), we
>> would need a bit more work.  When both path1 and path2 begin with a
>> double-dash, we would need to check if they match up to the next
>> slash, so that
>>
>>  - //host1/usr/src and //host1/usr/lib share the same root and the
>>former can be made to ../src relative to the latter;
>>
>>  - //host1/usr/src and //host2/usr/lib are of separate roots.
>>
>> or something.
>>
>>
> But how could we know which platform supports network pathnames and
> needs such implementation.
Similar to the has_dos_drive_prefix:

For Windows/Mingw we do like this

mingw.h
#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')

And all other platforms defines has_dos_drive_prefix() to be 0 here
git-compat-util.h
#ifndef has_dos_drive_prefix
#define has_dos_drive_prefix(path) 0
#endif

mingw.h:
#define has_unc_path_prefix(path) ((path)[0] == '/' && (path)[1] == '/')
(Or may be)
#define has_unc_path_prefix(path) (is_dir_sep((path)[0])   && 
is_dir_sep((path)[1]))
 


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-12 Thread Junio C Hamano
Jiang Xin  writes:

> 2013/9/13 Junio C Hamano :
>>
>> For systems that need POSIX escape hatch for Apollo Domain ;-), we
>> would need a bit more work.  When both path1 and path2 begin with a
>> double-dash, we would need to check if they match up to the next
>> slash, so that
>>
>>  - //host1/usr/src and //host1/usr/lib share the same root and the
>>former can be made to ../src relative to the latter;
>>
>>  - //host1/usr/src and //host2/usr/lib are of separate roots.
>>
>> or something.
>
> But how could we know which platform supports network pathnames and
> needs such implementation.

Near the end of

http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_12

is this:

If a pathname begins with two successive  characters, the
first component following the leading  characters may be
interpreted in an implementation-defined manner, although more than
two leading  characters shall be treated as a single 
character.

Two points to note are

 (1) Only paths that begin with exactly two slashes are special.

 (2) As it is "implementation-defined", we are not even allowed to
 treat that //host1/usr/src and //host1/usr/lib as sharing "the
 same root", and make the former to ../src relative to the
 latter.

So in the strictest sense, we do not have to bother. As long as we
make sure we do not molest anything that begins with exactly two
slashes.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-12 Thread Jiang Xin
2013/9/13 Junio C Hamano :
>
> For systems that need POSIX escape hatch for Apollo Domain ;-), we
> would need a bit more work.  When both path1 and path2 begin with a
> double-dash, we would need to check if they match up to the next
> slash, so that
>
>  - //host1/usr/src and //host1/usr/lib share the same root and the
>former can be made to ../src relative to the latter;
>
>  - //host1/usr/src and //host2/usr/lib are of separate roots.
>
> or something.
>
>

But how could we know which platform supports network pathnames and
needs such implementation.

-- 
Jiang Xin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-12 Thread Junio C Hamano
Johannes Sixt  writes:

> Am 12.09.2013 16:13, schrieb Torsten Bögershausen:
>> On 2013-09-12 11.12, Jiang Xin wrote:
>>> +static int have_same_root(const char *path1, const char *path2)
>>> +{
>>> +   int is_abs1, is_abs2;
>>> +
>>> +   is_abs1 = is_absolute_path(path1);
>>> +   is_abs2 = is_absolute_path(path2);
>>> +   return (is_abs1 && is_abs2 && !strncasecmp(path1, path2, 1)) ||
>>^^^
>> I wonder: should strncasecmp() be replaced with strncmp_icase() ?
>
> I don't think so: On POSIX, it is irrelevant, because the call will only
> compare a slash to a slash. On Windows, it compares the drive letters
> (or a slash); it is *always* case-insensitive, even if the volume
> mounted is NTFS with case-sensitivity enabled and core.ignorecase is false.

Ah, you are right, of course.  We could even do

tolower(path1[0]) == tolower(path2[0])

which might be more explicit.

For systems that need POSIX escape hatch for Apollo Domain ;-), we
would need a bit more work.  When both path1 and path2 begin with a
double-dash, we would need to check if they match up to the next
slash, so that

 - //host1/usr/src and //host1/usr/lib share the same root and the
   former can be made to ../src relative to the latter;

 - //host1/usr/src and //host2/usr/lib are of separate roots.

or something.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-12 Thread Johannes Sixt
Am 12.09.2013 16:13, schrieb Torsten Bögershausen:
> On 2013-09-12 11.12, Jiang Xin wrote:
>> +static int have_same_root(const char *path1, const char *path2)
>> +{
>> +int is_abs1, is_abs2;
>> +
>> +is_abs1 = is_absolute_path(path1);
>> +is_abs2 = is_absolute_path(path2);
>> +return (is_abs1 && is_abs2 && !strncasecmp(path1, path2, 1)) ||
>^^^
> I wonder: should strncasecmp() be replaced with strncmp_icase() ?

I don't think so: On POSIX, it is irrelevant, because the call will only
compare a slash to a slash. On Windows, it compares the drive letters
(or a slash); it is *always* case-insensitive, even if the volume
mounted is NTFS with case-sensitivity enabled and core.ignorecase is false.

> See dir.c: 
> int strncmp_icase(const char *a, const char *b, size_t count)
> {
>   return ignore_case ? strncasecmp(a, b, count) : strncmp(a, b, count);
> }

-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-12 Thread Junio C Hamano
Torsten Bögershausen  writes:

>> +static int have_same_root(const char *path1, const char *path2)
>> +{
>> +int is_abs1, is_abs2;
>> +
>> +is_abs1 = is_absolute_path(path1);
>> +is_abs2 = is_absolute_path(path2);
>> +return (is_abs1 && is_abs2 && !strncasecmp(path1, path2, 1)) ||
>^^^
> I wonder: should strncasecmp() be replaced with strncmp_icase() ?

True.

> See dir.c: 
> int strncmp_icase(const char *a, const char *b, size_t count)
> {
>   return ignore_case ? strncasecmp(a, b, count) : strncmp(a, b, count);
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-12 Thread Karsten Blees
Am 12.09.2013 11:12, schrieb Jiang Xin:
> Tvangeste found that the "relative_path" function could not work
> properly on Windows if "in" and "prefix" have dos driver prefix.
> ($gmane/234434)
> 
> e.g., When execute: test-path-utils relative_path "C:/a/b" "D:/x/y",
> should return "C:/a/b", but returns "../../C:/a/b", which is wrong.
> 
> So make relative_path honor dos_drive_prefix, and add test cases
> for it in t0060.
> 

I still don't think that cd'ing through the root is a Good Thing for absolute 
paths, as it is not possible to do so in general.

POSIX says the meaning of '//' is implementation-defined [1].

Cygwin supports //hostname/share/directory/file. You cannot cd to the hostname 
(i.e. root would be //hostname/share).

On Windows, we have drive letters, UNC paths and namespaces:
C:\directory\file
\\hostname\share\directory\file (same as cygwin)
\\?\C:\directory\file
\\?\UNC\hostname\share\directory\file

I'm not sure about '//' support on other git platforms (the most likely 
candidate would probably be HP-UX, as HP bought Apollo/DomainOS, which 
supported '//hostname/directory/file back in 1981).


You could of course handle all these special cases. However, with the POSIX 
definition above, the only reliable and future-proof way to check if we can cd 
out of a path component of an _absolute_ path is to actually try it until we 
get an error. And we probably don't want to do actual IO in relative_path 
(which is pure string manipulation so far).

[1] 
http://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xbd_chap04.html#tag_21_04_12


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-12 Thread Torsten Bögershausen
On 2013-09-12 11.12, Jiang Xin wrote:
> Tvangeste found that the "relative_path" function could not work
> properly on Windows if "in" and "prefix" have dos driver prefix.
> ($gmane/234434)
> 
> e.g., When execute: test-path-utils relative_path "C:/a/b" "D:/x/y",
> should return "C:/a/b", but returns "../../C:/a/b", which is wrong.
> 
> So make relative_path honor dos_drive_prefix, and add test cases
> for it in t0060.
> 
> Reported-by: Tvangeste 
> Helped-by: Johannes Sixt 
> Signed-off-by: Jiang Xin 
> ---
>  path.c| 20 
>  t/t0060-path-utils.sh |  4 
>  2 files changed, 24 insertions(+)
> 
> diff --git a/path.c b/path.c
> index 7f3324a..ffcdea1 100644
> --- a/path.c
> +++ b/path.c
> @@ -441,6 +441,16 @@ int adjust_shared_perm(const char *path)
>   return 0;
>  }
>  
> +static int have_same_root(const char *path1, const char *path2)
> +{
> + int is_abs1, is_abs2;
> +
> + is_abs1 = is_absolute_path(path1);
> + is_abs2 = is_absolute_path(path2);
> + return (is_abs1 && is_abs2 && !strncasecmp(path1, path2, 1)) ||
   ^^^
I wonder: should strncasecmp() be replaced with strncmp_icase() ?

See dir.c: 
int strncmp_icase(const char *a, const char *b, size_t count)
{
return ignore_case ? strncasecmp(a, b, count) : strncmp(a, b, count);
}
/Torsten











> +(!is_abs1 && !is_abs2);
> +}
> +
>  /*
>   * Give path as relative to prefix.
>   *
> @@ -461,6 +471,16 @@ const char *relative_path(const char *in, const char 
> *prefix,
>   else if (!prefix_len)
>   return in;
>  
> + if (have_same_root(in, prefix)) {
> + /* bypass dos_drive, for "c:" is identical to "C:" */
> + if (has_dos_drive_prefix(in)) {
> + i = 2;
> + j = 2;
> + }
> + } else {
> + return in;
> + }
> +
>   while (i < prefix_len && j < in_len && prefix[i] == in[j]) {
>   if (is_dir_sep(prefix[i])) {
>   while (is_dir_sep(prefix[i]))
> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
> index 76c7792..c3c3b33 100755
> --- a/t/t0060-path-utils.sh
> +++ b/t/t0060-path-utils.sh
> @@ -208,6 +208,10 @@ relative_path a/b/   a/b ./
>  relative_path a  a/b ../
>  relative_path x/ya/b ../../x/y
>  relative_path a/ca/b ../c
> +relative_path a/b/x/ya/b
> +relative_path /a/b   x/y /a/bPOSIX
> +relative_path d:/a/b D:/a/c  ../bMINGW
> +relative_path C:/a/b D:/a/c  C:/a/b  MINGW
>  relative_path a/b""   a/b
>  relative_path a/b""a/b
>  relative_path ""  /a/b./
> 

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-12 Thread Tvangeste
Thank you, this fixes the problem with `git svn rebase` on Windows for me.

--Tvangeste

On Thu, Sep 12, 2013 at 11:12 AM, Jiang Xin  wrote:
> Tvangeste found that the "relative_path" function could not work
> properly on Windows if "in" and "prefix" have dos driver prefix.
> ($gmane/234434)
>
> e.g., When execute: test-path-utils relative_path "C:/a/b" "D:/x/y",
> should return "C:/a/b", but returns "../../C:/a/b", which is wrong.
>
> So make relative_path honor dos_drive_prefix, and add test cases
> for it in t0060.
>
> Reported-by: Tvangeste 
> Helped-by: Johannes Sixt 
> Signed-off-by: Jiang Xin 
> ---
>  path.c| 20 
>  t/t0060-path-utils.sh |  4 
>  2 files changed, 24 insertions(+)
>
> diff --git a/path.c b/path.c
> index 7f3324a..ffcdea1 100644
> --- a/path.c
> +++ b/path.c
> @@ -441,6 +441,16 @@ int adjust_shared_perm(const char *path)
> return 0;
>  }
>
> +static int have_same_root(const char *path1, const char *path2)
> +{
> +   int is_abs1, is_abs2;
> +
> +   is_abs1 = is_absolute_path(path1);
> +   is_abs2 = is_absolute_path(path2);
> +   return (is_abs1 && is_abs2 && !strncasecmp(path1, path2, 1)) ||
> +  (!is_abs1 && !is_abs2);
> +}
> +
>  /*
>   * Give path as relative to prefix.
>   *
> @@ -461,6 +471,16 @@ const char *relative_path(const char *in, const char 
> *prefix,
> else if (!prefix_len)
> return in;
>
> +   if (have_same_root(in, prefix)) {
> +   /* bypass dos_drive, for "c:" is identical to "C:" */
> +   if (has_dos_drive_prefix(in)) {
> +   i = 2;
> +   j = 2;
> +   }
> +   } else {
> +   return in;
> +   }
> +
> while (i < prefix_len && j < in_len && prefix[i] == in[j]) {
> if (is_dir_sep(prefix[i])) {
> while (is_dir_sep(prefix[i]))
> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
> index 76c7792..c3c3b33 100755
> --- a/t/t0060-path-utils.sh
> +++ b/t/t0060-path-utils.sh
> @@ -208,6 +208,10 @@ relative_path a/b/ a/b ./
>  relative_path aa/b ../
>  relative_path x/y  a/b ../../x/y
>  relative_path a/c  a/b ../c
> +relative_path a/b  /x/ya/b
> +relative_path /a/b x/y /a/bPOSIX
> +relative_path d:/a/b   D:/a/c  ../bMINGW
> +relative_path C:/a/b   D:/a/c  C:/a/b  MINGW
>  relative_path a/b  ""   a/b
>  relative_path a/b  ""a/b
>  relative_path ""/a/b./
> --
> 1.8.3.rc2.14.g5ac1b82
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-12 Thread Jiang Xin
Tvangeste found that the "relative_path" function could not work
properly on Windows if "in" and "prefix" have dos driver prefix.
($gmane/234434)

e.g., When execute: test-path-utils relative_path "C:/a/b" "D:/x/y",
should return "C:/a/b", but returns "../../C:/a/b", which is wrong.

So make relative_path honor dos_drive_prefix, and add test cases
for it in t0060.

Reported-by: Tvangeste 
Helped-by: Johannes Sixt 
Signed-off-by: Jiang Xin 
---
 path.c| 20 
 t/t0060-path-utils.sh |  4 
 2 files changed, 24 insertions(+)

diff --git a/path.c b/path.c
index 7f3324a..ffcdea1 100644
--- a/path.c
+++ b/path.c
@@ -441,6 +441,16 @@ int adjust_shared_perm(const char *path)
return 0;
 }
 
+static int have_same_root(const char *path1, const char *path2)
+{
+   int is_abs1, is_abs2;
+
+   is_abs1 = is_absolute_path(path1);
+   is_abs2 = is_absolute_path(path2);
+   return (is_abs1 && is_abs2 && !strncasecmp(path1, path2, 1)) ||
+  (!is_abs1 && !is_abs2);
+}
+
 /*
  * Give path as relative to prefix.
  *
@@ -461,6 +471,16 @@ const char *relative_path(const char *in, const char 
*prefix,
else if (!prefix_len)
return in;
 
+   if (have_same_root(in, prefix)) {
+   /* bypass dos_drive, for "c:" is identical to "C:" */
+   if (has_dos_drive_prefix(in)) {
+   i = 2;
+   j = 2;
+   }
+   } else {
+   return in;
+   }
+
while (i < prefix_len && j < in_len && prefix[i] == in[j]) {
if (is_dir_sep(prefix[i])) {
while (is_dir_sep(prefix[i]))
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 76c7792..c3c3b33 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -208,6 +208,10 @@ relative_path a/b/ a/b ./
 relative_path aa/b ../
 relative_path x/y  a/b ../../x/y
 relative_path a/c  a/b ../c
+relative_path a/b  /x/ya/b
+relative_path /a/b x/y /a/bPOSIX
+relative_path d:/a/b   D:/a/c  ../bMINGW
+relative_path C:/a/b   D:/a/c  C:/a/b  MINGW
 relative_path a/b  ""   a/b
 relative_path a/b  ""a/b
 relative_path ""/a/b./
-- 
1.8.3.rc2.14.g5ac1b82

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html