Skip to content
Open
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions easybuild/easyblocks/generic/mesonninja.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,15 @@

from easybuild.tools import LooseVersion
from easybuild.framework.easyblock import EasyBlock
from easybuild.framework.easyconfig import CUSTOM
from easybuild.framework.easyconfig import BUILD, CUSTOM
from easybuild.tools.build_log import EasyBuildError
from easybuild.tools.filetools import change_dir, create_unused_dir, which
from easybuild.tools.modules import get_software_version
from easybuild.tools.run import run_shell_cmd

DEFAULT_CONFIGURE_CMD = 'meson'
DEFAULT_BUILD_CMD = 'ninja'
DEFAULT_TEST_CMD = 'meson'
DEFAULT_INSTALL_CMD = 'ninja'


Expand All @@ -59,7 +60,9 @@ def extra_options(extra_vars=None):
"This disabled costly asserts in code, typical for production.", CUSTOM],
'configure_cmd': [DEFAULT_CONFIGURE_CMD, "Configure command to use", CUSTOM],
'install_cmd': [DEFAULT_INSTALL_CMD, "Install command to use", CUSTOM],
'runtest': [None, "Meson target to test build, or True to use 'meson test'", BUILD],
'separate_build_dir': [True, "Perform build in a separate directory", CUSTOM],
'test_cmd': [DEFAULT_TEST_CMD, "Test command to use ('runtest' value is appended)", CUSTOM],
})
return extra_vars

Expand Down Expand Up @@ -149,11 +152,32 @@ def build_step(self, *args, **kwargs):

def test_step(self):
"""
Run tests using Ninja.
Run tests using Meson.
"""
if self.cfg['runtest']:
cmd = "%s %s %s" % (self.cfg['pretestopts'], self.cfg['runtest'], self.cfg['testopts'])
test_cmd = self.cfg.get('test_cmd') or DEFAULT_TEST_CMD
runtest = self.cfg['runtest']

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Thyre We need to be a bit more careful here I think, to avoid introducing a backwards-incompatible change...

For an exiting easyconfig file like cairomm-1.16.2-GCC-12.3.0.eb which uses MesonNinja as easyblock and includes this currently:

runtest = 'ninja test'

this (and the logic below) means trouble, because we're essentially prepending "meson " to it, which results in:

== testing...
  >> running shell command:
        export MESON_TESTTHREADS=16 &&  export MESON_NUM_PROCESSES=16 &&  meson ninja test

Our easyconfig files are being updated so they work nicely with this updated MesonNinja easyblock in easybuilders/easybuild-easyconfigs#25749, but we need to take into account that we don't control all easyconfig files out there.

We should try and come up with an approach where existing easyconfigs that include runtest = 'ninja test' keep working as expected...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_cmd should only be set to the default when runtest is True.
For all other values, we should keep this empty.

This should keep the old behavior for existing external EasyConfigs.

if runtest or test_cmd != DEFAULT_TEST_CMD:
# Make run_test a string (empty if it is e.g. a boolean)
if not isinstance(runtest, str):
runtest = ''
# Run tests as recommended in https://mesonbuild.com/Unit-tests.html#testing-tool
if test_cmd == DEFAULT_TEST_CMD:
runtest = 'test'

# Make sure Meson does not use more resources than we want.
# From the documentation:
# By default Meson uses as many concurrent processes as there are cores on the test machine.
if self.cfg.parallel >= 1 and 'meson' in test_cmd:
if 'MESON_TESTTHREADS' not in self.cfg['pretestopts']:
self.cfg['pretestopts'] += f" export MESON_TESTTHREADS={self.cfg.parallel} && "
Comment thread
Thyre marked this conversation as resolved.
Outdated
# Preferred way to set parallelism since Meson v1.7.0, but does not hurt to set both.
if 'MESON_NUM_PROCESSES' not in self.cfg['pretestopts']:
self.cfg['pretestopts'] += f" export MESON_NUM_PROCESSES={self.cfg.parallel} && "
Comment thread
Thyre marked this conversation as resolved.
Outdated

# Compose command filtering out empty values
cmd = ' '.join([x for x in (self.cfg['pretestopts'], test_cmd, runtest, self.cfg['testopts']) if x])
res = run_shell_cmd(cmd)

return res.output

def install_step(self):
Expand Down