Created attachment 370278 [details] [review]
Meson: Define G_ENABLE_DEBUG and friends

For reference, here is what meson doc says about build types:
http://mesonbuild.com/Running-Meson.html#configuring-the-source

Review of attachment 370278 [details] [review]:

::: meson.build
@@ +193,3 @@
+if buildtype.startswith('debug')
+  glib_debug_cflags += '-DG_ENABLE_DEBUG'
+elif buildtype == 'release'

No.

The old "--enable-debug=no" is *not* what we used for releases. Releases from unstable branches still had full debugging, and releases from stable branches only had '-DG_DISABLE_CAST_CHECKS' enabled.

Disabling everything was only ever meant for resource constrained environments, and with Meson those are much better served by using `--buildtype=plain` and injecting their own compiler flags.

Releases should just disable cast checks; see the equivalent check in GTK's meson.build file.

Meson doc says "release" is full optimization, and distro should build "plain". So I think it makes sense to disable everything on release build. The terminology in meson is counter intuitive.

(In reply to Xavier Claessens from comment #4)
> Meson doc says "release" is full optimization, and distro should build
> "plain".

I really care zero or less about what Meson considers "full optimization". :-P

Disabling pre-condition checks and assertions is *not* something we do, especially for releases. Full stop.

> So I think it makes sense to disable everything on release build.

No, it doesn't make any sense. It will lead to undiagnosable crash reports, to the effect that nobody will *ever* use `--buildtype=release`.

Again: we only ever allowed people to disable everything because they needed a very specific build of GLib/GTK for very specific environments. Those are never to be considered "release" builds, to the point that people that used to configure GLib and/or GTK with `--enable-debug=no` are *much* better served by using `--buildtype=plain` and figuring out what they need to pass themselves.

Created attachment 370297 [details] [review]
Meson: Define G_ENABLE_DEBUG and friends

Just as a side note, I remember discussing this, but I can't remember which package was involved and if it was on the IRC, on a bug at bugzilla, or at gitlab, so I don't have the final conclusions.

Having said this, I used to apply the following convention[0] by using both `buildtype` and `b_ndebug` (this one is used to enable or disable assertions) meson options:

- If builtype is a `debug` build, set the `G_ENABLE_DEBUG` macro.
- If builtype is a `debugoptimized` build, set the `G_DISABLE_CAST_CHECKS` macro (somehow equivalent to `debug=minimum`).
- If builtype is not a `debug*` build, set both `G_DISABLE_CAST_CHECKS` and `-G_DISABLE_CHECKS`.
- If `b_ndebug` is true, set `G_DISABLE_ASSERT`.

However, I have the feeling that this wasn't right and it was later changed. I applied a different solution in `gnome-settings-daemon`[1]:

- If buildtype is `debug*`, set the `G_ENABLE_DEBUG` macro.
- If buildtype is `release`, set the `G_DISABLE_CAST_CHECKS` macro.
- If `b_ndebug` is true, set `G_DISABLE_ASSERT`.

If this doesn't make sense for you, I can search harder and try to find that discussion.

Hope it helps,

[0] https://bugzilla.gnome.org/show_bug.cgi?id=792699#c7
[1] https://gitlab.gnome.org/GNOME/gnome-settings-daemon/blob/master/meson.build#L59

Review of attachment 370297 [details] [review]:

:+1:

Attachment 370297 [details] pushed as cc7e0f6 - Meson: Define G_ENABLE_DEBUG and friends