Subject: bug#14295: Support copy-file ACLs for Solaris etc.

From: Eli Zaretskii <[email-masked]>
Date: Sun, 28 Apr 2013 20:16:29 +0300

> Date: Sat, 27 Apr 2013 20:33:19 -0700
> From: Paul Eggert <[email-masked]>
> CC: Eli Zaretskii <[email-masked]>
> 
> Recently copy-file was augmented to add support for
> copying ACLs on abandoned-POSIX-style hosts.  Here's
> an additional patch to add support for copying ACLs on
> Solaris, HP-UX, etc.  Basically, the idea is to use
> Gnulib's support for ACLs.  I've tested this on
> GNU/Linux and Solaris, but not on Microsoft platforms,
> and am CC'ing this to Eli as a heads-up for that.

Thanks; see a few comments below.

> This changes the 'configure' option from --without-acl to
> --disable-acl if one wishes to disable ACL support when
> configuring Emacs; this is the option spelling that other GNU
> packages use.

How hard would it to support both?

> +bool
> +acl_errno_valid (int errnum)
> +{
> +  /* Recognize some common errors such as from an NFS mount that does
> +     not support ACLs, even when local drives do.  */
> +  switch (errnum)
> +    {
> +    case EBUSY: return false;
> +    case EINVAL: return false;
> +#if defined __APPLE__ && defined __MACH__
> +    case ENOENT: return false;
> +#endif
> +    case ENOSYS: return false;
> +#if defined ENOTSUP && ENOTSUP != EOPNOTSUPP
> +    case ENOTSUP: return false;
> +#endif
> +    case EOPNOTSUPP: return false;
> +    default: return true;
> +    }
> +}

This uses EOPNOTSUPP without #ifdef guards; is that universally
available?

>  #ifdef WINDOWSNT
> -  if (!NILP (preserve_extended_attributes))
> -    {
> -#ifdef HAVE_POSIX_ACL
> -      acl = acl_get_file (SDATA (encoded_file), ACL_TYPE_ACCESS);
> -      if (acl == NULL && !ACL_NOT_WELL_SUPPORTED (errno))
> -	report_file_error ("Getting ACL", Fcons (file, Qnil));
> -#endif
> -    }
>    if (!CopyFile (SDATA (encoded_file),
>  		 SDATA (encoded_newname),
>  		 FALSE))
> @@ -2069,17 +2040,6 @@
>        /* Restore original attributes.  */
>        SetFileAttributes (filename, attributes);
>      }
> -#ifdef HAVE_POSIX_ACL
> -  if (acl != NULL)
> -    {
> -      bool fail =
> -	acl_set_file (SDATA (encoded_newname), ACL_TYPE_ACCESS, acl) != 0;
> -      if (fail && !ACL_NOT_WELL_SUPPORTED (errno))
> -	report_file_error ("Setting ACL", Fcons (newname, Qnil));
> -
> -      acl_free (acl);
> -    }
> -#endif
>  #else /* not WINDOWSNT */
>    immediate_quit = 1;
>    ifd = emacs_open (SSDATA (encoded_file), O_RDONLY, 0);
> @@ -2103,12 +2063,6 @@
>  	    report_file_error ("Doing fgetfilecon", Fcons (file, Qnil));
>  	}
>  #endif
> -
> -#ifdef HAVE_POSIX_ACL
> -      acl = acl_get_fd (ifd);
> -      if (acl == NULL && !ACL_NOT_WELL_SUPPORTED (errno))
> -	report_file_error ("Getting ACL", Fcons (file, Qnil));
> -#endif
>      }
>  
>    if (out_st.st_mode != 0
> @@ -2156,7 +2110,7 @@
>    immediate_quit = 0;
>  
>  #ifndef MSDOS
> -  /* Preserve the original file modes, and if requested, also its
> +  /* Preserve the original file permissions, and if requested, also its
>       owner and group.  */
>    {
>      mode_t mode_mask = 07777;
> @@ -2173,8 +2127,16 @@
>  	      mode_mask |= 02000;
>  	  }
>        }
> -    if (fchmod (ofd, st.st_mode & mode_mask) != 0)
> -      report_file_error ("Doing chmod", Fcons (newname, Qnil));
> +
> +    switch (!NILP (preserve_extended_attributes)
> +	    ? qcopy_acl (SSDATA (encoded_file), ifd,
> +			 SSDATA (encoded_newname), ofd,
> +			 st.st_mode & mode_mask)
> +	    : fchmod (ofd, st.st_mode & mode_mask))
> +      {
> +      case -2: report_file_error ("Copying permissions from", list1 (file));
> +      case -1: report_file_error ("Copying permissions to", list1 (newname));
> +      }
>    }
>  #endif	/* not MSDOS */

This hunk looks wrong, or maybe I'm missing something: it looks like
you removed the ACL support code from the WINDOWSNT blocks, but added
the replacement qcopy_acl in a block that is not compiled on Windows
at all (and uses fchmod and st which are not initialized on Windows).

If I'm right, perhaps it is best to leave the WINDOWSNT parts alone:
since acl_get_file and acl_set_file are still being left in Emacs
sources elsewhere, there's no need to remove their calls here, and
leaving them alone will avoid the need to add qcopy_acl to w32.c.

> -#ifdef HAVE_POSIX_ACL
> +#ifdef HAVE_ACL_SET_FILE
>    absname = ENCODE_FILE (absname);
>  
>    acl = acl_get_file (SSDATA (absname), ACL_TYPE_ACCESS);

It sounds wrong to me to condition a call to acl_get_file with a macro
called HAVE_ACL_SET_FILE.

> @@ -3193,7 +3144,7 @@
>        fail = (acl_set_file (SSDATA (encoded_absname), ACL_TYPE_ACCESS,
>  			    acl)
>  	      != 0);
> -      if (fail && !ACL_NOT_WELL_SUPPORTED (errno))
> +      if (fail && acl_errno_valid (errno))

For this to work, acl-errno-valid.c will have to be compiled on
MS-Windows.  And for that, we will need to solve the EOPNOTSUPP issue
mentioned above, because Windows doesn't define it.