nghttp2 @1.24.0 does not build on PPC Leopard, Mac OS X 10.5.8, with configure.cxx_stdlib = macports-libstdc++ and macports-gcc-6 because "'AI_NUMERICSERV' was not declared in this scope"

[ballapete (Peter "Pete" Dyballa)]

I added to Portfile

  4 PortGroup           cxx11 1.1

and the build started. It ended here:

/opt/local/bin/g++-mp-6 -DHAVE_CONFIG_H -I. -I..  -DPKGDATADIR='"/opt/local/share/nghttp2"' -I../lib/includes -I../lib/includes -I../lib -I../src/includes -I../third-party -I/opt/local/include/libxml2 -I/opt/local/include/openssl -I/opt/local/include -DHAVE_CONFIG_H   -I/opt/local/include   -pipe -Os -D_GLIBCXX_USE_CXX11_ABI=0 -m32 -MT libnghttpx_a-shrpx_config.o -MD -MP -MF .deps/libnghttpx_a-shrpx_config.Tpo -c -o libnghttpx_a-shrpx_config.o `test -f 'shrpx_config.cc' || echo './'`shrpx_config.cc
shrpx_config.cc: In function 'int shrpx::configure_downstream_group(shrpx::Config*, bool, bool, const shrpx::TLSConfig&)':
shrpx_config.cc:3743:61: error: 'AI_NUMERICSERV' was not declared in this scope
   auto resolve_flags = numeric_addr_only ? AI_NUMERICHOST | AI_NUMERICSERV : 0;
                                                             ^~~~~~~~~~~~~~
