[PATCH] config.mak.uname: update settings for Solaris 11

[PATCH] config.mak.uname: update settings for Solaris 11

[Brad Smith]

Solaris 11.0 and newer have mkdtemp(), memmem(), strcasestr()
and strtoumax().

Signed-off-by: Brad Smith <brad@comstyle.com>
---
 config.mak.uname | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/config.mak.uname b/config.mak.uname
index 5a0b4200c3..1e933ead1e 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -190,9 +190,6 @@ ifeq ($(uname_S),SunOS)
 	SHELL_PATH = /bin/bash
 	SANE_TOOL_PATH = /usr/xpg6/bin:/usr/xpg4/bin
 	HAVE_ALLOCA_H = YesPlease
-	NO_STRCASESTR = YesPlease
-	NO_MEMMEM = YesPlease
-	NO_MKDTEMP = YesPlease
 	NO_REGEX = YesPlease
 	NO_MSGFMT_EXTENDED_OPTIONS = YesPlease
 	HAVE_DEV_TTY = YesPlease
@@ -202,7 +199,10 @@ ifeq ($(uname_S),SunOS)
 		NO_IPV6 = YesPlease
 		NO_SOCKADDR_STORAGE = YesPlease
 		NO_UNSETENV = YesPlease
+		NO_MKDTEMP = YesPlease
+		NO_MEMMEM = YesPlease
 		NO_SETENV = YesPlease
+		NO_STRCASESTR = YesPlease
 		NO_STRLCPY = YesPlease
 		NO_STRTOUMAX = YesPlease
 		GIT_TEST_CMP = cmp
@@ -212,23 +212,45 @@ ifeq ($(uname_S),SunOS)
 		NO_IPV6 = YesPlease
 		NO_SOCKADDR_STORAGE = YesPlease
 		NO_UNSETENV = YesPlease
+		NO_MKDTEMP = YesPlease
+		NO_MEMMEM = YesPlease
 		NO_SETENV = YesPlease
+		NO_STRCASESTR = YesPlease
 		NO_STRLCPY = YesPlease
 		NO_STRTOUMAX = YesPlease
 		GIT_TEST_CMP = cmp
         endif
         ifeq ($(uname_R),5.8)
 		NO_UNSETENV = YesPlease
+		NO_MKDTEMP = YesPlease
+		NO_MEMMEM = YesPlease
 		NO_SETENV = YesPlease
+		NO_STRCASESTR = YesPlease
 		NO_STRTOUMAX = YesPlease
 		GIT_TEST_CMP = cmp
         endif
         ifeq ($(uname_R),5.9)
 		NO_UNSETENV = YesPlease
+		NO_MKDTEMP = YesPlease
+		NO_MEMMEM = YesPlease
 		NO_SETENV = YesPlease
+		NO_STRCASESTR = YesPlease
 		NO_STRTOUMAX = YesPlease
 		GIT_TEST_CMP = cmp
         endif
+        ifeq ($(uname_R),5.10)
+		NO_UNSETENV = YesPlease
+		NO_MKDTEMP = YesPlease
+		NO_MEMMEM = YesPlease
+		NO_SETENV = YesPlease
+		NO_STRCASESTR = YesPlease
+		GIT_TEST_CMP = cmp
+        endif
+        ifeq ($(uname_R),5.11)
+		NO_UNSETENV = YesPlease
+		NO_SETENV = YesPlease
+		GIT_TEST_CMP = cmp
+        endif
 	INSTALL = /usr/ucb/install
 	TAR = gtar
 	BASIC_CFLAGS += -D__EXTENSIONS__ -D__sun__
-- 
2.49.0

Re: [PATCH] config.mak.uname: update settings for Solaris 11

[Jonathan Nieder]

Brad Smith wrote:

> Solaris 11.0 and newer have mkdtemp(), memmem(), strcasestr()
> and strtoumax().
>
> Signed-off-by: Brad Smith <brad@comstyle.com>
> ---
>  config.mak.uname | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)

Thanks!  That's from more than 10 years ago, so seems very reasonable
to rely on.  I assume this is tested :), so lgtm.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

