[v1/RFC,1/1] 'git clone <url> C:\cygwin\home\USER\repo' is working (again)

Commit Message

Torsten Bögershausen Nov. 26, 2018, 5:32 p.m. UTC

From: Torsten Bögershausen <[e-mail]>

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 <url> 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]

Reported-By: Steven Penny <[e-mail]>
Signed-off-by: Torsten Bögershausen <[e-mail]>
---

This is the first vesion of a patch.
Is there a chance that you test it ?

abspath.c       |  2 +-
 compat/cygwin.c | 18 ++++++++++++++----
 compat/cygwin.h | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 5 deletions(-)

Comments

Steven Penny Nov. 27, 2018, 12:35 a.m. UTC | #1

On Mon, Nov 26, 2018 at 11:32 AM wrote:
> This is the first vesion of a patch.
> Is there a chance that you test it ?

I can confirm that this fixes the issue.

Thank you!

Junio C Hamano Nov. 27, 2018, 1:16 a.m. UTC | #2

[e-mail] writes:

> Reported-By: Steven Penny <[e-mail]>
> Signed-off-by: Torsten Bögershausen <[e-mail]>
> ---
>
> This is the first vesion of a patch.
> Is there a chance that you test it ?
>
> abspath.c       |  2 +-
>  compat/cygwin.c | 18 ++++++++++++++----
>  compat/cygwin.h | 32 ++++++++++++++++++++++++++++++++
>  3 files changed, 47 insertions(+), 5 deletions(-)

I am hoping that the funny indentation above is merely an accidental
touch on the delete key and not a sign of MUA eating the patch to
make it unapplicable (and making it harder for those who want to
test to test it).

> diff --git a/compat/cygwin.c b/compat/cygwin.c
> index b9862d606d..c4a10cb5a1 100644
> --- a/compat/cygwin.c
> +++ b/compat/cygwin.c
> @@ -1,19 +1,29 @@
>  #include "../git-compat-util.h"
>  #include "../cache.h"
>  
> +int cygwin_skip_dos_drive_prefix(char **path)
> +{
> +	int ret = has_dos_drive_prefix(*path);
> +	*path += ret;
> +	return ret;
> +}

Mental note: this is exactly the same as mingw version.

I wonder if it makes the rest of the code simpler if we stripped
things like /cygdrive/c here exactly the sam way as we strip C:
For that, has_dos_drive_prefix() needs to know /cygdrive/[a-z],
which may not be a bad thing, I guess.  Let's read on.

>  int cygwin_offset_1st_component(const char *path)
>  {
> -	const char *pos = path;
> +	char *pos = (char *)path;
> +
>  	/* unc paths */
> -	if (is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
> +	if (!skip_dos_drive_prefix(&pos) &&
> +			is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {

When given C:\foo\bar, this strips prefix to leave \foo\bar in pos and
then realizes that it cannot be unc path (because it has dos prefix)
and goes on.  What is returned from the function is "\foo\bar" + 1 -
path, i.e. the offset in the original "C:\foo\bar" string of the
'f' in "foo", i.e. 3.

When given \foo\bar, pos stays the same as path, and it skips the
first backslash and returns the offset in the original string of the
'f' in "foo", i.e. 1.

Both cases return the moreal equivalent --- the offset of the first
component 'foo'.  So this looks correct for these two cases.

>  		/* skip server name */
> -		pos = strchr(pos + 2, '/');
> +		pos = strpbrk(pos + 2, "\\/");

This is to allow \\server\path in addition to //server/path; the
original looked only for '/' with strchr but we now look for either
'/' or '\', whichever comes earlier.  Both helpers return NULL when
they find no separator, so we should be able to handle the returned
pos from here on the same way as the original code.

>  		if (!pos)
>  			return 0; /* Error: malformed unc path */
>  
>  		do {
>  			pos++;
> -		} while (*pos && pos[0] != '/');
> +		} while (*pos && !is_dir_sep(*pos));

And whenever we looked for '/', we consider '\' its equivalent.

>  	}
> +
>  	return pos + is_dir_sep(*pos) - path;
>  }

Looks good so far.

Wait, did I just waste time by not looking at mingw.c version?  I
suspect this would be exactly the same ;-)

> diff --git a/compat/cygwin.h b/compat/cygwin.h
> index 8e52de4644..46f29c0a90 100644
> --- a/compat/cygwin.h
> +++ b/compat/cygwin.h
> @@ -1,2 +1,34 @@
> +#define has_dos_drive_prefix(path) \
> +	(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)

