From 7df0f13708b7b6fa358845d8b7c7e568cc3c52b8 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 14 Oct 2022 23:42:57 -0400 Subject: [PATCH 1/3] ENH: use system ninja if adequate Signed-off-by: Henry Schreiner --- .flake8 | 2 +- mesonpy/__init__.py | 73 +++++++++++++++++++++++++++++++++++--------- pyproject.toml | 5 +-- tests/test_pep517.py | 58 ++++++++++++++++++++--------------- 4 files changed, 96 insertions(+), 42 deletions(-) diff --git a/.flake8 b/.flake8 index cfb05843d..17f43dedd 100644 --- a/.flake8 +++ b/.flake8 @@ -1,4 +1,4 @@ [flake8] max-line-length = 127 -max-complexity = 10 +max-complexity = 12 extend-ignore = E203 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 60d95d0b3..43aa6f73c 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>=6.0', 'pytest-cov[toml]', '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/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 From 47f3ba6b6115c7ffdf9e151475399d3996de055e Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Thu, 17 Nov 2022 11:08:09 -0500 Subject: [PATCH 2/3] fix: address feedback Signed-off-by: Henry Schreiner --- pyproject.toml | 2 -- 1 file changed, 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 43aa6f73c..78636bd03 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,12 +41,10 @@ test = [ 'pytest>=6.0', 'pytest-cov[toml]', 'pytest-mock', - 'GitPython', 'auditwheel', 'Cython', 'pyproject-metadata>=0.6.1', 'wheel', - 'importlib_metadata; python_version<"3.8"', 'ninja', ] docs = [ From 9d47743bd86ec6bb032b04f389d637682c21aec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filipe=20La=C3=ADns?= Date: Fri, 18 Nov 2022 19:16:08 +0000 Subject: [PATCH 3/3] don't ask for patchelf if ninja is available and the project is pure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Filipe Laíns --- mesonpy/__init__.py | 10 +++++++++- tests/test_pep517.py | 13 ++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/mesonpy/__init__.py b/mesonpy/__init__.py index d51d9860f..b888a7087 100644 --- a/mesonpy/__init__.py +++ b/mesonpy/__init__.py @@ -1019,7 +1019,15 @@ def get_requires_for_build_wheel( # we may need patchelf if not shutil.which('patchelf'): # patchelf not already accessible on the system - dependencies.append(_depstr.patchelf) + 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 diff --git a/tests/test_pep517.py b/tests/test_pep517.py index 59f24cb3b..8600a4fd7 100644 --- a/tests/test_pep517.py +++ b/tests/test_pep517.py @@ -37,10 +37,17 @@ def run(cmd: List[str], *args: object, **kwargs: object) -> subprocess.Completed 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]: + + ninja_available = ninja is not None and [int(x) for x in ninja.split('.')] >= [1, 8, 2] + + if not ninja_available: expected |= {mesonpy._depstr.ninja} + 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