Use Meson to build and install

[cjmayo]

I got so fed up with setup.py/distutils/setuptools in trying to get the installation paths right that I wondered what it would like to completely replace them with Meson. It seems the Python packaging tools are great if you are creating a Python package or even a command-line script but not so easy for a desktop application.

This isn't ready to merge. e.g. I think there may well be some things missing in the extension compilation but at least it does install an application that starts. The first commit removes the Hugo submodule, this is just to make my testing quicker, not a suggestion.

This was a good bit of learning for me, I'm happy with that for its own sake. I think it could be easier to read and maintain. Sharing in case there is any interest.

[dragonmux]

I whole-heartedly approve of the suggestion of Meson as it is an excellent tool and makes for much easier to read and maintain build systems in my experience.

I think you will find too the ability for it to use subprojects and wrap along with its excellent support for cross-compilation will be of enormous help, combined with its blazing fast reconfigure times that never require deleting the build directory.

[mindhells]

Looks cool!

Not sure if it has anything to do with this but I've got a question/remark:
By doing #65 I realized the following:

My initial idea was to remove the fract4d_stdlib extension from setup and fract4dc module but I found that in some environments (like travis runner), and because of how the dynamic linking works on them, this libs needs to be loaded as Global dlopen(fract4d_stdlib_path, RTLD_GLOBAL | RTLD_NOW).
There are some workarounds to solve that but I'm not sure which environments are affected. I can tell that you can just remove the extension on MacOS.
The reason for removing this fract4d_stdlib extension is its sources are already included in the fract4dc module.

To figure out the problem with the stdlib I had to reproduce locally the travis environment. For that I created a docker image. I've included an utility script using docker-compose to load that image into a container and pass the tests. It's under docker/bin/run_tests.sh.

So I guess the problem is not how the fract4d_stdlib is built but how it's loaded:

cmap_module_handle = dlopen(new_filename, RTLD_GLOBAL | RTLD_NOW);

By then I found a workaround but it involved to patch every fract4dc import with something like:

import DLFCN
sys.setdlopenflags(sys.getdlopenflags() | DLFCN.RTLD_GLOBAL)

So the question is: Is it possible somehow in the install process to tell python how that extension should be loaded afterwards?
Would it make sense? ... at least we could drop all that second extension and the cmap_name.h stuff.

[cjmayo]

I whole-heartedly approve of the suggestion of Meson as it is an excellent tool and makes for much easier to read and maintain build systems in my experience.

Well, that's plenty of encouragement! I think we will need a consensus for a change like this, but before that, as a minimum better get the tests passing.

[cjmayo]

Looks cool!

Thanks

Not sure if it has anything to do with this but I've got a question/remark:

As long it is not for me - I am just a (plain) Python person.

[edyoung]

I am not a big fan of setuptools for sure. Since we are an app rather than a python lib it has always been a bit awkward.

I do want to make sure it is as easy as possible for a user to try gnofract4d. So as long as the steps a user has to do don't get much worse, I'm ok.

(Ideally I would like to restart releasing binaries, maybe a snap or something built with pyoxidizer, but that is a problem for another PR)

I will try this out this weekend, rather busy at the moment. Thanks for looking into it Chris!

[cjmayo]

Added a bit more, I think that is about as far as I can take it. It does install and appears to run but pytest segfaults. Hopefully there is an easy (when you know how) compile or link fix for that.

meson _build
ninja -C _build
ninja -C _build test

Can run from source directory as now (version is set to "test"), or else:

ninja -C _build install

The new default strip and LTO options can be overridden by passing: -Dstrip=false and -Db_lto=false

[dragonmux]

meson _build
ninja -C _build
ninja -C _build test

If you've not used setuptools on a given clone, you don't need to stick an _ in the name 'build', and it doesn't necessarily need to be called build

Can run from source directory as now (version is set to "test"), or else:

ninja -C _build install

The new default strip and LTO options can be overridden by passing: -Dstrip=false and -Db_lto=false

You can change settings on the fly by, in your build directory, running meson configure (on its own) to get a listing of the build's current settings, then meson configure <options> to change them, such as saying meson configure -Dstrip=false

