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

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

[Angelo Graziosi]

The build I tried with current master failed. This occurs in the 
nextstep build on OSX and also in the MSYS2/MINGW64 on Windows. The same 
master builds fine in the GTK build on GNU/Linux (but here there are 
other issues; see bug #20677).

The failure looks the same.

On OSX:

[...]
   GEN      dirent.h
   GEN      fcntl.h
/Applications/Xcode.app/Contents/Developer/usr/bin/make  all-am
   CC       acl-errno-valid.o
   CC       acl-internal.o
   CC       get-permissions.o
   CC       set-permissions.o
set-permissions.c:569:20: warning: incompatible pointer to integer 
conversion passing 'acl_t' (aka 'struct _acl *') to parameter of type 
'int' [-Wint-conversion]
           acl = acl_init (acl);
                           ^~~
/usr/include/sys/acl.h:149:27: note: passing argument to parameter 
'count' here
extern acl_t    acl_init(int count);
                              ^
set-permissions.c:585:26: error: use of undeclared identifier 'acl'
         ret = acl_set_fd (desc, acl);
                                 ^
set-permissions.c:587:47: error: use of undeclared identifier 'acl'
         ret = acl_set_file (name, ACL_TYPE_EXTENDED, acl);
                                                      ^
set-permissions.c:590:27: error: use of undeclared identifier 'saved_errno'
           if (! acl_errno_valid (saved_errno) && ! 
acl_extended_nontrivial (acl))
                                  ^
set-permissions.c:590:70: error: use of undeclared identifier 'acl'
           if (! acl_errno_valid (saved_errno) && ! 
acl_extended_nontrivial (acl))
 
      ^
1 warning and 4 errors generated.
make[2]: *** [set-permissions.o] Error 1
make[1]: *** [all] Error 2
make: *** [lib] Error 2
make: *** Waiting for unfinished jobs....
/Applications/Xcode.app/Contents/Developer/usr/bin/make -C doc/emacs info
   GEN      ../../info/emacs.info
/Applications/Xcode.app/Contents/Developer/usr/bin/make -C doc/misc info
   GEN      ../../info/ada-mode.info
   GEN      ../../info/auth.info
   GEN      ../../info/autotype.info
[...]
   GEN      ../../info/widget.info
   GEN      ../../info/wisent.info
   GEN      ../../info/woman.info
   GEN      ../../info/efaq-w32.info


On Windows (MSYS2/MINGW64):

[...]
   CC       acl-errno-valid.o
   CC       acl-internal.o
   CCLD     cmdproxy.exe
   CC       get-permissions.o
   CCLD     ddeclient.exe
   CC       set-permissions.o
set-permissions.c: In function 'set_acls':
set-permissions.c:496:7: error: #error Must have acl_delete_def_file 
(see POSIX 1003.1e draft 17).
  #     error Must have acl_delete_def_file (see POSIX 1003.1e draft 17).
        ^
set-permissions.c:534:7: warning: implicit declaration of function 
'acl_delete_def_file' [-Wimplicit-function-declaration]
        ret = acl_delete_def_file (name);
        ^
Makefile:1627: set di istruzioni per l'obiettivo "set-permissions.o" non 
riuscito
make[2]: *** [set-permissions.o] Errore 1
[...]
   CC       set-permissions.o
set-permissions.c: In function 'set_acls':
set-permissions.c:496:7: error: #error Must have acl_delete_def_file 
(see POSIX 1003.1e draft 17).
  #     error Must have acl_delete_def_file (see POSIX 1003.1e draft 17).
        ^
set-permissions.c:534:7: warning: implicit declaration of function 
'acl_delete_def_file' [-Wimplicit-function-declaration]
        ret = acl_delete_def_file (name);
        ^
Makefile:1627: set di istruzioni per l'obiettivo "set-permissions.o" non 
riuscito
[...]


To build I used the same emacs-master.tar.gz file from git repo. I did 
successful builds with master more o less 10 days ago.


Ciao,
Angelo.

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

[Eli Zaretskii]

> Date: Thu, 28 May 2015 14:55:50 +0200
> From: Angelo Graziosi <angelo.graziosi@alice.it>
> 
> The build I tried with current master failed. This occurs in the 
> nextstep build on OSX and also in the MSYS2/MINGW64 on Windows. The same 
> master builds fine in the GTK build on GNU/Linux (but here there are 
> other issues; see bug #20677).
> 
> The failure looks the same.
> 
> On OSX:
> 
> [...]
>    GEN      dirent.h
>    GEN      fcntl.h
> /Applications/Xcode.app/Contents/Developer/usr/bin/make  all-am
>    CC       acl-errno-valid.o
>    CC       acl-internal.o
>    CC       get-permissions.o
>    CC       set-permissions.o
> set-permissions.c:569:20: warning: incompatible pointer to integer 
> conversion passing 'acl_t' (aka 'struct _acl *') to parameter of type 
> 'int' [-Wint-conversion]
>            acl = acl_init (acl);
>                            ^~~
> /usr/include/sys/acl.h:149:27: note: passing argument to parameter 
> 'count' here
> extern acl_t    acl_init(int count);
>                               ^
> set-permissions.c:585:26: error: use of undeclared identifier 'acl'
>          ret = acl_set_fd (desc, acl);
>                                  ^
> set-permissions.c:587:47: error: use of undeclared identifier 'acl'
>          ret = acl_set_file (name, ACL_TYPE_EXTENDED, acl);
>                                                       ^
> set-permissions.c:590:27: error: use of undeclared identifier 'saved_errno'
>            if (! acl_errno_valid (saved_errno) && ! 
> acl_extended_nontrivial (acl))
>                                   ^
> set-permissions.c:590:70: error: use of undeclared identifier 'acl'
>            if (! acl_errno_valid (saved_errno) && ! 
> acl_extended_nontrivial (acl))
>  
>       ^
> 1 warning and 4 errors generated.
> make[2]: *** [set-permissions.o] Error 1

Should be fixed now.  (set-permissions.c cannot be compiled on MinGW,
and is unnecessary there, so I removed it from the build.)

P.S. Thanks to Paul who made this fix easy by merging the gnulib
changes in nt/gnulib.mk.

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

[Angelo Graziosi]

Il 28/05/2015 19:47, Eli Zaretskii ha scritto:
>> Date: Thu, 28 May 2015 14:55:50 +0200
>> From: Angelo Graziosi <angelo.graziosi@alice.it>
>>
>> The build I tried with current master failed. This occurs in the
>> nextstep build on OSX and also in the MSYS2/MINGW64 on Windows. The same
>> master builds fine in the GTK build on GNU/Linux (but here there are
>> other issues; see bug #20677).
>>
>> The failure looks the same.
>>
>> On OSX:
>>
>> [...]
>>     GEN      dirent.h
>>     GEN      fcntl.h
>> /Applications/Xcode.app/Contents/Developer/usr/bin/make  all-am
>>     CC       acl-errno-valid.o
>>     CC       acl-internal.o
>>     CC       get-permissions.o
>>     CC       set-permissions.o
>> set-permissions.c:569:20: warning: incompatible pointer to integer
>> conversion passing 'acl_t' (aka 'struct _acl *') to parameter of type
>> 'int' [-Wint-conversion]
>>             acl = acl_init (acl);
>>                             ^~~
>> /usr/include/sys/acl.h:149:27: note: passing argument to parameter
>> 'count' here
>> extern acl_t    acl_init(int count);
>>                                ^
>> set-permissions.c:585:26: error: use of undeclared identifier 'acl'
>>           ret = acl_set_fd (desc, acl);
>>                                   ^
>> set-permissions.c:587:47: error: use of undeclared identifier 'acl'
>>           ret = acl_set_file (name, ACL_TYPE_EXTENDED, acl);
>>                                                        ^
>> set-permissions.c:590:27: error: use of undeclared identifier 'saved_errno'
>>             if (! acl_errno_valid (saved_errno) && !
>> acl_extended_nontrivial (acl))
>>                                    ^
>> set-permissions.c:590:70: error: use of undeclared identifier 'acl'
>>             if (! acl_errno_valid (saved_errno) && !
>> acl_extended_nontrivial (acl))
>>
>>        ^
>> 1 warning and 4 errors generated.
>> make[2]: *** [set-permissions.o] Error 1
>
> Should be fixed now.  (set-permissions.c cannot be compiled on MinGW,
> and is unnecessary there, so I removed it from the build.)

This bug has been closed... but..

> P.S. Thanks to Paul who made this fix easy by merging the gnulib
> changes in nt/gnulib.mk.

current master still fails to build on OSX. Same error..


Ciao,
  Angelo.

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

[Paul Eggert]

On 05/29/2015 05:27 AM, Angelo Graziosi wrote:
> current master still fails to build on OSX. Same error..
>

Andreas, this build failure in GNU Emacs appears to be due to the recent 
changes to the ACL code in Gnulib.  Can you please take a look at it 
soon?  See:

http://bugs.gnu.org/20681

To build the latest version of GNU Emacs on OS X, you can do something 
like this:

git clone git://git.savannah.gnu.org/emacs.git
cd emacs
./autogen.sh
./configure --with-ns
make

bug#20681: [PATCH] acl-permissions: Fix build on Mac OS X and older AIX (Bug#20681)

[Andreas Gruenbacher]

* lib/set-permissions.c (set_acls): Fix more errors introduced in the acl
module rewrite.
---
 ChangeLog             |  4 ++++
 lib/set-permissions.c | 13 +++++++------
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index fb8b446..a44510a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2015-05-29  Andreas Gruenbacher  <andreas.gruenbacher@gmail.com>
 
+	acl-permissions: Fix build on Mac OS X and older AIX (Bug#20681)
+	* lib/set-permissions.c (set_acls): Fix more errors introduced in the acl
+	module rewrite.
+
 	acl-permissions: Fix build on Solaris and Cygwin
 	Reported by Tom G. Christensen <tgc@jupiterrise.com>:
 	* lib/set-permissions.c (set_acls): The count, entries, ace_count, and
diff --git a/lib/set-permissions.c b/lib/set-permissions.c
index db3696c..ba291f3 100644
--- a/lib/set-permissions.c
+++ b/lib/set-permissions.c
@@ -566,7 +566,7 @@ set_acls (struct permission_context *ctx, const char *name, int desc,
 	{
 	  acl_free (acl);
 
-	  acl = acl_init (acl);
+	  acl = acl_init (0);
 	  if (acl)
 	    {
 	      if (HAVE_ACL_SET_FD && desc != -1)
@@ -582,12 +582,13 @@ set_acls (struct permission_context *ctx, const char *name, int desc,
   else
     {
       if (HAVE_ACL_SET_FD && desc != -1)
-	ret = acl_set_fd (desc, acl);
+	ret = acl_set_fd (desc, ctx->acl);
       else
-	ret = acl_set_file (name, ACL_TYPE_EXTENDED, acl);
+	ret = acl_set_file (name, ACL_TYPE_EXTENDED, ctx->acl);
       if (ret != 0)
 	{
-	  if (! acl_errno_valid (saved_errno) && ! acl_extended_nontrivial (acl))
+	  if (! acl_errno_valid (errno)
+	      && ! acl_extended_nontrivial (ctx->acl))
 	    ret = 0;
 	}
     }
@@ -696,9 +697,9 @@ set_acls (struct permission_context *ctx, const char *name, int desc,
   if (ret == 0 && ctx->have_u)
     {
       if (desc != -1)
-	ret = fchacl (desc, &u.a, u.a.acl_len);
+	ret = fchacl (desc, &ctx->u.a, ctx->u.a.acl_len);
       else
-	ret = chacl (name, &u.a, u.a.acl_len);
+	ret = chacl (name, &ctx->u.a, ctx->u.a.acl_len);
       if (ret < 0)
 	{
 	  if (errno == ENOSYS && from_mode)
-- 
2.4.0

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

[Andreas Grünbacher]

2015-05-29 18:56 GMT+02:00 Paul Eggert <eggert@cs.ucla.edu>:
> Andreas, this build failure in GNU Emacs appears to be due to the recent
> changes to the ACL code in Gnulib.  Can you please take a look at it soon?

I hope that's all fixed now.

Thanks,
Andreas

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

[Paul Eggert]

On 05/29/2015 12:09 PM, Andreas Grünbacher wrote:
> 2015-05-29 18:56 GMT+02:00 Paul Eggert <eggert@cs.ucla.edu>:
>> Andreas, this build failure in GNU Emacs appears to be due to the recent
>> changes to the ACL code in Gnulib.  Can you please take a look at it soon?
> I hope that's all fixed now.
>
> Thanks,
> Andreas

Thanks for the quick fix.  I merged this into the Emacs master as 
commit  and am marking this bug as done again.  From the Emacs point of 
view this should also fix build problems on Solaris and Cygwin.

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

[Nick Andryshak]

> Thanks for the quick fix.  I merged this into the Emacs master as 
> commit  and am marking this bug as done again.  From the Emacs point of 
> view this should also fix build problems on Solaris and Cygwin.

Thanks Paul and Andreas, this does indeed fix the build problem on
Cygwin.

- Nick

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

[Andreas Grünbacher]

2015-05-29 21:56 GMT+02:00 Nick Andryshak <nandryshak@gmail.com>:
> Thanks Paul and Andreas, this does indeed fix the build problem on
> Cygwin.

Great, thanks.

Andreas

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

[Eli Zaretskii]

> Date: Fri, 29 May 2015 21:57:47 +0200
> From: Andreas Grünbacher <andreas.gruenbacher@gmail.com>
> Cc: 20681-done@debbugs.gnu.org, Paul Eggert <eggert@cs.ucla.edu>,
> 	Angelo Graziosi <angelo.graziosi@alice.it>
> 
> 2015-05-29 21:56 GMT+02:00 Nick Andryshak <nandryshak@gmail.com>:
> > Thanks Paul and Andreas, this does indeed fix the build problem on
> > Cygwin.
> 
> Great, thanks.

Not sure if that code is supposed to be compatible with MinGW, but if
it is, there's still a problem, since set-permissions.c won't compile:

  set-permissions.c: In function 'set_acls':
  set-permissions.c:496:7: error: #error Must have acl_delete_def_file (see POSIX 1003.1e draft 17).
   #     error Must have acl_delete_def_file (see POSIX 1003.1e draft 17).
	 ^
  set-permissions.c:534:7: warning: implicit declaration of function 'acl_delete_def_file' [-Wimplicit-function-declaration]
	 ret = acl_delete_def_file (name);
	 ^
  Makefile:1563: recipe for target `set-permissions.o' failed

It's not a problem for Emacs, since the MinGW build doesn't compile
that file, but it means that other projects using gnulib might not
build on MinGW.

Thanks.

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

[Andreas Grünbacher]

Eli,

2015-05-30 12:22 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:
> Not sure if that code is supposed to be compatible with MinGW, but if
> it is, there's still a problem, since set-permissions.c won't compile:

it doesn't seem like that part of the code has changed, but I may have
overlooked something.

Could you check if coreutils from
https://github.com/andreas-gruenbacher/coreutils,
master branch, builds on MinGW? (The upstream version of coreutils doesn't use
the latest version of gnulib yet, but this version does.)

If that fails, what config.h do you get there, and does it differ from
the config you get with
git://git.savannah.gnu.org/coreutils.git? Do the Savannah coreutils build?

Thanks,
Andreas

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

[Eli Zaretskii]

> Date: Sat, 30 May 2015 14:02:51 +0200
> From: Andreas Grünbacher <andreas.gruenbacher@gmail.com>
> Cc: Nick Andryshak <nandryshak@gmail.com>, 20681@debbugs.gnu.org, 
> 	Paul Eggert <eggert@cs.ucla.edu>, Angelo Graziosi <angelo.graziosi@alice.it>
> 
> Could you check if coreutils from
> https://github.com/andreas-gruenbacher/coreutils,
> master branch, builds on MinGW? (The upstream version of coreutils doesn't use
> the latest version of gnulib yet, but this version does.)

Coreutils doesn't build with MinGW for ages, so this is not a good
method of investigating the issue, IMO.

> If that fails, what config.h do you get there, and does it differ from
> the config you get with
> git://git.savannah.gnu.org/coreutils.git? Do the Savannah coreutils build?

Please tell what do you want to see in confg.h, specifically.  I can
force the Emacs development head to compile set-permissions.c, so
perhaps Emacs's config.h will do.

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

[Andreas Grünbacher]

2015-05-30 14:10 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:
> Please tell what do you want to see in confg.h, specifically.  I can
> force the Emacs development head to compile set-permissions.c, so
> perhaps Emacs's config.h will do.

It may. I have no idea what kind of acl functionality emacs on MinGW
should have though. What did it do before? Is the build failure new
or did it already exist before the acl rewrite? Which acl related
symbols do you have in config.h? If it has acl_get_file and acl_set_file,
where can I find documentation about what those functions do on
MinGW?

Thanks,
Andreas

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

[Eli Zaretskii]

> Date: Sat, 30 May 2015 15:06:16 +0200
> From: Andreas Grünbacher <andreas.gruenbacher@gmail.com>
> Cc: Nick Andryshak <nandryshak@gmail.com>, 20681@debbugs.gnu.org, 
> 	Paul Eggert <eggert@cs.ucla.edu>, Angelo Graziosi <angelo.graziosi@alice.it>
> 
> > Please tell what do you want to see in confg.h, specifically.  I can
> > force the Emacs development head to compile set-permissions.c, so
> > perhaps Emacs's config.h will do.
> 
> It may. I have no idea what kind of acl functionality emacs on MinGW
> should have though.

Emacs built with MinGW has the same functionality on Windows as Emacs
does on Posix hosts: get a file's ACL as a text string, set a file's
ACL from text string description, and copy ACL from one file to
another.

The relevant functions, acl_get_file, acl_set_file, acl_to_text,
acl_from_text, and acl_free are implemented as part of Emacs.

> What did it do before?

The only ACL-related function that Emacs on Windows was using from
Gnulib was acl_errno_valid.  All the rest were implemented in the
Emacs sources.

IOW, the Gnulib ACL functions were almost completely unused in the
MinGW build.  However, they did compile, which allowed us to keep the
Gnulib configuration very similar to what was used on Posix hosts.
Which is why I'm not sure it is worth your while to do more than
you've already done, unless you do want to make sure the code at least
compiles as it did before.

With that caveat, I answer below your questions, regardless.

> Is the build failure new or did it already exist before the acl
> rewrite?

It is new.  Before the rewrite, the file qcopy-acl.c would compile
with MinGW, although the function qcopy_acl was not called in the
MinGW build, and so qcopy-acl.o was not linked into the binary.  But
it would at least compile.

After the rewrite, qcopy-acl.c is just a thin wrapper around
get-permissions.c and set-permissions.c.  The former still compiles
with MinGW, but the latter doesn't.  The reason is this cpp
conditional:

  #    ifndef HAVE_ACL_DELETE_DEF_FILE
  #     error Must have acl_delete_def_file (see POSIX 1003.1e draft 17).
  #    endif

I don't understand why the code requires acl_delete_def_file where it
previously did not: it's not like setting permissions will absolutely
not work if there is no such function.  E.g., why not do something
like the following instead:

  #    ifndef HAVE_ACL_DELETE_DEF_FILE
		    ret = -1;
		    errno = ENOSYS;
  #    else
		    ret = acl_delete_def_file (name);

  #    endif

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.  After all, copying ACLs
from one file to another is much more frequent and important an
operation than setting ACLs from user-specified arguments.  What's
more, the assumption that if acl_get_file and acl_set_file are
available, then so should be acl_delete_def_file is yet another
obstacle, IMO gratuitous one.

> Which acl related symbols do you have in config.h?

HAVE_ACL_FREE
HAVE_ACL_FROM_TEXT
HAVE_ACL_GET_FILE
HAVE_ACL_SET_FILE
HAVE_SYS_ACL_H
USE_ACL

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

Thanks.

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

[Andreas Grünbacher]

Eli,

2015-05-31 16:29 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:
>> Date: Sat, 30 May 2015 15:06:16 +0200
>> From: Andreas Grünbacher <andreas.gruenbacher@gmail.com>
>> Cc: Nick Andryshak <nandryshak@gmail.com>, 20681@debbugs.gnu.org,
>>       Paul Eggert <eggert@cs.ucla.edu>, Angelo Graziosi <angelo.graziosi@alice.it>
>>
>> > Please tell what do you want to see in confg.h, specifically.  I can
>> > force the Emacs development head to compile set-permissions.c, so
>> > perhaps Emacs's config.h will do.
>>
>> It may. I have no idea what kind of acl functionality emacs on MinGW
>> should have though.
>
> Emacs built with MinGW has the same functionality on Windows as Emacs
> does on Posix hosts: get a file's ACL as a text string, set a file's
> ACL from text string description, and copy ACL from one file to
> another.
>
> The relevant functions, acl_get_file, acl_set_file, acl_to_text,
> acl_from_text, and acl_free are implemented as part of Emacs.

So those are the functions that gnulib is seeing? If so, then emacs
could provide
acl_delete_def_file as well, and everything will be fine again.

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.

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

>> What did it do before?
>
> The only ACL-related function that Emacs on Windows was using from
> Gnulib was acl_errno_valid.  All the rest were implemented in the
> Emacs sources.
>
> IOW, the Gnulib ACL functions were almost completely unused in the
> MinGW build.  However, they did compile, which allowed us to keep the
> Gnulib configuration very similar to what was used on Posix hosts.
> Which is why I'm not sure it is worth your while to do more than
> you've already done, unless you do want to make sure the code at least
> compiles as it did before.
>
> With that caveat, I answer below your questions, regardless.
>
>> Is the build failure new or did it already exist before the acl
>> rewrite?
>
> It is new.  Before the rewrite, the file qcopy-acl.c would compile
> with MinGW, although the function qcopy_acl was not called in the
> MinGW build, and so qcopy-acl.o was not linked into the binary.  But
> it would at least compile.
>
> After the rewrite, qcopy-acl.c is just a thin wrapper around
> get-permissions.c and set-permissions.c.  The former still compiles
> with MinGW, but the latter doesn't.  The reason is this cpp
> conditional:
>
>   #    ifndef HAVE_ACL_DELETE_DEF_FILE
>   #     error Must have acl_delete_def_file (see POSIX 1003.1e draft 17).
>   #    endif
>
> I don't understand why the code requires acl_delete_def_file where it
> previously did not: it's not like setting permissions will absolutely
> not work if there is no such function.

I see now what has changed: qcopy_acl didn't call acl_delete_def_file
before but it should have for directories. (The sibling function qset_acl
did call acl_delete_def_file even before the rewrite.)


> E.g., why not do something like the following instead:
>
>   #    ifndef HAVE_ACL_DELETE_DEF_FILE
>                     ret = -1;
>                     errno = ENOSYS;
>   #    else
>                     ret = acl_delete_def_file (name);
>
>   #    endif

Nope, the call is really needed.

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

> After all, copying ACLs from one file to another is much more frequent
> and important an operation than setting ACLs from user-specified
> arguments.

Probably, yes.

> What's more, the assumption that if acl_get_file and acl_set_file are
> available, then so should be acl_delete_def_file is yet another
> obstacle, IMO gratuitous one.

No, acl_get_file, acl_set_file, and acl_delete_def_file are all part of the
POSIX ACL API, and acl_delete_def_file is needed for directories.

>> Which acl related symbols do you have in config.h?
>
> HAVE_ACL_FREE
> HAVE_ACL_FROM_TEXT
> HAVE_ACL_GET_FILE
> HAVE_ACL_SET_FILE
> HAVE_SYS_ACL_H
> USE_ACL
>
>> 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.

Thanks,
Andreas

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

[Eli Zaretskii]

> Date: Sun, 31 May 2015 21:18:37 +0200
> From: Andreas Grünbacher <andreas.gruenbacher@gmail.com>
> Cc: Nick Andryshak <nandryshak@gmail.com>, 20681@debbugs.gnu.org, 
> 	Paul Eggert <eggert@cs.ucla.edu>, Angelo Graziosi <angelo.graziosi@alice.it>
> 
> > Emacs built with MinGW has the same functionality on Windows as Emacs
> > does on Posix hosts: get a file's ACL as a text string, set a file's
> > ACL from text string description, and copy ACL from one file to
> > another.
> >
> > The relevant functions, acl_get_file, acl_set_file, acl_to_text,
> > acl_from_text, and acl_free are implemented as part of Emacs.
> 
> So those are the functions that gnulib is seeing? If so, then emacs
> could provide
> acl_delete_def_file as well, and everything will be fine again.

Everything is fine already: I removed set-permissions.c from the MinGW
build.

While acl_delete_def_file could be added, it would be a stub that
always fails, so I see no point in doing that, as long as Gnulib ACL
code is effectively ignored on Windows.

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

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

> > E.g., why not do something like the following instead:
> >
> >   #    ifndef HAVE_ACL_DELETE_DEF_FILE
> >                     ret = -1;
> >                     errno = ENOSYS;
> >   #    else
> >                     ret = acl_delete_def_file (name);
> >
> >   #    endif
> 
> 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.

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

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.

> > After all, copying ACLs from one file to another is much more frequent
> > and important an operation than setting ACLs from user-specified
> > arguments.
> 
> Probably, yes.
> 
> > What's more, the assumption that if acl_get_file and acl_set_file are
> > available, then so should be acl_delete_def_file is yet another
> > obstacle, IMO gratuitous one.
> 
> No, acl_get_file, acl_set_file, and acl_delete_def_file are all part of the
> POSIX ACL API, and acl_delete_def_file is needed for directories.

For Posix platforms, yes.  I was thinking about non-Posix ones, where
there's no such necessary linkage.

> >> 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.  IMO, a
general-purpose program such as Emacs should not futz with the
inheritance flags in Windows ACLs, because doing so makes it very easy
to inadvertently make directories and files inaccessible or exempt
from important operations like system-wide backup.  Fortunately, Emacs
doesn't need these capabilities, at least not explicitly on the
acl_get_file and acl_set_file level.

Once again, please don't take the above as some objection to your
work, or request to make any changes for MinGW.  They are just
observations of a bystander at this point.

Thanks.

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

[Andreas Grünbacher]

2015-06-01 17:05 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:
>> 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

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

[Eli Zaretskii]

> Date: Mon, 1 Jun 2015 18:18:43 +0200
> From: Andreas Grünbacher <andreas.gruenbacher@gmail.com>
> Cc: Nick Andryshak <nandryshak@gmail.com>, 20681@debbugs.gnu.org, 
> 	Paul Eggert <eggert@cs.ucla.edu>, Angelo Graziosi <angelo.graziosi@alice.it>
> 
> 2015-06-01 17:05 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:
> >> 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.

Like I said: a lot of work for too little a gain.  I'm not
volunteering, sorry.  Emacs already has ACL support that is good
enough for me.

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

That problem doesn't exist on Windows, as ACLs are mapped to the same
representation, no matter what the underlying filesystem.

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

It made a lot of sense to me at the time.  Emacs (and other programs
I'm interested in) comes from the Posix world, so its "mindset" is
that of a Posix program.  And the Posix APIs for manipulating ACLs are
simple and well documented, so emulating them was easy.

As for ACL_TYPE_ACCESS, since Emacs uses only one type of ACLs, it
doesn't matter for an emulation how that type is identified.  It's
just a symbol.

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

??? If it were true, set-permissions.c would compile on Windows.  It
doesn't.  So there's more to this job than just copying the code.

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

It's a reason not to use Gnulib for any file-related operations in
Emacs on Windows, because Emacs on Windows uses Unicode APIs to access
files by their names.

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

[Andreas Grünbacher]

2015-06-01 19:39 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:
> ??? If it were true, set-permissions.c would compile on Windows.  It
> doesn't.

I've already explained, at length, why it currently doesn't compile, and
at least three ways of how it could be fixed.

> So there's more to this job than just copying the code.

Yes, adding some #ifdefs and autoconf checks, that kind of thing.
That's not so hard.

>> > 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.
>
> It's a reason not to use Gnulib for any file-related operations in
> Emacs on Windows, because Emacs on Windows uses Unicode APIs to access
> files by their names.

So it's okay to have this function in emacs:

  src/w32.c:acl_get_file (const char *fname, acl_type_t type)

but the same interface in gnulib wouldn't be okay? Right ...

I'll stop this discussion now, you're just not making sense.

Andreas

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

[Eli Zaretskii]

> Date: Mon, 1 Jun 2015 20:41:02 +0200
> From: Andreas Grünbacher <andreas.gruenbacher@gmail.com>
> Cc: Nick Andryshak <nandryshak@gmail.com>, 20681@debbugs.gnu.org, 
> 	Paul Eggert <eggert@cs.ucla.edu>, Angelo Graziosi <angelo.graziosi@alice.it>
> 
> 2015-06-01 19:39 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:
> > ??? If it were true, set-permissions.c would compile on Windows.  It
> > doesn't.
> 
> I've already explained, at length, why it currently doesn't compile, and
> at least three ways of how it could be fixed.
> 
> > So there's more to this job than just copying the code.
> 
> Yes, adding some #ifdefs and autoconf checks, that kind of thing.
> That's not so hard.

You have just explained at length why using emulation of Posix ACL
APIs would be wrong on Windows, so I assumed that copying the Emacs
implementation with minimal changes into Gnulib must also be wrong in
your opinion.

> >> > 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.
> >
> > It's a reason not to use Gnulib for any file-related operations in
> > Emacs on Windows, because Emacs on Windows uses Unicode APIs to access
> > files by their names.
> 
> So it's okay to have this function in emacs:
> 
>   src/w32.c:acl_get_file (const char *fname, acl_type_t type)
> 
> but the same interface in gnulib wouldn't be okay? Right ...

Emacs on Windows pretends to live in a UTF-8 locale, so file names are
encoded in UTF-8 before they are passed to functions like
acl_get_file; then acl_get_file and other such functions convert the
file names to UTF-16 before calling Windows Unicode APIs.

This works well for Emacs, but Gnulib is not just for Emacs, it is
used by many other projects that build out of the box with MinGW.
Most, if not all, of those projects do NOT pass UTF-8 file names to
library functions, and many of them don't even have facilities that
exist in Emacs to encode file names before calling library functions
-- they simply pass C char * strings to them, which will only work on
Windows if those strings are encoded in the current system codepage.

Therefore, functions that assume UTF-8 encoded file name CANNOT be
added to Gnulib, as they will break every project except Emacs.
Gnulib on Windows _requires_ file names in the system codepage, which
is no longer acceptable for Emacs, because Emacs has broken out of
this limitation several releases ago, and it can now visit files whose
names are not limited to system codepage.

> I'll stop this discussion now, you're just not making sense.

Maybe you should study the subject more deeply, before passing such
judgments.

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

[Angelo Graziosi]

Il 29/05/2015 21:45, Paul Eggert ha scritto:
> On 05/29/2015 12:09 PM, Andreas Grünbacher wrote:
>> 2015-05-29 18:56 GMT+02:00 Paul Eggert <eggert@cs.ucla.edu>:
>>> Andreas, this build failure in GNU Emacs appears to be due to the recent
>>> changes to the ACL code in Gnulib.  Can you please take a look at it
>>> soon?
>> I hope that's all fixed now.
>>
>> Thanks,
>> Andreas
>
> Thanks for the quick fix.  I merged this into the Emacs master as
> commit  and am marking this bug as done again.  From the Emacs point of
> view this should also fix build problems on Solaris and Cygwin.

The build seems to be fixed also on OSX. Thanks a lot!

Ciao,
  Angelo.