public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* [PATCH 0/3] Add more winsymlinks values
@ 2021-07-19 16:31 Jon Turney
  2021-07-19 16:31 ` [PATCH 1/3] Rename WSYM_sysfile to WSM_default Jon Turney
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Jon Turney @ 2021-07-19 16:31 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Jon Turney

I'm not sure this is the best idea, since it adds more configurations that
aren't going to get tested often, but the idea is that this would enable
proper and consistent control of the symlink type used from setup, as
discussed in [1].

[1] https://cygwin.com/pipermail/cygwin-apps/2021-May/041327.html

Jon Turney (3):
  Rename WSYM_sysfile to WSM_default
  Add winsymlinks:magic
  Add winsymlinks:wslstrict

 winsup/cygwin/environ.cc |  5 +++++
 winsup/cygwin/globals.cc |  8 +++++---
 winsup/cygwin/path.cc    | 16 ++++++++++++----
 winsup/doc/cygwinenv.xml | 29 ++++++++++++++++++++++++++++-
 winsup/doc/pathnames.xml |  7 +++----
 5 files changed, 53 insertions(+), 12 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] Rename WSYM_sysfile to WSM_default
  2021-07-19 16:31 [PATCH 0/3] Add more winsymlinks values Jon Turney
@ 2021-07-19 16:31 ` Jon Turney
  2021-07-19 16:31 ` [PATCH 2/3] Add winsymlinks:magic Jon Turney
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Jon Turney @ 2021-07-19 16:31 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Jon Turney

Rename WSYM_sysfile to WSM_default, since it selects more than just
sysfile with magic cookie now.
---
 winsup/cygwin/globals.cc | 4 ++--
 winsup/cygwin/path.cc    | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/winsup/cygwin/globals.cc b/winsup/cygwin/globals.cc
index 3b25c2803..066026421 100644
--- a/winsup/cygwin/globals.cc
+++ b/winsup/cygwin/globals.cc
@@ -53,7 +53,7 @@ enum exit_states
    "winsymlinks" setting of the CYGWIN environment variable. */
 enum winsym_t
 {
-  WSYM_sysfile = 0,
+  WSYM_default = 0,
   WSYM_lnk,
   WSYM_native,
   WSYM_nativestrict,
@@ -71,7 +71,7 @@ bool ignore_case_with_glob;
 bool pipe_byte;
 bool reset_com;
 bool wincmdln;
-winsym_t allow_winsymlinks = WSYM_sysfile;
+winsym_t allow_winsymlinks = WSYM_default;
 bool disable_pcon;
 
 bool NO_COPY in_forkee;
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index 1869fb8c8..cd029c5b4 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -2013,7 +2013,7 @@ symlink_worker (const char *oldpath, path_conv &win32_newpath, bool isdevice)
       /* Don't try native symlinks on FSes not supporting reparse points. */
       else if ((wsym_type == WSYM_native || wsym_type == WSYM_nativestrict)
 	       && !(win32_newpath.fs_flags () & FILE_SUPPORTS_REPARSE_POINTS))
-	wsym_type = WSYM_sysfile;
+	wsym_type = WSYM_default;
 
       /* Attach .lnk suffix when shortcut is requested. */
       if (wsym_type == WSYM_lnk && !win32_newpath.exists ()
@@ -2059,9 +2059,9 @@ symlink_worker (const char *oldpath, path_conv &win32_newpath, bool isdevice)
 	      __leave;
 	    }
 	  /* Otherwise, fall back to default symlink type. */
-	  wsym_type = WSYM_sysfile;
+	  wsym_type = WSYM_default;
 	  fallthrough;
-	case WSYM_sysfile:
+	case WSYM_default:
 	  if (win32_newpath.fs_flags () & FILE_SUPPORTS_REPARSE_POINTS)
 	    {
 	      res = symlink_wsl (oldpath, win32_newpath);
-- 
2.32.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/3] Add winsymlinks:magic
  2021-07-19 16:31 [PATCH 0/3] Add more winsymlinks values Jon Turney
  2021-07-19 16:31 ` [PATCH 1/3] Rename WSYM_sysfile to WSM_default Jon Turney
