From 0534aeb89129718e44e34d67f50dc2072bc6a144 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Mon, 18 Jun 2018 17:05:45 +0530 Subject: [PATCH 1/6] Start refusing non PEP 518 pyproject.toml files --- src/pip/_internal/req/req_install.py | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 7846f3c169f..a3b6f820b08 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -571,8 +571,30 @@ def get_pep_518_info(self): if os.path.isfile(self.pyproject_toml): with io.open(self.pyproject_toml, encoding="utf-8") as f: pp_toml = pytoml.load(f) - build_sys = pp_toml.get('build-system', {}) - return (build_sys.get('requires', ['setuptools', 'wheel']), True) + + build_system = pp_toml.get('build-system', {}) + if "requires" not in build_system: + raise InstallationError( + "{} does not comply with PEP 518 as it since it's " + "pyproject.toml file does not have a '[build-system]' " + "table with a 'requires' key." + .format(self) + ) + + requires = build_system["requires"] + is_list_of_str = isinstance(requires, list) and all( + isinstance(req, str) for req in requires + ) + if not is_list_of_str: + raise InstallationError( + "{} does not comply with PEP 518 as it since it's " + "pyproject.toml file contains [build-system].requires " + "which is not a list of strings." + .format(self) + ) + + return requires, True + return (['setuptools', 'wheel'], False) def run_egg_info(self): From a25cb53d025ad5e3c546e27f11a9c7afd3a9ffd6 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Mon, 18 Jun 2018 17:06:28 +0530 Subject: [PATCH 2/6] Add tests to verify refusal of non PEP 518 compliant pyproject.toml --- .../src/pep518_invalid_requires/MANIFEST.in | 1 + .../src/pep518_invalid_requires/pep518.py | 1 + .../pep518_invalid_requires/pyproject.toml | 2 ++ .../src/pep518_invalid_requires/setup.cfg | 0 .../data/src/pep518_invalid_requires/setup.py | 9 +++++++ .../src/pep518_missing_requires/MANIFEST.in | 1 + .../src/pep518_missing_requires/pep518.py | 1 + .../pep518_missing_requires/pyproject.toml | 0 .../src/pep518_missing_requires/setup.cfg | 0 .../data/src/pep518_missing_requires/setup.py | 9 +++++++ tests/functional/test_install.py | 24 +++++++++++++++++++ 11 files changed, 48 insertions(+) create mode 100644 tests/data/src/pep518_invalid_requires/MANIFEST.in create mode 100644 tests/data/src/pep518_invalid_requires/pep518.py create mode 100644 tests/data/src/pep518_invalid_requires/pyproject.toml create mode 100644 tests/data/src/pep518_invalid_requires/setup.cfg create mode 100644 tests/data/src/pep518_invalid_requires/setup.py create mode 100644 tests/data/src/pep518_missing_requires/MANIFEST.in create mode 100644 tests/data/src/pep518_missing_requires/pep518.py create mode 100644 tests/data/src/pep518_missing_requires/pyproject.toml create mode 100644 tests/data/src/pep518_missing_requires/setup.cfg create mode 100644 tests/data/src/pep518_missing_requires/setup.py diff --git a/tests/data/src/pep518_invalid_requires/MANIFEST.in b/tests/data/src/pep518_invalid_requires/MANIFEST.in new file mode 100644 index 00000000000..bec201fc83b --- /dev/null +++ b/tests/data/src/pep518_invalid_requires/MANIFEST.in @@ -0,0 +1 @@ +include pyproject.toml diff --git a/tests/data/src/pep518_invalid_requires/pep518.py b/tests/data/src/pep518_invalid_requires/pep518.py new file mode 100644 index 00000000000..7986d11379a --- /dev/null +++ b/tests/data/src/pep518_invalid_requires/pep518.py @@ -0,0 +1 @@ +#dummy diff --git a/tests/data/src/pep518_invalid_requires/pyproject.toml b/tests/data/src/pep518_invalid_requires/pyproject.toml new file mode 100644 index 00000000000..17bc681d654 --- /dev/null +++ b/tests/data/src/pep518_invalid_requires/pyproject.toml @@ -0,0 +1,2 @@ +[build-system] +requires = [1, 2, 3] # not a list of strings diff --git a/tests/data/src/pep518_invalid_requires/setup.cfg b/tests/data/src/pep518_invalid_requires/setup.cfg new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/data/src/pep518_invalid_requires/setup.py b/tests/data/src/pep518_invalid_requires/setup.py new file mode 100644 index 00000000000..b63f59926f7 --- /dev/null +++ b/tests/data/src/pep518_invalid_requires/setup.py @@ -0,0 +1,9 @@ +#!/usr/bin/env python +from setuptools import setup + +import simplewheel # ensure dependency is installed + +setup(name='pep518', + version='3.0', + py_modules=['pep518'], + ) diff --git a/tests/data/src/pep518_missing_requires/MANIFEST.in b/tests/data/src/pep518_missing_requires/MANIFEST.in new file mode 100644 index 00000000000..bec201fc83b --- /dev/null +++ b/tests/data/src/pep518_missing_requires/MANIFEST.in @@ -0,0 +1 @@ +include pyproject.toml diff --git a/tests/data/src/pep518_missing_requires/pep518.py b/tests/data/src/pep518_missing_requires/pep518.py new file mode 100644 index 00000000000..7986d11379a --- /dev/null +++ b/tests/data/src/pep518_missing_requires/pep518.py @@ -0,0 +1 @@ +#dummy diff --git a/tests/data/src/pep518_missing_requires/pyproject.toml b/tests/data/src/pep518_missing_requires/pyproject.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/data/src/pep518_missing_requires/setup.cfg b/tests/data/src/pep518_missing_requires/setup.cfg new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/data/src/pep518_missing_requires/setup.py b/tests/data/src/pep518_missing_requires/setup.py new file mode 100644 index 00000000000..b63f59926f7 --- /dev/null +++ b/tests/data/src/pep518_missing_requires/setup.py @@ -0,0 +1,9 @@ +#!/usr/bin/env python +from setuptools import setup + +import simplewheel # ensure dependency is installed + +setup(name='pep518', + version='3.0', + py_modules=['pep518'], + ) diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index 385db77ae34..b1944cb71ef 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -37,6 +37,30 @@ def test_pep518_uses_build_env(script, data, common_wheels, command, variant): ) +def test_pep518_refuses_missing_requires(script, data, common_wheels): + result = script.pip( + 'install', + '-f', common_wheels, + '-f', data.find_links, + data.src.join("pep518_missing_requires"), + expect_error=True + ) + assert result.returncode == 1 + assert "does not comply with PEP 518" in result.stderr + + +def test_pep518_refuses_invalid_requires(script, data, common_wheels): + result = script.pip( + 'install', + '-f', common_wheels, + '-f', data.find_links, + data.src.join("pep518_invalid_requires"), + expect_error=True + ) + assert result.returncode == 1 + assert "does not comply with PEP 518" in result.stderr + + def test_pep518_with_user_pip(script, virtualenv, pip_src, data, common_wheels): virtualenv.system_site_packages = True From 0eb566fa8236d340a1a9e52c41ff9d96493e762d Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Tue, 19 Jun 2018 01:00:10 +0530 Subject: [PATCH 3/6] Rework PEP 518 requirement logic --- src/pip/_internal/operations/prepare.py | 42 ++++++++++--------- src/pip/_internal/req/req_install.py | 56 ++++++++++++------------- 2 files changed, 49 insertions(+), 49 deletions(-) diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index ff9b1e6a1df..3ad721f019a 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -95,31 +95,33 @@ def dist(self, finder): def prep_for_dist(self, finder, build_isolation): # Before calling "setup.py egg_info", we need to set-up the build # environment. - build_requirements, isolate = self.req.get_pep_518_info() - should_isolate = build_isolation and isolate - - minimum_requirements = ('setuptools', 'wheel') - missing_requirements = set(minimum_requirements) - set( - pkg_resources.Requirement(r).key - for r in build_requirements - ) - if missing_requirements: - def format_reqs(rs): - return ' and '.join(map(repr, sorted(rs))) - logger.warning( - "Missing build time requirements in pyproject.toml for %s: " - "%s.", self.req, format_reqs(missing_requirements) - ) - logger.warning( - "This version of pip does not implement PEP 517 so it cannot " - "build a wheel without %s.", format_reqs(minimum_requirements) - ) + build_requirements = self.req.get_pep_518_info() + should_isolate = build_isolation and build_requirements is not None if should_isolate: + # Haven't implemented PEP 517 yet, so spew a warning about it if + # build-requirements don't include setuptools and wheel. + missing_requirements = {'setuptools', 'wheel'} - { + pkg_resources.Requirement(r).key for r in build_requirements + } + if missing_requirements: + logger.warning( + "Missing build requirements in pyproject.toml for %s.", + self.req, + ) + logger.warning( + "This version of pip does not implement PEP 517 so it " + "cannot build a wheel without %s.", + " and ".join(map(repr, sorted(missing_requirements))) + ) + + # Isolate in a BuildEnvironment and install the build-time + # requirements. self.req.build_env = BuildEnvironment() self.req.build_env.install_requirements( finder, build_requirements, - "Installing build dependencies") + "Installing build dependencies" + ) self.req.run_egg_info() self.req.assert_source_matches_version() diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index a3b6f820b08..7a24af16c0e 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -560,42 +560,40 @@ def pyproject_toml(self): return pp_toml def get_pep_518_info(self): - """Get a list of the packages required to build the project, if any, - and a flag indicating whether pyproject.toml is present, indicating - that the build should be isolated. + """Get PEP 518 build-time requirements. - Build requirements can be specified in a pyproject.toml, as described - in PEP 518. If this file exists but doesn't specify build - requirements, pip will default to installing setuptools and wheel. + Returns the list of the packages required to build the project, + specified as per PEP 518 within the package. If `pyproject.toml` is not + present, returns None to signify not using the same. """ - if os.path.isfile(self.pyproject_toml): - with io.open(self.pyproject_toml, encoding="utf-8") as f: - pp_toml = pytoml.load(f) + if not os.path.isfile(self.pyproject_toml): + return None - build_system = pp_toml.get('build-system', {}) - if "requires" not in build_system: - raise InstallationError( - "{} does not comply with PEP 518 as it since it's " - "pyproject.toml file does not have a '[build-system]' " - "table with a 'requires' key." - .format(self) - ) + with io.open(self.pyproject_toml, encoding="utf-8") as f: + pp_toml = pytoml.load(f) - requires = build_system["requires"] + # Extract the build requirements + requires = pp_toml.get("build-system", {}).get("requires", None) + + # Error out if it's not valid + error_reason = None + if requires is None: + error_reason = "is missing." + else: is_list_of_str = isinstance(requires, list) and all( - isinstance(req, str) for req in requires + isinstance(req, six.string_types) for req in requires ) if not is_list_of_str: - raise InstallationError( - "{} does not comply with PEP 518 as it since it's " - "pyproject.toml file contains [build-system].requires " - "which is not a list of strings." - .format(self) - ) - - return requires, True - - return (['setuptools', 'wheel'], False) + error_reason = "not a list of strings." + if error_reason is not None: + msg = ( + "{} does not comply with PEP 518 since pyproject.toml does " + "not contain a valid '[build-system].requires' key: {}" + ).format(self, error_reason) + raise InstallationError(msg) + + # If control flow reaches here, we're good to go. + return requires def run_egg_info(self): assert self.source_dir From fdd51011288312757c618039e632b40d4df56d23 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Tue, 19 Jun 2018 01:00:25 +0530 Subject: [PATCH 4/6] Cleanup Tests --- .../src/pep518_invalid_requires/setup.cfg | 0 .../data/src/pep518_invalid_requires/setup.py | 11 +++++----- .../src/pep518_missing_requires/setup.cfg | 0 .../data/src/pep518_missing_requires/setup.py | 11 +++++----- tests/functional/test_install.py | 22 +++++-------------- 5 files changed, 15 insertions(+), 29 deletions(-) delete mode 100644 tests/data/src/pep518_invalid_requires/setup.cfg delete mode 100644 tests/data/src/pep518_missing_requires/setup.cfg diff --git a/tests/data/src/pep518_invalid_requires/setup.cfg b/tests/data/src/pep518_invalid_requires/setup.cfg deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/tests/data/src/pep518_invalid_requires/setup.py b/tests/data/src/pep518_invalid_requires/setup.py index b63f59926f7..e8a92da312a 100644 --- a/tests/data/src/pep518_invalid_requires/setup.py +++ b/tests/data/src/pep518_invalid_requires/setup.py @@ -1,9 +1,8 @@ #!/usr/bin/env python from setuptools import setup -import simplewheel # ensure dependency is installed - -setup(name='pep518', - version='3.0', - py_modules=['pep518'], - ) +setup( + name='pep518_invalid_requires', + version='1.0.0', + py_modules=['pep518'], +) diff --git a/tests/data/src/pep518_missing_requires/setup.cfg b/tests/data/src/pep518_missing_requires/setup.cfg deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/tests/data/src/pep518_missing_requires/setup.py b/tests/data/src/pep518_missing_requires/setup.py index b63f59926f7..cbc5d28af04 100644 --- a/tests/data/src/pep518_missing_requires/setup.py +++ b/tests/data/src/pep518_missing_requires/setup.py @@ -1,9 +1,8 @@ #!/usr/bin/env python from setuptools import setup -import simplewheel # ensure dependency is installed - -setup(name='pep518', - version='3.0', - py_modules=['pep518'], - ) +setup( + name='pep518_missing_requires', + version='1.0.0', + py_modules=['pep518'], +) diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index b1944cb71ef..2a97f92164b 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -37,24 +37,12 @@ def test_pep518_uses_build_env(script, data, common_wheels, command, variant): ) -def test_pep518_refuses_missing_requires(script, data, common_wheels): - result = script.pip( - 'install', - '-f', common_wheels, - '-f', data.find_links, - data.src.join("pep518_missing_requires"), - expect_error=True - ) - assert result.returncode == 1 - assert "does not comply with PEP 518" in result.stderr - - -def test_pep518_refuses_invalid_requires(script, data, common_wheels): +@pytest.mark.parametrize( + "package", ["pep518_invalid_requires", "pep518_missing_requires"] +) +def test_pep518_refuses_invalid_requires(script, data, common_wheels, package): result = script.pip( - 'install', - '-f', common_wheels, - '-f', data.find_links, - data.src.join("pep518_invalid_requires"), + 'install', '-f', common_wheels, data.src.join(package), expect_error=True ) assert result.returncode == 1 From 8f1cd5fa40ea7842c6e2d82a28537256d8015cc4 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Tue, 19 Jun 2018 01:07:40 +0530 Subject: [PATCH 5/6] :newspaper: --- news/5416.bugfix | 1 + news/5512.bugfix | 1 + 2 files changed, 2 insertions(+) create mode 100644 news/5416.bugfix create mode 100644 news/5512.bugfix diff --git a/news/5416.bugfix b/news/5416.bugfix new file mode 100644 index 00000000000..2f7707e0ce6 --- /dev/null +++ b/news/5416.bugfix @@ -0,0 +1 @@ +Start refusing to install packages with non PEP-518 compliant pyproject.toml diff --git a/news/5512.bugfix b/news/5512.bugfix new file mode 100644 index 00000000000..2f7707e0ce6 --- /dev/null +++ b/news/5512.bugfix @@ -0,0 +1 @@ +Start refusing to install packages with non PEP-518 compliant pyproject.toml From 2b68c7983e1a71efe8cf638a34d515d909c26932 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Tue, 19 Jun 2018 15:28:32 +0530 Subject: [PATCH 6/6] Only disable isolation when build-system.requires is skipped --- src/pip/_internal/req/req_install.py | 35 +++++++++++++++++++--------- tests/functional/test_install.py | 20 ++++++++++++---- 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 7a24af16c0e..bd6d4e9fe95 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -33,7 +33,9 @@ PIP_DELETE_MARKER_FILENAME, running_under_virtualenv, ) from pip._internal.req.req_uninstall import UninstallPathSet -from pip._internal.utils.deprecation import RemovedInPip11Warning +from pip._internal.utils.deprecation import ( + RemovedInPip11Warning, RemovedInPip12Warning, +) from pip._internal.utils.hashes import Hashes from pip._internal.utils.logging import indent_log from pip._internal.utils.misc import ( @@ -575,22 +577,33 @@ def get_pep_518_info(self): # Extract the build requirements requires = pp_toml.get("build-system", {}).get("requires", None) - # Error out if it's not valid - error_reason = None + template = ( + "%s does not comply with PEP 518 since pyproject.toml " + "does not contain a valid '[build-system].requires' key: %s" + ) + if requires is None: - error_reason = "is missing." + logging.warn(template, self, "it is missing.") + warnings.warn( + "Future versions of pip will reject packages with " + "pyproject.toml files that do not comply with PEP 518.", + RemovedInPip12Warning, + ) + + # NOTE: Currently allowing projects to skip this key so that they + # can transition to a PEP 518 compliant pyproject.toml or + # push to update the PEP. + # Come pip 19.0, bring this to compliance with PEP 518. + return None else: + # Error out if it's not a list of strings is_list_of_str = isinstance(requires, list) and all( isinstance(req, six.string_types) for req in requires ) if not is_list_of_str: - error_reason = "not a list of strings." - if error_reason is not None: - msg = ( - "{} does not comply with PEP 518 since pyproject.toml does " - "not contain a valid '[build-system].requires' key: {}" - ).format(self, error_reason) - raise InstallationError(msg) + raise InstallationError( + template % (self, "it is not a list of strings.") + ) # If control flow reaches here, we're good to go. return requires diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index 2a97f92164b..b1025190706 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -37,18 +37,28 @@ def test_pep518_uses_build_env(script, data, common_wheels, command, variant): ) -@pytest.mark.parametrize( - "package", ["pep518_invalid_requires", "pep518_missing_requires"] -) -def test_pep518_refuses_invalid_requires(script, data, common_wheels, package): +def test_pep518_refuses_invalid_requires(script, data, common_wheels): result = script.pip( - 'install', '-f', common_wheels, data.src.join(package), + 'install', '-f', common_wheels, + data.src.join("pep518_invalid_requires"), expect_error=True ) assert result.returncode == 1 assert "does not comply with PEP 518" in result.stderr +def test_pep518_allows_but_warns_missing_requires(script, data, common_wheels): + result = script.pip( + 'install', '-f', common_wheels, + data.src.join("pep518_missing_requires"), + expect_stderr=True + ) + assert "does not comply with PEP 518" in result.stderr + assert "DEPRECATION" in result.stderr + assert result.returncode == 0 + assert result.files_created + + def test_pep518_with_user_pip(script, virtualenv, pip_src, data, common_wheels): virtualenv.system_site_packages = True