bug#42095: 28.0.50; Build fails on Windows/MinGW64

bug#42095: 28.0.50; Build fails on Windows/MinGW64

[Peder O. Klingenberg]

I wanted to refresh my Windows Emacs build tonight, for the first time
in some months.  I pulled up to
5ce5cf643840cd6efd25d987bc5b6f12478c50a6 by Paul Eggert and ran make -j4
as usual.  The build failed.  Likewise make bootstrap.  The error:

  CC       open.o
open.c: In function 'sys_open':
open.c:127:48: error: 'O_CLOEXEC' undeclared (first use in this function)
  127 |                   flags & ~(have_cloexec < 0 ? O_CLOEXEC : 0), mode);
      |                                                ^~~~~~~~~
open.c:127:48: note: each undeclared identifier is reported only once for each function it appears in

That line was changed a few hours ago, in commit
118c07e02e939c9f52688091509d4bff2a897032, also by Paul.

Checking out the commit prior to 118c07 seems to get past the error.
The line and its surroundings look fairly similar in that revision
(ffb89e), so I'm not sure what would be the proper fix.  Hence no
patch, sorry.

...Peder...
-- 
I wish a new life awaited _me_ in some off-world colony.

bug#42095: 28.0.50; Build fails on Windows/MinGW64

[Eli Zaretskii]

> From: "Peder O. Klingenberg" <peder@klingenberg.no>
> Date: Sat, 27 Jun 2020 23:28:17 +0200
> 
> I wanted to refresh my Windows Emacs build tonight, for the first time
> in some months.  I pulled up to
> 5ce5cf643840cd6efd25d987bc5b6f12478c50a6 by Paul Eggert and ran make -j4
> as usual.  The build failed.  Likewise make bootstrap.  The error:
> 
>   CC       open.o
> open.c: In function 'sys_open':
> open.c:127:48: error: 'O_CLOEXEC' undeclared (first use in this function)
>   127 |                   flags & ~(have_cloexec < 0 ? O_CLOEXEC : 0), mode);
>       |                                                ^~~~~~~~~
> open.c:127:48: note: each undeclared identifier is reported only once for each function it appears in
> 
> That line was changed a few hours ago, in commit
> 118c07e02e939c9f52688091509d4bff2a897032, also by Paul.

Thanks for reporting this.  lib/open.c should not be compiled in the
MinGW build.

Paul, this problem should be solved in Gnulib, because its open.m4
unconditionally forces lib/open.c to be compiled in the MinGW build:

  case "$host_os" in
    mingw* | pw*)
      REPLACE_OPEN=1
      ;;

The latest changes made this code run even in the MinGW build, not
sure why (perhaps because 'getrandom' uses 'open'?).  Emacs cannot use
Gnulib's 'open' (or any other function that takes file names and
operates on files), because Gnulib on Windows doesn't support UTF-8
encoded file names, which Emacs needs.  We have our own replacements
for 'open' and its ilk.  That is why nt/gnulib-cfg.mk says

  OMIT_GNULIB_MODULE_open = true

But this is no longer enough, since these latest changes.

Please provide a way to reliably avoid compiling lib/open.c on
Windows.  I needed to hack configure to make it build here.

Thanks.

P.S.  The 'getrandom' module had other MinGW-related problems, some of
which I fixed in master, and others were reported to the Gnulib
mailing list:

  https://lists.gnu.org/archive/html/bug-gnulib/2020-06/msg00059.html

bug#42095: 28.0.50; Build fails on Windows/MinGW64

[Eli Zaretskii]

> Date: Sun, 28 Jun 2020 18:48:42 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: "Peder O. Klingenberg" <peder@klingenberg.no>, 42095@debbugs.gnu.org
> 
> P.S.  The 'getrandom' module had other MinGW-related problems, some of
> which I fixed in master, and others were reported to the Gnulib
> mailing list:
> 
>   https://lists.gnu.org/archive/html/bug-gnulib/2020-06/msg00059.html