@ 2021-07-19 16:31 ` Jon Turney
  2021-07-19 16:31 ` [PATCH 3/3] Add winsymlinks:wslstrict Jon Turney
  2021-07-21  8:19 ` [PATCH 0/3] Add more winsymlinks values Corinna Vinschen
  3 siblings, 0 replies; 12+ messages in thread
From: Jon Turney @ 2021-07-19 16:31 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Jon Turney

Add winsymlinks:magic, to explicitly select always using plain files
containing a magic cookie to represent a symlink.
---
 winsup/cygwin/environ.cc |  2 ++
 winsup/cygwin/globals.cc |  3 ++-
 winsup/cygwin/path.cc    |  3 ++-
 winsup/doc/cygwinenv.xml | 16 +++++++++++++++-
 winsup/doc/pathnames.xml |  7 +++----
 5 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/winsup/cygwin/environ.cc b/winsup/cygwin/environ.cc
index 3a03657db..a7a52feeb 100644
--- a/winsup/cygwin/environ.cc
+++ b/winsup/cygwin/environ.cc
@@ -82,6 +82,8 @@ set_winsymlinks (const char *buf)
     allow_winsymlinks = WSYM_lnk;
   else if (ascii_strncasematch (buf, "lnk", 3))
     allow_winsymlinks = WSYM_lnk;
+  else if (ascii_strncasematch (buf, "magic", 5))
+    allow_winsymlinks = WSYM_magic;
   /* Make sure to try native symlinks only on systems supporting them. */
   else if (ascii_strncasematch (buf, "native", 6))
     allow_winsymlinks = ascii_strcasematch (buf + 6, "strict")
diff --git a/winsup/cygwin/globals.cc b/winsup/cygwin/globals.cc
index 066026421..b15980bb3 100644
--- a/winsup/cygwin/globals.cc
+++ b/winsup/cygwin/globals.cc
@@ -57,7 +57,8 @@ enum winsym_t
   WSYM_lnk,
   WSYM_native,
   WSYM_nativestrict,
-  WSYM_nfs
+  WSYM_nfs,
+  WSYM_magic,
 };
 
 exit_states NO_COPY exit_state;
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index cd029c5b4..edb3b27ee 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -2071,6 +2071,7 @@ symlink_worker (const char *oldpath, path_conv &win32_newpath, bool isdevice)
 	  /* On FSes not supporting reparse points, or in case of an error
 	     creating the WSL symlink, fall back to creating the plain old
 	     SYSTEM file symlink. */
+	  wsym_type = WSYM_magic;
 	  break;
 	default:
 	  break;
@@ -2211,7 +2212,7 @@ symlink_worker (const char *oldpath, path_conv &win32_newpath, bool isdevice)
 		  * sizeof (WCHAR);
 	  cp += *plen;
 	}
-      else
+      else /* wsym_type == WSYM_magic */
 	{
 	  /* Default technique creating a symlink. */
 	  buf = tp.t_get ();
diff --git a/winsup/doc/cygwinenv.xml b/winsup/doc/cygwinenv.xml
index a52b6ac19..496088292 100644
--- a/winsup/doc/cygwinenv.xml
+++ b/winsup/doc/cygwinenv.xml
@@ -76,11 +76,23 @@ in addition to the normal UNIX argv list.  Defaults to not set.</para>
 </listitem>
 
 <listitem>
-<para><envar>winsymlinks:{lnk,native,nativestrict}</envar> - if set to just
+<para><envar>winsymlinks:{lnk,magic,native,nativestrict}</envar></para>
+
+<itemizedlist mark="square">
+<listitem>
+<para>If set to just
 <literal>winsymlinks</literal> or <literal>winsymlinks:lnk</literal>,
 Cygwin creates symlinks as Windows shortcuts with a special header and
 the R/O attribute set.</para>
+</listitem>
 
+<listitem>
+<para>If set to <literal>winsymlinks:magic</literal> Cygwin creates symlinks
+as plain files containing a magic cookie followed by the path to which the
+link points.</para>
+</listitem>
+
+<listitem>
 <para>If set to <literal>winsymlinks:native</literal> or
 <literal>winsymlinks:nativestrict</literal>, Cygwin creates symlinks as
 native Windows symlinks on filesystems and OS versions supporting them.</para>
@@ -92,6 +104,8 @@ some reason, it will fall back to creating Cygwin default symlinks
 with <literal>winsymlinks:native</literal>, while with
 <literal>winsymlinks:nativestrict</literal> the <literal>symlink(2)</literal>
 system call will immediately fail.</para>
+</listitem>
+</itemizedlist>
 
 <para>For more information on symbolic links, see
 <xref linkend="pathnames-symlinks"></xref>.</para>
diff --git a/winsup/doc/pathnames.xml b/winsup/doc/pathnames.xml
index 2966bdabf..132d8a3e1 100644
--- a/winsup/doc/pathnames.xml
+++ b/winsup/doc/pathnames.xml
@@ -389,10 +389,9 @@ ways.</para>
 <itemizedlist mark="bullet">
 
 <listitem>
-<para>The default symlinks created by Cygwin are either special reparse
-points shared with WSL on Windows 10, or plain files containing a magic
-cookie followed by the path to which the link points.  The reparse point
-is used on NTFS, the plain file on almost any other filesystem.</para>
+<para>The default symlinks created by Cygwin are either special reparse points
+shared with WSL on NTFS on Windows 10 1607 or later, otherwise plain files
+containing a magic cookie followed by the path to which the link points.</para>
 
 <note><para>Symlinks created by really old Cygwin releases (prior to
 Cygwin 1.7.0) are usually readable.  However, you could run into problems
-- 
2.32.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/3] Add winsymlinks:wslstrict
  2021-07-19 16:31 [PATCH 0/3] Add more winsymlinks values Jon Turney
  2021-07-19 16:31 ` [PATCH 1/3] Rename WSYM_sysfile to WSM_default Jon Turney
  2021-07-19 16:31 ` [PATCH 2/3] Add winsymlinks:magic Jon Turney
@ 2021-07-19 16:31 ` Jon Turney
  2021-07-21  8:19 ` [PATCH 0/3] Add more winsymlinks values Corinna Vinschen
  3 siblings, 0 replies; 12+ messages in thread
From: Jon Turney @ 2021-07-19 16:31 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Jon Turney

Add winsymlinks:wslstrict, so we have a spanning set of values for
winsymlinks.
---
 winsup/cygwin/environ.cc |  3 +++
 winsup/cygwin/globals.cc |  1 +
 winsup/cygwin/path.cc    |  7 +++++++
 winsup/doc/cygwinenv.xml | 15 ++++++++++++++-
 4 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/environ.cc b/winsup/cygwin/environ.cc
index a7a52feeb..b53b018fd 100644
--- a/winsup/cygwin/environ.cc
+++ b/winsup/cygwin/environ.cc
@@ -88,6 +88,9 @@ set_winsymlinks (const char *buf)
   else if (ascii_strncasematch (buf, "native", 6))
     allow_winsymlinks = ascii_strcasematch (buf + 6, "strict")
 			? WSYM_nativestrict : WSYM_native;
+  else if (ascii_strncasematch (buf, "wsl", 3))
+    allow_winsymlinks = ascii_strcasematch (buf + 3, "strict")
+			? WSYM_wslstrict : WSYM_default;
 }
 
 /* The structure below is used to set up an array which is used to
diff --git a/winsup/cygwin/globals.cc b/winsup/cygwin/globals.cc
index b15980bb3..9459d8bcb 100644
--- a/winsup/cygwin/globals.cc
+++ b/winsup/cygwin/globals.cc
@@ -59,6 +59,7 @@ enum winsym_t
   WSYM_nativestrict,
   WSYM_nfs,
   WSYM_magic,
+  WSYM_wslstrict,
 };
 
 exit_states NO_COPY exit_state;
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index edb3b27ee..57ec8be72 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -2062,12 +2062,19 @@ symlink_worker (const char *oldpath, path_conv &win32_newpath, bool isdevice)
 	  wsym_type = WSYM_default;
 	  fallthrough;
 	case WSYM_default:
+	case WSYM_wslstrict:
 	  if (win32_newpath.fs_flags () & FILE_SUPPORTS_REPARSE_POINTS)
 	    {
 	      res = symlink_wsl (oldpath, win32_newpath);
 	      if (!res)
 		__leave;
 	    }
+	  /* Strictly wsl? */
+	  if (wsym_type == WSYM_wslstrict)
+	    {
+	      __seterrno ();
+	      __leave;
+	    }
 	  /* On FSes not supporting reparse points, or in case of an error
 	     creating the WSL symlink, fall back to creating the plain old
 	     SYSTEM file symlink. */
diff --git a/winsup/doc/cygwinenv.xml b/winsup/doc/cygwinenv.xml
index 496088292..b98e27243 100644
--- a/winsup/doc/cygwinenv.xml
+++ b/winsup/doc/cygwinenv.xml
@@ -76,7 +76,7 @@ in addition to the normal UNIX argv list.  Defaults to not set.</para>
 </listitem>
 
 <listitem>
-<para><envar>winsymlinks:{lnk,magic,native,nativestrict}</envar></para>
+<para><envar>winsymlinks:{lnk,magic,native,nativestrict,wsl,wslstrict}</envar></para>
 
 <itemizedlist mark="square">
 <listitem>
@@ -105,6 +105,19 @@ with <literal>winsymlinks:native</literal>, while with
 <literal>winsymlinks:nativestrict</literal> the <literal>symlink(2)</literal>
 system call will immediately fail.</para>
 </listitem>
+
+<listitem>
+<para>If set to <literal>winsymlinks:wsl</literal> or
+<literal>winsymlinks:wslstrict</literal>, Cygwin creates symlinks as special
+reparse points, defined by WSL.</para>
+
+<para>With <literal>winsymlinks:wsl</literal>, if Cygwin fails to create a WSL
+symlink for some reason, it will fall back to creating a Cygwin magic cookie
+symlink, while with <literal>winsymlinks:wslstrict</literal> the
+<literal>symlink(2)</literal> system call will immediately fail.</para>
+
+<para><literal>winsymlinks:wsl</literal> is the default behaviour.</para>
+</listitem>
 </itemizedlist>
 
 <para>For more information on symbolic links, see
-- 
2.32.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Add more winsymlinks values
  2021-07-19 16:31 [PATCH 0/3] Add more winsymlinks values Jon Turney
                   ` (2 preceding siblings ...)
  2021-07-19 16:31 ` [PATCH 3/3] Add winsymlinks:wslstrict Jon Turney
@ 2021-07-21  8:19 ` Corinna Vinschen
  2021-07-21 10:24   ` Christian Franke
  2021-07-22 13:53   ` Jon Turney
  3 siblings, 2 replies; 12+ messages in thread
