[RFC/PATCH 0/2] OpenSolaris 2008.11 portability fixes

Two small patches that seem to make things better for me on OpenSolaris
2008.11; I am primarily interested in learning if this regresses for other
vintages of Solaris.

Junio C Hamano (2):
  Teach Solaris that _XOPEN_SOURCE=600 really menas XPG6
  OpenSolaris 200811 (SunOS 5.11) does not want OLD_ICONV

 Makefile          |    4 +++-
 git-compat-util.h |    2 ++
 2 files changed, 5 insertions(+), 1 deletions(-)

In git-compat-util.h, we do

    #define _XOPEN_SOURCE 600
    #define _XOPEN_SOURCE_EXTENDED 1

unless we are on BSD or SCO.

On OpenSolaris (200811), /usr/include/sys/feature_tests.h has this nice
table:

    Feature Test Macro				     Specification
    ------------------------------------------------  -------------
    _XOPEN_SOURCE                                         XPG3
    _XOPEN_SOURCE && _XOPEN_VERSION = 4                   XPG4
    _XOPEN_SOURCE && _XOPEN_SOURCE_EXTENDED = 1           XPG4v2
    _XOPEN_SOURCE = 500                                   XPG5
    _XOPEN_SOURCE = 600  (or POSIX_C_SOURCE=200112L)      XPG6

Later in the same header, compilation with -c99 is made to fail if _XPG6 is
not set, like this:

    #if defined(_STDC_C99) && (defined(__XOPEN_OR_POSIX) && !defined(_XPG6))
    #error "Compiler or options invalid for pre-UNIX 03 X/Open applications \
            and pre-2001 POSIX applications"
    #elif ...

The problem is that they check things in an order that is inconvenient for
us.  When they see _XOPEN_SOURCE_EXTENDED, they declare that we are XPG4v2,
regardless of the value of _XOPEN_SOURCE.

To work around this problem, do not define _XOPEN_SOURCE_EXTENDED on
Sun's.

Signed-off-by: Junio C Hamano <redacted>
---
 git-compat-util.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index c7cf2d5..4236647 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -41,8 +41,10 @@
 
 #if !defined(__APPLE__) && !defined(__FreeBSD__)  && !defined(__USLC__) && !defined(_M_UNIX)
 #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */
+#ifndef __sun__
 #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */
 #endif
+#endif
 #define _ALL_SOURCE 1
 #define _GNU_SOURCE 1
 #define _BSD_SOURCE 1

-- 
1.6.3.1.145.gb74d77

Signed-off-by: Junio C Hamano <redacted>
---
 Makefile |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 26d180c..a23846b 100644
--- a/Makefile
+++ b/Makefile

@@ -698,7 +698,9 @@ ifeq ($(uname_S),SunOS)
 	NO_MEMMEM = YesPlease
 	NO_HSTRERROR = YesPlease
 	NO_MKDTEMP = YesPlease
-	OLD_ICONV = UnfortunatelyYes
+	ifneq ($(uname_R),5.11)
+		OLD_ICONV = UnfortunatelyYes
+	endif
 	ifeq ($(uname_R),5.8)
 		NO_UNSETENV = YesPlease
 		NO_SETENV = YesPlease

-- 
1.6.3.1.145.gb74d77

Junio C Hamano wrote:

__sun__ is not defined by the SUNWspro compiler.  We can fix that by adding
a -D__sun__ in the SunOS section of the Makefile though.

A more important issue, is that now that this causes _XPG6 to be defined,
sun's header files _require_ a c99 compiler to be used.

Here's the comment that goes along with the partial snippet that you showed
above from /usr/include/sys/feature_tests.h:

  /*
   * It is invalid to compile an XPG3, XPG4, XPG4v2, or XPG5 application
   * using c99.  The same is true for POSIX.1-1990, POSIX.2-1992, POSIX.1b,
   * and POSIX.1c applications. Likewise, it is invalid to compile an XPG6
   * or a POSIX.1-2001 application with anything other than a c99 or later
   * compiler.  Therefore, we force an error in both cases.
   */

And the rest of the macro implements the check for source/compiler
suitability.

So we either require compiling with a c99 compiler (by ensuring that
_XPG6 is set) or exclude compiling with a c99 compiler on sun (by
ensuring that _XPG6 is not set).  Actually, this would only affect Solaris
versions which support XPG6.  Solaris 11 and 10 do.  I don't know about
Solaris 9 and 8.  Solaris 7 doesn't.

Which do we want to do?

 #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */
 #endif
+#endif
 #define _ALL_SOURCE 1
 #define _GNU_SOURCE 1
 #define _BSD_SOURCE 1

On Thu, May 28, 2009 at 11:46:50AM -0500, Brandon Casey wrote:

