Issues with Cygwin64 on Windows11

[Corinna Vinschen]

On Aug  2 13:12, Takashi Yano wrote:
> On Mon, 1 Aug 2022 21:14:52 -0400
> Ken Brown wrote:
> > On 7/31/2022 8:23 PM, Takashi Yano wrote:
> > > On Sun, 31 Jul 2022 17:21:32 +0900
> > > Takashi Yano wrote:
> > >> On Sun, 31 Jul 2022 09:11:17 +0300
> > >> Dimax wrote:
> > >>> $ ln -s /cygdrive/C/XOL/ work
> > >>> $ ls -all work
> > >>> lrwxrwxrwx 1 Alex None 11 Jul 31 09:09 work -> /mnt/C/XOL/
> > >>> $ cd ~/work/
> > >>> -bash: cd: /home/Alex/work/: No such file or directory
> > >>>
> > >> Thanks for the report. This seems to happen only when
> > >> the drive letter is uppercase.
> > >>
> > >> ln -s /cygdrive/c/XOL/ work
> > >> works.
> > >>
> > >> Anyway, I think this is a problem of cygwin1.dll.
> > >> [...]
> > > I found the patch attached solves the issue.
> > > 
> > > Corinna, WDYT?
> > 
> > I'm not Corinna, but replacing oldpath by normpath doesn't seem like
> > the right thing to do at the time of symlink creation.  If I create
> > a symlink under Cygwin, I expect the target to be used under Cygwin
> > exactly as I enter it.

Apart from the normalized mnt prefix, that's right.

> Hmm, that's the point.
> 
> > The internal replacement of the cygdrive prefix by /mnt for WSL
> > compatibility is fine, as long as I never see it except under WSL.
> > But since WSL doesn't recognize /mnt/<uppercase drive letter>, I
> > don't think Cygwin should convert /<cygdrive prefix>/<uppercase
> > drive letter>.  Users who want WSL interoperability just have to use
> > lowercase drive letters.

Yes, I agree.  I'm surprised anyway that somebody is using uppercase
drive letters voluntarily.

> Then, what about the v2 patch attached?

symlink_wsl is doing the right thing, as Ken points out.  I think, the
solution is not to change symlink creation but rather symlink
evaluation.  Locally I applied the below patch and fixed the issue:

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index ea695ed997b4..6b519d0bbe2d 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -2651,7 +2651,7 @@ check_reparse_point_target (HANDLE h, bool remote, PREPARSE_DATA_BUFFER rp,
 		}
 	      rp->ReparseDataLength = path_len + sizeof (DWORD);
 	    }
-	  else if (islower (path_buf[drv_prefix_len + 1])
+	  else if (isalpha (path_buf[drv_prefix_len + 1])
 		   && (path_len == drv_prefix_len + 2
 		       || path_buf[drv_prefix_len + 2] == '/'))
 	    {

This does not fix cases like /cygdrive/./c or something like that,
but I wonder if we really have to handle them...?

The idea was to convert /mnt into the cygdrive prefix only if this is a
cygdrive path and not to touch /mnt if it's a user generated path.

Therefore, the more effort the code makes to be clever, the higher
chances are that an incorrect conversion takes place.

Having said that, of course, we could change check_reparse_point_target
to handle multiple slashes or dot path components, kind of like the
below...

...but the point of discussion is, how much effort do we want to
put into this?

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index ea695ed997b4..c7f743b3c7bf 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -2628,7 +2628,7 @@ check_reparse_point_target (HANDLE h, bool remote, PREPARSE_DATA_BUFFER rp,
       char *path_buf = rpl->LxSymlinkReparseBuffer.PathBuffer;
       DWORD path_len = rpl->ReparseDataLength - sizeof (DWORD);
       bool full_path = false;
-      const size_t drv_prefix_len = strlen ("/mnt");
+      size_t drv_prefix_len = 4; /* strlen ("/mnt") */
       PBYTE utf16_ptr;
       PWCHAR utf16_buf;
       int utf16_bufsize;
@@ -2651,15 +2651,30 @@ check_reparse_point_target (HANDLE h, bool remote, PREPARSE_DATA_BUFFER rp,
 		}
 	      rp->ReparseDataLength = path_len + sizeof (DWORD);
 	    }
-	  else if (islower (path_buf[drv_prefix_len + 1])
+	  else
+	    {
+	      /* Skip multiple slashes and "." path components between
+	         /mnt prefix and the drive letter. */
+	      while (true)
+		{
+		  if (path_buf[drv_prefix_len + 1] == '/')
+		    ++drv_prefix_len;
+		  else if (path_buf[drv_prefix_len + 1] == '.'
+			   && path_buf[drv_prefix_len + 2] == '/')
+		    drv_prefix_len += 2;
+		  else
+		    break;
+		}
+	      if (isalpha (path_buf[drv_prefix_len + 1])
 		   && (path_len == drv_prefix_len + 2
 		       || path_buf[drv_prefix_len + 2] == '/'))
-	    {
-	      /* Skip forward to the slash leading the drive letter.
-		 That leaves room for adding the colon. */
-	      path_buf += drv_prefix_len;
-	      path_len -= drv_prefix_len;
-	      full_path = true;
+		{
+		  /* Skip forward to the slash leading the drive letter.
+		     That leaves room for adding the colon. */
+		  path_buf += drv_prefix_len;
+		  path_len -= drv_prefix_len;
+		  full_path = true;
+		}
 	    }
 	}
       /* Compute buffer for path converted to UTF-16. */

> > I'm tempted to go even further and say that Cygwin shouldn't ever
> > convert the cygdrive prefix to /mnt, on the grounds that users who
> > care about WSL interoperability can simply use /mnt as their
> > cygdrive prefix.  But maybe that ship has sailed.

In hindsight this might have been a step too far.  I was trying to allow
interoperability and reduce the number of problems based on different
drive letter handling.  And, well, this is the first time a user has
a problem due to that :}

The ship hasn't sailed entirely.  We can revert this decision for 3.4
and just keep the /mnt conversion in check_reparse_point_target for
backward compat.  Or we just fix the problem at hand and otherwise
keep the code as is...?


Corinna