After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 791421 - Various build related improvements
Various build related improvements
Status: RESOLVED OBSOLETE
Product: gnome-boxes
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: --
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2017-12-09 20:26 UTC by Iñigo Martínez
Modified: 2018-01-11 11:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
meson: Use LINGUAS file for help generation (1.82 KB, patch)
2017-12-09 20:34 UTC, Iñigo Martínez
committed Details | Review
meson: Change build options (3.62 KB, patch)
2017-12-09 20:35 UTC, Iñigo Martínez
none Details | Review
meson: Add support for building libcommon (4.23 KB, patch)
2017-12-09 20:35 UTC, Iñigo Martínez
none Details | Review
meson: Add support for search provider (2.12 KB, patch)
2017-12-09 20:36 UTC, Iñigo Martínez
committed Details | Review
meson: Fix post install script (2.36 KB, patch)
2017-12-09 20:38 UTC, Iñigo Martínez
committed Details | Review
meson: Change build options (3.62 KB, patch)
2017-12-10 11:40 UTC, Iñigo Martínez
committed Details | Review
meson: Add support for building libcommon (4.24 KB, patch)
2017-12-10 11:42 UTC, Iñigo Martínez
committed Details | Review
meson: Support Installed tests (2.80 KB, patch)
2017-12-10 11:43 UTC, Iñigo Martínez
committed Details | Review
build: Remove autotools (41.37 KB, patch)
2017-12-10 11:51 UTC, Iñigo Martínez
reviewed Details | Review
meson: Rename Boxes desktop file (2.47 KB, patch)
2017-12-10 11:52 UTC, Iñigo Martínez
none Details | Review
meson: Use meson's i18n module (2.47 KB, patch)
2017-12-10 11:53 UTC, Iñigo Martínez
committed Details | Review
meson: Make oVirt support optional (4.88 KB, patch)
2017-12-10 11:54 UTC, Iñigo Martínez
accepted-commit_now Details | Review
meson: Remove gresources vala argument (1.10 KB, patch)
2017-12-10 12:54 UTC, Iñigo Martínez
committed Details | Review
meson: Use localedir option (1.46 KB, patch)
2017-12-10 13:02 UTC, Iñigo Martínez
none Details | Review
meson: Use localedir option (1.09 KB, patch)
2017-12-18 14:29 UTC, Iñigo Martínez
committed Details | Review
build: Rename Boxes desktop file (2.90 KB, patch)
2017-12-22 08:12 UTC, Iñigo Martínez
committed Details | Review
meson: Use meson's convention (4.86 KB, patch)
2017-12-22 08:58 UTC, Iñigo Martínez
committed Details | Review
Use glib's version as target-glib (1.44 KB, patch)
2017-12-22 08:59 UTC, Iñigo Martínez
committed Details | Review
build: Fix search provider ini file (3.79 KB, patch)
2018-01-02 10:39 UTC, Iñigo Martínez
committed Details | Review
meson: Look for spice-gtk vala bindings (780 bytes, patch)
2018-01-06 16:44 UTC, Pavel Grunt
committed Details | Review

Description Iñigo Martínez 2017-12-09 20:26:30 UTC
This bug includes various meson improvements.
Comment 1 Iñigo Martínez 2017-12-09 20:34:03 UTC
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.
Comment 2 Iñigo Martínez 2017-12-09 20:35:14 UTC
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.
Comment 3 Iñigo Martínez 2017-12-09 20:35:58 UTC
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.
Comment 4 Iñigo Martínez 2017-12-09 20:36:54 UTC
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.
Comment 5 Iñigo Martínez 2017-12-09 20:38:48 UTC
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.
Comment 6 Iñigo Martínez 2017-12-10 11:40:18 UTC
Created attachment 365309 [details] [review]
meson: Change build options

A minor change to follow the "code conventions" used in the build files.
Comment 7 Iñigo Martínez 2017-12-10 11:42:38 UTC
Created attachment 365310 [details] [review]
meson: Add support for building libcommon

A minor change to follow the "code conventions" used in the build files.
Comment 8 Iñigo Martínez 2017-12-10 11:43:38 UTC
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.
Comment 9 Iñigo Martínez 2017-12-10 11:51:22 UTC
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
Comment 10 Iñigo Martínez 2017-12-10 11:52:24 UTC
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.
Comment 11 Iñigo Martínez 2017-12-10 11:53:25 UTC
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.
Comment 12 Iñigo Martínez 2017-12-10 11:54:18 UTC
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.
Comment 13 Iñigo Martínez 2017-12-10 12:54:18 UTC
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.
Comment 14 Iñigo Martínez 2017-12-10 13:02:57 UTC
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.
Comment 15 Iñigo Martínez 2017-12-10 13:13:30 UTC
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
Comment 16 Iñigo Martínez 2017-12-18 14:29:30 UTC
Created attachment 365697 [details] [review]
meson: Use localedir option

The "meson: Use localedir option" was not properly tested, so here goes an update.
Comment 17 Felipe Borges 2017-12-19 10:41:35 UTC
Review of attachment 365295 [details] [review]:

lgtm
Comment 18 Felipe Borges 2017-12-19 10:42:23 UTC
Review of attachment 365298 [details] [review]:

sure!
Comment 19 Felipe Borges 2017-12-19 10:43:40 UTC
Review of attachment 365299 [details] [review]:

sure!
Comment 20 Felipe Borges 2017-12-19 10:44:35 UTC
Review of attachment 365309 [details] [review]:

Thanks! I didn't know about these patterns.
Comment 21 Iñigo Martínez 2017-12-19 14:26:49 UTC
Comment on attachment 365295 [details] [review]
meson: Use LINGUAS file for help generation

