From da119d1d0aaa8675a824f8a9c9dd2a9c616dd683 Mon Sep 17 00:00:00 2001 From: Daniele Nicolodi Date: Thu, 9 Feb 2023 22:35:53 +0100 Subject: [PATCH] MAINT: consolidate validation of options from build frontend PEP 517 specifies that the options are received as a dictionary with string keys and string values, thus there is no need to verify that. --- mesonpy/__init__.py | 96 +++++++++++++++++++++----------------------- tests/test_pep517.py | 31 ++++++++++++-- 2 files changed, 72 insertions(+), 55 deletions(-) diff --git a/mesonpy/__init__.py b/mesonpy/__init__.py index 11ffddf86..54bda103e 100644 --- a/mesonpy/__init__.py +++ b/mesonpy/__init__.py @@ -14,6 +14,7 @@ import argparse import collections import contextlib +import difflib import functools import importlib.machinery import io @@ -76,7 +77,7 @@ MesonArgsKeys = Literal['dist', 'setup', 'compile', 'install'] MesonArgs = Mapping[MesonArgsKeys, List[str]] else: - MesonArgs = None + MesonArgs = dict _COLORS = { @@ -685,6 +686,44 @@ def _strings(value: Any, name: str) -> List[str]: return scheme(table, 'tool.meson-python') +def _validate_config_settings(config_settings: Dict[str, Any]) -> Dict[str, Any]: + """Validate options received from build frontend.""" + + def _string(value: Any, name: str) -> str: + if not isinstance(value, str): + raise ConfigError(f'Only one value for "{name}" can be specified') + return value + + def _bool(value: Any, name: str) -> bool: + return True + + def _string_or_strings(value: Any, name: str) -> List[str]: + return list([value,] if isinstance(value, str) else value) + + options = { + 'builddir': _string, + 'editable-verbose': _bool, + 'dist-args': _string_or_strings, + 'setup-args': _string_or_strings, + 'compile-args': _string_or_strings, + 'install-args': _string_or_strings, + } + assert all(f'{name}-args' in options for name in _MESON_ARGS_KEYS) + + config = {} + for key, value in config_settings.items(): + parser = options.get(key) + if parser is None: + matches = difflib.get_close_matches(key, options.keys(), n=2) + if matches: + alternatives = ' or '.join(f'"{match}"' for match in matches) + raise ConfigError(f'Unknown option "{key}". Did you mean {alternatives}?') + else: + raise ConfigError(f'Unknown option "{key}"') + config[key] = parser(value, key) + return config + + class Project(): """Meson project wrapper to generate Python artifacts.""" @@ -1056,59 +1095,14 @@ def editable(self, directory: Path) -> pathlib.Path: @contextlib.contextmanager def _project(config_settings: Optional[Dict[Any, Any]]) -> Iterator[Project]: """Create the project given the given config settings.""" - if config_settings is None: - config_settings = {} - - # expand all string values to single element tuples and convert collections to tuple - config_settings = { - key: tuple(value) if isinstance(value, Collection) and not isinstance(value, str) else (value,) - for key, value in config_settings.items() - } - - builddir_value = config_settings.get('builddir', {}) - if len(builddir_value) > 0: - if len(builddir_value) != 1: - raise ConfigError('Only one value for configuration entry "builddir" can be specified') - builddir = builddir_value[0] - if not isinstance(builddir, str): - raise ConfigError(f'Configuration entry "builddir" should be a string not {type(builddir)}') - else: - builddir = None - - def _validate_string_collection(key: str) -> None: - assert isinstance(config_settings, Mapping) - problematic_items: Sequence[Any] = list(filter(None, ( - item if not isinstance(item, str) else None - for item in config_settings.get(key, ()) - ))) - if problematic_items: - s = ', '.join(f'"{item}" ({type(item)})' for item in problematic_items) - raise ConfigError(f'Configuration entries for "{key}" must be strings but contain: {s}') - - meson_args_keys = _MESON_ARGS_KEYS - meson_args_cli_keys = tuple(f'{key}-args' for key in meson_args_keys) - - for key in config_settings: - known_keys = ('builddir', 'editable-verbose', *meson_args_cli_keys) - if key not in known_keys: - import difflib - matches = difflib.get_close_matches(key, known_keys, n=3) - if len(matches): - alternatives = ' or '.join(f'"{match}"' for match in matches) - raise ConfigError(f'Unknown configuration entry "{key}". Did you mean {alternatives}?') - else: - raise ConfigError(f'Unknown configuration entry "{key}"') - for key in meson_args_cli_keys: - _validate_string_collection(key) + settings = _validate_config_settings(config_settings or {}) + meson_args = {name: settings.get(f'{name}-args', []) for name in _MESON_ARGS_KEYS} with Project.with_temp_working_dir( - build_dir=builddir, - meson_args=typing.cast(MesonArgs, { - key: config_settings.get(f'{key}-args', ()) - for key in meson_args_keys - }), - editable_verbose=bool(config_settings.get('editable-verbose')) + build_dir=settings.get('builddir'), + meson_args=typing.cast(MesonArgs, meson_args), + editable_verbose=bool(settings.get('editable-verbose')) ) as project: yield project diff --git a/tests/test_pep517.py b/tests/test_pep517.py index 6e14227d6..b3c933f75 100644 --- a/tests/test_pep517.py +++ b/tests/test_pep517.py @@ -66,8 +66,7 @@ def test_invalid_config_settings(capsys, package_pure, tmp_path_session): with pytest.raises(SystemExit): method(tmp_path_session, {'invalid': ()}) out, err = capsys.readouterr() - assert out.splitlines()[-1].endswith( - 'Unknown configuration entry "invalid"') + assert out.splitlines()[-1].endswith('Unknown option "invalid"') def test_invalid_config_settings_suggest(capsys, package_pure, tmp_path_session): @@ -75,5 +74,29 @@ def test_invalid_config_settings_suggest(capsys, package_pure, tmp_path_session) with pytest.raises(SystemExit): method(tmp_path_session, {'setup_args': ()}) out, err = capsys.readouterr() - assert out.splitlines()[-1].endswith( - 'Unknown configuration entry "setup_args". Did you mean "setup-args" or "dist-args"?') + assert out.splitlines()[-1].endswith('Unknown option "setup_args". Did you mean "setup-args" or "dist-args"?') + + +def test_validate_config_settings_invalid(): + with pytest.raises(mesonpy.ConfigError, match='Unknown option "invalid"'): + mesonpy._validate_config_settings({'invalid': ()}) + + +def test_validate_config_settings_repeated(): + with pytest.raises(mesonpy.ConfigError, match='Only one value for "builddir" can be specified'): + mesonpy._validate_config_settings({'builddir': ['one', 'two']}) + + +def test_validate_config_settings_str(): + config = mesonpy._validate_config_settings({'setup-args': '-Dfoo=true'}) + assert config['setup-args'] == ['-Dfoo=true'] + + +def test_validate_config_settings_list(): + config = mesonpy._validate_config_settings({'setup-args': ['-Done=1', '-Dtwo=2']}) + assert config['setup-args'] == ['-Done=1', '-Dtwo=2'] + + +def test_validate_config_settings_tuple(): + config = mesonpy._validate_config_settings({'setup-args': ('-Done=1', '-Dtwo=2')}) + assert config['setup-args'] == ['-Done=1', '-Dtwo=2']