From: Corinna Vinschen @ 2021-07-21  8:19 UTC (permalink / raw)
  To: cygwin-patches

On Jul 19 17:31, Jon Turney wrote:
> I'm not sure this is the best idea, since it adds more configurations that
> aren't going to get tested often, but the idea is that this would enable
> proper and consistent control of the symlink type used from setup, as
> discussed in [1].
> 
> [1] https://cygwin.com/pipermail/cygwin-apps/2021-May/041327.html

Why isn't it sufficient to use 'winsymlinks:native' from setup?

The way we express symlinks shouldn't be a user choice, really.  The
winsymlinks thingy was only ever introduced in a desperate attempt to
improve access to symlinks from native tools, and I still don't see a
way around that.  But either way, what's the advantage in allowing the
user complete control over the type, even if the type is only useful in
Cygwin?


Corinna

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Add more winsymlinks values
  2021-07-21  8:19 ` [PATCH 0/3] Add more winsymlinks values Corinna Vinschen
@ 2021-07-21 10:24   ` Christian Franke
  2021-07-22  8:03     ` Corinna Vinschen
  2021-07-22 13:53   ` Jon Turney
  1 sibling, 1 reply; 12+ messages in thread
From: Christian Franke @ 2021-07-21 10:24 UTC (permalink / raw)
  To: cygwin-patches

