Make MesonNinja respect the toolchainopts with buildtype as well as --debug and --optimization flags
#3454
Conversation
Micket
force-pushed
the
mesonrelease2
branch
from
9a9534a
to
f54380c
Compare
boegel
changed the title
MesonNinja respect the toolchainopts with buildtype as well as --debug and --optimization flags
Fix typo Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
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? :/ |
|
mesonbuild/meson-python#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 |
|
Alright, so to summarize
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. |
|
Test report by @Micket Overview of tested easyconfigs (in order)
Build succeeded for 4 out of 4 (4 easyconfigs in total) |
|
Test report by @Micket Overview of tested easyconfigs (in order)
Build succeeded for 3 out of 4 (4 easyconfigs in total) |
Micket
force-pushed
the
mesonrelease2
branch
2 times, most recently
from
e260fbf
to
7f1e5fc
Compare
|
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
force-pushed
the
mesonrelease2
branch
2 times, most recently
from
64253f2
to
a32737a
Compare
|
OK, so meson is really strange when specifying both buildtype and optmization/debug flags. In particular 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. |
|
Test report by @Micket Overview of tested easyconfigs (in order)
Build succeeded for 40 out of 41 (41 easyconfigs in total) |
|
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. |
| @property | ||
| def optimization(self): | ||
| """Optimization level""" | ||
| if self.toolchain.options.get('noopt', False): | ||
| return 0 | ||
| elif self.toolchain.options.get('lowopt', False): | ||
| return 1 | ||
| else: | ||
| return 2 | ||
|
|
There was a problem hiding this comment.
wait, what if we have 'opt': True (as in, use -O3)?
surely this needs another elif?
There was a problem hiding this comment.
| @property | |
| def optimization(self): | |
| """Optimization level""" | |
| if self.toolchain.options.get('noopt', False): | |
| return 0 | |
| elif self.toolchain.options.get('lowopt', False): | |
| return 1 | |
| else: | |
| return 2 | |
| @property | |
| def optimization(self): | |
| """Optimization level""" | |
| if self.toolchain.options.get('noopt', False): | |
| return 0 | |
| elif self.toolchain.options.get('lowopt', False): | |
| return 1 | |
| elif self.toolchain.options.get('opt', False): | |
| return 3 | |
| else: | |
| return 2 | |
Fixes #3280
Should also merge easybuilders/easybuild-easyconfigs#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.