Created attachment 363534 [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.
- The character separator from multi-word options has been changed to underscore.

[0] https://wiki.gnome.org/Initiatives/GnomeGoals/MesonPorting

Created attachment 363535 [details] [review]
build: Remove default warning level

gnome-disk-utility 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 363536 [details] [review]
build: Remove unused defines

meson generates the config.h file with multiple defines to be used as guards, in the same way as autotools does. However, almost all of them are not used.

This patch removes the unused defines.

Created attachment 363537 [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 363538 [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 363539 [details] [review]
build: Use datadir on post install script

The previous approach used a fixed path for datadir. However, its location might change.

This patch uses the provided datadir path, if the user provides one, or the default path.

Created attachment 363540 [details] [review]
build: Move the icons files

The icons' files locations are not exactly in their final installation location.

This patch creates the proper directory tree needed on the installation process so they do not have to be copied one by one to the final destination.

Created attachment 363584 [details] [review]
build: Document custom build switches

Hello,

thanks a lot for caring about the state of the meson config.
For me it works well in two different scenarios.

But I realized that it is hard for new meson users to find out how the options are to be set. Passing e.g. -D enable-foo=true instead of -D foo=true is just ignored without warning. The meson configure command is not documented in meson --help and so I made some writeup in the INSTALL file and printed how to use the switches after configuration.

What do you think about that?
Regards,
Kai

Created attachment 363616 [details] [review]
build: Add a configure script

(In reply to Kai Lüke from comment #8)
> Created attachment 363584 [details] [review] [review]
> thanks a lot for caring about the state of the meson config.
> For me it works well in two different scenarios.

Thank you for taking time for testing it :)

> But I realized that it is hard for new meson users to find out how the
> options are to be set. Passing e.g. -D enable-foo=true instead of -D
> foo=true is just ignored without warning. The meson configure command is not
> documented in meson --help and so I made some writeup in the INSTALL file
> and printed how to use the switches after configuration.
> 
> What do you think about that?

Fair enough. You are totally right, despite the fact that it's enough for me, meson's documentation should improve. Although meson's developers are working hard including new features, the documentation remains one step behind. There has also been a discussion in desktop-devel-list about this issue[0].

I'm convinced that, once meson's use is extended, this issue will be adressed.

Your patch looks good to me. I'm also including a `configure` script, which `simulates` a configure script from autotools. It could be removed once meson's documentation improves.

This patch also is useful for gnome-continuous support[1].

[0] https://mail.gnome.org/archives/desktop-devel-list/2017-July/msg00000.html
[1] https://git.gnome.org/browse/gnome-continuous/tree/patches

Review of attachment 363616 [details] [review]:

Is it really a good idea to support two ways of building? I guess it can cause confusion in a GNOME Builder project on which file to select when opening the source directory.

And also because the ./configure script does not exactly behave the same since it can't be run twice.

Maybe it's better to file it for gnome-continuous directly and leave it away?

::: configure
@@ +6,3 @@
+# Copyright 2017 Iñigo Martínez <inigomartinez@gmail.com>
+# Licensed under the new-BSD license (http://www.opensource.org/licenses/bsd-license.php)
+

In general I would add as many safeguards as possible from here:
https://coderwall.com/p/fkfaqq/safer-bash-scripts-with-set-euxo-pipefail

E.g. a "set -e" here captures missing write permissions for "mkdir -p" but this is not a big deal since meson will also fail later.

(In reply to Kai Lüke from comment #10)
> Review of attachment 363616 [details] [review] [review]:
> Maybe it's better to file it for gnome-continuous directly and leave it away?

I agree that it may cause confussion. It would help to gnome-continuous, though it's going to support meson at some point[0].

> In general I would add as many safeguards as possible from here:
> https://coderwall.com/p/fkfaqq/safer-bash-scripts-with-set-euxo-pipefail
> 
> E.g. a "set -e" here captures missing write permissions for "mkdir -p" but
> this is not a big deal since meson will also fail later.

The configure script is just a workaround, and I'm not a bash script expert either, so it probably has some pitfals. However the hints in the link are nice. I used before `-e` and `-x`, but I wasn't aware of the other two. Thanks!

[0] https://bugzilla.gnome.org/show_bug.cgi?id=789146#c3

I just wasn't sure, did not want to turn down the last patch ;) We could ask if someone has experience if it's good to have such a wrapper script (And place a hint somewhere for usage with Builder).
But the script should probably make use of "meson configure" if the build directory is present already (but I don't know if this works when the meson version has changed).
If you want to work on it for end user experience we can reopen this bug.
Concerning continuous I need to figure out what the current state is for submitting the patch.

Thanks a lot and many greetings,
Kai