[pacman-dev] [PATCH 2/5] meson.build: Fix detection of symbols

[Mark Weiman]

On Mon, 2021-04-19 at 15:46 +0100, Emil Velikov wrote:
> On Mon, 19 Apr 2021 at 15:05, Allan McRae <allan at archlinux.org>
> wrote:
> > 
> > On 19/4/21 9:34 pm, Emil Velikov wrote:
> > > On Mon, 19 Apr 2021 at 08:10, Allan McRae <allan at 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?
> 

>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