-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Implement pep 503 data-requires-python #3877
Changes from 6 commits
c210198
dbcefb7
86b80cc
f7373a4
17d830f
4c31a55
1d10fca
d4e22ea
bfd8794
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
) | ||
from pip.utils.deprecation import RemovedInPip9Warning, RemovedInPip10Warning | ||
from pip.utils.logging import indent_log | ||
from pip.utils.packaging import check_requires_python | ||
from pip.exceptions import ( | ||
DistributionNotFound, BestVersionAlreadyInstalled, InvalidWheelFilename, | ||
UnsupportedWheel, | ||
|
@@ -33,6 +34,7 @@ | |
from pip._vendor.packaging.version import parse as parse_version | ||
from pip._vendor.packaging.utils import canonicalize_name | ||
from pip._vendor.requests.exceptions import SSLError | ||
from pip._vendor.distlib.compat import unescape | ||
|
||
|
||
__all__ = ['FormatControl', 'fmt_ctl_handle_mutual_exclude', 'PackageFinder'] | ||
|
@@ -640,6 +642,12 @@ def _link_package_versions(self, link, search): | |
self._log_skipped_link( | ||
link, 'Python version is incorrect') | ||
return | ||
|
||
if not check_requires_python(link.requires_python): | ||
logger.debug("The package %s is incompatible with the python" | ||
"version in use. Acceptable python versions are:%s", | ||
link, link.requires_python) | ||
return | ||
logger.debug('Found link %s, version: %s', link, version) | ||
|
||
return InstallationCandidate(search.supplied, version, link) | ||
|
@@ -828,7 +836,8 @@ def links(self): | |
url = self.clean_link( | ||
urllib_parse.urljoin(self.base_url, href) | ||
) | ||
yield Link(url, self) | ||
pyrequire = anchor.get('data-requires-python') | ||
yield Link(url, self, requires_python=pyrequire) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would put the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
||
_clean_re = re.compile(r'[^a-z0-9$&+,/:;=?@.#%_\\|-]', re.I) | ||
|
||
|
@@ -842,18 +851,39 @@ def clean_link(self, url): | |
|
||
class Link(object): | ||
|
||
def __init__(self, url, comes_from=None): | ||
def __init__(self, url, comes_from=None, requires_python=None): | ||
""" | ||
Object representing a parsed link from https://pypi.python.org/simple/* | ||
|
||
url: | ||
url of the resource pointed to (href of the link) | ||
comes_form: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. form -> from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed. |
||
<Not sure> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at the code that instantiates it, it appears that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or a string sometimes 😢 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
requires_python: | ||
String containing the `Requires-Python` metadata field, specified | ||
in PEP 345. This is to understand pep 503. The `requires_python` | ||
string will be unescaped as pep 503 requires `<` and `>` to be | ||
escaped, then stored under the `requires_python` attribute. | ||
""" | ||
|
||
# url can be a UNC windows share | ||
if url.startswith('\\\\'): | ||
url = path_to_url(url) | ||
|
||
self.url = url | ||
self.comes_from = comes_from | ||
if not requires_python: | ||
self.requires_python = None | ||
else: | ||
self.requires_python = unescape(requires_python) | ||
|
||
def __str__(self): | ||
if self.requires_python: | ||
rp = ' (requires-python:%s)' % self.requires_python | ||
else: | ||
rp = '' | ||
if self.comes_from: | ||
return '%s (from %s)' % (self.url, self.comes_from) | ||
return '%s (from %s)%s' % (self.url, self.comes_from, rp) | ||
else: | ||
return str(self.url) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
from __future__ import absolute_import | ||
import logging | ||
import sys | ||
|
||
from pip._vendor.packaging import specifiers | ||
from pip._vendor.packaging import version | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
def check_requires_python(requires_python): | ||
""" | ||
Check if the python version in used match the `requires_python` specifier passed. | ||
|
||
Return `True` if the version of python in use matches the requirement. | ||
Return `False` if the version of python in use does not matches the requirement. | ||
Raises an InvalidSpecifier if `requires_python` have an invalid format. | ||
""" | ||
if requires_python is None: | ||
# The package provides no information | ||
return True | ||
try: | ||
requires_python_specifier = specifiers.SpecifierSet(requires_python) | ||
except specifiers.InvalidSpecifier as e: | ||
logger.debug( | ||
"Package %s has an invalid Requires-Python entry - %s" % ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't know the package name here, so this log will be confusing. I'd leave the error uncaught here and log it where we're already catching it in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
requires_python, e)) | ||
raise specifiers.InvalidSpecifier(*e.args) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not simply There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My bad, I though a bare |
||
|
||
# We only use major.minor.micro | ||
python_version = version.parse('.'.join(map(str, sys.version_info[:3]))) | ||
return python_version in requires_python_specifier | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say we certainly don't want to raise and crash
pip install
because an index server has a malformeddata-requires
.In such case, we can either ignore the malformed
data-requires-python
(and accept the link) or decide to ignore the link altogether.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I went for accepting the link, as ignoring can be hard to debug on end-user side.