Re: m4-1.4.7: inconsistency in src/m4.h

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Martin,

[Re-adding bug-m4, and CC'ing m4-patches since there is a patch.  Please
keep replies on the list, so that others may see the resolution of this
thread.]

According to Martin Koeppe on 10/13/2006 3:36 PM:
> 
> On Fri, 13 Oct 2006 address@hidden wrote:
> 
>> Martin Koeppe wrote:
>>>
>>> in src/m4.h I found, that the test for _POSIX_VERSION should only be
>>> made, after unistd.h has been included. Otherwise the test should be
>>> omitted, as _POSIX_VERSION gets #defined in <unistd.h>.
>>
>> Thanks for the report.  Which platform was this on?  Did the test
>> suite still pass in spite of this?  The goal of that check is to find
>> if any one of a number of macros is defined, in order that the rest of
>> the code can assume Unix semantics and define the __unix__ m4 macro. 
>> If that macro still ended up defined on your platform, then the test
>> suite will pass, and I don't need to worry about this. But if it
>> caused a testsuite failure, then I am more than willing to rearrange
>> the code slightly.
> 
> The test suite failed, that's why I looked into it. The platform is
> Interix 3.5 aka Services for Unix. But that's not the point.

You're probably the first person to report a build on Interix.  I
personally have not tried Interix, so I'm relying on help from others like
you if you want things resolved.

> 
> If I just wanted it to compile, I could use CPPFLAGS=-Dunix or such, but
> I think, if there is already a test for _POSIX_VERSION, then there
> should be cases where this test is successful. Interix's <unistd.h> has
> _POSIX_VERSION, so that should be evaluated then. On my Linux,
> _POSIX_VERSION is also #defined in <unistd.h>, so it seems to be in a
> standard location. So why not include it, before the test is done?

OK, you are right that POSIX states that <unistd.h> must define
_POSIX_VERSION.  And non-POSIX platforms, such as Mingw, are in their best
interest to advertise that they are not POSIX compliant by having
<unistd.h> leave this undefined.

My question is whether having m4 provide the builtin __unix__ is even the
right thing for Interix.  It's a shame that there are so many viable
porting targets for Windows.  Mingw, for example, implements system(2) by
calling Windows cmd.exe instead of /bin/sh, therefore, I made sure that
when M4 is compiled for mingw, it provides the builtin __windows__ instead
of __unix__.  On the other hand, cygwin intentionally strives for Linux
compatibility, and having m4 provide the builtin __unix__ is absolutely
the right thing to do.  Does Interix provide a /bin/sh that is good enough
for system(2), and therefore m4's syscmd builtin, to really behave like a
true Unix system?  Perhaps it would be better if Interix stuck with
__windows__?  But since Interix does define _POSIX_VERSION in <unistd.h>,
it is worth a shot to let it behave like a unix implementation.

> 
> The problem was on src/builtin.c. The very first #include is "m4.h", and
> there the only #include before the test is "config.h", so no chance for
> <unistd.h> I think.
> 
> On other platforms the compiler may #define unix or such itself, so the
> test succeeds. Indeed if I would use the MS compiler, this would be the
> case according to docs, but with gcc, it apparently isn't.

gcc has a set of pre-defined macros according to the platform it is
targeting.  The fact that "unix" is not one of them on Interix is an
interesting observation; especially since on Cygwin, gcc DOES define
"unix" by default.

> 
>> By the way, M4 can include <unistd.h> blindly, thanks to gnulib
>> portability code (and in fact, already does so, via "unistd--.h").  There
>> is no need to check for HAVE_UNISTD_H, only to rearrange the
>> gnulib header inclusion to occur prior to the platform macro detection
>> section.
> 
> Ok, that's up to you. I didn't look that deep into it.
> 
> Martin
> 

Here's what I'm installing.  Let me know if you would be willing to try a
snapshot tarball on Interix before I release 1.4.8 in the next few weeks.

2006-10-13  Eric Blake  <address@hidden>

        * src/m4.h (UNIX, W32_NATIVE, OS2): Move platform checks after
        header files are included, since <unistd.h> can affect the tests.
        * THANKS: Updated.
        Reported by Martin Koeppe.

- --
Life is short - so eat dessert first!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFFMGPs84KuGfSFAYARAtiKAKCB2wDtqt3b50HNXqWABDrb/eKZaQCfU8ZL
BED2fc56dAKO9zL5Lm9M/dA=
=Q0dM
-----END PGP SIGNATURE-----

Index: src/m4.h
===================================================================
RCS file: /sources/m4/m4/src/m4.h,v
retrieving revision 1.1.1.1.2.30
diff -u -p -r1.1.1.1.2.30 m4.h
--- src/m4.h    13 Oct 2006 22:25:32 -0000      1.1.1.1.2.30
+++ src/m4.h    14 Oct 2006 04:07:37 -0000
@@ -25,24 +25,6 @@
 
 #include <config.h>
 
-/* Canonicalize UNIX recognition macros.  */
-#if defined unix || defined __unix || defined __unix__ \
-  || defined _POSIX_VERSION || defined _POSIX2_VERSION \
-  || defined __NetBSD__ || defined __OpenBSD__ \
-  || defined __APPLE__ || defined __APPLE_CC__
-# define UNIX 1
-#endif
-
-/* Canonicalize Windows recognition macros.  */
-#if (defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__
-# define W32_NATIVE 1
-#endif
-
-/* Canonicalize OS/2 recognition macro.  */
-#ifdef __EMX__
-# define OS2 1
-#endif
-
 #include <ctype.h>
 #include <errno.h>
 #include <string.h>
@@ -64,6 +46,24 @@
 #include "xalloc.h"
 #include "xvasprintf.h"
 
+/* Canonicalize UNIX recognition macros.  */
+#if defined unix || defined __unix || defined __unix__ \
+  || defined _POSIX_VERSION || defined _POSIX2_VERSION \
+  || defined __NetBSD__ || defined __OpenBSD__ \
+  || defined __APPLE__ || defined __APPLE_CC__
+# define UNIX 1
+#endif
+
+/* Canonicalize Windows recognition macros.  */
+#if (defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__
+# define W32_NATIVE 1
+#endif
+
+/* Canonicalize OS/2 recognition macro.  */
+#ifdef __EMX__
+# define OS2 1
+#endif
+
 /* If FALSE is defined, we presume TRUE is defined too.  In this case,
    merely typedef boolean as being int.  Or else, define these all.  */
 #ifndef FALSE