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 785414 - Port to meson build system
Port to meson build system
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: general
git master
Other Linux
: Normal enhancement
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
Depends on:
Blocks: 782980 787588
 
 
Reported: 2017-07-25 20:57 UTC by Iñigo Martínez
Modified: 2018-01-18 11:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Moved libgd and gvc modules (9.42 KB, patch)
2017-07-25 20:58 UTC, Iñigo Martínez
none Details | Review
Port to meson build system (459.27 KB, patch)
2017-07-25 20:59 UTC, Iñigo Martínez
none Details | Review
Move libgd and gvc modules (10.38 KB, patch)
2017-09-12 13:56 UTC, Iñigo Martínez
none Details | Review
Port to meson build system (453.06 KB, patch)
2017-09-12 14:41 UTC, Iñigo Martínez
none Details | Review
Remove autotools (149.21 KB, patch)
2017-09-12 14:42 UTC, Iñigo Martínez
none Details | Review
Port to meson build system (104.74 KB, patch)
2017-09-12 17:12 UTC, Iñigo Martínez
none Details | Review
Remove autotools (149.99 KB, patch)
2017-09-12 17:13 UTC, Iñigo Martínez
none Details | Review
Add update scripts replacements (8.79 KB, patch)
2017-09-13 12:19 UTC, Iñigo Martínez
none Details | Review
build: Move libgd and gvc modules (10.04 KB, patch)
2018-01-16 14:13 UTC, Iñigo Martínez
committed Details | Review
build: Port to meson build system (105.38 KB, patch)
2018-01-17 20:35 UTC, Iñigo Martínez
none Details | Review
build: Port to meson build system (105.57 KB, patch)
2018-01-17 22:10 UTC, Georges Basile Stavracas Neto
committed Details | Review
application: Initialize clutter-gtk either with Cheese or Wacom (1.49 KB, patch)
2018-01-17 22:10 UTC, Georges Basile Stavracas Neto
committed Details | Review
common: Don't unconditionally define HAVE_WAYLAND (870 bytes, patch)
2018-01-17 22:10 UTC, Georges Basile Stavracas Neto
committed Details | Review
build: Remove autotools (151.95 KB, patch)
2018-01-18 08:30 UTC, Iñigo Martínez
committed Details | Review

Description Iñigo Martínez 2017-07-25 20:57:33 UTC
This is a work in progress port to meson build system, just released for comments.

I have reviewed several times this patch, but being rather big, there are several details that need to be paid attention, so I will continue reviewing it in the following days. I also have to update it with the last changes that have been applied these last days.

Regarding the patch, there are two main points that still need some more work:

  - In the user accounts panel, in the gdbus code generation, 'annotation' option is used, which is not yet supported on meson's gnome module. I tried different workarounds, but I still haven't fixed it. At the moment, I've included the generated files along with the patch, but the idea is to fix this properly.
  - There are some git commands in the autotools files, which update gnome-settings-daemon. I suppose that they do share code. I still haven't done anything with this.
  
As I said, the patch is in a "work in progress" state, and any suggestion or comment would be very appreciated.

I have also created a wip branch[0] to ease it's testing.
Comment 1 Iñigo Martínez 2017-07-25 20:58:45 UTC
Created attachment 356384 [details] [review]
Moved libgd and gvc modules

This patch moves libgd and gvc modules to subprojects directory, which eases the transition to meson.
Comment 2 Iñigo Martínez 2017-07-25 20:59:20 UTC
Created attachment 356385 [details] [review]
Port to meson build system

