Re: Symbolic link bug in recent Cygwin DLL build

Re: Symbolic link bug in recent Cygwin DLL build

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1214 bytes --]

Hi Mark,

On Apr 15 23:53, Mark Geisert wrote:
> After installing a recent DLL built from the git source tree I noticed:
> 
> ~ ln -s /tmp/foo .
> 
> ~ ls -l foo
> lrwxrwxrwx 1 Mark None 12 Apr 15 23:44 foo -> /mnt/tmp/foo

Huh?  That works for me, independently of /tmp/foo existing or not:

  $ ln -s /tmp/foo .
  $ ls -l foo
  lrwxrwxrwx 1 corinna Users 8 Apr 16 10:38 foo -> /tmp/foo

Since you're building the DLL yourself, can you please debug this with
GDB?  It would be very important to find out what on your system adds
the /mnt prefix!

It could occur in creating the symlink, that's in path.cc, function
symlink_wsl(), or it could occur in reading the symlink, path.cc,
function check_reparse_point_target(), in the else if (rp->ReparseTag ==
IO_REPARSE_TAG_LX_SYMLINK) branch at line 2534.

As for /mnt itself, it's the WSL equivalent to the cygdrive prefix.
When creating WSL symlinks, Cygwin converts the cygdrive prefix to
/mnt, and when reading WSL symlinks, a leading /mnt is converted
to the current cygdrive prefix on the fly.

We should just move further discussions to the cygwin-developers ML.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Symbolic link bug in recent Cygwin DLL build

[Mark Geisert]

Hi Corinna,

Corinna Vinschen wrote:
> Hi Mark,
> 
> On Apr 15 23:53, Mark Geisert wrote:
>> After installing a recent DLL built from the git source tree I noticed:
>>
>> ~ ln -s /tmp/foo .
>>
>> ~ ls -l foo
>> lrwxrwxrwx 1 Mark None 12 Apr 15 23:44 foo -> /mnt/tmp/foo
> 
> Huh?  That works for me, independently of /tmp/foo existing or not:
> 
>    $ ln -s /tmp/foo .
>    $ ls -l foo
>    lrwxrwxrwx 1 corinna Users 8 Apr 16 10:38 foo -> /tmp/foo
> 
> Since you're building the DLL yourself, can you please debug this with
> GDB?  It would be very important to find out what on your system adds
> the /mnt prefix!
> 
> It could occur in creating the symlink, that's in path.cc, function
> symlink_wsl(), or it could occur in reading the symlink, path.cc,
> function check_reparse_point_target(), in the else if (rp->ReparseTag ==
> IO_REPARSE_TAG_LX_SYMLINK) branch at line 2534.
> 
> As for /mnt itself, it's the WSL equivalent to the cygdrive prefix.
> When creating WSL symlinks, Cygwin converts the cygdrive prefix to
> /mnt, and when reading WSL symlinks, a leading /mnt is converted
> to the current cygdrive prefix on the fly.
> 
> We should just move further discussions to the cygwin-developers ML.

Sorry about the ML flub.  Your mention of cygdrive prefix gave me some dread. 
 From day 1 I've suppressed the prefix on any Cygwin machine I've administered; 
I use /c or /d etc to refer to drives.  That means having this line in /etc/fstab:
     none / cygdrive binary,posix=0,user 0 0

The "/mnt" is being added at path.cc:1892.  It's replacing the current cygdrive 
prefix with "/mnt".  I'm unclear why we're in a function named symlink_wsl() 
since I'm not using WSL at all.  Maybe it means WSL-compatible?  OK if so.

Does that string replacement need to be skipped in this situation of "empty" 
cygdrive prefix?  Or is it something more subtle?

..mark

Re: Symbolic link bug in recent Cygwin DLL build

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 3406 bytes --]

Hi Mark,

