GNOME Bugzilla – Bug 790236
Various meson related improvements
Last modified: 2018-08-10 09:29:33 UTC
This bug includes various meson improvements.
Created attachment 363431 [details] [review] build: Rename build options Following the new meson porting guidelines[0], this patch renames the build options. The list of changes is as follows: - Remove the `enable` prefix from boolean options. - Remove the `with` prefix from string options. - The character separator from multi-word options has been changed to underscore (`_`). [0] https://wiki.gnome.org/Initiatives/GnomeGoals/MesonPorting
Created attachment 363433 [details] [review] build: Remove default warning level option dconf uses 1 as the project's default warning level. However, meson already sets this option by default. This patch removes the option from project's default options.
Created attachment 363434 [details] [review] build: Use meson 0.43.0 features meson 0.43.0 comes with a new function in the compiler's object called get_supported_arguments, which allows checking multiple optiones at once. This patch bumps meson's version number and also takes advantage of this new feature.
Created attachment 363435 [details] [review] build: Remove macro definition meson uses the compiler's HAVE_CONFIG_H macro definiton, inherited from autotools behaviours. However it is not necessary. This patch removes the use of the macro definition.
Created attachment 364068 [details] [review] build: Remove default options A slightly modified patch where it also removes debugoptimized build type.
Created attachment 364069 [details] [review] build: Use meson 0.43.0 features A follow up update due to the changes in the previous patch.
Created attachment 364130 [details] [review] build: Retrieve D-Bus and gio paths D-Bus' session bus services' paths and gio's module's paths are hardcoded. However, these directories can be retrieved by checking this information from their pkgconfig files. This patch retrieves the paths for their correspondant pkgconfig files and uses those paths as installation directories. The user can override this paths by using the provided options.
Comment on attachment 363431 [details] [review] build: Rename build options > >-if get_option('enable-man') >+if get_option('man') > xsltproc = find_program('xsltproc', required: false) > assert(xsltproc.found(), 'xsltproc is required for enable-man') > I think this might need to be changed now to like: assert(xsltproc.found(), 'xsltproc is required for enabling man')
Created attachment 370700 [details] [review] build: Rename build options (In reply to Brendan L from comment #8) > I think this might need to be changed now to like: > > assert(xsltproc.found(), 'xsltproc is required for enabling man') Right! I have updated the patch with a slightly modified message: 'xsltproc is required for man generation'. What do you think?
Created attachment 370701 [details] [review] build: Remove default options A minor update due to version number change that also affects this patch.
Created attachment 370703 [details] [review] build: Use get_supported_arguments helper I have updated the commit message (and description) as it better describes it. The version number bump also affected this patch.
Created attachment 370705 [details] [review] build: Retrieve D-Bus and gio paths from pkg-config files An update to the previous patch that follows the new approach by redefining some variables present in the `pkg-config` files that makes paths to be located under prefix.
Created attachment 370753 [details] [review] build: Retrieve D-Bus and gio paths from pkg-config files I noticed that `libdconfsettings` was using a hardcoded `gio_module_dir`. I have updated the patch to also reuse the `gio_module_dir` from the `gio-2.0.pc` file.
Created attachment 370754 [details] [review] build: Remove config.h definitions Although many files include the `config.h` file, no defitions are used at all. Due to this, all the definitions have been removed. However, the `config.h` file is still generated and included in many files, as it may be useful for future global definitions.
Created attachment 370755 [details] [review] build: Avoid building libraries twice dconf builts a number of internal static libraries which in some cases are duplicated. This duplication comes from autotools that used to built two libraries for each library, one with PIC enabled and the other one without it. This has been changed to build only one library for each library to be built, except `libdconf-common-hidden` which hides some symbols for the GIO module to be built.
Created attachment 370756 [details] [review] build: Fix internal dependencies meson is able to generate internal dependencies for handling built libraries. These internal dependencies depend on other dependencies as well, based on the includes exposed by their headers. This have been fixed by using proper internal dependencies for these libraries.
Created attachment 370767 [details] [review] build: Use get_supported_arguments helper I've extended the use of `get_supported_arguments` helper to `fvisibility` compiler option which is used when building `libdconf-common-hidden`.
Created attachment 370769 [details] [review] build: Fix internal dependencies The last update also affected this patch that has been updated accordingly.
Created attachment 370770 [details] [review] build: Remove unnecessary variables There are some defined variables that are not used. Some of these variables are also used as install directories, that are equal to default installation directories. These all unnecessary variables and explicit default installation directories have been removed.
Created attachment 370771 [details] [review] build: Use gnome's gtkdoc_html_dir function meson's `gtkdoc_html_dir` function returns the path where HTML files will be installed for a given module. Instead of hard coding the directory, this function is used to set documentations installation directory.
Created attachment 370773 [details] [review] build: Fix parameters order The install parameters should be the last parameters when calling a function. The `configuration` parameter has been moved leaving install parameters at the end.
Created attachment 370775 [details] [review] build: Fix pkg-config exec_prefix variable The `exec_prefix` variable in the pkg-config file should point to `prefix` but it is pointing to `libexecdir`. This has been updated to point to `${prefix}`.
Created attachment 370777 [details] [review] build: Remove duplicated warning parameter The `dconf` binary uses the `-w` compiler argument which is already added by meson by default. This duplicated parameter has been removed.
Created attachment 370778 [details] [review] build: Use completions dir from pkg-config file The `bash-completions` pkg-config file provides a variable with the location of the completion files. This variable is checked to set the installation directory of the completion file provided by dconf. As a side note, the variable in the `bash-completions` pkg-config file is not totally correct, because it provides a directory that is relative to prefix while it should be relative to datadir.
To ease the review of this patches and test them I've created a wip branch: https://git.gnome.org/browse/dconf/log/?h=wip/inigomartinez/meson
> > Right! I have updated the patch with a slightly modified message: 'xsltproc > is required for man generation'. > > What do you think? Looks fine to me. I tried your WIP branch and it builds fine for me. There is still this bug present though: https://bugzilla.gnome.org/show_bug.cgi?id=794446
Iñigo, would you mind opening a GitLab merge request for these changes? That’ll make them easier to review. Thanks. (There’s an open issue about fully migrating dconf to GitLab: https://gitlab.gnome.org/Infrastructure/GitLab/issues/309)
Sure. I have reviewed the commits present here and included a pair of new commits with minor / aesthetic changes: https://gitlab.gnome.org/GNOME/dconf/merge_requests/11 I'm closing this bug as obsolete.