After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 793340 - Use meson best practices
Use meson best practices
Status: RESOLVED FIXED
Product: swell-foop
Classification: Applications
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: swell-foop-maint
swell-foop-maint
Depends on:
Blocks:
 
 
Reported: 2018-02-09 12:02 UTC by Robert Roth
Modified: 2018-03-18 19:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (6.13 KB, patch)
2018-02-09 12:02 UTC, Robert Roth
none Details | Review
meson: Change config.h handling (6.37 KB, patch)
2018-02-10 15:01 UTC, Iñigo Martínez
none Details | Review
build: Move config.vapi to vapi directory (2.86 KB, patch)
2018-02-10 15:03 UTC, Iñigo Martínez
none Details | Review
build: Add PACKAGE_URL global variable (1.96 KB, patch)
2018-02-10 15:04 UTC, Iñigo Martínez
none Details | Review
build: Move config.vapi to vapi directory (2.86 KB, patch)
2018-03-15 08:57 UTC, Iñigo Martínez
none Details | Review
build: Remove autotools (27.03 KB, patch)
2018-03-15 08:58 UTC, Iñigo Martínez
none Details | Review

Description Robert Roth 2018-02-09 12:02:45 UTC
Created attachment 368193 [details] [review]
Proposed patch

Apply meson best practices early next cycle, based on detailed review of Iñigo Martínez.
Comment 1 Iñigo Martínez 2018-02-10 15:01:49 UTC
Created attachment 368215 [details] [review]
meson: Change config.h handling

The `config.h` file handling, which is done through the `config.vapi` file, has been changed following Vala recommendations[0].

To do this, the global variables in the `config.vapi` file has been moved to the `Config` namespace.

The `DATADIR` and `LOCALEDIR` variables, although they still exist in the `config.vapi` file, they are not set in the `config.h` file because they will be set by the compiler's macro definitions when using compiler flags.

[0] https://wiki.gnome.org/Projects/Vala/GameDevelopmentSeries/Setup
Comment 2 Iñigo Martínez 2018-02-10 15:03:20 UTC
Created attachment 368216 [details] [review]
build: Move config.vapi to vapi directory

The standard place following Vala recommendations[0] for the `config.vapi` file is the `vapi` directory.

The `config.vapi` file has been moved to the `vapi` directory, and both meson and autotools have been modified to handle it properly.

This `vapi` directory is also used in case of of using external/experimental vapi libraries.

[0] https://wiki.gnome.org/Projects/Vala/GameDevelopmentSeries/Setup
Comment 3 Iñigo Martínez 2018-02-10 15:04:20 UTC
Created attachment 368217 [details] [review]
build: Add PACKAGE_URL global variable

The website value is inmutable and might be used in the whole source code. This value has been moved to the `PACKAGE_URL` global variable that has been usually used in autotools to hold this value.
Comment 4 Robert Roth 2018-03-15 01:34:34 UTC
@ Iñigo Martínez could you re-check all the patches in this issue?
The last two fail when applied in order, I have tried to apply them manually, but can't get everything to work as usual.

Even with the first two applied, the config vapi handling patch seems to break the autotools build, as config.h is not found.

Of course, the goal would be to have a good meson build, let's assume there's no autotools (as once we have a working and elegant meson build I'm dropping autotools).
Comment 5 Iñigo Martínez 2018-03-15 08:57:24 UTC
Created attachment 369713 [details] [review]
build: Move config.vapi to vapi directory

(In reply to Robert Roth from comment #4)
> @ Iñigo Martínez could you re-check all the patches in this issue?
> The last two fail when applied in order, I have tried to apply them
> manually, but can't get everything to work as usual.

Both should be applied after applying attachment 368193 [details] [review]. The only significant change I've noticed is, the version number change in `configure.ac`. Here goes an updated patch.

> Even with the first two applied, the config vapi handling patch seems to
> break the autotools build, as config.h is not found.

I have checked autotools building with every patch and it works here. `config.h` is also created properly in the root source directory:

  config.status: creating config.h

However, it's true that I don't know much about autotools, so I don't know what is going on here.

Just in case, I have created a wip branch[0] that I hope could help with this.

[0] https://git.gnome.org/browse/swell-foop/log/?h=wip/inigomartinez/meson
Comment 6 Iñigo Martínez 2018-03-15 08:58:54 UTC
Created attachment 369714 [details] [review]
build: Remove autotools

(In reply to Robert Roth from comment #4)
> Of course, the goal would be to have a good meson build, let's assume
> there's no autotools (as once we have a working and elegant meson build I'm
> dropping autotools).

Why not remove it then? ;)

Here goes a patch to remove autotools support to avoid the burden of maintaining multiple build systems.

The wip branch[0] has also been updated.

[0] https://git.gnome.org/browse/swell-foop/log/?h=wip/inigomartinez/meson
Comment 7 Robert Roth 2018-03-18 19:21:08 UTC
Thanks for all your contributions, I have reviewed them, first wanted to squash the branch changes during merge, but decided to have the history. This was pushed to master, marking as fixed.

This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.