Corinna Vinschen wrote:
> On Jul 19 17:31, Jon Turney wrote:
>> I'm not sure this is the best idea, since it adds more configurations that
>> aren't going to get tested often, but the idea is that this would enable
>> proper and consistent control of the symlink type used from setup, as
>> discussed in [1].
>>
>> [1] https://cygwin.com/pipermail/cygwin-apps/2021-May/041327.html
> Why isn't it sufficient to use 'winsymlinks:native' from setup?
>
> The way we express symlinks shouldn't be a user choice, really.  The
> winsymlinks thingy was only ever introduced in a desperate attempt to
> improve access to symlinks from native tools, and I still don't see a
> way around that.  But either way, what's the advantage in allowing the
> user complete control over the type, even if the type is only useful in
> Cygwin?
>

WSL compatible symlinks introduce several issues with non-Cygwin 
Copy/Archive/Backup tools (robocopy behaves strange, 7-Zip stores these 
as empty files, ...). If WSL itself is not used on a machine, there is 
possibly no benefit using such symlinks for Cygwin there.

I usually prefer the old "magic" cookie SYSTEM files, in particular on 
portable installs for "rescue" purposes. Patch 2/3 would allow to select 
these.

+1 from me for this enhancement.

Christian


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Add more winsymlinks values
  2021-07-21 10:24   ` Christian Franke
@ 2021-07-22  8:03     ` Corinna Vinschen
  0 siblings, 0 replies; 12+ messages in thread
