Re: Regression in e02ca72: git svn rebase is broken on Windows

From: Jiang Xin <worldhello.net@gmail.com>
Date: Sep 11, 2013, 03:19 UTC
Message-ID: <CANYiYbFaNP=UUKX-5N7qQppMRehuPgn8p1uN_UOJ6bDwTjzKEQ@mail.gmail.com>
In-Reply-To: <xmqq38pczjw6.fsf@gitster.dls.corp.google.com>

2013/9/11 Junio C Hamano <gitster@pobox.com>:

Show 37 quoted lines

> Tvangeste <i.4m.l33t@yandex.ru> writes:
>
>> Hi,
>>
>> After bisecting this problem I ended up with the mentioned commit that completely breaks git-svn for me on Windows (mingw/msys version).
>>
>> ==========
>> #> git svn rebase
>> warning: unable to access '': Invalid argument
>> warning: unable to access '': Invalid argument
>> fatal: unable to access '../../../../w:/work/my/repo.git/.git/config': Invalid argument
>> fatal: index file open failed: Invalid argument
>> Cannot rebase: You have unstaged changes.
>> Please commit or stash them.
>> rebase refs/remotes/trunk: command returned error: 1
>> ==========
>>
>> Please note that I use the official git repository as-is, this one (no additional patches):
>> git://git.kernel.org/pub/scm/git/git.git
>>
>> e02ca72f70ed8f0268a81f72cb3230c72e538e77 is the first bad commit
>> commit e02ca72f70ed8f0268a81f72cb3230c72e538e77
>> Author: Jiang Xin
>> Date:   Tue Jun 25 23:53:43 2013 +0800
>>
>>     path.c: refactor relative_path(), not only strip prefix
>>
>> Thanks,
>>   --Tvangeste
>
> The suspect commit and symptom look consistent.  You started from a
> directory whose absolute path is "w:/work/..." and the updated code
> mistakenly thoguht that something that begins with "w" (not '/') is
> not an absolute, so added a series of ../ to make it relative, or
> something silly like that.
>
> Jiang?

I tested 'relative_path' function using 'test-path-utils', and got the following result:

    $ ./test-path-utils relative_path 'C:/a/b' 'D:/x/y'
    ../../../C:/a/b

    $ ./test-path-utils relative_path '/a/b' 'x/y'
    ../..//a/b

    $ ./test-path-utils relative_path 'a/b' '/x/y'
    ../../../a/b

For the first case, in and prefix are on different ROOT, and for the other two cases, one path is a relative path, and another is an absolute path.

I write a patch to test whether two paths (in and prefix) have the same root. The result after applied the patch:

    $ ./test-path-utils relative_path 'C:/a/b' 'C:/x/y'
    ../../a/b

    $ ./test-path-utils relative_path 'C:/a/b' 'D:/x/y'
    C:/a/b

    $ ./test-path-utils relative_path '/a/b' 'x/y'
    /a/b

    $ ./test-path-utils relative_path 'a/b' '/x/y'
    a/b

diff --git a/path.c b/path.c
index 7f3324a..51f5d28 100644
--- a/path.c
+++ b/path.c
@@ -441,6 +441,25 @@ int adjust_shared_perm(const char *path)
        return 0;
 }

+static int have_same_root(const char *path1, const char *path2)
+{
+       /* for POSIX:
+
+          return ((path1 && is_dir_sep(*path1)) ^
+                  (path2 && is_dir_sep(*path2))) == 0;
+       */
+       return path1 && path2 && *path1 && *path2 && (
+               (is_dir_sep(*path1) &&
+                is_dir_sep(*path2)) ||
+               (*(path1+1) == ':' &&
+                *(path2+1) == ':' &&
+                !strncasecmp(path1, path2, 1)) ||
+               (!is_dir_sep(*path1) &&
+                !is_dir_sep(*path2) &&
+                *(path1+1) != ':' &&
+                *(path2+1) != ':'));
+}
+
 /*
  * Give path as relative to prefix.
  *
@@ -461,6 +480,9 @@ const char *relative_path(const char *in, const
char *prefix,
        else if (!prefix_len)
                return in;

+       if (!have_same_root(in, prefix))
+               return in;
+

Should I write the function have_same_root as inline function or macro
like 'is_dir_sep'?

-- 
Jiang Xin