GNOME Bugzilla – Bug 785414
Port to meson build system
Last modified: 2018-01-18 11:52:46 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.
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.
Created attachment 356385 [details] [review] Port to meson build system Port to meson build system.
Sorry, I missed to add the link to the wip branch: https://git.gnome.org/browse/gnome-control-center/log/?h=wip/inigomartinez/meson
(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?
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
(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
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.
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
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.
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.
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.
(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.
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
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.
(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.
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.
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
Review of attachment 366886 [details] [review]: This looks good, and doesn't break anything.
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
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.
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.
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.
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.
Thank you very much for taking your time on testing this and fixing some of the remaining issues :)
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
Attachment 366994 [details] pushed as dc0988d - build: Remove autotools