Re: [PATCH v3 1/1] git clone C:\cygwin\home\USER\repo' is working (again)

2018-12-08 Thread Steven Penny
On Sat, Dec 8, 2018 at 9:11 AM wrote:
> Changes since V2:

latest patch still fixes original issue - thanks

> - Settled on a better name:
>   The common code is in compat/win32/path-utils.c/h
>   [...]
> - The "DOS" moniker is still used for 2 reasons:
>   Windows inherited the "drive letter" concept from DOS,
>   and everybody (tm) familar with the code and the path handling
>   in Git is used to that wording.
>   Even if there was a better name, it needed to be addressed
>   in a patch series different from this one.
>   Here I want to fix a reported regression.

i still disagree with this - but i understand if naming argument is out of scope
for thread

> And, before any cleanup is done, I sould like to ask if anybody
> can build the code with VS and confirm that it works, please ?

sorry but i am decidedly *not* interested in doing this. i use cygwin
specifically so that i can avoid VS. hopefully someone else will be able to
test. cheers


[PATCH v3 1/1] git clone C:\cygwin\home\USER\repo' is working (again)

2018-12-08 Thread tboegi
From: Torsten Bögershausen 

A regression for cygwin users was introduced with commit 05b458c,
 "real_path: resolve symlinks by hand".

In the the commit message we read:
  The current implementation of real_path uses chdir() in order to resolve
symlinks.  Unfortunately this isn't thread-safe as chdir() affects a
  process as a whole...

The old (and non-thread-save) OS calls chdir()/pwd() had been
replaced by a string operation.
The cygwin layer "knows" that "C:\cygwin" is an absolute path,
but the new string operation does not.

"git clone  C:\cygwin\home\USER\repo" fails like this:
fatal: Invalid path '/home/USER/repo/C:\cygwin\home\USER\repo'

The solution is to implement has_dos_drive_prefix(), skip_dos_drive_prefix()
is_dir_sep(), offset_1st_component() and convert_slashes() for cygwin
in the same way as it is done in 'Git for Windows' in compat/mingw.[ch]

Extract the needed code into compat/win32/path-utils.[ch] and use it
for cygwin as well.

Reported-by: Steven Penny 
Helped-by: Johannes Schindelin 
Signed-off-by: Torsten Bögershausen 
---
Changes since V2:
- Settled on a better name:
  The common code is in compat/win32/path-utils.c/h
- Skip the 2 patches which "only" do a cleanup (for a moment)
  put those cleanups onto the "todo stack".
- The "DOS" moniker is still used for 2 reasons:
  Windows inherited the "drive letter" concept from DOS,
  and everybody (tm) familar with the code and the path handling
  in Git is used to that wording.
  Even if there was a better name, it needed to be addressed
  in a patch series different from this one.
  Here I want to fix a reported regression.
   
And, before any cleanup is done, I sould like to ask if anybody
can build the code with VS and confirm that it works, please ?

Thanks for the reviews, testing and comment.

compat/cygwin.c   | 19 ---
 compat/cygwin.h   |  2 --
 compat/mingw.c| 29 +
 compat/mingw.h| 20 
 compat/win32/path-utils.c | 28 
 compat/win32/path-utils.h | 20 
 config.mak.uname  |  3 ++-
 git-compat-util.h |  3 ++-
 8 files changed, 53 insertions(+), 71 deletions(-)
 delete mode 100644 compat/cygwin.c
 delete mode 100644 compat/cygwin.h
 create mode 100644 compat/win32/path-utils.c
 create mode 100644 compat/win32/path-utils.h

diff --git a/compat/cygwin.c b/compat/cygwin.c
deleted file mode 100644
index b9862d606d..00
--- a/compat/cygwin.c
+++ /dev/null
@@ -1,19 +0,0 @@
-#include "../git-compat-util.h"
-#include "../cache.h"
-
-int cygwin_offset_1st_component(const char *path)
-{
-   const char *pos = path;
-   /* unc paths */
-   if (is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
-   /* skip server name */
-   pos = strchr(pos + 2, '/');
-   if (!pos)
-   return 0; /* Error: malformed unc path */
-
-   do {
-   pos++;
-   } while (*pos && pos[0] != '/');
-   }
-   return pos + is_dir_sep(*pos) - path;
-}
diff --git a/compat/cygwin.h b/compat/cygwin.h
deleted file mode 100644
index 8e52de4644..00
--- a/compat/cygwin.h
+++ /dev/null
@@ -1,2 +0,0 @@
-int cygwin_offset_1st_component(const char *path);
-#define offset_1st_component cygwin_offset_1st_component
diff --git a/compat/mingw.c b/compat/mingw.c
index 34b3880b29..27e397f268 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -350,7 +350,7 @@ static inline int needs_hiding(const char *path)
return 0;
 
/* We cannot use basename(), as it would remove trailing slashes */
-   mingw_skip_dos_drive_prefix((char **));
+   win_path_utils_skip_dos_drive_prefix((char **));
if (!*path)
return 0;
 
@@ -2275,33 +2275,6 @@ pid_t waitpid(pid_t pid, int *status, int options)
return -1;
 }
 
-int mingw_skip_dos_drive_prefix(char **path)
-{
-   int ret = has_dos_drive_prefix(*path);
-   *path += ret;
-   return ret;
-}
-
-int mingw_offset_1st_component(const char *path)
-{
-   char *pos = (char *)path;
-
-   /* unc paths */
-   if (!skip_dos_drive_prefix() &&
-   is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
-   /* skip server name */
-   pos = strpbrk(pos + 2, "\\/");
-   if (!pos)
-   return 0; /* Error: malformed unc path */
-
-   do {
-   pos++;
-   } while (*pos && !is_dir_sep(*pos));
-   }
-
-   return pos + is_dir_sep(*pos) - path;
-}
-
 int xutftowcsn(wchar_t *wcs, const char *utfs, size_t wcslen, int utflen)
 {
int upos = 0, wpos = 0;
diff --git a/compat/mingw.h b/compat/mingw.h
index 8c24ddaa3e..30d9fb3e36 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -443,32 +443,12 @@ HANDLE winansi_get_osfhandle(int fd);
  * git specific compatibility
  */
 
-#define