Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce dependency on ctypes when discovering glibc version. #6678

Merged
merged 9 commits into from
Jul 21, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions src/pip/_internal/utils/glibc.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,32 @@
from typing import Optional, Tuple


def glibc_version_string():
def glibc_version_string_os():
pradyunsg marked this conversation as resolved.
Show resolved Hide resolved
# type: () -> Optional[str]
"Returns glibc version string, or None if not using glibc."
"""Returns glibc version string, or None if not using glibc.

try: # https://github.com/pypa/pip/issues/6675#issue-463147612
glibc_version = os.confstr("CS_GNU_LIBC_VERSION").split()
This should be paired with glibc_version_string_ctypes as a fallback:
pradyunsg marked this conversation as resolved.
Show resolved Hide resolved
glibc_ver = glibc_version_string_os() or glibc_version_string_ctypes()
"""

# os.confstr is quite a bit faster than ctypes.DLL.
# It's also less likely to be broken or missing.

# This strategy is used in the standard library platform module:
# https://github.com/python/cpython/blob/fcf1d003bf4f0100c9d0921ff3d70e1127ca1b71/Lib/platform.py#L175-L183

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a quick note: you don't need any of these three empty lines. Also, the three sentences can follow each other like a paragraph, with the link on its own line at the end.

try:
# os.confstr("CS_GNU_LIBC_VERSION") returns a string like "glibc 2.17":
_, version = os.confstr("CS_GNU_LIBC_VERSION").split()
except (AttributeError, OSError, ValueError):
pass # os.confstr() or CS_GNU_LIBC_VERSION not available...
else:
if len(glibc_version) == 2:
return glibc_version[-1]
# os.confstr() or CS_GNU_LIBC_VERSION not available (or a bad value)...
return None
return version


def glibc_version_string_ctypes():
# type: () -> Optional[str]
"Fallback ctypes implementation of glibc_version_string_os."

try:
pradyunsg marked this conversation as resolved.
Show resolved Hide resolved
import ctypes
Expand Down Expand Up @@ -69,7 +84,7 @@ def check_glibc_version(version_str, required_major, minimum_minor):

def have_compatible_glibc(required_major, minimum_minor):
# type: (int, int) -> bool
version_str = glibc_version_string() # type: Optional[str]
version_str = glibc_version_string_os() or glibc_version_string_ctypes()
if version_str is None:
return False
return check_glibc_version(version_str, required_major, minimum_minor)
Expand Down Expand Up @@ -99,7 +114,7 @@ def libc_ver():
Returns a tuple of strings (lib, version) which default to empty strings
in case the lookup fails.
"""
glibc_version = glibc_version_string()
glibc_version = glibc_version_string_os() or glibc_version_string_ctypes()
if glibc_version is None:
return ("", "")
else:
Expand Down
28 changes: 27 additions & 1 deletion tests/unit/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
)
from pip._internal.utils.deprecation import PipDeprecationWarning, deprecated
from pip._internal.utils.encoding import BOMS, auto_decode
from pip._internal.utils.glibc import check_glibc_version
from pip._internal.utils.glibc import (
check_glibc_version, glibc_version_string_ctypes, glibc_version_string_os,
)
from pip._internal.utils.hashes import Hashes, MissingHashes
from pip._internal.utils.misc import (
call_subprocess, egg_link_path, ensure_dir, format_command_args,
Expand Down Expand Up @@ -701,6 +703,30 @@ def test_manylinux_check_glibc_version(self):
# Didn't find the warning we were expecting
assert False

def test_glibc_version_string_os_fail(self, monkeypatch):

if hasattr(os, "confstr"):
pradyunsg marked this conversation as resolved.
Show resolved Hide resolved

def raises(error):
raise error()

monkeypatch.setattr(os, "confstr", lambda x: raises(ValueError))
assert glibc_version_string_os() is None

monkeypatch.setattr(os, "confstr", lambda x: raises(OSError))
assert glibc_version_string_os() is None

monkeypatch.setattr(os, "confstr", lambda x: "XXX")
assert glibc_version_string_os() is None

monkeypatch.delattr(os, "confstr")

assert glibc_version_string_os() is None

def test_glibc_version_string_ctypes_fail(self, monkeypatch):
monkeypatch.setitem(sys.modules, "ctypes", None)
assert glibc_version_string_ctypes() is None


@pytest.mark.parametrize('version_info, expected', [
(None, None),
Expand Down