[...]
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -190,9 +190,6 @@ ifeq ($(uname_S),SunOS)
>  	SHELL_PATH = /bin/bash
>  	SANE_TOOL_PATH = /usr/xpg6/bin:/usr/xpg4/bin
>  	HAVE_ALLOCA_H = YesPlease
> -	NO_STRCASESTR = YesPlease
> -	NO_MEMMEM = YesPlease
> -	NO_MKDTEMP = YesPlease
>  	NO_REGEX = YesPlease
>  	NO_MSGFMT_EXTENDED_OPTIONS = YesPlease
>  	HAVE_DEV_TTY = YesPlease
> @@ -202,7 +199,10 @@ ifeq ($(uname_S),SunOS)
>  		NO_IPV6 = YesPlease
>  		NO_SOCKADDR_STORAGE = YesPlease
>  		NO_UNSETENV = YesPlease
> +		NO_MKDTEMP = YesPlease
> +		NO_MEMMEM = YesPlease
>  		NO_SETENV = YesPlease
> +		NO_STRCASESTR = YesPlease
>  		NO_STRLCPY = YesPlease
>  		NO_STRTOUMAX = YesPlease
>  		GIT_TEST_CMP = cmp
> @@ -212,23 +212,45 @@ ifeq ($(uname_S),SunOS)
>  		NO_IPV6 = YesPlease
>  		NO_SOCKADDR_STORAGE = YesPlease
>  		NO_UNSETENV = YesPlease
> +		NO_MKDTEMP = YesPlease
> +		NO_MEMMEM = YesPlease
>  		NO_SETENV = YesPlease
> +		NO_STRCASESTR = YesPlease
>  		NO_STRLCPY = YesPlease
>  		NO_STRTOUMAX = YesPlease
>  		GIT_TEST_CMP = cmp
>          endif
>          ifeq ($(uname_R),5.8)
>  		NO_UNSETENV = YesPlease
> +		NO_MKDTEMP = YesPlease
> +		NO_MEMMEM = YesPlease
>  		NO_SETENV = YesPlease
> +		NO_STRCASESTR = YesPlease
>  		NO_STRTOUMAX = YesPlease
>  		GIT_TEST_CMP = cmp
>          endif
>          ifeq ($(uname_R),5.9)
>  		NO_UNSETENV = YesPlease
> +		NO_MKDTEMP = YesPlease
> +		NO_MEMMEM = YesPlease
>  		NO_SETENV = YesPlease
> +		NO_STRCASESTR = YesPlease
>  		NO_STRTOUMAX = YesPlease
>  		GIT_TEST_CMP = cmp
>          endif
> +        ifeq ($(uname_R),5.10)
> +		NO_UNSETENV = YesPlease
> +		NO_MKDTEMP = YesPlease
> +		NO_MEMMEM = YesPlease
> +		NO_SETENV = YesPlease
> +		NO_STRCASESTR = YesPlease
> +		GIT_TEST_CMP = cmp
> +        endif
> +        ifeq ($(uname_R),5.11)
> +		NO_UNSETENV = YesPlease
> +		NO_SETENV = YesPlease
> +		GIT_TEST_CMP = cmp
> +        endif

Not about this change: do we want to retire some of the cases for old
versions at some point, or to collapse them so they can share more?
Seems nice for maintainability.

Sincerely,
Jonathan

Re: [PATCH] config.mak.uname: update settings for Solaris 11

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Brad Smith wrote:
>
>> Solaris 11.0 and newer have mkdtemp(), memmem(), strcasestr()
>> and strtoumax().
>>
>> Signed-off-by: Brad Smith <brad@comstyle.com>
>> ---
>>  config.mak.uname | 28 +++++++++++++++++++++++++---
>>  1 file changed, 25 insertions(+), 3 deletions(-)
>
> Thanks!  That's from more than 10 years ago, so seems very reasonable
> to rely on.  I assume this is tested :), so lgtm.

One thing I didn't check myself is if the new make directive lines
(ifeq and endif) are indented with SPs, not HTs.  I heard that newer
gnumake is pickier than before?

To truly test this you'd have to have access to 5.5 or older, 5.6,
5.7, 5.8, 5.9, 5.10, and 5.11, unless we declare that it is good
enough to eyeball and to see the set of variables for these existing
releases hasn't changed ;-)  At least it does not break 5.10 and 5.11

This patch is good, and I'll queue it as-is; thanks for writing and
reviewing.

Outside the theme of this patch, should we also attach good-until
date on each of these entries, with scheduled deprecation/removal
for old ones?  If [*] and [**] can be believed, 5.6 for example have
met its EOL in July 2006.

[*] https://computernewb.com/wiki/Oracle_Solaris_End_of_Life_Date
[**] https://en.wikipedia.org/wiki/Oracle_Solaris#Version_history

>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
> [...]
>> --- a/config.mak.uname
>> +++ b/config.mak.uname
>> @@ -190,9 +190,6 @@ ifeq ($(uname_S),SunOS)
>>  	SHELL_PATH = /bin/bash
>>  	SANE_TOOL_PATH = /usr/xpg6/bin:/usr/xpg4/bin
>>  	HAVE_ALLOCA_H = YesPlease
> ...
> Not about this change: do we want to retire some of the cases for old
> versions at some point, or to collapse them so they can share more?
> Seems nice for maintainability.

Heh, I should have read to the end of your message.

Marking for retirement is certainly a good idea.

As there are way too more stale entries than entries for supported
releases (which are only 5.10 and 5.11 if [*] can be believed),
restructuring to share more may not be, depending on how it is done.
The current one already says "These are common to all releases" and
then independently list additional variables for each and every
release, so you need to look at only two lists to see which
variables are applicable to one single release.

We cannot change it to "all releases should use these", followed by
"if you are at or older than 5.10, additionally use these", followed
by "if you are at or older than 5.9, additionally use these", ...,
as we may add or remove these variables as releases progress and
gain features, which may lose NO_FROTZ that used to signal the lack
of frotz feature, or may gain HAVE_NITFOL that signals the gain of
nitfol feature, in a newer release.

The current arrangement allows us to get rid the support for a
single release fairly safely by just removing its release-specific
addition part---if it makes some variables in release-specific
addition part shared across all releases, like NO_UNSETENV and
NO_SETENV already are, the result may be redundant and more verbose
than it necessarily is, but cleaning up to move these ones to common
section can be done independently from such a removal.

And the same argument can be made when you add a new release, like
this patch does, to separate the addition of a support and cleaning
up.  It is clear from the patch text that we are losing three from
the commonly shared part and adding them to all the existing ones,
plus writing release specific additions for 5.10 and 5.11.

After the dust settles from this patch, we may want to see if there
are ones like NO_SETENV/NO_UNSETENV/GIT_TEST_CMP that can be shared
across all releases and move them to the commonly shared part as a
follow-up patch.

Thanks.