Re: Cygwin Git with Windows paths
On Tue, Nov 20, 2018 at 4:36 AM Torsten Bögershausen wrote: > Could you please post a "git diff" of your instrumented code, > so that I/we can follow the debugging, especially what the printouts mean? > > I think we need to understand what is going on in abspath.c > diff --git a/abspath.c b/abspath.c index 9857985..09548e5 100644 --- a/abspath.c +++ b/abspath.c @@ -14,6 +14,7 @@ int is_directory(const char *path) /* removes the last path component from 'path' except if 'path' is root */ static void strip_last_component(struct strbuf *path) { + printf("strip_last_component, path, [%s]\n", path->buf); size_t offset = offset_1st_component(path->buf); size_t len = path->len; @@ -30,6 +31,8 @@ static void strip_last_component(struct strbuf *path) /* get (and remove) the next component in 'remaining' and place it in 'next' */ static void get_next_component(struct strbuf *next, struct strbuf *remaining) { + printf("get_next_component, next, [%s]\n", next->buf); + printf("get_next_component, remaining, [%s]\n", remaining->buf); char *start = NULL; char *end = NULL;
Re: Cygwin Git with Windows paths
On 20.11.18 01:17, Steven Penny wrote: > On Sun, Nov 18, 2018 at 11:21 PM Torsten Bögershausen wrote: >> If nothing works, >> it may help to add some fprintf(stderr,...) in the functions used >> by 05b458c104708141d2f: >> >> strip_last_component(), >> get_next_component() >> real_path_internal() > > I didnt see any "real_path_internal" in the current codebase - however i added > some "printf" to the other 2 and got this: > > $ git clone git://github.com/benhoyt/goawk 'C:\cygwin64\tmp\goawk' > get_next_component, next, [] > get_next_component, remaining, [C:\cygwin64\tmp\goawk] > Cloning into 'C:\cygwin64\tmp\goawk'... > get_next_component, next, [] > get_next_component, remaining, [C:\cygwin64\tmp\goawk/.git] > fatal: Invalid path '/usr/local/cache/git/C:\cygwin64\tmp\goawk': No such file > or directory > Could you please post a "git diff" of your instrumented code, so that I/we can follow the debugging, especially what the printouts mean? I think we need to understand what is going on in abspath.c
Re: Cygwin Git with Windows paths
On Sun, Nov 18, 2018 at 11:21 PM Torsten Bögershausen wrote: > If nothing works, > it may help to add some fprintf(stderr,...) in the functions used > by 05b458c104708141d2f: > > strip_last_component(), > get_next_component() > real_path_internal() I didnt see any "real_path_internal" in the current codebase - however i added some "printf" to the other 2 and got this: $ git clone git://github.com/benhoyt/goawk 'C:\cygwin64\tmp\goawk' get_next_component, next, [] get_next_component, remaining, [C:\cygwin64\tmp\goawk] Cloning into 'C:\cygwin64\tmp\goawk'... get_next_component, next, [] get_next_component, remaining, [C:\cygwin64\tmp\goawk/.git] fatal: Invalid path '/usr/local/cache/git/C:\cygwin64\tmp\goawk': No such file or directory
Re: Cygwin Git with Windows paths
On 2018-11-19 04:33, Junio C Hamano wrote: > "Randall S. Becker" writes: > >>> Torsten Bögershausen writes: >>> And it may even be that we need a special handling for the "\" to be treated as "/". >>> >>> I do not do Windows, but is_dir_sep() needs to be tweaked if you want to do >>> that. >> >> Heavy Cygwin user here. It is used in my environment for >> cross-compilation. Everything should be done using / separators in >> Cygwin, not \. So /cygdrive/c, /cygdrive/d always prefaces the >> path rather than C:\ or D:\, which won't parse. It is, >> essentially, a bash environment, including that git completions >> work properly. Backslash ends up doing what it would in bash. > > In short, in your opinion, the original message in this thread > expresses an invalid wish, as C:\path\to\dir\ is not a valid way to > spell the path to the directory, and it should be written as > /cygdrive/c/path/to/dir instead? > > How well does this argument work in the real world, when another > claim in the original message > > This causes problems for any non-Cygwin tools that might call Git: > > http://github.com/golang/go/issues/23155 > > is taken into account, I wonder, though? > Back to the original email, where the path embedded in '' and the bash does not interpret the "\", I think. > $ git clone git://github.com/benhoyt/goawk 'C:\cygwin64\tmp\goawk' > Cloning into 'C:\cygwin64\tmp\goawk'... > fatal: Invalid path '/home/Steven/C:\cygwin64\tmp\goawk': No such file or > directory >It seems the problem is that Git thinks the Windows form path is relative >because it does not start with "/". >A Git Bisect reveals this: >05b458c104708141d2fad211d79703b3b99cc5a8 is the first bad commit >commit 05b458c104708141d2fad211d79703b3b99cc5a8 >Author: Brandon Williams >Date: Mon Dec 12 10:16:52 2016 -0800 The first question is, does this work under Git for Windows ? Looking into 05b458c104708141d2fad, it seems as if the following functions need to be "overridden" for cygwin, similar as we do it for mingw: is_dir_sep() offset_1st_component() find_last_dir_sep() If nothing works, it may help to add some fprintf(stderr,...) in the functions used by 05b458c104708141d2f: strip_last_component(), get_next_component() real_path_internal()
Re: Cygwin Git with Windows paths
"Randall S. Becker" writes: >> Torsten Bögershausen writes: >> >> > And it may even be that we need a special handling for the "\" to be >> > treated as "/". >> >> I do not do Windows, but is_dir_sep() needs to be tweaked if you want to do >> that. > > Heavy Cygwin user here. It is used in my environment for > cross-compilation. Everything should be done using / separators in > Cygwin, not \. So /cygdrive/c, /cygdrive/d always prefaces the > path rather than C:\ or D:\, which won't parse. It is, > essentially, a bash environment, including that git completions > work properly. Backslash ends up doing what it would in bash. In short, in your opinion, the original message in this thread expresses an invalid wish, as C:\path\to\dir\ is not a valid way to spell the path to the directory, and it should be written as /cygdrive/c/path/to/dir instead? How well does this argument work in the real world, when another claim in the original message This causes problems for any non-Cygwin tools that might call Git: http://github.com/golang/go/issues/23155 is taken into account, I wonder, though?
RE: Cygwin Git with Windows paths
> -Original Message- > From: git-ow...@vger.kernel.org On Behalf Of > Junio C Hamano > Sent: November 18, 2018 19:07 > To: Torsten Bögershausen > Cc: Steven Penny ; git@vger.kernel.org > Subject: Re: Cygwin Git with Windows paths > > Torsten Bögershausen writes: > > > And it may even be that we need a special handling for the "\" to be > > treated as "/". > > I do not do Windows, but is_dir_sep() needs to be tweaked if you want to do > that. Heavy Cygwin user here. It is used in my environment for cross-compilation. Everything should be done using / separators in Cygwin, not \. So /cygdrive/c, /cygdrive/d always prefaces the path rather than C:\ or D:\, which won't parse. It is, essentially, a bash environment, including that git completions work properly. Backslash ends up doing what it would in bash.
Re: Cygwin Git with Windows paths
Torsten Bögershausen writes: > And it may even be that we need a special handling for the "\" to be treated > as "/". I do not do Windows, but is_dir_sep() needs to be tweaked if you want to do that.
Re: Cygwin Git with Windows paths
On Sun, Nov 18, 2018 at 12:28 PM Torsten Bögershausen wrote: > Thanks for testing. > It looks as if there is more work to be done then just a simple patch. > > My last question for today: > Does > > git clone '/cgdrive/c/my/dir' > > work ? yes - these all work and resolve to same path: git clone /tmp/goawk git clone /cygdrive/c/cygwin64/tmp/goawk git clone /proc/cygdrive/c/cygwin64/tmp/goawk however i would caution that any fix should not rely on "C:", as users are allowed to install to other volumes such as "D:". Perhaps a better solution would be for Git to just take the path as is, rather than converting it to an absolute path?
Re: Cygwin Git with Windows paths
On Sun, Nov 18, 2018 at 11:34:04AM -0600, Steven Penny wrote: > On Sun, Nov 18, 2018 at 11:15 AM Torsten Bögershausen wrote: > > But it may be that we need to pull in more stuff, similar to mingw, > > to get the C: stuff working, see > > "skip_dos_drive_prefix" > > > > And it may even be that we need a special handling for the "\" to be treated > > as "/". > > > > If you implement "skip_dos_drive_prefix" similar to mingw, > > (rename mingw into cygwin) does > > > > git clone C:/my/dir/ > > work ? > > I added this to "compat/cygwin.h": > > #define has_dos_drive_prefix(path) \ > (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0) > int mingw_skip_dos_drive_prefix(char **path); > #define skip_dos_drive_prefix mingw_skip_dos_drive_prefix > > and added this to "compat/cygwin.c": > > int mingw_skip_dos_drive_prefix(char **path) { > int ret = has_dos_drive_prefix(*path); > *path += ret; > return ret; > } > > but still, these dont work: > > git clone C:/my/dir > git clone 'C:\my\dir' Thanks for testing. It looks as if there is more work to be done then just a simple patch. My last question for today: Does git clone '/cgdrive/c/my/dir' work ?
Re: Cygwin Git with Windows paths
On Sun, Nov 18, 2018 at 11:15 AM Torsten Bögershausen wrote: > But it may be that we need to pull in more stuff, similar to mingw, > to get the C: stuff working, see > "skip_dos_drive_prefix" > > And it may even be that we need a special handling for the "\" to be treated > as "/". > > If you implement "skip_dos_drive_prefix" similar to mingw, > (rename mingw into cygwin) does > > git clone C:/my/dir/ > work ? I added this to "compat/cygwin.h": #define has_dos_drive_prefix(path) \ (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0) int mingw_skip_dos_drive_prefix(char **path); #define skip_dos_drive_prefix mingw_skip_dos_drive_prefix and added this to "compat/cygwin.c": int mingw_skip_dos_drive_prefix(char **path) { int ret = has_dos_drive_prefix(*path); *path += ret; return ret; } but still, these dont work: git clone C:/my/dir git clone 'C:\my\dir'
Re: Cygwin Git with Windows paths
On Sun, Nov 18, 2018 at 10:23:19AM -0600, Steven Penny wrote: > On Sun, Nov 18, 2018 at 9:41 AM Torsten Bögershausen wrote: > > Thanks for the report > > It seams as if "C:" is not recognized as an absolute path under > > cygwin. > > May be it should ? > > > > Does the following help ? (fully untested) > > that looks promising - but its not getting pulled in where it needs to be. > perhaps another file need to be modified to utilize that macro? The macro should be utilized, see git-compat-util.h: #if defined(__CYGWIN__) #include "compat/cygwin.h" #endif And further down #ifndef has_dos_drive_prefix static inline int git_has_dos_drive_prefix(const char *path) { return 0; } #define has_dos_drive_prefix git_has_dos_drive_prefix #endif #ifndef skip_dos_drive_prefix static inline int git_skip_dos_drive_prefix(char **path) { return 0; } But it may be that we need to pull in more stuff, similar to mingw, to get the C: stuff working, see "skip_dos_drive_prefix" And it may even be that we need a special handling for the "\" to be treated as "/". If you implement "skip_dos_drive_prefix" similar to mingw, (rename mingw into cygwin) does git clone C:/my/dir/ work ? That would be a progress, I think.
Re: Cygwin Git with Windows paths
On Sun, Nov 18, 2018 at 9:41 AM Torsten Bögershausen wrote: > Thanks for the report > It seams as if "C:" is not recognized as an absolute path under > cygwin. > May be it should ? > > Does the following help ? (fully untested) that looks promising - but its not getting pulled in where it needs to be. perhaps another file need to be modified to utilize that macro?
Re: Cygwin Git with Windows paths
On Sun, Nov 18, 2018 at 07:21:58AM -0800, Steven Penny wrote: > Cygwin programs can handle Unix form paths: > >$ ls /var >cache lib log run tmp > > and also Windows form paths: > >$ ls 'C:\cygwin64\var' >cache lib log run tmp > > However current Cygwin Git cannot: > >$ git clone git://github.com/benhoyt/goawk 'C:\cygwin64\tmp\goawk' >Cloning into 'C:\cygwin64\tmp\goawk'... >fatal: Invalid path '/home/Steven/C:\cygwin64\tmp\goawk': No such file or >directory > > It seems the problem is that Git thinks the Windows form path is relative > because it does not start with "/". A Git Bisect reveals this: > > 05b458c104708141d2fad211d79703b3b99cc5a8 is the first bad commit > commit 05b458c104708141d2fad211d79703b3b99cc5a8 > Author: Brandon Williams > Date: Mon Dec 12 10:16:52 2016 -0800 > >real_path: resolve symlinks by hand > >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 and not just an individual thread. Instead perform >the symlink resolution by hand so that the calls to chdir() can be >removed, making real_path one step closer to being reentrant. > >Signed-off-by: Brandon Williams >Signed-off-by: Junio C Hamano > > This causes problems for any non-Cygwin tools that might call Git: > > http://github.com/golang/go/issues/23155 > Thanks for the report It seams as if "C:" is not recognized as an absolute path under cygwin. May be it should ? Does the following help ? (fully untested) diff --git a/compat/cygwin.h b/compat/cygwin.h index 8e52de4644..12814e1edb 100644 --- a/compat/cygwin.h +++ b/compat/cygwin.h @@ -1,2 +1,4 @@ int cygwin_offset_1st_component(const char *path); #define offset_1st_component cygwin_offset_1st_component +#define has_dos_drive_prefix(path) \ + (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
Cygwin Git with Windows paths
Cygwin programs can handle Unix form paths: $ ls /var cache lib log run tmp and also Windows form paths: $ ls 'C:\cygwin64\var' cache lib log run tmp However current Cygwin Git cannot: $ git clone git://github.com/benhoyt/goawk 'C:\cygwin64\tmp\goawk' Cloning into 'C:\cygwin64\tmp\goawk'... fatal: Invalid path '/home/Steven/C:\cygwin64\tmp\goawk': No such file or directory It seems the problem is that Git thinks the Windows form path is relative because it does not start with "/". A Git Bisect reveals this: 05b458c104708141d2fad211d79703b3b99cc5a8 is the first bad commit commit 05b458c104708141d2fad211d79703b3b99cc5a8 Author: Brandon Williams Date: Mon Dec 12 10:16:52 2016 -0800 real_path: resolve symlinks by hand 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 and not just an individual thread. Instead perform the symlink resolution by hand so that the calls to chdir() can be removed, making real_path one step closer to being reentrant. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano This causes problems for any non-Cygwin tools that might call Git: http://github.com/golang/go/issues/23155