[dragonmux]

A couple of general notes about Meson usage:

It intelligently tracks what values are assigned to all available settings for a project and automatically reconfigures on changes to meson.build files it knows about, and config via meson configure, as part of running the chosen build tool.

You can also have multiple builds configured differently co-existing via running meson <dir> multiple times with different options following <dir>, so you could do CC=gcc CXX=g++ meson build-gcc and CC=clang CXX=clang++ meson build-clang and keep both builds going simultaneously.

[edyoung]

typo, THEREADS should be THREADS. (I can't remember what that define actually does, there is some chance it is obsolete)

[cjmayo]

Thanks, definitely not obsolete - that was what was causing pytest to crash!

[dragonmux]

As I'd like to try and take this forward and help you get to the finishing line, I have done a clone and build, however.. when I try running ninja test (or meson test -v to see what's going wrong), this step fails on account of:

ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: --cov=fract4d --cov=fract4dgui --cov=fract4d_compiler
  inifile: /data/Programming/Gnofract4d/meson/pytest.ini
  rootdir: /data/Programming/Gnofract4d/meson

1/1 testing FAIL           0.32s (exit status 4)

I run Arch, so this is clearly a change working its way towards production.. Python is 3.8.3, pytest is 5.4.3. I'm not familiar enough with pytest to debug this

[edyoung]

Maybe arch has pytest3 or pylint3 or something?

My concern with the meson change is that we are making it harder for someone to just download the source, build and run. Now they have to install ninja and meson (using pip3, the distro one probably isn't recent enough). Would be good to understand what the readme will look like. Perhaps we can keep a setup.py which simplifies the steps...

Another avenue of course is to come up with an easily-consumable binary package. If we can produce that it's less important how easy build-from-source is. I will try and find the time to see how much hassle it is to build a snap image.

[cjmayo]

As I'd like to try and take this forward and help you get to the finishing line, I have done a clone and build, however.. when I try running ninja test (or meson test -v to see what's going wrong), this step fails on account of:

ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: --cov=fract4d --cov=fract4dgui --cov=fract4d_compiler
  inifile: /data/Programming/Gnofract4d/meson/pytest.ini
  rootdir: /data/Programming/Gnofract4d/meson

1/1 testing FAIL           0.32s (exit status 4)

I run Arch, so this is clearly a change working its way towards production.. Python is 3.8.3, pytest is 5.4.3. I'm not familiar enough with pytest to debug this

https://pypi.org/project/pytest-cov/ missing by the look of it. (pytest 5.4.3 working fine here, although I see now I've broken the version tests).

[dragonmux]

@edyoung I think the same argument can be made about cmake as this is not a tool that comes out the box on most distros and even after Debian/Ubuntu's build-essential is installed.. however, I think you may find the argument for Meson's inbuilt packaging support to be compelling:

Meson supports destdir on install out the box which makes writing packaging descriptions considerably easier - for example: https://github.com/DX-MON/crunch/blob/master/package/PKGBUILD or in the case of Debian, the hilariously simple https://github.com/DX-MON/crunch/blob/master/package/debian/rules

So I think you'll find building an installable package this way very easy to do and low maintenance. You also get a dist archive out of the process (meson dist) that is ready for upload to a GitHub release.

[edyoung]

Sounds good. I would just like to see what instructions we need to put in README.md to cover the 'install source dist; build; run locally' path.

[cjmayo]

Had to rebase because of the removal of fract4d_stdlib, took advantage to do a few tidy ups and reduce the number of commits.

Needed to increase the timeout for the test target but now passes (a few new skips for the comparison of the version to the website and documentation).

So, still things to think about but we have got to the point of passing tests.

[cjmayo]

README change would look something like this:

--- a/README.md
+++ b/README.md
@@ -14,7 +14,8 @@ Basic Installation
 
 Run:
 
-    ./setup.py build
+    meson builddir
+    meson compile -C builddir
 
 You can then run Gnofract 4D in the local directory:
 
@@ -22,7 +23,7 @@ You can then run Gnofract 4D in the local directory:
 
 You can also install it:
 
-    sudo ./setup.py install
+    sudo meson install -C builddir
 
 You can then run it as:
 
