[arch-dev-public] RFC: Dropping -DCMAKE_BUILD_TYPE from packages using cmake
Hi, Currently most of our packages which use the cmake build system are built with -DCMAKE_BUILD_TYPE=Release. This provides a reasonable (according to upstream) set of C(XX)FLAGS defaults which are appended to and override our default C(XX)FLAGS. So, for instance, our cmake packages are being built with -O3 instead of our default -O2. Besides the inconsistency that this brings to our binaries, IMO it's not a good idea to override our default C(XX)FLAGS unless there's a good reason to. This will also cause issues once we start building debug packages by default (-DCMAKE_BUILD_TYPE=Release also adds DNDEBUG). Therefore I'm proposing to drop -DCMAKE_BUILD_TYPE from all our cmake packages, building them with our default C(XX)FLAGS as all other packages. Comments?
Le 10/03/2018 à 11:34, Antonio Rojas via arch-dev-public a écrit :
Hi, Currently most of our packages which use the cmake build system are built with -DCMAKE_BUILD_TYPE=Release. This provides a reasonable (according to upstream) set of C(XX)FLAGS defaults which are appended to and override our default C(XX)FLAGS. So, for instance, our cmake packages are being built with -O3 instead of our default -O2. Besides the inconsistency that this brings to our binaries, IMO it's not a good idea to override our default C(XX)FLAGS unless there's a good reason to. This will also cause issues once we start building debug packages by default (-DCMAKE_BUILD_TYPE=Release also adds DNDEBUG). Therefore I'm proposing to drop -DCMAKE_BUILD_TYPE from all our cmake packages, building them with our default C(XX)FLAGS as all other packages. Comments?
As long as you accept exceptions to this (I have scientific stuff in mind, like NetCDF — currently not built with CMake but will be in next release), I’m fine with this.
El sábado, 10 de marzo de 2018 14:34:16 (CET) Bruno Pagani via arch-dev-public escribió:
As long as you accept exceptions to this (I have scientific stuff in mind, like NetCDF — currently not built with CMake but will be in next release), I’m fine with this.
This is just about stopping systematically doing this. Of course particular packages may have to modify their build options for certain reasons (although personally I'd prefer if it was done transparently by setting the C(XX)FLAGS explicitely instead of relying on semi-obscure cmake presets)
On 03/10/2018 05:34 AM, Antonio Rojas via arch-dev-public wrote:
Hi, Currently most of our packages which use the cmake build system are built with -DCMAKE_BUILD_TYPE=Release. This provides a reasonable (according to upstream) set of C(XX)FLAGS defaults which are appended to and override our default C(XX)FLAGS. So, for instance, our cmake packages are being built with -O3 instead of our default -O2. Besides the inconsistency that this brings to our binaries, IMO it's not a good idea to override our default C(XX)FLAGS unless there's a good reason to. This will also cause issues once we start building debug packages by default (-DCMAKE_BUILD_TYPE=Release also adds DNDEBUG). Therefore I'm proposing to drop -DCMAKE_BUILD_TYPE from all our cmake packages, building them with our default C(XX)FLAGS as all other packages. Comments?
This theoretically sounds like a fantastic idea, but I'm not really sure what CMake's deal with build flags are in the first place. What is the default build type, and does CMake even look at build flags from the environment at all (at least in a sane manner)? For example, I am currently the maintainer of a package that has, apparently, historically used: -DCMAKE_C_FLAGS:STRING="${CFLAGS}" \ -DCMAKE_CXX_FLAGS:STRING="${CXXFLAGS}" \ -DCMAKE_BUILD_TYPE=Release \ IIRC the first two are because there are situations where CMake simply ignores environment *FLAGS altogether. Maybe this only impacts incremental builds because of some stupid CMakeCache rule where the cache shall not be overwritten under any circumstances because KitWare in its terrible glory has so decreed, but I cannot swear this is the only such situation... As for the buildtype, I was under the impression this is because 1) CMake simply has no BUILD_TYPE=plain as far as I know, and 2) failing to specify it means CMake does something strange and magical ranging from "assume the default build type is Debug" to "read some arbitrary build type that upstream was allowed to set in CMakeLists.txt because clearly this should be enforced by the build system for hysterical raisins". I have never felt brave enough to debug where environment *FLAGS go anyways, nor if/when/where they get overridden somehow by CMake when they "clash" with the BUILD_TYPE flags. ... While we are at it, someone has seen fit to create some arbitrary meson wrapper called "arch-meson" (note we have never shipped an arch-cmake nor an arch-configure, nor an arch-setup-py nor even an arch-make etc.), which aside for being somewhat magical also most certainly sets --buildtype release I distinctly remember this being something we do not want to support in makepkg due to us being fundamentally opposed to the end result where e.g. Debian packaging has magic in every which direction and you have to have a degree in Debian packaging plus go spelunking through addons provided by six or seven packages just to figure out how the actual build system for packages is being configured and run. I *know* that meson's buildtype=plain does the sane thing and delegates to the environment *FLAGS (or rather, the makepkg.conf *FLAGS and OPTIONS=(debug) if specified). In fact, the Meson people have called me out for Arch being hypocrites, because we hardcode buildtype=release. Which was primarily news to me because I did not realize we even have a wrapper in the first place. -- Eli Schwartz Bug Wrangler and Trusted User
El domingo, 11 de marzo de 2018 1:44:07 (CET) Eli Schwartz via arch-dev-public escribió:
This theoretically sounds like a fantastic idea, but I'm not really sure what CMake's deal with build flags are in the first place. What is the default build type, and does CMake even look at build flags from the environment at all (at least in a sane manner)?
This is very poorly documented, so you have to dig into the cmake code to figure it out. The default build type is None, which means CMAKE_C(XX)_FLAGS is used (see /usr/share/cmake-3.10/Modules/CMakeCInformation.cmake:117), whose value is taken from the environment C(XX)FLAGS (see /usr/share/cmake-3.10/Modules/CMakeCXXInformation.cmake:198). So yes, not setting CMAKE_BUILD_TYPE will simply use the build flags from C(XX)FLAGS. If you want to test yourself, run cmake with the -LAH flag and check the output for CMAKE_C(XX)_FLAGS.
For example, I am currently the maintainer of a package that has, apparently, historically used:
-DCMAKE_C_FLAGS:STRING="${CFLAGS}" \ -DCMAKE_CXX_FLAGS:STRING="${CXXFLAGS}" \ -DCMAKE_BUILD_TYPE=Release \
The first two lines should not be necessary (see above)
On 03/11/2018 05:00 AM, Antonio Rojas via arch-dev-public wrote:
This is very poorly documented, so you have to dig into the cmake code to figure it out. The default build type is None, which means CMAKE_C(XX)_FLAGS is used (see /usr/share/cmake-3.10/Modules/CMakeCInformation.cmake:117), whose value is taken from the environment C(XX)FLAGS (see /usr/share/cmake-3.10/Modules/CMakeCXXInformation.cmake:198). So yes, not setting CMAKE_BUILD_TYPE will simply use the build flags from C(XX)FLAGS. If you want to test yourself, run cmake with the -LAH flag and check the output for CMAKE_C(XX)_FLAGS.
Oh, huh, nice. This seems to work. I wonder why whenever I googled for information about this it seemed to imply the default buildtype was a debug build... oh wait, this is CMake we are talking about, google may be the *only* source of information but it still isn't a *good* source. :p
For example, I am currently the maintainer of a package that has, apparently, historically used:
-DCMAKE_C_FLAGS:STRING="${CFLAGS}" \ -DCMAKE_CXX_FLAGS:STRING="${CXXFLAGS}" \ -DCMAKE_BUILD_TYPE=Release \
The first two lines should not be necessary (see above)
But I still think this is "necessary". That CMake module mentions
# use _INIT variables so that this only happens the first time
I've double-checked, and if you don't explicitly set the CFLAGS and CXXFLAGS then CMake unconditionally reuses the cache, which is ironic because CMake also contains a touted feature where changing the CFLAGS causes any object files that depend on those flags, to be properly rebuilt. Except they don't get rebuilt because they don't even get changed... I know repository PKGBUILDs are typically built in a clean chroot, so cached builds make no difference there, but then neither do unquoted srcdir/pkgdir. It is still not helpful to people who e.g. check out a PKGBUILD from svntogit and rebuild it locally for any of a number of reasons to have unpredictable behavior due to assumptions about always building in a clean chroot. So I think maybe we should still (or start?) specifying this. -- Eli Schwartz Bug Wrangler and Trusted User
El domingo, 11 de marzo de 2018 21:58:16 (CET) Eli Schwartz via arch-dev-public escribió:
I know repository PKGBUILDs are typically built in a clean chroot, so cached builds make no difference there, but then neither do unquoted srcdir/pkgdir. It is still not helpful to people who e.g. check out a PKGBUILD from svntogit and rebuild it locally for any of a number of reasons to have unpredictable behavior due to assumptions about always building in a clean chroot. So I think maybe we should still (or start?) specifying this.
You don't need to build in a clean chroot to prevent this from happening, you just need to make sure you clean up your build dir before rebuilding after having changed the build flags, which IMO is a reasonable assumption to make.
Le 11/03/2018 à 10:00, Antonio Rojas via arch-dev-public a écrit :
El domingo, 11 de marzo de 2018 1:44:07 (CET) Eli Schwartz via arch-dev-public escribió:
This theoretically sounds like a fantastic idea, but I'm not really sure what CMake's deal with build flags are in the first place. What is the default build type, and does CMake even look at build flags from the environment at all (at least in a sane manner)? This is very poorly documented, so you have to dig into the cmake code to figure it out. The default build type is None, which means CMAKE_C(XX)_FLAGS is used (see /usr/share/cmake-3.10/Modules/CMakeCInformation.cmake:117), whose value is taken from the environment C(XX)FLAGS (see /usr/share/cmake-3.10/Modules/CMakeCXXInformation.cmake:198). So yes, not setting CMAKE_BUILD_TYPE will simply use the build flags from C(XX)FLAGS. If you want to test yourself, run cmake with the -LAH flag and check the output for CMAKE_C(XX)_FLAGS.
Some projects seems to default to Debug instead of None… So should we specify a BUILD_TYPE of None instead of Release?
El martes, 13 de marzo de 2018 11:23:07 (CET) Bruno Pagani via arch-dev-public escribió:
Some projects seems to default to Debug instead of None… So should we specify a BUILD_TYPE of None instead of Release?
Not sure if it's a good idea to systematically override the build type when it has been explicitely set by upstream. Some projects may be doing so for good reasons. Although explicitely setting it to Debug in a release tarball seems odd, do you have any example of such a project?
Le 13/03/2018 à 13:42, Antonio Rojas via arch-dev-public a écrit :
El martes, 13 de marzo de 2018 11:23:07 (CET) Bruno Pagani via arch-dev-public escribió:
Some projects seems to default to Debug instead of None… So should we specify a BUILD_TYPE of None instead of Release?
Not sure if it's a good idea to systematically override the build type when it has been explicitely set by upstream. Some projects may be doing so for good reasons. Although explicitely setting it to Debug in a release tarball seems odd, do you have any example of such a project?
The one I had in mind: https://github.com/Unidata/netcdf-c/blob/master/CMakeLists.txt#L122 Release tarball is the same, they don’t change anything w.r.t. CMake in release tarball. Maybe it’s just one project and all other Arch Linux packages are not affected, but dropping BUILD_TYPE automatically at repo scale does not seems a good idea. I would prefer a todo list with instructions on checking what BUILD_TYPE and more importantly FLAGS where actually used by the build, even if I know you would rather have avoided one.
On March 13, 2018 2:07:31 PM GMT+01:00, Bruno Pagani via arch-dev-public <arch-dev-public@archlinux.org> wrote:
Le 13/03/2018 à 13:42, Antonio Rojas via arch-dev-public a écrit :
El martes, 13 de marzo de 2018 11:23:07 (CET) Bruno Pagani via arch-dev-public escribió:
Some projects seems to default to Debug instead of None… So should we specify a BUILD_TYPE of None instead of Release?
Not sure if it's a good idea to systematically override the build type when it has been explicitely set by upstream. Some projects may be doing so for good reasons. Although explicitely setting it to Debug in a release tarball seems odd, do you have any example of such a project?
The one I had in mind: https://github.com/Unidata/netcdf-c/blob/master/CMakeLists.txt#L122
Release tarball is the same, they don’t change anything w.r.t. CMake in release tarball. Maybe it’s just one project and all other Arch Linux packages are not affected, but dropping BUILD_TYPE automatically at repo scale does not seems a good idea. I would prefer a todo list with instructions on checking what BUILD_TYPE and more importantly FLAGS where actually used by the build, even if I know you would rather have avoided one.
I think a TODO and checking flags would be a good idea, I also encountered similar things and even somehow ignoring flags fully but provide custom switches to set them. If we touch this on a distro scale anyway, now would be a good time to ensure our distro FLAGS are indeed set for cmake stuff as we expect. Obviously there are weird things going on in some of them :p Cheers, Levente
On Sun, Mar 11, 2018 at 1:43 AM Eli Schwartz via arch-dev-public < arch-dev-public@archlinux.org> wrote:
While we are at it, someone has seen fit to create some arbitrary meson wrapper called "arch-meson" (note we have never shipped an arch-cmake nor an arch-configure, nor an arch-setup-py nor even an arch-make etc.), which aside for being somewhat magical also most certainly sets --buildtype release
I distinctly remember this being something we do not want to support in makepkg due to us being fundamentally opposed to the end result where e.g. Debian packaging has magic in every which direction and you have to have a degree in Debian packaging plus go spelunking through addons provided by six or seven packages just to figure out how the actual build system for packages is being configured and run.
It's not magical. This doesn't even compare to dpkg-buildpackage's hooks and buildsystem handling, which changes behavior drastically based on what's installed and what the source tree looks like. arch-meson is just a wrapper providing some defaults. You don't use the wrapper, you don't get the defaults. For that matter, I'm all for putting an arch-configure helper into our autoconf package. I've had all kinds of issues because builds forgot to set or mishandled some installation directory option from autoconf's general set. We should avoid having to repeat the same options over and over. I *know* that meson's buildtype=plain does the sane thing and delegates
to the environment *FLAGS (or rather, the makepkg.conf *FLAGS and OPTIONS=(debug) if specified).
I did not use plain because meson handles languages other than C and C++. Our FLAGS override the buildtype arguments anyway. We get the best of both worlds. We can move arch-meson's buildtype to debugoptimized as soon as we start building debug packages.
On 13/03/18 21:22, Jan Alexander Steffens via arch-dev-public wrote:
For that matter, I'm all for putting an arch-configure helper into our autoconf package.
Don't muck around with vanilla packages. Put it in another package (devtools). A
On 2018-03-13 14:40, Allan McRae via arch-dev-public wrote:
On 13/03/18 21:22, Jan Alexander Steffens via arch-dev-public wrote:
For that matter, I'm all for putting an arch-configure helper into our autoconf package.
Don't muck around with vanilla packages. Put it in another package (devtools).
A
Makes no sense unless devtools become part of base-devel. It's hardly any meddling as it doesn't hinder possibility to use default configure script or meson binary. Bartłomiej
On 14/03/18 04:19, Bartłomiej Piotrowski via arch-dev-public wrote:
On 2018-03-13 14:40, Allan McRae via arch-dev-public wrote:
On 13/03/18 21:22, Jan Alexander Steffens via arch-dev-public wrote:
For that matter, I'm all for putting an arch-configure helper into our autoconf package.
Don't muck around with vanilla packages. Put it in another package (devtools).
A
Makes no sense unless devtools become part of base-devel. It's hardly any meddling as it doesn't hinder possibility to use default configure script or meson binary.
It is Arch specific, so should be in an Arch specific package. A
On 03/13/2018 07:22 AM, Jan Alexander Steffens wrote:
It's not magical. This doesn't even compare to dpkg-buildpackage's hooks and buildsystem handling, which changes behavior drastically based on what's installed and what the source tree looks like.
arch-meson is just a wrapper providing some defaults. You don't use the wrapper, you don't get the defaults.
"I object on principle to ever using the wrapper" -- then why does the wrapper exist if we don't use it because we don't have to? This wrapper changes depending on which version of meson you have installed.
For that matter, I'm all for putting an arch-configure helper into our autoconf package. I've had all kinds of issues because builds forgot to set or mishandled some installation directory option from autoconf's general set. We should avoid having to repeat the same options over and over.
The fact that autoconf has weird bugs like needing to set localstatedir and sysconfdir to their defaults minus the /usr prefix, is an autoconf bug. Maybe we could work on having meson not require setting bizarre options just to get sane defaults? And arch-meson contains at least seven options that simply repeat the defaults shown by running `meson ..; meson configure`. (Setting the prefix automatically propagates to everything specified with relative paths, which is most things, and libexecdir and sbindir could just specify a *different* relative path.)
I did not use plain because meson handles languages other than C and C++. Our FLAGS override the buildtype arguments anyway. We get the best of both worlds.
So other languages, which completely disregard our CFLAGS but which meson itself has magic wrappers for... and the solution is to add a magic script to use in the PKGBUILD which does not respect makepkg.conf but can add debug symbols only loosely related to what we were trying to get? A script which arbitrarily adds debug symbols to packages that were rebuilt with OPTIONS=(!debug) by users who wanted to make local modifications? Maybe the solution is to get support for these languages to respect our FLAGS regardless of whether the build system is implemented in meson. Or I guess we could just replace our packaging system with meson so we can consistently get debug info for nonconformant languages... since apparently meson is so perfect we should do literally everything directly in meson. Or we could at least make a *proper* wrapper which can talk to makepkg and determine if debug info is wanted. This is dead simple, as libmakepkg allows creating bash functions that run in makepkg itself and can talk to check_option() It is also even more magical, because then there isn't even an obvious executable file to look at... Another option would be to look at CFLAGS (which are exported) and enable debug iff "-g" is present. (Maybe meson could do this and translate that to vala or rust or java or whatever flags, since it is trying to provide seamless support for this everywhere...)
We can move arch-meson's buildtype to debugoptimized as soon as we start building debug packages.
So then if someone checks out our PKGBUILDs and tries to build non-debug versions, our FLAGS override and erase the buildtype resulting in a plain old release build? Because no, appending our FLAGS to the buildtype FLAGS is *not* the same as overriding them. That's just adding more FLAGS and praying that every buildtype FLAG is contra-indicated by an opposing makepkg.conf FLAG. -- Eli Schwartz Bug Wrangler and Trusted User
On 2018-03-13 15:32, Eli Schwartz via arch-dev-public wrote:
This wrapper changes depending on which version of meson you have installed.
The wrapper is part of the package that changes.
The fact that autoconf has weird bugs like needing to set localstatedir and sysconfdir to their defaults minus the /usr prefix, is an autoconf bug. Maybe we could work on having meson not require setting bizarre options just to get sane defaults?
Neither is a bug. It's just a convenience script. Do you propose to work on upstreaming defaults that make sense for Arch, but not necessarily for what other distributions do or aim for?
(Setting the prefix automatically propagates to everything specified with relative paths, which is most things, and libexecdir and sbindir could just specify a *different* relative path.)
(Which is completely unrelated to our use case, even if you put this between parentheses.)
So other languages, which completely disregard our CFLAGS but which meson itself has magic wrappers for... and the solution is to add a magic script to use in the PKGBUILD which does not respect makepkg.conf but can add debug symbols only loosely related to what we were trying to get? A script which arbitrarily adds debug symbols to packages that were rebuilt with OPTIONS=(!debug) by users who wanted to make local modifications?
Other languages ignore CFLAGS (hint: these are CFLAGS after all) anyway. Users wanting to make local modifications were always on their own.
since apparently meson is so perfect we should do literally everything directly in meson.
A cheap hyperbole. Obviously unnecessary if you want to be taken seriously.
Or we could at least make a *proper* wrapper which can talk to makepkg and determine if debug info is wanted.
Patches welcome. Would be more productive use of your time than making a storm in the glass of water here. Bartłomiej
On 2018-03-10 11:34, Antonio Rojas via arch-dev-public wrote:
Hi, Currently most of our packages which use the cmake build system are built with -DCMAKE_BUILD_TYPE=Release. This provides a reasonable (according to upstream) set of C(XX)FLAGS defaults which are appended to and override our default C(XX)FLAGS. So, for instance, our cmake packages are being built with -O3 instead of our default -O2. Besides the inconsistency that this brings to our binaries, IMO it's not a good idea to override our default C(XX)FLAGS unless there's a good reason to. This will also cause issues once we start building debug packages by default (-DCMAKE_BUILD_TYPE=Release also adds DNDEBUG). Therefore I'm proposing to drop -DCMAKE_BUILD_TYPE from all our cmake packages, building them with our default C(XX)FLAGS as all other packages. Comments?
Sounds good. I'm also surprised that it's how it works, honestly. Are LDFLAGS taken into account regardless of CMAKE_BUILD_TYPE? Will there a to do list to track the progress? Bartłomiej
El domingo, 11 de marzo de 2018 19:11:13 (CET) Bartłomiej Piotrowski via arch-dev-public escribió:
Sounds good. I'm also surprised that it's how it works, honestly. Are LDFLAGS taken into account regardless of CMAKE_BUILD_TYPE? Will there a to do list to track the progress?
Yes, linker flags work the same way: CMAKE_SHARED_LINKER_FLAGS is taken from the env LDFLAGS and then the different build types append stuff to it (Release doesn't seem to add anything here). If everybody agrees with the change, I'd prefer mass-removing it from svn - I'd rather not add yet another huge todo list that will be sitting there unfinished for months.
Hi, On 10-03-18, Antonio Rojas via arch-dev-public wrote:
Currently most of our packages which use the cmake build system are built with -DCMAKE_BUILD_TYPE=Release. This provides a reasonable (according to upstream) set of C(XX)FLAGS defaults which are appended to and override our default C(XX)FLAGS. So, for instance, our cmake packages are being built with -O3 instead of our default -O2. Besides the inconsistency that this brings to our binaries, IMO it's not a good idea to override our default C(XX)FLAGS unless there's a good reason to. This will also cause issues once we start building debug packages by default (-DCMAKE_BUILD_TYPE=Release also adds DNDEBUG). Therefore I'm proposing to drop -DCMAKE_BUILD_TYPE from all our cmake packages, building them with our default C(XX)FLAGS as all other packages. Comments?
This sounds like a good idea, with a small caveat: I have seen a few projects implement some form of: if (NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE Release) endif () That is, for these projects, CMAKE_BUILD_TYPE defaults to "Release" if not set. Thus, I suggest we explicitely set the build type to "None" instead. Baptiste
participants (8)
-
Allan McRae
-
Antonio Rojas
-
Baptiste Jonglez
-
Bartłomiej Piotrowski
-
Bruno Pagani
-
Eli Schwartz
-
Jan Alexander Steffens
-
Levente Polyak