Btw, do we have any tests in our test suite to test the getrandom
function?  I only found gnutls-tests, but they only seem to call this
for new enough version of GnuTLS.  Is it possible to come up with a
few tests?

bug#42095: 28.0.50; Build fails on Windows/MinGW64

[Paul Eggert]

On 6/28/20 12:36 PM, Eli Zaretskii wrote:
> do we have any tests in our test suite to test the getrandom
> function?

No. Contributions would be welcome, I assume. Testing randomness is nontrivial,
though.

bug#42095: 28.0.50; Build fails on Windows/MinGW64

[Eli Zaretskii]

> Cc: peder@klingenberg.no, 42095@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 28 Jun 2020 13:35:41 -0700
> 
> On 6/28/20 12:36 PM, Eli Zaretskii wrote:
> > do we have any tests in our test suite to test the getrandom
> > function?
> 
> No. Contributions would be welcome, I assume. Testing randomness is nontrivial,
> though.

I just wanted something to cause the call to getrandom, just to see it
working and producing some reasonable value.  Would that be easy to
arrange?

Thanks.

bug#42095: 28.0.50; Build fails on Windows/MinGW64

[Paul Eggert]

On 6/28/20 7:28 PM, Eli Zaretskii wrote:
> I just wanted something to cause the call to getrandom, just to see it
> working and producing some reasonable value.  Would that be easy to
> arrange?

If all we want to do is call getrandom, the Gnulib module 'getrandom-tests' does
that.

bug#42095: 28.0.50; Build fails on Windows/MinGW64

[Eli Zaretskii]

> Cc: peder@klingenberg.no, 42095@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 28 Jun 2020 23:56:44 -0700
> 
> On 6/28/20 7:28 PM, Eli Zaretskii wrote:
> > I just wanted something to cause the call to getrandom, just to see it
> > working and producing some reasonable value.  Would that be easy to
> > arrange?
> 
> If all we want to do is call getrandom, the Gnulib module 'getrandom-tests' does
> that.

I meant to make sure it works when invoked from Emacs.  Sorry for not
making that clear.

bug#42095: 28.0.50; Build fails on Windows/MinGW64

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 121 bytes --]

On 6/29/20 7:52 AM, Eli Zaretskii wrote:
> I meant to make sure it works when invoked from Emacs.

I added the attached.

[-- Attachment #2: 0001-test-src-fns-tests.el-test-secure-hash-Test-getrando.patch --]
[-- Type: text/x-patch, Size: 1131 bytes --]

From d0872638b4f6330bdece465d6cbf5c1d85306c35 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 29 Jun 2020 09:54:45 -0700
Subject: [PATCH] * test/src/fns-tests.el (test-secure-hash): Test getrandom
 format.

---
 test/src/fns-tests.el | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/test/src/fns-tests.el b/test/src/fns-tests.el
index b65543a64b..f1faf58659 100644
--- a/test/src/fns-tests.el
+++ b/test/src/fns-tests.el
@@ -890,6 +890,8 @@ test-secure-hash
   (should (equal (secure-hash 'sha512 "foobar")
                  (concat "0a50261ebd1a390fed2bf326f2673c145582a6342d5"
                          "23204973d0219337f81616a8069b012587cf5635f69"
-                         "25f1b56c360230c19b273500ee013e030601bf2425"))))
-
-(provide 'fns-tests)
+                         "25f1b56c360230c19b273500ee013e030601bf2425")))
+  ;; Test that a call to getrandom returns the right format.
+  ;; This does not test randomness; it's merely a format check.
+  (should (string-match "\\`[0-9a-f]\\{128\\}\\'"
+                        (secure-hash 'sha512 'iv-auto 100))))
-- 
2.17.1

bug#42095: 28.0.50; Build fails on Windows/MinGW64

[Eli Zaretskii]

> Cc: peder@klingenberg.no, 42095@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Mon, 29 Jun 2020 09:56:42 -0700
> 
> > I meant to make sure it works when invoked from Emacs.
> 
> I added the attached.