From: Corinna Vinschen @ 2021-07-22  8:03 UTC (permalink / raw)
  To: cygwin-patches

On Jul 21 12:24, Christian Franke wrote:
> Corinna Vinschen wrote:
> > On Jul 19 17:31, Jon Turney wrote:
> > > I'm not sure this is the best idea, since it adds more configurations that
> > > aren't going to get tested often, but the idea is that this would enable
> > > proper and consistent control of the symlink type used from setup, as
> > > discussed in [1].
> > > 
> > > [1] https://cygwin.com/pipermail/cygwin-apps/2021-May/041327.html
> > Why isn't it sufficient to use 'winsymlinks:native' from setup?
> > 
> > The way we express symlinks shouldn't be a user choice, really.  The
> > winsymlinks thingy was only ever introduced in a desperate attempt to
> > improve access to symlinks from native tools, and I still don't see a
> > way around that.  But either way, what's the advantage in allowing the
> > user complete control over the type, even if the type is only useful in
> > Cygwin?
> > 
> 
> WSL compatible symlinks introduce several issues with non-Cygwin
> Copy/Archive/Backup tools (robocopy behaves strange, 7-Zip stores these as
> empty files, ...).

Native backup tools are supposed to store unknown reparse points
verbatim,  If they don't, it's a bug in these tools which not only
affects Cygwin, but every unknown reparse point.


Corinna

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Add more winsymlinks values
  2021-07-21  8:19 ` [PATCH 0/3] Add more winsymlinks values Corinna Vinschen
  2021-07-21 10:24   ` Christian Franke
@ 2021-07-22 13:53   ` Jon Turney
  2021-07-22 14:21     ` Corinna Vinschen
  1 sibling, 1 reply; 12+ messages in thread
From: Jon Turney @ 2021-07-22 13:53 UTC (permalink / raw)
  To: Cygwin Patches

On 21/07/2021 09:19, Corinna Vinschen wrote:
> On Jul 19 17:31, Jon Turney wrote:
>> I'm not sure this is the best idea, since it adds more configurations that
>> aren't going to get tested often, but the idea is that this would enable
>> proper and consistent control of the symlink type used from setup, as
>> discussed in [1].
>>
>> [1] https://cygwin.com/pipermail/cygwin-apps/2021-May/041327.html
> 
> Why isn't it sufficient to use 'winsymlinks:native' from setup?

I think in the default Windows configuration (developer mode off, no 
SeCreateSymbolicLinkPrivilege), 'native' will try to create a native 
symlink and fail, and fallback to WSL IO_REPARSE_TAG_LX_SYMLINK reparse 
point, then magic cookie + sys attribute.

This leads to cygwin installations with WSL symlinks created by 
post-install scripts, which can't be put into Docker containers [1], 
which is the original problem I was trying to fix.

[1] https://cygwin.com/pipermail/cygwin/2020-August/245994.html

I haven't yet looked at adding 'native' symlink support to setup itself, 
but it's probably going to be a bit of a pain.

