On Mon, 19 Apr 2021 at 15:05, Allan McRae <allan@archlinux.org> wrote:
On 19/4/21 9:34 pm, Emil Velikov wrote:
On Mon, 19 Apr 2021 at 08:10, Allan McRae <allan@archlinux.org> wrote:
On 17/4/21 1:45 pm, Mark Weiman wrote:
This patch changes the behavior of meson to define configuration options *only* when the symbol checked is present. Currently, it defines all of them in config.h whether the symbol exists or not and the code that looks for it doesn't check the macro's value, but whether it's defined.
Remember back when we used autotools and all this just worked! :D
Patch looks good to me.
Food for thought:
Usually the more robust approach is to always set the respective defines to 0/1 and evaluate them directly (aka #if HAVE_). In addition one could set -Werror=undef in the build to catch any issues.
This will produce clear traces with potential issues, while #if defined will silently fallback to the "other" path. If people are OK with the above, I will follow-up with some patches.
Note: above suggestion is not meant to dismiss the original patch.
I still live in the good old autotools days where there were lots of #undef in config.h...
It appears some of that still happens. This is my build/config.h:
#undef HAVE_STRUCT_STATFS_F_FLAGS #define HAVE_STRUCT_STATVFS_F_FLAG
But it is not consistent.
Speaking of consistency - here is a quick grep from an old-ish checkout: $ git grep HAVE_LIBGPGME ... lib/libalpm/be_sync.c:#ifndef HAVE_LIBGPGME // not defined, ok ... lib/libalpm/sync.c:#ifdef HAVE_LIBGPGME // is defined, ok meson.build:conf.set('HAVE_LIBGPGME', gpgme.found()) // hmm we always define it, right? Looking at the above one would say - we need CI to catch it. But we do - see the arch-no-gpg config, still has gpgme installed :-\ Since the headers are in the standard location, they end up used. Even if the final binary is missing the DT_NEEDED on gpgme. Mind you that link thing is likely meson just adding a linker flag (don't recall which one) to drop unneeded libraries from the DT_NEEDED list. Disclaimer: above is an educated guess, didn't stare at the build logs (yet). Hmm HAVE_LIBSSL, ENABLE_NLS seems similar.
I'm not sure whether the better solution is to add more #undef, or to go to the #if 0/1 style.
IMHO the above example/braindump illustrates why I'm in favour of 0/1 style. In addition, we had far too many bugs in Mesa and X (Xorg or otherwise) so we gradually switched to 0/1. -Emil