GNOME Bugzilla – Bug 783819
Port to meson build system
Last modified: 2018-03-23 10:20:54 UTC
Created attachment 353819 [details] [review] Port to meson build system Port to meson build system. Although the port is complete (surely there is space for further improvements), there is a minor issue with meson related to gtkdoc documentation generation[0]. It does work with meson 0.40.[01], but those versions lack a feature regarding custom variables in pkgconfig's pc file generation[1] which has been added in 0.41.0 and is used in the patch. Any suggestion is greatly appreciated, [0] https://github.com/mesonbuild/meson/issues/1946 [1] http://mesonbuild.com/Release-notes-for-0-41-0.html#pkgconfig-support-for-custom-variables
Link to the GNOME goal: https://wiki.gnome.org/Initiatives/GnomeGoals/MesonPorting As far as I'm concerned, I've never learned meson and I'm familiar with the Autotools. For GtkSourceView and gspell I've marked "not wanted for now" in the gnome goal. Partly for the reasons explained in this mail: https://mail.gnome.org/archives/desktop-devel-list/2017-February/msg00018.html Meson is still young, as you said there are some problems for gtk-doc. Also by looking quickly at the patch, it sucks to list all the CFLAGS (test_cflags in the main meson.build), with the Autotools there is the AX_COMPILER_FLAGS macro. In GNOME there is this continuous stream of maintenance stuff to do, unrelated to actually implement features… Porting to new versions of libraries (especially GTK+), updating the build system to use the autoconf-archive macros, now rewriting from scratch the build system to use meson, etc etc. It's a bit painful. So in short, for meson in Devhelp, there is no hurry, I would prefer to wait the next development cycle or next year. But maybe other Devhelp maintainers have another opinion.
Created attachment 354261 [details] [review] Port to meson build system I'm sorry I wasn't aware of the thread in the mailing list. I joined the desktop-devel-list in April and I hadn't read the previous mails. I agree with you that meson is still young, though I think that the best way of improving it is by testing it in real situations. After some months working with meson, in spite of the problems I may have had due to its youth, I have been quite comfortable with it. However, after having worked several years with autotools, there have always been things that elude me, though maybe I have never spent so much time with it. I understand that those flags may draw your attention. I must say that, even though I tried my best, it probably wasn't done the best way possible. meson has 3 warning levels[1]: the first one adds '-Wall' and '-Winvalid-pch' flags, the second one adds '-Wextra', and finally, the third one adds '-Wpedantic'. I have created a new patch which removes the flags included in those levels and which takes advantage of meson options. The aim of those meson options is for the developer not to care about them, in a similar way as AX_COMPILER_FLAGS does. As I mentioned before, meson still has space for improvement, in the same way as you said in your mail[1], but I think that, at the moment, it is an interesting option and it is moving in the right direction. If you think that you may reconsider meson at any point, I may continue working on it. Maybe a branch for meson would be an interesting idea as other applications have done. What do you think? [0] https://mail.gnome.org/archives/desktop-devel-list/2017-February/msg00018.html [1] http://mesonbuild.com/Quick-guide.html#compiling-a-meson-project
Yes, if you have write access to git.gnome.org, please push a wip/meson branch. At least with the branch we know where in the git history the commit has been created.
Done!
As I said here: https://bugzilla.gnome.org/show_bug.cgi?id=784351#c3 I think we won't keep the Autotools ad vitam aeternam, longer term keeping the Autotools will be problematic because of missing macros. So the Meson port should be done before creating the libdevhelp repo, see bug #784351.
I might have some spare time in the next few days. Would you like me to review and update the patches to devhelp's current status?
Yes it would be useful, I think we can merge the branch this development cycle (for GNOME 3.28) and drop Autotools. It will force me to learn Meson.
Created attachment 360113 [details] [review] Port to meson build system An updated patch with the latest changes in master branch. I would say that it's fully ported, though there are some subtle changes: - devhelp-3.devhelp2 file is actually generated as devhelp.devhelp2. - The GIR file generated with autotools has `Dh,dh` as `identifier prefixes` and `dh,dh` as `symbol prefixes`, while the one generated with meson has `Dh` only as `identifier prefixes` and `dh` as `symbol prefixes`. Please, let me know if there is any problem with these changes and I can try to fix them.
Created attachment 360116 [details] [review] build: Remove autotools As you may know, some applications are removing autotools to avoid the burden of maintaining two build systems. This patch removes autotools in case it is considered.
In order to ease testing the meson port, I've updated the wip branch: https://git.gnome.org/browse/devhelp/log/?h=wip/meson Please, let me know any issue or suggestion you may have.
Thanks, I'll look at it in the coming month.
Review of attachment 360113 [details] [review]: I've started to read the Meson docs and look at your commits. When I launch the devhelp app, I don't see the Devhelp API reference, so there is something wrong with the gtk-doc build. I've skimmed the main meson.build, some comments below: ::: meson.build @@ +93,3 @@ + ['HAVE_SYS_STAT_H', 'sys/stat.h'], + ['HAVE_SYS_TYPES_H', 'sys/types.h'], + ['HAVE_UNISTD_H', 'unistd.h'] Are all these headers included somewhere in the Devhelp source code? I doubt so. @@ +105,3 @@ + ['HAVE_DCGETTEXT', 'dcgettext'], + ['HAVE_GETTEXT', 'gettext'], + ['HAVE_ICONV', 'iconv'] iconv is not used in Devhelp, why is there a check for it? @@ +111,3 @@ + check_functions += [ + ['HAVE_CFLOCALECOPYCURRENT', 'CFLocaleCopyCurrent'], + ['HAVE_CFPREFERENCESCOPYAPPVALUE', 'CFPreferencesCopyAppValue'] git grep CFLocaleCopyCurrent and git grep CFPreferencesCopyAppValue return nothing in the source code of Devhelp. Why did you include the check for those functions? I think you can remove that. @@ +120,3 @@ + +# compiler flags +common_flags = ['-DHAVE_CONFIG_H'] This is an Autotoolism, a later commit can remove all HAVE_CONFIG_H's from the source code and remove the flag. @@ +150,3 @@ + '-Wundef', + '-Wunused-but-set-variable', + '-Wwrite-strings' In the autoconf-archive git repository, I see that there are new flags that have been added recently in ax_compiler_flags_cflags.m4. So it's better to keep the list in sync with the ones from the *.m4 macro.
Review of attachment 360113 [details] [review]: ::: meson_options.txt @@ +1,2 @@ +option('enable-gtk-doc', type: 'boolean', value: false, description: 'use gtk-doc to build documentation') +option('enable-introspection', type: 'boolean', value: true, description: 'Enable GObject Introspection (depends on GObject)') Is there now a convention in GNOME for naming those options? I know that "enable" should be removed from the option names, this is an Autotoolism.
Another thing, the build API wrapper is needed for gnome-continuous. This should be added with a separate commit.
Have you used a build system converter? http://mesonbuild.com/Build-system-converters.html Because the meson.build that you provide look very "Autotoolish". I think the code that results from such a converter is quite ugly, so it should be avoided if possible, or it needs to be heavily cleaned up afterwards. In Meson things can be done differently, so it's possible to simplify the code compared to what is done with Autotools.
(In reply to Sébastien Wilmet from comment #12) > Review of attachment 360113 [details] [review] [review]: > > I've started to read the Meson docs and look at your commits. Thank you very much for taking your time. > When I launch the devhelp app, I don't see the Devhelp API reference, so > there is something wrong with the gtk-doc build. This is something that worries me and that we should investigate. > + ['HAVE_UNISTD_H', 'unistd.h'] > > Are all these headers included somewhere in the Devhelp source code? I doubt > so. ... > + ['HAVE_CFPREFERENCESCOPYAPPVALUE', 'CFPreferencesCopyAppValue'] > > git grep CFLocaleCopyCurrent and git grep CFPreferencesCopyAppValue return > nothing in the source code of Devhelp. Why did you include the check for > those functions? I think you can remove that. I would please ask you to read this[0]. I've just removed all these checks. > @@ +120,3 @@ > + > +# compiler flags > +common_flags = ['-DHAVE_CONFIG_H'] > > This is an Autotoolism, a later commit can remove all HAVE_CONFIG_H's from > the source code and remove the flag. There isn't any issue if it's missing, at least on the checks I've made, so I've removed it. > @@ +150,3 @@ > + '-Wundef', > + '-Wunused-but-set-variable', > + '-Wwrite-strings' > > In the autoconf-archive git repository, I see that there are new flags that > have been added recently in ax_compiler_flags_cflags.m4. So it's better to > keep the list in sync with the ones from the *.m4 macro. The flags list in `devhelp's` meson build, is a trimmed down version of this list. It's a trimmed down version because I've removed all the flags which are included in `-Wall`, `-Wextra` and `-pedantic` flags, which can be included by using meson's `warning levels`[1]. [0] https://bugzilla.gnome.org/show_bug.cgi?id=786149#c44 [1] https://mail.gnome.org/archives/desktop-devel-list/2017-July/msg00008.html
(In reply to Sébastien Wilmet from comment #13) > Review of attachment 360113 [details] [review] [review]: > > ::: meson_options.txt > @@ +1,2 @@ > +option('enable-gtk-doc', type: 'boolean', value: false, description: 'use > gtk-doc to build documentation') > +option('enable-introspection', type: 'boolean', value: true, description: > 'Enable GObject Introspection (depends on GObject)') > > Is there now a convention in GNOME for naming those options? I know that > "enable" should be removed from the option names, this is an Autotoolism. There is a reason for this[0]. In the moment you wrote this, there was no convention yet. However, yesterday Emmanuele Bassi uploaded the new meson porting guidelines[0]. I've already send some patches to update the ports I made with those guidelines[2][3][4] (and I still have much to do). [0] https://bugzilla.gnome.org/show_bug.cgi?id=788146#c14 [1] https://wiki.gnome.org/action/diff/Initiatives/GnomeGoals/MesonPorting?action=diff&rev1=112&rev2=113 [2] https://bugzilla.gnome.org/show_bug.cgi?id=786149 [3] https://bugzilla.gnome.org/show_bug.cgi?id=790177 [4] https://bugzilla.gnome.org/show_bug.cgi?id=790178
(In reply to Sébastien Wilmet from comment #14) > Another thing, the build API wrapper is needed for gnome-continuous. This > should be added with a separate commit. I'm aware of this[0][1]. [0] https://bugzilla.gnome.org/show_bug.cgi?id=786149#c36 [1] https://bugzilla.gnome.org/show_bug.cgi?id=786639#c6
(In reply to Sébastien Wilmet from comment #15) > Have you used a build system converter? > http://mesonbuild.com/Build-system-converters.html > > Because the meson.build that you provide look very "Autotoolish". No, I haven't used a converter for neither `devhelp` nor the other >20 ports I've made[0] (the list is in chronological order). Yes, it might look `autotoolish`, but there is a reason for this[1]. ([2][3][4] are linked from that explanation and I might also add another one[5]). > I think the code that results from such a converter is quite ugly, so it > should be avoided if possible, or it needs to be heavily cleaned up > afterwards. In Meson things can be done differently, so it's possible to > simplify the code compared to what is done with Autotools. I don't know how the results are from such converters, because as I mentioned before, I've never used them. Yes, I know things can be done differently in meson, and this has been done in collaboration with other maintainers[6][7][8], and also yes, it's possible to simplify the code compared to what is done with autotools, but not always[9][11]. [0] https://inigomartinez.github.io/2017/10/11/meson-and-gnome/ [1] https://bugzilla.gnome.org/show_bug.cgi?id=788146#c14 [2] https://bugzilla.gnome.org/show_bug.cgi?id=783819#c1 [3] https://bugzilla.gnome.org/show_bug.cgi?id=784762#c1 [4] https://bugzilla.gnome.org/show_bug.cgi?id=784561#c5 [5] https://mail.gnome.org/archives/gnomecc-list/2017-November/msg00000.html [6] https://bugzilla.gnome.org/show_bug.cgi?id=786149 [7] https://bugzilla.gnome.org/show_bug.cgi?id=784762 [8] https://bugzilla.gnome.org/show_bug.cgi?id=784922 [9] https://bugzilla.gnome.org/show_bug.cgi?id=786149#c10 [10] https://bugzilla.gnome.org/show_bug.cgi?id=786149#c16
For HAVE_UNISTD_H and other such Autotools macros, do a `git grep`, if it returns nothing it means that the source code doesn't use the macro, so you can remove it from the meson.build.
Created attachment 363698 [details] [review] build: Port to meson build system An updated meson port patch with the following changes: - Renamed option names following the new meson porting guidelines. - Removed unnecessary defines. - Removed HAVE_CONFIG_H macro definition. - Bumped version meson number to 0.43 to take advantage of `get_supported_arguments`. - Added `-Werror=unknown-warning-option` flag, which may be useful for clang, but it's not for GCC. - AppStream metainfo file name has been fixed. - Removed unnecessary mkdb arguments from documentation generation with GtkDoc. - Removed options set in project's description which already default options for meson.
Created attachment 363703 [details] [review] build: Add a configure script A configure script that might be added for those people who are familiar to autotools. This can be also useful for gnome-continuous or similar tools which need a configure script. The patch may be patched in devhelp's source code, or it can be included as a patch to be applied by gnome-continuous. However, gnome-continuous may understand meson natively at some point so it might also be unnecessary.
Finally, the wip branch[0] has also been updated to ease the review and testing of the patches. Please, let me know any comment or suggestion you may have. [0] https://git.gnome.org/browse/devhelp/log/?h=wip/meson
Created attachment 363710 [details] [review] build: Port to meson build system A quick update to the meson port patch regarding an issue with absolute paths and the pkgconfig integration, as it creates wrong library paths in the `.pc` file. There is an opened issue[0] regarding this problem. [0] https://github.com/mesonbuild/meson/issues/2469
Created attachment 363807 [details] [review] build: Port to meson build system While I was trying to figure out the problem with GtkDoc[0] missing Devhelp API reference, I noticed that GtkDoc wasn't fixing references properly. This update fixes the issue. [0] https://bugzilla.gnome.org/show_bug.cgi?id=783819#c12
(In reply to Sébastien Wilmet from comment #12) > When I launch the devhelp app, I don't see the Devhelp API reference, so > there is something wrong with the gtk-doc build. I've been trying to figure out what's going on behind this issue. First of all, I would like to highlight the main differences between the autotools build and the meson build at file system level. - There is no `libdevhelp-3.la`. meson does not use libtool, so it doesn't create this file nor install it. - The identifier and symbol prefixes in the GIR file are slightly different. autotools uses `Dh,dh` as identifier prefixes and `dh,dh` as symbol prefixes while meson creates the GIR file with `Dh` as identifier prefix and `dh` as symbol prefix. - The devhelp file is created as `devhelp-3.devhelp2` in autotools and as `devhelp.devhelp2` in meson. My knowledge regarding GtkDoc is quite limited, as I've never used it. Although I use Devhelp as a documentation source for different libraries, I've never used it to check devhelp's documentation, so I don't know exactly how do I reproduce the issue. I would like to know if the change in the name is a critical issue and if you think it would be the source of the issue regarding missing devhelp API reference.
In Meson I really don't like the fact that all variables are global, see: https://github.com/mesonbuild/meson/issues/2607 For a small project like Devhelp this shouldn't be a major problem, it should still be manageable IF you follow my convention to name global variables in UPPER_CASE (that is, variables that are used in several meson.build files). ----- Regarding the coding style for meson.build files, there is currently no standard like PEP 8 for Python. The most logical to me is to indent with tabs, with a tab width of 8 characters. And for function calls which take a lot of arguments, one arg per line aligned on the opening parenthesis. You can see an example of this coding style here: https://github.com/swilmet/gnome-c-utils ----- For having the equivalent of AX_COMPILER_FLAGS, please copy the code from: https://github.com/mesonbuild/meson/issues/2609 (or from gnome-c-utils linked above). ----- Debugging build issues is not fun, I thought that Meson would cause less problems. But Meson is still young, maybe the gtk-doc support is buggy. I don't want to debug that myself, it works fine with the Autotools.
Also: Please *first* remove all the Autotools files, and *then* add the Meson build system. We are not interested to maintain two build systems in parallel.
(In reply to Sébastien Wilmet from comment #27) > ----- > > Regarding the coding style for meson.build files, there is currently no > standard like PEP 8 for Python. The most logical to me is to indent with > tabs, with a tab width of 8 characters. And for function calls which take a > lot of arguments, one arg per line aligned on the opening parenthesis. > > You can see an example of this coding style here: > https://github.com/swilmet/gnome-c-utils > > ---- Forgot to link to the Meson issue: https://github.com/mesonbuild/meson/issues/2608 See this comment: https://github.com/mesonbuild/meson/issues/2608#issuecomment-347879938 Iñigo: so I think we can keep your coding style, it looks the same as used in Meson unit tests, you can forget my comment.
(In reply to Iñigo Martínez from comment #26) > My knowledge regarding GtkDoc is quite limited, as I've never used it. > Although I use Devhelp as a documentation source for different libraries, > I've never used it to check devhelp's documentation, so I don't know exactly > how do I reproduce the issue. > > I would like to know if the change in the name is a critical issue and if > you think it would be the source of the issue regarding missing devhelp API > reference. The best course of action here is to first ensure that the output files (at least filenames) are the same with Meson and Autotools, for gtk-doc. With the same filenames, the Devhelp API doc should appear in Devhelp.
Comment on attachment 363703 [details] [review] build: Add a configure script No longer needed, GNOME Continuous supports meson now.
Comment on attachment 360116 [details] [review] build: Remove autotools Finally I think it'll be easier to just add the meson build system in parallel to Autotools, as a first step. Like it is currently done in GLib for example. Meson bugs can be fixed later. When building devhelp with meson works perfectly, we can remove Autotools. So even if the meson patch is not perfect, it can be merged (but first it needs to be updated because there has been quite a lot of changes in the past few months).
Created attachment 368934 [details] [review] build: Port to meson build system I have update the meson port patch which is now based on the work in the `wip/devhelp-next` branch[0]. I have also created a new wip branch[1] to ease its testing. Probably `wip/meson` branch[2] can be removed now. Please, let me know any comments you may have. [0] https://bugzilla.gnome.org/show_bug.cgi?id=784351#c5 [1] https://git.gnome.org/browse/devhelp/log/?h=wip/inigomartinez/devhelp-next-meson [2] https://git.gnome.org/browse/devhelp/log/?h=wip/meson
Review of attachment 368934 [details] [review]: I prefer if you don't touch the Makefile.am files. As soon as the meson build works fine, we can drop all the Autotools files and switch to meson. In the meantime when a tarball is released with `make distcheck`, downstreams must use configure and make, not meson. So it's not very useful to distribute meson files in tarballs when the Autotools must still be used. As soon as GNOME 3.28 is released I'll merge the wip/devhelp-next branch and the meson branch as well. The meson build can be improved over time.
(In reply to Sébastien Wilmet from comment #27) > In Meson I really don't like the fact that all variables are global, see: > https://github.com/mesonbuild/meson/issues/2607 > > For a small project like Devhelp this shouldn't be a major problem, it > should still be manageable IF you follow my convention to name global > variables in UPPER_CASE (that is, variables that are used in several > meson.build files). Please add the following comment at the top of the root meson.build: # Convention: # - Local variables in lower_case. # - Global variables in UPPER_CASE. # See: https://github.com/mesonbuild/meson/issues/2607 And follow the convention. It'll make the code easier to understand. (this is my only unusual requirement for meson).
I'm not sure Meson is the right choice. Bazel looks like better, see: https://en.wikipedia.org/wiki/Bazel_(software) (Meson looks like a toy compared to Bazel).
I think Bazel has a more solid foundation than Meson, but for GNOME a lot of macros/extensions are missing with Bazel and I don't want to develop all those extensions (for GSettings, GResources, GIR, GTK-Doc, enum types, etc etc). So in the end I've ported the build system to Meson myself, sometimes by looking at your patch (thanks!), even if I don't like everything with Meson. And I've now removed the Autotools build system. The first Devhelp release with Meson will be 3.29.1.