BUG: improve validation of pyproject.toml meson-python configuration

mesonpy/__init__.py

@@ -14,6 +14,7 @@
 import argparse
 import collections
 import contextlib
+import difflib
 import functools
 import importlib.machinery
 import io
@@ -41,6 +42,7 @@
 else:
     import tomllib
 
+import packaging.version
 import pyproject_metadata
 
 import mesonpy._compat
@@ -75,7 +77,7 @@
     MesonArgsKeys = Literal['dist', 'setup', 'compile', 'install']
     MesonArgs = Mapping[MesonArgsKeys, List[str]]
 else:
-    MesonArgs = None
+    MesonArgs = dict
 
 
 _COLORS = {
@@ -654,15 +656,83 @@ def build_editable(self, directory: Path, verbose: bool = False) -> pathlib.Path
         return wheel_file
 
 
+def _validate_pyproject_config(pyproject: Dict[str, Any]) -> Dict[str, Any]:
+
+    def _table(scheme: Dict[str, Callable[[Any, str], Any]]) -> Callable[[Any, str], Dict[str, Any]]:
+        def func(value: Any, name: str) -> Dict[str, Any]:
+            if not isinstance(value, dict):
+                raise ConfigError(f'Configuration entry "{name}" must be a table')
+            table = {}
+            for key, val in value.items():
+                check = scheme.get(key)
+                if check is None:
+                    raise ConfigError(f'Unknown configuration entry "{name}.{key}"')
+                table[key] = check(val, f'{name}.{key}')
+            return table
+        return func
+
+    def _strings(value: Any, name: str) -> List[str]:
+        if not isinstance(value, list) or not all(isinstance(x, str) for x in value):
+            raise ConfigError(f'Configuration entry "{name}" must be a list of strings')
+        return value
+
+    scheme = _table({
+        'args': _table({
+            name: _strings for name in _MESON_ARGS_KEYS
+        })
+    })
+
+    table = pyproject.get('tool', {}).get('meson-python', {})
+    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."""
 
     _ALLOWED_DYNAMIC_FIELDS: ClassVar[List[str]] = [
         'version',
     ]
-    _metadata: Optional[pyproject_metadata.StandardMetadata]
+    _metadata: pyproject_metadata.StandardMetadata
 
-    def __init__(  # noqa: C901
+    def __init__(
         self,
         source_dir: Path,
         working_dir: Path,
@@ -712,29 +782,13 @@ def __init__(  # noqa: C901
                     self._meson_cross_file.write_text(cross_file_data)
                     self._meson_args['setup'].extend(('--cross-file', os.fspath(self._meson_cross_file)))
 
-        # load config -- PEP 621 support is optional
-        self._config = tomllib.loads(self._source_dir.joinpath('pyproject.toml').read_text())
-        self._pep621 = 'project' in self._config
-        if self.pep621:
-            self._metadata = pyproject_metadata.StandardMetadata.from_pyproject(self._config, self._source_dir)
-        else:
-            print(
-                '{yellow}{bold}! Using Meson to generate the project metadata '
-                '(no `project` section in pyproject.toml){reset}'.format(**_STYLES)
-            )
-            self._metadata = None
+        # load pyproject.toml
+        pyproject = tomllib.loads(self._source_dir.joinpath('pyproject.toml').read_text())
 
-        if self._metadata:
-            self._validate_metadata()
-
-        # load meson args
-        for key in self._get_config_key('args'):
-            self._meson_args[key].extend(self._get_config_key(f'args.{key}'))
-        # XXX: We should validate the user args to make sure they don't conflict with ours.
-
-        self._check_for_unknown_config_keys({
-            'args': _MESON_ARGS_KEYS,
-        })
+        # load meson args from pyproject.toml
+        pyproject_config = _validate_pyproject_config(pyproject)
+        for key, value in pyproject_config.get('args', {}).items():
+            self._meson_args[key].extend(value)
 
         # meson arguments from the command line take precedence over
         # arguments from the configuration file thus are added later
@@ -764,17 +818,21 @@ def __init__(  # noqa: C901
         # run meson setup
         self._configure(reconfigure=reconfigure)
 
-        # set version if dynamic (this fetches it from Meson)
-        if self._metadata and 'version' in self._metadata.dynamic:
-            self._metadata.version = self.version
+        # package metadata
+        if 'project' in pyproject:
+            self._metadata = pyproject_metadata.StandardMetadata.from_pyproject(pyproject, self._source_dir)
+        else:
+            self._metadata = pyproject_metadata.StandardMetadata(
+                name=self._meson_name, version=packaging.version.Version(self._meson_version))
+            print(
+                '{yellow}{bold}! Using Meson to generate the project metadata '
+                '(no `project` section in pyproject.toml){reset}'.format(**_STYLES)
+            )
+        self._validate_metadata()
 
-    def _get_config_key(self, key: str) -> Any:
-        value: Any = self._config
-        for part in f'tool.meson-python.{key}'.split('.'):
-            if not isinstance(value, Mapping):
-                raise ConfigError(f'Configuration entry "tool.meson-python.{key}" should be a TOML table not {type(value)}')
-            value = value.get(part, {})
-        return value
+        # set version from meson.build if dynamic
+        if 'version' in self._metadata.dynamic:
+            self._metadata.version = packaging.version.Version(self._meson_version)
 
     def _run(self, cmd: Sequence[str]) -> None:
         """Invoke a subprocess."""
@@ -814,8 +872,6 @@ def _configure(self, reconfigure: bool = False) -> None:
     def _validate_metadata(self) -> None:
         """Check the pyproject.toml metadata and see if there are any issues."""
 
-        assert self._metadata
-
         # check for unsupported dynamic fields
         unsupported_dynamic = {
             key for key in self._metadata.dynamic
@@ -834,17 +890,6 @@ def _validate_metadata(self) -> None:
                     f'expected {self._metadata.requires_python}'
                 )
 
-    def _check_for_unknown_config_keys(self, valid_args: Mapping[str, Collection[str]]) -> None:
-        config = self._config.get('tool', {}).get('meson-python', {})
-
-        for key, valid_subkeys in config.items():
-            if key not in valid_args:
-                raise ConfigError(f'Unknown configuration key "tool.meson-python.{key}"')
-
-            for subkey in valid_args[key]:
-                if subkey not in valid_subkeys:
-                    raise ConfigError(f'Unknown configuration key "tool.meson-python.{key}.{subkey}"')
-
     @cached_property
     def _wheel_builder(self) -> _WheelBuilder:
         return _WheelBuilder(
@@ -949,45 +994,18 @@ def _meson_version(self) -> str:
 
     @property
     def name(self) -> str:
-        """Project name. Specified in pyproject.toml."""
-        name = self._metadata.name if self._metadata else self._meson_name
-        assert isinstance(name, str)
-        return name.replace('-', '_')
+        """Project name."""
+        return str(self._metadata.name).replace('-', '_')
 
     @property
     def version(self) -> str:
-        """Project version. Either specified in pyproject.toml or meson.build."""
-        if self._metadata and 'version' not in self._metadata.dynamic:
-            version = str(self._metadata.version)
-        else:
-            version = self._meson_version
-        assert isinstance(version, str)
-        return version
+        """Project version."""
+        return str(self._metadata.version)
 
     @cached_property
     def metadata(self) -> bytes:
-        """Project metadata."""
-        # the rest of the keys are only available when using PEP 621 metadata
-        if not self.pep621:
-            data = textwrap.dedent(f'''
-                Metadata-Version: 2.1
-                Name: {self.name}
-                Version: {self.version}
-            ''').strip()
-            return data.encode()
-
-        # re-import pyproject_metadata to raise ModuleNotFoundError if it is really missing
-        import pyproject_metadata  # noqa: F401
-        assert self._metadata
-
-        core_metadata = self._metadata.as_rfc822()
-        # use self.version as the version may be dynamic -- fetched from Meson
-        #
-        # we need to overwrite this field in the RFC822 field as
-        # pyproject_metadata removes 'version' from the dynamic fields when
-        # giving it a value via the dataclass
-        core_metadata.headers['Version'] = [self.version]
-        return bytes(core_metadata)
+        """Project metadata as an RFC822 message."""
+        return bytes(self._metadata.as_rfc822())
 
     @property
     def license_file(self) -> Optional[pathlib.Path]:
@@ -1002,11 +1020,6 @@ def is_pure(self) -> bool:
         """Is the wheel "pure" (architecture independent)?"""
         return bool(self._wheel_builder.is_pure)
 
-    @property
-    def pep621(self) -> bool:
-        """Does the project use PEP 621 metadata?"""
-        return self._pep621
-
     def sdist(self, directory: Path) -> pathlib.Path:
         """Generates a sdist (source distribution) in the specified directory."""
         # generate meson dist file
@@ -1082,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
 

tests/packages/unsupported-dynamic/meson.build

@@ -0,0 +1,5 @@
+# SPDX-FileCopyrightText: 2021 The meson-python developers
+#
+# SPDX-License-Identifier: MIT
+
+project('unsupported-dynamic', version: '1.0.0')

tests/test_metadata.py

@@ -10,11 +10,11 @@ def test_no_pep621(sdist_library):
     with tarfile.open(sdist_library, 'r:gz') as sdist:
         sdist_pkg_info = sdist.extractfile('library-1.0.0/PKG-INFO').read().decode()
 
-    assert sdist_pkg_info == textwrap.dedent('''
+    assert sdist_pkg_info == textwrap.dedent('''\
         Metadata-Version: 2.1
         Name: library
         Version: 1.0.0
-    ''').strip()
+    ''')
 
 
 def test_pep621(sdist_full_metadata):
@@ -61,10 +61,10 @@ def test_pep621(sdist_full_metadata):
 
 def test_dynamic_version(sdist_dynamic_version):
     with tarfile.open(sdist_dynamic_version, 'r:gz') as sdist:
-        sdist_pkg_info = sdist.extractfile('dynamic_version-1.0.0/PKG-INFO').read().decode().strip()
+        sdist_pkg_info = sdist.extractfile('dynamic_version-1.0.0/PKG-INFO').read().decode()
 
-    assert sdist_pkg_info == textwrap.dedent('''
+    assert sdist_pkg_info == textwrap.dedent('''\
         Metadata-Version: 2.1
         Name: dynamic-version
         Version: 1.0.0
-    ''').strip()
+    ''')