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

Feature/better resolver #2267

Merged
merged 18 commits into from
May 28, 2018
Merged
Show file tree
Hide file tree
Changes from 5 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
5 changes: 3 additions & 2 deletions pipenv/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
PIPENV_SHELL,
PIPENV_PYTHON,
PIPENV_VIRTUALENV,
PIPENV_CACHE_DIR,
)

# Backport required for earlier versions of Python.
Expand Down Expand Up @@ -1494,7 +1495,7 @@ def pip_install(
)
if verbose:
click.echo('$ {0}'.format(pip_command), err=True)
c = delegator.run(pip_command, block=block)
c = delegator.run(pip_command, block=block, env={'PIP_CACHE_DIR': PIPENV_CACHE_DIR})
return c


Expand All @@ -1506,7 +1507,7 @@ def pip_download(package_name):
source['url'],
project.download_location,
)
c = delegator.run(cmd)
c = delegator.run(cmd, env={'PIP_CACHE_DIR': PIPENV_CACHE_DIR})
if c.return_code == 0:
break

Expand Down
63 changes: 43 additions & 20 deletions pipenv/patched/piptools/repositories/pypi.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import hashlib
import os
import sys
from contextlib import contextmanager
from shutil import rmtree

Expand All @@ -20,14 +21,17 @@
SafeFileCache,
)

from notpip._vendor.packaging.requirements import InvalidRequirement
from notpip._vendor.packaging.requirements import InvalidRequirement, Requirement
from notpip._vendor.packaging.version import Version, InvalidVersion, parse as parse_version
from notpip._vendor.packaging.specifiers import SpecifierSet
from notpip._vendor.pyparsing import ParseException

from ..cache import CACHE_DIR
from pipenv.environments import PIPENV_CACHE_DIR
from ..exceptions import NoCandidateFound
from ..utils import (fs_str, is_pinned_requirement, lookup_table,
make_install_requirement)
from ..utils import (fs_str, is_pinned_requirement, lookup_table, as_tuple, key_from_req,
make_install_requirement, format_requirement, dedup)

from .base import BaseRepository


Expand Down Expand Up @@ -159,7 +163,12 @@ def find_best_match(self, ireq, prereleases=None):
if ireq.editable:
return ireq # return itself as the best match

all_candidates = self.find_all_candidates(ireq.name)
py_version = parse_version(os.environ.get('PIP_PYTHON_VERSION', str(sys.version_info[:3])))
all_candidates = [
c for c in self.find_all_candidates(ireq.name)
if SpecifierSet(c.requires_python).contains(py_version)
]