> The way we express symlinks shouldn't be a user choice, really.  The
> winsymlinks thingy was only ever introduced in a desperate attempt to
> improve access to symlinks from native tools, and I still don't see a
> way around that.  But either way, what's the advantage in allowing the
> user complete control over the type, even if the type is only useful in
> Cygwin?
  If we can come up with a fixed policy that works everywhere, there is 
no advantage.  But that seems unlikely :)

I could buy an argument that 'native' should be the default (although 
maybe all that does is slow things down in the majority of installs?).


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Add more winsymlinks values
  2021-07-22 13:53   ` Jon Turney
@ 2021-07-22 14:21     ` Corinna Vinschen
  2021-07-28 19:55       ` Jon Turney
  0 siblings, 1 reply; 12+ messages in thread
From: Corinna Vinschen @ 2021-07-22 14:21 UTC (permalink / raw)
  To: cygwin-patches

On Jul 22 14:53, Jon Turney wrote:
> On 21/07/2021 09:19, Corinna Vinschen wrote:
> > On Jul 19 17:31, Jon Turney wrote:
> > > I'm not sure this is the best idea, since it adds more configurations that
> > > aren't going to get tested often, but the idea is that this would enable
> > > proper and consistent control of the symlink type used from setup, as
> > > discussed in [1].
> > > 
> > > [1] https://cygwin.com/pipermail/cygwin-apps/2021-May/041327.html
> > 
> > Why isn't it sufficient to use 'winsymlinks:native' from setup?
> 
> I think in the default Windows configuration (developer mode off, no
> SeCreateSymbolicLinkPrivilege), 'native' will try to create a native symlink
> and fail, and fallback to WSL IO_REPARSE_TAG_LX_SYMLINK reparse point, then
> magic cookie + sys attribute.
> 
> This leads to cygwin installations with WSL symlinks created by post-install
> scripts, which can't be put into Docker containers [1], which is the
> original problem I was trying to fix.
> 
> [1] https://cygwin.com/pipermail/cygwin/2020-August/245994.html

Did nobody ask the Docker guys why they fail to support perfectly
valid reparse points?

> I haven't yet looked at adding 'native' symlink support to setup itself, but
> it's probably going to be a bit of a pain.

That may be not a bad idea after all.  Setup typically runs as elevated
process, so it has the required permissions to create native symlinks.
Scripts could then run with CYGWIN=winsymlinks:native by default.

As long as nobody has the hare-brained idea to move a Cygwin distro
manually, native symlinks should be just as well as Cygwin symlinks.

> > The way we express symlinks shouldn't be a user choice, really.  The
> > winsymlinks thingy was only ever introduced in a desperate attempt to
> > improve access to symlinks from native tools, and I still don't see a
> > way around that.  But either way, what's the advantage in allowing the
> > user complete control over the type, even if the type is only useful in
> > Cygwin?
>  If we can come up with a fixed policy that works everywhere, there is no
> advantage.  But that seems unlikely :)
> 
> I could buy an argument that 'native' should be the default (although maybe
> all that does is slow things down in the majority of installs?).

It may slow down installations a tiny little bit because the target
paths have to be converted to POSIX, but I doubt this is more than just
a marginal slowdown.


Corinna

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Add more winsymlinks values
  2021-07-22 14:21     ` Corinna Vinschen
@ 2021-07-28 19:55       ` Jon Turney
  2021-07-29 10:23         ` Corinna Vinschen
  0 siblings, 1 reply; 12+ messages in thread
From: Jon Turney @ 2021-07-28 19:55 UTC (permalink / raw)
  To: Cygwin Patches

On 22/07/2021 15:21, Corinna Vinschen wrote:
> On Jul 22 14:53, Jon Turney wrote:
>> On 21/07/2021 09:19, Corinna Vinschen wrote:
>>> On Jul 19 17:31, Jon Turney wrote:
>>>> I'm not sure this is the best idea, since it adds more configurations that
>>>> aren't going to get tested often, but the idea is that this would enable
>>>> proper and consistent control of the symlink type used from setup, as
>>>> discussed in [1].
>>>>
>>>> [1] https://cygwin.com/pipermail/cygwin-apps/2021-May/041327.html
>>>
>>> Why isn't it sufficient to use 'winsymlinks:native' from setup?
>>
>> I think in the default Windows configuration (developer mode off, no
>> SeCreateSymbolicLinkPrivilege), 'native' will try to create a native symlink
>> and fail, and fallback to WSL IO_REPARSE_TAG_LX_SYMLINK reparse point, then
>> magic cookie + sys attribute.
>>
>> This leads to cygwin installations with WSL symlinks created by post-install
>> scripts, which can't be put into Docker containers [1], which is the
>> original problem I was trying to fix.
>>
>> [1] https://cygwin.com/pipermail/cygwin/2020-August/245994.html
> 
> Did nobody ask the Docker guys why they fail to support perfectly
> valid reparse points?

