.git subdirectory is created in the wrong place on checkout when using relative directory

[QbProg]

Testing git for windows 2.4.3 2nd release candidate. I can reproduce it using Windows 8.1 64bit (on 2 pcs) , but not using Windows 7 64bit. Reproducing it only when calling from command prompt (not bash), and also using the eclipse debugger.

When calling a clone command like this

C:> git clone http://..../path local_dir or also C:> git clone C:\dir local_dir

the .git directory is created inside local_dir\local_dir.git instead of local_dir.git

this happens only if local_dir is passed as a relative path AND if the current directory is the root of a drive. Using an absolute path it works:

C:>git clone C:\dir C:\local_dir works AND it works if the current directory is different from root C:\test>git clone C:\dir local_dir

since it was not easily reproducible I set-up the sdk and debugged the exe with eclipse. On a first glipse it seems that the current directory is not correctly restored in one of the intermediate clone steps. I hope to find the exact failure during next week.

probably chdir doesn't works correctly if you try to set back the directory to root ? abspath.c:145 <--- maybe here?

[QbProg]

I also tested it better, it's reproducible in Windows 7 with the same criterias

[QbProg]

I spotted the cause: mingw_chdir("S:/") fails in mingw.c this is because normalize_ntpath removes the trailing slash in mingw.c:341 , and if you try to run _wchdir("S:") that function fails (it probably just changes the current drive) , you have to use _wchdir("S:\") to switch to the root directory.

I'm not sure how to do a patch without messing-up everything, anyway in 1.9.5 it worked correctly.

[dscho]

@QbProg could you install the Git SDK -- unless you have it already -- and try out this patch?

diff --git a/compat/mingw.c b/compat/mingw.c
index 8e1993f..d1baaeb 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -316,7 +316,7 @@ static void process_phantom_symlinks(void)
 }

 /* Normalizes NT paths as returned by some low-level APIs. */
-static wchar_t *normalize_ntpath(wchar_t *wbuf)
+static wchar_t *normalize_ntpath(wchar_t *wbuf, int keep_trailing_slashes)
 {
    int i;
    /* fix absolute path prefixes */
@@ -338,8 +338,9 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)
        if (wbuf[i] == '\\')
            wbuf[i] = '/';
    /* remove potential trailing slashes */
-   while (i && wbuf[i - 1] == '/')
-       wbuf[--i] = 0;
+   if (!keep_trailing_slashes)
+       while (i && wbuf[i - 1] == '/')
+           wbuf[--i] = 0;
    return wbuf;
 }

@@ -621,7 +622,7 @@ int mingw_chdir(const char *dirname)
        CloseHandle(hnd);
    }

-   result = _wchdir(normalize_ntpath(wdirname));
+   result = _wchdir(normalize_ntpath(wdirname, 0));
    current_directory_len = GetCurrentDirectoryW(0, NULL);
    return result;
 }
@@ -2290,7 +2291,7 @@ int readlink(const char *path, char *buf, size_t bufsiz)
     * so convert to a (hopefully large enough) temporary buffer, then memcpy
     * the requested number of bytes (including '\0' for robustness).
     */
-   if ((len = xwcstoutf(tmpbuf, normalize_ntpath(wbuf), MAX_LONG_PATH)) < 0)
+   if ((len = xwcstoutf(tmpbuf, normalize_ntpath(wbuf, 1), MAX_LONG_PATH)) < 0)
        return -1;
    memcpy(buf, tmpbuf, min(bufsiz, len + 1));
    return min(bufsiz, len);

[QbProg]

it seems not working. IHMO it is the _wchdir(normalize_ntpath(wdirname,0)) which should have 1 for keep_trailing_slashes since it's in that point that the function fails

I don't know if it's correct but you could automatically avoid the trailing slash deletion only if the path is a root. You could check this using Win32 "PathIsRoot" (requires shlwapi) or else just check if (strlen == 3 && str[1] == L':' && (str[2] == L'\' || str[2] == L'/')) (given that you have a correct absolute path and you already stripped special path indicators)

[QbProg]

This patch should solve the issue , but I didn't run the tests

diff --git a/compat/mingw.c b/compat/mingw.c
index 1d10707..706b527 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -333,13 +333,18 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)
            *wbuf = '\\';
        }
    }
+
+   int is_root = (wcslen(wbuf) == 3) && (wbuf[1] == L':') && (wbuf[2] == L'\\' || wbuf[2] == '/');
+
    /* convert backslashes to slashes */
    for (i = 0; wbuf[i]; i++)
        if (wbuf[i] == '\\')
            wbuf[i] = '/';
    /* remove potential trailing slashes */
-   while (i && wbuf[i - 1] == '/')
-       wbuf[--i] = 0;
+   if(!is_root)
+       while (i && wbuf[i - 1] == '/')
+           wbuf[--i] = 0;
+
    return wbuf;
 }

[dscho]

I actually would really prefer to have a solution that does not depend on testing for the root directory. Are you sure that _wchdir(L"C:/Windows/") fails?

[QbProg]

_wchdir("C:") fails, while _wchdir("C:\") succeed. This is windows related I think.

[dscho]

What about _wchdir(L"C:/");? And _wchdir(L"C:/Windows/");?

[QbProg]

they work. Also _wchdir(L"C:/Windows") works because is not root

the problem is exactly that normalize_ntpath removes the trailing slash.

[dscho]

@QbProg yep, so I just mixed up my 0s and 1s, right? Could you please test my patch again (which would maybe also handle cases such as _wchdir(L"//server/"), at least that is as general as I wanted the patch to be), with the 0 and the 1 properly adjusted, as per your analysis?

[QbProg]

@dscho the patch with the 0 and 1 switched works! I was not able to test a network path since I don't have a network here :) Thank you!

[dscho]

Thanks for testing! As to the network share, I will test that once I am done releasing 2.4.4.1.

[dscho]

@QbProg I am busy working toward a 3rd release candidate of Git for Windows 2.x which will include this fix.

[dscho]

@QbProg could you please test the new Git for Windows 2.4.4.2?

[QbProg]

tested rc3 (git version 2.4.4.windows.2), it's fixed! thank you!

[dscho]

Excellent!