Thanks, the test passes on MS-Windows.

bug#42095: 28.0.50; Build fails on Windows/MinGW64

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

[ccing bug-gnulib; this is about <https://bugs.gnu.org/42095>.]

> The latest changes made this code run even in the MinGW build, not
> sure why (perhaps because 'getrandom' uses 'open'?).

That's part of it. It's also needed for recent changes to the getloadavg module.
And I see that the at-internal module also depends on the 'open' module, I
presume because lib/openat-proc.c opens /proc/self/fd/NNN/, but this surely
won't work in mingw.

As I understand it the basic issue here is that we want O_CLOEXEC support even
on platforms that lack it, and we don't want all the calling software to play
games with "#ifdef O_CLOEXEC" and the like.

I attempted to work around this particular problem by installing the attached
workaround into Gnulib and updating Emacs accordingly. Please give it a try.
Perhaps this should be fixed more systematically in Gnulib instead of worked
around, but I suppose that might entail some merging between Emacs's and
Gnulib's ways of dealing with file names under MS-Windows and I'll leave it to
the MS-Windows experts to figure out how to do that, or whether they want to do
it at all.

[-- Attachment #2: 0001-getrandom-do-not-depend-on-open-on-mingw.patch --]
[-- Type: text/x-patch, Size: 3678 bytes --]

From a01be1645717a029e571fd0dbb42245f05aff88c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 28 Jun 2020 13:18:11 -0700
Subject: [PATCH] =?UTF-8?q?getrandom:=20do=20not=20depend=20on=20=E2=80=98?=
 =?UTF-8?q?open=E2=80=99=20on=20mingw?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Similarly for at-internal, getloadavg.  These modules do not call
the ‘open’ function when they are compiled on mingw.  On mingw,
this avoids having to compile open.c when building Emacs, which
does its own thing with ‘open’.
* modules/at-internal, modules/getloadavg, modules/getrandom:
(Depends-on): Don’t depend on ‘open’ on mingw.
(Depends-on): Require AC_CANONICAL_HOST, for host_os.
* modules/getloadavg (Depends-on):
Depend on intprops, open, stdbool, stdlib only if compiling
getloadavg.c.
---
 ChangeLog           | 14 ++++++++++++++
 modules/at-internal |  3 ++-
 modules/getloadavg  |  9 +++++----
 modules/getrandom   |  3 ++-
 4 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 84a1bd472..c6e26120e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2020-06-28  Paul Eggert  <eggert@cs.ucla.edu>
+
+	getrandom: do not depend on ‘open’ on mingw
+	Similarly for at-internal, getloadavg.  These modules do not call
+	the ‘open’ function when they are compiled on mingw.  On mingw,
+	this avoids having to compile open.c when building Emacs, which
+	does its own thing with ‘open’.
+	* modules/at-internal, modules/getloadavg, modules/getrandom:
+	(Depends-on): Don’t depend on ‘open’ on mingw.
+	(Depends-on): Require AC_CANONICAL_HOST, for host_os.
+	* modules/getloadavg (Depends-on):
+	Depend on intprops, open, stdbool, stdlib only if compiling
+	getloadavg.c.
+
 2020-06-28  Bruno Haible  <bruno@clisp.org>
 
 	doc: Add a note about sigprocmask vs. pthread_sigmask.
diff --git a/modules/at-internal b/modules/at-internal
index d58c9b0b4..2a37779f7 100644
--- a/modules/at-internal
+++ b/modules/at-internal
@@ -9,11 +9,12 @@ Depends-on:
 errno
 fcntl-h
 intprops
-open
+open       [case $host_os in mingw*) false;; *) :;; esac]
 sys_stat
 unistd
 
 configure.ac:
+AC_REQUIRE([AC_CANONICAL_HOST])
 
 Makefile.am:
 lib_SOURCES += openat-priv.h openat-proc.c
diff --git a/modules/getloadavg b/modules/getloadavg
index 8adb9a784..21955b782 100644
--- a/modules/getloadavg
+++ b/modules/getloadavg
@@ -7,12 +7,13 @@ m4/getloadavg.m4
 
 Depends-on:
 extensions