It seems so e.g. [1]. The answer isn't much beyond "yes, that doesn't 
work", though.

[1] https://github.com/moby/moby/issues/41058#issuecomment-692968944

>> I haven't yet looked at adding 'native' symlink support to setup itself, but
>> it's probably going to be a bit of a pain.
> 
> That may be not a bad idea after all.  Setup typically runs as elevated
> process, so it has the required permissions to create native symlinks.
> Scripts could then run with CYGWIN=winsymlinks:native by default.
> 
> As long as nobody has the hare-brained idea to move a Cygwin distro
> manually, native symlinks should be just as well as Cygwin symlinks.

I'm pretty reluctant to add this to setup in any form which isn't 
initially  "keep doing what we currently do, unless you explicitly ask 
for symlinks to be made a different way".  (especially since when we 
changed what we were doing in Cygwin 3.1.5, it opened this whole can of 
worms)

So I don't think that gets us any further forward if setup doesn't have 
useful control over the kinds of symlinks made by post-install scripts.

>>> The way we express symlinks shouldn't be a user choice, really.  The
>>> winsymlinks thingy was only ever introduced in a desperate attempt to
>>> improve access to symlinks from native tools, and I still don't see a
>>> way around that.  But either way, what's the advantage in allowing the
>>> user complete control over the type, even if the type is only useful in
>>> Cygwin?
 >>
>> If we can come up with a fixed policy that works everywhere, there is no
>> advantage.  But that seems unlikely :)
>>
>> I could buy an argument that 'native' should be the default (although maybe
>> all that does is slow things down in the majority of installs?).
> 
> It may slow down installations a tiny little bit because the target
> paths have to be converted to POSIX, but I doubt this is more than just
> a marginal slowdown.

My assumption was that "the majority of installs" are running where 
native symlink creation isn't permitted, so the slowdown I meant was 
that adds "try to create a native symlink, fail and fallback" for every 
symlink creation.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Add more winsymlinks values
  2021-07-28 19:55       ` Jon Turney
@ 2021-07-29 10:23         ` Corinna Vinschen
  2021-07-29 10:40           ` Corinna Vinschen
  0 siblings, 1 reply; 12+ messages in thread
From: Corinna Vinschen @ 2021-07-29 10:23 UTC (permalink / raw)
  To: cygwin-patches

On Jul 28 20:55, Jon Turney wrote:
> On 22/07/2021 15:21, Corinna Vinschen wrote:
> > On Jul 22 14:53, Jon Turney wrote:
> > > [1] https://cygwin.com/pipermail/cygwin/2020-August/245994.html
> > 
> > Did nobody ask the Docker guys why they fail to support perfectly
> > valid reparse points?
> 
> It seems so e.g. [1]. The answer isn't much beyond "yes, that doesn't work",
> though.
> 
> [1] https://github.com/moby/moby/issues/41058#issuecomment-692968944

D'oh!

> > > I haven't yet looked at adding 'native' symlink support to setup itself, but
> > > it's probably going to be a bit of a pain.
> > 
> > That may be not a bad idea after all.  Setup typically runs as elevated
> > process, so it has the required permissions to create native symlinks.
> > Scripts could then run with CYGWIN=winsymlinks:native by default.
> > 
> > As long as nobody has the hare-brained idea to move a Cygwin distro
> > manually, native symlinks should be just as well as Cygwin symlinks.
> 
> I'm pretty reluctant to add this to setup in any form which isn't initially
> "keep doing what we currently do, unless you explicitly ask for symlinks to
> be made a different way".  (especially since when we changed what we were
> doing in Cygwin 3.1.5, it opened this whole can of worms)
> 
> So I don't think that gets us any further forward if setup doesn't have
> useful control over the kinds of symlinks made by post-install scripts.

Ok, then, by all means, lets' add a few options to the CYGWIN=winsymlinks
setting.  Just s/WSYM_magic/WSYM_sysfile/.

> >>
> > > If we can come up with a fixed policy that works everywhere, there is no
> > > advantage.  But that seems unlikely :)
> > > 
> > > I could buy an argument that 'native' should be the default (although maybe
> > > all that does is slow things down in the majority of installs?).
> > 
> > It may slow down installations a tiny little bit because the target
> > paths have to be converted to POSIX, but I doubt this is more than just
> > a marginal slowdown.
> 
> My assumption was that "the majority of installs" are running where native
> symlink creation isn't permitted, so the slowdown I meant was that adds "try
> to create a native symlink, fail and fallback" for every symlink creation.

Uh, ok.  That's avoidable, though.  The Cygwin code is lacking a check
for SeCreateSymbolicLinkPrivilege being present and enabled in the
current user token.  This could be done once and then stored in a static
var for later consumption.


Corinna

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Add more winsymlinks values
  2021-07-29 10:23         ` Corinna Vinschen
