search

easybuilders / easybuild-easyblocks

Collection of easyblocks that implement support for building and installing software with EasyBuild.
https://easybuild.io
GNU General Public License v2.0
106 stars 285 forks source link

Make `MesonNinja` respect the toolchainopts with buildtype as well as `--debug` and `--optimization` flags #3454

Closed Micket closed 1 month ago

Micket commented 1 month ago

Fixes #3280

Should also merge https://github.com/easybuilders/easybuild-easyconfigs/pull/21619 once this is merged.

In this version, i double up on both buildtype as well as --optimizationand --debug flags to respect the toolchainopts. I think this will be the better approach, but i haven't tested anything yet.

Micket commented 1 month ago

WARNING: Recommend using either -Dbuildtype or -Doptimization + -Ddebug. Using both is redundant since they override each other. See: https://mesonbuild.com/Builtin-options.html#build-type-options

ok, so apparently meson doesn't want you to do this. I find this strange, as just using the optimization + debug options wouldn't cover things like -DNDEBUG defines. Maybe noone using meson uses such defines in their C/C++ codes? :/

Micket commented 1 month ago

https://github.com/mesonbuild/meson-python/pull/325 No this seems to indicate otherwise. We certainly always want the NDEBUG set for our builds...

edit: see also https://mesonbuild.com/Release-notes-for-0-45-0.html#b_ndebug-ifrelease

Micket commented 1 month ago

Alright, so to summarize

  1. I add the -v flag so we can see what we are compiling
  2. I set the build type, in case they have some "if-release" logic that adds extra flags and such.
  3. I set optimization and debug on top of that, which produces a warning that I disagree with due to the point mentioned above.
  4. I set the -bn_debug=true flag for all builds that aren't noopt, so that we get -DNDEBUG when building debugoptimized as well as release. I think by default, the meson-python actually adds the flag -bn_debug=if-release but if we are enabling debug symbols, you don't want to squash this. They mention in the issue tracking n_debug flag that they don't add it automatically to release because they expect packagers to set all their flags themselves anyway.. and i guess they are right (now).

All in all this brings meson builds in line with the rest of the stuff we build, and I don't think any of it is controversial. Wouldn't surprise me if we see a large speedup in some packages after these changes, but it's hard to test things like "cairo" or "Wayland" for performance.

Micket commented 1 month ago

Test report by @Micket

Overview of tested easyconfigs (in order)

Build succeeded for 4 out of 4 (4 easyconfigs in total) vera-skylake-build - Linux Rocky Linux 8.9, x86_64, Intel Xeon Processor (Skylake, IBRS, no TSX), Python 3.6.8 See https://gist.github.com/Micket/1c22578d1e11a2d81ddd7f38a3c0030e for a full test report.

Micket commented 1 month ago

Test report by @Micket

Overview of tested easyconfigs (in order)

Build succeeded for 3 out of 4 (4 easyconfigs in total) vera-skylake-build - Linux Rocky Linux 8.9, x86_64, Intel Xeon Processor (Skylake, IBRS, no TSX), Python 3.6.8 See https://gist.github.com/Micket/d182683aa7be4718a6ad9167888a51d9 for a full test report.

Micket commented 1 month ago

So, Wayland apparently needs to be built with asserts, else it fails the test suite. Which means, one can't actually test a true install... Not sure how much it impacts performance, or if we care about tools like wayland for that sort of performance in HPC settings, so I'm making n_debug into an option.

Micket commented 1 month ago

OK, so meson is really strange when specifying both buildtype and optmization/debug flags. They claim that they mutually exclusive, but I disagree; the value of "buildtype" itself has value given things like

if get_option('buildtype') == 'release':
   # custom logic here

In particular -Db_ndebug=if-release is something a lot of things set (all of meson-python projects do this by default), so, it's not some little thing either. Though, if-release also leaves a lot to be desired; what about debugoptimized? is that supposed to be a release-like thing? Again debug is being used vaguely here.

I checked the meson.build from all the stuff that uses MesonNinja to see what, if any, such custom logic anyone might be doing, and apart from some harmless flags and b_ndebug=if-release, I couldn't find anything that worried me, so I'm going to go with the option of making buildtype and optimization/debug mutually exclusive for our easyblock as well, and just rely on optimization, debug, and b_ndebug flags.

Micket commented 1 month ago

Test report by @Micket

Overview of tested easyconfigs (in order)

Build succeeded for 40 out of 41 (41 easyconfigs in total) vera-skylake-build - Linux Rocky Linux 8.9, x86_64, Intel Xeon Processor (Skylake, IBRS, no TSX), Python 3.6.8 See https://gist.github.com/Micket/49102596d81d8d925377b46e82b7475f for a full test report.

Micket commented 1 month ago

The error on cairomm is something unrelated, 2 tests crashes for me no matter what easyblock i use, even in 4.9.2, so it shouldn't let this block the merge.