So we either require compiling with a c99 compiler (by ensuring that
_XPG6 is set) or exclude compiling with a c99 compiler on sun (by
ensuring that _XPG6 is not set).  Actually, this would only affect Solaris
versions which support XPG6.  Solaris 11 and 10 do.  I don't know about
Solaris 9 and 8.  Solaris 7 doesn't.

Which do we want to do?

FWIW, I didn't even try using sun's cc. I have been doing all of my
builds using gcc 3.1.1 (from around 2002, which is what happens to be
available on the old-ish Solaris install at my university).

-Peff

Jeff King wrote:

On Thu, May 28, 2009 at 11:46:50AM -0500, Brandon Casey wrote:

quoted

So we either require compiling with a c99 compiler (by ensuring that
_XPG6 is set) or exclude compiling with a c99 compiler on sun (by
ensuring that _XPG6 is not set).  Actually, this would only affect Solaris
versions which support XPG6.  Solaris 11 and 10 do.  I don't know about
Solaris 9 and 8.  Solaris 7 doesn't.

Which do we want to do?

FWIW, I didn't even try using sun's cc. I have been doing all of my
builds using gcc 3.1.1 (from around 2002, which is what happens to be
available on the old-ish Solaris install at my university).

It's not the compiler that prevents compilation.  It's the header files.

You mentioned that on Solaris 8 there was only a single declaration for
iconv.  There was not a macro check for _XPG6.  I suspect that Solaris 8
does not support XPG6, so your compilation is just falling back to XPG4.
The same thing happens on Solaris 7.  On Solaris 10, if the macros are
set such that _XPG6 becomes set, then compilation will fail if the
compiler is not a c99 one.

-brandon

On Thu, May 28, 2009 at 02:40:14PM -0500, Brandon Casey wrote:

It's not the compiler that prevents compilation.  It's the header files.

You mentioned that on Solaris 8 there was only a single declaration for
iconv.  There was not a macro check for _XPG6.  I suspect that Solaris 8
does not support XPG6, so your compilation is just falling back to XPG4.

OK, that makes sense.

The same thing happens on Solaris 7.  On Solaris 10, if the macros are
set such that _XPG6 becomes set, then compilation will fail if the
compiler is not a c99 one.

Right. What I was trying to say is "gcc 3.1.1 is probably c99 enough,
and I am using it already". That is, if you were asking how painful it
would be to require a c99 compiler for Solaris git, I am saying I don't
personally care.

But I don't know what is "normal".

-Peff

On May 23, 2009, at 9:24 PM, Junio C Hamano wrote:

Until this commit a simple 'make prefix=...' worked just fine. Now I  
have to explicitly add '-std=c99' to the gcc commandline. Is there a  
reason why the makefile doesn't add this switch automatically?

tom

Tomas Carnecky wrote:

On May 23, 2009, at 9:24 PM, Junio C Hamano wrote:

quoted

diff --git a/git-compat-util.h b/git-compat-util.h
index c7cf2d5..4236647 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -41,8 +41,10 @@

#if !defined(__APPLE__) && !defined(__FreeBSD__)  &&
!defined(__USLC__) && !defined(_M_UNIX)
#define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD
needs 600 for S_ISLNK() */
+#ifndef __sun__
#define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */
#endif
+#endif
#define _ALL_SOURCE 1
#define _GNU_SOURCE 1
#define _BSD_SOURCE 1
-- 
1.6.3.1.145.gb74d77

Until this commit a simple 'make prefix=...' worked just fine. Now I
have to explicitly add '-std=c99' to the gcc commandline. Is there a
reason why the makefile doesn't add this switch automatically?

There are additionally patches on 'next' and more in the pipeline on 'pu'
which adjust these feature macros when compiling on Solaris.  See the
bc/solaris series 8fccb00 which was merged to 'pu'.

Also, if you happen to be using the Sun Studio suite 12 with c-compiler
version 5.9, I'd be interested to know whether you can compile diff-delta.c,
or whether you get an error (see commit 203ee91f).

-brandon

On Jun 11, 2009, at 5:50 PM, Brandon Casey wrote:

There are additionally patches on 'next' and more in the pipeline on  
'pu'
which adjust these feature macros when compiling on Solaris.  See the
bc/solaris series 8fccb00 which was merged to 'pu'.

Alright, just wanted to make sure that issue is known.

Also, if you happen to be using the Sun Studio suite 12 with c- 
compiler
version 5.9, I'd be interested to know whether you can compile diff- 
delta.c,
or whether you get an error (see commit 203ee91f).

I happen to have access to some of my university's solaris boxes with  
the following compiler: Sun C 5.9 SunOS_sparc Patch 124867-02  
2007/11/27, I hope that is good enough.

$ gmake CC=/opt/SUNWspro/bin/c99 CFLAGS="" OPENSSLDIR=/usr/sfw V=1  
diff-delta.o
GIT_VERSION = 1.6.3.2.354.g5787c
     * new build flags or prefix
