csiro-coasts / EMS

Environmental Modelling Suite
Other
15 stars 5 forks source link

emslib build fails on Debian 12 with C++12 due to conflict between min and max macro definitions #21

Closed sharon-tickell closed 10 months ago

sharon-tickell commented 1 year ago

I am attempting to build ems v1.5.0 on Debian 12 with all of the latest hdf5 and netcdf libraries for use in RECOM2. With the combination of dependencies specified in this Dockerfile, the build fails due to a conflict between the min and max macros defined in emsmath.h and the macros of the same names defined in stl_algobase.h from the standard libraries.

I've can provide a complete build log if that's needed, but the critical bits are:

mpicc -I./include -g -O2  -I/usr/local/include -o io/nc_buffered.o -c  io/nc_buffered.cpp -D_GLIBCXX_GTHREAD_USE_WEAK=0
In file included from /usr/include/c++/12/bits/specfun.h:45,
                 from /usr/include/c++/12/cmath:1935,
                 from /usr/include/c++/12/math.h:36,
                 from ./include/gridlib.h:58,
                 from ./include/ems.h:52,
                 from io/nc_buffered.cpp:45:
/usr/include/c++/12/bits/stl_algobase.h:278:56: error: macro "min" passed 3 arguments, but takes just 2
  278 |     min(const _Tp& __a, const _Tp& __b, _Compare __comp)
      |                                                        ^
In file included from ./include/ems.h:42:
./include/emsmath.h:37: note: macro "min" defined here
   37 | #define min(x,y) ((x)<(y) ? (x) : (y) )
      | 
/usr/include/c++/12/bits/stl_algobase.h:300:56: error: macro "max" passed 3 arguments, but takes just 2
  300 |     max(const _Tp& __a, const _Tp& __b, _Compare __comp)
      |                                                        ^
./include/emsmath.h:33: note: macro "max" defined here
   33 | #define max(x,y) ((x)>(y) ? (x) : (y) )
      | 

(note: same error with the unwrapped gcc as for mpicc)

The macros in emsmath.h are only conditionally defined, but unfortunately emsmath.h is @included by ems.h before anything includes the standard library, so the emsmath.h versions with an incorrect number of arguments win.

Adding #include <math.h> to the top of emsmath.h so that the standard library macros get defined first appears to be enough to resolve this issue and allow the build to succeed.

frizwi commented 1 year ago

What gcc version does Debian 12 come with? EMS compliance is 10.x at the moment, I think 11.x is okay but hasn't been qualified for anything newer

sharon-tickell commented 1 year ago

@frizwi : https://packages.debian.org/search?searchon=names&keywords=gcc

Debian 12 (bookworm) comes with gcc 12.2 at the moment, while Debian 11 (bullseye) is on 10.2. There is a gcc-11 package for bookworm, but it appears that gcc-10 is no longer available.

RECOM2 needs a python 3.11 base image, which can be Debian 11 or 12, but Debian 11 is old enough that it tends to raise a bunch of red flags for unpatched libraries with security scans: we'd really like to upgrade if we can, but not being able to build EMS (or trust that it has been tested) will be the showstopper for that if it's not actually supported.

For Ubuntu-flavours: 20.04 (focal) has 9.3 by default, with 10.5 installable and 22.04 (Jammy) has 11.2 by default and 10.5 still installable.

sharon-tickell commented 1 year ago

@frizwi : FYI I also note that according to the gcc site at https://gcc.gnu.org/, the supported gcc releases are currently 13.2, 12.3 and 11.4 - notably v10.x is not in that list

frizwi commented 10 months ago

Fixed in latest. Now 11.x compliant. Have opened up EMS-248 in our internal bitbucket to address dynamic linking issue with gcc 12.x