Port to meson build system.
Comment 3 Iñigo Martínez 2017-07-25 21:00:22 UTC
Sorry, I missed to add the link to the wip branch: https://git.gnome.org/browse/gnome-control-center/log/?h=wip/inigomartinez/meson
Comment 4 Bastien Nocera 2017-08-11 09:38:08 UTC
(In reply to Iñigo Martínez from comment #1)
> Created attachment 356384 [details] [review] [review]
> Moved libgd and gvc modules
> 
> This patch moves libgd and gvc modules to subprojects directory, which eases
> the transition to meson.

Where are the patches for the meson ports for those?
Comment 5 Iñigo Martínez 2017-08-11 15:42:57 UTC
I don't know exactly what do you mean.

The attached patch[0] moves 'libgd' and 'gvc' from their location in the source code root, to the subprojects directory where they must reside. The patch also modifies autotools to support those libraries from those directories.

Then the meson port could make use of the same subprojects as autotools, but another change must be done though. The commit pointers of gvc and libgd are moved to the commits where meson is properly supported.

Are you referring to [0] and [1] or to [2] and [3]?

The meson port patch[1] also modifies the 
[0] https://bugzilla.gnome.org/attachment.cgi?id=356384
[1] https://bugzilla.gnome.org/attachment.cgi?id=356384
[2] https://git.gnome.org/browse/libgnome-volume-control/commit/?id=3093bdb0777db1b1431bede19373ae5eac98accd
[3] https://git.gnome.org/browse/libgd/commit/?id=cc90107531640bcba6c3c58e5cf6aec94d498763
Comment 6 Bastien Nocera 2017-08-14 09:31:44 UTC
(In reply to Iñigo Martínez from comment #5)
> Then the meson port could make use of the same subprojects as autotools, but
> another change must be done though. The commit pointers of gvc and libgd are
> moved to the commits where meson is properly supported.
> 
> Are you referring to [0] and [1] or to [2] and [3]?

Was to [2] and [3]. Thanks.

> The meson port patch[1] also modifies the 
> [0] https://bugzilla.gnome.org/attachment.cgi?id=356384
> [1] https://bugzilla.gnome.org/attachment.cgi?id=356384
> [2]
> https://git.gnome.org/browse/libgnome-volume-control/commit/
> ?id=3093bdb0777db1b1431bede19373ae5eac98accd
> [3]
> https://git.gnome.org/browse/libgd/commit/
> ?id=cc90107531640bcba6c3c58e5cf6aec94d498763
Comment 7 Iñigo Martínez 2017-09-12 13:56:33 UTC
Created attachment 359633 [details] [review]
Move libgd and gvc modules

The patch update fixes `the gvc/gvc-mixer-control.c` file, whose location wasn't updated after moving the gvc module location.

This bug URL has also been included in the commit message.
Comment 8 Iñigo Martínez 2017-09-12 14:41:11 UTC
Created attachment 359638 [details] [review]
Port to meson build system

This patch updates the meson port which is now fully working and also has been updated to changes up to '3.26.0'.

The `annotation` issue is fixed now, as meson has added support for it to the `gdbus_codegen` function[0]. Due to this issue, meson requirement has been raised to `0.43.0`, which, at the moment, is a development version. 

The meson port does not include those targets (`update-from-gsd` in `panels/common` and `panels/info`, and `update-from-nma` in `panels/network/wireless-security`) which are used to update shared code. I'm not sure if meson's build files should be the proper place for doing so, but although meson does not have support for that kind of functionality, if needed, some kind of workaround could be made. 

All the code has been uploaded to the wip branch[1].

Finally, as this is one of the biggest port I've done, I wanted to measure the improvement on building times, so I've done a "benchmark":

- autotools + make: time (./autogen.sh --prefix=/tmp/gcc --enable-debug=yes --enable-compile-warnings=yes --enable-documentation --enable-more-warnings --with-x --with-cheese; make install)

real	4m29,223s
user	3m59,784s
sys	0m33,556s

- meson + ninja: time (~/Development/meson/meson.py --prefix=/tmp/gcc --libdir=lib -Denable-documentation=true _build; ninja -C _build install)

real	1m34,726s
user	5m22,009s
sys	0m21,560s

[0] https://github.com/mesonbuild/meson/pull/2266
[1] https://git.gnome.org/browse/gnome-control-center/log/?h=wip/inigomartinez/meson
Comment 9 Iñigo Martínez 2017-09-12 14:42:41 UTC
Created attachment 359639 [details] [review]
Remove autotools

Some applications are removing autotools to avoid the burden of maintaining two build systems. This patch removes autotools in case it is considered.
Comment 10 Iñigo Martínez 2017-09-12 17:12:06 UTC
Created attachment 359647 [details] [review]
Port to meson build system

This patch update changes autotools files to include files added by the port to meson build system when distributable package is created.
Comment 11 Iñigo Martínez 2017-09-12 17:13:30 UTC
Created attachment 359648 [details] [review]
Remove autotools

This patch includes the modifications done by the last update of the port to meson build system patch.
Comment 12 Bastien Nocera 2017-09-12 18:12:27 UTC
(In reply to Iñigo Martínez from comment #8)
> The meson port does not include those targets (`update-from-gsd` in
> `panels/common` and `panels/info`, and `update-from-nma` in
> `panels/network/wireless-security`) which are used to update shared code.
> I'm not sure if meson's build files should be the proper place for doing so,
> but although meson does not have support for that kind of functionality, if
> needed, some kind of workaround could be made. 

Those need a replacement somehow, as they're really not something you'd want to do by hand.
Comment 13 Iñigo Martínez 2017-09-13 12:19:01 UTC
Created attachment 359703 [details] [review]
Add update scripts replacements

This patch adds replacement targets for those used to update shared code. The `update-from-gsd` target has been split into two:

- `update-info-from-gsd` which updates the "info" panel with code from `gnome-settings-daemon`.
- `update-commom-from-gsd` which updates the "common" code from `gnome-settings-daemon`.

I have been testing it here. The `update-info-from-gsd` target works and creates a new commit with new updates. The `update-common-from-gsd` target fails because there are no updates. Finally, `update-from-nma` target fails due to multiple collisions.

More testing and/or any new suggestion would be very appreciated.

I have updated the wip branch with this code[0].

[0] https://git.gnome.org/browse/gnome-control-center/log/?h=wip/inigomartinez/meson
Comment 14 Iñigo Martínez 2017-09-14 05:38:51 UTC
I forgot to say that meson port isn't installing the icons exactly where they were installed by autotools. This is because some icons seemed displaced from their places. For example:

- share/gnome-control-center/icons/hicolor/16x16/devices/audio-headset.svg
- share/gnome-control-center/icons/hicolor/24x24/devices/audio-headset.svg
- share/gnome-control-center/icons/hicolor/32x32/devices/audio-headset.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-center-back.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-center-back-testing.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-center.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-center-testing.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-left-back.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-left-back-testing.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-left-side.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-left-side-testing.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-left.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-left-testing.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-right-back.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-right-back-testing.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-right-side.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-right-side-testing.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-right.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-speaker-right-testing.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-subwoofer.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/audio-subwoofer-testing.svg
- share/icons/hicolor/16x16/apps/multimedia-volume-control.svg
- share/icons/hicolor/22x22/apps/multimedia-volume-control.svg
- share/icons/hicolor/32x32/apps/multimedia-volume-control.svg
- share/icons/hicolor/48x48/apps/devices/audio-headset.svg

The meson files has the following changes regarding some of those icons:

- share/icons/hicolor/48x48/apps/devices/audio-headset.svg -> share/gnome-control-center/icons/hicolor/48x48/devices/audio-headset.svg
- share/gnome-control-center/icons/hicolor/48x48/devices/* -> share/gnome-control-center/icons/hicolor/scalable/devices/*

Please do not hesitate to tell me what needs to be fixed so that I can fix it as soon as possible.
Comment 15 Bastien Nocera 2018-01-15 13:19:42 UTC
(In reply to Iñigo Martínez from comment #13)
> Created attachment 359703 [details] [review] [review]
> Add update scripts replacements
> 
> This patch adds replacement targets for those used to update shared code.
> The `update-from-gsd` target has been split into two:
> 
> - `update-info-from-gsd` which updates the "info" panel with code from
> `gnome-settings-daemon`.
> - `update-commom-from-gsd` which updates the "common" code from
> `gnome-settings-daemon`.
> 
> I have been testing it here. The `update-info-from-gsd` target works and
> creates a new commit with new updates. The `update-common-from-gsd` target
> fails because there are no updates. Finally, `update-from-nma` target fails
> due to multiple collisions.
> 
> More testing and/or any new suggestion would be very appreciated.

Can you please merge those 3 targets into one, but disable the nma one for now? I'll update it once you've rebased this patch.
Comment 16 Iñigo Martínez 2018-01-16 14:13:15 UTC
Created attachment 366886 [details] [review]
build: Move libgd and gvc modules

An update to the patch that moves libgd and gvc modules to subprojects directory, which eases the transition to meson.
Comment 17 Iñigo Martínez 2018-01-17 20:35:17 UTC
Created attachment 366969 [details] [review]
build: Port to meson build system

Here goes and update to the meson build port. I have been reviewing the whole meson port and I have made some improvements.

There is still work to be done, but I think that it's now in a good shape.

I have also updated the wip branch to ease its testing[0].

[0] https://git.gnome.org/browse/gnome-control-center/log/?h=wip/inigomartinez/meson
Comment 18 Georges Basile Stavracas Neto 2018-01-17 21:07:25 UTC
Review of attachment 366886 [details] [review]:

This looks good, and doesn't break anything.
Comment 19 Georges Basile Stavracas Neto 2018-01-17 22:09:13 UTC
Review of attachment 366969 [details] [review]:

A few comments. Overall, looks pretty good.

::: meson.build
@@ +221,3 @@
+    dependency('gudev-1.0')
+  ]
+endif

Missing HAVE_WAYLAND in config_h

::: meson_options.txt
@@ +6,3 @@
+option('wayland', type: 'boolean', value: true, description: 'build with Wayland support')
+option('gnome_session_libexecdir', type: 'string', value: '', description: 'Directory for gnome-session\'s libexecdir')
+option('documentation', type: 'boolean', value: false, description: 'build documentation')

Should be sorted on alphabetic order

::: shell/meson.build
@@ +86,3 @@
+  shell_deps += cheese_deps
+endif
+

Missing a similar "if have_wacom..." check, for Wacom also requires clutter-gtk
Comment 20 Georges Basile Stavracas Neto 2018-01-17 22:10:35 UTC
Created attachment 366974 [details] [review]
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.
Comment 21 Georges Basile Stavracas Neto 2018-01-17 22:10:42 UTC
Created attachment 366975 [details] [review]
application: Initialize clutter-gtk either with Cheese or Wacom

Both require clutter-gtk, but only Wacom was being checked. This would
make compiling with Cheese disabled fail.
Comment 22 Georges Basile Stavracas Neto 2018-01-17 22:10:49 UTC
Created attachment 366976 [details] [review]
common: Don't unconditionally define HAVE_WAYLAND

This header unconditionally defines HAVE_WAYLAND when GTK
is built with Wayland support. This breaks the build when
building with Wayland disabled.
Comment 23 Georges Basile Stavracas Neto 2018-01-17 23:24:34 UTC
Attachment 366886 [details] pushed as d7012d0 - build: Move libgd and gvc modules to subprojects
Attachment 366974 [details] pushed as 32edd67 - build: Port to meson build system
Attachment 366975 [details] pushed as dc5e2fd - application: Initialize clutter-gtk either with Cheese or Wacom
Attachment 366976 [details] pushed as 62f07b2 - common: Don't unconditionally define HAVE_WAYLAND

Keeping this bug open until further testing is done and the Autotools files can be safely dropped.
Comment 24 Iñigo Martínez 2018-01-18 08:13:46 UTC
Thank you very much for taking your time on testing this and fixing some of the remaining issues :)
Comment 25 Iñigo Martínez 2018-01-18 08:30:46 UTC
Created attachment 366994 [details] [review]
build: Remove autotools

I have updated the patch to fully remove autotools as build system following the latest changes.

Please note that before applying this patch, jhbuild[0] and gnome-continuous ((it seems to be already updated[1] and also working[2]) should be updated.

[0] https://git.gnome.org/browse/jhbuild/tree/modulesets/gnome-suites-core-3.28.modules#n177
[1] https://git.gnome.org/browse/gnome-continuous/tree/manifest.json#n1108
[2] http://build.gnome.org/#/build/20180117.45
Comment 26 Bastien Nocera 2018-01-18 11:52:40 UTC
Attachment 366994 [details] pushed as dc0988d - build: Remove autotools