-intprops
-open
-stdbool
-stdlib
+intprops  [test $HAVE_GETLOADAVG = 0]
+open      [case $host_os in mingw*) false;; *) test $HAVE_GETLOADAVG = 0;; esac]
+stdbool   [test $HAVE_GETLOADAVG = 0]
+stdlib    [test $HAVE_GETLOADAVG = 0]
 
 configure.ac:
+AC_REQUIRE([AC_CANONICAL_HOST])
 gl_GETLOADAVG
 if test $HAVE_GETLOADAVG = 0; then
   AC_LIBOBJ([getloadavg])
diff --git a/modules/getrandom b/modules/getrandom
index 76437eb5a..f1c359706 100644
--- a/modules/getrandom
+++ b/modules/getrandom
@@ -9,9 +9,10 @@ Depends-on:
 sys_random
 fcntl-h             [test $HAVE_GETRANDOM = 0 || test $REPLACE_GETRANDOM = 1]
 minmax              [test $HAVE_GETRANDOM = 0 || test $REPLACE_GETRANDOM = 1]
-open                [test $HAVE_GETRANDOM = 0 || test $REPLACE_GETRANDOM = 1]
+open                [case $host_os in mingw*) false;; *) test $HAVE_GETRANDOM = 0 || test $REPLACE_GETRANDOM = 1;; esac]
 
 configure.ac:
+AC_REQUIRE([AC_CANONICAL_HOST])
 gl_FUNC_GETRANDOM
 if test $HAVE_GETRANDOM = 0 || test $REPLACE_GETRANDOM = 1; then
   AC_LIBOBJ([getrandom])
-- 
2.17.1

bug#42095: 28.0.50; Build fails on Windows/MinGW64

[Eli Zaretskii]

> Cc: 42095@debbugs.gnu.org, "Peder O. Klingenberg" <peder@klingenberg.no>,
>  Gnulib bugs <bug-gnulib@gnu.org>
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 28 Jun 2020 13:34:36 -0700
> 
> As I understand it the basic issue here is that we want O_CLOEXEC support even
> on platforms that lack it, and we don't want all the calling software to play
> games with "#ifdef O_CLOEXEC" and the like.

AFAICT, O_CLOEXEC is already supported by the 'open' implementation we
use on MS-Windows ('sys_open' in w32.c), so I think we are good there.

> I attempted to work around this particular problem by installing the attached
> workaround into Gnulib and updating Emacs accordingly. Please give it a try.

Thanks, it builds fine.

> Perhaps this should be fixed more systematically in Gnulib instead of worked
> around, but I suppose that might entail some merging between Emacs's and
> Gnulib's ways of dealing with file names under MS-Windows and I'll leave it to
> the MS-Windows experts to figure out how to do that, or whether they want to do
> it at all.

As long as config.h is included in a Gnulib source file that calls
'open' (and AFAIK all Gnulib files do that), the call will be
redirected to 'sys_open' mentioned above, and everything is supposed
to "just work".

bug#42095: 28.0.50; Build fails on Windows/MinGW64

[Paul Eggert]

On 6/29/20 7:13 AM, Eli Zaretskii wrote:

> Thanks, it builds fine.

Thanks for checking; closing the bug report.

bug#42095: 28.0.50; Build fails on Windows/MinGW64

[Bruno Haible]

> > do we have any tests in our test suite to test the getrandom
> > function?
>
> No. Contributions would be welcome, I assume. Testing randomness is nontrivial,
> though.

Maybe Emacs has no unit test for getrandom(), but gnulib has:

https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=tests/test-getrandom.c;h=0e9fbb0c26b9f54087be4cf7371b0488dba59f4e;hb=HEAD

It does not really check randomness. It only verifies that different calls
produce different results.

If someone wants to provide such tests, I recall that Knuth's TAOCP II
contains the mathematical foundations of such analysis.

Bruno