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

Pip18 support #672

Merged
merged 8 commits into from
Aug 29, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
20 changes: 20 additions & 0 deletions .appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ environment:
PIP: 9.0.1
- TOXENV: py27-pip9.0.3
PIP: 9.0.3
- TOXENV: py27-pip10.0.1
PIP: 10.0.1
- TOXENV: py27-pipmaster
PIP: master
- TOXENV: py27-piplatest
Expand All @@ -17,6 +19,8 @@ environment:
PIP: 9.0.1
- TOXENV: py34-pip9.0.3
PIP: 9.0.3
- TOXENV: py34-pip10.0.1
PIP: 10.0.1
- TOXENV: py34-pipmaster
PIP: master
- TOXENV: py34-piplatest
Expand All @@ -27,6 +31,8 @@ environment:
PIP: 9.0.1
- TOXENV: py35-pip9.0.3
PIP: 9.0.3
- TOXENV: py35-pip10.0.1
PIP: 10.0.1
- TOXENV: py35-pipmaster
PIP: master
- TOXENV: py35-piplatest
Expand All @@ -37,10 +43,24 @@ environment:
PIP: 9.0.1
- TOXENV: py36-pip9.0.3
PIP: 9.0.3
- TOXENV: py36-pip10.0.1
PIP: 10.0.1
- TOXENV: py36-pipmaster
PIP: master
- TOXENV: py36-piplatest
PIP: latest
- TOXENV: py37-pip8.1.1
PIP: 8.1.1
- TOXENV: py37-pip9.0.1
PIP: 9.0.1
- TOXENV: py37-pip9.0.3
PIP: 9.0.3
- TOXENV: py37-pip10.0.1
PIP: 10.0.1
- TOXENV: py37-pipmaster
PIP: master
- TOXENV: py37-piplatest
PIP: latest

install:
- C:\Python36\python -m pip install tox
Expand Down
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ env:
- PIP=8.1.1
- PIP=9.0.1
- PIP=9.0.3
- PIP=10.0.1
- PIP=master
Copy link
Contributor

Choose a reason for hiding this comment

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

Also PIP=18.0 should be tested. The master branch is not equal to the 18.0 version anymore, e.g. currently in master the Command class lives in pip._internal.cli.base_command, but in Pip 18.0 it's still in pip._internal.basecommand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Latest is tested, which is 18. It doesn't particularly matter because the import shims are rewritten to handle both possibilities gracefully

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, I forgot that latest==18 (currently).

- PIP=latest

Expand Down
28 changes: 16 additions & 12 deletions piptools/_compat/pip_compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,21 @@
import importlib

def do_import(module_path, subimport=None, old_path=None):
internal = 'pip._internal.{0}'.format(module_path)
old_path = old_path or module_path
pip9 = 'pip.{0}'.format(old_path)
try:
_tmp = importlib.import_module(internal)
except ImportError:
_tmp = importlib.import_module(pip9)
if subimport:
return getattr(_tmp, subimport, _tmp)
return _tmp

prefixes = ["pip._internal", "pip"]
paths = [module_path, old_path]
search_order = ["{0}.{1}".format(p, pth) for p in prefixes for pth in paths if pth is not None]
package = subimport if subimport else None
for to_import in search_order:
if not subimport:
to_import, _, package = to_import.rpartition(".")
try:
imported = importlib.import_module(to_import)
except ImportError:
continue
else:
return getattr(imported, package)


InstallRequirement = do_import('req.req_install', 'InstallRequirement')
parse_requirements = do_import('req.req_file', 'parse_requirements')
Expand All @@ -24,7 +28,7 @@ def do_import(module_path, subimport=None, old_path=None):
PackageFinder = do_import('index', 'PackageFinder')
FormatControl = do_import('index', 'FormatControl')
Wheel = do_import('wheel', 'Wheel')
Command = do_import('basecommand', 'Command')
cmdoptions = do_import('cmdoptions')
Command = do_import('cli.base_command', 'Command', old_path='basecommand')
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this doesn't work with Pip 18.0.

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 does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the new import shims it tries each possibility and falls back to the next ones

>>> def do_import(module_path, subimport=None, old_path=None):
  2         old_path = old_path or module_path
  3         prefixes = ["pip._internal", "pip"]
  4         paths = [module_path, old_path]
  5         search_order = ["{0}.{1}".format(p, pth) for p in prefixes for pth in paths if pth is not None]
  6         package = subimport if subimport else None
  7         for to_import in search_order:
  8             if not subimport:
  9                 to_import, _, package = to_import.rpartition(".")
 10             print('Import target: %s\tpackage: %s' % (to_import, package))
>>> do_import('cli.base_command', 'Command', old_path='basecommand')
Import target: pip._internal.cli.base_command   package: Command
Import target: pip._internal.basecommand        package: Command
Import target: pip.cli.base_command     package: Command
Import target: pip.basecommand  package: Command

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And in case you need proof:

 ~/g/pip-tools   pip18-support    vf tmp --python=python3.6
