From 4ec34b9013ccbb322120a1561520cf4fab31c7f7 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Fri, 5 Mar 2021 17:29:21 +0800 Subject: [PATCH 01/10] Run pip in isolated env by building zip --- news/8214.bugfix.rst | 2 + src/pip/_internal/build_env.py | 100 ++++++++++++++++++++++----------- 2 files changed, 69 insertions(+), 33 deletions(-) create mode 100644 news/8214.bugfix.rst diff --git a/news/8214.bugfix.rst b/news/8214.bugfix.rst new file mode 100644 index 00000000000..22224f380e5 --- /dev/null +++ b/news/8214.bugfix.rst @@ -0,0 +1,2 @@ +Prevent packages already-installed alongside with pip to be injected into an +isolated build environment during build-time dependency population. diff --git a/src/pip/_internal/build_env.py b/src/pip/_internal/build_env.py index fa22d6377c6..fc8a0f1e2c2 100644 --- a/src/pip/_internal/build_env.py +++ b/src/pip/_internal/build_env.py @@ -1,14 +1,17 @@ """Build Environment used for isolation during sdist building """ +import contextlib import logging import os +import pathlib import sys import textwrap +import zipfile from collections import OrderedDict from sysconfig import get_paths from types import TracebackType -from typing import TYPE_CHECKING, Iterable, List, Optional, Set, Tuple, Type +from typing import TYPE_CHECKING, Iterable, Iterator, List, Optional, Set, Tuple, Type from pip._vendor.pkg_resources import Requirement, VersionConflict, WorkingSet @@ -37,6 +40,61 @@ def __init__(self, path): self.lib_dirs = get_prefixed_libs(path) +@contextlib.contextmanager +def _create_standalone_pip() -> Iterator[str]: + """Create a zip file containing specified pip installation.""" + source = pathlib.Path(pip_location).resolve().parent + with TempDirectory() as tmp_dir: + pip_zip = os.path.join(tmp_dir.path, "pip.zip") + with zipfile.ZipFile(pip_zip, "w") as zf: + for child in source.rglob("*"): + arcname = child.relative_to(source.parent) + zf.write(child, arcname.as_posix()) + yield os.path.join(pip_zip, "pip") + + +def _install_requirements( + standalone_pip: str, + finder: "PackageFinder", + requirements: Iterable[str], + prefix: _Prefix, + message: str, +) -> None: + args = [ + sys.executable, standalone_pip, 'install', + '--ignore-installed', '--no-user', '--prefix', prefix.path, + '--no-warn-script-location', + ] # type: List[str] + if logger.getEffectiveLevel() <= logging.DEBUG: + args.append('-v') + for format_control in ('no_binary', 'only_binary'): + formats = getattr(finder.format_control, format_control) + args += [ + '--' + format_control.replace('_', '-'), + ','.join(sorted(formats or {':none:'})) + ] + index_urls = finder.index_urls + if index_urls: + args.extend(['-i', index_urls[0]]) + for extra_index in index_urls[1:]: + args.extend(['--extra-index-url', extra_index]) + else: + args.append('--no-index') + for link in finder.find_links: + args.extend(['--find-links', link]) + + for host in finder.trusted_hosts: + args.extend(['--trusted-host', host]) + if finder.allow_all_prereleases: + args.append('--pre') + if finder.prefer_binary: + args.append('--prefer-binary') + args.append('--') + args.extend(requirements) + with open_spinner(message) as spinner: + call_subprocess(args, spinner=spinner) + + class BuildEnvironment: """Creates and manages an isolated environment to install build deps """ @@ -160,38 +218,14 @@ def install_requirements( prefix.setup = True if not requirements: return - args = [ - sys.executable, os.path.dirname(pip_location), 'install', - '--ignore-installed', '--no-user', '--prefix', prefix.path, - '--no-warn-script-location', - ] # type: List[str] - if logger.getEffectiveLevel() <= logging.DEBUG: - args.append('-v') - for format_control in ('no_binary', 'only_binary'): - formats = getattr(finder.format_control, format_control) - args.extend(('--' + format_control.replace('_', '-'), - ','.join(sorted(formats or {':none:'})))) - - index_urls = finder.index_urls - if index_urls: - args.extend(['-i', index_urls[0]]) - for extra_index in index_urls[1:]: - args.extend(['--extra-index-url', extra_index]) - else: - args.append('--no-index') - for link in finder.find_links: - args.extend(['--find-links', link]) - - for host in finder.trusted_hosts: - args.extend(['--trusted-host', host]) - if finder.allow_all_prereleases: - args.append('--pre') - if finder.prefer_binary: - args.append('--prefer-binary') - args.append('--') - args.extend(requirements) - with open_spinner(message) as spinner: - call_subprocess(args, spinner=spinner) + with _create_standalone_pip() as standalone_pip: + _install_requirements( + standalone_pip, + finder, + requirements, + prefix, + message, + ) class NoOpBuildEnvironment(BuildEnvironment): From 638b562048f033cd79b236e673cba09202a396b9 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Fri, 5 Mar 2021 22:57:45 +0800 Subject: [PATCH 02/10] Pass parent certificate location to isolated pip --- src/pip/_internal/build_env.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pip/_internal/build_env.py b/src/pip/_internal/build_env.py index fc8a0f1e2c2..0067d840536 100644 --- a/src/pip/_internal/build_env.py +++ b/src/pip/_internal/build_env.py @@ -13,6 +13,7 @@ from types import TracebackType from typing import TYPE_CHECKING, Iterable, Iterator, List, Optional, Set, Tuple, Type +from pip._vendor.certifi import where from pip._vendor.pkg_resources import Requirement, VersionConflict, WorkingSet from pip import __file__ as pip_location @@ -63,7 +64,7 @@ def _install_requirements( args = [ sys.executable, standalone_pip, 'install', '--ignore-installed', '--no-user', '--prefix', prefix.path, - '--no-warn-script-location', + '--cert', where(), '--no-warn-script-location', ] # type: List[str] if logger.getEffectiveLevel() <= logging.DEBUG: args.append('-v') @@ -82,7 +83,6 @@ def _install_requirements( args.append('--no-index') for link in finder.find_links: args.extend(['--find-links', link]) - for host in finder.trusted_hosts: args.extend(['--trusted-host', host]) if finder.allow_all_prereleases: From e8e5153612287d9114ec1a3fdac719ebd02826d3 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Sat, 6 Mar 2021 02:26:56 +0800 Subject: [PATCH 03/10] Better name temp directory --- src/pip/_internal/build_env.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pip/_internal/build_env.py b/src/pip/_internal/build_env.py index 0067d840536..7c07f8e256e 100644 --- a/src/pip/_internal/build_env.py +++ b/src/pip/_internal/build_env.py @@ -45,7 +45,7 @@ def __init__(self, path): def _create_standalone_pip() -> Iterator[str]: """Create a zip file containing specified pip installation.""" source = pathlib.Path(pip_location).resolve().parent - with TempDirectory() as tmp_dir: + with TempDirectory(kind="standalone-pip") as tmp_dir: pip_zip = os.path.join(tmp_dir.path, "pip.zip") with zipfile.ZipFile(pip_zip, "w") as zf: for child in source.rglob("*"): From 7067359751247a6c4a2f5f3f8d91c21e45ef009c Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Sat, 6 Mar 2021 02:27:40 +0800 Subject: [PATCH 04/10] Less diff --- src/pip/_internal/build_env.py | 84 +++++++++++++++++----------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/src/pip/_internal/build_env.py b/src/pip/_internal/build_env.py index 7c07f8e256e..a9e6262457f 100644 --- a/src/pip/_internal/build_env.py +++ b/src/pip/_internal/build_env.py @@ -54,47 +54,6 @@ def _create_standalone_pip() -> Iterator[str]: yield os.path.join(pip_zip, "pip") -def _install_requirements( - standalone_pip: str, - finder: "PackageFinder", - requirements: Iterable[str], - prefix: _Prefix, - message: str, -) -> None: - args = [ - sys.executable, standalone_pip, 'install', - '--ignore-installed', '--no-user', '--prefix', prefix.path, - '--cert', where(), '--no-warn-script-location', - ] # type: List[str] - if logger.getEffectiveLevel() <= logging.DEBUG: - args.append('-v') - for format_control in ('no_binary', 'only_binary'): - formats = getattr(finder.format_control, format_control) - args += [ - '--' + format_control.replace('_', '-'), - ','.join(sorted(formats or {':none:'})) - ] - index_urls = finder.index_urls - if index_urls: - args.extend(['-i', index_urls[0]]) - for extra_index in index_urls[1:]: - args.extend(['--extra-index-url', extra_index]) - else: - args.append('--no-index') - for link in finder.find_links: - args.extend(['--find-links', link]) - for host in finder.trusted_hosts: - args.extend(['--trusted-host', host]) - if finder.allow_all_prereleases: - args.append('--pre') - if finder.prefer_binary: - args.append('--prefer-binary') - args.append('--') - args.extend(requirements) - with open_spinner(message) as spinner: - call_subprocess(args, spinner=spinner) - - class BuildEnvironment: """Creates and manages an isolated environment to install build deps """ @@ -219,7 +178,7 @@ def install_requirements( if not requirements: return with _create_standalone_pip() as standalone_pip: - _install_requirements( + self._install_requirements( standalone_pip, finder, requirements, @@ -227,6 +186,47 @@ def install_requirements( message, ) + @staticmethod + def _install_requirements( + standalone_pip: str, + finder: "PackageFinder", + requirements: Iterable[str], + prefix: _Prefix, + message: str, + ) -> None: + args = [ + sys.executable, standalone_pip, 'install', + '--ignore-installed', '--no-user', '--prefix', prefix.path, + '--no-warn-script-location', '--cert', where(), + ] # type: List[str] + if logger.getEffectiveLevel() <= logging.DEBUG: + args.append('-v') + for format_control in ('no_binary', 'only_binary'): + formats = getattr(finder.format_control, format_control) + args.extend(('--' + format_control.replace('_', '-'), + ','.join(sorted(formats or {':none:'})))) + + index_urls = finder.index_urls + if index_urls: + args.extend(['-i', index_urls[0]]) + for extra_index in index_urls[1:]: + args.extend(['--extra-index-url', extra_index]) + else: + args.append('--no-index') + for link in finder.find_links: + args.extend(['--find-links', link]) + + for host in finder.trusted_hosts: + args.extend(['--trusted-host', host]) + if finder.allow_all_prereleases: + args.append('--pre') + if finder.prefer_binary: + args.append('--prefer-binary') + args.append('--') + args.extend(requirements) + with open_spinner(message) as spinner: + call_subprocess(args, spinner=spinner) + class NoOpBuildEnvironment(BuildEnvironment): """A no-op drop-in replacement for BuildEnvironment From 4bf083cde6a88359446f76da53e1a942eb00058e Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Sat, 6 Mar 2021 04:40:14 +0800 Subject: [PATCH 05/10] Monkey-patch .pem path into standalone pip instead --- src/pip/_internal/build_env.py | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/pip/_internal/build_env.py b/src/pip/_internal/build_env.py index a9e6262457f..0b5ee7e2f38 100644 --- a/src/pip/_internal/build_env.py +++ b/src/pip/_internal/build_env.py @@ -41,6 +41,28 @@ def __init__(self, path): self.lib_dirs = get_prefixed_libs(path) +_CERTIFI_WHERE_PATCH = """ +from pip._vendor import certifi +certifi.where = lambda: {pem!r} +""" + + +def _format_main_py(source: pathlib.Path) -> bytes: + """Create a patched pip/__main__.py for the standalone pip. + + The default ``certifi.where()`` relies on the certificate bundle being a + real physical file on-disk, so we monkey-patch it to return the one used + by this process instead. + + Passing ``--cert`` to the standalone pip does not work, since ``requests`` + calls ``where()`` unconditionally on import. + """ + with source.open("rb") as f: + content = f.read() + patch = _CERTIFI_WHERE_PATCH.format(pem=where()).encode("utf-8") + return patch + content + + @contextlib.contextmanager def _create_standalone_pip() -> Iterator[str]: """Create a zip file containing specified pip installation.""" @@ -49,8 +71,11 @@ def _create_standalone_pip() -> Iterator[str]: pip_zip = os.path.join(tmp_dir.path, "pip.zip") with zipfile.ZipFile(pip_zip, "w") as zf: for child in source.rglob("*"): - arcname = child.relative_to(source.parent) - zf.write(child, arcname.as_posix()) + arcname = child.relative_to(source.parent).as_posix() + if arcname == "pip/__main__.py": + zf.writestr(arcname, _format_main_py(child)) + else: + zf.write(child, arcname) yield os.path.join(pip_zip, "pip") @@ -197,7 +222,7 @@ def _install_requirements( args = [ sys.executable, standalone_pip, 'install', '--ignore-installed', '--no-user', '--prefix', prefix.path, - '--no-warn-script-location', '--cert', where(), + '--no-warn-script-location', ] # type: List[str] if logger.getEffectiveLevel() <= logging.DEBUG: args.append('-v') From 0d183d3ee3474e2679663833e323c100c46403ef Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Sat, 6 Mar 2021 05:01:12 +0800 Subject: [PATCH 06/10] Better docstring --- src/pip/_internal/build_env.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/pip/_internal/build_env.py b/src/pip/_internal/build_env.py index 0b5ee7e2f38..45670734f0e 100644 --- a/src/pip/_internal/build_env.py +++ b/src/pip/_internal/build_env.py @@ -65,7 +65,11 @@ def _format_main_py(source: pathlib.Path) -> bytes: @contextlib.contextmanager def _create_standalone_pip() -> Iterator[str]: - """Create a zip file containing specified pip installation.""" + """Create a "standalone pip" zip file. + + The zip file contains a (modified) copy of the pip currently running. + It will be used to install requirements into the build environment. + """ source = pathlib.Path(pip_location).resolve().parent with TempDirectory(kind="standalone-pip") as tmp_dir: pip_zip = os.path.join(tmp_dir.path, "pip.zip") From 40d0529ee3f6e8a602ddc27d7d636c25108a63cd Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Sat, 6 Mar 2021 17:35:34 +0800 Subject: [PATCH 07/10] Patch __init__.py instead of __main__.py The standalone pip doesn't have a correct sys.path when __main__.py is invoked, and we'd be patching the wrong certifi at that point. The sys.path is guaranteed to be correct when __init__.py is loaded (otherwise we wouldn't be able to import it in the first place). --- src/pip/_internal/build_env.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pip/_internal/build_env.py b/src/pip/_internal/build_env.py index 45670734f0e..85691a1f2e2 100644 --- a/src/pip/_internal/build_env.py +++ b/src/pip/_internal/build_env.py @@ -47,8 +47,8 @@ def __init__(self, path): """ -def _format_main_py(source: pathlib.Path) -> bytes: - """Create a patched pip/__main__.py for the standalone pip. +def _format_init_py(source: pathlib.Path) -> bytes: + """Create a patched pip/__init__.py for the standalone pip. The default ``certifi.where()`` relies on the certificate bundle being a real physical file on-disk, so we monkey-patch it to return the one used @@ -76,8 +76,8 @@ def _create_standalone_pip() -> Iterator[str]: with zipfile.ZipFile(pip_zip, "w") as zf: for child in source.rglob("*"): arcname = child.relative_to(source.parent).as_posix() - if arcname == "pip/__main__.py": - zf.writestr(arcname, _format_main_py(child)) + if arcname == "pip/__init__.py": + zf.writestr(arcname, _format_init_py(child)) else: zf.write(child, arcname) yield os.path.join(pip_zip, "pip") From 2eb7e887ffb291c7eca279fccf9789e64a50b9a0 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Sat, 6 Mar 2021 20:52:13 +0800 Subject: [PATCH 08/10] Patch certifi to use environ for passing cert --- src/pip/_vendor/certifi/core.py | 16 +++++++++++++ tools/vendoring/patches/certifi.patch | 34 ++++++++++++++++++++++++--- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/pip/_vendor/certifi/core.py b/src/pip/_vendor/certifi/core.py index 8987449f6b5..b8140cf1ae7 100644 --- a/src/pip/_vendor/certifi/core.py +++ b/src/pip/_vendor/certifi/core.py @@ -8,7 +8,21 @@ """ import os + +class _PipPatchedCertificate(Exception): + pass + + try: + # Return a certificate file on disk for a standalone pip zipapp running in + # an isolated build environment to use. Passing --cert to the standalone + # pip does not work since requests calls where() unconditionally on import. + _PIP_STANDALONE_CERT = os.environ.get("_PIP_STANDALONE_CERT") + if _PIP_STANDALONE_CERT: + def where(): + return _PIP_STANDALONE_CERT + raise _PipPatchedCertificate() + from importlib.resources import path as get_path, read_text _CACERT_CTX = None @@ -38,6 +52,8 @@ def where(): return _CACERT_PATH +except _PipPatchedCertificate: + pass except ImportError: # This fallback will work for Python versions prior to 3.7 that lack the diff --git a/tools/vendoring/patches/certifi.patch b/tools/vendoring/patches/certifi.patch index 9d5395a7b6b..a36a0020ff5 100644 --- a/tools/vendoring/patches/certifi.patch +++ b/tools/vendoring/patches/certifi.patch @@ -1,13 +1,41 @@ diff --git a/src/pip/_vendor/certifi/core.py b/src/pip/_vendor/certifi/core.py -index 5d2b8cd32..8987449f6 100644 +index 5d2b8cd32..b8140cf1a 100644 --- a/src/pip/_vendor/certifi/core.py +++ b/src/pip/_vendor/certifi/core.py -@@ -33,7 +33,7 @@ try: +@@ -8,7 +8,21 @@ This module returns the installation location of cacert.pem or its contents. + """ + import os + ++ ++class _PipPatchedCertificate(Exception): ++ pass ++ ++ + try: ++ # Return a certificate file on disk for a standalone pip zipapp running in ++ # an isolated build environment to use. Passing --cert to the standalone ++ # pip does not work since requests calls where() unconditionally on import. ++ _PIP_STANDALONE_CERT = os.environ.get("_PIP_STANDALONE_CERT") ++ if _PIP_STANDALONE_CERT: ++ def where(): ++ return _PIP_STANDALONE_CERT ++ raise _PipPatchedCertificate() ++ + from importlib.resources import path as get_path, read_text + + _CACERT_CTX = None +@@ -33,11 +47,13 @@ try: # We also have to hold onto the actual context manager, because # it will do the cleanup whenever it gets garbage collected, so # we will also store that at the global level as well. - _CACERT_CTX = get_path("certifi", "cacert.pem") + _CACERT_CTX = get_path("pip._vendor.certifi", "cacert.pem") _CACERT_PATH = str(_CACERT_CTX.__enter__()) - + return _CACERT_PATH + ++except _PipPatchedCertificate: ++ pass + + except ImportError: + # This fallback will work for Python versions prior to 3.7 that lack the From 9cab77eb17c3ed32298d554e5283bbe82d6f9579 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Sat, 6 Mar 2021 20:55:02 +0800 Subject: [PATCH 09/10] Pass in cert path with private environ --- src/pip/_internal/build_env.py | 31 +++---------------------------- 1 file changed, 3 insertions(+), 28 deletions(-) diff --git a/src/pip/_internal/build_env.py b/src/pip/_internal/build_env.py index 85691a1f2e2..82bb7295da0 100644 --- a/src/pip/_internal/build_env.py +++ b/src/pip/_internal/build_env.py @@ -41,28 +41,6 @@ def __init__(self, path): self.lib_dirs = get_prefixed_libs(path) -_CERTIFI_WHERE_PATCH = """ -from pip._vendor import certifi -certifi.where = lambda: {pem!r} -""" - - -def _format_init_py(source: pathlib.Path) -> bytes: - """Create a patched pip/__init__.py for the standalone pip. - - The default ``certifi.where()`` relies on the certificate bundle being a - real physical file on-disk, so we monkey-patch it to return the one used - by this process instead. - - Passing ``--cert`` to the standalone pip does not work, since ``requests`` - calls ``where()`` unconditionally on import. - """ - with source.open("rb") as f: - content = f.read() - patch = _CERTIFI_WHERE_PATCH.format(pem=where()).encode("utf-8") - return patch + content - - @contextlib.contextmanager def _create_standalone_pip() -> Iterator[str]: """Create a "standalone pip" zip file. @@ -75,11 +53,7 @@ def _create_standalone_pip() -> Iterator[str]: pip_zip = os.path.join(tmp_dir.path, "pip.zip") with zipfile.ZipFile(pip_zip, "w") as zf: for child in source.rglob("*"): - arcname = child.relative_to(source.parent).as_posix() - if arcname == "pip/__init__.py": - zf.writestr(arcname, _format_init_py(child)) - else: - zf.write(child, arcname) + zf.write(child, child.relative_to(source.parent).as_posix()) yield os.path.join(pip_zip, "pip") @@ -253,8 +227,9 @@ def _install_requirements( args.append('--prefer-binary') args.append('--') args.extend(requirements) + extra_environ = {"_PIP_STANDALONE_CERT": where()} with open_spinner(message) as spinner: - call_subprocess(args, spinner=spinner) + call_subprocess(args, spinner=spinner, extra_environ=extra_environ) class NoOpBuildEnvironment(BuildEnvironment): From bba1226a031d44cd745dca11610113aba6bb5099 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Sat, 6 Mar 2021 21:10:06 +0800 Subject: [PATCH 10/10] Handle zipapp inception --- src/pip/_internal/build_env.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/pip/_internal/build_env.py b/src/pip/_internal/build_env.py index 82bb7295da0..755dbb1f712 100644 --- a/src/pip/_internal/build_env.py +++ b/src/pip/_internal/build_env.py @@ -45,12 +45,19 @@ def __init__(self, path): def _create_standalone_pip() -> Iterator[str]: """Create a "standalone pip" zip file. - The zip file contains a (modified) copy of the pip currently running. + The zip file's content is identical to the currently-running pip. It will be used to install requirements into the build environment. """ source = pathlib.Path(pip_location).resolve().parent + + # Return the current instance if it is already a zip file. This can happen + # if a PEP 517 requirement is an sdist itself. + if not source.is_dir() and source.parent.name == "__env_pip__.zip": + yield str(source) + return + with TempDirectory(kind="standalone-pip") as tmp_dir: - pip_zip = os.path.join(tmp_dir.path, "pip.zip") + pip_zip = os.path.join(tmp_dir.path, "__env_pip__.zip") with zipfile.ZipFile(pip_zip, "w") as zf: for child in source.rglob("*"): zf.write(child, child.relative_to(source.parent).as_posix())