[PATCH] Introduce the 'usertemp' filesystem type

[PATCH] Introduce the 'usertemp' filesystem type

[Johannes Schindelin]

	* mount.cc (mount_info::from_fstab_line): support mounting the
	current user's temp folder as /tmp/. This is particularly
	useful a feature when Cygwin's own files are write-protected.

	* pathnames.xml: document the new usertemp file system type

Detailed explanation:

In the context of Windows, there is a per-user directory for temporary
files, by default specified via the environment variable %TEMP%. Let's
allow to use that directory for our /tmp/ directory.

With this patch, we introduce the special filesystem type "usertemp":
By specifying

	none /tmp usertemp binary,posix=0 0 0

in /etc/fstab, the /tmp/ directory gets auto-mounted to the directory
specified by the %TEMP% variable.

This feature comes handy in particularly in scenarios where the
administrator might want to write-protect the entire Cygwin directory
yet still needs to allow users to write into the /tmp/ directory.
This is the case in the context of Git for Windows, where the
Cygwin (MSys2) root directory lives inside C:\Program Files and hence
/tmp/ would not be writable otherwise.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/mount.cc   | 17 +++++++++++++++++
 winsup/doc/pathnames.xml | 24 +++++++++++++++++++++---
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/mount.cc b/winsup/cygwin/mount.cc
index 6cf3ddf..0b3dbdc 100644
--- a/winsup/cygwin/mount.cc
+++ b/winsup/cygwin/mount.cc
@@ -1139,6 +1139,8 @@ mount_info::from_fstab_line (char *line, bool user)
   unsigned mount_flags = MOUNT_SYSTEM | MOUNT_BINARY;
   if (!strcmp (fs_type, "cygdrive"))
     mount_flags |= MOUNT_NOPOSIX;
+  if (!strcmp (fs_type, "usertemp"))
+    mount_flags |= MOUNT_IMMUTABLE;
   if (!fstab_read_flags (&c, mount_flags, false))
     return true;
   if (mount_flags & MOUNT_BIND)
@@ -1163,6 +1165,21 @@ mount_info::from_fstab_line (char *line, bool user)
       slashify (posix_path, cygdrive, 1);
       cygdrive_len = strlen (cygdrive);
     }
