Skip to content

Commit

Permalink
use import hooks to patch distutils/setuptools (pypa#1688)
Browse files Browse the repository at this point in the history
  • Loading branch information
gaborbernat authored Mar 4, 2020
1 parent c310a95 commit caae48b
Show file tree
Hide file tree
Showing 10 changed files with 119 additions and 98 deletions.
3 changes: 3 additions & 0 deletions docs/changelog/1663.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Having `distutils configuration <https://docs.python.org/3/install/index.html#distutils-configuration-files>`_
files that set ``prefix`` and ``install_scripts`` cause installation of packages in the wrong location -
by :user:`gaborbernat`.
2 changes: 2 additions & 0 deletions docs/changelog/1682.bugfix.rst
Original file line number Diff line number Diff line change
@@ -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`.
2 changes: 2 additions & 0 deletions docs/changelog/1684.bugfix.rst
Original file line number Diff line number Diff line change
@@ -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`.
1 change: 1 addition & 0 deletions docs/changelog/1685.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix ``attr:`` import fails for setuptools - by :user:`gaborbernat`.

This file was deleted.

99 changes: 99 additions & 0 deletions src/virtualenv/create/via_global_ref/_virtualenv.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
"""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.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
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):
module = super(_VirtualenvLoader, self).load_module(fullname)
patch_dist(module)
module.__loader__ = None # distlib fallback
return module

sys.meta_path.append(_VirtualenvImporter())
20 changes: 8 additions & 12 deletions src/virtualenv/create/via_global_ref/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
26 changes: 0 additions & 26 deletions src/virtualenv/create/via_global_ref/builtin/python2/site.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ def main():
load_host_site()
if global_site_package_enabled:
add_global_site_package()
fix_install()


def load_host_site():
Expand Down Expand Up @@ -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()
13 changes: 3 additions & 10 deletions tests/unit/create/test_creator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 / "venv")), "--activators", "", "--creator", creator])

app = Path(__file__).parent / "console_app"
dest = tmp_path / "console_app"
Expand All @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/seed/test_boostrap_link_via_app_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit caae48b

Please sign in to comment.