ENH: use system ninja if adequate

Signed-off-by: Henry Schreiner <[email protected]>

Author: henryiii

mesonpy/__init__.py

@@ -69,6 +69,7 @@ class _depstr:
     """
     patchelf = 'patchelf >= 0.11.0'
     wheel = 'wheel >= 0.36.0'  # noqa: F811
+    ninja = 'ninja >= 1.8.2'
 
 
 _COLORS = {
@@ -995,6 +996,34 @@ def _project(config_settings: Optional[Dict[Any, Any]]) -> Iterator[Project]:
         yield project
 
 
+def _get_ninja_if_needed() -> List[str]:
+    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:
+            # The meson search function skips forward if the version is not readable or too low
+            continue
+        if candidate_version < (1, 8, 2):
+            continue
+        return []
+
+    return [_depstr.ninja]
+
+
+def get_requires_for_build_sdist(
+    config_settings: Optional[Dict[str, str]] = None,
+) -> List[str]:
+    return _get_ninja_if_needed()
+
+
 def build_sdist(
     sdist_directory: str,
     config_settings: Optional[Dict[Any, Any]] = None,
@@ -1009,13 +1038,13 @@ def build_sdist(
 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
-                dependencies.append(_depstr.patchelf)
+    dependencies = [_depstr.wheel, *_get_ninja_if_needed()]
+    if sys.platform.startswith('linux'):
+        # we may need patchelf
+        if not shutil.which('patchelf'):
+            # patchelf not already accessible on the system
+            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"',
@@ -42,11 +40,14 @@ test = [
   'pytest',
   'pytest-cov',
   'pytest-mock',
+  'pytest-virtualenv',
   'GitPython',
   'auditwheel',
   'Cython',
   'pyproject-metadata>=0.6.1',
-  'importlib_metadata; python_version<"3.8"'
+  'importlib_metadata; python_version<"3.8"',
+  'ninja',
+  'build',
 ]
 docs = [
   'furo>=2021.08.31',

tests/conftest.py

@@ -5,6 +5,8 @@
 import os.path
 import pathlib
 import shutil
+import subprocess
+import sys
 import tempfile
 import venv
 
@@ -103,3 +105,57 @@ def fixture(tmp_dir_session):
     globals()[f'package_{normalized}'] = generate_package_fixture(package)
     globals()[f'sdist_{normalized}'] = generate_sdist_fixture(package)
     globals()[f'wheel_{normalized}'] = generate_wheel_fixture(package)
+
+
+@pytest.fixture(scope='session')
+def pep518_wheelhouse(tmpdir_factory):
+    wheelhouse = tmpdir_factory.mktemp('wheelhouse')
+    dist = tmpdir_factory.mktemp('dist')
+    subprocess.run(
+        [sys.executable, '-m', 'build', '--wheel', '--outdir', str(dist)],
+        cwd=str(package_dir.parent.parent),
+        check=True,
+    )
+    (wheel_path,) = dist.visit('*.whl')
+    subprocess.run(
+        [
+            sys.executable,
+            '-m',
+            'pip',
+            'download',
+            '-q',
+            '-d',
+            str(wheelhouse),
+            str(wheel_path),
+        ],
+        check=True,
+    )
+    subprocess.run(
+        [
+            sys.executable,
+            '-m',
+            'pip',
+            'download',
+            '-q',
+            '-d',
+            str(wheelhouse),
+            'build',
+            'colorama',
+            'meson',
+            'ninja',
+            'patchelf',
+            'pyproject-metadata',
+            'tomli',
+            'typing-extensions',
+            'wheel',
+        ],
+        check=True,
+    )
+    return str(wheelhouse)
+
+
+@pytest.fixture
+def pep518(pep518_wheelhouse, monkeypatch):
+    monkeypatch.setenv('PIP_FIND_LINKS', pep518_wheelhouse)
+    monkeypatch.setenv('PIP_NO_INDEX', 'true')
+    return pep518_wheelhouse

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,34 @@
 from .conftest import cd_package
 
 
-if platform.system() == 'Linux':
-    VENDORING_DEPS = {mesonpy._depstr.patchelf}
-else:
-    VENDORING_DEPS = set()
-
-
-@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 mock.called:  # sanity check for the future if we add another usage
-        mock.assert_called_once_with('patchelf')
+@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}
+    if system_patchelf is None and sys.platform.startswith('linux'):
+        expected |= {mesonpy._depstr.patchelf}
+    if ninja is None or [int(x) for x in ninja.split('.')] < [1, 8, 2]:
+        expected |= {mesonpy._depstr.ninja}
 
     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

tests/test_pep518.py

@@ -0,0 +1,21 @@
+import pytest
+
+from .conftest import cd_package
+
+
+@pytest.mark.parametrize(
+    ('package'),
+    [
+        'scipy-like',
+    ]
+)
+@pytest.mark.parametrize(
+    'build_args', ['', '--wheel'], ids=['sdist_to_wheel', 'wheel_directly']
+)
+def test_pep518(pep518, virtualenv, package, build_args, tmp_path):
+    dist = tmp_path / 'dist'
+
+    virtualenv.run('python -m pip install build')
+
+    with cd_package(package) as package_dir:
+        virtualenv.run(f'python -m build --outdir={dist} {build_args}', cwd=package_dir)