Subject: Re: bug#20681: Build failure [MSYS2/MINGW64, OSX]

From: Andreas Grünbacher <[email-masked]>
Date: Mon, 1 Jun 2015 18:18:43 +0200

2015-06-01 17:05 GMT+02:00 Eli Zaretskii <[email-masked]>:
>> But shouldn't Windows ACL support be added to get_permissions() and
>> set_permissions() proper instead of emulating Windows ACL support through
>> the POSIX draft ACL interface? The _to_text() and _from_text() functions
>> could be modified to take a struct permission_context argument; they could
>> be moved into gnulib or remain part of emacs.
>
> What would be the benefit of doing that, though?  You will see in the
> Emacs sources that currently acl_to_text produces the Windows-specific
> SDDL string representation of the file's DACL security descriptor;
> making that Posix-compliant in order for those functions to be able to
> work with the likes of acl_from_mode is an extremely non-trivial and
> thankless job.

You would add a Windows ACL to struct permission_context, add support for
it to get_permissions and set_permissions, and add
permission_context_from_text() and permission_context_to_text() functions.

The _from_text and _to_text functions could initially only support POSIX and
Windows ACLs, which would be what emacs supports today; support
for other kinds of ACLs could be added later at any point.

As a result, emacs would grow better acl support over time, and so would
the other packages using get_permissions and set_permissions.

>> As a side effect of that, cp in coreutils would then also do the right thing on
>> Windows (if someone made coreutils build there again).
>
> If cp just copies the ACLs, it doesn't need all this machinery.  It
> just need to handle ACLs as an opaque object understood only by
> acl_get_file and acl_set_file.

There's a bit more to copying acls if you want to copy between file
systems which
support different kinds of acls (or no acls) in a reasonable way. The
cp utility is
dealing with this kind of problem, and I can't think of a lot of reason why that
logic shouldn't be shared with emacs.

>> Nope, the call is really needed.
>
> I believe you, but I still don't understand why this couldn't support
> systems that don't have the notion of default ACL, or don't need to
> remove it when the file in question is actually a directory.
>
> You might have in mind Posix platforms that adhere to the relevant
> Posix draft, in which case it's exactly my point: this code makes it
> harder for non-Posix platforms to support these routines when it's
> easy to emulate acl_get_file and acl_set_file, but supporting addition
> or removal of the default ACL is hard or ineffective/pointless.

I'm talking *exactly* about systems which don't have POSIX ACLs. It doesn't
make a whole lot of sense to emulate Windows as POSIX ACLs, and it makes
no sense to emulate them as ACL_TYPE_ACCESS. If you emulate
POSIX ACLs, then you need to emulate both ACL_TYPE_ACCESS and
ACL_TYPE_DEFAULT, or ACL_TYPE_EXTENDED instead of
ACL_TYPE_ACCESS and ACL_TYPE_DEFAULT. But even that isn't really
all that useful; there is no reason why Windows ACLs can't be implemented
as a new kind instead of forcing them through a POSIX-y interface.

>> > More generally, copying ACLs from one file to another does not need
>> > any analysis of what's in the ACLs being copied: it could simply treat
>> > the ACLs as an opaque object, something whose structure and semantics
>> > are known only to the innards of acl_get_file and acl_set_file.  So
>> > building qcopy_acl on top of get_permissions and set_permissions,
>> > which are more flexible, and do require that knowledge, is IMO
>> > fundamentally wrong, as it makes much harder to port this simple and
>> > important Gnulib function to other platforms.
>>
>> I disagree. Have a look at struct permission_context; acl_get_file() and
>> acl_set_file() is part of the POSIX draft ACL API but other platforms
>> have totally different data structures and interfaces. Windows ACLs
>> should be implemented as another kind of ACLs, not disguised as
>> POSIX ACLs.
>
> See above: doing so is of course possible, but it's a lot of hassle
> for very little, if any, benefits.  It is very easy to provide on
> MS-Windows the minimum emulation of Posix interfaces that allows to
> (a) copy ACLs from one file to another and (b) convert ACLs to and
> from text representation for human consumption or for logging. That's
> all Emacs needs, and that's all most other programs will ever need.
> Adding anything else to the soup raises the bar much higher, because
> the semantics of Windows ACLs is very different.  For starters, where
> Posix platforms have 3 rwx access bits, on Windows there are 7
> standard access rights; mapping those 7 to the 3 Posix bits will at
> best seriously limit what Windows programs can do with ACLs.  And then
> there are the issues with the ordering of ACEs in an ACL, with
> implicit access rights via group membership, etc. etc.
>
> So I think the depth of compliance which is expected by
> set-permissions.c is too much for Windows, way beyond the proverbial
> 80-20 point.

Adding Windows ACL support to get_permissions and set_permissions
really doesn't take a whole lot more than copying the code from emacs into
gnulib and testing the result. It's really not such a big deal.

> There's also the minor (but important for Emacs) point of supporting
> file names with characters outside of the current system codepage,
> which Gnulib can only provide in UTF-8 locales, something that doesn't
> exist on Windows.

This has nothing to do with get_permissions and set_permissions.

>> >> If it has acl_get_file and acl_set_file, where can I find
>> >> documentation about what those functions do on MinGW?
>> >
>> > They do what you'd expect, but support only ACL_TYPE_ACCESS.  They
>> > know about ACL_TYPE_DEFAULT, but always return EINVAL for it, since
>> > it's next to impossible (and not recommended) to have a directory on
>> > Windows which has no default ACLs associated with it.
>> >
>> > You can see their sources in src/w32.c in the Emacs repository.
>>
>> That's just wrong. Windows ACLs can contain effective as well as inheritable
>> permissions; the split between types as with POSIX ACLs doesn't exist.
>
> Not sure what you are saying here: which part of what I wrote is
> "wrong", exactly?
>
> If what you mean is that ACL_TYPE_ACCESS could or should include the
> inheritance flags, then that's what is currently implemented in Emacs,
> but reporting or adding/removing those flags is not supported.

The whole point of splitting POSIX ACLs into the two types ACL_TYPE_ACCESS
and ACL_TYPE_DEFAULT is to get rid of inheritance flags in ACLs. Having
inheritance flags in ACL_TYPE_ACCESS or ACL_TYPE_DEFAULT is totally wrong.
Windows doesn't have this split, and neither does ACL_TYPE_EXTENDED.

Andreas