mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.65k stars 1.64k forks source link

Cannot get rid of NDEBUG compiler flag #12366

Open wokast opened 1 year ago

wokast commented 1 year ago

Describe the bug I am trying to compile a test suite (C++) with assertions enabled but meson always added a compiler flag -DNDEBUG, thus deactivating all assertions. For meson configure, I used options -Ddebug=true -Db_ndebug=false to no avail. Interestingly, meson configure does not list the -DNDEBUG flag, even though build.ninja does include it. I found that this only happens when using a HDF5 dependency, in an environment where HDF5 is detected via h5cc and not pkg-config. It seems that the NDEBUG is propagated directly from h5cc output (which does include NDEBUG) into the main compile flags. I can see no other origin, in particular I checked that no environment variables contain any NDEBUG.

To Reproduce

Minimal example:

file meson.build

project('Bla', ['cpp','c'], default_options : ['cpp_std=c++11'],
        version : '1.6', license : 'CC BY-NC-SA 4.0')

#no ill effects from those
dep_boost = dependency('boost')  
dep_gsl   = dependency('gsl', version : '>=2.0') #found via pkg-config

#this dependency deactivates all assertions
dep_h5    = dependency('hdf5') # found via h5cc

exe_tst = executable('bla', sources : 'bla.cpp', dependencies : [dep_boost, dep_gsl, dep_h5])

file bla.cpp

#include<cassert>

int main() {
  assert(false);
}

building

meson setup -Ddebug=true -Db_ndebug=false  mbuild
ninja -C mbuild
./mbuild/bla  #does not fail with assertion as expected

Expected behavior When giving the -Db_ndebug=false option to meson configure, I would expect that it is guaranteed that no -DNDEBUG compiler argument is used, i.e. that the code is compiled with assertions activated. Currently, the b_ndebug parameter seems only to control whether or not meson adds NDEBUG on its own but does not enforce the user choice when NDEBUG propagates from dependencies (I don't know if that propagation is a bug or a feature but this is another topic). More generally, I expect a reliable way to compile code with assertions activated.

system parameters

tristan957 commented 1 year ago

This is entirely an HDF5 issue. Meson already provides way to not pass -DNDEBUG. Please open an issue there.

eli-schwartz commented 1 year ago

More generally, I expect a reliable way to compile code with assertions activated.

I do too!

But in this case it appears that hdf5 sticks assertion-deactivating code into your build behind our backs. What is next, will the hdf5 developers decide to stick #define NDEBUG at the tops of all their headers? :sob:

wokast commented 1 year ago

Thanks for the prompt feedback. So if I understand correctly, it is meson policy to pass all flags from configuration tools like h5cc verbatim into the final build arguments, and if any such tool outputs flags that conflict with the users intent, that is considered a problem of the dependency? I should maybe clarify that the build was using plain gcc for compilation, and not h5cc. I understand that meson cannot be expected to fix compiler wrappers. However, this is not the issue in my case. The NDEBUG was apparently inserted by meson, which got it from the h5cc that was only used for the detection of the hdf5 dependency, not for compilation. I checked in buiild.ninja that the compiler command was plain gcc, not h5cc, and the ARGS= variable contains -DNDEBUG

From my perspective as a user, it is not important where flags come from, but it is absolutely essential to be able to compile real-world code with assertions activated, even if the code has dependencies as cumbersome as hdf5. It would be great if meson could enforce -Db_ndebug=false, removing NDEBUG from the final argument list no matter where it came from.

Also, I note that the NDEBUG that originated from h5cc did not show up in the compiler args listed by meson configure, which is a bit misleading.

tristan957 commented 1 year ago

if any such tool outputs flags that conflict with the users intent, that is considered a problem of the dependency

When you set b_ndebug, you are requesting that Meson do something with the -DNDEBUG flag.

The NDEBUG was apparently inserted by meson, which got it from the h5cc that was only used for the detection of the hdf5 dependency, not for compilation.

It was inserted by your dependency, not Meson.

Also, I note that the NDEBUG that originated from h5cc did not show up in the compiler args listed by meson configure, which is a bit misleading.

c_args are user supplied flags, not those added by dependencies or projects themselves.

eli-schwartz commented 1 year ago

Thanks for the prompt feedback. So if I understand correctly, it is meson policy to pass all flags from configuration tools like h5cc verbatim into the final build arguments, and if any such tool outputs flags that conflict with the users intent, that is considered a problem of the dependency?

I didn't say that. I said that hdf5 is being a troublemaker and behaving in ways we don't expect.

It is absolutely a problem of the dependency. Hdf5 is behaving in a broken manner. This is true, regardless of whether meson tries to work around the problem -- and that's exactly what meson would be doing here, working around a bug in another project. It's not about whether meson is fulfilling its behavioral contract for the b_ndebug option.

Regardless of any other factor, hdf5 should have a bug report submitted to it, telling them that their tool is emitting troublesome and misleading arguments that are breaking tools which don't expect it. Meson is probably not the only project affected by this.

From my perspective as a user, it is not important where flags come from, but it is absolutely essential to be able to compile real-world code with assertions activated, even if the code has dependencies as cumbersome as hdf5. It would be great if meson could enforce -Db_ndebug=false, removing NDEBUG from the final argument list no matter where it came from.

To clarify, from meson's side we expect a dependency to only ever provide the arguments vital for using that dependency, and then as a global b_ndebug matter we do not remove NDEBUG from the final argument list, we merely expect that it is only "supposed" to be optionally added by us (we only add it when it is enabled).

Also, I note that the NDEBUG that originated from h5cc did not show up in the compiler args listed by meson configure, which is a bit misleading.

It's not misleading at all. The compiler args listed by meson configure are the project-wide user args, not dependency args or per target args. They don't list add_project_arguments() flags either. All they do is list the settings that can be modified by meson configure -Dc_args= (and which were originally initialized from $CFLAGS).

Any target that doesn't link to hdf5 won't have the brokenly inserted -DNDEBUG compiler arg.

wokast commented 1 year ago

Thanks for the explanation. It is not a bug then, but a design decision. I simply misinterpreted b_ndebug=false as meaning "build with assertions activated" while in reality it means "Do not explicitly add -DNDEBUG". I will just have to write my own pkg-config file for hdf5 and other badly packaged libraries.

dcbaker commented 1 year ago

We do sometimes work around bogus flags form upstream packages. We'd prefer to do that though only after the upstream project has declared that the issue is not a bug, or that they wont fix it. LLVM is in this situation. So if you file a bug with hdf5 and they won't fix it then I'd be happy to consider working around it in Meson.

dcbaker commented 1 year ago

Actually, I just looked at the hdf5 package config setup. It’s already an absolute disaster that we’re doing a ton of workarounds for, so I don’t see why one more workaround hurts anything

wokast commented 1 year ago

Incidentally, there already was a related upstream issue https://github.com/HDFGroup/hdf5/issues/3526

However, most people use hdf5 packaged by distributions or in my case conda-forge, which is still broken in that regard.

eli-schwartz commented 1 year ago

Actually, I just looked at the hdf5 package config setup. It’s already an absolute disaster that we’re doing a ton of workarounds for, so I don’t see why one more workaround hurts anything

Right, I was actually very gently, very subtly, implying that above, and it's why I was so careful to redefine the problem as "not about b_ndebug, but about whether we should work around it locally, and about whether it's still something upstream needs to fix regardless".

Frankly, hdf5 is a mess in so many respects it's impossible to count them all, and there's a whole bucket full of those respects that are buildsystem respects that cause issues for us. It is probably a never-ending task to fix them.