+  else if (!strcmp (fs_type, "usertemp"))
+    {
+      WCHAR tmp[MAX_PATH];
+
+      if (GetEnvironmentVariableW (L"TEMP", tmp, sizeof(tmp)) && *tmp)
+	{
+          DWORD len;
+          char mb_tmp[len = sys_wcstombs (NULL, 0, tmp)];
+          sys_wcstombs (mb_tmp, len, tmp);
+
+	  int res = mount_table->add_item (mb_tmp, posix_path, mount_flags);
+	  if (res && get_errno () == EMFILE)
+	    return false;
+	}
+    }
   else
     {
       int res = mount_table->add_item (native_path, posix_path, mount_flags);
diff --git a/winsup/doc/pathnames.xml b/winsup/doc/pathnames.xml
index cdbf9fa..166c504 100644
--- a/winsup/doc/pathnames.xml
+++ b/winsup/doc/pathnames.xml
@@ -74,9 +74,10 @@ doesn't matter if you write <literal>FAT</literal> into this field even if
 the filesystem is NTFS.  Cygwin figures out the filesystem type and its
 capabilities by itself.</para>
 
-<para>The only exception is the file system type cygdrive.  This type is
-used to set the cygdrive prefix.  For a description of the cygdrive prefix
-see <xref linkend="cygdrive"></xref></para>
+<para>The only two exceptions are the file system types cygdrive and usertemp.
+The cygdrive type is used to set the cygdrive prefix.  For a description of
+the cygdrive prefix see <xref linkend="cygdrive"></xref>, for a description of
+the usertemp file system type see <xref linkend="usertemp"></xref></para>
 
 <para>The fourth field describes the mount options associated
 with the filesystem.  It is formatted as a comma separated list of
@@ -354,6 +355,23 @@ independently from the current cygdrive prefix:</para>
 
 </sect2>
 
+<sect2 id="usertemp"><title>The usertemp file system type</title>
+
+<para>On Windows, the environment variable <literal>TEMP</literal> specifies
+the location of the temp folder.  It serves the same purpose as the /tmp/
+directory in Unix systems.  In contrast to /tmp/, it is by default a
+different folder for every Windows.  By using the special purpose usertemp
+file system, that temp folder can be mapped to /tmp/.  This is particularly
+useful in setups where the administrator wants to write-protect the entire
+Cygwin directory.  The usertemp file system can be configured in /etc/fstab
+like this:</para>
+
+<screen>
+  none /tmp usertemp binary,posix=0 0 0
+</screen>
+
+</sect2>
+
 <sect2 id="pathnames-symlinks"><title>Symbolic links</title>
 
 <para>Symbolic links are not present and supported on Windows until Windows
-- 
2.5.2.windows.2

Re: [PATCH] Introduce the 'usertemp' filesystem type

[Corinna Vinschen]

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

Hi Johannes,


Preliminaries: we need a copyright assignment from you before being able
to include your patches.  Please see https://cygwin.com/assign.txt.

On Sep 16 09:35, Johannes Schindelin wrote:
> 	* mount.cc (mount_info::from_fstab_line): support mounting the
> 	current user's temp folder as /tmp/. This is particularly
> 	useful a feature when Cygwin's own files are write-protected.
> 
> 	* pathnames.xml: document the new usertemp file system type
> 
> Detailed explanation:
> 
> In the context of Windows, there is a per-user directory for temporary
> files, by default specified via the environment variable %TEMP%. Let's
> allow to use that directory for our /tmp/ directory.
> 
> With this patch, we introduce the special filesystem type "usertemp":
> By specifying
> 
> 	none /tmp usertemp binary,posix=0 0 0
> 
> in /etc/fstab, the /tmp/ directory gets auto-mounted to the directory
> specified by the %TEMP% variable.

In theory you could also utilize /etc/fstab.d/$USER, without the need to
implement a usertemp mount type.

> This feature comes handy in particularly in scenarios where the
> administrator might want to write-protect the entire Cygwin directory
> yet still needs to allow users to write into the /tmp/ directory.
> This is the case in the context of Git for Windows, where the
> Cygwin (MSys2) root directory lives inside C:\Program Files and hence
> /tmp/ would not be writable otherwise.

You are aware that this breaks interoperability in some cases where
files are shared in /tmp, right?  It's very important to stress the fact
that one user's /tmp is not the same as another user's /tmp when working
on the same machine in this scenario, e.g. via terminal services.
X server sockets won't be in the expected place, etc.  Also, what about
/var/tmp, /var/log, /var/run, /dev/mqueue, /dev/shm?

Creating this kind of mount type only solves part of the problem in this
scenario.  If the Admins insist to install the Cygwin directory
structure in an unexpected way, a full solution appears to be much more
extensive.

Wouldn't it be simpler to install a generic, writable temp dir and just
point to it via standard mount point?

As for the patch itself:

- The ChangeLog entry is missing.

> diff --git a/winsup/cygwin/mount.cc b/winsup/cygwin/mount.cc
> index 6cf3ddf..0b3dbdc 100644
> --- a/winsup/cygwin/mount.cc
> +++ b/winsup/cygwin/mount.cc
> @@ -1139,6 +1139,8 @@ mount_info::from_fstab_line (char *line, bool user)
>    unsigned mount_flags = MOUNT_SYSTEM | MOUNT_BINARY;
>    if (!strcmp (fs_type, "cygdrive"))
>      mount_flags |= MOUNT_NOPOSIX;
> +  if (!strcmp (fs_type, "usertemp"))
> +    mount_flags |= MOUNT_IMMUTABLE;
>    if (!fstab_read_flags (&c, mount_flags, false))
>      return true;
>    if (mount_flags & MOUNT_BIND)
> @@ -1163,6 +1165,21 @@ mount_info::from_fstab_line (char *line, bool user)
>        slashify (posix_path, cygdrive, 1);
>        cygdrive_len = strlen (cygdrive);
>      }
> +  else if (!strcmp (fs_type, "usertemp"))
> +    {
> +      WCHAR tmp[MAX_PATH];
> +
> +      if (GetEnvironmentVariableW (L"TEMP", tmp, sizeof(tmp)) && *tmp)

- Maybe using GetTempPathW instead?  It always returns a path.

> +	{
> +          DWORD len;
> +          char mb_tmp[len = sys_wcstombs (NULL, 0, tmp)];

- len = sys_wcstombs() + 1

  Alternatively, use tmp_pathbuf:

  tmp_pathbuf tp;
  char mb_tmp = tp.c_get ();

> +          sys_wcstombs (mb_tmp, len, tmp);
> +
> +	  int res = mount_table->add_item (mb_tmp, posix_path, mount_flags);
> +	  if (res && get_errno () == EMFILE)
> +	    return false;
> +	}
> +    }
>    else
>      {
>        int res = mount_table->add_item (native_path, posix_path, mount_flags);

- What about adding a mount flags to allow fillout_mntent to print out
  the mount type?  This isn't essential, I'm just wondering if there
  might be a good reason to see the type in mount(1) output.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Introduce the 'usertemp' filesystem type

[Johannes Schindelin]

Hi Corinna,

On Tue, 20 Oct 2015, Corinna Vinschen wrote:

> Preliminaries: we need a copyright assignment from you before being able
> to include your patches.  Please see https://cygwin.com/assign.txt.

Thanks for pointing that out. It is in the works.

> On Sep 16 09:35, Johannes Schindelin wrote:
> > 	* mount.cc (mount_info::from_fstab_line): support mounting the
> > 	current user's temp folder as /tmp/. This is particularly
> > 	useful a feature when Cygwin's own files are write-protected.
> > 
> > 	* pathnames.xml: document the new usertemp file system type

BTW I thought this would be the preferred form of the ChangeLog entry: as
part of the commit message. At least that is how I interpreted this part:

	ChangeLogs should not be sent as "diffs". Just send the complete
	ChangeLog entry, which is ideally part of the output of
	`git format-patch' anyway.

of https://cygwin.com/contrib.html

> > By specifying
> > 
> > 	none /tmp usertemp binary,posix=0 0 0
> > 
> > in /etc/fstab, the /tmp/ directory gets auto-mounted to the directory
> > specified by the %TEMP% variable.
> 
> In theory you could also utilize /etc/fstab.d/$USER, without the need to
> implement a usertemp mount type.

This is unfortunately not possible. The use case that triggered this patch
is Git for Windows (which does not use Cygwin directly, but the MSys2
runtime which is derived from Cygwin). In that case, we really want to
install the program either into C:\Program Files or onto a USB stick and
the /tmp/ directory needs to refer to $TEMP no matter what, even if a
previously unseen user starts it (which of course precludes the use of
/etc/fstab.d/$USER).

> > This feature comes handy in particularly in scenarios where the
> > administrator might want to write-protect the entire Cygwin directory
> > yet still needs to allow users to write into the /tmp/ directory.
> > This is the case in the context of Git for Windows, where the
> > Cygwin (MSys2) root directory lives inside C:\Program Files and hence
> > /tmp/ would not be writable otherwise.
> 
> You are aware that this breaks interoperability in some cases where
> files are shared in /tmp, right?

Yes. These cases do not occur in the context of Git for Windows.

Please also note that I took pains to make this an opt-in feature. It
should not be used by default in any Cygwin installation, but I think it
would be nice for power users to have the *option* to use it.

The thing is: Cygwin's /tmp/ might be reused by processes running as
different users, but the user temp can be reused by processes of the
*same* user, whether the processes are Cygwin or not. Which is another
nice thing to have in some scenarios.

> It's very important to stress the fact that one user's /tmp is not the
> same as another user's /tmp when working on the same machine in this
> scenario, e.g. via terminal services.

Indeed. In Git for Windows' case, this is actually a feature. That way,
different users' files are encapsulated from each other.

> X server sockets won't be in the expected place, etc.  Also, what about
> /var/tmp, /var/log, /var/run, /dev/mqueue, /dev/shm?

Git for Windows is intended to be used as an individual application,
without any daemons. So the idea that programs started by the user need to
write into the pseudo root file system does not apply here.

> Creating this kind of mount type only solves part of the problem in this
> scenario.  If the Admins insist to install the Cygwin directory
> structure in an unexpected way, a full solution appears to be much more
> extensive.

Git for Windows comes with its own installer that already takes care of
the configuration. If any admin insists to do things differently, they are
on their own.

> Wouldn't it be simpler to install a generic, writable temp dir and just
> point to it via standard mount point?

As I said, in a multi-user setting, or even worse: in a portable
application, this is simply not possible other than via the strategy
implemented by this patch.

> - The ChangeLog entry is missing.

See above. Do you want me to include the diff to winsup/cygwin/ChangeLog?

> > diff --git a/winsup/cygwin/mount.cc b/winsup/cygwin/mount.cc
> > index 6cf3ddf..0b3dbdc 100644
> > --- a/winsup/cygwin/mount.cc
> > +++ b/winsup/cygwin/mount.cc
> > @@ -1163,6 +1165,21 @@ mount_info::from_fstab_line (char *line, bool user)
> >        slashify (posix_path, cygdrive, 1);
> >        cygdrive_len = strlen (cygdrive);
> >      }
> > +  else if (!strcmp (fs_type, "usertemp"))
> > +    {
> > +      WCHAR tmp[MAX_PATH];
> > +
> > +      if (GetEnvironmentVariableW (L"TEMP", tmp, sizeof(tmp)) && *tmp)
> 
> - Maybe using GetTempPathW instead?  It always returns a path.

Good idea! It *does* respect the TMP variable, falling back to TEMP and
then USERPROFILE. I was worried for a moment that the user could no longer
override the behavior by setting an environment variable explicitly.

> > +	{
> > +          DWORD len;
> > +          char mb_tmp[len = sys_wcstombs (NULL, 0, tmp)];
> 
> - len = sys_wcstombs() + 1

Whoops. I always get that wrong.

But... actually... Did you know that `sys_wcstombs()` returns something
different than advertised? The documentation says:

	- dst == NULL; len is ignored, the return value is the number
	  of bytes required for the string without the trailing NUL, just
	  like the return value of the wcstombs function.

But when I call

	small_printf("len of 1: %d\n", sys_wcstombs(NULL, 0, L"1"));

it prints "len of 1: 2", i.e. the number of bytes requires for the string
*with* the trailing NUL, disagreeing with the comment in strfuncs.cc.

So that is why this patch works.

BTW I realized that the 3-parameter version of sys_wcstombs() does
something different than advertised when working on a fix for the other
patch I submitted to cygwin-patches:

	https://github.com/git-for-windows/msys2-runtime/commit/e87aaa7d5

How do you want to proceed from here? Should I fix sys_wcstombs() when the
fourth parameter is -1? Or is this not a fix, but I would rather break
things?

>   Alternatively, use tmp_pathbuf:
> 
>   tmp_pathbuf tp;
>   char mb_tmp = tp.c_get ();

Sure, that would work, too.

> > +          sys_wcstombs (mb_tmp, len, tmp);
> > +
> > +	  int res = mount_table->add_item (mb_tmp, posix_path, mount_flags);
> > +	  if (res && get_errno () == EMFILE)
> > +	    return false;
> > +	}
> > +    }
> >    else
> >      {
> >        int res = mount_table->add_item (native_path, posix_path, mount_flags);
> 
> - What about adding a mount flags to allow fillout_mntent to print out
>   the mount type?  This isn't essential, I'm just wondering if there
>   might be a good reason to see the type in mount(1) output.

You mean something like this?

-- snip --
diff --git a/winsup/cygwin/include/sys/mount.h
b/winsup/cygwin/include/sys/mount.h
index 458cf80..ec92794 100644
--- a/winsup/cygwin/include/sys/mount.h
+++ b/winsup/cygwin/include/sys/mount.h
@@ -44,7 +44,8 @@ enum
   MOUNT_DOS =		0x40000,	/* convert leading spaces and
trailing
 					   dots and spaces to private use
area */
   MOUNT_IHASH =		0x80000,	/* Enforce hash values for
inode numbers */
-  MOUNT_BIND =		0x100000	/* Allows bind syntax in fstab
   file. */
+  MOUNT_BIND =		0x100000,	/* Allows bind syntax in fstab
file. */
+  MOUNT_USER_TEMP =	0x200000	/* Mount the user's $TMP. */
 };
 
 int mount (const char *, const char *, unsigned __flags);
diff --git a/winsup/cygwin/mount.cc b/winsup/cygwin/mount.cc
index e37ad16..4340cc4 100644
--- a/winsup/cygwin/mount.cc
+++ b/winsup/cygwin/mount.cc
@@ -1184,6 +1184,7 @@ mount_info::from_fstab_line (char *line, bool user)
           char mb_tmp[len = sys_wcstombs (NULL, 0, tmp)];
           sys_wcstombs (mb_tmp, len, tmp);
 
+	  mount_flags |= MOUNT_USER_TEMP;
 	  int res = mount_table->add_item (mb_tmp, posix_path,
mount_flags);
 	  if (res && get_errno () == EMFILE)
 	    return false;
@@ -1771,6 +1772,9 @@ fillout_mntent (const char *native_path, const char
*posix_path, unsigned flags)
   if (flags & (MOUNT_BIND))
     strcat (_my_tls.locals.mnt_opts, (char *) ",bind");
 
+  if (flags & (MOUNT_USER_TEMP))
+    strcat (_my_tls.locals.mnt_opts, (char *) ",usertemp");
+
   ret.mnt_opts = _my_tls.locals.mnt_opts;
 
   ret.mnt_freq = 1;
-- snap --

Yep, good idea!

Thank you,
Johannes

Re: [PATCH] Introduce the 'usertemp' filesystem type

[Corinna Vinschen]

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

Hi Johannes,

On Oct 20 13:47, Johannes Schindelin wrote:
> On Tue, 20 Oct 2015, Corinna Vinschen wrote:
> > On Sep 16 09:35, Johannes Schindelin wrote:
> > > 	* mount.cc (mount_info::from_fstab_line): support mounting the
> > > 	current user's temp folder as /tmp/. This is particularly
> > > 	useful a feature when Cygwin's own files are write-protected.
> > > 
> > > 	* pathnames.xml: document the new usertemp file system type
> 
> BTW I thought this would be the preferred form of the ChangeLog entry: as
> part of the commit message. At least that is how I interpreted this part:
> 
> 	ChangeLogs should not be sent as "diffs". Just send the complete
> 	ChangeLog entry, which is ideally part of the output of
> 	`git format-patch' anyway.
> 
> of https://cygwin.com/contrib.html

Duh, I missed that from your OP.  Somehow I started reading with
"Detailed explanation:".  Sorry about that.

> > > By specifying
> > > 
> > > 	none /tmp usertemp binary,posix=0 0 0
> > > 
> > > in /etc/fstab, the /tmp/ directory gets auto-mounted to the directory
> > > specified by the %TEMP% variable.
> > 
> > In theory you could also utilize /etc/fstab.d/$USER, without the need to
> > implement a usertemp mount type.
> 
> This is unfortunately not possible. The use case that triggered this patch
> is Git for Windows (which does not use Cygwin directly, but the MSys2
> runtime which is derived from Cygwin).

Editorial note: 

It's a bit hard to understand why we need Git for Windows while there's
a full fledged git available as part of the Cygwin distro.  It's also
very frustrating that a Git for Windows standalone tool gets a lot of
press coverage while nobody seems to care that Git for Windows under
Cygwin exists for ages.  Sigh.

> Indeed. In Git for Windows' case, this is actually a feature. That way,
> different users' files are encapsulated from each other.
> [...]
> As I said, in a multi-user setting, or even worse: in a portable
> application, this is simply not possible other than via the strategy
> implemented by this patch.

Here's a question.  If it's really only about git, why do you need
to redirect /tmp, rather than having git use $TMP directly?  That would
be much less intrusive than having to change the underlying POSIX
layer, isn't it?

> > - The ChangeLog entry is missing.
> 
> See above. Do you want me to include the diff to winsup/cygwin/ChangeLog?

No, my bad, sorry.

> [...]
> > > +          char mb_tmp[len = sys_wcstombs (NULL, 0, tmp)];
> > 
> > - len = sys_wcstombs() + 1
> 
> Whoops. I always get that wrong.
> 
> But... actually... Did you know that `sys_wcstombs()` returns something
> different than advertised? The documentation says:
> 
> 	- dst == NULL; len is ignored, the return value is the number
> 	  of bytes required for the string without the trailing NUL, just
> 	  like the return value of the wcstombs function.
> 
> But when I call
> 
> 	small_printf("len of 1: %d\n", sys_wcstombs(NULL, 0, L"1"));
> 
> it prints "len of 1: 2", i.e. the number of bytes requires for the string
> *with* the trailing NUL, disagreeing with the comment in strfuncs.cc.

Drat.  You're right.  As usual I wonder why nobody ever noticed this.
As soon as the nwc parameter is larger than the number of non-0 wchars
in the source string, sys_cp_wcstombs returns the length including the
trailing NUL.

And looking through the Cygwin sources the usage is rather erratic,
sometimes with, sometimes without + 1 :(

> How do you want to proceed from here? Should I fix sys_wcstombs() when the
> fourth parameter is -1? Or is this not a fix, but I would rather break
> things?

No, this needs fixing, but it also would break things.  I have to take
a stab at fixing this throughout Cygwin first.

> > - What about adding a mount flags to allow fillout_mntent to print out
> >   the mount type?  This isn't essential, I'm just wondering if there
> >   might be a good reason to see the type in mount(1) output.
> 
> You mean something like this?
> 
> -- snip --

Yes, that's what I had in mind.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Introduce the 'usertemp' filesystem type

[Corinna Vinschen]

On Oct 21 20:23, Corinna Vinschen wrote:
> On Oct 20 13:47, Johannes Schindelin wrote:
> > On Tue, 20 Oct 2015, Corinna Vinschen wrote:
> > > On Sep 16 09:35, Johannes Schindelin wrote:
> > [...]
> > > > +          char mb_tmp[len = sys_wcstombs (NULL, 0, tmp)];
> > > 
> > > - len = sys_wcstombs() + 1
> > 
> > Whoops. I always get that wrong.
> > 
> > But... actually... Did you know that `sys_wcstombs()` returns something
> > different than advertised? The documentation says:
> > 
> > 	- dst == NULL; len is ignored, the return value is the number
> > 	  of bytes required for the string without the trailing NUL, just
> > 	  like the return value of the wcstombs function.
> > 
> > But when I call
> > 
> > 	small_printf("len of 1: %d\n", sys_wcstombs(NULL, 0, L"1"));
> > 
> > it prints "len of 1: 2", i.e. the number of bytes requires for the string
> > *with* the trailing NUL, disagreeing with the comment in strfuncs.cc.
> 
> Drat.  You're right.  As usual I wonder why nobody ever noticed this.
> As soon as the nwc parameter is larger than the number of non-0 wchars
> in the source string, sys_cp_wcstombs returns the length including the
> trailing NUL.
> 
> And looking through the Cygwin sources the usage is rather erratic,
> sometimes with, sometimes without + 1 :(
> 
> > How do you want to proceed from here? Should I fix sys_wcstombs() when the
> > fourth parameter is -1? Or is this not a fix, but I would rather break
> > things?
> 
> No, this needs fixing, but it also would break things.  I have to take
> a stab at fixing this throughout Cygwin first.

I just pushed a patch to the git repo supposed to fix sys_cp_wcstombs
return value inconsistency.  It should now always return the length of
the string without the trailing NUL.  Please give it a try.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Introduce the 'usertemp' filesystem type

[Johannes Schindelin]

Hi Corinna,

On Wed, 21 Oct 2015, Corinna Vinschen wrote:

> On Oct 20 13:47, Johannes Schindelin wrote:
> > On Tue, 20 Oct 2015, Corinna Vinschen wrote:
> > > On Sep 16 09:35, Johannes Schindelin wrote:
> > >
> > > > By specifying
> > > > 
> > > > 	none /tmp usertemp binary,posix=0 0 0
> > > > 
> > > > in /etc/fstab, the /tmp/ directory gets auto-mounted to the
> > > > directory specified by the %TEMP% variable.
> > > 
> > > In theory you could also utilize /etc/fstab.d/$USER, without the
> > > need to implement a usertemp mount type.
> > 
> > This is unfortunately not possible. The use case that triggered this
> > patch is Git for Windows (which does not use Cygwin directly, but the
> > MSys2 runtime which is derived from Cygwin).
> 
> Editorial note: 
> 
> It's a bit hard to understand why we need Git for Windows while there's
> a full fledged git available as part of the Cygwin distro.  It's also
> very frustrating that a Git for Windows standalone tool gets a lot of
> press coverage while nobody seems to care that Git for Windows under
> Cygwin exists for ages.  Sigh.

I actually used Cygwin Git myself. However, I had to work with a large
project and the POSIX emulation just put a serious dent into my
productivity.

Don't get me wrong: I love what Cygwin does. It is just that for using Git
efficiently, we needed to go directly to the Win32 API. So Git for Windows
is substantially faster, and takes less resources, than Cygwin Git.

Take for example the good old stat() call. If you need to run `git
status`, you require the modified time of every tracked file, so you call
stat() on it, right? There is an equivalent call in the Win32 API, but it
does not quite get as much information as the stat() call. So to emulate
the stat() call on Windows, we need to call additional Win32 API functions
to fill in those data. The really stupidly annoying thing about this is
that quite often, Git does not even care about those data and just throws
them away!

Another thing is that Git often calls external processes. The POSIX call
is `fork()`. But `fork()` does so much more than spawn a process: it
emulates the copy-on-write feature and also copies the file descriptors
and all that, but Git does not care and ignores all that and all the
effort was spent for naught.

That is why Git for Windows is so much faster than Cygwin Git.

It will get even faster in the future, when we convert more scripts into
builtins, because the shell we use is of course the venerable Bash, using
Cygwin's excellent fork() emulation.

So please understand that I am very, very thankful that Cygwin exists and
is maintained and developed further. At the same time, I think it is
important to develop Git for Windows further, to improve the user
experience.

> > Indeed. In Git for Windows' case, this is actually a feature. That way,
> > different users' files are encapsulated from each other.
> > [...]
> > As I said, in a multi-user setting, or even worse: in a portable
> > application, this is simply not possible other than via the strategy
> > implemented by this patch.
> 
> Here's a question.  If it's really only about git, why do you need
> to redirect /tmp, rather than having git use $TMP directly?

Ah, if it only was that easy! Then I would not bother you at all and do
exactly what you suggested.

The truth, however, is that Git expects quite a few utilities to be
available, utilities that are usually present in any Unix-like system. But
not on Windows. So we need to rely on tools that are shipped with MSys2 to
perform those tasks. One such tool is ssh-agent which *does* want to write
into `/tmp/` (see https://github.com/git-for-windows/git/issues/86).

At the same time, we really have to install our pseudo root file system
into `C:\Program Files` because this is the way things work on Windows. So
we *have* to mount `/tmp/`, and what better way than the Windows way: use
the dedicated per-user temporary directory? That is the way things work on
Windows.

> That would be much less intrusive than having to change the underlying
> POSIX layer, isn't it?

It would be less intrusive, I agree. But it would also not solve the
problem, so it is not a solution ;-)

> > [...]
> > > > +          char mb_tmp[len = sys_wcstombs (NULL, 0, tmp)];
> > > 
> > > - len = sys_wcstombs() + 1
> > 
> > Whoops. I always get that wrong.
> > 
> > But... actually... Did you know that `sys_wcstombs()` returns something
> > different than advertised? The documentation says:
> > 
> > 	- dst == NULL; len is ignored, the return value is the number
> > 	  of bytes required for the string without the trailing NUL, just
> > 	  like the return value of the wcstombs function.
> > 
> > But when I call
> > 
> > 	small_printf("len of 1: %d\n", sys_wcstombs(NULL, 0, L"1"));
> > 
> > it prints "len of 1: 2", i.e. the number of bytes requires for the string
> > *with* the trailing NUL, disagreeing with the comment in strfuncs.cc.
> 
> Drat.  You're right.  As usual I wonder why nobody ever noticed this.
> As soon as the nwc parameter is larger than the number of non-0 wchars
> in the source string, sys_cp_wcstombs returns the length including the
> trailing NUL.
> 
> And looking through the Cygwin sources the usage is rather erratic,
> sometimes with, sometimes without + 1 :(

Thanks for confirming!

> > How do you want to proceed from here? Should I fix sys_wcstombs() when
> > the fourth parameter is -1? Or is this not a fix, but I would rather
> > break things?
> 
> No, this needs fixing, but it also would break things.  I have to take a
> stab at fixing this throughout Cygwin first.

I saw your patch and will try to rework my patch as soon as possible so
that I can resubmit once the paperwork re: copyright is done.

Thank you,
Johannes

[PATCH v2] Introduce the 'usertemp' filesystem type

[Johannes Schindelin]

	* mount.cc (mount_info::from_fstab_line): support mounting the
	current user's temp folder as /tmp/. This is particularly
	useful a feature when Cygwin's own files are write-protected.

	* pathnames.xml: document the new usertemp file system type

Detailed explanation:

In the context of Windows, there is a per-user directory for temporary
files, by default specified via the environment variable %TEMP%. Let's
allow to use that directory for our /tmp/ directory.

With this patch, we introduce the special filesystem type "usertemp":
By specifying

	none /tmp usertemp binary,posix=0 0 0

in /etc/fstab, the /tmp/ directory gets auto-mounted to the directory
specified by the %TEMP% variable.

This feature comes handy in particularly in scenarios where the
administrator might want to write-protect the entire Cygwin directory
yet still needs to allow users to write into the /tmp/ directory.
This is the case in the context of Git for Windows, where the
Cygwin (MSys2) root directory lives inside C:\Program Files and hence
/tmp/ would not be writable otherwise.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/include/sys/mount.h |  3 ++-
 winsup/cygwin/mount.cc            | 21 +++++++++++++++++++++
 winsup/doc/pathnames.xml          | 24 +++++++++++++++++++++---
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/winsup/cygwin/include/sys/mount.h b/winsup/cygwin/include/sys/mount.h
index 458cf80..ec92794 100644
--- a/winsup/cygwin/include/sys/mount.h
+++ b/winsup/cygwin/include/sys/mount.h
@@ -44,7 +44,8 @@ enum
   MOUNT_DOS =		0x40000,	/* convert leading spaces and trailing
 					   dots and spaces to private use area */
   MOUNT_IHASH =		0x80000,	/* Enforce hash values for inode numbers */
-  MOUNT_BIND =		0x100000	/* Allows bind syntax in fstab file. */
+  MOUNT_BIND =		0x100000,	/* Allows bind syntax in fstab file. */
+  MOUNT_USER_TEMP =	0x200000	/* Mount the user's $TMP. */
 };
 
 int mount (const char *, const char *, unsigned __flags);
diff --git a/winsup/cygwin/mount.cc b/winsup/cygwin/mount.cc
index c04805b..fc080c9 100644
--- a/winsup/cygwin/mount.cc
+++ b/winsup/cygwin/mount.cc
@@ -1172,6 +1172,8 @@ mount_info::from_fstab_line (char *line, bool user)
   unsigned mount_flags = MOUNT_SYSTEM | MOUNT_BINARY;
   if (!strcmp (fs_type, "cygdrive"))
     mount_flags |= MOUNT_NOPOSIX;
+  if (!strcmp (fs_type, "usertemp"))
+    mount_flags |= MOUNT_IMMUTABLE;
   if (!fstab_read_flags (&c, mount_flags, false))
     return true;
   if (mount_flags & MOUNT_BIND)
@@ -1196,6 +1198,22 @@ mount_info::from_fstab_line (char *line, bool user)
       slashify (posix_path, cygdrive, 1);
       cygdrive_len = strlen (cygdrive);
     }
+  else if (!strcmp (fs_type, "usertemp"))
+    {
+      WCHAR tmp[PATH_MAX + 1];
+
+      if (GetTempPathW (PATH_MAX, tmp))
+	{
+	  tmp_pathbuf tp;
+	  char *mb_tmp = tp.c_get ();
+	  sys_wcstombs (mb_tmp, PATH_MAX, tmp);
+
+	  mount_flags |= MOUNT_USER_TEMP;
+	  int res = mount_table->add_item (mb_tmp, posix_path, mount_flags);
+	  if (res && get_errno () == EMFILE)
+	    return false;
+	}
+    }
   else
     {
       int res = mount_table->add_item (native_path, posix_path, mount_flags);
@@ -1653,6 +1671,9 @@ fillout_mntent (const char *native_path, const char *posix_path, unsigned flags)
   if (flags & (MOUNT_BIND))
     strcat (_my_tls.locals.mnt_opts, (char *) ",bind");
 
+  if (flags & (MOUNT_USER_TEMP))
+    strcat (_my_tls.locals.mnt_opts, (char *) ",usertemp");
+
   ret.mnt_opts = _my_tls.locals.mnt_opts;
 
   ret.mnt_freq = 1;
diff --git a/winsup/doc/pathnames.xml b/winsup/doc/pathnames.xml
index 9077303..9941633 100644
--- a/winsup/doc/pathnames.xml
+++ b/winsup/doc/pathnames.xml
@@ -74,9 +74,10 @@ doesn't matter if you write <literal>FAT</literal> into this field even if
 the filesystem is NTFS.  Cygwin figures out the filesystem type and its
 capabilities by itself.</para>
 
-<para>The only exception is the file system type cygdrive.  This type is
-used to set the cygdrive prefix.  For a description of the cygdrive prefix
-see <xref linkend="cygdrive"></xref></para>
+<para>The only two exceptions are the file system types cygdrive and usertemp.
+The cygdrive type is used to set the cygdrive prefix.  For a description of
+the cygdrive prefix see <xref linkend="cygdrive"></xref>, for a description of
+the usertemp file system type see <xref linkend="usertemp"></xref></para>
 
 <para>The fourth field describes the mount options associated
 with the filesystem.  It is formatted as a comma separated list of
@@ -354,6 +355,23 @@ independently from the current cygdrive prefix:</para>
 
 </sect2>
 
+<sect2 id="usertemp"><title>The usertemp file system type</title>
+
+<para>On Windows, the environment variable <literal>TEMP</literal> specifies
+the location of the temp folder.  It serves the same purpose as the /tmp/
+directory in Unix systems.  In contrast to /tmp/, it is by default a
+different folder for every Windows.  By using the special purpose usertemp
+file system, that temp folder can be mapped to /tmp/.  This is particularly
+useful in setups where the administrator wants to write-protect the entire
+Cygwin directory.  The usertemp file system can be configured in /etc/fstab
+like this:</para>
+
+<screen>
+  none /tmp usertemp binary,posix=0 0 0
+</screen>
+
+</sect2>
+
 <sect2 id="pathnames-symlinks"><title>Symbolic links</title>
 
 <para>Symbolic links are not present and supported on Windows until Windows
Interdiff vs v1:

diff --git a/winsup/cygwin/include/sys/mount.h b/winsup/cygwin/include/sys/mount.h
index 458cf80..ec92794 100644
--- a/winsup/cygwin/include/sys/mount.h
+++ b/winsup/cygwin/include/sys/mount.h
@@ -44,7 +44,8 @@ enum
   MOUNT_DOS =		0x40000,	/* convert leading spaces and trailing
 					   dots and spaces to private use area */
   MOUNT_IHASH =		0x80000,	/* Enforce hash values for inode numbers */
-  MOUNT_BIND =		0x100000	/* Allows bind syntax in fstab file. */
+  MOUNT_BIND =		0x100000,	/* Allows bind syntax in fstab file. */
+  MOUNT_USER_TEMP =	0x200000	/* Mount the user's $TMP. */
 };
 
 int mount (const char *, const char *, unsigned __flags);
diff --git a/winsup/cygwin/mount.cc b/winsup/cygwin/mount.cc
index be5de61..fc080c9 100644
--- a/winsup/cygwin/mount.cc
+++ b/winsup/cygwin/mount.cc
@@ -1200,14 +1200,15 @@ mount_info::from_fstab_line (char *line, bool user)
     }
   else if (!strcmp (fs_type, "usertemp"))
     {
-      WCHAR tmp[MAX_PATH];
+      WCHAR tmp[PATH_MAX + 1];
 
-      if (GetEnvironmentVariableW (L"TEMP", tmp, sizeof(tmp)) && *tmp)
+      if (GetTempPathW (PATH_MAX, tmp))
 	{
-          DWORD len;
-          char mb_tmp[len = sys_wcstombs (NULL, 0, tmp)];
-          sys_wcstombs (mb_tmp, len, tmp);
+	  tmp_pathbuf tp;
+	  char *mb_tmp = tp.c_get ();
+	  sys_wcstombs (mb_tmp, PATH_MAX, tmp);
 
+	  mount_flags |= MOUNT_USER_TEMP;
 	  int res = mount_table->add_item (mb_tmp, posix_path, mount_flags);
 	  if (res && get_errno () == EMFILE)
 	    return false;
@@ -1670,6 +1671,9 @@ fillout_mntent (const char *native_path, const char *posix_path, unsigned flags)
   if (flags & (MOUNT_BIND))
     strcat (_my_tls.locals.mnt_opts, (char *) ",bind");
 
+  if (flags & (MOUNT_USER_TEMP))
+    strcat (_my_tls.locals.mnt_opts, (char *) ",usertemp");
+
   ret.mnt_opts = _my_tls.locals.mnt_opts;
 
   ret.mnt_freq = 1;

-- 
2.1.4

Re: [PATCH v2] Introduce the 'usertemp' filesystem type

[Johannes Schindelin]

... and here is the interdiff vs v1 (compare also to the 'pseudo Pull
Request' at
https://github.com/dscho/msys2-runtime/compare/tempfs-cygwin-v1-rebased...tempfs-cygwin-v2):

diff --git a/winsup/cygwin/include/sys/mount.h
b/winsup/cygwin/include/sys/mount.h
index 458cf80..ec92794 100644
--- a/winsup/cygwin/include/sys/mount.h
+++ b/winsup/cygwin/include/sys/mount.h
@@ -44,7 +44,8 @@ enum
   MOUNT_DOS =		0x40000,	/* convert leading spaces and trailing
 					   dots and spaces to private use area */
   MOUNT_IHASH =		0x80000,	/* Enforce hash values for inode numbers */
-  MOUNT_BIND =		0x100000	/* Allows bind syntax in fstab file. */
+  MOUNT_BIND =		0x100000,	/* Allows bind syntax in fstab file. */
+  MOUNT_USER_TEMP =	0x200000	/* Mount the user's $TMP. */
 };
 
 int mount (const char *, const char *, unsigned __flags);
diff --git a/winsup/cygwin/mount.cc b/winsup/cygwin/mount.cc
index be5de61..fc080c9 100644
--- a/winsup/cygwin/mount.cc
+++ b/winsup/cygwin/mount.cc
@@ -1200,14 +1200,15 @@ mount_info::from_fstab_line (char *line, bool user)
     }
   else if (!strcmp (fs_type, "usertemp"))
     {
-      WCHAR tmp[MAX_PATH];
+      WCHAR tmp[PATH_MAX + 1];
 
-      if (GetEnvironmentVariableW (L"TEMP", tmp, sizeof(tmp)) && *tmp)
+      if (GetTempPathW (PATH_MAX, tmp))
 	{
-          DWORD len;
-          char mb_tmp[len = sys_wcstombs (NULL, 0, tmp)];
-          sys_wcstombs (mb_tmp, len, tmp);
+	  tmp_pathbuf tp;
+	  char *mb_tmp = tp.c_get ();
+	  sys_wcstombs (mb_tmp, PATH_MAX, tmp);
 
+	  mount_flags |= MOUNT_USER_TEMP;
 	  int res = mount_table->add_item (mb_tmp, posix_path, mount_flags);
 	  if (res && get_errno () == EMFILE)
 	    return false;
@@ -1670,6 +1671,9 @@ fillout_mntent (const char *native_path, const char *posix_path, unsigned flags)
   if (flags & (MOUNT_BIND))
     strcat (_my_tls.locals.mnt_opts, (char *) ",bind");
 
+  if (flags & (MOUNT_USER_TEMP))
+    strcat (_my_tls.locals.mnt_opts, (char *) ",usertemp");
+
   ret.mnt_opts = _my_tls.locals.mnt_opts;
 
   ret.mnt_freq = 1;

Re: [PATCH v2] Introduce the 'usertemp' filesystem type

[Corinna Vinschen]

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

Hi Johannes,

On Dec  1 15:02, Johannes Schindelin wrote:
> 	* mount.cc (mount_info::from_fstab_line): support mounting the
> 	current user's temp folder as /tmp/. This is particularly
> 	useful a feature when Cygwin's own files are write-protected.
> 
> 	* pathnames.xml: document the new usertemp file system type

patch looks good.  We just have to wait for the ok in terms of the
copyright assignment.  Might take a day or two.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2] Introduce the 'usertemp' filesystem type

[Johannes Schindelin]

Hi Corinna,

On Tue, 1 Dec 2015, Corinna Vinschen wrote:

> On Dec  1 15:02, Johannes Schindelin wrote:
> > 	* mount.cc (mount_info::from_fstab_line): support mounting the
> > 	current user's temp folder as /tmp/. This is particularly
> > 	useful a feature when Cygwin's own files are write-protected.
> > 
> > 	* pathnames.xml: document the new usertemp file system type
> 
> patch looks good.  We just have to wait for the ok in terms of the
> copyright assignment.  Might take a day or two.

Sure thing. Thank you for your patience!

Ciao,
Johannes

Re: [PATCH v2] Introduce the 'usertemp' filesystem type

[Corinna Vinschen]

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

On Dec  1 15:50, Johannes Schindelin wrote:
> Hi Corinna,
> 
> On Tue, 1 Dec 2015, Corinna Vinschen wrote:
> 
> > On Dec  1 15:02, Johannes Schindelin wrote:
> > > 	* mount.cc (mount_info::from_fstab_line): support mounting the
> > > 	current user's temp folder as /tmp/. This is particularly
> > > 	useful a feature when Cygwin's own files are write-protected.
> > > 
> > > 	* pathnames.xml: document the new usertemp file system type
> > 
> > patch looks good.  We just have to wait for the ok in terms of the
> > copyright assignment.  Might take a day or two.
> 
> Sure thing. Thank you for your patience!

Thank you in return!  Patch applied.  I'm just going to create a
developer snapshot which should be up at https://cygwin.com/snapshots/
in half an hour at the latest.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]