From 21a5afb6e7f77f638f325ceb5b269b31ea1bb02b Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 14 Oct 2022 23:42:57 -0400 Subject: [PATCH] ENH: use system ninja if adequate Signed-off-by: Henry Schreiner --- mesonpy/__init__.py | 73 +++++++++++++++++++++++++++++++++++--------- pyproject.toml | 5 +-- setup.cfg | 2 +- tests/test_pep517.py | 58 ++++++++++++++++++++--------------- tests/test_pep518.py | 1 - 5 files changed, 96 insertions(+), 43 deletions(-) diff --git a/mesonpy/__init__.py b/mesonpy/__init__.py index cb7ad8879..d51d9860f 100644 --- a/mesonpy/__init__.py +++ b/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,16 @@ 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) + + 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 + dependencies.append(_depstr.patchelf) + return dependencies diff --git a/pyproject.toml b/pyproject.toml index a18c3d46f..89c1893a4 100644 --- a/pyproject.toml +++ b/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"', @@ -43,10 +41,13 @@ test = [ 'pytest', 'pytest-cov', 'pytest-mock', + 'GitPython', 'auditwheel', 'Cython', 'pyproject-metadata>=0.6.1', 'wheel', + 'importlib_metadata; python_version<"3.8"', + 'ninja', ] docs = [ 'furo>=2021.08.31', diff --git a/setup.cfg b/setup.cfg index fbdc1c703..c10950a39 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [flake8] max-line-length = 127 -max-complexity = 10 +max-complexity = 12 extend-ignore = E203 [mypy] diff --git a/tests/test_pep517.py b/tests/test_pep517.py index 62b1c2a6c..59f24cb3b 100644 --- a/tests/test_pep517.py +++ b/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 diff --git a/tests/test_pep518.py b/tests/test_pep518.py index a2367fa6c..085438a8a 100644 --- a/tests/test_pep518.py +++ b/tests/test_pep518.py @@ -15,7 +15,6 @@ @pytest.mark.parametrize( 'build_arg', ['', '--wheel'], ids=['sdist_to_wheel', 'wheel_directly'] ) -@pytest.mark.xfail(sys.platform.startswith('cygwin'), reason='ninja pip package not available on cygwin', strict=True) def test_pep518(pep518, package, build_arg, tmp_path): dist = tmp_path / 'dist'