Re: acl: add support for MacOS X 10.5

[Jim Meyering]

Bruno Haible <address@hidden> wrote:
> Here comes the reworked patch for MacOS X 10.5 support in copy_acl, used
> by the 'copy-file' module. With it, now the test passes on MacOS X 10.5,
> and of course still passes on Linux and FreeBSD.
>
> The main difficulty here was to realize that in MacOS X, unlike many other
> flavors of ACLs, an ACL does *not* contain the file mode (rwxrwxrwx). They
> are separate. In the code I created a macro MODE_INSIDE_ACL to denote the
> "normal" situation.
>
> The consequences are that
>   - The qset_acl function requires a different logic when !MODE_INSIDE_ACL.
>     When MODE_INSIDE_ACL, one optimizes system calls by calling chmod only
>     when setting the ACL failed or was not enough. When !MODE_INSIDE_ACL,
>     one has to call chmod always.
>   - Similarly in copy_acl, but here the code changes are smaller.
>   - When !MODE_INSIDE_ACL, a trivial ACL has 0 entries, not 3 entries.
>
>
> Another realization is that this piece of code in acl.c:
>
>       char acl_text[] = "u::---,g::---,o::---";
>
>       if (mode & S_IRUSR) acl_text[ 3] = 'r';
>       if (mode & S_IWUSR) acl_text[ 4] = 'w';
>       if (mode & S_IXUSR) acl_text[ 5] = 'x';
>
> works only on FreeBSD (and possibly IRIX). Basically, every ACL flavor comes
> with its own textual representation of ACLs, and most of them expect the
> syntax "other:rwx", not "other::rwx".
>
>
> Also, I added a comment explaining why the conversion mode_t -> acl_t goes
> through the textual representation rather than through acl_init() and
> acl_create_entry().
>
>
>> for future patches to my attention please use git format-patch,
>> since that makes it easier for me to apply and ensure that when I review
>
> Find it attached as attachment. Now I'll have to learn how to amend previous
> commits...
>
> OK to commit? After this, I'll turn to the other 5 platforms.

Looks good.
You're welcome to commit regardless of the acl_trivial
issue mentioned below.  Since ACL support is currently broken
on MacOS, this is sure to be an improvement.

> 2008-05-22  Bruno Haible  <address@hidden>
>
>       Make copy_acl work on MacOS X 10.5.
>       * lib/acl-internal.h (MODE_INSIDE_ACL): New macro.
>       (ACL_NOT_WELL_SUPPORTED): On MacOS X, also handle ENOENT.
>       * lib/acl.c (qset_acl): Add different code branch for !MODE_INSIDE_ACL.
>       If MODE_INSIDE_ACL, don't assume that every system has the same text
>       representation for ACLs as FreeBSD.
>       * lib/copy-acl.c (copy_acl): Add support for platforms with
>       !MODE_INSIDE_ACL.
>       * lib/file-has-acl.c (file_has_acl): Likewise.
>       * m4/acl.m4 (gl_FUNC_ACL): Test for some functions that are witness
>       of FreeBSD or MacOS X, respectively.
...
> *** lib/copy-acl.c.orig       2008-05-23 01:08:01.000000000 +0200
> --- lib/copy-acl.c    2008-05-23 00:54:12.000000000 +0200
> ***************
> *** 40,45 ****
> --- 40,46 ----
>
>   #if USE_ACL && HAVE_ACL_GET_FILE && HAVE_ACL_SET_FILE && HAVE_ACL_FREE
>     /* POSIX 1003.1e (draft 17 -- abandoned) specific version.  */
> +   /* Linux, FreeBSD, MacOS X, IRIX, Tru64 */
>
>     acl_t acl;
>     if (HAVE_ACL_GET_FD && source_desc != -1)
> ***************
> *** 70,81 ****
>         int n = acl_entries (acl);
>
>         acl_free (acl);
> !       /* On most hosts an ACL is trivial if n == 3, and it cannot be
> !          less than 3.  On IRIX 6.5 it is also trivial if n == -1.
>            For simplicity and safety, assume the ACL is trivial if n <= 3.
>            Also see file-has-acl.c for some of the other possibilities;
>            it's not clear whether that complexity is needed here.  */
> !       if (n <= 3)
>           {
>             if (chmod_or_fchmod (dst_name, dest_desc, mode) != 0)
>               saved_errno = errno;
> --- 71,83 ----
>         int n = acl_entries (acl);
>
>         acl_free (acl);
> !       /* On most hosts with MODE_INSIDE_ACL an ACL is trivial if n == 3,
> !          and it cannot be less than 3.  On IRIX 6.5 it is also trivial if
> !          n == -1.

Once you've investigated Solaris ACLs you might want to adjust that
comment.  I seem to recall seeing trivial ones with 4 or 5 entries.

>            For simplicity and safety, assume the ACL is trivial if n <= 3.
>            Also see file-has-acl.c for some of the other possibilities;
>            it's not clear whether that complexity is needed here.  */
> !       if (n <= 3 * MODE_INSIDE_ACL)
>           {
>             if (chmod_or_fchmod (dst_name, dest_desc, mode) != 0)
>               saved_errno = errno;

Have you considered using acl_trivial (also used in file-has-acl.c)
or a replacement that encapsulates this test?