unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Paul Eggert <eggert@cs.ucla.edu>
Cc: 14295@debbugs.gnu.org
Subject: bug#14295: Support copy-file ACLs for Solaris etc.
Date: Sun, 28 Apr 2013 20:16:29 +0300	[thread overview]
Message-ID: <83ehdushhu.fsf@gnu.org> (raw)
In-Reply-To: <517C987F.6060902@cs.ucla.edu>

> Date: Sat, 27 Apr 2013 20:33:19 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: Eli Zaretskii <eliz@gnu.org>
> 
> 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.





  reply	other threads:[~2013-04-28 17:16 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-28  3:33 bug#14295: Support copy-file ACLs for Solaris etc Paul Eggert
2013-04-28 17:16 ` Eli Zaretskii [this message]
2013-04-29  6:15   ` Paul Eggert
2013-04-29 17:07     ` Eli Zaretskii
2013-05-07 21:36       ` Paul Eggert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83ehdushhu.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=14295@debbugs.gnu.org \
    --cc=eggert@cs.ucla.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

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).