On Mon, 2021-04-19 at 15:46 +0100, Emil Velikov wrote: > On Mon, 19 Apr 2021 at 15:05, Allan McRae <[email protected]> > wrote: > > > > On 19/4/21 9:34 pm, Emil Velikov wrote: > > > On Mon, 19 Apr 2021 at 08:10, Allan McRae <[email protected]> > > > 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? >
>From my testing to write these patches, if gpgme.found() is false, an `#undef HAVE_LIBGPGME` will be set in config.h. This is where my note in one of my other emails came from. I would need to test this further, but a random "oh no, I forgot to install gpgme" after building didn't seem to cause any errors on FreeBSD. > 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 Mark
