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