GNOME Bugzilla – Bug 792948
Various meson related improvements
Last modified: 2018-01-30 12:55:07 UTC
This bug includes various meson improvements.
Created attachment 367503 [details] [review] build: Use meson's options for library type meson's `default_library` can be used to know the type of library to be created, so there is no need to have another option for it.
Created attachment 367504 [details] [review] build: Rename build options Following the meson porting guidelines, this patch renames the build options. The list of changes is as follows: - Remove the with prefix from string options. - The character separator from multi-word options has been changed to underscore. It also changes the introspection meson variable to be consistent with the one used for alsa.
Created attachment 367505 [details] [review] build: Remove dependencies' variables A set of different variables are used to hold dependencies. However, no individual use of them is done. This patch removes these variables and holds their objects directly in the array of dependencies.
Created attachment 367506 [details] [review] build: Do not use headers on library building Headers are not necessary to be passed to the library compilation function because the compiler will found them. In the other hand they are necessary for the proper GIR generation. This patch splits headers and sources, uses only sources for the library building and uses both for GIR generation. It also allows getting both separately.
Created attachment 367507 [details] [review] build: Use meson's library function meson features a `library` function which based on the `default_library`, already used, builds a shared or a static library. This patch takes advantage of this function, there is no need to conditionally split the library creation.
Created attachment 367508 [details] [review] build: Make use of assert function meson has support for `assert` function, which halts meson's execution showing a given message when a given condition is false. This patch takes advantage of this function to slightly improve meson's build file readibility. It also removes a duplicated check for `pkglibdir` being set when introspection is also set, because this check is already done when a shared library is being created, that is a precondition for introspection generation.
Created attachment 367509 [details] [review] Remove the include directory variable The variable which holds the current directory is not necessary because this is already included when building the library. However, it might be interessant for any package using the library to include the directory where headers are present, so the current directory is appended to the library dependency without the include directory variable.
Created attachment 367510 [details] [review] build: Remove config.h template file The `config.h` can be generated without any template. This patch removes the template file and modifies the build file to not make any use of it. It also removes the variable which holds the generated configuration file target, as it will not be necessary for any packages building libgnome-volume-control.
For ease the testing of all the patches, I have created a wip branch: https://git.gnome.org/browse/libgnome-volume-control/log/?h=wip/inigomartinez/meson There is also a weird behaviour with meson's options to choose between `shared` and `static` library. Please take a look at: https://github.com/mesonbuild/meson/issues/2910
Review of attachment 367503 [details] [review]: Sure, will need a gnome-shell patch to go with it.
Review of attachment 367504 [details] [review]: Sure, will also need a gnome-shell change.
Review of attachment 367505 [details] [review]: Sure
Review of attachment 367506 [details] [review]: > function because the compiler will found them. In the other hand "find" and "On the other hand". Looks good otherwise.
Review of attachment 367507 [details] [review]: Looks good
Review of attachment 367508 [details] [review]: Looks good
Review of attachment 367509 [details] [review]: Looks fine
Review of attachment 367510 [details] [review]: Sure.
(In reply to Iñigo Martínez from comment #1) > Created attachment 367503 [details] [review] [review] > build: Use meson's options for library type > > meson's `default_library` can be used to know the type of library to be > created, so there is no need to have another option for it. I have made a terrible mistake with this one. Only user options can be set when using any project as subproject[0], so this patch actually doesn't work. Seems that I'm not the only one with the same confusion[1] :(. 2 patches are not necessary anymore, 4 have been updated and 2 still remain the same. [0] http://mesonbuild.com/Reference-manual.html#subproject [1] https://github.com/mesonbuild/meson/issues/2612
Created attachment 367589 [details] [review] build: Rename build options Updated patch.
Created attachment 367590 [details] [review] build: Do not use headers on library building Updated patch.
Created attachment 367591 [details] [review] build: Make use of assert function Updated patch.
Created attachment 367592 [details] [review] build: Remove the include directory variable Updated patch.
Review of attachment 367589 [details] [review]: Sure, will need gnome-shell changes.
Review of attachment 367590 [details] [review]: Sure.
Review of attachment 367591 [details] [review]: Fine.
Review of attachment 367592 [details] [review]: Yep.
attachment 367510 [details] [review] pushed as 6a0ac9b - build: Remove config.h template file attachment 367592 [details] [review] pushed as ef5d07f - build: Remove the include directory variable attachment 367591 [details] [review] pushed as 69eac7d - build: Make use of assert function attachment 367590 [details] [review] pushed as f764bb6 - build: Do not use headers on library building attachment 367505 [details] [review] pushed as 68b7fcc - build: Remove dependencies' variables attachment 367589 [details] [review] pushed as b4854a8 - build: Rename build options
(In reply to Bastien Nocera from comment #23) > Review of attachment 367589 [details] [review] [review]: > > Sure, will need gnome-shell changes. Just pushed a merge request: https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/13 I will follow its merging there so I'll close this one. Thank you!