On Apr 16 20:10, Mark Geisert wrote:
> Corinna Vinschen wrote:
> > On Apr 15 23:53, Mark Geisert wrote:
> > > After installing a recent DLL built from the git source tree I noticed:
> > > 
> > > ~ ln -s /tmp/foo .
> > > 
> > > ~ ls -l foo
> > > lrwxrwxrwx 1 Mark None 12 Apr 15 23:44 foo -> /mnt/tmp/foo
> > 
> > Huh?  That works for me, independently of /tmp/foo existing or not:
> > 
> >    $ ln -s /tmp/foo .
> >    $ ls -l foo
> >    lrwxrwxrwx 1 corinna Users 8 Apr 16 10:38 foo -> /tmp/foo
> > 
> > Since you're building the DLL yourself, can you please debug this with
> > GDB?  It would be very important to find out what on your system adds
> > the /mnt prefix!
> > 
> > It could occur in creating the symlink, that's in path.cc, function
> > symlink_wsl(), or it could occur in reading the symlink, path.cc,
> > function check_reparse_point_target(), in the else if (rp->ReparseTag ==
> > IO_REPARSE_TAG_LX_SYMLINK) branch at line 2534.
> > 
> > As for /mnt itself, it's the WSL equivalent to the cygdrive prefix.
> > When creating WSL symlinks, Cygwin converts the cygdrive prefix to
> > /mnt, and when reading WSL symlinks, a leading /mnt is converted
> > to the current cygdrive prefix on the fly.
> > 
> > We should just move further discussions to the cygwin-developers ML.
> 
> Sorry about the ML flub.  Your mention of cygdrive prefix gave me some
> dread. From day 1 I've suppressed the prefix on any Cygwin machine I've
> administered; I use /c or /d etc to refer to drives.  That means having this
> line in /etc/fstab:
>     none / cygdrive binary,posix=0,user 0 0
> 
> The "/mnt" is being added at path.cc:1892.  It's replacing the current
> cygdrive prefix with "/mnt".

Ok, that explains it.  It makes this function exactly this teeny little
bit more complicated :-P

>I'm unclear why we're in a function named
> symlink_wsl() since I'm not using WSL at all.  Maybe it means
> WSL-compatible?  OK if so.

git log --grep symlinks d2e0b65a7fdc..HEAD

> Does that string replacement need to be skipped in this situation of "empty"
> cygdrive prefix?  Or is it something more subtle?

It's a bit more subtil.  The code also tries to convert the cygdrive
prefix to /mnt if no drive letter follows, so you can create a symlink
to the cygdrive directory alone:

  $ ln -s /cygdrive /tmp/cygdrive_symlink

That also allowed the conversion cygdrive prefix to /mnt without having
to look for the drive letters.  Now, with the cygdrive prefix being /,
the first path component has to be checked for being... what?

- Just check for a single letter from a to z?  That would also wrongly
  catch /x if /x is a real subdir in the root dir. 

- Check for an existing drive letter from a to z?  That would break
  the symlink conversion on the WSL side if the symlink was supposed
  to be created pointing to, say, an USB thumb drive which was not
  plugged in at the time of the symlink creation.

- Just skip any /mnt prefix if the cygdrive prefix is /?  That would
  make all drive letter paths incompatible with WSL, and in contrast to
  /mnt symlinks, it does not autommagically "fix" drive letter paths
  if you ever change your cygdrive prefix, which is a neat side-effect
  of the new symlinks, IMHO.

I'm leaning towards solution 3.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Symbolic link bug in recent Cygwin DLL build

[Mark Geisert]