candidates_by_version = lookup_table(all_candidates, key=lambda c: c.version, unique=True)
try:
matching_versions = ireq.specifier.filter((candidate.version for candidate in all_candidates),
Expand Down Expand Up @@ -188,21 +197,33 @@ def get_json_dependencies(self, ireq):
raise TypeError('Expected pinned InstallRequirement, got {}'.format(ireq))

def gen(ireq):
if self.DEFAULT_INDEX_URL in self.finder.index_urls:

url = 'https://pypi.org/pypi/{0}/json'.format(ireq.req.name)
r = self.session.get(url)

# TODO: Latest isn't always latest.
latest = list(r.json()['releases'].keys())[-1]
if str(ireq.req.specifier) == '=={0}'.format(latest):
latest_url = 'https://pypi.org/pypi/{0}/{1}/json'.format(ireq.req.name, latest)
latest_requires = self.session.get(latest_url)
for requires in latest_requires.json().get('info', {}).get('requires_dist', {}):
i = InstallRequirement.from_line(requires)
if self.DEFAULT_INDEX_URL not in self.finder.index_urls:
Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you so much for cleaning this garbage up, seriously this was so horrible to look at

return

url = 'https://pypi.org/pypi/{0}/json'.format(ireq.req.name)
releases = self.session.get(url).json()['releases']

matches = [
r for r in releases
if '=={0}'.format(r) == str(ireq.req.specifier)
]
if not matches:
return

release_requires = self.session.get(
'https://pypi.org/pypi/{0}/{1}/json'.format(
ireq.req.name, matches[0],
),
).json()
try:
requires_dist = release_requires['info']['requires_dist']
except KeyError:
return

if 'extra' not in repr(i.markers):
yield i
for requires in requires_dist:
i = InstallRequirement.from_line(requires)
if 'extra' not in repr(i.markers):
yield i

try:
if ireq not in self._json_dep_cache:
Expand All @@ -226,7 +247,6 @@ def get_dependencies(self, ireq):

return json_results


def get_legacy_dependencies(self, ireq):
"""
Given a pinned or an editable InstallRequirement, returns a set of
Expand All @@ -245,7 +265,10 @@ def get_legacy_dependencies(self, ireq):
setup_requires = self.finder.get_extras_links(
dist.get_metadata_lines('requires.txt')
)
except TypeError:
ireq.version = dist.version
ireq.project_name = dist.project_name
ireq.req = dist.as_requirement()
Copy link
Member

Choose a reason for hiding this comment

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

I don’t quite understand why these are needed :|

Copy link
Member Author

Choose a reason for hiding this comment

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

You just have to believe me that they are, based on errors I encountered

Copy link
Member Author

Choose a reason for hiding this comment

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

Sometimes the InstallRequirement doesn't properly get these values set on it during the resolution process because of ephemeral wheel building moving on too quickly or something along those lines, it's quite hard to pin down exactly where in the stack it is going wrong which is why I added an additional safeguard to try and get the actual name of the package in question and resolve the actual requirement and version -- these elements are critical for downstream resolution if we want to provide real conflict resolution

except (TypeError, ValueError):
pass

if ireq not in self._dependencies_cache:
Expand Down
6 changes: 3 additions & 3 deletions pipenv/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from pathlib2 import Path

from .cmdparse import Script
from .vendor.requirementslib import Requirement
from .vendor.requirementslib.requirements import Requirement
from .utils import (
atomic_open_for_write,
mkdir_p,
Expand Down Expand Up @@ -728,8 +728,8 @@ def add_package_to_pipfile(self, package_name, dev=False):
# Read and append Pipfile.
p = self.parsed_pipfile
# Don't re-capitalize file URLs or VCSs.
package = Requirement.from_line(package_name)
converted = first(package.as_pipfile().values())
package = Requirement.from_line(package_name.strip())
_, converted = package.pipfile_entry
key = 'dev-packages' if dev else 'packages'
# Set empty group if it doesn't exist yet.
if key not in p:
Expand Down
98 changes: 46 additions & 52 deletions pipenv/utils.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
# -*- coding: utf-8 -*-
import errno
import logging
import os
import re
import hashlib
import tempfile
import sys
import shutil
import logging
import sys

import crayons
import parse
import six
import stat
import warnings
from click import echo as click_echo

from click import echo as click_echo
from first import first

try:
from weakref import finalize
except ImportError:
Expand All @@ -28,9 +28,10 @@ def __init__(self, *args, **kwargs):
def detach(self):
return False

logging.basicConfig(level=logging.ERROR)

from time import time

logging.basicConfig(level=logging.ERROR)
try:
from urllib.parse import urlparse
except ImportError:
Expand All @@ -48,7 +49,7 @@ def detach(self):
from .environments import (
PIPENV_MAX_ROUNDS,
PIPENV_CACHE_DIR,
PIPENV_MAX_RETRIES
PIPENV_MAX_RETRIES,
)

try:
Expand Down Expand Up @@ -219,16 +220,16 @@ def prepare_pip_source_args(sources, pip_args=None):
def actually_resolve_reps(
deps, index_lookup, markers_lookup, project, sources, verbose, clear, pre, req_dir=None
):
from .patched.notpip._internal import basecommand, req
from .patched.notpip._vendor import requests as pip_requests
from .patched.notpip._internal import basecommand
from .patched.notpip._internal.req import parse_requirements
from .patched.notpip._internal.exceptions import DistributionNotFound
from .patched.notpip._vendor.requests.exceptions import HTTPError
from pipenv.patched.piptools.resolver import Resolver
from pipenv.patched.piptools.repositories.pypi import PyPIRepository
from pipenv.patched.piptools.scripts.compile import get_pip_command
from pipenv.patched.piptools import logging as piptools_logging
from pipenv.patched.piptools.exceptions import NoCandidateFound
from ._compat import TemporaryDirectory
from ._compat import TemporaryDirectory, NamedTemporaryFile

class PipCommand(basecommand.Command):
"""Needed for pip-tools."""
Expand All @@ -240,54 +241,43 @@ class PipCommand(basecommand.Command):
req_dir = TemporaryDirectory(suffix='-requirements', prefix='pipenv-')
cleanup_req_dir = True
for dep in deps:
if dep:
if dep.startswith('-e '):
constraint = req.InstallRequirement.from_editable(
dep[len('-e '):]
)
else:
fd, t = tempfile.mkstemp(
prefix='pipenv-', suffix='-requirement.txt', dir=req_dir.name
)
with os.fdopen(fd, 'w') as f:
f.write(dep)
constraint = [
c for c in req.parse_requirements(t, session=pip_requests)
][
0
]
# extra_constraints = []
if ' -i ' in dep:
index_lookup[constraint.name] = project.get_source(
url=dep.split(' -i ')[1]
).get(
'name'
)
if constraint.markers:
markers_lookup[constraint.name] = str(
constraint.markers
).replace(
'"', "'"
)
constraints.append(constraint)
if not dep:
continue
url = None
if ' -i ' in dep:
dep, url = dep.split(' -i ')
req = Requirement.from_line(dep)
constraints.append(req.as_line())
Copy link
Member

Choose a reason for hiding this comment

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

What does this from_lineto_line combination do? Can’t dep be added directly into constraints?

Copy link
Member Author

Choose a reason for hiding this comment

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

The short answer is no, the long answer is 'a lot of things happen here'. The line we pass in could be anything, but the intermediary step ensures that, for example, if it is a local path, we tell pip to name it properly, or if it is a wheel, we do the same. This step also ensures that we format the line in the proper filesystem-appropriate format, we supply the formatted markers correctly, it essentially offers us an extra layer of sanity checking and validation. At some point we can stop passing 'deps' around at all and just pass requirement objects, I just haven't done the code cleanup for that.

# extra_constraints = []
if url:
index_lookup[req.name] = project.get_source(url=url).get('name')
if req.markers:
markers_lookup[req.name] = req.markers.replace('"', "'")
constraints_file = None
pip_command = get_pip_command()
pip_args = []
if sources:
pip_args = prepare_pip_source_args(sources, pip_args)
with NamedTemporaryFile(mode='w', prefix='pipenv-', suffix='-constraints.txt', dir=req_dir.name, delete=False) as f:
f.write(u'\n'.join([_constraint for _constraint in constraints]))
constraints_file = f.name
if verbose:
print('Using pip: {0}'.format(' '.join(pip_args)))
pip_args = pip_args.extend(['--cache-dir', PIPENV_CACHE_DIR])
pip_options, _ = pip_command.parse_args(pip_args)
session = pip_command._build_session(pip_options)
pypi = PyPIRepository(
pip_options=pip_options, use_json=False, session=session
pip_options=pip_options, use_json=True, session=session
)
if verbose:
logging.log.verbose = True
piptools_logging.log.verbose = True
resolved_tree = set()

resolver = Resolver(
constraints=constraints,
constraints=parse_requirements(
constraints_file,
finder=pypi.finder, session=pypi.session, options=pip_options,
),
repository=pypi,
clear_caches=clear,
prereleases=pre,
Expand Down Expand Up @@ -1130,7 +1120,7 @@ def extract_uri_from_vcs_dep(dep):
return None


def install_or_update_vcs(vcs_obj, src_dir, name, rev=None):
def install_or_update_vcs(vcs_obj, src_dir, name, rev=None):
target_dir = os.path.join(src_dir, name)
target_rev = vcs_obj.make_rev_options(rev)
if not os.path.exists(target_dir):
Expand All @@ -1139,16 +1129,18 @@ def install_or_update_vcs(vcs_obj, src_dir, name, rev=None):
return vcs_obj.get_revision(target_dir)


def get_vcs_deps(project, pip_freeze=None, which=None, verbose=False, clear=False, pre=False, allow_global=False, dev=False):
from ._compat import vcs
def get_vcs_deps(
project, pip_freeze=None, which=None, verbose=False, clear=False,
pre=False, allow_global=False, dev=False):
from .patched.notpip._internal.vcs import VcsSupport
section = 'vcs_dev_packages' if dev else 'vcs_packages'
lines = []
lockfiles = []
try:
packages = getattr(project, section)
except AttributeError:
return [], []
vcs_registry = vcs()
vcs_registry = VcsSupport
vcs_uri_map = {
extract_uri_from_vcs_dep(v): {'name': k, 'ref': v.get('ref')}
for k, v in packages.items()
Expand All @@ -1164,13 +1156,15 @@ def get_vcs_deps(project, pip_freeze=None, which=None, verbose=False, clear=Fals
pipfile_rev = vcs_uri_map[_vcs_match]['ref']
src_dir = os.environ.get('PIP_SRC', os.path.join(project.virtualenv_location, 'src'))
mkdir_p(src_dir)
pipfile_req = Requirement.from_pipfile(pipfile_name, [], packages[pipfile_name])
names = {pipfile_name}
_pip_uri = line.lstrip('-e ')
backend_name = str(_pip_uri.split('+', 1)[0])
backend = vcs_registry._registry[first(b for b in vcs_registry if b == backend_name)]
__vcs = backend(url=_pip_uri)

backend = vcs_registry()._registry.get(pipfile_req.vcs)
# TODO: Why doesn't pip freeze list 'git+git://' formatted urls?
if line.startswith('-e ') and not '{0}+'.format(pipfile_req.vcs) in line:
line = line.replace('-e ', '-e {0}+'.format(pipfile_req.vcs))
installed = Requirement.from_line(line)
__vcs = backend(url=installed.req.uri)

names.add(installed.normalized_name)
locked_rev = None
for _name in names:
Expand Down
Loading