Metanl note: this also looks the same as mingw version.

> +int cygwin_offset_1st_component(const char *path);
> +#define offset_1st_component cygwin_offset_1st_component
> +

So, my real questions are

 - Is there a point in having cygwin specific variant of these, or
   can we just borrow from mingw version (with some refactoring)?
   Is there a point in doing so (e.g. if mingw plans to move to
   reject forward slashes, attempting to share is pointless).

 - Would it make it better (or worse) to treat the /cygdrive/c thing
   as another way to spell dos-drive-prefix?  If the answer is "it
   is a good idea", then that answers the previous question
   automatically (we cannot gain much by sharing, as mingw side
   won't want to treat /cygdrive/c any differently).

Steven Penny Nov. 27, 2018, 2:49 a.m. UTC | #3

On Mon, Nov 26, 2018 at 7:16 PM Junio C Hamano wrote:
> I wonder if it makes the rest of the code simpler if we stripped
> things like /cygdrive/c here exactly the sam way as we strip C:
> For that, has_dos_drive_prefix() needs to know /cygdrive/[a-z],
> which may not be a bad thing, I guess.  Let's read on.

With full paths, Cygwin can traverse drives:

    $ cd 'C:\Users'
    $ pwd
    /cygdrive/c/Users

    $ cd 'D:\Testing'
    $ pwd
    /cygdrive/d/Testing

If you strip the drive, you can still navigate within the same drive:

    $ cd 'C:\Users'
    $ pwd
    /cygdrive/c/Users

    $ cd '\Windows'
    $ pwd
    /cygdrive/c/Windows

but you can no longer traverse drives:

    $ cd '\Testing'
    sh: cd: \Testing: No such file or directory

So a good first question for me would be: why are we stripping "C:" or similar
in the first place?

>  - Is there a point in having cygwin specific variant of these, or
>    can we just borrow from mingw version (with some refactoring)?
>    Is there a point in doing so (e.g. if mingw plans to move to
>    reject forward slashes, attempting to share is pointless).

I would say these could be merged into a "win.h" or similar. Cygwin typically
leans toward the "/unix/style" while MINGW has been more tolerant of
"C:\Windows\Style" and "C:/Mixed/Style" paths, i dont see that changing.

Junio C Hamano Nov. 27, 2018, 5:23 a.m. UTC | #4

Steven Penny <[e-mail]> writes:

> If you strip the drive, you can still navigate within the same drive:
>
>     $ cd 'C:\Users'
>     $ pwd
>     /cygdrive/c/Users
>
>     $ cd '\Windows'
>     $ pwd
>     /cygdrive/c/Windows
>
> but you can no longer traverse drives:
>
>     $ cd '\Testing'
>     sh: cd: \Testing: No such file or directory

Sorry, but I fail to see the point the last example wants to make.
If it were

    $ cd /cygdrive/d/Testing
    $ cd /cygdrive/c/Users
    $ cd ../../d/Testing

and the last step fails, then I would suspect it would make sense to
treat /cygdrive/$X exactly like how we would treat $C:, because

    $ cd C:Users
    $ cd ../D:Testing

would not make sense, either, which is an indication that these two
are quite similar.  On the other hand, if "cd ../../d/Testing" above
does not fail and does what non-DOS users would expect, then that
strongly argues that treating /cygdrive/$X any specially is a mistake.

> So a good first question for me would be: why are we stripping "C:" or similar
> in the first place?

Sorry, but I do not see the connection to this question and the
above example.  The reason why we strip C: is because the letter
that comes after that colon determines if we are talking about
absolute path (in other words, the current directory does not play a
role in determining which directory the path refers to), unlike the
POSIX codepath where it is always the first letter in the pathname.

C:\Users is a directory whose name is Users at the top level of the
C: drive. C0\Users tells us that in the current directory, there is
a directory whose name is C0 and in it, there is a filesystem entity
whose name is Users.  So the colon that follows an alpha (in this
case, C:) is quite special, compared to other letters (in this
example, I used '0' to contrast its effect with ':').  So it is very
understandable why we want to have has_dos_prefix() and
skip_dos_prefix().

> I would say these could be merged into a "win.h" or similar. Cygwin typically
> leans toward the "/unix/style" while MINGW has been more tolerant of
> "C:\Windows\Style" and "C:/Mixed/Style" paths, i dont see that changing.

I'd defer to Windows folks to decide if a unified win.h is a good
idea.

Thanks.

Steven Penny Nov. 27, 2018, 6:20 a.m. UTC | #5

On Mon, Nov 26, 2018 at 11:23 PM Junio C Hamano wrote:
> Sorry, but I do not see the connection to this question and the
> above example.  The reason why we strip C: is because the letter
> that comes after that colon determines if we are talking about
> absolute path (in other words, the current directory does not play a
> role in determining which directory the path refers to), unlike the
> POSIX codepath where it is always the first letter in the pathname.

while it is true that "C:" and similar do not have a bearing on a path being
absolute versus relative, it does have a bearing on what drive the entry is to
be found.

That is to say "C:\tmp\file.txt" does not equal "D:\tmp\file.txt".

Starting with an absolute path like "C:\tmp\file.txt", after stripping that
would yield "\tmp\file.txt" or "/tmp/file.txt". Starting with a relative path
like "C:tmp\file.txt", after stripping that would yield "tmp\file.txt" or
"tmp/file.txt".

However in all cases we have lost the concept of what drive the file is located
on, and Windows will assume the file exists on the current drive.

So I would expect "git clone URL D:\tmp" to fail if the current directory is on
"C:". Upon testing cross drive clones work fine though with this patch, so maybe
the drive is added back at another place in the code.

Johannes Schindelin Nov. 27, 2018, 12:49 p.m. UTC | #6

Hi Torsten,

On Mon, 26 Nov 2018, [e-mail] wrote:

> diff --git a/compat/cygwin.c b/compat/cygwin.c
> index b9862d606d..c4a10cb5a1 100644
> --- a/compat/cygwin.c
> +++ b/compat/cygwin.c
> @@ -1,19 +1,29 @@
>  #include "../git-compat-util.h"
>  #include "../cache.h"
>  
> +int cygwin_skip_dos_drive_prefix(char **path)
> +{
> +	int ret = has_dos_drive_prefix(*path);
> +	*path += ret;
> +	return ret;
> +}
> +
>  int cygwin_offset_1st_component(const char *path)
>  {
> -	const char *pos = path;
> +	char *pos = (char *)path;
> +
>  	/* unc paths */
> -	if (is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
> +	if (!skip_dos_drive_prefix(&pos) &&
> +			is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {

It takes a little folding and knotting of the brain to understand that
this `!skip_dos_drive_prefix(&pos)` has *nothing* to do with the comment
`unc paths` nor with the test whether the paths starts with two directory
separators.

As a consequence, I would highly suggest to turn this into:

	if (skip_dos_drive_prefix(&pos))
		; /* absolute path with DOS drive prefix */
  	/* unc paths */
	else if (is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {

That makes the code a lot easier to understand, and as a consequence a lot
harder to mess up in the future.

Thanks,
Dscho

>  		/* skip server name */
> -		pos = strchr(pos + 2, '/');
> +		pos = strpbrk(pos + 2, "\\/");
>  		if (!pos)
>  			return 0; /* Error: malformed unc path */
>  
>  		do {
>  			pos++;
> -		} while (*pos && pos[0] != '/');
> +		} while (*pos && !is_dir_sep(*pos));
>  	}
> +
>  	return pos + is_dir_sep(*pos) - path;
>  }
> diff --git a/compat/cygwin.h b/compat/cygwin.h
> index 8e52de4644..46f29c0a90 100644
> --- a/compat/cygwin.h
> +++ b/compat/cygwin.h
> @@ -1,2 +1,34 @@
> +#define has_dos_drive_prefix(path) \
> +	(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
> +
> +
> +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)
> +int cygwin_skip_dos_drive_prefix(char **path);
> +#define skip_dos_drive_prefix cygwin_skip_dos_drive_prefix
> +static inline int cygwin_is_dir_sep(int c)
> +{
> +	return c == '/' || c == '\\';
> +}
> +#define is_dir_sep cygwin_is_dir_sep
> +static inline char *cygwin_find_last_dir_sep(const char *path)
> +{
> +	char *ret = NULL;
> +	for (; *path; ++path)
> +		if (is_dir_sep(*path))
> +			ret = (char *)path;
> +	return ret;
> +}
> +static inline void convert_slashes(char *path)
> +{
> +	for (; *path; path++)
> +		if (*path == '\\')
> +			*path = '/';
> +}
> +#define find_last_dir_sep cygwin_find_last_dir_sep
>  int cygwin_offset_1st_component(const char *path);
>  #define offset_1st_component cygwin_offset_1st_component
> -- 
> 2.19.0.271.gfe8321ec05
> 
>

Johannes Schindelin Nov. 27, 2018, 12:55 p.m. UTC | #7

Hi Junio,

On Tue, 27 Nov 2018, Junio C Hamano wrote:

> Steven Penny <[e-mail]> writes:
> 
> > If you strip the drive, you can still navigate within the same drive:
> >
> >     $ cd 'C:\Users'
> >     $ pwd
> >     /cygdrive/c/Users
> >
> >     $ cd '\Windows'
> >     $ pwd
> >     /cygdrive/c/Windows
> >
> > but you can no longer traverse drives:
> >
> >     $ cd '\Testing'
> >     sh: cd: \Testing: No such file or directory
> 
> Sorry, but I fail to see the point the last example wants to make.

I agree. For me, the real test is this:

me@work ~
$ cd /cygdrive

me@work /cygdrive
$ ls
c  d

So `/cygdrive` *is* a valid directory in Cygwin.

> > I would say these could be merged into a "win.h" or similar. Cygwin typically
> > leans toward the "/unix/style" while MINGW has been more tolerant of
> > "C:\Windows\Style" and "C:/Mixed/Style" paths, i dont see that changing.
> 
> I'd defer to Windows folks to decide if a unified win.h is a good
> idea.

We already have such a thing, but it is not just `win.h`, it is
`compat/win32/`. I would think that the best idea would be to move the
MINGW variants to `compat/win32/path-utils.c` and declare them in
`compat/win32/path-utils.h`, renaming them from `mingw_*()` to
`win32_*()`.

Ciao,
Dscho

Achim Gratz Nov. 27, 2018, 8:05 p.m. UTC | #8

[e-mail] writes:
> 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]

Please use the Cygwin API path conversion functions for C code and the
cygpath program for shell code instead of trying to re-implement your
own handling (which is prone to introduce subtle bugs or at least
different heuristics from what cygwin itself uses).

https://cygwin.com/cygwin-api/cygwin-functions.html#func-cygwin-path


Regards,
Achim.

Achim Gratz Nov. 27, 2018, 8:10 p.m. UTC | #9

Junio C Hamano writes:
> I wonder if it makes the rest of the code simpler if we stripped
> things like /cygdrive/c here exactly the sam way as we strip C:
> For that, has_dos_drive_prefix() needs to know /cygdrive/[a-z],
> which may not be a bad thing, I guess.  Let's read on.

The cygdrive prefix can be configured by the user to something
arbitrarily different, so if you're hoping to simplify the string
handling this way you'll most likely be disappointed.  It is exactly
that fact that led to the introduction of /proc/cygdrive as an
alternative prefix which doesn't depend on any configuration.


Regards,
Achim.

Junio C Hamano Nov. 28, 2018, 4:10 a.m. UTC | #10

Johannes Schindelin <[e-mail]> writes:

>> Sorry, but I fail to see the point the last example wants to make.
>
> I agree. For me, the real test is this:
>
> me@work ~
> $ cd /cygdrive
>
> me@work /cygdrive
> $ ls
> c  d
>
> So `/cygdrive` *is* a valid directory in Cygwin.

That supports the code that does not special case a path that begins
with /cygdrive/ and simply treats it as a full path and freely use
relative path, I guess.  Very good point.

Junio C Hamano Nov. 28, 2018, 4:12 a.m. UTC | #11

Johannes Schindelin <[e-mail]> writes:

> It takes a little folding and knotting of the brain to understand that
> this `!skip_dos_drive_prefix(&pos)` has *nothing* to do with the comment
> `unc paths` nor with the test whether the paths starts with two directory
> separators.
>
> As a consequence, I would highly suggest to turn this into:
>
> 	if (skip_dos_drive_prefix(&pos))
> 		; /* absolute path with DOS drive prefix */
>   	/* unc paths */
> 	else if (is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
>
> That makes the code a lot easier to understand, and as a consequence a lot
> harder to mess up in the future.

Excellent.  With or without "unc paths" comment, the separation does
make the logic more clear.

Houder Nov. 28, 2018, 5:55 a.m. UTC | #12

> > me@work /cygdrive
> > $ ls
> > c  d
> >
> > So `/cygdrive` *is* a valid directory in Cygwin.
> 
> That supports the code that does not special case a path that begins
> with /cygdrive/ and simply treats it as a full path and freely use
> relative path, I guess.  Very good point.

Please read

    https://cygwin.com/cygwin-ug-net/using.html#cygdrive
    ( The cygdrive path prefix )

.... you can access arbitary drives on your system by using the cygdrive path
prefix. The default value for this prefix is /cygdrive ...
....

The cygdrive prefix is a >>> virtual directory <<< under which all drives on
a system are subsumed ...
....

The cygdrive prefix may be CHANGED in the fstab file as outlined above !!!!!
....

To simplify scripting, Cygwin also provides a /proc/cygdrive symlink, ...

=====

Johannes Schindelin Nov. 28, 2018, 8:46 a.m. UTC | #13

Hi J.H.,

On Wed, 28 Nov 2018, J.H. van de Water wrote:

> > > me@work /cygdrive
> > > $ ls
> > > c  d
> > >
> > > So `/cygdrive` *is* a valid directory in Cygwin.
> > 
> > That supports the code that does not special case a path that begins
> > with /cygdrive/ and simply treats it as a full path and freely use
> > relative path, I guess.  Very good point.
> 
> Please read
> 
>     https://cygwin.com/cygwin-ug-net/using.html#cygdrive
>     ( The cygdrive path prefix )
> 
> .... you can access arbitary drives on your system by using the cygdrive path
> prefix. The default value for this prefix is /cygdrive ...
> ....
> 
> The cygdrive prefix is a >>> virtual directory <<< under which all drives on
> a system are subsumed ...
> ....
> 
> The cygdrive prefix may be CHANGED in the fstab file as outlined above !!!!!
> ....
> 
> To simplify scripting, Cygwin also provides a /proc/cygdrive symlink, ...
> 
> =====

That's all very interesting, but I fail to see the relevance with regards
to the issue at hand, namely whether to special-case `/cygdrive` as a
special prefix that cannot be treated as directory in Git.

I still maintain that it should not be special-cased, no matter whether it
is a virtual directory or whether it can be renamed to `/jh-likes-cygwin`
or whatever.

Ciao,
Dscho

Houder Nov. 28, 2018, 9:01 a.m. UTC | #14

On 2018-11-28 09:46, Johannes Schindelin wrote:
> Hi J.H.,
> 
> On Wed, 28 Nov 2018, J.H. van de Water wrote:
> 
>> > > me@work /cygdrive
>> > > $ ls
>> > > c  d
>> > >
>> > > So `/cygdrive` *is* a valid directory in Cygwin.
>> >
>> > That supports the code that does not special case a path that begins
>> > with /cygdrive/ and simply treats it as a full path and freely use
>> > relative path, I guess.  Very good point.
>> 
>> Please read
>> 
>>     https://cygwin.com/cygwin-ug-net/using.html#cygdrive
>>     ( The cygdrive path prefix )
>> 
>> .... you can access arbitary drives on your system by using the 
>> cygdrive path
>> prefix. The default value for this prefix is /cygdrive ...
>> ....
>> 
>> The cygdrive prefix is a >>> virtual directory <<< under which all 
>> drives on
>> a system are subsumed ...
>> ....
>> 
>> The cygdrive prefix may be CHANGED in the fstab file as outlined above 
>> !!!!!
>> ....
>> 
>> To simplify scripting, Cygwin also provides a /proc/cygdrive symlink, 
>> ...
>> 
>> =====
> 
> That's all very interesting, but I fail to see the relevance with 
> regards
> to the issue at hand, namely whether to special-case `/cygdrive` as a
> special prefix that cannot be treated as directory in Git.
> 
> I still maintain that it should not be special-cased, no matter whether 
> it
> is a virtual directory or whether it can be renamed to 
> `/jh-likes-cygwin`
> or whatever.

Ok. Sorry about the noise.

 From your post I got the impression that you believed that there will 
always
be a directory called /cygdrive on Cygwin.

My point: it can have a different name.

Regards,

Henri

Johannes Schindelin Nov. 28, 2018, 9:35 a.m. UTC | #15

Hi J.H.

On Wed, 28 Nov 2018, Houder wrote:

> On 2018-11-28 09:46, Johannes Schindelin wrote:
> > 
> > On Wed, 28 Nov 2018, J.H. van de Water wrote:
> > 
> > > > > me@work /cygdrive
> > > > > $ ls
> > > > > c  d
> > > > >
> > > > > So `/cygdrive` *is* a valid directory in Cygwin.
> > > >
> > > > That supports the code that does not special case a path that begins
> > > > with /cygdrive/ and simply treats it as a full path and freely use
> > > > relative path, I guess.  Very good point.
> > > 
> > > Please read
> > > 
> > >     https://cygwin.com/cygwin-ug-net/using.html#cygdrive
> > >     ( The cygdrive path prefix )
> > > 
> > > .... you can access arbitary drives on your system by using the cygdrive
> > > path
> > > prefix. The default value for this prefix is /cygdrive ...
> > > ....
> > > 
> > > The cygdrive prefix is a >>> virtual directory <<< under which all drives
> > > on
> > > a system are subsumed ...
> > > ....
> > > 
> > > The cygdrive prefix may be CHANGED in the fstab file as outlined above
> > > !!!!!
> > > ....
> > > 
> > > To simplify scripting, Cygwin also provides a /proc/cygdrive symlink, ...
> > > 
> > > =====
> > 
> > That's all very interesting, but I fail to see the relevance with regards
> > to the issue at hand, namely whether to special-case `/cygdrive` as a
> > special prefix that cannot be treated as directory in Git.
> > 
> > I still maintain that it should not be special-cased, no matter whether it
> > is a virtual directory or whether it can be renamed to `/jh-likes-cygwin`
> > or whatever.
> 
> Ok. Sorry about the noise.
> 
> From your post I got the impression that you believed that there will always
> be a directory called /cygdrive on Cygwin.

I know it can be different. In MSYS2 it is set to `/` via this line in
`/etc/fstab`:

	none / cygdrive binary,posix=0,noacl,user 0 0

Which is just to say that I am fully aware of the option to rename it.

> My point: it can have a different name.

Indeed.

And whatever name you give it, Cygwin can handle it as if it were a
regular directory. So we *must not* special-case it in Git.

Ciao,
Johannes

Patch

diff --git a/abspath.c b/abspath.c
index 9857985329..77a281f789 100644
--- a/abspath.c
+++ b/abspath.c
@@ -55,7 +55,7 @@  static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
 
 	strbuf_reset(resolved);
 	strbuf_add(resolved, remaining->buf, offset);
-#ifdef GIT_WINDOWS_NATIVE
+#if defined(GIT_WINDOWS_NATIVE) || defined(__CYGWIN__)
 	convert_slashes(resolved->buf);
 #endif
 	strbuf_remove(remaining, 0, offset);
diff --git a/compat/cygwin.c b/compat/cygwin.c
index b9862d606d..c4a10cb5a1 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -1,19 +1,29 @@ 
 #include "../git-compat-util.h"
 #include "../cache.h"
 
+int cygwin_skip_dos_drive_prefix(char **path)
+{
+	int ret = has_dos_drive_prefix(*path);
+	*path += ret;
+	return ret;
+}
+
 int cygwin_offset_1st_component(const char *path)
 {
-	const char *pos = path;
+	char *pos = (char *)path;
+
 	/* unc paths */
-	if (is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
+	if (!skip_dos_drive_prefix(&pos) &&
+			is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
 		/* skip server name */
-		pos = strchr(pos + 2, '/');
+		pos = strpbrk(pos + 2, "\\/");
 		if (!pos)
 			return 0; /* Error: malformed unc path */
 
 		do {
 			pos++;
-		} while (*pos && pos[0] != '/');
+		} while (*pos && !is_dir_sep(*pos));
 	}
+
 	return pos + is_dir_sep(*pos) - path;
 }
diff --git a/compat/cygwin.h b/compat/cygwin.h
index 8e52de4644..46f29c0a90 100644
--- a/compat/cygwin.h
+++ b/compat/cygwin.h
@@ -1,2 +1,34 @@ 
+#define has_dos_drive_prefix(path) \
+	(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
+
+
+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)
+int cygwin_skip_dos_drive_prefix(char **path);
+#define skip_dos_drive_prefix cygwin_skip_dos_drive_prefix
+static inline int cygwin_is_dir_sep(int c)
+{
+	return c == '/' || c == '\\';
+}
+#define is_dir_sep cygwin_is_dir_sep
+static inline char *cygwin_find_last_dir_sep(const char *path)
+{
+	char *ret = NULL;
+	for (; *path; ++path)
+		if (is_dir_sep(*path))
+			ret = (char *)path;
+	return ret;
+}
+static inline void convert_slashes(char *path)
+{
+	for (; *path; path++)
+		if (*path == '\\')
+			*path = '/';
+}
+#define find_last_dir_sep cygwin_find_last_dir_sep
 int cygwin_offset_1st_component(const char *path);
 #define offset_1st_component cygwin_offset_1st_component