GNOME Bugzilla – Bug 790486
Various meson related improvements
Last modified: 2017-11-27 09:29:15 UTC
This bug includes various meson improvements.
Created attachment 363900 [details] [review] build: Rename 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.
Created attachment 363901 [details] [review] build: Remove unnecessary dependency checks There are multiple deprecated checks, related to IPv6 and X window system, which are not necessary anymore. This patch removes those checks and also their meson options.
Created attachment 363902 [details] [review] build: Remove default warning level gnome-session uses 1 as the project's default warning level. However, meson already uses this level as the default warning level. This patch removes the warning level from project's default options.
Created attachment 363903 [details] [review] build: Remove unused defines meson generates the `config.h` file with multiple defines to be used as compile-time options, in the same way as autotools does. However, some of them are not used. This patch removes those unused defines.
Created attachment 363904 [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 options at once. This patch takes advantage of this new feature.
Created attachment 363905 [details] [review] build: Comment session selector and tracker checks The session selector support and session tracker support are next to each other. This patch comments where meson checks session selector and the session tracker support to avoid confussions.
Review of attachment 363900 [details] [review]: Looks good.
Review of attachment 363901 [details] [review]: There seems to be quite a lot of unnecessary line shuffling, especially in the top-level meson.build, which makes the patch hard to read. Could we cut down on those? I would also have renamed the options after removing the ipv6 and xtrans dependency, but that's just nitpicking. ::: tools/meson.build @@ -6,3 @@ deps = session_deps + [ - sm_dep, - ice_dep Why swap those 2 lines? @@ +44,2 @@ dependency('epoxy'), + dependency('gl'), Or this one. @@ +49,3 @@ programs += [['gnome-session-check-accelerated-gl-helper', deps, cflags, session_libexecdir]] +deps += gtk_dep Or this change?
Review of attachment 363902 [details] [review]: Sure.
Review of attachment 363903 [details] [review]: ::: meson.build @@ -219,3 @@ - output: 'config.h', - configuration: config_h -) Why move this? Unless it fixes a bug, I'd do it separately, if at all.
Review of attachment 363904 [details] [review]: ++
Review of attachment 363905 [details] [review]: Sure, though I would add a bit more information as to what those are, because it resembles the "n++; // increment n" type of comments ;) "Check for session selector GTK+ UI" and "Check for session tracking backend"?
Created attachment 363921 [details] [review] build: Remove unnecessary dependency checks (In reply to Bastien Nocera from comment #8) > Review of attachment 363901 [details] [review] [review]: > > There seems to be quite a lot of unnecessary line shuffling, especially in > the top-level meson.build, which makes the patch hard to read. Could we cut > down on those? I usually try to put the dependencies sorted because I think that they might help developers/maintainers. However, it is just an aesthetic change, so I have reverted it. > I would also have renamed the options after removing the ipv6 and xtrans > dependency, but that's just nitpicking. I had also my doubts about this but I changed the order :(
Created attachment 363922 [details] [review] build: Remove unused defines (In reply to Bastien Nocera from comment #10) > Review of attachment 363903 [details] [review] [review]: > > ::: meson.build > @@ -219,3 @@ > - output: 'config.h', > - configuration: config_h > -) > > Why move this? Unless it fixes a bug, I'd do it separately, if at all. I have moved it back to its place.
Created attachment 363923 [details] [review] build: Comment session selector and tracker checks (In reply to Bastien Nocera from comment #12) > Review of attachment 363905 [details] [review] [review]: > > Sure, though I would add a bit more information as to what those are, > because it resembles the "n++; // increment n" type of comments ;) > > "Check for session selector GTK+ UI" and "Check for session tracking > backend"? Agree, it sounds better. Changed.
Review of attachment 363921 [details] [review]: Looks good.
Review of attachment 363922 [details] [review]: Sure.
Review of attachment 363923 [details] [review]: Sure.
Comment on attachment 363900 [details] [review] build: Rename build options Pushed as 9c02bffc - build: Rename build options
Comment on attachment 363921 [details] [review] build: Remove unnecessary dependency checks Pushed as 4ccee7b1 - build: Remove unnecessary dependency checks
Comment on attachment 363902 [details] [review] build: Remove default warning level Pushed as 67b0ee9f - build: Remove default warning level
Comment on attachment 363922 [details] [review] build: Remove unused defines Pushed as 6c39b0e8 - build: Remove unused defines
Comment on attachment 363904 [details] [review] build: Use meson 0.43.0 features Pushed as b059c286 - build: Use meson 0.43.0 features
Comment on attachment 363923 [details] [review] build: Comment session selector and tracker checks Pushed as 8948157b - build: Comment session selector and tracker checks
Created attachment 363966 [details] [review] build: Restore options default values During the migration to meson build system, the default value of different options were changed. The options are the following: - session-tracking: systemd (without consolekit support by default) - docbook: true - man: true This patch restores those default values.
Review of attachment 363966 [details] [review]: ++
Comment on attachment 363966 [details] [review] build: Restore options default values Pushed as f62989f1 - build: Restore options default values