/opt/SUNWspro/bin/c99 -o diff-delta.o -c   -D__EXTENSIONS__ -D__sun__ - 
I/usr/sfw/include -DSHA1_HEADER='<openssl/sha.h>'  -DNO_STRCASESTR - 
DNO_MKDTEMP -DNO_MKSTEMPS -DNO_MEMMEM diff-delta.c
$ echo $?
0

(btw, I have to clear CFLAGS when compiling with the sun compiler, as  
it doesn't understand -Wall)

tom

Tomas Carnecky wrote:

On Jun 11, 2009, at 5:50 PM, Brandon Casey wrote:

quoted

There are additionally patches on 'next' and more in the pipeline on 'pu'
which adjust these feature macros when compiling on Solaris.  See the
bc/solaris series 8fccb00 which was merged to 'pu'.

Alright, just wanted to make sure that issue is known.

quoted

Also, if you happen to be using the Sun Studio suite 12 with c-compiler
version 5.9, I'd be interested to know whether you can compile
diff-delta.c,
or whether you get an error (see commit 203ee91f).

I happen to have access to some of my university's solaris boxes with
the following compiler: Sun C 5.9 SunOS_sparc Patch 124867-02
2007/11/27, I hope that is good enough.

$ gmake CC=/opt/SUNWspro/bin/c99 CFLAGS="" OPENSSLDIR=/usr/sfw V=1
diff-delta.o
GIT_VERSION = 1.6.3.2.354.g5787c
    * new build flags or prefix
/opt/SUNWspro/bin/c99 -o diff-delta.o -c   -D__EXTENSIONS__ -D__sun__
-I/usr/sfw/include -DSHA1_HEADER='<openssl/sha.h>'  -DNO_STRCASESTR
-DNO_MKDTEMP -DNO_MKSTEMPS -DNO_MEMMEM diff-delta.c
$ echo $?
0

Ok, great.  Looks like Sun fixed the flaw that was present in the 5.8
compiler, and no other changes need to be made to git.  To be absolutely
sure, and if you have a moment, can you try to compile the code snippet at
the end of this email?

If you name the saved the code "test.c", then just compile with

   /opt/SUNWspro/bin/c99 -c test.c

The Sun C 5.8 compiler complains like this for me:

   "test.c", line 12: identifier redeclared: test_func
           current : function(pointer to const struct a_struct {int b, array[-1] of pointer to char c}) returning pointer to void
           previous: function(pointer to const struct a_struct {int b, array[-1] of pointer to char c}) returning pointer to void : "test.c", line 4
   c99: acomp failed for test.c

If the 5.9 compiler successfully compiles it, then this new version of Sun's
compiler correctly handles c99 flex arrays.

(btw, I have to clear CFLAGS when compiling with the sun compiler, as it
doesn't understand -Wall)

Yes, that's correct.

-brandon


--->8--- test.c --->8---
struct a_struct;

extern void *test_func(const struct a_struct *f);

struct a_struct {
        int b;
        char* c[];
};

void *test_func(const struct a_struct *f)
{
        return 0;
}

On Jun 11, 2009, at 7:13 PM, Brandon Casey wrote:

Ok, great.  Looks like Sun fixed the flaw that was present in the 5.8
compiler, and no other changes need to be made to git.  To be  
absolutely
sure, and if you have a moment, can you try to compile the code  
snippet at
the end of this email?

If you name the saved the code "test.c", then just compile with

  /opt/SUNWspro/bin/c99 -c test.c

The Sun C 5.8 compiler complains like this for me:

  "test.c", line 12: identifier redeclared: test_func
          current : function(pointer to const struct a_struct {int  
b, array[-1] of pointer to char c}) returning pointer to void
          previous: function(pointer to const struct a_struct {int  
b, array[-1] of pointer to char c}) returning pointer to void :  
"test.c", line 4
  c99: acomp failed for test.c

If the 5.9 compiler successfully compiles it, then this new version  
of Sun's
compiler correctly handles c99 flex arrays.

It compiles without errors.

tom

Tomas Carnecky wrote:

On Jun 11, 2009, at 7:13 PM, Brandon Casey wrote:

quoted

Ok, great.  Looks like Sun fixed the flaw that was present in the 5.8
compiler, and no other changes need to be made to git.  To be absolutely
sure, and if you have a moment, can you try to compile the code
snippet at
the end of this email?

If you name the saved the code "test.c", then just compile with

  /opt/SUNWspro/bin/c99 -c test.c

The Sun C 5.8 compiler complains like this for me:

  "test.c", line 12: identifier redeclared: test_func
          current : function(pointer to const struct a_struct {int b,
array[-1] of pointer to char c}) returning pointer to void
          previous: function(pointer to const struct a_struct {int b,
array[-1] of pointer to char c}) returning pointer to void : "test.c",
line 4
  c99: acomp failed for test.c

If the 5.9 compiler successfully compiles it, then this new version of
Sun's
compiler correctly handles c99 flex arrays.

It compiles without errors.

Great.  Thanks.

-brandon