Corinna Vinschen wrote:
> On Apr 16 20:10, Mark Geisert wrote:
>> Sorry about the ML flub.  Your mention of cygdrive prefix gave me some
>> dread. From day 1 I've suppressed the prefix on any Cygwin machine I've
>> administered; I use /c or /d etc to refer to drives.  That means having this
>> line in /etc/fstab:
>>      none / cygdrive binary,posix=0,user 0 0
>>
>> The "/mnt" is being added at path.cc:1892.  It's replacing the current
>> cygdrive prefix with "/mnt".
> 
> Ok, that explains it.  It makes this function exactly this teeny little
> bit more complicated :-P
> 
>> I'm unclear why we're in a function named
>> symlink_wsl() since I'm not using WSL at all.  Maybe it means
>> WSL-compatible?  OK if so.
> 
> git log --grep symlinks d2e0b65a7fdc..HEAD >
>> Does that string replacement need to be skipped in this situation of "empty"
>> cygdrive prefix?  Or is it something more subtle?
> 
> It's a bit more subtil.  The code also tries to convert the cygdrive
> prefix to /mnt if no drive letter follows, so you can create a symlink
> to the cygdrive directory alone:
> 
>    $ ln -s /cygdrive /tmp/cygdrive_symlink
> 
> That also allowed the conversion cygdrive prefix to /mnt without having
> to look for the drive letters.  Now, with the cygdrive prefix being /,
> the first path component has to be checked for being... what?
> 
> - Just check for a single letter from a to z?  That would also wrongly
>    catch /x if /x is a real subdir in the root dir.
> 
> - Check for an existing drive letter from a to z?  That would break
>    the symlink conversion on the WSL side if the symlink was supposed
>    to be created pointing to, say, an USB thumb drive which was not
>    plugged in at the time of the symlink creation.
> 
> - Just skip any /mnt prefix if the cygdrive prefix is /?  That would
>    make all drive letter paths incompatible with WSL, and in contrast to
>    /mnt symlinks, it does not autommagically "fix" drive letter paths
>    if you ever change your cygdrive prefix, which is a neat side-effect
>    of the new symlinks, IMHO.
> 
> I'm leaning towards solution 3.

That seems OK to me but I don't have much depth of experience about possible 
downsides.  If it was instead decided that we need to require a "/path" for a 
cygdrive prefix in /etc/fstab I would just go back to creating symlinks like /c 
-> /cygdrive/c like I did long ago.

You want I should submit a patch for solution 3?

..mark

Re: Symbolic link bug in recent Cygwin DLL build

[Corinna Vinschen]

On Apr 21 00:15, Mark Geisert wrote:
> Corinna Vinschen wrote:
> > On Apr 16 20:10, Mark Geisert wrote:
> > > [...]
> > >      none / cygdrive binary,posix=0,user 0 0
> > > [...]
> > git log --grep symlinks d2e0b65a7fdc..HEAD >
> > > Does that string replacement need to be skipped in this situation of "empty"
> > > cygdrive prefix?  Or is it something more subtle?
> > [...]
> >    $ ln -s /cygdrive /tmp/cygdrive_symlink
> > 
> > That also allowed the conversion cygdrive prefix to /mnt without having
> > to look for the drive letters.  Now, with the cygdrive prefix being /,
> > the first path component has to be checked for being... what?
> > 
> > - Just check for a single letter from a to z?  That would also wrongly
> >    catch /x if /x is a real subdir in the root dir.
> > 
> > - Check for an existing drive letter from a to z?  That would break
> >    the symlink conversion on the WSL side if the symlink was supposed
> >    to be created pointing to, say, an USB thumb drive which was not
> >    plugged in at the time of the symlink creation.
> > 
> > - Just skip any /mnt prefix if the cygdrive prefix is /?  That would
> >    make all drive letter paths incompatible with WSL, and in contrast to
> >    /mnt symlinks, it does not autommagically "fix" drive letter paths
> >    if you ever change your cygdrive prefix, which is a neat side-effect
> >    of the new symlinks, IMHO.
> > 
> > I'm leaning towards solution 3.
> 
> That seems OK to me but I don't have much depth of experience about possible
> downsides.  If it was instead decided that we need to require a "/path" for
> a cygdrive prefix in /etc/fstab I would just go back to creating symlinks
> like /c -> /cygdrive/c like I did long ago.

We can't do that these days without breaking lots of installations
already using this prefix.

> You want I should submit a patch for solution 3?

No, the patch is really simple and I already have it in the loop.

However, to get the full functionality it might still be a good idea to
change the cygdrive prefix to something else(*) and use symlinks to
/proc/cygdrive/X in / instead.


Thanks,
Corinna

(*) Funny enough I'm using /mnt as cygdrive prefix for ages... which
    means, I had to change it to something else first, otherwise I
    wouldn't have been able to test if the WSL symlinks are doing the
    right thing in terms of cygdrive prefix translation...

-- 
Corinna Vinschen
Cygwin Maintainer