su2code/SU2

Please make the use of system-wide Meson and Ninja easier

frx-wintermute opened this issue · 9 comments

Hello, I am new to SU2.

I cloned the git repository and wanted to build it from source (on Debian GNU/Linux).
I noticed that the 'meson.py' script downloads a bunch of external dependencies, including Meson and Ninja (as documented in https://su2code.github.io/docs_v7/Build-SU2-Linux-MacOS/). I wanted to use the system-wide versions of Meson and Ninja that I have installed on my Debian box, hence I had to modify the 'meson.build' file, in order to drop the following lines:

if meson.version() != '0.61.1'
  error('SU2 must be configured with the extended Meson script (./meson.py) in the SU2 root directory.')
endif

And then I had to manually download the relevant external dependencies.
After that, I issued:

$ meson setup -D_configuration-options_ build
$ ninja -C build install

The build was successful, but using system-wide Meson and Ninja could be made easier.

I think that the 'meson.py' script could have an option to avoid downloading Meson and Ninja and to avoid the automatic invocation of the downloaded Meson.

Moreover, the 'meson.build' file should not exit unsuccessfully on the basis of meson.version(), or, at least, should check that meson.version() is equal to or later than a given minimum version.

Thank you for the feedback but for 99% of cases it is easier for users if we download the dependencies for them. The philosophy behind SU2 has always been to provide as much of a self contained package as possible.

Yes, I understand that, on some OSes, self contained bundles (shipping build tools and external dependencies) are easier to handle.

What I am suggesting is not to completely disable this self contained approach: I am just saying that there should be an option to disable it, so that life could be made easier for both classes of users (those who benefit from the self contained approach and those who prefer using system-wide build tools).
Please note that a "mandatory" self contained approach makes SU2 less likely to be packaged for inclusion into distributions such as Debian, because it means that more modifications are needed to create a proper package.

If you have a solution that allows you to use system-wide via an option and self-contained by default, please open a pull request.
As long as this option warns about having to manage dependencies manually I'm fine with it.
If you know a clean way of detecting the system-wide ninja and bypassing the ninja dependency that would also be an improvement.

Thank you for the feedback but for 99% of cases it is easier for users if we download the dependencies for them. The philosophy behind SU2 has always been to provide as much of a self contained package as possible.

But this check will allow users to use external dependencies just fine, as long as they happen to use an external 0.61.1?

...

I am not entirely sure I understand the issue here. You want to make it easier for users by downloading the dependencies, so you make it harder if they went and got their own dependencies? If someone has gone to the effort of getting their own dependencies instead of using your documented meson.py, it would seem like that inherently means they are the 1% of use cases and you could probably just leave them to it.

Is the issue rather that only Meson 0.61.1 has been tested to work?

In part yes. Some old versions do not work.
In part no. I am trying to ensure people use our "extended meson.py" by at least checking that the version is the one we expect.

This is based on having answered 10's or maybe 100 questions on CFD online and here on GitHub about not being able to build the code because they were using the "wrong" meson.py.

As I said it has room for improvement both for experts and novices, it seems like either of you would be capable to help, implement it, open a PR, and we'll go from there. The limited time I have for this project is better spent on other things.

Your meson.build project() function currently doesn't specify a meson_version: '>=0.61.1' -- if you were to use that, meson would natively check what version of meson you're using, and error out if you used an incompatible version (i.e. an older one).

If you want to lock to an exact version, and not allow newer versions of Meson either, then you can just specify a version without a comparison operator.

$ git --no-pager diff --cached 
diff --git a/meson.build b/meson.build
index ca86992ea..b68a03c52 100644
--- a/meson.build
+++ b/meson.build
@@ -1,15 +1,12 @@
 project('SU2', 'c', 'cpp',
         version: '7.5.1 "Blackbird"',
+        meson_version: '0.61.1',
         license: 'LGPL2',
         default_options: ['buildtype=release',
                           'warning_level=0',
                           'c_std=c99',
                           'cpp_std=c++11'])
 
-if meson.version() != '0.61.1'
-  error('SU2 must be configured with the extended Meson script (./meson.py) in the SU2 root directory.')
-endif
-
 pymod = import('python')
 python = pymod.find_installation()
 

I used this patch, and then ran the following:

$ meson setup builddir --wipe
The Meson build system
Version: 1.0.1
Source dir: /tmp/SU2
Build dir: /tmp/SU2/builddir
Build type: native build

meson.build:1:0: ERROR: Meson version is 1.0.1 but project requires 0.61.1

Using the >= comparison, it configures fine, and downloads a git submodule for mutationpp/coolprop on demand. From a hasty look around, it seems like the other git submodules are used as externals/ directories, not meson subprojects. Another project I know uses e.g.

if not fs.exists('submoduledir/README.md')
    error('Missing the `submoduledir` submodule! Run `git submodule update --init` to fix this.')
endif

for each submodule before it gets processed by meson. This is less automatic than wrapping it in meson.py, but does provide pretty clear directions... the sticking point is if people download tarballs. You currently point people to github's autogenerated archives, which is NOT sufficient as it doesn't have submodules included. I would actually suggest using meson dist --include-subprojects --no-tests to produce your own tarballs and attach them as additional releases artifacts. Meson does include submodules (whether or not they are subprojects) automatically to the tarball it creates.

If any of this interests you I could provide a PR implementing it.

I have opened PR #1951