make[3]: *** [libnghttpx_a-shrpx_config.o] Error 1
make[3]: Leaving directory `/opt/local/var/macports/build/_opt_local_var_macports_sources_nue.de.rsync.macports.org_macports_release_tarballs_ports_www_nghttp2/nghttp2/work/nghttp2-1.24.0/src'
make[2]: *** [all-recursive] Error 1

[ballapete (Peter "Pete" Dyballa)]

main.log with macports-gcc-6

[kencu (Ken)]

It's a missing define in leopard. You have to make a patch to define it to 0. See ​https://trac.macports.org/browser/trunk/dports/multimedia/libmms/files/src_mms-common.h.patch?rev=87000

Or ​https://trac.macports.org/attachment/ticket/27988/patch-src-mms.h.diff

[ballapete (Peter "Pete" Dyballa)]

Replying to kencu:

It's a missing define in leopard. You have to make a patch to define it to 0. See ​https://trac.macports.org/browser/trunk/dports/multimedia/libmms/files/src_mms-common.h.patch?rev=87000

Or ​https://trac.macports.org/attachment/ticket/27988/patch-src-mms.h.diff

nghttp2-1.24.0 does not have a file src/mms.h.

[ballapete (Peter "Pete" Dyballa)]

Only src/shrpx_config.cc has

auto resolve_flags = numeric_addr_only ? AI_NUMERICHOST | AI_NUMERICSERV : 0;

so either the macros should be defined here or directly substituted.

[kencu (Ken)]

Yep, that is the file it goes in (at least there -- you'll find out when you fix that one).

If you wouldn't mind making a patch, nobody would have to come along and fix it again later.

[kencu (Ken)]

BTW, Marius, I could slip a bunch of these missing #defines into snowleopardfixes easily enough as well....

[ballapete (Peter "Pete" Dyballa)]

The folloging patch makes nghttp2 build on PPC Snow Leopard:

--- src/shrpx_config.cc~	2017-07-02 10:46:31.000000000 +0200
+++ src/shrpx_config.cc	2017-07-09 22:29:41.000000000 +0200
@@ -43,6 +43,21 @@
 #endif // HAVE_UNISTD_H
 #include <dirent.h>
 
+#ifdef __APPLE__                                                // this block only for Macs
+# ifndef __MAC_OS_X_VERSION_MIN_REQUIRED                        // are AvailabilityMacros.h or Availability.h not yet included?
+#  if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1050     // then for Leopard and later…
+#   include <Availability.h>                                    // …either include this…
+#  else
+#   include <AvailabilityMacros.h>                              // …or include that
+#  endif
+# endif
+# if __MAC_OS_X_VERSION_MIN_REQUIRED < 1060                     // and for some OS versions do this…
+#  ifndef AI_NUMERICSERV
+#   define AI_NUMERICSERV 0
+#  endif
+# endif                                                         // finish OS version discrimination
+#endif                                                          // finish Apple case
+
 #include <cstring>
 #include <cerrno>
 #include <limits>

Portfile needs a line

patchfiles-append   src-shrpx_config.diff

[ballapete (Peter "Pete" Dyballa)]

Patch for src/shrpx_config.cc

[kencu (Ken)]

beautiful!

[Schamschula (Marius Schamschula)]

In e75e44c81132941f88b76bebe38df115477e17fe/macports-ports:

nghttp2: fix Leopard build

Closes: #54446

[barracuda156]

Replying to ballapete:

The folloging patch makes nghttp2 build on PPC Snow Leopard:

--- src/shrpx_config.cc~	2017-07-02 10:46:31.000000000 +0200
+++ src/shrpx_config.cc	2017-07-09 22:29:41.000000000 +0200
@@ -43,6 +43,21 @@
 #endif // HAVE_UNISTD_H
 #include <dirent.h>
 
+#ifdef __APPLE__                                                // this block only for Macs
+# ifndef __MAC_OS_X_VERSION_MIN_REQUIRED                        // are AvailabilityMacros.h or Availability.h not yet included?
+#  if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1050     // then for Leopard and later…
+#   include <Availability.h>                                    // …either include this…
+#  else
+#   include <AvailabilityMacros.h>                              // …or include that
+#  endif
+# endif
+# if __MAC_OS_X_VERSION_MIN_REQUIRED < 1060                     // and for some OS versions do this…
+#  ifndef AI_NUMERICSERV
+#   define AI_NUMERICSERV 0
+#  endif
+# endif                                                         // finish OS version discrimination
+#endif                                                          // finish Apple case
+
 #include <cstring>
 #include <cerrno>
 #include <limits>

Portfile needs a line

patchfiles-append   src-shrpx_config.diff

Sorry, maybe I miss something here, but this looks a) unnecessarily complicated and b) also wrong (and working just by chance).

This should work fine:

#ifndef AI_NUMERICSERV
#define AI_NUMERICSERV 0
#endif

This is wrong, there should be no leading underscore here: # if __MAC_OS_X_VERSION_MIN_REQUIRED < 1060. Because AvailabilityMacros.h, which is used for Tiger here, does not have those: ​https://github.com/alexey-lysiuk/macos-sdk/blob/a79de83d781f7fb76d329aee456bab9f72f935d1/MacOSX10.4u.sdk/usr/include/AvailabilityMacros.h#L83-L93 This perhaps still works as-is, but simply because __MAC_OS_X_VERSION_MIN_REQUIRED is undefined and defaults to 0, which is still < 1060, so condition is evaluated as true. I do not see any reason to use conditioning on OS versions, however.

[Schamschula (Marius Schamschula)]

Remember, I'm flying blind on legacy MacOS X versions. I have no way of testing any of this. I have to trust PRs/patches submitted to me.

[ballapete (Peter "Pete" Dyballa)]

__MAC_OS_X_VERSION_MIN_REQUIRED vs. MAC_OS_X_VERSION_MIN_REQUIRED comes from working on the pre-compiled C source which helps understanding why the compiler fails. The patch grew so complicated because newer versions of Mac OS X or macOS have AI_NUMERICSERV and I still do not know what it's good for. So I decided to preserve it. ND – natural dumbness.

[barracuda156]

In 176155345f9f1f498e372160c816bc994c295864/macports-ports (master):

nghttp2: update one patch

See: #54446

[barracuda156]

Replying to Schamschula:

Remember, I'm flying blind on legacy MacOS X versions. I have no way of testing any of this. I have to trust PRs/patches submitted to me.

Yeah, I realize that, no issues. It also seemed to have a desired effect (nghttp2 built on old systems), just not in a way it was thought (unless I am wrong here).

[ballapete (Peter "Pete" Dyballa)]

Replying to barracuda156:

Replying to ballapete: […]

This is wrong, there should be no leading underscore here: # if __MAC_OS_X_VERSION_MIN_REQUIRED < 1060. Because AvailabilityMacros.h, which is used for Tiger here, does not have those: ​https://github.com/alexey-lysiuk/macos-sdk/blob/a79de83d781f7fb76d329aee456bab9f72f935d1/MacOSX10.4u.sdk/usr/include/AvailabilityMacros.h#L83-L93 This perhaps still works as-is, but simply because __MAC_OS_X_VERSION_MIN_REQUIRED is undefined and defaults to 0, which is still < 1060, so condition is evaluated as true. I do not see any reason to use conditioning on OS versions, however.

There are some more "faulty" patches out there! Just try

find /opt/local/var/macports/sources/*/macports/release/tarballs/ports/*/*/files -type f -exec grep -n __MAC_OS_X_VERSION_MIN_REQUIRED {} /dev/null \;

or

ggrep -Rn __MAC_OS_X_VERSION_MIN_REQUIRED /opt/local/var/macports/sources/*/macports/release/tarballs/ports/*/*/files

and you'll some more such examples (134 of usage, in 58 patch files). I think I just copied from others…

I had some conftest.c source file from some configure run. Into its main() function I put the lines you don't like and pre-compiled it. I got:

leopard pete 267 /\  grep -n MAC_OS conftest.*
conftest.c:16:#ifndef __MAC_OS_X_VERSION_MIN_REQUIRED                        // are AvailabilityMacros.h or Availability.h not yet included?
conftest.c:17:# if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1050     // then for Leopard and later…
conftest.c:23:#if __MAC_OS_X_VERSION_MIN_REQUIRED < 1060                     // and for some OS versions do this…
conftest.cpp:114:#define __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ 1058
conftest.cpp:168:#define __MAC_OS_X_VERSION_MIN_REQUIRED __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__
conftest.cpp:171:#define __MAC_OS_X_VERSION_MAX_ALLOWED 1058

or, to cite a bit more from the pre-compiled output file conftest.cpp:

167 # 60 "/usr/include/AvailabilityInternal.h" 3 4
168 #define __MAC_OS_X_VERSION_MIN_REQUIRED __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__
169 
170 
171 #define __MAC_OS_X_VERSION_MAX_ALLOWED 1058
172 
173 
174 
175 
176 #define __AVAILABILITY_INTERNAL__MAC_10_6 __AVAILABILITY_INTERNAL_UNAVAILABLE
177 # 79 "/usr/include/AvailabilityInternal.h" 3 4

On PPC Leopard. On PPC Tiger I can check later that the additional guard makes some sense… From "theory" it looks as if __MAC_OS_X_VERSION_MIN_REQUIRED is not #defined on Tiger, so AvailabilityMacros.h gets #included. Which looks like correct behaviour, because afterwards we can be perfectly sure that __MAC_OS_X_VERSION_MIN_REQUIRED is definitely #defined and we can proceed with AI_NUMERICSERV.

[kencu (Ken)]

__MAC_OS_X_VERSION_MIN_REQUIRED is something Jeremy often wanted to use, and he was around a lot at that time.

It is defined on Leopard and newer, and also is defined on any Tiger port that is using legacysupport, as I added it to that.

In general, we should just use MAC_OS_X_VERSION_MIN_REQUIRED instead -- but I didn't want to keep changing Jeremy's patches to that, so I left it most often.

Regarding AI_NUMERICSERV -- I'm not really certain that defining it (to a made-up value of zero or any other number) is a good idea on systems that don't actually support that functionality.

For example, once you define AI_NUMERICSERV 0 then a later block of code looking to see if it should use AI_NUMERICSERV by seeing if it is defined to something will see that it is defined, and so use it -- and this is plain wrong.

[ballapete (Peter "Pete" Dyballa)]

Replying to kencu:

Regarding AI_NUMERICSERV -- I'm not really certain that defining it (to a made-up value of zero or any other number) is a good idea on systems that don't actually support that functionality.

For example, once you define AI_NUMERICSERV 0 then a later block of code looking to see if it should use AI_NUMERICSERV by seeing if it is defined to something will see that it is defined, and so use it -- and this is plain wrong.

To correct this issue would require some work, I think… And understanding!

[ballapete (Peter "Pete" Dyballa)]

IMO it is OK the way it is handled. There are only two places where this macro is used:

./src/shrpx_tls_test.cc:210:  hints.ai_flags = AI_NUMERICHOST | AI_NUMERICSERV;
./src/shrpx_config.cc:4532:  auto resolve_flags = numeric_addr_only ? AI_NUMERICHOST | AI_NUMERICSERV : 0;

numeric_addr_only is a boolean value, the result of the comparison is later used here:

 4583	      if (!addr.dns) {
 4584	        if (resolve_hostname(&addr.addr, addr.host.c_str(), addr.port,
 4585	                             downstreamconf.family, resolve_flags) == -1) {
 4586	          LOG(FATAL) << "Resolving backend address failed: " << hostport;
 4587	          return -1;
 4588	        }

For testing with Ruby it is used too:

./third-party/mruby/mrbgems/mruby-socket/src/const.cstub:52:#if defined(AI_NUMERICHOST)
./third-party/mruby/mrbgems/mruby-socket/src/const.cstub:53:  define_const(AI_NUMERICHOST);
./third-party/mruby/mrbgems/mruby-socket/src/const.cstub:55:#if defined(AI_NUMERICSERV)
./third-party/mruby/mrbgems/mruby-socket/src/const.cstub:56:  define_const(AI_NUMERICSERV);
./third-party/mruby/mrbgems/mruby-socket/src/const.def:19:AI_NUMERICHOST
./third-party/mruby/mrbgems/mruby-socket/src/const.def:20:AI_NUMERICSERV

The last two lines seem to be just items on a long list, which is OK. In const.cstub each define_const(…) line is followed by #endif. So #defineing it artificially could cause a problem – but Ruby is not used here, this code never reached.