diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 52a98496..75db4707 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,8 +7,18 @@ Unreleased ========== - Add ``ProjectBuilder.metadata_path`` helper (`PR #303`_, Fixes `#301`_) +- Added a ``build.__main__.build_package_via_sdist`` method (`PR #304`_) + +Breaking Changes +---------------- + +- Binary distributions are now built via the sdist by default in the CLI (`PR #304`_, Fixes `#257`_) + - ``python -m build`` will now build a sdist, extract it, and build a wheel from the source +- As a side-effect of `PR #304`_, ``build.__main__.build_package`` no longer does CLI error handling (print nice message and exit the program) .. _PR #303: https://github.com/pypa/build/pull/303 +.. _PR #304: https://github.com/pypa/build/pull/304 +.. _#257: https://github.com/pypa/build/issues/257 .. _#301: https://github.com/pypa/build/issues/301 diff --git a/src/build/__main__.py b/src/build/__main__.py index ebbadd25..efd8745f 100644 --- a/src/build/__main__.py +++ b/src/build/__main__.py @@ -3,13 +3,18 @@ from __future__ import print_function import argparse +import contextlib import os +import shutil import subprocess import sys +import tarfile +import tempfile +import textwrap import traceback import warnings -from typing import Iterable, List, Optional, Sequence, TextIO, Type, Union +from typing import Iterable, Iterator, List, Optional, Sequence, TextIO, Type, Union import build @@ -28,6 +33,16 @@ __all__ = ['build', 'main', 'main_parser'] +if sys.version_info[0] == 2: + + def _indent(text, prefix): # type: (str, str) -> str + return ''.join(prefix + line for line in text.splitlines(True)) + + +else: + from textwrap import indent as _indent + + def _showwarning(message, category, filename, lineno, file=None, line=None): # pragma: no cover # type: (Union[Warning, str], Type[Warning], str, int, Optional[TextIO], Optional[str]) -> None prefix = 'WARNING' @@ -57,54 +72,41 @@ def _format_dep_chain(dep_chain): # type: (Sequence[str]) -> str return ' -> '.join(dep.partition(';')[0].strip() for dep in dep_chain) -def _build_in_isolated_env(builder, outdir, distributions, config_settings): - # type: (ProjectBuilder, str, List[str], ConfigSettingsType) -> None - for distribution in distributions: - with IsolatedEnvBuilder() as env: - builder.python_executable = env.executable - builder.scripts_dir = env.scripts_dir - # first install the build dependencies - env.install(builder.build_system_requires) - # then get the extra required dependencies from the backend (which was installed in the call above :P) - env.install(builder.get_requires_for_build(distribution)) - builder.build(distribution, outdir, config_settings) - - -def _build_in_current_env(builder, outdir, distributions, config_settings, skip_dependency_check=False): - # type: (ProjectBuilder, str, List[str], ConfigSettingsType, bool) -> None - for dist in distributions: - if not skip_dependency_check: - missing = builder.check_dependencies(dist) - if missing: - _error( - 'Missing dependencies:' - + ''.join('\n\t' + dep for deps in missing for dep in (deps[0], _format_dep_chain(deps[1:])) if dep) - ) +def _build_in_isolated_env(builder, outdir, distribution, config_settings): + # type: (ProjectBuilder, str, str, Optional[ConfigSettingsType]) -> str + with IsolatedEnvBuilder() as env: + builder.python_executable = env.executable + builder.scripts_dir = env.scripts_dir + # first install the build dependencies + env.install(builder.build_system_requires) + # then get the extra required dependencies from the backend (which was installed in the call above :P) + env.install(builder.get_requires_for_build(distribution)) + return builder.build(distribution, outdir, config_settings or {}) - builder.build(dist, outdir, config_settings) +def _build_in_current_env(builder, outdir, distribution, config_settings, skip_dependency_check=False): + # type: (ProjectBuilder, str, str, Optional[ConfigSettingsType], bool) -> str + if not skip_dependency_check: + missing = builder.check_dependencies(distribution) + if missing: + dependencies = ''.join('\n\t' + dep for deps in missing for dep in (deps[0], _format_dep_chain(deps[1:])) if dep) + _error('Missing dependencies:{}'.format(dependencies)) -def build_package(srcdir, outdir, distributions, config_settings=None, isolation=True, skip_dependency_check=False): - # type: (str, str, List[str], Optional[ConfigSettingsType], bool, bool) -> None - """ - Run the build process. + return builder.build(distribution, outdir, config_settings or {}) - :param srcdir: Source directory - :param outdir: Output directory - :param distributions: Distributions to build (sdist and/or wheel) - :param config_settings: Configuration settings to be passed to the backend - :param isolation: Isolate the build in a separate environment - :param skip_dependency_check: Do not perform the dependency check - """ - if not config_settings: - config_settings = {} +def _build(isolation, builder, outdir, distribution, config_settings, skip_dependency_check): + # type: (bool, ProjectBuilder, str, str, Optional[ConfigSettingsType], bool) -> str + if isolation: + return _build_in_isolated_env(builder, outdir, distribution, config_settings) + else: + return _build_in_current_env(builder, outdir, distribution, config_settings, skip_dependency_check) + + +@contextlib.contextmanager +def _handle_build_error(): # type: () -> Iterator[None] try: - builder = ProjectBuilder(srcdir) - if isolation: - _build_in_isolated_env(builder, outdir, distributions, config_settings) - else: - _build_in_current_env(builder, outdir, distributions, config_settings, skip_dependency_check) + yield except BuildException as e: _error(str(e)) except BuildBackendException as e: @@ -126,6 +128,54 @@ def build_package(srcdir, outdir, distributions, config_settings=None, isolation _error(str(e)) +def build_package(srcdir, outdir, distributions, config_settings=None, isolation=True, skip_dependency_check=False): + # type: (str, str, List[str], Optional[ConfigSettingsType], bool, bool) -> None + """ + Run the build process. + + :param srcdir: Source directory + :param outdir: Output directory + :param distribution: Distribution to build (sdist or wheel) + :param config_settings: Configuration settings to be passed to the backend + :param isolation: Isolate the build in a separate environment + :param skip_dependency_check: Do not perform the dependency check + """ + builder = ProjectBuilder(srcdir) + for distribution in distributions: + _build(isolation, builder, outdir, distribution, config_settings, skip_dependency_check) + + +def build_package_via_sdist(srcdir, outdir, distributions, config_settings=None, isolation=True, skip_dependency_check=False): + # type: (str, str, List[str], Optional[ConfigSettingsType], bool, bool) -> None + """ + Build a sdist and then the specified distributions from it. + + :param srcdir: Source directory + :param outdir: Output directory + :param distribution: Distribution to build (only wheel) + :param config_settings: Configuration settings to be passed to the backend + :param isolation: Isolate the build in a separate environment + :param skip_dependency_check: Do not perform the dependency check + """ + if 'sdist' in distributions: + raise ValueError('Only binary distributions are allowed but sdist was specified') + + builder = ProjectBuilder(srcdir) + sdist = _build(isolation, builder, outdir, 'sdist', config_settings, skip_dependency_check) + + # extract sdist + sdist_name = os.path.basename(sdist) + sdist_out = tempfile.mkdtemp(dir=outdir, prefix='build-via-sdist-') + with tarfile.open(sdist) as t: + t.extractall(sdist_out) + builder = ProjectBuilder(os.path.join(sdist_out, sdist_name[: -len('.tar.gz')])) + for distribution in distributions: + _build(isolation, builder, outdir, distribution, config_settings, skip_dependency_check) + + # remove sdist source if there was no exception + shutil.rmtree(sdist_out, ignore_errors=True) + + def main_parser(): # type: () -> argparse.ArgumentParser """ Construct the main parser. @@ -133,7 +183,27 @@ def main_parser(): # type: () -> argparse.ArgumentParser # mypy does not recognize module.__path__ # https://github.com/python/mypy/issues/1422 paths = build.__path__ # type: Iterable[Optional[str]] # type: ignore - parser = argparse.ArgumentParser() + parser = argparse.ArgumentParser( + description=_indent( # textwrap.indent + textwrap.dedent( + ''' + A simple, correct PEP 517 package builder. + + By default, a source distribution (sdist) is built from {srcdir} + and a binary distribution (wheel) is built from the sdist. + This is recommended as it will ensure the sdist can be used + to build wheels. + + Pass -s/--sdist and/or -w/--wheel to build a specific distribution. + If you do this, the default behavior will be disabled, and all + artifacts will be built from {srcdir} (even if you combine + -w/--wheel with -s/--sdist, the wheel will be built from {srcdir}). + ''' + ).strip(), + ' ', + ), + formatter_class=argparse.RawTextHelpFormatter, + ) parser.add_argument( 'srcdir', type=str, @@ -151,13 +221,13 @@ def main_parser(): # type: () -> argparse.ArgumentParser '--sdist', '-s', action='store_true', - help='build a source distribution (enabled by default if no target is specified)', + help='build a source distribution (disables the default behavior)', ) parser.add_argument( '--wheel', '-w', action='store_true', - help='build a wheel (enabled by default if no target is specified)', + help='build a wheel (disables the default behavior)', ) parser.add_argument( '--outdir', @@ -187,7 +257,7 @@ def main_parser(): # type: () -> argparse.ArgumentParser return parser -def main(cli_args, prog=None): # type: (List[str], Optional[str]) -> None +def main(cli_args, prog=None): # type: (List[str], Optional[str]) -> None # noqa: C901 """ Parse the CLI arguments and invoke the build process. @@ -218,14 +288,20 @@ def main(cli_args, prog=None): # type: (List[str], Optional[str]) -> None if args.wheel: distributions.append('wheel') - # default targets - if not distributions: - distributions = ['sdist', 'wheel'] - # outdir is relative to srcdir only if omitted. outdir = os.path.join(args.srcdir, 'dist') if args.outdir is None else args.outdir - build_package(args.srcdir, outdir, distributions, config_settings, not args.no_isolation, args.skip_dependency_check) + if distributions: + build_call = build_package + else: + build_call = build_package_via_sdist + distributions = ['wheel'] + try: + with _handle_build_error(): + build_call(args.srcdir, outdir, distributions, config_settings, not args.no_isolation, args.skip_dependency_check) + except Exception as e: # pragma: no cover + print(traceback.format_exc()) + _error(str(e)) def entrypoint(): # type: () -> None diff --git a/tests/conftest.py b/tests/conftest.py index ae5a0e57..ea21f677 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -170,6 +170,11 @@ def test_bad_wheel_path(packages_path): return os.path.join(packages_path, 'test-bad-wheel') +@pytest.fixture +def test_cant_build_via_sdist_path(packages_path): + return os.path.join(packages_path, 'test-cant-build-via-sdist') + + @pytest.fixture def test_no_permission(packages_path): path = os.path.join(packages_path, 'test-no-permission') diff --git a/tests/packages/test-cant-build-via-sdist/backend_bad_sdist.py b/tests/packages/test-cant-build-via-sdist/backend_bad_sdist.py new file mode 100644 index 00000000..01583379 --- /dev/null +++ b/tests/packages/test-cant-build-via-sdist/backend_bad_sdist.py @@ -0,0 +1,23 @@ +# SPDX-License-Identifier: MIT + +import os.path +import tarfile +import zipfile + + +def build_sdist(sdist_directory, config_settings=None): + name = 'test_cant_build_via_sdist-1.0.0' + file = '{}.tar.gz'.format(name) + with tarfile.open(os.path.join(sdist_directory, file), 'w') as t: + t.add('pyproject.toml', '{}/pyproject.toml'.format(name)) + t.add('backend_bad_sdist.py', '{}/backend_bad_sdist.py'.format(name)) + return file + + +def build_wheel(wheel_directory, config_settings=None, metadata_directory=None): + if not os.path.isfile('some-file-that-is-needed-for-build.txt'): + raise FileNotFoundError('some-file-that-is-needed-for-build.txt is missing!') + # pragma: no cover + file = 'test_cant_build_via_sdist-1.0.0-py2.py3-none-any.whl' + zipfile.ZipFile(os.path.join(wheel_directory, file), 'w').close() + return file diff --git a/tests/packages/test-cant-build-via-sdist/pyproject.toml b/tests/packages/test-cant-build-via-sdist/pyproject.toml new file mode 100644 index 00000000..e74afad0 --- /dev/null +++ b/tests/packages/test-cant-build-via-sdist/pyproject.toml @@ -0,0 +1,4 @@ +[build-system] +build-backend = 'backend_bad_sdist' +backend-path = ['.'] +requires = [] diff --git a/tests/packages/test-cant-build-via-sdist/some-file-that-is-needed-for-build.txt b/tests/packages/test-cant-build-via-sdist/some-file-that-is-needed-for-build.txt new file mode 100644 index 00000000..e69de29b diff --git a/tests/packages/test-setuptools/setup.cfg b/tests/packages/test-setuptools/setup.cfg index 058472c3..bf198b64 100644 --- a/tests/packages/test-setuptools/setup.cfg +++ b/tests/packages/test-setuptools/setup.cfg @@ -1,3 +1,6 @@ [metadata] name = test_setuptools version = 1.0.0 + +[bdist_wheel] +universal = 1 diff --git a/tests/test_integration.py b/tests/test_integration.py index f3d0a2d2..dd098430 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -2,6 +2,7 @@ import os import os.path +import platform import re import shutil import subprocess @@ -20,6 +21,11 @@ import build.__main__ +IS_WINDOWS = os.name == 'nt' +IS_PYPY3 = sys.version_info[0] == 3 and platform.python_implementation() == 'PyPy' +IS_PY35 = sys.version_info[:2] == (3, 5) + + INTEGRATION_SOURCES = { 'dateutil': ('dateutil/dateutil', '2.8.1'), 'pip': ('pypa/pip', '20.2.1'), @@ -101,6 +107,8 @@ def _ignore_folder(base, filenames): def test_build(monkeypatch, project, args, call, tmp_path): if project == 'flit' and '--no-isolation' in args: pytest.xfail("can't build flit without isolation due to missing dependencies") + if project == 'Solaar' and IS_WINDOWS and (IS_PYPY3 or IS_PY35): + pytest.xfail('Solaar fails building wheels via sdists on Windows in Python 3.5 or PyPy 3') monkeypatch.chdir(tmp_path) monkeypatch.setenv('SETUPTOOLS_SCM_PRETEND_VERSION', 'dummy') # for the projects that use setuptools_scm diff --git a/tests/test_main.py b/tests/test_main.py index 985475a8..902c609f 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -3,6 +3,7 @@ import contextlib import io import os +import re import sys import pytest @@ -22,59 +23,82 @@ @pytest.mark.parametrize( - ('cli_args', 'build_args'), + ('cli_args', 'build_args', 'hook'), [ ( [], - [cwd, out, ['sdist', 'wheel'], {}, True, False], + [cwd, out, ['wheel'], {}, True, False], + 'build_package_via_sdist', ), ( ['-n'], - [cwd, out, ['sdist', 'wheel'], {}, False, False], + [cwd, out, ['wheel'], {}, False, False], + 'build_package_via_sdist', ), ( ['-s'], [cwd, out, ['sdist'], {}, True, False], + 'build_package', ), ( ['-w'], [cwd, out, ['wheel'], {}, True, False], + 'build_package', + ), + ( + ['-s', '-w'], + [cwd, out, ['sdist', 'wheel'], {}, True, False], + 'build_package', ), ( ['source'], - ['source', os.path.join('source', 'dist'), ['sdist', 'wheel'], {}, True, False], + ['source', os.path.join('source', 'dist'), ['wheel'], {}, True, False], + 'build_package_via_sdist', ), ( ['-o', 'out'], - [cwd, 'out', ['sdist', 'wheel'], {}, True, False], + [cwd, 'out', ['wheel'], {}, True, False], + 'build_package_via_sdist', ), ( ['source', '-o', 'out'], - ['source', 'out', ['sdist', 'wheel'], {}, True, False], + ['source', 'out', ['wheel'], {}, True, False], + 'build_package_via_sdist', ), ( ['-x'], - [cwd, out, ['sdist', 'wheel'], {}, True, True], + [cwd, out, ['wheel'], {}, True, True], + 'build_package_via_sdist', ), ( ['-C--flag1', '-C--flag2'], - [cwd, out, ['sdist', 'wheel'], {'--flag1': '', '--flag2': ''}, True, False], + [cwd, out, ['wheel'], {'--flag1': '', '--flag2': ''}, True, False], + 'build_package_via_sdist', ), ( ['-C--flag=value'], - [cwd, out, ['sdist', 'wheel'], {'--flag': 'value'}, True, False], + [cwd, out, ['wheel'], {'--flag': 'value'}, True, False], + 'build_package_via_sdist', ), ( ['-C--flag1=value', '-C--flag2=other_value', '-C--flag2=extra_value'], - [cwd, out, ['sdist', 'wheel'], {'--flag1': 'value', '--flag2': ['other_value', 'extra_value']}, True, False], + [cwd, out, ['wheel'], {'--flag1': 'value', '--flag2': ['other_value', 'extra_value']}, True, False], + 'build_package_via_sdist', ), ], ) -def test_parse_args(mocker, cli_args, build_args): +def test_parse_args(mocker, cli_args, build_args, hook): mocker.patch('build.__main__.build_package') + mocker.patch('build.__main__.build_package_via_sdist') build.__main__.main(cli_args) - build.__main__.build_package.assert_called_with(*build_args) + + if hook == 'build_package': + build.__main__.build_package.assert_called_with(*build_args) + elif hook == 'build_package_via_sdist': + build.__main__.build_package_via_sdist.assert_called_with(*build_args) + else: + raise ValueError('Unknown hook {}'.format(hook)) # pragma: no cover def test_prog(): @@ -151,21 +175,46 @@ def test_build_no_isolation_with_check_deps(mocker, test_flit_path, missing_deps @pytest.mark.isolated def test_build_raises_build_exception(mocker, test_flit_path): - error = mocker.patch('build.__main__._error') mocker.patch('build.ProjectBuilder.get_requires_for_build', side_effect=build.BuildException) mocker.patch('build.env._IsolatedEnvVenvPip.install') - build.__main__.build_package(test_flit_path, '.', ['sdist']) - - error.assert_called_with('') + with pytest.raises(build.BuildException): + build.__main__.build_package(test_flit_path, '.', ['sdist']) @pytest.mark.isolated def test_build_raises_build_backend_exception(mocker, test_flit_path): - error = mocker.patch('build.__main__._error') mocker.patch('build.ProjectBuilder.get_requires_for_build', side_effect=build.BuildBackendException(Exception('a'))) mocker.patch('build.env._IsolatedEnvVenvPip.install') - build.__main__.build_package(test_flit_path, '.', ['sdist']) msg = "Backend operation failed: Exception('a'{})".format(',' if sys.version_info < (3, 7) else '') - error.assert_called_with(msg) + with pytest.raises(build.BuildBackendException, match=re.escape(msg)): + build.__main__.build_package(test_flit_path, '.', ['sdist']) + + +def test_build_package(tmp_dir, test_setuptools_path): + build.__main__.build_package(test_setuptools_path, tmp_dir, ['sdist', 'wheel']) + + assert sorted(os.listdir(tmp_dir)) == [ + 'test_setuptools-1.0.0-py2.py3-none-any.whl', + 'test_setuptools-1.0.0.tar.gz', + ] + + +def test_build_package_via_sdist(tmp_dir, test_setuptools_path): + build.__main__.build_package_via_sdist(test_setuptools_path, tmp_dir, ['wheel']) + + assert sorted(os.listdir(tmp_dir)) == [ + 'test_setuptools-1.0.0-py2.py3-none-any.whl', + 'test_setuptools-1.0.0.tar.gz', + ] + + +def test_build_package_via_sdist_cant_build(tmp_dir, test_cant_build_via_sdist_path): + with pytest.raises(build.BuildBackendException): + build.__main__.build_package_via_sdist(test_cant_build_via_sdist_path, tmp_dir, ['wheel']) + + +def test_build_package_via_sdist_invalid_distribution(tmp_dir, test_setuptools_path): + with pytest.raises(ValueError, match='Only binary distributions are allowed but sdist was specified'): + build.__main__.build_package_via_sdist(test_setuptools_path, tmp_dir, ['sdist'])