@ 2021-07-29 10:40           ` Corinna Vinschen
  0 siblings, 0 replies; 12+ messages in thread
From: Corinna Vinschen @ 2021-07-29 10:40 UTC (permalink / raw)
  To: cygwin-patches

On Jul 29 12:23, Corinna Vinschen wrote:
> On Jul 28 20:55, Jon Turney wrote:
> > On 22/07/2021 15:21, Corinna Vinschen wrote:
> > > On Jul 22 14:53, Jon Turney wrote:
> > > > [1] https://cygwin.com/pipermail/cygwin/2020-August/245994.html
> > > 
> > > Did nobody ask the Docker guys why they fail to support perfectly
> > > valid reparse points?
> > 
> > It seems so e.g. [1]. The answer isn't much beyond "yes, that doesn't work",
> > though.
> > 
> > [1] https://github.com/moby/moby/issues/41058#issuecomment-692968944
> 
> D'oh!
> 
> > > > I haven't yet looked at adding 'native' symlink support to setup itself, but
> > > > it's probably going to be a bit of a pain.
> > > 
> > > That may be not a bad idea after all.  Setup typically runs as elevated
> > > process, so it has the required permissions to create native symlinks.
> > > Scripts could then run with CYGWIN=winsymlinks:native by default.
> > > 
> > > As long as nobody has the hare-brained idea to move a Cygwin distro
> > > manually, native symlinks should be just as well as Cygwin symlinks.
> > 
> > I'm pretty reluctant to add this to setup in any form which isn't initially
> > "keep doing what we currently do, unless you explicitly ask for symlinks to
> > be made a different way".  (especially since when we changed what we were
> > doing in Cygwin 3.1.5, it opened this whole can of worms)
> > 
> > So I don't think that gets us any further forward if setup doesn't have
> > useful control over the kinds of symlinks made by post-install scripts.
> 
> Ok, then, by all means, lets' add a few options to the CYGWIN=winsymlinks
> setting.  Just s/WSYM_magic/WSYM_sysfile/.

Also winsymlinks:sys or winsymlinks:sysfile

Thanks,
Corinna

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-07-29 10:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-19 16:31 [PATCH 0/3] Add more winsymlinks values Jon Turney
2021-07-19 16:31 ` [PATCH 1/3] Rename WSYM_sysfile to WSM_default Jon Turney
2021-07-19 16:31 ` [PATCH 2/3] Add winsymlinks:magic Jon Turney
2021-07-19 16:31 ` [PATCH 3/3] Add winsymlinks:wslstrict Jon Turney
2021-07-21  8:19 ` [PATCH 0/3] Add more winsymlinks values Corinna Vinschen
2021-07-21 10:24   ` Christian Franke
2021-07-22  8:03     ` Corinna Vinschen
2021-07-22 13:53   ` Jon Turney
2021-07-22 14:21     ` Corinna Vinschen
2021-07-28 19:55       ` Jon Turney
2021-07-29 10:23         ` Corinna Vinschen
2021-07-29 10:40           ` Corinna Vinschen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).