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)
