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

Implement pep 503 data-requires-python #3877

Merged
merged 9 commits into from
Aug 11, 2016
Merged
Show file tree
Hide file tree
Changes from 7 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
39 changes: 36 additions & 3 deletions pip/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -32,7 +33,9 @@
from pip._vendor import html5lib, requests, six
from pip._vendor.packaging.version import parse as parse_version
from pip._vendor.packaging.utils import canonicalize_name
from pip._vendor.packaging import specifiers
from pip._vendor.requests.exceptions import SSLError
from pip._vendor.distlib.compat import unescape


__all__ = ['FormatControl', 'fmt_ctl_handle_mutual_exclude', 'PackageFinder']
Expand Down Expand Up @@ -640,6 +643,16 @@ def _link_package_versions(self, link, search):
self._log_skipped_link(
link, 'Python version is incorrect')
return
try:
support_this_python = check_requires_python(link.requires_python)
except specifiers.InvalidSpecifier:
support_this_python = True

if not support_this_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)
Expand Down Expand Up @@ -828,7 +841,8 @@ def links(self):
url = self.clean_link(
urllib_parse.urljoin(self.base_url, href)
)
yield Link(url, self)
pyrequire = unescape(anchor.get('data-requires-python'))
Copy link
Member

Choose a reason for hiding this comment

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

We need to check that the attribute is not None before passing it to unescape, because unescape doesn't like None.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

yield Link(url, self, requires_python=pyrequire)
Copy link
Member

Choose a reason for hiding this comment

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

I would put the unescape here since we are already dealing with html here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


_clean_re = re.compile(r'[^a-z0-9$&+,/:;=?@.#%_\\|-]', re.I)

Expand All @@ -842,18 +856,37 @@ 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_from:
<Not sure>
Copy link
Member

Choose a reason for hiding this comment

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

Looking at the code that instantiates it, it appears that comes_from is the HTMLPage instance from which the link was found.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.
Copy link
Member

Choose a reason for hiding this comment

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

I'd add:

This may be specified by a data-requires-python attribute in the HTML link tag, as described in PEP 503.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

"""

# 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 = requires_python
Copy link
Member

Choose a reason for hiding this comment

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

nitpicking but self.requires_python = requires_python if requires_python else None should fit in the 79 characters

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it fits.


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)

Expand Down
33 changes: 33 additions & 0 deletions pip/utils/packaging.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
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.
Copy link
Member

Choose a reason for hiding this comment

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

used -> use

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops.


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
Copy link
Member

Choose a reason for hiding this comment

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

Maybe put the Raise on a new line.
And either put Return/Raise or Returns/Raises ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably a bad vim gwip, fixed.

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" % (
Copy link
Member

Choose a reason for hiding this comment

The 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 _link_package_versions().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

requires_python, e))
raise

# We only use major.minor.micro
python_version = version.parse('.'.join(map(str, sys.version_info[:3])))
return python_version in requires_python_specifier