Jonathan Nieder <jrnieder@xxxxxxxxx> writes: > Brad Smith wrote: > >> Solaris 11.0 and newer have mkdtemp(), memmem(), strcasestr() >> and strtoumax(). >> >> Signed-off-by: Brad Smith <brad@xxxxxxxxxxxx> >> --- >> 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@xxxxxxxxx> > > [...] >> --- 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.
