GNOME Bugzilla – Bug 784354
Port to meson build system
Last modified: 2018-01-07 15:44:52 UTC
Created attachment 354697 [details] [review] Port to meson build system It's a complete meson build port. I've been doing permutations with the options, testing what I know about the program and its plugins and, as far as I know, everything I tried is working. The port also has some special features due to meson's inner workings, but I tried to get a clean and consistent port.
Created attachment 354877 [details] [review] Port to meson build system An update to the meson build port with some improvements. I have also created a wip branch with the meson build port to ease its testing: https://git.gnome.org/browse/eog/log/?h=wip/inigomartinez/meson
This fails to compile the marshaller code for me: src/eog-marshal.c:54:1: error: no previous prototype for 'eog_marshal_VOID__INT_INT' [-Werror=missing-prototypes] Looking at bug 781755 and [1] we may need to depend on a more recent Glib and Meson, no? [1]: https://github.com/mesonbuild/meson/pull/2049
Yes, you are right. I'm sorry, I was focused on tests and I forgot to say that it needs a newer (unreleased) version of meson (0.42.0). Although the port has 0.41.0, I used it because it was the latest released version. Let me please fix it, and I will also review the whole patch in search of any improvement.
Created attachment 356610 [details] [review] Port to meson build system Updated meson build port patch. Most of them are aesthethic changes. The main changes are as follows: - Changed meson value for 0.42.0 as minimum version to avoid a problem with marshalers generation. - Changed config.h value setting, which improves readibility. - Removed C99 as standard, using the compiler's default standard instead. - Removed LIBEOG_3 as section name in linker script. Although the meson port is working fine, there is a minor issue regarding EXIF-related documentation generation, which is already noted in the code. I have also updated the wip branch[0] for testing, which should work with the last development meson version. [0] https://git.gnome.org/browse/eog/log/?h=wip/inigomartinez/meson
Created attachment 356611 [details] [review] Remove autotools Some applications are removing autotools once they acquire a complete meson build port. This patch removes autotools in case it is considered.
Created attachment 356830 [details] [review] Port to meson build system Very small update of the meson build port. Although the patch just intended to remove a duplicated comment line, I had also created a new list for the extra compiler flags, as I was reusing the list with the flags that were going to be tested, which I think makes the build file clearer.
Created attachment 356851 [details] [review] Port to meson build system Although some applications are moving to meson as the only build system, this patch update makes autotools to also distribute the meson build files, just in case both build system are available.
Created attachment 356852 [details] [review] Remove autotools As a follow up of the previous patch update, this patch update removes autotools. The wip branch has also been updated[0]. [0] https://git.gnome.org/browse/eog/log/?h=wip/inigomartinez/meson
Created attachment 359773 [details] [review] Port to meson build system A follow up patch update which bumps eog's version number to 3.26. It has also some aesthetic changes.
Created attachment 359774 [details] [review] Remove autotools A follow up patch update.
Created attachment 359775 [details] [review] Raise glib version requirement > This fails to compile the marshaller code for me: > > src/eog-marshal.c:54:1: error: no previous prototype for 'eog_marshal_VOID__INT_INT' [-Werror=missing-prototypes] > > Looking at bug 781755 and [1] we may need to depend on a more recent Glib and Meson, no? > > [1]: https://github.com/mesonbuild/meson/pull/2049 Seems that I wasn't fully aware of the problem, as I thought that it was located in meson, until I hit it again in another meson port[0]. Actually the problem also depended on `glib-genmarshal` which should be a recent version. This patch raises the glib version to solve this problem. Although raising glib version doesn't imply a recent `glib-genmarshal`, it should at least cover the general case, as they are usually distributed along. [0] https://bugzilla.gnome.org/show_bug.cgi?id=787381
Finally, it hasn't been noted in the comments, but the port meson has one minor issue. The documentation generated by gtkdoc depends on a dinamically created `eog-docs.xml` file, which can conditionally hold `exif` data. At the moment the meson's gtkdoc module doesn't have support for this, but there is an opened issue to support it[0]. As a workaround, meson uses a pre-created `eog-docs.xml` file. [0] https://github.com/mesonbuild/meson/issues/2271
Nice, I'll try taking a look at soonish so we can get this in for 3.28. (In reply to Iñigo Martínez from comment #12) > > The documentation generated by gtkdoc depends on a dinamically created > `eog-docs.xml` file, which can conditionally hold `exif` data. At the moment > the meson's gtkdoc module doesn't have support for this, but there is an > opened issue to support it[0]. Yeah, that clutch is (was?) necessary when building without Exif support. However, looking at the XInclude specs[1] I wonder if we could make it static using an empty xi:fallback element. I think the build error that is fixed by it was caused by the XML file not being there since it's not disabled in the Makefile already. [1] https://www.w3.org/TR/xinclude/#fallback_element
(In reply to Felix Riemann from comment #13) > Nice, I'll try taking a look at soonish so we can get this in for 3.28. > > (In reply to Iñigo Martínez from comment #12) > > > > The documentation generated by gtkdoc depends on a dinamically created > > `eog-docs.xml` file, which can conditionally hold `exif` data. At the moment > > the meson's gtkdoc module doesn't have support for this, but there is an > > opened issue to support it[0]. > > Yeah, that clutch is (was?) necessary when building without Exif support. > However, looking at the XInclude specs[1] I wonder if we could make it > static using an empty xi:fallback element. I think the build error that is > fixed by it was caused by the XML file not being there since it's not > disabled in the Makefile already. > > [1] https://www.w3.org/TR/xinclude/#fallback_element I have just tried the 'xi:fallback' approach and it works :). It could be applied as a permanent solution, or as a temporal solution until meson gets support for generated files in gtkdoc. Let me know what approach fits better for you and I will update the meson patch.
Oh, I think I'll have to clean-up our #ifdef's a bit before this works. I noticed that vars like HAVE_EXIF and HAVE_LCMS are defined as 1 and 0 by meson in config.h while configure defined it to 1 but #undef'd it if it wasn't active. This works while the feature is enabled but fails to compile if it gets disabled as eog uses #ifdef and #if inconsistently. Or is there another way to achieve this in meson? Changing the config_h.set10() commands to just .set() didn't work, as it defines the macros without values. (In reply to Iñigo Martínez from comment #14) > (In reply to Felix Riemann from comment #13) > > Nice, I'll try taking a look at soonish so we can get this in for 3.28. > > > > (In reply to Iñigo Martínez from comment #12) > > > > > > The documentation generated by gtkdoc depends on a dinamically created > > > `eog-docs.xml` file, which can conditionally hold `exif` data. At the moment > > > the meson's gtkdoc module doesn't have support for this, but there is an > > > opened issue to support it[0]. > > > > Yeah, that clutch is (was?) necessary when building without Exif support. > > However, looking at the XInclude specs[1] I wonder if we could make it > > static using an empty xi:fallback element. I think the build error that is > > fixed by it was caused by the XML file not being there since it's not > > disabled in the Makefile already. > > > > [1] https://www.w3.org/TR/xinclude/#fallback_element > > I have just tried the 'xi:fallback' approach and it works :). > > It could be applied as a permanent solution, or as a temporal solution until > meson gets support for generated files in gtkdoc. > > Let me know what approach fits better for you and I will update the meson > patch. I tried with the autotools build and apparently the problem we were trying to fix there, wasn't that the file wasn't created but that it was empty. Considering this to be a pretty unusual configuration I'd say having an empty page in the developer docs is not a problem. We could also add a notice to the page that reminds the user that an empty page means that Exif support is disabled.
Comment on attachment 359775 [details] [review] Raise glib version requirement The change has been recently applied to autotools in 9fe34b11eedbec4a294168a6a4936cf66702bef0.
Created attachment 363334 [details] [review] build: Port to meson build system This patch update has the following major changes: - I have rebases the changes with eog 3.27.1 changes, so it's now up to date. - This also includes the glib version raise patch which has been oboleted by a recent patch. - I have removed a lot of configuration settings which are present in autotools but actually don't seem to be used. This includes the following: - All the standard headers checks. - Some function checks. - Some feature variables. - Extra variables. - Some package configuration settings, although they are not used, have been left as they might be useful in the future, but they can also be removed if desired. - meson version has been raised to 0.43, as it includes a nice feature to check flag options supported by the compiler. - A `LINGUAS` file has been included for yelp support, because `linguas` parameter has been deprecated and is going to be removed on meson. (In reply to Felix Riemann from comment #15) > Oh, I think I'll have to clean-up our #ifdef's a bit before this works. > I noticed that vars like HAVE_EXIF and HAVE_LCMS are defined as 1 and 0 by > meson in config.h while configure defined it to 1 but #undef'd it if it > wasn't active. > > This works while the feature is enabled but fails to compile if it gets > disabled as eog uses #ifdef and #if inconsistently. > > Or is there another way to achieve this in meson? Changing the > config_h.set10() commands to just .set() didn't work, as it defines the > macros without values. Although it's true that the inconsitency regarding guards it's not good, I should have checked this better. I usually do full builds, with all the options enabled, to check if I'm missing something. In this case, I should also have tried different permutations, and also all the options disabled. I've made some changes and it should build properly now. Please, let me know if there is still any issue with this. [0] https://bugzilla.gnome.org/attachment.cgi?id=359775 [1] https://git.gnome.org/browse/eog/commit/?id=9fe34b11eedbec4a294168a6a4936cf66702bef0
Created attachment 363335 [details] [review] build: Remove autotools A followup update to `Remove autotools`. Regarding this patch, if this option is considered, there are steps before/after to be done. One of the steps is related to `gnome-continuous`[0]. Actually the `configure` script is not strictly necessary, as `gnome-continuous` may patch it on the fly[1]. It's up to the maintainer to include it in the source code tree. Another step is related to `jhbuild`. It must be updated to set the build system to meson. This also implies a release with meson build system merged. There is more information in the GNOME wiki[2]. [0] https://mail.gnome.org/archives/desktop-devel-list/2017-April/msg00080.html [1] https://git.gnome.org/browse/gnome-continuous/tree/patches [2] https://wiki.gnome.org/Initiatives/GnomeGoals/MesonPorting
Created attachment 363336 [details] [review] build: Remove conditional EXIF documentation (In reply to Felix Riemann from comment #15) > I tried with the autotools build and apparently the problem we were trying > to fix there, wasn't that the file wasn't created but that it was empty. > > Considering this to be a pretty unusual configuration I'd say having an > empty page in the developer docs is not a problem. We could also add a > notice to the page that reminds the user that an empty page means that Exif > support is disabled. This `RFC` patch makes `EXIF` documentation generation non-conditional, so it's build always when using meson. However, as you already know, when `EXIF` support is disabled it generates a blank page. Please, let me know if this mostly fits what you have in mind, so we can improve it.
As always all these changes have been included in the wip branch: https://git.gnome.org/browse/eog/log/?h=wip/inigomartinez/meson
Created attachment 363406 [details] [review] build: Port to meson build system There are new meson porting guidelines[0] for naming the options, so option names have been changed. I've updated the patch renaming those options accordingly. [0] https://wiki.gnome.org/Initiatives/GnomeGoals/MesonPorting
Review of attachment 363336 [details] [review]: Yeah, I think this is a viable option. There's probably not too many people hitting this case and we can see if can get a hint onto that page if libexif is disabled.
(In reply to Iñigo Martínez from comment #18) > Created attachment 363335 [details] [review] [review] > build: Remove autotools > > A followup update to `Remove autotools`. > > Regarding this patch, if this option is considered, there are steps > before/after to be done. > > One of the steps is related to `gnome-continuous`[0]. Actually the > `configure` script is not strictly necessary, as `gnome-continuous` may > patch it on the fly[1]. It's up to the maintainer to include it in the > source code tree. From what I gather it is sufficient to include that dummy configure script in the tarball to keep gnome-continuous working. That's no big deal. > Another step is related to `jhbuild`. It must be updated to set the build > system to meson. This also implies a release with meson build system merged. Replacing 'autotools' with 'meson' sounds manageable as well. ;-)
Review of attachment 363406 [details] [review]: This looking really good so far. There's just one thing I noticed. The eog.pc that gets created now also contains the information necessary to link to the private libeog.so. The problem is that it adds $(prefix)//usr/lib64/eog to the search path which obviously wrong. This probably (haven't tried) breaks the eog-plugins build. I am not sure if that's not a bug with meson itself. On one hand install_dir is shown to be used with absolute paths[0] but then it's said[1] that it should be prefix-relative. Apperently the latter seems to apply for the pkg-config support as setting libeog's install_dir relative to the prefix makes it work. [0]: http://mesonbuild.com/Installing.html#installing [1]: http://mesonbuild.com/Reference-manual.html#executable
Review of attachment 363406 [details] [review]: commit b55f66560e6eff902b08346198ceb15b5500aed3 Author: Iñigo Martínez <> Date: Thu Jun 29 18:05:02 2017 +0200 build: Port to meson build system meson is a build system focused on speed an ease of use, which helps speeding up the software development. This patch adds meson support along autotools. https://bugzilla.gnome.org/show_bug.cgi?id=784354
Review of attachment 363336 [details] [review]: commit b55f66560e6eff902b08346198ceb15b5500aed3 Author: Iñigo Martínez <> Date: Thu Jun 29 18:05:02 2017 +0200 build: Port to meson build system meson is a build system focused on speed an ease of use, which helps speeding up the software development. This patch adds meson support along autotools. https://bugzilla.gnome.org/show_bug.cgi?id=784354
Allright, I pushed the first two patches with the pkg-config fix as they work pretty well. Thanks a lot for the patches. :) commit c2e02d7b12b48a5c013110d9ebfb64edf2ebf5e8 Author: Felix Riemann <> Date: Sun Nov 12 18:46:29 2017 +0100 build: Set prefix-relative install_dir for libeog While installing seems to have no issues with absolute paths for install_dir the pkgconfig integration in meson does and create wrong library paths in the .pc file. https://bugzilla.gnome.org/show_bug.cgi?id=784354 I'll leave autoconf in place for the tomorrows release and see if everything works well in terms if "make distcheck" for the next release with meson.
Okay, it seems the RPATH was not being set by meson as libtool does. commit 67984c03d748b4d36203c1e2c9a6e8f1492cfd14 Author: Felix Riemann <> Date: Sun Nov 12 19:48:30 2017 +0100 build: meson: Set RPATH for eog binary Otherwise it wouldn't be able to find it after installation https://bugzilla.gnome.org/show_bug.cgi?id=784354
Created attachment 363453 [details] [review] build: Add symbol.map to distributable files (In reply to Felix Riemann from comment #27) > Allright, I pushed the first two patches with the pkg-config fix as they > work pretty well. Thanks a lot for the patches. :) Thank you for taking your time in testing the patches, and also fixing the issue with pkg-config :) > I'll leave autoconf in place for the tomorrows release and see if everything > works well in terms if "make distcheck" for the next release with meson. I've tried it on advance, as I wanted to get ahead of any problem that might arise. There was a mistake regarding the new linker script file, `symbol.map` which was not included in the distributable file list. This patch adds the file to that list. I've tried the whole process and meson build is complete (and it works). BTW, since meson 0.41.0 is able to generate also dist files by using the `ninja dist` command[0]. [0] http://mesonbuild.com/Release-notes-for-0-41-0.html#a-target-for-creating-tarballs
Created attachment 363454 [details] [review] build: Add configure script for meson I'll uploading this just in case it might help to those who are used to autotools and also to gnome-continuous. However: - eog patching is not mandatory because this patch can be used directly on gnome-continuous. - gnome-continuous might not need these patches in the near future[0]. Don't hesitate to mark it as obsolete if you think that it's actually unnecessary. [0] https://bugzilla.gnome.org/show_bug.cgi?id=789146#c3
Review of attachment 363453 [details] [review]: Seems good. :-) commit 04d0d80166e3fb09e268af4bdc0469654954c8df Author: Iñigo Martínez <> Date: Sun Nov 12 19:55:48 2017 +0100 build: Add symbol.map to distributable files symbol.map file which is used as a linker script, is missing from the distributable file list in autotools. https://bugzilla.gnome.org/show_bug.cgi?id=784354
Since we have no other changes queued for 3.27.4, I think this might be a good opportunity to drop autotools support. If it causes any problems, one can safely revert to version 3.27.2. To start I just switched the jhbuild moduleset to use meson to build eog.
And done... :) commit b2dcab7e582c5b9e3d6131361b074bf215bb40fe Author: Iñigo Martínez <> Date: Thu Aug 3 16:09:48 2017 +0200 build: Remove autotools To avoid the burden of maintaining multiple build systems, this patch removes autotools support. https://bugzilla.gnome.org/show_bug.cgi?id=784354 The extra configure script for gnome-continuous seems no longer necessary: https://mail.gnome.org/archives/desktop-devel-list/2017-December/msg00007.html Thanks again Iñigo for preparing all this. :) --- This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.