GNOME Bugzilla – Bug 791421
Various build related improvements
Last modified: 2018-01-11 11:04:13 UTC
This bug includes various meson improvements.
Created attachment 365295 [details] [review] meson: Use LINGUAS file for help generation The linguas parameter is deprecated in gnome's yelp function for the future meson's versions. The patch uses the LINGUAS file instead of the linguas parameter. It also bumps meson's version to support this feature properly. The polish translations has also been added as it was present in autotools but not in meson.
Created attachment 365296 [details] [review] meson: Change build options Following the new meson porting guidelines, this patch renames the build options as follows: - Remove the 'enable' prefix from boolean options. - The character separator in multi-word options has been changed to underscore. The `debug` and `strict-cc` options have also been removed because they can be enabled by using meson's options.
Created attachment 365297 [details] [review] meson: Add support for building libcommon Autotools builds a static library called libcommon which is used to build gnome-boxes but also gnome-boxes-search-provider executable. This patch adds support for building this library.
Created attachment 365298 [details] [review] meson: Add support for search provider meson does not build gnome-boxes-search-provider and hence, it does not generate the search provider service file. This patch builds the gnome-boxes-search-provider executable and generates its service file.
Created attachment 365299 [details] [review] meson: Fix post install script meson uses a post install script for compiling gsettings schemas, updating the icon cache and also updating the desktop database. However, it assumes that the data directory is the share directory under prefix, which might not be correct because the user can choose a different directory. This patch uses the data directory used by meson by passing it to the post install script.
Created attachment 365309 [details] [review] meson: Change build options A minor change to follow the "code conventions" used in the build files.
Created attachment 365310 [details] [review] meson: Add support for building libcommon A minor change to follow the "code conventions" used in the build files.
Created attachment 365311 [details] [review] meson: Support Installed tests Although Boxes has support for Installed tests, this support is limited only to autotools, and meson lacks this feature. This patch adds support for Installed tests on meson.
Created attachment 365312 [details] [review] build: Remove autotools Once the support for Installed tests is in place, the meson build port is able to build fully and install all the files and is on par with autotools. So to avoid the burden of maintaining multiple build systems, this patch removes autotools support. Regarding "Remove autotools", if it's finally merged there are steps before/after to be done. One step is related to `jhbuild`. It must be updated to set the build system to meson. This usually also implies a release with meson build system merged. I don't know how this applies to BuildStream. Another step is usually related to `gnome-continuous`, but IIRC, gnome-boxes does not use it. There is more information in the GNOME wiki[0]. [0] https://wiki.gnome.org/Initiatives/GnomeGoals/MesonPorting
Created attachment 365313 [details] [review] meson: Rename Boxes desktop file The desktop file has a wrong extension due to the compatibility reasons with autotools. However, now that autotools is removed, the file can be renamed properly. This patch renames the file leaving only a `.in` extension which is used as an indication that it must be translated.
Created attachment 365314 [details] [review] meson: Use meson's i18n module meson has a i18n module which can be used to merge translations using `msgfmt`. This patch takes advantage of this module instead of using custom targets.
Created attachment 365315 [details] [review] meson: Make oVirt support optional Although there is an option to make oVirt support optinal, the GObject-based library to access oVirt REST API must be available to build gnome-boxes. This patch makes the library dependency optional if the oVirt support is disabled.
Created attachment 365321 [details] [review] meson: Remove gresources vala argument The gresources vala argument is not necessary because the resource files are already generated using the gnome's compile resources function, and files added to the boxes source files. This patch removes the vala argument which is not necessary anymore.
Created attachment 365323 [details] [review] meson: Use localedir option The default location for localedir is used, which is usually under datadir directory. However, the user might choose another location by using the available options. This patch uses the localedir option instead of using the default location.
I have uploaded all these changes to a wip branch[0]. This is particularly important for those commits after removing autotools which only affect to meson. The commit flow is more clear in the branch. [0] https://git.gnome.org/browse/gnome-boxes/log/?h=wip/inigomartinez/meson
Created attachment 365697 [details] [review] meson: Use localedir option The "meson: Use localedir option" was not properly tested, so here goes an update.
Review of attachment 365295 [details] [review]: lgtm
Review of attachment 365298 [details] [review]: sure!
Review of attachment 365299 [details] [review]: sure!
Review of attachment 365309 [details] [review]: Thanks! I didn't know about these patterns.
Comment on attachment 365295 [details] [review] meson: Use LINGUAS file for help generation Pushed as b23671c4 - meson: Use LINGUAS file for help generation
Comment on attachment 365309 [details] [review] meson: Change build options Pushed as 69d5f433 - meson: Change build options
Comment on attachment 365299 [details] [review] meson: Fix post install script Pushed as f90438d5 - meson: Fix post install script
Review of attachment 365310 [details] [review]: Thanks!
Review of attachment 365311 [details] [review]: sure!
Review of attachment 365314 [details] [review]: lgtm
Review of attachment 365315 [details] [review]: sure!
Review of attachment 365321 [details] [review]: sure!
Review of attachment 365697 [details] [review]: sure!
Review of attachment 365312 [details] [review]: I will probably hold this one and the subsequent patch for a little while until I can make a more educated decision considering the implications regarding CI and downstreams. For example, downstreams that don't ship meson or python3, etc...
Thanks a lot Iñigo for your contributions!
Comment on attachment 365310 [details] [review] meson: Add support for building libcommon Pushed as 0429f0a8 - meson: Add support for building libcommon
Comment on attachment 365298 [details] [review] meson: Add support for search provider Pushed as b8a452ec - meson: Add support for search provider
Comment on attachment 365311 [details] [review] meson: Support Installed tests Pushed as 8c948eaf - meson: Support Installed tests
Comment on attachment 365321 [details] [review] meson: Remove gresources vala argument Pushed as 7a496d8e - meson: Remove gresources vala argument
Comment on attachment 365697 [details] [review] meson: Use localedir option Pushed as e4d4d136 - meson: Use localedir option
Created attachment 365873 [details] [review] build: Rename Boxes desktop file I have updated the "meson: Rename Boxes desktop file", which it was expected to be applied once autotools was removed. The "meson: Use meson's i18n module" depends on this first issue, so once it's merged, the i18n patch can also be applied. The "meson: Make oVirt support option" is also expected to be applied once autotools is removed. Finally, be aware that at the moment autotools doesn't work. There is a fix for that in #791393.
Created attachment 365876 [details] [review] meson: Use meson's convention The convention when naming dependencies is to use the '_dep' suffix, which helps detecting dependency variables. The variables have been renamed to follow this convention. The dependencies that are also used once, are declared in the target's dependency array which helps meson processing less and also one less variable is needed.
Created attachment 365877 [details] [review] Use glib's version as target-glib meson's is able to resolve the expected vala `target-glib` by using glib's version. At the moment, it appends the 2.38 as the `target-glib` but also 2.44 is also added by using vala's argument. The glib's and gio's versions have been raised to 2.44 along what is used by gobject dependency, and the vala argument is removed, so meson itself is able to add the correct `target-glib` version.
Review of attachment 365876 [details] [review]: sure!
Review of attachment 365877 [details] [review]: ok.
Comment on attachment 365876 [details] [review] meson: Use meson's convention Pushed as db01f16c - meson: Use meson's convention
Comment on attachment 365877 [details] [review] Use glib's version as target-glib Pushed as e3caa8ea - meson: Use glib's version as target glib
Created attachment 366183 [details] [review] build: Fix search provider ini file The search provider ini file has two fields `Title` and `Icon` that are not necessary. These two fields have been removed. The translation merge process has also been disabled because is not necessary to translate this file.
Review of attachment 366183 [details] [review]: sure. Thanks for your patches!
Comment on attachment 366183 [details] [review] build: Fix search provider ini file Pushed as 33d3c878 - meson: Fix search provider ini file
Created attachment 366425 [details] [review] meson: Look for spice-gtk vala bindings
Review of attachment 366425 [details] [review]: Thank you!
Comment on attachment 366425 [details] [review] meson: Look for spice-gtk vala bindings Attachment 366425 [details] pushed as 617168e - meson: Look for spice-gtk vala bindings
(In reply to Iñigo Martínez from comment #11) > Created attachment 365314 [details] [review] [review] > meson: Use meson's i18n module > (In reply to Iñigo Martínez from comment #12) > Created attachment 365315 [details] [review] [review] > meson: Make oVirt support optional > Is there any reason why these two patches haven’t been committed?
(In reply to Piotr Drąg from comment #50) > Is there any reason why these two patches haven’t been committed? Yes, `meson: Use meson's i18n module` (attachment 365314 [details] [review]) depends on `build: Rename Boxes desktop file` (attachment 365873 [details] [review], comment 37) which has not been reviewed yet. Initially the patch depended on the `build: Remove autotools` (attachment 365312 [details] [review]) patch, because the changes to be applied due to the rename of the desktop file were only applied to meson build files. However, this patch is not going to be merged yet, so I ended extending those changes also to autotools build files. Once `build: Rename Boxes desktop file` is applied, it could be merged. Regarding `meson: Make oVirt support optional` (attachment 365315 [details] [review]), it only applies to meson, so due to the changes on the source files it needs the `build: Remove autotools` (attachment 365312 [details] [review]) patch to be applied. I have also tried to extend it to autotools but failed :(.
Review of attachment 365873 [details] [review]: Ok. Let's do this one now.
Feel free to push them now. As we spoke on IRC, I would like to hold the autotools build machinery along meson until 3.28 because of some downstreams that don't have python3 or ninja yet.
Comment on attachment 365873 [details] [review] build: Rename Boxes desktop file Pushed as 24df159a - build: Rename Boxes desktop file
Comment on attachment 365314 [details] [review] meson: Use meson's i18n module Pushed as 1b3fb58a - meson: Use meson's i18n module
(In reply to Felipe Borges from comment #53) > Feel free to push them now. As we spoke on IRC, I would like to hold the > autotools build machinery along meson until 3.28 because of some downstreams > that don't have python3 or ninja yet. Sure! I've just pushed `build: Rename Boxes desktop file` and `meson: Use meson's i18n module` patches. I have double checked the files involved in the translations (`POTFILES.[in|skip]`) and the translations themself, which I sometimes forget :( `meson: Make oVirt support optional` is still missing, but it can be applied after 3.28.
Thanks a lot for all your contributions to the project, Iñigo!
-- 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/gnome-boxes/issues/176.