Skip to content

Commit

Permalink
fix: use system ninja if adequate
Browse files Browse the repository at this point in the history
Signed-off-by: Henry Schreiner <[email protected]>
  • Loading branch information
henryiii committed Oct 15, 2022
1 parent d18930f commit 44ee2b4
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 27 deletions.
25 changes: 24 additions & 1 deletion mesonpy/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class _depstr:
"""
patchelf = 'patchelf >= 0.11.0'
wheel = 'wheel >= 0.36.0' # noqa: F811
ninja = 'ninja >= 1.7'


_COLORS = {
Expand Down Expand Up @@ -951,6 +952,27 @@ def _project(config_settings: Optional[Dict[Any, Any]]) -> Iterator[Project]:
yield project


def _get_ninja_if_needed() -> List[str]:
# only add ninja if it is not present or is too low a version
ninja = shutil.which('ninja')
if ninja is None:
return []

result = subprocess.run(['ninja', '--version'], check=False, text=True, capture_output=True)
try:
candidate_version = float('.'.join(result.stdout.split('.')[:2]))
except ValueError:
return [_depstr.ninja]

return [] if candidate_version >= 1.7 else [_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,
Expand All @@ -965,13 +987,14 @@ def build_sdist(
def get_requires_for_build_wheel(
config_settings: Optional[Dict[str, str]] = None,
) -> List[str]:
dependencies = [_depstr.wheel]
dependencies = [_depstr.wheel, *_get_ninja_if_needed()]
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)

return dependencies


Expand Down
2 changes: 0 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ build-backend = 'mesonpy'
backend-path = ['.']
requires = [
'meson>=0.63.3',
'ninja',
'pyproject-metadata>=0.5.0',
'tomli>=1.0.0',
'typing-extensions>=3.7.4; python_version<"3.8"',
Expand All @@ -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',
'typing-extensions>=3.7.4; python_version<"3.8"',
Expand Down
56 changes: 32 additions & 24 deletions tests/test_pep517.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# SPDX-License-Identifier: MIT

import platform
import shutil
import subprocess
import sys

from typing import List

import pytest

Expand All @@ -9,28 +13,32 @@
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', [True, False], ids=['patchelf', 'nopatchelf'])
@pytest.mark.parametrize('ninja', [None, '1.6', '1.10'], ids=['noninjga', '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 is not 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 and sys.platform.startswith('linux'):
expected |= {mesonpy._depstr.patchelf}
if ninja is None or float(ninja) < 1.7:
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

0 comments on commit 44ee2b4

Please sign in to comment.