From b175e6f1fc98570416ab94388aad4c0b62fd5ec5 Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Tue, 3 Mar 2020 15:05:08 +0000 Subject: [PATCH 1/3] start creating import hooks Signed-off-by: Bernat Gabor --- docs/changelog/1663.bugfix.rst | 3 + docs/changelog/1682.bugfix.rst | 2 + docs/changelog/1684.bugfix.rst | 2 + docs/changelog/1685.bugfix.rst | 1 + .../_distutils_patch_virtualenv.py | 49 ------------ .../create/via_global_ref/_virtualenv.py | 78 +++++++++++++++++++ src/virtualenv/create/via_global_ref/api.py | 20 ++--- .../via_global_ref/builtin/python2/site.py | 26 ------- tests/unit/create/test_creator.py | 13 +--- .../seed/test_boostrap_link_via_app_data.py | 2 +- 10 files changed, 98 insertions(+), 98 deletions(-) create mode 100644 docs/changelog/1663.bugfix.rst create mode 100644 docs/changelog/1682.bugfix.rst create mode 100644 docs/changelog/1684.bugfix.rst create mode 100644 docs/changelog/1685.bugfix.rst delete mode 100644 src/virtualenv/create/via_global_ref/_distutils_patch_virtualenv.py create mode 100644 src/virtualenv/create/via_global_ref/_virtualenv.py diff --git a/docs/changelog/1663.bugfix.rst b/docs/changelog/1663.bugfix.rst new file mode 100644 index 000000000..a250e768c --- /dev/null +++ b/docs/changelog/1663.bugfix.rst @@ -0,0 +1,3 @@ +Having `distutils configuration `_ +files that set ``prefix`` and ``install_scripts`` cause installation of packages in the wrong location - +by :user:`gaborbernat`. diff --git a/docs/changelog/1682.bugfix.rst b/docs/changelog/1682.bugfix.rst new file mode 100644 index 000000000..496a87e75 --- /dev/null +++ b/docs/changelog/1682.bugfix.rst @@ -0,0 +1,2 @@ +Fix startup on Python 2 is slower for virtualenv - this was due to setuptools calculating it's working set distribution +- by :user:`gaborbernat`. diff --git a/docs/changelog/1684.bugfix.rst b/docs/changelog/1684.bugfix.rst new file mode 100644 index 000000000..1d9024763 --- /dev/null +++ b/docs/changelog/1684.bugfix.rst @@ -0,0 +1,2 @@ +Fix entry points are not populated for editable installs on Python 2 due to setuptools working set being calculated +before ``easy_install.pth`` runs - by :user:`gaborbernat`. diff --git a/docs/changelog/1685.bugfix.rst b/docs/changelog/1685.bugfix.rst new file mode 100644 index 000000000..145753147 --- /dev/null +++ b/docs/changelog/1685.bugfix.rst @@ -0,0 +1 @@ +Fix ``attr:`` import fails for setuptools - by :user:`gaborbernat`. diff --git a/src/virtualenv/create/via_global_ref/_distutils_patch_virtualenv.py b/src/virtualenv/create/via_global_ref/_distutils_patch_virtualenv.py deleted file mode 100644 index d963c435f..000000000 --- a/src/virtualenv/create/via_global_ref/_distutils_patch_virtualenv.py +++ /dev/null @@ -1,49 +0,0 @@ -# -*- coding: utf-8 -*- -""" -Distutils allows user to configure some arguments via a configuration file: -https://docs.python.org/3/install/index.html#distutils-configuration-files - -Some of this arguments though don't make sense in context of the virtual environment files, let's fix them up. -""" -import os -import sys - -VIRTUALENV_PATCH_FILE = os.path.join(__file__) - - -def patch(dist_of): - # we cannot allow the prefix override as that would get packages installed outside of the virtual environment - old_parse_config_files = dist_of.Distribution.parse_config_files - - def parse_config_files(self, *args, **kwargs): - result = old_parse_config_files(self, *args, **kwargs) - install_dict = self.get_option_dict("install") - - if "prefix" in install_dict: # the prefix governs where to install the libraries - install_dict["prefix"] = VIRTUALENV_PATCH_FILE, os.path.abspath(sys.prefix) - - if "install_scripts" in install_dict: # the install_scripts governs where to generate console scripts - script_path = os.path.abspath(os.path.join(os.path.dirname(__file__), "__SCRIPT_DIR__")) - install_dict["install_scripts"] = VIRTUALENV_PATCH_FILE, script_path - - return result - - dist_of.Distribution.parse_config_files = parse_config_files - - -def run(): - # patch distutils - from distutils import dist - - patch(dist) - - # patch setuptools (that has it's own copy of the dist package) - try: - from setuptools import dist - except ImportError: - pass # if setuptools is not around that's alright, just don't patch - else: - patch(dist) - - -run() diff --git a/src/virtualenv/create/via_global_ref/_virtualenv.py b/src/virtualenv/create/via_global_ref/_virtualenv.py new file mode 100644 index 000000000..8df743f43 --- /dev/null +++ b/src/virtualenv/create/via_global_ref/_virtualenv.py @@ -0,0 +1,78 @@ +"""Patches that are applied at runtime to the virtual environment""" +# -*- coding: utf-8 -*- + +import os +import sys + +VIRTUALENV_PATCH_FILE = os.path.join(__file__) + + +def patch_dist(dist): + """ + Distutils allows user to configure some arguments via a configuration file: + https://docs.python.org/3/install/index.html#distutils-configuration-files + + Some of this arguments though don't make sense in context of the virtual environment files, let's fix them up. + """ + # we cannot allow some install config as that would get packages installed outside of the virtual environment + old_parse_config_files = dist.Distribution.parse_config_files + + def parse_config_files(self, *args, **kwargs): + result = old_parse_config_files(self, *args, **kwargs) + install = self.get_option_dict("install") + + if "prefix" in install: # the prefix governs where to install the libraries + install["prefix"] = VIRTUALENV_PATCH_FILE, os.path.abspath(sys.prefix) + + if "install_scripts" in install: # the install_scripts governs where to generate console scripts + script_path = os.path.abspath(os.path.join(os.path.dirname(__file__), "__SCRIPT_DIR__")) + install["install_scripts"] = VIRTUALENV_PATCH_FILE, script_path + + return result + + dist.Distribution.parse_config_files = parse_config_files + + +# Import hook that patches some modules to ignore configuration values that break package installation in case +# of virtual environments. +_DISTUTILS_PATCH = "distutils.dist", "setuptools.dist" +if sys.version_info > (3, 4): + # https://docs.python.org/3/library/importlib.html#setting-up-an-importer + from importlib.machinery import SOURCE_SUFFIXES, FileFinder, SourceFileLoader + + class _VirtualEnvLoader(SourceFileLoader): + def exec_module(self, module): + super().exec_module(module) + if module.__name__ in _DISTUTILS_PATCH: + patch_dist(module) + + sys.path_hooks.insert(0, FileFinder.path_hook((_VirtualEnvLoader, SOURCE_SUFFIXES))) +else: + # https://www.python.org/dev/peps/pep-0302/ + from imp import find_module + from pkgutil import ImpImporter, ImpLoader + + class _VirtualenvImporter(object, ImpImporter): + def __init__(self, path=None): + object.__init__(self) + ImpImporter.__init__(self, path) + + def find_module(self, fullname, path=None): + if fullname in _DISTUTILS_PATCH: + try: + return _VirtualenvLoader(fullname, *find_module(fullname.split(".")[-1], path)) + except ImportError: + pass + return None + + class _VirtualenvLoader(object, ImpLoader): + def __init__(self, fullname, file, filename, etc): + object.__init__(self) + ImpLoader.__init__(self, fullname, file, filename, etc) + + def load_module(self, fullname): + result = super(_VirtualenvLoader, self).load_module(fullname) + patch_dist(result) + return result + + sys.meta_path.append(_VirtualenvImporter()) diff --git a/src/virtualenv/create/via_global_ref/api.py b/src/virtualenv/create/via_global_ref/api.py index 0d3907356..c01301ff3 100644 --- a/src/virtualenv/create/via_global_ref/api.py +++ b/src/virtualenv/create/via_global_ref/api.py @@ -53,19 +53,15 @@ def create(self): def patch_distutils_via_pth(self): """Patch the distutils package to not be derailed by its configuration files""" - if self.interpreter.version_info.major == 3: - return # TODO: remove this, for her to bypass: https://github.com/pypa/pip/issues/7778 - patch_file = Path(__file__).parent / "_distutils_patch_virtualenv.py" - with ensure_file_on_disk(patch_file, self.app_data) as resolved_path: + with ensure_file_on_disk(Path(__file__).parent / "_virtualenv.py", self.app_data) as resolved_path: text = resolved_path.read_text() - text = text.replace('"__SCRIPT_DIR__"', repr(os.path.relpath(str(self.script_dir), str(self.purelib)))) - patch_path = self.purelib / "_distutils_patch_virtualenv.py" - logging.debug("add distutils patch file %s", patch_path) - patch_path.write_text(text) - - pth = self.purelib / "_distutils_patch_virtualenv.pth" - logging.debug("add distutils patch file %s", pth) - pth.write_text("import _distutils_patch_virtualenv") + text = text.replace('"__SCRIPT_DIR__"', repr(os.path.relpath(str(self.script_dir), str(self.purelib)))) + dest_path = self.purelib / "_virtualenv.py" + logging.debug("create %s", dest_path) + dest_path.write_text(text) + pth = self.purelib / "_virtualenv.pth" + logging.debug("create virtualenv import hook file %s", pth) + pth.write_text("import _virtualenv") def _args(self): return super(ViaGlobalRefApi, self)._args() + [("global", self.enable_system_site_package)] diff --git a/src/virtualenv/create/via_global_ref/builtin/python2/site.py b/src/virtualenv/create/via_global_ref/builtin/python2/site.py index 80322ef02..366908e77 100644 --- a/src/virtualenv/create/via_global_ref/builtin/python2/site.py +++ b/src/virtualenv/create/via_global_ref/builtin/python2/site.py @@ -19,7 +19,6 @@ def main(): load_host_site() if global_site_package_enabled: add_global_site_package() - fix_install() def load_host_site(): @@ -163,29 +162,4 @@ def add_global_site_package(): site.PREFIXES = orig_prefixes -def fix_install(): - def patch(dist_of): - # we cannot allow the prefix override as that would get packages installed outside of the virtual environment - old_parse_config_files = dist_of.Distribution.parse_config_files - - def parse_config_files(self, *args, **kwargs): - result = old_parse_config_files(self, *args, **kwargs) - install_dict = self.get_option_dict("install") - if "prefix" in install_dict: - install_dict["prefix"] = "virtualenv.patch", abs_path(sys.prefix) - return result - - dist_of.Distribution.parse_config_files = parse_config_files - - from distutils import dist - - patch(dist) - try: - from setuptools import dist - - patch(dist) - except ImportError: - pass # if setuptools is not around that's alright, just don't patch - - main() diff --git a/tests/unit/create/test_creator.py b/tests/unit/create/test_creator.py index 6dd259d4d..34b90079f 100644 --- a/tests/unit/create/test_creator.py +++ b/tests/unit/create/test_creator.py @@ -137,7 +137,7 @@ def test_create_no_seed(python, creator, isolated, system, coverage_env, special # force a close of these on system where the limit is low-ish (e.g. MacOS 256) gc.collect() purelib = result.creator.purelib - patch_files = {purelib / "{}.{}".format("_distutils_patch_virtualenv", i) for i in ("py", "pyc", "pth")} + patch_files = {purelib / "{}.{}".format("_virtualenv", i) for i in ("py", "pyc", "pth")} patch_files.add(purelib / "__pycache__") content = set(result.creator.purelib.iterdir()) - patch_files assert not content, "\n".join(ensure_text(str(i)) for i in content) @@ -353,17 +353,9 @@ def test_create_long_path(current_fastest, tmp_path): subprocess.check_call([str(result.creator.script("pip")), "--version"]) -@pytest.mark.skipif(PY3, reason="https://github.com/pypa/pip/issues/7778") @pytest.mark.parametrize("creator", set(PythonInfo.current_system().creators().key_to_class) - {"builtin"}) def test_create_distutils_cfg(creator, tmp_path, monkeypatch): - cmd = [ - ensure_text(str(tmp_path)), - "--activators", - "", - "--creator", - creator, - ] - result = cli_run(cmd) + result = cli_run([ensure_text(str(tmp_path)), "--activators", "", "--creator", creator]) app = Path(__file__).parent / "console_app" dest = tmp_path / "console_app" @@ -380,6 +372,7 @@ def test_create_distutils_cfg(creator, tmp_path, monkeypatch): setup_cfg.write_text(setup_cfg.read_text() + conf) monkeypatch.chdir(dest) # distutils will read the setup.cfg from the cwd, so change to that + install_demo_cmd = [str(result.creator.script("pip")), "install", str(dest), "--no-use-pep517"] subprocess.check_call(install_demo_cmd) diff --git a/tests/unit/seed/test_boostrap_link_via_app_data.py b/tests/unit/seed/test_boostrap_link_via_app_data.py index 3c4c6697a..82c49ae41 100644 --- a/tests/unit/seed/test_boostrap_link_via_app_data.py +++ b/tests/unit/seed/test_boostrap_link_via_app_data.py @@ -96,7 +96,7 @@ def test_base_bootstrap_link_via_app_data(tmp_path, coverage_env, current_fastes # pip is greedy here, removing all packages removes the site-package too if site_package.exists(): purelib = result.creator.purelib - patch_files = {purelib / "{}.{}".format("_distutils_patch_virtualenv", i) for i in ("py", "pyc", "pth")} + patch_files = {purelib / "{}.{}".format("_virtualenv", i) for i in ("py", "pyc", "pth")} patch_files.add(purelib / "__pycache__") post_run = set(site_package.iterdir()) - patch_files assert not post_run, "\n".join(str(i) for i in post_run) From 78ab88d7744f28787eec423d197192efea34aeb5 Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Wed, 4 Mar 2020 10:31:44 +0000 Subject: [PATCH 2/3] fix distlib Signed-off-by: Bernat Gabor --- src/virtualenv/create/via_global_ref/_virtualenv.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/virtualenv/create/via_global_ref/_virtualenv.py b/src/virtualenv/create/via_global_ref/_virtualenv.py index 8df743f43..fe1c6151c 100644 --- a/src/virtualenv/create/via_global_ref/_virtualenv.py +++ b/src/virtualenv/create/via_global_ref/_virtualenv.py @@ -45,6 +45,7 @@ def exec_module(self, module): super().exec_module(module) if module.__name__ in _DISTUTILS_PATCH: patch_dist(module) + module.__loader__ = None # distlib fallback sys.path_hooks.insert(0, FileFinder.path_hook((_VirtualEnvLoader, SOURCE_SUFFIXES))) else: @@ -71,8 +72,9 @@ def __init__(self, fullname, file, filename, etc): ImpLoader.__init__(self, fullname, file, filename, etc) def load_module(self, fullname): - result = super(_VirtualenvLoader, self).load_module(fullname) - patch_dist(result) - return result + module = super(_VirtualenvLoader, self).load_module(fullname) + patch_dist(module) + module.__loader__ = None # distlib fallback + return module sys.meta_path.append(_VirtualenvImporter()) From c429b9ac2ceda9c98857c4f86be591b4f7e9b053 Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Wed, 4 Mar 2020 12:12:04 +0000 Subject: [PATCH 3/3] fix pypy3 Signed-off-by: Bernat Gabor --- .../create/via_global_ref/_virtualenv.py | 39 ++++++++++++++----- tests/unit/create/test_creator.py | 2 +- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/virtualenv/create/via_global_ref/_virtualenv.py b/src/virtualenv/create/via_global_ref/_virtualenv.py index fe1c6151c..f12d7e42e 100644 --- a/src/virtualenv/create/via_global_ref/_virtualenv.py +++ b/src/virtualenv/create/via_global_ref/_virtualenv.py @@ -38,16 +38,35 @@ def parse_config_files(self, *args, **kwargs): _DISTUTILS_PATCH = "distutils.dist", "setuptools.dist" if sys.version_info > (3, 4): # https://docs.python.org/3/library/importlib.html#setting-up-an-importer - from importlib.machinery import SOURCE_SUFFIXES, FileFinder, SourceFileLoader - - class _VirtualEnvLoader(SourceFileLoader): - def exec_module(self, module): - super().exec_module(module) - if module.__name__ in _DISTUTILS_PATCH: - patch_dist(module) - module.__loader__ = None # distlib fallback - - sys.path_hooks.insert(0, FileFinder.path_hook((_VirtualEnvLoader, SOURCE_SUFFIXES))) + from importlib.abc import MetaPathFinder + from importlib.util import find_spec + from threading import Lock + + class _Finder(MetaPathFinder): + """A meta path finder that allows patching the imported distutils modules""" + + fullname = None + lock = Lock() + + def find_spec(self, fullname, path, target=None): + if fullname in _DISTUTILS_PATCH and self.fullname is None: + with self.lock: + self.fullname = fullname + try: + spec = find_spec(fullname, path) + if spec is not None: + old = spec.loader.exec_module + + def exec_module(module): + old(module) + patch_dist(module) + + spec.loader.exec_module = exec_module + return spec + finally: + self.fullname = None + + sys.meta_path.insert(0, _Finder()) else: # https://www.python.org/dev/peps/pep-0302/ from imp import find_module diff --git a/tests/unit/create/test_creator.py b/tests/unit/create/test_creator.py index 34b90079f..da2855aec 100644 --- a/tests/unit/create/test_creator.py +++ b/tests/unit/create/test_creator.py @@ -355,7 +355,7 @@ def test_create_long_path(current_fastest, tmp_path): @pytest.mark.parametrize("creator", set(PythonInfo.current_system().creators().key_to_class) - {"builtin"}) def test_create_distutils_cfg(creator, tmp_path, monkeypatch): - result = cli_run([ensure_text(str(tmp_path)), "--activators", "", "--creator", creator]) + result = cli_run([ensure_text(str(tmp_path / "venv")), "--activators", "", "--creator", creator]) app = Path(__file__).parent / "console_app" dest = tmp_path / "console_app"