Running virtualenv with interpreter /home/hawk/.pyenv/shims/python3.6
Using base prefix '/home/hawk/.pyenv/versions/3.6.5'
New python executable in /home/hawk/.virtualenvs/tempenv-3369325970666/bin/python3.6
Also creating executable in /home/hawk/.virtualenvs/tempenv-3369325970666/bin/python
Installing setuptools, pip, wheel...done.
  ³ tempenv-3369325970666  ~/g/pip-tools   pip18-support    pip install -e .
Obtaining file:///home/hawk/git/pip-tools
[...]
Installing collected packages: click, first, six, pip-tools
  Running setup.py develop for pip-tools
Successfully installed click-6.7 first-2.0.1 pip-tools six-1.11.0
  ³ tempenv-3369325970666  ~/g/pip-tools   pip18-support    pip install ptpython
Collecting ptpython
  Downloading https://files.pythonhosted.org/packages/d0/dd/163a698a86b9de92857f128117034bdb36f5a784d839eea3bc07e2c858ae/ptpython-0.41-py3-none-any.whl (47kB)
    100% |████████████████████████████████| 51kB 1.1MB/s
[...]

  ³ tempenv-3369325970666  ~/g/pip-tools   pip18-support    ptpython
>>> from pip import __version__ as pip_version
>>> print(pip_version)
18.0
>>> from piptools._compat import Command
>>> Command
<class 'pip._internal.basecommand.Command'>
>>> 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI if you are using the code for importing from the pip_compat file I recommend grabbing the updated version, it borrows some concepts from https://github.com/brettcannon/modutil to do imports a bit more efficiently

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, OK, great that it works. Sorry for the incorrect analysis.

Good work! 👍

cmdoptions = do_import('cli.cmdoptions', old_path='cmdoptions')
get_installed_distributions = do_import('utils.misc', 'get_installed_distributions', old_path='utils')
PyPI = do_import('models.index', 'PyPI')
122 changes: 74 additions & 48 deletions piptools/repositories/pypi.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@
make_install_requirement)
from .base import BaseRepository


try:
from pip._internal.operations.prepare import RequirementPreparer
from pip._internal.resolve import Resolver as PipResolver
from pip._internal.req.req_tracker import RequirementTracker
except ImportError:
pass
@contextmanager
def RequirementTracker():
yield

try:
from pip._internal.cache import WheelCache
Expand All @@ -49,7 +49,6 @@ class PyPIRepository(BaseRepository):
def __init__(self, pip_options, session):
self.session = session
self.pip_options = pip_options
self.wheel_cache = WheelCache(CACHE_DIR, pip_options.format_control)

index_urls = [pip_options.index_url] + pip_options.extra_index_urls
if pip_options.no_index:
Expand Down Expand Up @@ -130,6 +129,62 @@ def find_best_match(self, ireq, prereleases=None):
best_candidate.project, best_candidate.version, ireq.extras, constraint=ireq.constraint
)

def resolve_reqs(self, download_dir, ireq, wheel_cache):
results = None
try:
from pip._internal.operations.prepare import RequirementPreparer
from pip._internal.resolve import Resolver as PipResolver
except ImportError:
# Pip 9 and below
reqset = RequirementSet(
self.build_dir,
self.source_dir,
download_dir=download_dir,
wheel_download_dir=self._wheel_download_dir,
session=self.session,
wheel_cache=wheel_cache
)
results = reqset._prepare_file(self.finder, ireq)
else:
# pip >= 10
preparer_kwargs = {
'build_dir': self.build_dir,
'src_dir': self.source_dir,
'download_dir': download_dir,
'wheel_download_dir': self._wheel_download_dir,
'progress_bar': 'off',
'build_isolation': False
}
resolver_kwargs = {
'finder': self.finder,
'session': self.session,
'upgrade_strategy': "to-satisfy-only",
'force_reinstall': False,
'ignore_dependencies': False,
'ignore_requires_python': False,
'ignore_installed': True,
'isolated': False,
'wheel_cache': wheel_cache,
'use_user_site': False
}
resolver = None
preparer = None
with RequirementTracker() as req_tracker:
# Pip 18 uses a requirement tracker to prevent fork bombs
if req_tracker:
preparer_kwargs['req_tracker'] = req_tracker
preparer = RequirementPreparer(**preparer_kwargs)
resolver_kwargs['preparer'] = preparer
reqset = RequirementSet()
ireq.is_direct = True
reqset.add_requirement(ireq)
resolver = PipResolver(**resolver_kwargs)
resolver.require_hashes = False
results = resolver._resolve_one(reqset, ireq)
reqset.cleanup_files()

return set(results)

