Makefile: define __sun__ on SunOS
A message that is not in the archive (sent off-list or not yet received): [email protected]
cover enhancing builds on Solaris
From: Brandon Casey <[email protected]>
Junio,
Here is a re-roll of the work on Solaris which integrates the ideas from you and Jeff. This should replace bc/solaris in pu.
This should allow compiling on Solaris with or without a c99 compiler, GCC or SUNWspro.
Solaris 7 should be able to compile when using GCC and bash.
-brandon
Brandon Casey (7):
Makefile: use /usr/ucb/install on SunOS platforms rather than
ginstall
Makefile: add NEEDS_RESOLV to optionally add -lresolv to compile
arguments
diff-delta.c: "diff.h" is not a required include
On Solaris choose the OLD_ICONV iconv() declaration based on the UNIX
spec
git-compat-util.h: tweak the way _XOPEN_SOURCE is set on Solaris
Makefile: define __sun__ on SunOS
Makefile: add section for SunOS 5.7
Junio C Hamano (1):
Makefile: introduce SANE_TOOL_PATH for prepending required elements
to PATH
Makefile | 40 +++++++++++++++++++++++++++++++++++----- diff-delta.c | 1 - git-compat-util.h | 17 ++++++++++++++--- git-sh-setup.sh | 2 ++ utf8.c | 2 +- 5 files changed, 52 insertions(+), 10 deletions(-)
PATCH 1/8 Makefile: use /usr/ucb/install on SunOS platforms rather than ginstall
From: Brandon Casey <[email protected]>
We can avoid a GNU dependency by using /usr/ucb/install.
--- Makefile | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Makefile +1 -1
@@ -726,7 +726,7 @@ ifeq ($(uname_S),SunOS) NO_C99_FORMAT = YesPlease NO_STRTOUMAX = YesPlease endif - INSTALL = ginstall + INSTALL = /usr/ucb/install TAR = gtar BASIC_CFLAGS += -D__EXTENSIONS__ endif
Signature · 1 line
1.6.3.1.24.g152f4
PATCH 2/8 Makefile: add NEEDS_RESOLV to optionally add -lresolv to compile arguments
From: Brandon Casey <[email protected]>
This library is required on Solaris when compiling with NO_IPV6 since hstrerror resides in libresolv. Additionally, Solaris 7 will need it, since inet_ntop and inet_pton reside there too.
--- Makefile | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-)
Makefile +10 -1
@@ -95,6 +95,10 @@ all:: # Define NEEDS_SOCKET if linking with libc is not enough (SunOS, # Patrick Mauritz). # +# Define NEEDS_RESOLV if linking with -lnsl and/or -lsocket is not enough. +# Notably on Solaris hstrerror resides in libresolv and on Solaris 7 +# inet_ntop and inet_pton additionally reside there. +# # Define NO_MMAP if you want to avoid mmap. # # Define NO_PTHREADS if you do not have or do not want to use Pthreads.
@@ -708,7 +712,6 @@ ifeq ($(uname_S),SunOS) SHELL_PATH = /bin/bash NO_STRCASESTR = YesPlease NO_MEMMEM = YesPlease - NO_HSTRERROR = YesPlease NO_MKDTEMP = YesPlease NO_MKSTEMPS = YesPlease ifneq ($(uname_R),5.11)
@@ -726,6 +729,9 @@ ifeq ($(uname_S),SunOS) NO_C99_FORMAT = YesPlease NO_STRTOUMAX = YesPlease endif + ifdef NO_IPV6 + NEEDS_RESOLV = YesPlease + endif INSTALL = /usr/ucb/install TAR = gtar BASIC_CFLAGS += -D__EXTENSIONS__
@@ -981,6 +987,9 @@ endif ifdef NEEDS_NSL EXTLIBS += -lnsl endif +ifdef NEEDS_RESOLV + EXTLIBS += -lresolv +endif ifdef NO_D_TYPE_IN_DIRENT BASIC_CFLAGS += -DNO_D_TYPE_IN_DIRENT endif
Signature · 1 line
1.6.3.1.24.g152f4
PATCH 3/8 diff-delta.c: "diff.h" is not a required include
From: Brandon Casey <[email protected]>
This file (diff.h) provides declarations for some functions that are implemented in diff-delta.c. The SUNWspro C99 compiler complains about it. There is nothing defined in "diff.h" that is required by diff-delta.c, so don't #include it.
--- diff-delta.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff-delta.c +0 -1
@@ -12,7 +12,6 @@*/ #include "git-compat-util.h" -#include "delta.h" /* maximum hash entry list for the same hash bucket */ #define HASH_LIMIT 64
Signature · 1 line
1.6.3.1.24.g152f4
PATCH 4/8 On Solaris choose the OLD_ICONV iconv() declaration based on the UNIX spec
From: Brandon Casey <[email protected]>
OLD_ICONV is only necessary on Solaris until UNIX03. This is indicated by the private macro _XPG6 which is set in /usr/include/sys/feature_tests.h.
--- Makefile | 3 --- utf8.c | 2 +- 2 files changed, 1 insertions(+), 4 deletions(-)
Makefile +0 -3
@@ -714,9 +714,6 @@ ifeq ($(uname_S),SunOS) NO_MEMMEM = YesPlease NO_MKDTEMP = YesPlease NO_MKSTEMPS = YesPlease - ifneq ($(uname_R),5.11) - OLD_ICONV = UnfortunatelyYes - endif ifeq ($(uname_R),5.8) NO_UNSETENV = YesPlease NO_SETENV = YesPlease
utf8.c +1 -1
@@ -354,7 +354,7 @@ int is_encoding_utf8(const char *name)* with iconv. If the conversion fails, returns NULL. */ #ifndef NO_ICONV -#ifdef OLD_ICONV +#if defined(OLD_ICONV ) || (defined(__sun__) && !defined(_XPG6))typedef const char *iconv_ibp ;#else typedef char *iconv_ibp ;
Signature · 1 line
1.6.3.1.24.g152f4
PATCH 5/8 git-compat-util.h: tweak the way _XOPEN_SOURCE is set on Solaris
From: Brandon Casey <[email protected]>
On Solaris, when _XOPEN_EXTENDED is set, its header file forces the programs to be XPG4v2, defeating any _XOPEN_SOURCE setting to say we are XPG5 or XPG6. Also on Solaris, XPG6 programs must be compiled with a c99 compiler, while non XPG6 programs must be compiled with a pre-c99 compiler.
So when compiling on Solaris, always refrain from setting _XOPEN_EXTENDED, and then set _XOPEN_SOURCE to 600 or 500 based on whether a c99 compiler is being used or not.
--- git-compat-util.h | 17 ++++++++++++++--- 1 files changed, 14 insertions(+), 3 deletions(-)
git-compat-util.h +14 -3
@@ -39,12 +39,23 @@/* Approximation of the length of the decimal representation of this type. */ #define decimal_length (x) ((int)(sizeof(x) * 2.56 + 0.5) + 1) -#if !defined(__APPLE__) && !defined(__FreeBSD__) && !defined(__USLC__) && !defined(_M_UNIX) +#if defined(__sun__) +/* +* On Solaris, when _XOPEN_EXTENDED is set, its header file +* forces the programs to be XPG4v2, defeating any _XOPEN_SOURCE +* setting to say we are XPG5 or XPG6. Also on Solaris, +* XPG6 programs must be compiled with a c99 compiler, while +* non XPG6 programs must be compiled with a pre-c99 compiler. +*/ +# if __STDC_VERSION__ -0 >=199901L +# define _XOPEN_SOURCE 600 +# else +# define _XOPEN_SOURCE 500 +# endif +#elif !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
Signature · 1 line
1.6.3.1.24.g152f4
PATCH 6/8 Makefile: define __sun__ on SunOS
From: Brandon Casey <[email protected]>
The SUNWspro compiler does not define __sun__ (like GCC does). A check of this macro was recently added to detect compilation on SunOS and to modify the handling of the NO_ICONV and _XOPEN_SOURCE feature macros.
--- Makefile | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Makefile +1 -1
@@ -731,7 +731,7 @@ ifeq ($(uname_S),SunOS) endif INSTALL = /usr/ucb/install TAR = gtar - BASIC_CFLAGS += -D__EXTENSIONS__ + BASIC_CFLAGS += -D__EXTENSIONS__ -D__sun__ endif ifeq ($(uname_O),Cygwin) NO_D_TYPE_IN_DIRENT = YesPlease
Signature · 1 line
1.6.3.1.24.g152f4
PATCH 7/8 Makefile: introduce SANE_TOOL_PATH for prepending required elements to PATH
From: Junio C Hamano <[email protected]>
Some platforms (like SunOS and family) have kept their common binaries at some historical moment in time, and introduced new binaries with modern features in a special location like /usr/xpg4/bin or /usr/ucb. Some of the features provided by these modern binaries are expected and required by git. If the featureful binaries are not in the users path, then git could end up using the less featureful binary and fail.
So provide a mechanism to prepend elements to the users PATH at runtime so the modern binaries will be found.
--- Makefile | 14 ++++++++++++++ git-sh-setup.sh | 2 ++ 2 files changed, 16 insertions(+), 0 deletions(-)
Makefile +14 -0
@@ -3,6 +3,11 @@ all:: # Define V=1 to have a more verbose compile. # +# Define SHELL_PATH to a POSIX shell if your /bin/sh is broken. +# +# Define SANE_TOOL_PATH to a colon-separated list of paths to prepend +# to PATH if your tools in /usr/bin are broken. +# # Define SNPRINTF_RETURNS_BOGUS if your are on a system which snprintf() # or vsnprintf() return -1 instead of number of characters which would # have been written to the final string if enough space had been available.
@@ -710,6 +715,7 @@ ifeq ($(uname_S),SunOS) NEEDS_SOCKET = YesPlease NEEDS_NSL = YesPlease SHELL_PATH = /bin/bash + SANE_TOOL_PATH = /usr/xpg6/bin:/usr/xpg4/bin NO_STRCASESTR = YesPlease NO_MEMMEM = YesPlease NO_MKDTEMP = YesPlease
@@ -881,6 +887,13 @@ endif -include config.mak.autogen -include config.mak +ifdef SANE_TOOL_PATH +BROKEN_PATH_FIX = s|^. @@PATH@@|PATH=$(SANE_TOOL_PATH)| +PATH := $(SANE_TOOL_PATH):${PATH} +else +BROKEN_PATH_FIX = d +endif + ifeq ($(uname_S),Darwin) ifndef NO_FINK ifeq ($(shell test -d /sw/lib && echo y),y)
@@ -1291,6 +1304,7 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \ -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \ -e 's/@@NO_CURL@@/$(NO_CURL)/g' \ + -e '/^# @@PATH@@/$(BROKEN_PATH_FIX)' \ [email protected] >$@+ && \ chmod +x $@+ && \ mv $@+ $@
git-sh-setup.sh +2 -0
@@ -11,6 +11,8 @@ # exporting it. unset CDPATH +# @@PATH@@:$PATH + die() { echo >&2 "$@" exit 1
Signature · 1 line
1.6.3.1.24.g152f4
PATCH 8/8 Makefile: add section for SunOS 5.7
From: Brandon Casey <[email protected]>
--- Makefile | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
Makefile +10 -0
@@ -720,6 +720,16 @@ ifeq ($(uname_S),SunOS) NO_MEMMEM = YesPlease NO_MKDTEMP = YesPlease NO_MKSTEMPS = YesPlease + ifeq ($(uname_R),5.7) + NEEDS_RESOLV = YesPlease + NO_IPV6 = YesPlease + NO_SOCKADDR_STORAGE = YesPlease + NO_UNSETENV = YesPlease + NO_SETENV = YesPlease + NO_STRLCPY = YesPlease + NO_C99_FORMAT = YesPlease + NO_STRTOUMAX = YesPlease + endif ifeq ($(uname_R),5.8) NO_UNSETENV = YesPlease NO_SETENV = YesPlease
Signature · 1 line
1.6.3.1.24.g152f4
Re: [PATCH 7/8] Makefile: introduce SANE_TOOL_PATH for prepending required elements to PATH
In reply to Brandon CaseyQuoted 11 lines · Brandon Casey
From: Junio C Hamano <[email protected]>
Some platforms (like SunOS and family) have kept their common binaries at some historical moment in time, and introduced new binaries with modern features in a special location like
/usr/xpg4/binor/usr/ucb. Some of the features provided by these modern binaries are expected and required by git. If the featureful binaries are not in the users path, then git could end up using the less featureful binary and fail.So provide a mechanism to prepend elements to the users PATH at runtime so the modern binaries will be found.
So this bit me already, and it's only been in next for a day. :) I _already_ have /usr/xpg4/bin in my PATH before /usr/bin, but with this patch, I get it stuck at the _beginning_ of my PATH automagically. Which overrides, against my wishes, the "even more sane than /usr/xpg4/bin" part of my PATH that comes at the beginning.
Specifically, I have "~peff/local/bin" at the beginning of my PATH which contains a 'vi' that points to vim. Running "git rebase -i" now puts /usr/xpg4/bin at the beginning of the PATH (before ~peff/local/bin), which means I end up running the crappy system vi instead. For bonus fun, "git commit" still runs the correct 'vi' because it doesn't happen to be implemented as a shell script.
Am I crazy for not having EDITOR=vim instead of EDITOR=vi? Perhaps. But I wanted to point out that tweaking the PATH behind the user's back does cause surprises in the real world.
-Peff
Re: [PATCH 7/8] Makefile: introduce SANE_TOOL_PATH for prepending required elements to PATH
Quoted 11 lines · Jeff King
On Fri, Jun 05, 2009 at 06:36:15PM -0500, Brandon Casey wrote:
...Quoted 2 lines
So provide a mechanism to prepend elements to the users PATH at runtime so the modern binaries will be found.
So this bit me already, and it's only been in next for a day. :) I _already_ have
/usr/xpg4/binin my PATH before/usr/bin, but with this patch, I get it stuck at the _beginning_ of my PATH automagically. Which overrides, against my wishes, the "even more sane than/usr/xpg4/bin" part of my PATH that comes at the beginning.
Quoted 3 lines
Specifically, I have "~peff/local/bin" at the beginning of my PATH which contains a 'vi' that points to vim. Running "git rebase -i" now puts
/usr/xpg4/binat the beginning of the PATH (before ~peff/local/bin),
In git-sh-setup, we do "unset CDPATH" ourselves to help and protect clueless people, even though "people should have a sane environment". Even though I suspect that anybody who is using Solaris for anything real would not be using /usr/bin tools themselves (i.e. it should not be necessary for us fixing their PATH), there may be people who do not know. I think helping them with path munging falls into the same category, but at the same time, the remedy looks worse than the disease.
We could further uglify the patch like this.
Makefile | 5 +++-- git-sh-setup.sh | 28 +++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-)
Makefile +3 -2
@@ -881,7 +881,8 @@ endif -include config.mak ifdef SANE_TOOL_PATH -BROKEN_PATH_FIX = s|^. @@PATH@@|PATH=$(SANE_TOOL_PATH)| +SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH)) +BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|' PATH := $(SANE_TOOL_PATH):${PATH} else BROKEN_PATH_FIX = d
@@ -1288,7 +1289,7 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \ -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \ -e 's/@@NO_CURL@@/$(NO_CURL)/g' \ - -e '/^# @@PATH@@/$(BROKEN_PATH_FIX)' \ + -e $(BROKEN_PATH_FIX) \ [email protected] >$@+ && \ chmod +x $@+ && \ mv $@+ $@
git-sh-setup.sh +27 -1
@@ -11,7 +11,33 @@ # exporting it. unset CDPATH -# @@PATH@@:$PATH +git_broken_path_fix () { + case ":$PATH:" in + *:$1:*) : ok ;; + *) + PATH=$( + SANE_TOOL_PATH="$1" + IFS=: path= sep= + set x $PATH + shift + for elem + do + case "$SANE_TOOL_PATH:$elem" in + (?*:/bin | ?*:/usr/bin) + path="$path$sep$SANE_TOOL_PATH" + sep=: + SANE_TOOL_PATH= + esac + path="$path$sep$elem" + sep=: + done + echo "$path" + ) + ;; + esac +} + +# @@BROKEN_PATH_FIX@@ die() { echo >&2 "$@"
Re: [PATCH 7/8] Makefile: introduce SANE_TOOL_PATH for prepending required elements to PATH
Quoted 17 lines · Junio C Hamano
We could further uglify the patch like this.
[...]git-sh-setup.sh +15
-0+git_broken_path_fix () { + case ":$PATH:" in + *:$1:*) : ok ;; + *) + PATH=$( + SANE_TOOL_PATH="$1" + IFS=: path= sep= + set x $PATH + shift + for elem + do + case "$SANE_TOOL_PATH:$elem" in + (?*:/bin | ?*:/usr/bin) + path="$path$sep$SANE_TOOL_PATH" + sep=:
git-sh-setup.sh line 38
Quoted 10 lines
git-sh-setup.sh +10
-0+ SANE_TOOL_PATH= + esac + path="$path$sep$elem" + sep=: + done + echo "$path" + ) + ;; + esac +}
Wow. That _is_ ugly, but it actually addresses exactly both my concern and Brandon's. I kind of like it.
-Peff
Re: [PATCH 7/8] Makefile: introduce SANE_TOOL_PATH for prepending required elements to PATH
I never received the referenced email. I'll try to extract from gmane and test.
-brandon
Quoted 34 lines · Jeff King
Quoted 17 lines · Junio C Hamano
We could further uglify the patch like this.
[...]git-sh-setup.sh +15
-0+git_broken_path_fix () { + case ":$PATH:" in + *:$1:*) : ok ;; + *) + PATH=$( + SANE_TOOL_PATH="$1" + IFS=: path= sep= + set x $PATH + shift + for elem + do + case "$SANE_TOOL_PATH:$elem" in + (?*:/bin | ?*:/usr/bin) + path="$path$sep$SANE_TOOL_PATH" + sep=:git-sh-setup.sh line 38
Quoted 10 lines
git-sh-setup.sh +10
-0+ SANE_TOOL_PATH= + esac + path="$path$sep$elem" + sep=: + done + echo "$path" + ) + ;; + esac +}Wow. That _is_ ugly, but it actually addresses exactly both my concern and Brandon's. I kind of like it.
-Peff
Re: [PATCH 7/8] Makefile: introduce SANE_TOOL_PATH for prepending required elements to PATH
Quoted 2 lines · Brandon Casey
I never received the referenced email. I'll try to extract from gmane and test.
This patch works for me.
-brandon
Quoted 39 lines
Quoted 32 lines · Jeff King
Quoted 17 lines · Junio C Hamano
We could further uglify the patch like this.
[...]git-sh-setup.sh +15
-0+git_broken_path_fix () { + case ":$PATH:" in + *:$1:*) : ok ;; + *) + PATH=$( + SANE_TOOL_PATH="$1" + IFS=: path= sep= + set x $PATH + shift + for elem + do + case "$SANE_TOOL_PATH:$elem" in + (?*:/bin | ?*:/usr/bin) + path="$path$sep$SANE_TOOL_PATH" + sep=:git-sh-setup.sh line 38
Quoted 10 lines
git-sh-setup.sh +10
-0+ SANE_TOOL_PATH= + esac + path="$path$sep$elem" + sep=: + done + echo "$path" + ) + ;; + esac +}Wow. That _is_ ugly, but it actually addresses exactly both my concern and Brandon's. I kind of like it.
Quoted 1 line
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] Makefile: introduce SANE_TOOL_PATH for prepending required elements to PATH
In reply to Jeff KingQuoted 27 lines · Jeff King
Quoted 11 lines · Brandon Casey
From: Junio C Hamano <[email protected]>
Some platforms (like SunOS and family) have kept their common binaries at some historical moment in time, and introduced new binaries with modern features in a special location like
/usr/xpg4/binor/usr/ucb. Some of the features provided by these modern binaries are expected and required by git. If the featureful binaries are not in the users path, then git could end up using the less featureful binary and fail.So provide a mechanism to prepend elements to the users PATH at runtime so the modern binaries will be found.
So this bit me already, and it's only been in next for a day. :) I _already_ have
/usr/xpg4/binin my PATH before/usr/bin, but with this patch, I get it stuck at the _beginning_ of my PATH automagically. Which overrides, against my wishes, the "even more sane than/usr/xpg4/bin" part of my PATH that comes at the beginning.Specifically, I have "~peff/local/bin" at the beginning of my PATH which contains a 'vi' that points to vim. Running "git rebase -i" now puts
/usr/xpg4/binat the beginning of the PATH (before ~peff/local/bin), which means I end up running the crappy system vi instead. For bonus fun, "git commit" still runs the correct 'vi' because it doesn't happen to be implemented as a shell script.
Quoted 3 lines
Am I crazy for not having EDITOR=vim instead of EDITOR=vi? Perhaps. But I wanted to point out that tweaking the PATH behind the user's back does cause surprises in the real world.
Good points. I'm fine with dropping this patch, especially when it causes problems for a real Solaris user, which I'm not.
I don't like that git has a dependency on the user's PATH being set correctly though. That's why I liked the patch. I guess I could modify all the uses of sed and friends to look like $SED and then set SED to /usr/xpg4/bin/sed on Solaris. It doesn't sound like that is necessary in practice though.
btw, this patch does help the test suite when the test suite is run using make. The patch added SANE_TOOL_PATH to PATH in the Makefile.
-brandon
Re: [PATCH 7/8] Makefile: introduce SANE_TOOL_PATH for prepending required elements to PATH
Quoted 6 lines · Brandon Casey
Quoted 3 lines
Am I crazy for not having EDITOR=vim instead of EDITOR=vi? Perhaps. But I wanted to point out that tweaking the PATH behind the user's back does cause surprises in the real world.
Good points. I'm fine with dropping this patch, especially when it causes problems for a real Solaris user, which I'm not.
Let me point out that I'm also not a real Solaris user. These days all I use it for is test-compiling git. So you can take my report with a grain of salt.
Quoted 5 lines
I don't like that git has a dependency on the user's PATH being set correctly though. That's why I liked the patch. I guess I could modify all the uses of sed and friends to look like $SED and then set SED to
/usr/xpg4/bin/sedon Solaris. It doesn't sound like that is necessary in practice though.
Yeah, I think requiring the user's PATH to be set correctly and tweaking the PATH behind the user's back are both unsatisfactory solutions. Using $SED everywhere solves both problems, but would probably be quite annoying to maintain. So I guess it is a matter of picking our poison.
-Peff
Re: [PATCH 7/8] Makefile: introduce SANE_TOOL_PATH for prepending required elements to PATH
Quoted 11 lines · Jeff King
Quoted 5 lines · Brandon Casey
Quoted 3 lines
Am I crazy for not having EDITOR=vim instead of EDITOR=vi? Perhaps. But I wanted to point out that tweaking the PATH behind the user's back does cause surprises in the real world.
Good points. I'm fine with dropping this patch, especially when it causes problems for a real Solaris user, which I'm not.
Let me point out that I'm also not a real Solaris user. These days all I use it for is test-compiling git. So you can take my report with a grain of salt.
heh.
Quoted 10 lines
Quoted 5 lines
I don't like that git has a dependency on the user's PATH being set correctly though. That's why I liked the patch. I guess I could modify all the uses of sed and friends to look like $SED and then set SED to
/usr/xpg4/bin/sedon Solaris. It doesn't sound like that is necessary in practice though.Yeah, I think requiring the user's PATH to be set correctly and tweaking the PATH behind the user's back are both unsatisfactory solutions. Using $SED everywhere solves both problems, but would probably be quite annoying to maintain. So I guess it is a matter of picking our poison.
agreed.
-brandon
PATCH v2 3/8 diff-delta.c: "delta.h" is not a required include
In reply to Brandon CaseyFrom: Brandon Casey <[email protected]>
When compiling diff-delta.c with the SUNWspro C99 compiler, it complains
"diff-delta.c", line 314: identifier redeclared: create_delta
There is nothing in "delta.h" that is required by diff-delta.c, so don't include it.
--- diff-delta.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff-delta.c +0 -1
@@ -12,7 +12,6 @@*/ #include "git-compat-util.h" -#include "delta.h" /* maximum hash entry list for the same hash bucket */ #define HASH_LIMIT 64
Signature · 1 line
1.6.3.1.24.g152f4
Nicolas Pitre
NAKRe: [PATCH v2 3/8] diff-delta.c: "delta.h" is not a required include
Quoted 10 lines · Brandon Casey
From: Brandon Casey <[email protected]>
When compiling
diff-delta.cwith the SUNWspro C99 compiler, it complains"
diff-delta.c", line 314: identifier redeclared:create_deltaThere is nothing in "
delta.h" that is required bydiff-delta.c, so don't include it.Signed-off-by Brandon Casey <[email protected]>
NAK.
This is common practice to include the header file declaring function prototypes into the file defining the actual function so to make sure the declaration matches with the definition. Deleting that include is actively ignoring a problem instead of fixing the cause of it.
Nicolas
Re: [PATCH v2 3/8] diff-delta.c: "delta.h" is not a required include
Quoted 11 lines · Nicolas Pitre
Quoted 9 lines · Brandon Casey
From: Brandon Casey <[email protected]>
When compiling
diff-delta.cwith the SUNWspro C99 compiler, it complains"
diff-delta.c", line 314: identifier redeclared:create_deltaThere is nothing in "
delta.h" that is required bydiff-delta.c, so don't include it.
Quoted 8 lines
Quoted 1 line
Signed-off-by Brandon Casey <[email protected]>NAK.
This is common practice to include the header file declaring function prototypes into the file defining the actual function so to make sure the declaration matches with the definition. Deleting that include is actively ignoring a problem instead of fixing the cause of it.
It doesn't seem to like the structure being redeclared with a flex array member and being passed as a const argument.
# cat > test.c <<EOF
struct a_struct;
extern void *test_func(const struct a_struct *f);
struct a_struct {
int a;
int b;
char* c[];
};void *test_func(const struct a_struct *f)
{
return 0;
}EOF
# /opt/SUNWspro/bin/c99 -c test.c
"test.c", line 13: identifier redeclared: test_func
current : function(pointer to const struct a_struct {int a, int b, array[-1] of pointer to char c}) returning pointer to void
previous: function(pointer to const struct a_struct {int a, int b, array[-1] of pointer to char c}) returning pointer to void : "test.c", line 4
c99: acomp failed for test.c
If either the flex array is removed from the structure, or const is removed from test_func argument, test.c will compile. Compiling with -O0 doesn't help.
-brandon
Re: [PATCH v2 3/8] diff-delta.c: "delta.h" is not a required include
Quoted 51 lines · Brandon Casey
Quoted 11 lines · Nicolas Pitre
Quoted 9 lines · Brandon Casey
From: Brandon Casey <[email protected]>
When compiling
diff-delta.cwith the SUNWspro C99 compiler, it complains"
diff-delta.c", line 314: identifier redeclared:create_deltaThere is nothing in "
delta.h" that is required bydiff-delta.c, so don't include it.Quoted 8 lines
Quoted 1 line
Signed-off-by Brandon Casey <[email protected]>NAK.
This is common practice to include the header file declaring function prototypes into the file defining the actual function so to make sure the declaration matches with the definition. Deleting that include is actively ignoring a problem instead of fixing the cause of it.
It doesn't seem to like the structure being redeclared with a flex array member and being passed as a const argument.
# cat > test.c <<EOFstruct
a_struct;extern void *test_func(const struct
a_struct*f);struct a_struct { int a; int b; char* c[]; };void *test_func(const struct
a_struct*f){ return 0; }EOF
#
/opt/SUNWspro/bin/c99-ctest.c
"test.c", line 13: identifier redeclared:test_func
current :function(pointer to const struct a_struct {int a, int b, array[-1] of pointer to char c})returning pointer to void
previous:function(pointer to const struct a_struct {int a, int b, array[-1] of pointer to char c})returning pointer to void : "test.c", line 4
c99: acomp failed fortest.c
Quoted 2 lines
If either the flex array is removed from the structure, or const is removed from
test_funcargument,test.cwill compile. Compiling with -O0 doesn't help.
What if you define FLEX_ARRAY to 1, or even 0?
If neither of those work then I'd simply remove the const. Generated code should be exactly the same with gcc. There is no const with sizeof_delta_index() which is already inconsistent.
Kind of weird nevertheless.
Nicolas
Re: [PATCH v2 3/8] diff-delta.c: "delta.h" is not a required include
Quoted 53 lines · Nicolas Pitre
Quoted 51 lines · Brandon Casey
Quoted 11 lines · Nicolas Pitre
Quoted 9 lines · Brandon Casey
From: Brandon Casey <[email protected]>
When compiling
diff-delta.cwith the SUNWspro C99 compiler, it complains"
diff-delta.c", line 314: identifier redeclared:create_deltaThere is nothing in "
delta.h" that is required bydiff-delta.c, so don't include it.Quoted 8 lines
Quoted 1 line
Signed-off-by Brandon Casey <[email protected]>NAK.
This is common practice to include the header file declaring function prototypes into the file defining the actual function so to make sure the declaration matches with the definition. Deleting that include is actively ignoring a problem instead of fixing the cause of it.
It doesn't seem to like the structure being redeclared with a flex array member and being passed as a const argument.
# cat > test.c <<EOFstruct
a_struct;extern void *test_func(const struct
a_struct*f);struct
a_struct{
int a;
int b;
char* c[];
};void *test_func(const struct
a_struct*f)
{
return 0;
}
EOF#
/opt/SUNWspro/bin/c99-ctest.c
"test.c", line 13: identifier redeclared:test_func
current :function(pointer to const struct a_struct {int a, int b, array[-1] of pointer to char c})returning pointer to void
previous:function(pointer to const struct a_struct {int a, int b, array[-1] of pointer to char c})returning pointer to void : "test.c", line 4
c99: acomp failed fortest.c
Quoted 4 lines
Quoted 2 lines
If either the flex array is removed from the structure, or const is removed from
test_funcargument,test.cwill compile. Compiling with -O0 doesn't help.What if you define
FLEX_ARRAYto 1, or even 0?
I tried that with my test.c example and '1' works, but not '0'. I'll try setting FLEX_ARRAY to 1 and running git's test suite on Monday.
Quoted 5 lines
If neither of those work then I'd simply remove the const. Generated code should be exactly the same with gcc. There is no const with
sizeof_delta_index()which is already inconsistent.Kind of weird nevertheless.
Yes.
-brandon
PATCH 1/1 git-compat-util.h: avoid using c99 flex array feature with Sun compiler 5.8
From: Brandon Casey <[email protected]>
The Sun c99 compiler as recent as version 5.8 Patch 121016-06 2007/08/01 produces an error when compiling diff-delta.c. This source file #includes the delta.h header file which pre-declares a struct which is later defined to contain a flex array member. The Sun c99 compiler fails to compile diff-delta.c and gives the following error:
"diff-delta.c", line 314: identifier redeclared: create_delta
current : function(pointer to const struct delta_index {unsigned long memsize, pointer to const void src_buf, unsigned long src_size, unsigned int hash_mask, array[-1] of pointer to struct index_entry {..} hash}, pointer to const void, unsigned long, pointer to unsigned long, unsigned long) returning pointer to void
previous: function(pointer to const struct delta_index {unsigned long memsize, pointer to const void src_buf, unsigned long src_size, unsigned int hash_mask, array[-1] of pointer to struct index_entry {..} hash}, pointer to const void, unsigned long, pointer to unsigned long, unsigned long) returning pointer to void : "delta.h", line 44
c99: acomp failed for diff-delta.c
So, avoid using this c99 feature when compiling with the Sun c compilers version 5.8 and older (the most recent version tested).
---
This should avoid the flex array problems when using the Sun c99 compiler.
This patch is on top of the new bc/solaris (a7a24ee7).
Since this checks the version of the Sun compiler, it should give Sun the opportunity to fix the compiler in newer releases. If someone has Sun Studio 12? where __SUNPRO_C is set to 0x590, maybe they can test.
-brandon
git-compat-util.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
git-compat-util.h +1 -1
@@ -7,7 +7,7 @@/* * See if our compiler is known to support flexible array members. */ -#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >=199901L ) +#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >=199901L ) && (!defined(__SUNPRO_C) || (__SUNPRO_C >0x580 ))# define FLEX_ARRAY /* empty */ #elif defined(__GNUC__)# if (__GNUC__ >=3 )
Signature · 1 line
1.6.3.1.24.g152f4
Re: [PATCH 2/8] Makefile: add NEEDS_RESOLV to optionally add -lresolv to compile arguments
In reply to Brandon CaseyQuoted 16 lines · Brandon Casey
From: Brandon Casey <[email protected]>
This library is required on Solaris when compiling with
NO_IPV6since hstrerror resides in libresolv. Additionally, Solaris 7 will need it, sinceinet_ntopandinet_ptonreside there too.Signed-off-by Brandon Casey <[email protected]>--- Makefile | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-)Makefile +0
-0
Makefile all:: · line 104
Quoted 10 lines
Makefile +4
-0@@ -95,6 +95,10 @@ all:: # Define NEEDS_SOCKET if linking with libc is not enough (SunOS, # Patrick Mauritz). # +# Define NEEDS_RESOLV if linking with -lnsl and/or -lsocket is not enough. +# Notably on Solaris hstrerror resides in libresolv and on Solaris 7 +# inet_ntop and inet_pton additionally reside there. +# # Define NO_MMAP if you want to avoid mmap. # # Define NO_PTHREADS if you do not have or do not want to use Pthreads.
Could you please add this build configuration variable to configure.ac and config.mak.in, to be able to autodetect this situation?
CC-ed Ralf Wildenhues and David Syzdek (who hopefully can produce autoconf patch to squash with this one).
Signature · 3 lines
Jakub Narebski Poland ShadeHawk on #git
PATCH 1/1 configure: test whether -lresolv is needed
Check if -lresolv is needed for hstrerror; set NEEDS_RESOLV accordingly, and substitute in config.mak.in.
---
* Jakub Narebski wrote on Sat, Jun 06, 2009 at 09:29:34AM CEST:
Quoted 3 lines
CC-ed Ralf Wildenhues and David Syzdek (who hopefully can produce autoconf patch to squash with this one).
Completely untested, but also completely mechanical. HTH.
Cheers,
Ralf
config.mak.in | 1 + configure.ac | 9 +++++++++ 2 files changed, 10 insertions(+), 0 deletions(-)
config.mak.in +1 -0
@@ -33,6 +33,7 @@ NO_EXPAT=@NO_EXPAT@ NO_LIBGEN_H=@NO_LIBGEN_H@ NEEDS_LIBICONV=@NEEDS_LIBICONV@ NEEDS_SOCKET=@NEEDS_SOCKET@ +NEEDS_RESOLV=@NEEDS_RESOLV@ NO_SYS_SELECT_H=@NO_SYS_SELECT_H@ NO_D_INO_IN_DIRENT=@NO_D_INO_IN_DIRENT@ NO_D_TYPE_IN_DIRENT=@NO_D_TYPE_IN_DIRENT@
configure.ac +9 -0
@@ -467,6 +467,15 @@ AC_CHECK_LIB([c], [socket], AC_SUBST(NEEDS_SOCKET) test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket" +# +# Define NEEDS_RESOLV if linking with -lnsl and/or -lsocket is not enough. +# Notably on Solaris hstrerror resides in libresolv and on Solaris 7 +# inet_ntop and inet_pton additionally reside there. +AC_CHECK_LIB([resolv], [hstrerror], +[NEEDS_RESOLV=], +[NEEDS_RESOLV=YesPlease]) +AC_SUBST(NEEDS_RESOLV) +test -n "$NEEDS_RESOLV" && LIBS="$LIBS -lresolv" ## Checks for header files. AC_MSG_NOTICE([CHECKS for header files])
Signature · 1 line
1.6.3.2.199.g31f34
Re: [PATCH 2/8] Makefile: add NEEDS_RESOLV to optionally add -lresolv to compile arguments
In reply to Jakub NarebskiQuoted 23 lines · Jakub Narebski
Quoted 21 lines · Brandon Casey
From: Brandon Casey <[email protected]>
This library is required on Solaris when compiling with
NO_IPV6since hstrerror resides in libresolv. Additionally, Solaris 7 will need it, sinceinet_ntopandinet_ptonreside there too.Signed-off-by Brandon Casey <[email protected]>---
Makefile | 11 ++++++++++-1 files changed, 10 insertions(+), 1 deletions(-)Makefile +2
-0@@ -95,6 +95,10 @@ all:: # Define NEEDS_SOCKET if linking with libc is not enough (SunOS, # Patrick Mauritz). # +# Define NEEDS_RESOLV if linking with -lnsl and/or -lsocket is not enough. +# Notably on Solaris hstrerror resides in libresolv and on Solaris 7
Quoted 8 lines
Makefile all:: · line 104
Quoted 5 lines
Makefile +2
-0+# inet_ntop and inet_pton additionally reside there. +# # Define NO_MMAP if you want to avoid mmap. # # Define NO_PTHREADS if you do not have or do not want to use Pthreads.Could you please add this build configuration variable to configure.ac and config.mak.in, to be able to autodetect this situation?
I'll take a look at it, but autoconf is not a strong suit of mine. Plus, I doubt I will actually be able to test anything on the platforms I have access to that need to set NEEDS_RESOLV, since any autoconf installation is likely to be very old.
Quoted 2 lines
CC-ed Ralf Wildenhues and David Syzdek (who hopefully can produce autoconf patch to squash with this one).
Yes please. :)
FYI:
Solaris 7 needs -lresov since inet_ntop and inet_pton reside there. Additionally, since NO_IPV6 is set, hstrerror is called in connect.c and hstrerror also resides in libresolv.
On more modern Solaris, inet_ntop and inet_pton reside somewhere else, and since NO_IPV6 does not need to be set, -lresolv is not needed.
-brandon
Re: [PATCH 0/8] enhancing builds on Solaris
In reply to Brandon CaseyLooked good except for 3/8 which I did not quite understand.
Re: [PATCH 0/8] enhancing builds on Solaris
Quoted 1 line · Junio C Hamano
Looked good except for 3/8 which I did not quite understand.
I get this error when compiling using the SUNWspro c99 compiler when delta.h is included in diff-delta.c:
"diff-delta.c", line 314: identifier redeclared: create_delta
current : function(pointer to const struct delta_index {unsigned long memsize, pointer to const void src_buf, unsigned long src_size, unsigned int hash_mask, array[-1] of pointer to struct index_entry {..} hash}, pointer to const void, unsigned long, pointer to unsigned long, unsigned long) returning pointer to void
previous: function(pointer to const struct delta_index {unsigned long memsize, pointer to const void src_buf, unsigned long src_size, unsigned int hash_mask, array[-1] of pointer to struct index_entry {..} hash}, pointer to const void, unsigned long, pointer to unsigned long, unsigned long) returning pointer to void : "delta.h", line 44
c99: acomp failed for diff-delta.c
gmake: *** [diff-delta.o] Error 2
I don't see any difference between those two "current" and "previous" statements.
I thought I knew why the error was occurring, but now I don't think I do. There are other function declarations in delta.h that are implemented in diff-delta.c, and those functions are both declared and implemented _before_ create_delta in delta.h and diff-delta.c respectively.
But, there does not seem to be anything declared in delta.h that is required by diff-delta.c.
Maybe the commit message should be shortened to something more like:
The SUNWspro C99 compiler complains: "identifier redeclared: create_delta" when delta.h is included. There is nothing in "delta.h" that is required by diff-delta.c, so don't #include it.
Hmm, well, I just noticed that in the commit message I said "diff.h" everywhere when I meant to say "delta.h". Maybe that is the confusion.
-brandon
Re: [PATCH 0/8] enhancing builds on Solaris
In reply to Brandon CaseyQuoted 3 lines · Brandon Casey
From: Brandon Casey <[email protected]>
This should replace bc/solaris in pu.
Just to be clear, this series is built on top of master.
-brandon
Re: [PATCH 0/8] enhancing builds on Solaris
In reply to Brandon CaseyQuoted 7 lines · Brandon Casey
Here is a re-roll of the work on Solaris which integrates the ideas from you and Jeff. This should replace bc/solaris in pu.
This should allow compiling on Solaris with or without a c99 compiler, GCC or SUNWspro.
Solaris 7 should be able to compile when using GCC and bash.
With the exception of 7/8 (which I already complained about separately), these look reasonable to me, and my setup still passes the same tests (though that is perhaps not saying much, as I was already using gcc).
-Peff