@@ -44,19 +45,20 @@ On Ubuntu, these can be installed with:
     sudo apt install gir1.2-gtk-3.0 python3-gi-cairo python3-cairo
 
 To build from source you also need:
+- Meson and Ninja
 - headers for libpng and libjpeg
 - Python headers
 - pkg-config
 
 On Ubuntu, these can be installed with:
 
-    sudo apt install libjpeg-dev libpng-dev pkg-config python3-dev
+    sudo apt install libjpeg-dev libpng-dev meson pkg-config python3-dev
 
 If FFmpeg is installed it will be possible to create videos.
 
 On MacOS, you can install the dependencies using brew:
 
-    brew install librsvg python3 pkg-config cairo gtk+3 pygobject3 py3cairo libpng jpeg
+    brew install meson librsvg python3 pkg-config cairo gtk+3 pygobject3 py3cairo libpng jpeg

On Ubuntu meson pulls in ninja-build, I haven't tried MacOS although brew.sh does include meson which depends on ninja. The problem would be that Ubuntu 18.04 only has meson 0.45, so we might need a fallback to pip3 install meson, or I can see what we would lose by downgrading the requirement from 0.50.

[cjmayo]

I still think meson is better for development and maintenance but at the moment, without e.g. a Flatpak available, for the user pip seems easier to use. I don't suggest proceeding with this for now.

[mindhells]

I haven't had the time to try this myself, I had a glance at your comments and it looks very promising from the developer perspective. Nevertheless, I'm yet not familiar enough with Python packaging to give a useful insight when I do.

In this discussion they mention this module that might be interesting:

• https://thiblahute.gitlab.io/mesonpep517/

This is a python module that implements pep517 for the meson build system.

[dragonmux]

I'll see if I can find the time (probably Friday) to see where this is at against the current state of master as it ties in really nicely with #109.

Meson supports sanitisers out the box, allowing all the C++ code to be put through ASAN, TSAN, UBSAN etc by simply saying meson configure -Db_sanitize= followed by an appropriate sanitizer - eg, leak, or address,undefined. I think this will greatly help with the goal of #109 and is far cheaper than running valgrind and friends (as useful as they are)

[cjmayo]

I had rebased this on master but stopped because more work is needed on tox and the GitHub workflow. I've pushed it here now anyway in case it saves time. One change included is to avoid a dependency on pytest unless the test target is actually run, and even then drop pytest-cov.

[cjmayo]

Tox and checks working (not here though, maybe draft PRs are ignored), by going back to the way setup.py does things - building in the workflow and copying the fract4dc file instead of linking (pylint target doesn't like the link). Seems I was a bit optimistic that I had removed the pytest dependency from meson, maybe that target should just go.

Updated the codecov-action used, we aren't getting reports written to PRs, unfortunately not sure this fixes it.

[da2ce7]

Great work! Moving to Meson is a really great development! :)

[codecov]

Codecov Report

Attention: Patch coverage is 55.55556% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 85.70%. Comparing base (a94f8e7) to head (b3e0d7c).

Files	Patch %	Lines
fract4d/fractconfig.py	60.00%	2 Missing ⚠️
fract4d/options.py	50.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #133      +/-   ##
==========================================
- Coverage   85.74%   85.70%   -0.04%     
==========================================
  Files          55       55              
  Lines       10418    10424       +6     
==========================================
+ Hits         8933     8934       +1     
- Misses       1485     1490       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cjmayo]

Just a refresh to get it working with master again (GTK 4 was the least of the changes).

Would need more work I expect - and setup.py is still working!

[cjmayo]

I'm getting some pressure from Gentoo to not use setup.py any more. I've just copied this PR to it's own branch here on the project, which makes it look a bit more "official" (and it's a bit big to carry as a patch) - I think that will do for that now.

Not sure how much more would be needed here. Plus setup.py still seems easier for the ordinary user, while it still works.

[edyoung]

Do feel free to merge whenever you feel it's ready, btw. Unfortunately I don't have much time at the moment to do reviews

[cjmayo]

OK. Let's go for it. There are housekeeping changes that will be needed and will just create conflicts.

Rebased on master.