GNOME Bugzilla – Bug 791486
Meson: Rework the config.h generation
Last modified: 2018-05-24 20:00:35 UTC
I it annoying to keep that template file in sync and shouldn't be needed. Removing it breaks the build, we should investigate why.
Created attachment 366453 [details] [review] meson: drop config.h template Use separate configuration_data() for the substitutions specific to pkg-config files, which should not end up in our config.h, and get rid of the config.h.meson template. We can't use a single configuration_data() here because things like an unquoted #define prefix /foo/bar in config.h will break compilation. ------------------------------------------------------------ To discuss: What to do about these? /* Enable extensions on AIX 3, Interix. */ #ifndef _ALL_SOURCE #mesondefine _ALL_SOURCE #endif /* Enable threading extensions on Solaris. */ #ifndef _POSIX_PTHREAD_SEMANTICS #mesondefine _POSIX_PTHREAD_SEMANTICS #endif /* Enable extensions on HP NonStop. */ #ifndef _TANDEM_SOURCE #mesondefine _TANDEM_SOURCE #endif /* Enable general extensions on Solaris. */ #ifndef __EXTENSIONS__ #mesondefine __EXTENSIONS__ #endif /* Define WORDS_BIGENDIAN to 1 if your processor stores words with the most significant byte first (like Motorola and SPARC, unlike Intel). */ #if defined AC_APPLE_UNIVERSAL_BUILD # if defined __BIG_ENDIAN__ # define WORDS_BIGENDIAN 1 # endif #else # ifndef WORDS_BIGENDIAN # undef WORDS_BIGENDIAN # endif #endif /* Enable large inode numbers on Mac OS X 10.5. */ #ifndef _DARWIN_USE_64_BIT_INODE # define _DARWIN_USE_64_BIT_INODE 1 #endif My take: - _ALL_SOURCE (AIX 3, Interix): drop - _TANDEM_SOURCE (HP NonStop): drop - _POSIX_PTHREAD_SEMANTICS (Solaris): - __EXTENSIONS__ (Solaris): add FIXME comments to meson.build and let Solaris folks figure it out, there is an open issue about the desired host_system id even: https://github.com/mesonbuild/meson/issues/1578 - WORDS_BIGENDIAN: drop, not used inside glib - _DARWIN_USE_64_BIT_INODE (Mac OS X 10.5): drop, 10.5 is >10 years old, all later versions default to 64-bit according to https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man2/stat64.2.html
The build breaks because we use a single configuration_data() object for all variables, and then we use the same object for filling up various templates. Additionally, we're not setting a value for various keys in the meson template, so they end up unset. We should have a single configuration_data() object for the config.h, just like we have a single object for glib-config.h; additionally, we should do an audit of all the keys set by Autotools, to ensure that we're populating the config.h correctly.
Note that in bug #788773 I changed to use pkg config generator, so it won't need configuration variables anymore.
I retitled the bug because I think dropping the template is just one step in the process. We need to ensure that config.h generated the Meson is functionally equivalent to the Autotools one — modulo Autotools-specific bits, which should be double checked in case we're implicitly relying on them. If that is not possible, then we'll have to figure out how to fake it — and rely on the template if that's the only way to do so.
(In reply to Tim-Philipp Müller from comment #1) > My take: > > - _ALL_SOURCE (AIX 3, Interix): drop > - _TANDEM_SOURCE (HP NonStop): drop > > - _POSIX_PTHREAD_SEMANTICS (Solaris): > - __EXTENSIONS__ (Solaris): add FIXME comments to meson.build and let > Solaris folks figure it out, there is an open issue about the desired > host_system id even: https://github.com/mesonbuild/meson/issues/1578 > > - WORDS_BIGENDIAN: drop, not used inside glib > > - _DARWIN_USE_64_BIT_INODE (Mac OS X 10.5): drop, 10.5 is >10 years old, > all later versions default to 64-bit according to > https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ > ManPages/man2/stat64.2.html From a maintainability point of view, I would prefer it if the Meson and autotools builds behaved the same. So if you’re going to drop old #defines and build conditions, I would like them to be dropped from autotools too, preferably in a preparatory commit which justifies why the option is no longer needed. The reason should not be ‘because we don’t want to support it in Meson’. Since we’re aiming for Meson to become the default build system for GLib, I don’t want any regressions for the supported platforms (https://wiki.gnome.org/Projects/GLib/SupportedPlatforms). OpenIndiana is sort of on that list. If the _DARWIN_USE_64_BIT_INODE stuff is definitely for OS X 10.5, then it can definitely be dropped. Our minimum dependency is 10.7 now (see the supported platforms page).
Created attachment 372369 [details] [review] meson: drop config.h template
(In reply to Tim-Philipp Müller from comment #1) > My take: > > - _ALL_SOURCE (AIX 3, Interix): drop > - _TANDEM_SOURCE (HP NonStop): drop > > - _POSIX_PTHREAD_SEMANTICS (Solaris): > - __EXTENSIONS__ (Solaris): add FIXME comments to meson.build and let > Solaris folks figure it out, there is an open issue about the desired > host_system id even: https://github.com/mesonbuild/meson/issues/1578 > > - WORDS_BIGENDIAN: drop, not used inside glib > > - _DARWIN_USE_64_BIT_INODE (Mac OS X 10.5): drop, 10.5 is >10 years old, > all later versions default to 64-bit according to > https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ > ManPages/man2/stat64.2.html Did you make the exercise to review all diff between meson/autotools config.h and those are the only difference? My take: 1) _ALL_SOURCE, _TANDEM_SOURCE, WORDS_BIGENDIAN and _DARWIN_USE_64_BIT_INODE can be dropped from autotools too for consistency because we have no way to ensure those platform works and there is no point in even trying. 2) _POSIX_PTHREAD_SEMANTICS and __EXTENSIONS__, IMHO a FIXME in meson.build is enough because until we have a CI we are pretty sure there are plenty other stuff that needs fixing anyway. No point in pretending we support something if we cannot test it. And then we can close this bug, right?
I *think* those were the only ones different/missing, yes. Fully agree with you Xavier. In general I think we should be radical here with the Meson build and drop rather than cargo-cult. GLib has good enough test coverage to make sure that if something is amiss on some platform it will be discovered fairly quickly if one ever tries to build on those with Meson. It's not like these are super-subtle things.
Actually, I've been greping for them, and they are only defined in config.h.win32.in. I guess they are added automatically by autoconf, so we cannot drop them, can we? So I just went through all AC_DEFINE we have in configure.ac, here is the list we don't seems to have in meson: Missing flags I haven't looked: ENABLE_GC_FRIENDLY_DEFAULT DISABLE_MEM_POOLS HAVE_LIBC_ENABLE_SECURE HAVE_SYS_SELECT_H NO_FD_SET POSIX_MEMALIGN_WITH_COMPLIANT_ALLOCS HAVE_DBUS1 meson.build says those are not used, drop from autotools? HAVE_LONG_LONG_FORMAT HAVE_INT64_AND_I64 Those are for solaris, I would be in favor of just adding a FIXME _XOPEN_SOURCE_EXTENDED _XOPEN_SOURCE __EXTENSIONS__ fam is not built at all, do we care? Can we even just drop that code from git? HAVE_FAM HAVE_FAM_NO_EXISTS Shouldn't we error in configure for that one? THREADS_NONE
oh, and from glib-gettext.m4: HAVE_CFPREFERENCESCOPYAPPVALUE HAVE_CFLOCALECOPYCURRENT
oh, and some where in meson but with FIXME around them: HAVE_UNIX98_PRINTF HAVE_BIND_TEXTDOMAIN_CODESET GLIB_USING_SYSTEM_PRINTF # FIXME: defines in config.h that are not actually used anywhere # (we add them for now to minimise the diff) glib_conf.set('HAVE_DLFCN_H', 1) glib_conf.set('__EXTENSIONS__', 1) glib_conf.set('STDC_HEADERS', 1) # THREADS_NONE glib_conf.set('SIZEOF___INT64', 8)
> oh, and some where in meson but with FIXME around them: > > HAVE_UNIX98_PRINTF No idea, will have to check the autotools check. > HAVE_BIND_TEXTDOMAIN_CODESET Should be easy, right? conf.set('', cc.has_function()) > GLIB_USING_SYSTEM_PRINTF Right, bunch of tedious checks needed here. Might as well always use the built-in one tbh ;) > # FIXME: defines in config.h that are not actually used anywhere > # (we add them for now to minimise the diff) > glib_conf.set('HAVE_DLFCN_H', 1) Should be easy, right? conf.set(.., cc.has_header()) > glib_conf.set('__EXTENSIONS__', 1) Think it was a solaris thing? Can only find https://docs.oracle.com/cd/E19253-01/816-5175/standards-5/index.html > glib_conf.set('STDC_HEADERS', 1) I think that's fine to keep, it will be true on all toolchains from this century. Maybe not ancient visual studio versions, but I wouldn't worry about those. > # THREADS_NONE > glib_conf.set('SIZEOF___INT64', 8) Is this a windows thing, checking if __int64 is available? Doesn't seem to be used anywhere, let's try dropping it?
(In reply to Xavier Claessens from comment #10) > oh, and from glib-gettext.m4: > HAVE_CFPREFERENCESCOPYAPPVALUE > HAVE_CFLOCALECOPYCURRENT Those 2 can be dropped, they are not used in glib, and for API added in macosx 10.2 and 10.3, we depend on newer version than that. Not sure it's worth changing the glib-gettext.m4, it's copy/paster code from gettext it seems.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/1313.