def get_dependencies(self, ireq):
"""
Given a pinned or an editable InstallRequirement, returns a set of
Expand All @@ -156,50 +211,21 @@ def get_dependencies(self, ireq):
if not os.path.isdir(self._wheel_download_dir):
os.makedirs(self._wheel_download_dir)

wheel_cache = WheelCache(CACHE_DIR, self.pip_options.format_control)
prev_tracker = os.environ.get('PIP_REQ_TRACKER')
try:
# Pip < 9 and below
reqset = RequirementSet(
self.build_dir,
self.source_dir,
download_dir=download_dir,
wheel_download_dir=self._wheel_download_dir,
session=self.session,
wheel_cache=self.wheel_cache,
)
self._dependencies_cache[ireq] = reqset._prepare_file(
self.finder,
ireq
)
except TypeError:
# Pip >= 10 (new resolver!)
preparer = RequirementPreparer(
build_dir=self.build_dir,
src_dir=self.source_dir,
download_dir=download_dir,
wheel_download_dir=self._wheel_download_dir,
progress_bar='off',
build_isolation=False
)
reqset = RequirementSet()
ireq.is_direct = True
reqset.add_requirement(ireq)
self.resolver = PipResolver(
preparer=preparer,
finder=self.finder,
session=self.session,
upgrade_strategy="to-satisfy-only",
force_reinstall=False,
ignore_dependencies=False,
ignore_requires_python=False,
ignore_installed=True,
isolated=False,
wheel_cache=self.wheel_cache,
use_user_site=False,
)
self.resolver.resolve(reqset)
self._dependencies_cache[ireq] = reqset.requirements.values()
reqset.cleanup_files()
return set(self._dependencies_cache[ireq])
self._dependencies_cache[ireq] = self.resolve_reqs(download_dir, ireq, wheel_cache)
finally:
if 'PIP_REQ_TRACKER' in os.environ:
if prev_tracker:
os.environ['PIP_REQ_TRACKER'] = prev_tracker
else:
del os.environ['PIP_REQ_TRACKER']
try:
self.wheel_cache.cleanup()
Copy link
Member

Choose a reason for hiding this comment

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

@techalchemy shouldn't there be a wheel_cache.cleanup() instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch! And how unfortunate that the error is masked by the try-except. Maybe instead of try-except, it would be better to use:

if hasattr(wheel_cache, 'cleanup'):
    wheel_cache.cleanup()

IMHO that's easier to read and less error prone.

Copy link
Member

@atugushev atugushev Oct 29, 2019

Choose a reason for hiding this comment

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

The hasattr() has a quircks on Python 2, see the explanation: https://hynek.me/articles/hasattr/

Take a look at my suggestion in #968, btw.

except AttributeError:
pass
return self._dependencies_cache[ireq]

def get_hashes(self, ireq):
"""
Expand Down
7 changes: 7 additions & 0 deletions tests/test_repositories.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
from mock import MagicMock, patch
from pip._vendor.packaging.version import parse as parse_version
from pip import __version__ as pip_version
from piptools._compat import PackageFinder, InstallRequirement

from piptools.repositories.pypi import PyPIRepository
from piptools.scripts.compile import get_pip_command
import pytest


def test_pypirepo_build_dir_is_str():
Expand All @@ -13,6 +16,10 @@ def test_pypirepo_source_dir_is_str():
assert isinstance(get_pypi_repository().source_dir, str)


@pytest.mark.skipif(
parse_version(pip_version) >= parse_version('10.0.0'),
reason="RequirementSet objects don't take arguments after pip 10."
)
def test_pypirepo_calls_reqset_with_str_paths():
"""
Make sure that paths passed to RequirementSet init are str.
Expand Down
5 changes: 4 additions & 1 deletion tox.ini
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tox]
envlist =
py{27,34,35,36,py}-pip{8.1.1,9.0.1,9.0.3,latest,master}
py{27,34,35,36,37,py}-pip{8.1.1,9.0.1,9.0.3,10.0.1,latest,master}
flake8
readme
skip_missing_interpreters = True
Expand All @@ -12,6 +12,7 @@ deps =
pip8.1.1: pip==8.1.1
pip9.0.1: pip==9.0.1
pip9.0.3: pip==9.0.3
pip10.0.1: pip==10.0.1
coverage
mock
pytest
Expand All @@ -21,6 +22,7 @@ setenv =
pip8.1.1: PIP=8.1.1
pip9.0.1: PIP=9.0.1
pip9.0.3: PIP=9.0.3
pip10.0.1: PIP=10.0.1
install_command= python -m pip install {opts} {packages}
commands =
pip --version
Expand All @@ -42,5 +44,6 @@ PIP =
8.1.1: pip8.1.1
9.0.1: pip9.0.1
9.0.3: pip9.0.3
10.0.1: pip10.0.1
latest: piplatest
master: pipmaster