Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: validate build-system table schema #365

Merged
merged 2 commits into from Sep 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.rst
Expand Up @@ -3,6 +3,16 @@ Changelog
+++++++++


Unreleased
==========

- Add schema validation for ``build-system`` table to check conformity
with PEP 517 and PEP 518 (`PR #365`_, Fixes `#364`_)

.. _PR #365: https://github.com/pypa/build/pull/365
.. _#364: https://github.com/pypa/build/issues/364


0.7.0 (16-09-2021)
==================

Expand Down
95 changes: 63 additions & 32 deletions src/build/__init__.py
Expand Up @@ -94,12 +94,33 @@ def __str__(self) -> str:
return f'Backend operation failed: {self.exception!r}'


class BuildSystemTableValidationError(BuildException):
"""
Exception raised when the ``[build-system]`` table in pyproject.toml is invalid.
"""

def __str__(self) -> str:
return f'Failed to validate `build-system` in pyproject.toml: {self.args[0]}'


class TypoWarning(Warning):
"""
Warning raised when a potential typo is found
"""


@contextlib.contextmanager
def _working_directory(path: str) -> Iterator[None]:
current = os.getcwd()

os.chdir(path)

try:
yield
finally:
os.chdir(current)


def _validate_source_directory(srcdir: str) -> None:
if not os.path.isdir(srcdir):
raise BuildException(f'Source {srcdir} is not a directory')
Expand Down Expand Up @@ -153,25 +174,51 @@ def check_dependency(


def _find_typo(dictionary: Mapping[str, str], expected: str) -> None:
if expected not in dictionary:
for obj in dictionary:
if difflib.SequenceMatcher(None, expected, obj).ratio() >= 0.8:
warnings.warn(
f"Found '{obj}' in pyproject.toml, did you mean '{expected}'?",
TypoWarning,
)
for obj in dictionary:
if difflib.SequenceMatcher(None, expected, obj).ratio() >= 0.8:
warnings.warn(
f"Found '{obj}' in pyproject.toml, did you mean '{expected}'?",
TypoWarning,
)


@contextlib.contextmanager
def _working_directory(path: str) -> Iterator[None]:
current = os.getcwd()
def _parse_build_system_table(pyproject_toml: Mapping[str, Any]) -> Dict[str, Any]:
# If pyproject.toml is missing (per PEP 517) or [build-system] is missing
# (per PEP 518), use default values
if 'build-system' not in pyproject_toml:
_find_typo(pyproject_toml, 'build-system')
return _DEFAULT_BACKEND

os.chdir(path)
build_system_table = dict(pyproject_toml['build-system'])

try:
yield
finally:
os.chdir(current)
# If [build-system] is present, it must have a ``requires`` field (per PEP 518)
if 'requires' not in build_system_table:
_find_typo(build_system_table, 'requires')
raise BuildSystemTableValidationError('`requires` is a required property')
elif not isinstance(build_system_table['requires'], list) or not all(
isinstance(i, str) for i in build_system_table['requires']
):
raise BuildSystemTableValidationError('`requires` must be an array of strings')

if 'build-backend' not in build_system_table:
_find_typo(build_system_table, 'build-backend')
# If ``build-backend`` is missing, inject the legacy setuptools backend
# but leave ``requires`` intact to emulate pip
build_system_table['build-backend'] = _DEFAULT_BACKEND['build-backend']
elif not isinstance(build_system_table['build-backend'], str):
raise BuildSystemTableValidationError('`build-backend` must be a string')

if 'backend-path' in build_system_table and (
not isinstance(build_system_table['backend-path'], list)
or not all(isinstance(i, str) for i in build_system_table['backend-path'])
):
raise BuildSystemTableValidationError('`backend-path` must be an array of strings')

unknown_props = build_system_table.keys() - {'requires', 'build-backend', 'backend-path'}
if unknown_props:
raise BuildSystemTableValidationError(f'Unknown properties: {", ".join(unknown_props)}')

return build_system_table


class ProjectBuilder:
Expand Down Expand Up @@ -219,23 +266,7 @@ def __init__(
except TOMLDecodeError as e:
raise BuildException(f'Failed to parse {spec_file}: {e} ')

build_system = spec.get('build-system')
# if pyproject.toml is missing (per PEP 517) or [build-system] is missing (per PEP 518),
# use default values.
if build_system is None:
_find_typo(spec, 'build-system')
build_system = _DEFAULT_BACKEND
# if [build-system] is present, it must have a ``requires`` field (per PEP 518).
elif 'requires' not in build_system:
_find_typo(build_system, 'requires')
raise BuildException(f"Missing 'build-system.requires' in {spec_file}")
# if ``build-backend`` is missing, inject the legacy setuptools backend
# but leave ``requires`` alone to emulate pip.
elif 'build-backend' not in build_system:
_find_typo(build_system, 'build-backend')
build_system['build-backend'] = _DEFAULT_BACKEND['build-backend']

self._build_system = build_system
self._build_system = _parse_build_system_table(spec)
self._backend = self._build_system['build-backend']
self._scripts_dir = scripts_dir
self._hook_runner = runner
Expand Down
61 changes: 60 additions & 1 deletion tests/test_projectbuilder.py
Expand Up @@ -570,4 +570,63 @@ def test_log(mocker, caplog, test_flit_path):
('INFO', 'something'),
]
if sys.version_info >= (3, 8): # stacklevel
assert [(record.lineno) for record in caplog.records] == [305, 305, 338, 368, 368, 562]
assert caplog.records[-1].lineno == 562


@pytest.mark.parametrize(
('pyproject_toml', 'parse_output'),
[
(
{'build-system': {'requires': ['foo']}},
{'requires': ['foo'], 'build-backend': 'setuptools.build_meta:__legacy__'},
),
(
{'build-system': {'requires': ['foo'], 'build-backend': 'bar'}},
{'requires': ['foo'], 'build-backend': 'bar'},
),
(
{'build-system': {'requires': ['foo'], 'build-backend': 'bar', 'backend-path': ['baz']}},
{'requires': ['foo'], 'build-backend': 'bar', 'backend-path': ['baz']},
),
],
)
def test_parse_valid_build_system_table_type(pyproject_toml, parse_output):
assert build._parse_build_system_table(pyproject_toml) == parse_output


@pytest.mark.parametrize(
('pyproject_toml', 'error_message'),
[
(
{'build-system': {}},
'`requires` is a required property',
),
(
{'build-system': {'requires': 'not an array'}},
'`requires` must be an array of strings',
),
(
{'build-system': {'requires': [1]}},
'`requires` must be an array of strings',
),
(
{'build-system': {'requires': ['foo'], 'build-backend': ['not a string']}},
'`build-backend` must be a string',
),
(
{'build-system': {'requires': ['foo'], 'backend-path': 'not an array'}},
'`backend-path` must be an array of strings',
),
(
{'build-system': {'requires': ['foo'], 'backend-path': [1]}},
'`backend-path` must be an array of strings',
),
(
{'build-system': {'requires': ['foo'], 'unknown-prop': False}},
'Unknown properties: unknown-prop',
),
],
)
def test_parse_invalid_build_system_table_type(pyproject_toml, error_message):
with pytest.raises(build.BuildSystemTableValidationError, match=error_message):
build._parse_build_system_table(pyproject_toml)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather instantiate a ProjectBuilder here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd have to serialise them all as TOML, create a tmp dir for each and write them on disk - I don't think I can disentangle the disk I/O with the mocker.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can, as you should be able to patch builtins.open, but I'll leave that up to you. This is just a nitpick.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I might leave that for another day then 😛