ENH: use system ninja if adequate

PR #175

Co-authored-by: Filipe Laíns <[email protected]>

Author: henryiii

.flake8

@@ -1,4 +1,4 @@
 [flake8]
 max-line-length = 127
-max-complexity = 10
+max-complexity = 12
 extend-ignore = E203

mesonpy/__init__.py

@@ -64,14 +64,6 @@
 __version__ = '0.11.0.dev0'
 
 
-class _depstr:
-    """Namespace that holds the requirement strings for dependencies we *might*
-    need at runtime. Having them in one place makes it easier to update.
-    """
-    patchelf = 'patchelf >= 0.11.0'
-    wheel = 'wheel >= 0.36.0'  # noqa: F811
-
-
 _COLORS = {
     'cyan': '\33[36m',
     'yellow': '\33[93m',
@@ -82,6 +74,16 @@ class _depstr:
     'reset': '\33[0m',
 }
 _NO_COLORS = {color: '' for color in _COLORS}
+_NINJA_REQUIRED_VERSION = '1.8.2'
+
+
+class _depstr:
+    """Namespace that holds the requirement strings for dependencies we *might*
+    need at runtime. Having them in one place makes it easier to update.
+    """
+    patchelf = 'patchelf >= 0.11.0'
+    wheel = 'wheel >= 0.36.0'  # noqa: F811
+    ninja = f'ninja >= {_NINJA_REQUIRED_VERSION}'
 
 
 def _init_colors() -> Dict[str, str]:
@@ -565,6 +567,12 @@ def __init__(
         self._build_dir = pathlib.Path(build_dir).absolute() if build_dir else (self._working_dir / 'build')
         self._install_dir = self._working_dir / 'install'
         self._meson_native_file = self._source_dir / '.mesonpy-native-file.ini'
+        self._env = os.environ.copy()
+
+        # prepare environment
+        ninja_path = _env_ninja_command()
+        if ninja_path is not None:
+            self._env.setdefault('NINJA', str(ninja_path))
 
         # load config -- PEP 621 support is optional
         self._config = tomllib.loads(self._source_dir.joinpath('pyproject.toml').read_text())
@@ -637,7 +645,7 @@ def _get_config_key(self, key: str) -> Any:
     def _proc(self, *args: str) -> None:
         """Invoke a subprocess."""
         print('{cyan}{bold}+ {}{reset}'.format(' '.join(args), **_STYLES))
-        subprocess.check_call(list(args))
+        subprocess.check_call(list(args), env=self._env)
 
     def _meson(self, *args: str) -> None:
         """Invoke Meson."""
@@ -957,6 +965,37 @@ def _validate_string_collection(key: str) -> None:
         yield project
 
 
+def _env_ninja_command(*, version: str = _NINJA_REQUIRED_VERSION) -> Optional[pathlib.Path]:
+    """
+    Returns the path to ninja, or None if no ninja found.
+    """
+    required_version = tuple(int(v) for v in version.split('.'))
+    env_ninja = os.environ.get('NINJA', None)
+    ninja_candidates = [env_ninja] if env_ninja else ['ninja', 'ninja-build', 'samu']
+    for ninja in ninja_candidates:
+        ninja_path = shutil.which(ninja)
+        if ninja_path is None:
+            continue
+
+        result = subprocess.run([ninja_path, '--version'], check=False, text=True, capture_output=True)
+
+        try:
+            candidate_version = tuple(int(x) for x in result.stdout.split('.')[:3])
+        except ValueError:
+            continue
+        if candidate_version < required_version:
+            continue
+        return pathlib.Path(ninja_path)
+
+    return None
+
+
+def get_requires_for_build_sdist(
+    config_settings: Optional[Dict[str, str]] = None,
+) -> List[str]:
+    return [_depstr.ninja] if _env_ninja_command() is None else []
+
+
 def build_sdist(
     sdist_directory: str,
     config_settings: Optional[Dict[Any, Any]] = None,
@@ -972,12 +1011,24 @@ def get_requires_for_build_wheel(
     config_settings: Optional[Dict[str, str]] = None,
 ) -> List[str]:
     dependencies = [_depstr.wheel]
-    with _project(config_settings) as project:
-        if not project.is_pure and platform.system() == 'Linux':
-            # we may need patchelf
-            if not shutil.which('patchelf'):  # XXX: This is slightly dangerous.
-                # patchelf not already acessible on the system
+
+    if _env_ninja_command() is None:
+        dependencies.append(_depstr.ninja)
+
+    if sys.platform.startswith('linux'):
+        # we may need patchelf
+        if not shutil.which('patchelf'):
+            # patchelf not already accessible on the system
+            if _env_ninja_command() is not None:
+                # we have ninja available, so we can run Meson and check if the project needs patchelf
+                with _project(config_settings) as project:
+                    if not project.is_pure:
+                        dependencies.append(_depstr.patchelf)
+            else:
+                # we can't check if the project needs patchelf, so always add it
+                # XXX: wait for https://github.com/mesonbuild/meson/pull/10779
                 dependencies.append(_depstr.patchelf)
+
     return dependencies
 
 

pyproject.toml

@@ -3,7 +3,6 @@ build-backend = 'mesonpy'
 backend-path = ['.']
 requires = [
   'meson>=0.63.3',
-  'ninja',
   'pyproject-metadata>=0.5.0',
   'tomli>=1.0.0; python_version<"3.11"',
   'typing-extensions>=3.7.4; python_version<"3.8"',
@@ -27,7 +26,6 @@ classifiers = [
 dependencies = [
   'colorama; os_name == "nt"',
   'meson>=0.63.3',
-  'ninja',
   'pyproject-metadata>=0.5.0', # not a hard dependency, only needed for projects that use PEP 621 metadata
   'tomli>=1.0.0; python_version<"3.11"',
   'typing-extensions>=3.7.4; python_version<"3.8"',
@@ -47,6 +45,7 @@ test = [
   'Cython',
   'pyproject-metadata>=0.6.1',
   'wheel',
+  'ninja',
 ]
 docs = [
   'furo>=2021.08.31',

tests/test_pep517.py

@@ -1,6 +1,10 @@
 # SPDX-License-Identifier: MIT
 
-import platform
+import shutil
+import subprocess
+import sys
+
+from typing import List
 
 import pytest
 
@@ -9,28 +13,41 @@
 from .conftest import cd_package
 
 
-if platform.system() == 'Linux':
-    VENDORING_DEPS = {mesonpy._depstr.patchelf}
-else:
-    VENDORING_DEPS = set()
+@pytest.mark.parametrize('package', ['pure', 'library'])
+@pytest.mark.parametrize('system_patchelf', ['patchelf', None], ids=['patchelf', 'nopatchelf'])
+@pytest.mark.parametrize('ninja', [None, '1.8.1', '1.8.3'], ids=['noninja', 'oldninja', 'newninja'])
+def test_get_requires_for_build_wheel(monkeypatch, package, system_patchelf, ninja):
+    def which(prog: str) -> bool:
+        if prog == 'patchelf':
+            return system_patchelf
+        if prog == 'ninja':
+            return ninja and 'ninja'
+        if prog in ('ninja-build', 'samu'):
+            return None
+        # smoke check for the future if we add another usage
+        raise AssertionError(f'Called with {prog}, tests not expecting that usage')
+
+    def run(cmd: List[str], *args: object, **kwargs: object) -> subprocess.CompletedProcess:
+        if cmd != ['ninja', '--version']:
+            # smoke check for the future if we add another usage
+            raise AssertionError(f'Called with {cmd}, tests not expecting that usage')
+        return subprocess.CompletedProcess(cmd, 0, f'{ninja}\n', '')
+
+    monkeypatch.setattr(shutil, 'which', which)
+    monkeypatch.setattr(subprocess, 'run', run)
+
+    expected = {mesonpy._depstr.wheel}
 
+    ninja_available = ninja is not None and [int(x) for x in ninja.split('.')] >= [1, 8, 2]
 
-@pytest.mark.parametrize(
-    ('package', 'system_patchelf', 'expected'),
-    [
-        ('pure', True, set()),  # pure and system patchelf
-        ('library', True, set()),  # not pure and system patchelf
-        ('pure', False, set()),  # pure and no system patchelf
-        ('library', False, VENDORING_DEPS),  # not pure and no system patchelf
-    ]
-)
-def test_get_requires_for_build_wheel(mocker, package, expected, system_patchelf):
-    mock = mocker.patch('shutil.which', return_value=system_patchelf)
+    if not ninja_available:
+        expected |= {mesonpy._depstr.ninja}
 
-    if mock.called:  # sanity check for the future if we add another usage
-        mock.assert_called_once_with('patchelf')
+    if (
+        system_patchelf is None and sys.platform.startswith('linux')
+        and (not ninja_available or (ninja_available and package != 'pure'))
+    ):
+        expected |= {mesonpy._depstr.patchelf}
 
     with cd_package(package):
-        assert set(mesonpy.get_requires_for_build_wheel()) == expected | {
-            mesonpy._depstr.wheel,
-        }
+        assert set(mesonpy.get_requires_for_build_wheel()) == expected