Pushed as b23671c4 - meson: Use LINGUAS file for help generation
Comment 22 Iñigo Martínez 2017-12-19 14:28:07 UTC
Comment on attachment 365309 [details] [review]
meson: Change build options

Pushed as 69d5f433 - meson: Change build options
Comment 23 Iñigo Martínez 2017-12-19 14:30:05 UTC
Comment on attachment 365299 [details] [review]
meson: Fix post install script

Pushed as f90438d5 - meson: Fix post install script
Comment 24 Felipe Borges 2017-12-21 14:32:43 UTC
Review of attachment 365310 [details] [review]:

Thanks!
Comment 25 Felipe Borges 2017-12-21 14:33:26 UTC
Review of attachment 365311 [details] [review]:

sure!
Comment 26 Felipe Borges 2017-12-21 14:34:05 UTC
Review of attachment 365314 [details] [review]:

lgtm
Comment 27 Felipe Borges 2017-12-21 14:34:46 UTC
Review of attachment 365315 [details] [review]:

sure!
Comment 28 Felipe Borges 2017-12-21 14:36:50 UTC
Review of attachment 365321 [details] [review]:

sure!
Comment 29 Felipe Borges 2017-12-21 14:37:15 UTC
Review of attachment 365697 [details] [review]:

sure!
Comment 30 Felipe Borges 2017-12-21 14:40:18 UTC
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...
Comment 31 Felipe Borges 2017-12-21 14:40:46 UTC
Thanks a lot Iñigo for your contributions!
Comment 32 Iñigo Martínez 2017-12-21 20:16:01 UTC
Comment on attachment 365310 [details] [review]
meson: Add support for building libcommon

Pushed as 0429f0a8 - meson: Add support for building libcommon
Comment 33 Iñigo Martínez 2017-12-21 20:17:19 UTC
Comment on attachment 365298 [details] [review]
meson: Add support for search provider

Pushed as b8a452ec - meson: Add support for search provider
Comment 34 Iñigo Martínez 2017-12-21 20:20:34 UTC
Comment on attachment 365311 [details] [review]
meson: Support Installed tests

Pushed as 8c948eaf - meson: Support Installed tests
Comment 35 Iñigo Martínez 2017-12-21 20:22:56 UTC
Comment on attachment 365321 [details] [review]
meson: Remove gresources vala argument

Pushed as 7a496d8e - meson: Remove gresources vala argument
Comment 36 Iñigo Martínez 2017-12-21 20:24:38 UTC
Comment on attachment 365697 [details] [review]
meson: Use localedir option

Pushed as e4d4d136 - meson: Use localedir option
Comment 37 Iñigo Martínez 2017-12-22 08:12:37 UTC
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.
Comment 38 Iñigo Martínez 2017-12-22 08:58:47 UTC
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.
Comment 39 Iñigo Martínez 2017-12-22 08:59:42 UTC
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.
Comment 40 Felipe Borges 2018-01-02 08:45:56 UTC
Review of attachment 365876 [details] [review]:

sure!
Comment 41 Felipe Borges 2018-01-02 08:46:25 UTC
Review of attachment 365877 [details] [review]:

ok.
Comment 42 Iñigo Martínez 2018-01-02 08:59:10 UTC
Comment on attachment 365876 [details] [review]
meson: Use meson's convention

Pushed as db01f16c - meson: Use meson's convention
Comment 43 Iñigo Martínez 2018-01-02 09:00:11 UTC
Comment on attachment 365877 [details] [review]
Use glib's version as target-glib

Pushed as e3caa8ea - meson: Use glib's version as target glib
Comment 44 Iñigo Martínez 2018-01-02 10:39:19 UTC
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.
Comment 45 Felipe Borges 2018-01-04 12:01:31 UTC
Review of attachment 366183 [details] [review]:

sure. Thanks for your patches!
Comment 46 Iñigo Martínez 2018-01-04 13:59:06 UTC
Comment on attachment 366183 [details] [review]
build: Fix search provider ini file

Pushed as 33d3c878 - meson: Fix search provider ini file
Comment 47 Pavel Grunt 2018-01-06 16:44:23 UTC
Created attachment 366425 [details] [review]
meson: Look for spice-gtk vala bindings
Comment 48 Felipe Borges 2018-01-08 09:28:07 UTC
Review of attachment 366425 [details] [review]:

Thank you!
Comment 49 Felipe Borges 2018-01-08 09:33:40 UTC
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
Comment 50 Piotr Drąg 2018-01-08 14:42:11 UTC
(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?
Comment 51 Iñigo Martínez 2018-01-09 09:40:52 UTC
(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 :(.
Comment 52 Felipe Borges 2018-01-09 12:40:14 UTC
Review of attachment 365873 [details] [review]:

Ok. Let's do this one now.
Comment 53 Felipe Borges 2018-01-09 12:41:34 UTC
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 54 Iñigo Martínez 2018-01-09 14:02:01 UTC
Comment on attachment 365873 [details] [review]
build: Rename Boxes desktop file

Pushed as 24df159a - build: Rename Boxes desktop file
Comment 55 Iñigo Martínez 2018-01-09 14:03:11 UTC
Comment on attachment 365314 [details] [review]
meson: Use meson's i18n module

Pushed as 1b3fb58a - meson: Use meson's i18n module
Comment 56 Iñigo Martínez 2018-01-09 14:08:25 UTC
(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.
Comment 57 Felipe Borges 2018-01-09 14:14:00 UTC
Thanks a lot for all your contributions to the project, Iñigo!
Comment 58 GNOME Infrastructure Team 2018-01-11 11:04:13 UTC
-- 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.