Merge branch 'master' into remove-obsolete-git-urls

docs/html/reference/pip_install.rst

@@ -384,18 +384,18 @@ where ``setup.py`` is not in the root of project, the "subdirectory" component
 is used. The value of the "subdirectory" component should be a path starting
 from the root of the project to where ``setup.py`` is located.
 
-So if your repository layout is:
+If your repository layout is::
 
-    - pkg_dir/
+   pkg_dir
+   ├── setup.py  # setup.py for package "pkg"
+   └── some_module.py
+   other_dir
+   └── some_file
+   some_other_file
 
-      - setup.py  # setup.py for package ``pkg``
+Then, to install from this repository, the syntax would be::
-      - some_module.py
-    - other_dir/
 
-      - some_file
+   $ pip install -e "vcs+protocol://repo_url/#egg=pkg&subdirectory=pkg_dir"
-    - some_other_file
-
-You'll need to use ``pip install -e "vcs+protocol://repo_url/#egg=pkg&subdirectory=pkg_dir"``.
 
 
 Git
@@ -940,6 +940,17 @@ Examples
 
       $ pip install --pre SomePackage
 
+
+#. Install packages from source.
+
+   Do not use any binary packages::
+
+     $ pip install SomePackage1 SomePackage2 --no-binary :all:
+
+   Specify ``SomePackage1`` to be installed from source::
+
+     $ pip install SomePackage1 SomePackage2 --no-binary SomePackage1
+
 ----
 
 .. [1] This is true with the exception that pip v7.0 and v7.0.1 required quotes

news/3191.doc

@@ -0,0 +1 @@
+Clarify the usage of --no-binary command.

news/7402.bugfix

@@ -0,0 +1 @@
+Reject VCS URLs with an empty revision.

news/7856.bugfix

@@ -0,0 +1 @@
+Uninstallation no longer fails on trying to remove non-existent files.

news/7924.bugfix

@@ -1 +0,0 @@
-Catch ``subprocess.CalledProcessError`` when checking for the presence of executable within ``need_executable`` using pytest.

src/pip/_internal/cli/cmdoptions.py

@@ -475,12 +475,12 @@ def no_binary():
         "--no-binary", dest="format_control", action="callback",
         callback=_handle_no_binary, type="str",
         default=format_control,
-        help="Do not use binary packages. Can be supplied multiple times, and "
+        help='Do not use binary packages. Can be supplied multiple times, and '
-             "each time adds to the existing value. Accepts either :all: to "
+             'each time adds to the existing value. Accepts either ":all:" to '
-             "disable all binary packages, :none: to empty the set, or one or "
+             'disable all binary packages, ":none:" to empty the set (notice '
-             "more package names with commas between them (no colons). Note "
+             'the colons), or one or more package names with commas between '
-             "that some packages are tricky to compile and may fail to "
+             'them (no colons). Note that some packages are tricky to compile '
-             "install when this option is used on them.",
+             'and may fail to install when this option is used on them.',
     )
 
 
@@ -491,12 +491,12 @@ def only_binary():
         "--only-binary", dest="format_control", action="callback",
         callback=_handle_only_binary, type="str",
         default=format_control,
-        help="Do not use source packages. Can be supplied multiple times, and "
+        help='Do not use source packages. Can be supplied multiple times, and '
-             "each time adds to the existing value. Accepts either :all: to "
+             'each time adds to the existing value. Accepts either ":all:" to '
-             "disable all source packages, :none: to empty the set, or one or "
+             'disable all source packages, ":none:" to empty the set, or one '
-             "more package names with commas between them. Packages without "
+             'or more package names with commas between them. Packages '
-             "binary distributions will fail to install when this option is "
+             'without binary distributions will fail to install when this '
-             "used on them.",
+             'option is used on them.',
     )
 
 

src/pip/_internal/cli/req_command.py

@@ -255,7 +255,6 @@ class RequirementCommand(IndexGroupCommand):
         make_install_req = partial(
             install_req_from_req_string,
             isolated=options.isolated_mode,
-            wheel_cache=wheel_cache,
             use_pep517=use_pep517,
         )
         # The long import name and duplicated invocation is needed to convince
@@ -266,6 +265,7 @@ class RequirementCommand(IndexGroupCommand):
             return pip._internal.resolution.resolvelib.resolver.Resolver(
                 preparer=preparer,
                 finder=finder,
+                wheel_cache=wheel_cache,
                 make_install_req=make_install_req,
                 use_user_site=use_user_site,
                 ignore_dependencies=options.ignore_dependencies,
@@ -279,6 +279,7 @@ class RequirementCommand(IndexGroupCommand):
         return pip._internal.resolution.legacy.resolver.Resolver(
             preparer=preparer,
             finder=finder,
+            wheel_cache=wheel_cache,
             make_install_req=make_install_req,
             use_user_site=use_user_site,
             ignore_dependencies=options.ignore_dependencies,
@@ -295,7 +296,6 @@ class RequirementCommand(IndexGroupCommand):
         options,          # type: Values
         finder,           # type: PackageFinder
         session,          # type: PipSession
-        wheel_cache,      # type: Optional[WheelCache]
         check_supported_wheels=True,  # type: bool
     ):
         # type: (...) -> List[InstallRequirement]
@@ -313,7 +313,6 @@ class RequirementCommand(IndexGroupCommand):
                 req_to_add = install_req_from_parsed_requirement(
                     parsed_req,
                     isolated=options.isolated_mode,
-                    wheel_cache=wheel_cache,
                 )
                 req_to_add.is_direct = True
                 requirement_set.add_requirement(req_to_add)
@@ -322,7 +321,6 @@ class RequirementCommand(IndexGroupCommand):
             req_to_add = install_req_from_line(
                 req, None, isolated=options.isolated_mode,
                 use_pep517=options.use_pep517,
-                wheel_cache=wheel_cache
             )
             req_to_add.is_direct = True
             requirement_set.add_requirement(req_to_add)
@@ -332,7 +330,6 @@ class RequirementCommand(IndexGroupCommand):
                 req,
                 isolated=options.isolated_mode,
                 use_pep517=options.use_pep517,
-                wheel_cache=wheel_cache
             )
             req_to_add.is_direct = True
             requirement_set.add_requirement(req_to_add)
@@ -345,7 +342,6 @@ class RequirementCommand(IndexGroupCommand):
                 req_to_add = install_req_from_parsed_requirement(
                     parsed_req,
                     isolated=options.isolated_mode,
-                    wheel_cache=wheel_cache,
                     use_pep517=options.use_pep517
                 )
                 req_to_add.is_direct = True

src/pip/_internal/commands/download.py

@@ -107,13 +107,7 @@ class DownloadCommand(RequirementCommand):
             globally_managed=True,
         )
 
-        reqs = self.get_requirements(
+        reqs = self.get_requirements(args, options, finder, session)
-            args,
-            options,
-            finder,
-            session,
-            None
-        )
 
         preparer = self.make_requirement_preparer(
             temp_build_dir=directory,

src/pip/_internal/commands/install.py

@@ -299,7 +299,7 @@ class InstallCommand(RequirementCommand):
         try:
             reqs = self.get_requirements(
                 args, options, finder, session,
-                wheel_cache, check_supported_wheels=not options.target_dir,
+                check_supported_wheels=not options.target_dir,
             )
 
             warn_deprecated_install_options(

src/pip/_internal/commands/wheel.py

@@ -133,10 +133,7 @@ class WheelCommand(RequirementCommand):
             globally_managed=True,
         )
 
-        reqs = self.get_requirements(
+        reqs = self.get_requirements(args, options, finder, session)
-            args, options, finder, session,
-            wheel_cache
-        )
 
         preparer = self.make_requirement_preparer(
             temp_build_dir=directory,

src/pip/_internal/exceptions.py

@@ -147,7 +147,7 @@ class HashError(InstallationError):
         triggering requirement.
 
         :param req: The InstallRequirement that provoked this error, with
-            populate_link() having already been called
+            its link already populated by the resolver's _populate_link().
 
         """
         return '    {}'.format(self._requirement_name())

src/pip/_internal/operations/freeze.py

@@ -120,13 +120,11 @@ def freeze(
                         line_req = install_req_from_editable(
                             line,
                             isolated=isolated,
-                            wheel_cache=wheel_cache,
                         )
                     else:
                         line_req = install_req_from_line(
                             COMMENT_RE.sub('', line).strip(),
                             isolated=isolated,
-                            wheel_cache=wheel_cache,
                         )
 
                     if not line_req.name:

src/pip/_internal/req/constructors.py

@@ -36,7 +36,6 @@ if MYPY_CHECK_RUNNING:
     from typing import (
         Any, Dict, Optional, Set, Tuple, Union,
     )
-    from pip._internal.cache import WheelCache
     from pip._internal.req.req_file import ParsedRequirement
 
 
@@ -223,7 +222,6 @@ def install_req_from_editable(
     use_pep517=None,  # type: Optional[bool]
     isolated=False,  # type: bool
     options=None,  # type: Optional[Dict[str, Any]]
-    wheel_cache=None,  # type: Optional[WheelCache]
     constraint=False  # type: bool
 ):
     # type: (...) -> InstallRequirement
@@ -242,7 +240,6 @@ def install_req_from_editable(
         install_options=options.get("install_options", []) if options else [],
         global_options=options.get("global_options", []) if options else [],
         hash_options=options.get("hashes", {}) if options else {},
-        wheel_cache=wheel_cache,
         extras=parts.extras,
     )
 
@@ -387,7 +384,6 @@ def install_req_from_line(
     use_pep517=None,  # type: Optional[bool]
     isolated=False,  # type: bool
     options=None,  # type: Optional[Dict[str, Any]]
-    wheel_cache=None,  # type: Optional[WheelCache]
     constraint=False,  # type: bool
     line_source=None,  # type: Optional[str]
 ):
@@ -406,7 +402,6 @@ def install_req_from_line(
         install_options=options.get("install_options", []) if options else [],
         global_options=options.get("global_options", []) if options else [],
         hash_options=options.get("hashes", {}) if options else {},
-        wheel_cache=wheel_cache,
         constraint=constraint,
         extras=parts.extras,
     )
@@ -416,7 +411,6 @@ def install_req_from_req_string(
     req_string,  # type: str
     comes_from=None,  # type: Optional[InstallRequirement]
     isolated=False,  # type: bool
-    wheel_cache=None,  # type: Optional[WheelCache]
     use_pep517=None  # type: Optional[bool]
 ):
     # type: (...) -> InstallRequirement
@@ -439,15 +433,13 @@ def install_req_from_req_string(
         )
 
     return InstallRequirement(
-        req, comes_from, isolated=isolated, wheel_cache=wheel_cache,
+        req, comes_from, isolated=isolated, use_pep517=use_pep517
-        use_pep517=use_pep517
     )
 
 
 def install_req_from_parsed_requirement(
     parsed_req,  # type: ParsedRequirement
     isolated=False,  # type: bool
-    wheel_cache=None,  # type: Optional[WheelCache]
     use_pep517=None  # type: Optional[bool]
 ):
     # type: (...) -> InstallRequirement
@@ -458,7 +450,6 @@ def install_req_from_parsed_requirement(
             use_pep517=use_pep517,
             constraint=parsed_req.constraint,
             isolated=isolated,
-            wheel_cache=wheel_cache
         )
 
     else:
@@ -468,7 +459,6 @@ def install_req_from_parsed_requirement(
             use_pep517=use_pep517,
             isolated=isolated,
             options=parsed_req.options,
-            wheel_cache=wheel_cache,
             constraint=parsed_req.constraint,
             line_source=parsed_req.line_source,
         )

src/pip/_internal/req/req_install.py

@@ -30,7 +30,6 @@ from pip._internal.operations.install.legacy import install as install_legacy
 from pip._internal.operations.install.wheel import install_wheel
 from pip._internal.pyproject import load_pyproject_toml, make_pyproject_path
 from pip._internal.req.req_uninstall import UninstallPathSet
-from pip._internal.utils import compatibility_tags
 from pip._internal.utils.deprecation import deprecated
 from pip._internal.utils.hashes import Hashes
 from pip._internal.utils.logging import indent_log
@@ -55,8 +54,6 @@ if MYPY_CHECK_RUNNING:
         Any, Dict, Iterable, List, Optional, Sequence, Union,
     )
     from pip._internal.build_env import BuildEnvironment
-    from pip._internal.cache import WheelCache
-    from pip._internal.index.package_finder import PackageFinder
     from pip._vendor.pkg_resources import Distribution
     from pip._vendor.packaging.specifiers import SpecifierSet
     from pip._vendor.packaging.markers import Marker
@@ -111,7 +108,6 @@ class InstallRequirement(object):
         install_options=None,  # type: Optional[List[str]]
         global_options=None,  # type: Optional[List[str]]
         hash_options=None,  # type: Optional[Dict[str, List[str]]]
-        wheel_cache=None,  # type: Optional[WheelCache]
         constraint=False,  # type: bool
         extras=()  # type: Iterable[str]
     ):
@@ -126,7 +122,6 @@ class InstallRequirement(object):
             self.source_dir = os.path.normpath(os.path.abspath(source_dir))
         self.editable = editable
 
-        self._wheel_cache = wheel_cache
         if link is None and req and req.url:
             # PEP 508 URL requirement
             link = Link(req.url)
@@ -241,32 +236,6 @@ class InstallRequirement(object):
             state=", ".join(state),
         )
 
-    def populate_link(self, finder, upgrade, require_hashes):
-        # type: (PackageFinder, bool, bool) -> None
-        """Ensure that if a link can be found for this, that it is found.
-
-        Note that self.link may still be None - if Upgrade is False and the
-        requirement is already installed.
-
-        If require_hashes is True, don't use the wheel cache, because cached
-        wheels, always built locally, have different hashes than the files
-        downloaded from the index server and thus throw false hash mismatches.
-        Furthermore, cached wheels at present have undeterministic contents due
-        to file modification times.
-        """
-        if self.link is None:
-            self.link = finder.find_requirement(self, upgrade)
-        if self._wheel_cache is not None and not require_hashes:
-            old_link = self.link
-            supported_tags = compatibility_tags.get_supported()
-            self.link = self._wheel_cache.get(
-                link=self.link,
-                package_name=self.name,
-                supported_tags=supported_tags,
-            )
-            if old_link != self.link:
-                logger.debug('Using cached wheel link: %s', self.link)
-
     # Things that are valid for all kinds of requirements?
     @property
     def name(self):

src/pip/_internal/req/req_uninstall.py

@@ -585,11 +585,6 @@ class UninstallPathSet(object):
 class UninstallPthEntries(object):
     def __init__(self, pth_file):
         # type: (str) -> None
-        if not os.path.isfile(pth_file):
-            raise UninstallationError(
-                "Cannot remove entries from nonexistent file {}".format(
-                    pth_file)
-            )
         self.file = pth_file
         self.entries = set()  # type: Set[str]
         self._saved_lines = None  # type: Optional[List[bytes]]
@@ -613,6 +608,14 @@ class UninstallPthEntries(object):
     def remove(self):
         # type: () -> None
         logger.debug('Removing pth entries from %s:', self.file)
+
+        # If the file doesn't exist, log a warning and return
+        if not os.path.isfile(self.file):
+            logger.warning(
+                "Cannot remove entries from nonexistent file {}".format(
+                    self.file)
+            )
+            return
         with open(self.file, 'rb') as fh:
             # windows uses '\r\n' with py3k, but uses '\n' with py2.x
             lines = fh.readlines()

src/pip/_internal/resolution/legacy/resolver.py

@@ -30,6 +30,7 @@ from pip._internal.exceptions import (
 )
 from pip._internal.req.req_set import RequirementSet
 from pip._internal.resolution.base import BaseResolver
+from pip._internal.utils.compatibility_tags import get_supported
 from pip._internal.utils.logging import indent_log
 from pip._internal.utils.misc import dist_in_usersite, normalize_version_info
 from pip._internal.utils.packaging import (
@@ -42,6 +43,7 @@ if MYPY_CHECK_RUNNING:
     from typing import DefaultDict, List, Optional, Set, Tuple
     from pip._vendor import pkg_resources
 
+    from pip._internal.cache import WheelCache
     from pip._internal.distributions import AbstractDistribution
     from pip._internal.index.package_finder import PackageFinder
     from pip._internal.operations.prepare import RequirementPreparer
@@ -112,6 +114,7 @@ class Resolver(BaseResolver):
         self,
         preparer,  # type: RequirementPreparer
         finder,  # type: PackageFinder
+        wheel_cache,  # type: Optional[WheelCache]
         make_install_req,  # type: InstallRequirementProvider
         use_user_site,  # type: bool
         ignore_dependencies,  # type: bool
@@ -134,6 +137,7 @@ class Resolver(BaseResolver):
 
         self.preparer = preparer
         self.finder = finder
+        self.wheel_cache = wheel_cache
 
         self.upgrade_strategy = upgrade_strategy
         self.force_reinstall = force_reinstall
@@ -166,7 +170,7 @@ class Resolver(BaseResolver):
 
         # Actually prepare the files, and collect any exceptions. Most hash
         # exceptions cannot be checked ahead of time, because
-        # req.populate_link() needs to be called before we can make decisions
+        # _populate_link() needs to be called before we can make decisions
         # based on link type.
         discovered_reqs = []  # type: List[InstallRequirement]
         hash_errors = HashErrors()
@@ -256,6 +260,35 @@ class Resolver(BaseResolver):
         self._set_req_to_reinstall(req_to_install)
         return None
 
+    def _populate_link(self, req):
+        # type: (InstallRequirement) -> None
+        """Ensure that if a link can be found for this, that it is found.
+
+        Note that req.link may still be None - if the requirement is already
+        installed and not needed to be upgraded based on the return value of
+        _is_upgrade_allowed().
+
+        If preparer.require_hashes is True, don't use the wheel cache, because
+        cached wheels, always built locally, have different hashes than the
+        files downloaded from the index server and thus throw false hash
+        mismatches. Furthermore, cached wheels at present have undeterministic
+        contents due to file modification times.
+        """
+        upgrade = self._is_upgrade_allowed(req)
+        if req.link is None:
+            req.link = self.finder.find_requirement(req, upgrade)
+
+        if self.wheel_cache is None or self.preparer.require_hashes:
+            return
+        cached_link = self.wheel_cache.get(
+            link=req.link,
+            package_name=req.name,
+            supported_tags=get_supported(),
+        )
+        if req.link != cached_link:
+            logger.debug('Using cached wheel link: %s', cached_link)
+            req.link = cached_link
+
     def _get_abstract_dist_for(self, req):
         # type: (InstallRequirement) -> AbstractDistribution
         """Takes a InstallRequirement and returns a single AbstractDist \
@@ -274,11 +307,8 @@ class Resolver(BaseResolver):
                 req, skip_reason
             )
 
-        upgrade_allowed = self._is_upgrade_allowed(req)
-
         # We eagerly populate the link, since that's our "legacy" behavior.
-        require_hashes = self.preparer.require_hashes
+        self._populate_link(req)
-        req.populate_link(self.finder, upgrade_allowed, require_hashes)
         abstract_dist = self.preparer.prepare_linked_requirement(req)
 
         # NOTE

src/pip/_internal/resolution/resolvelib/candidates.py

@@ -30,7 +30,6 @@ def make_install_req_from_link(link, parent):
         comes_from=parent.comes_from,
         use_pep517=parent.use_pep517,
         isolated=parent.isolated,
-        wheel_cache=parent._wheel_cache,
         constraint=parent.constraint,
         options=dict(
             install_options=parent.install_options,

src/pip/_internal/resolution/resolvelib/resolver.py

@@ -16,6 +16,7 @@ if MYPY_CHECK_RUNNING:
 
     from pip._vendor.resolvelib.resolvers import Result
 
+    from pip._internal.cache import WheelCache
     from pip._internal.index.package_finder import PackageFinder
     from pip._internal.operations.prepare import RequirementPreparer
     from pip._internal.req.req_install import InstallRequirement
@@ -27,6 +28,7 @@ class Resolver(BaseResolver):
         self,
         preparer,  # type: RequirementPreparer
         finder,  # type: PackageFinder
+        wheel_cache,  # type: Optional[WheelCache]
         make_install_req,  # type: InstallRequirementProvider
         use_user_site,  # type: bool
         ignore_dependencies,  # type: bool

src/pip/_internal/vcs/versioncontrol.py

@@ -11,7 +11,7 @@ import sys
 from pip._vendor import pkg_resources
 from pip._vendor.six.moves.urllib import parse as urllib_parse
 
-from pip._internal.exceptions import BadCommand
+from pip._internal.exceptions import BadCommand, InstallationError
 from pip._internal.utils.compat import samefile
 from pip._internal.utils.misc import (
     ask_path_exists,
@@ -436,6 +436,12 @@ class VersionControl(object):
         rev = None
         if '@' in path:
             path, rev = path.rsplit('@', 1)
+            if not rev:
+                raise InstallationError(
+                    "The URL {!r} has an empty revision (after @) "
+                    "which is not supported. Include a revision after @ "
+                    "or remove @ from the URL.".format(url)
+                )
         url = urllib_parse.urlunsplit((scheme, netloc, path, query, ''))
         return url, rev, user_pass
 

tests/conftest.py

@@ -16,10 +16,9 @@ from setuptools.wheel import Wheel
 from pip._internal.cli.main import main as pip_entry_point
 from pip._internal.utils.temp_dir import global_tempdir_manager
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
-from tests.lib import DATA_DIR, SRC_DIR, TestData
+from tests.lib import DATA_DIR, SRC_DIR, PipTestEnvironment, TestData
 from tests.lib.certs import make_tls_cert, serialize_cert, serialize_key
 from tests.lib.path import Path
-from tests.lib.scripttest import PipTestEnvironment
 from tests.lib.server import make_mock_server, server_running
 from tests.lib.venv import VirtualEnvironment
 
@@ -370,7 +369,7 @@ def script(tmpdir, virtualenv, script_factory):
     Return a PipTestEnvironment which is unique to each test function and
     will execute all commands inside of the unique virtual environment for this
     test function. The returned object is a
-    ``tests.lib.scripttest.PipTestEnvironment``.
+    ``tests.lib.PipTestEnvironment``.
     """
     return script_factory(tmpdir.joinpath("workspace"), virtualenv)
 

tests/functional/test_uninstall.py

@@ -553,6 +553,53 @@ def test_uninstall_editable_and_pip_install(script, data):
     assert "FSPkg" not in {p["name"] for p in json.loads(list_result2.stdout)}
 
 
+def test_uninstall_editable_and_pip_install_easy_install_remove(script, data):
+    """Try uninstall after pip install -e after pip install
+    and removing easy-install.pth"""
+    # SETUPTOOLS_SYS_PATH_TECHNIQUE=raw removes the assumption that `-e`
+    # installs are always higher priority than regular installs.
+    # This becomes the default behavior in setuptools 25.
+    script.environ['SETUPTOOLS_SYS_PATH_TECHNIQUE'] = 'raw'
+
+    # Rename easy-install.pth to pip-test.pth
+    easy_install_pth = join(script.site_packages_path, 'easy-install.pth')
+    pip_test_pth = join(script.site_packages_path, 'pip-test.pth')
+    os.rename(easy_install_pth, pip_test_pth)
+
+    # Install FSPkg
+    pkg_path = data.packages.joinpath("FSPkg")
+    script.pip('install', '-e', '.',
+               expect_stderr=True, cwd=pkg_path)
+
+    # Rename easy-install.pth to pip-test-fspkg.pth
+    pip_test_fspkg_pth = join(script.site_packages_path, 'pip-test-fspkg.pth')
+    os.rename(easy_install_pth, pip_test_fspkg_pth)
+
+    # Confirm that FSPkg is installed
+    list_result = script.pip('list', '--format=json')
+    assert {"name": "FSPkg", "version": "0.1.dev0"} \
+        in json.loads(list_result.stdout)
+
+    # Remove pip-test-fspkg.pth
+    os.remove(pip_test_fspkg_pth)
+
+    # Uninstall will fail with given warning
+    uninstall = script.pip('uninstall', 'FSPkg', '-y')
+    assert "Cannot remove entries from nonexistent file" in uninstall.stderr
+
+    assert join(
+        script.site_packages, 'FSPkg.egg-link'
+    ) in uninstall.files_deleted, list(uninstall.files_deleted.keys())
+
+    # Confirm that FSPkg is uninstalled
+    list_result = script.pip('list', '--format=json')
+    assert {"name": "FSPkg", "version": "0.1.dev0"} \
+        not in json.loads(list_result.stdout)
+
+    # Rename pip-test.pth back to easy-install.pth
+    os.rename(pip_test_pth, easy_install_pth)
+
+
 def test_uninstall_ignores_missing_packages(script, data):
     """Uninstall of a non existent package prints a warning and exits cleanly
     """

tests/functional/test_yaml.py

@@ -1,15 +1,16 @@
-"""Tests for the resolver
+"""
+Tests for the resolver
 """
 
 import os
 import re
 
 import pytest
+import yaml
 
 from tests.lib import DATA_DIR, create_basic_wheel_for_package, path_to_url
-from tests.lib.yaml_helpers import generate_yaml_tests, id_func
 
-_conflict_finder_re = re.compile(
+_conflict_finder_pat = re.compile(
     # Conflicting Requirements: \
     # A 1.0.0 requires B == 2.0.0, C 1.0.0 requires B == 1.0.0.
     r"""
@@ -24,7 +25,49 @@ _conflict_finder_re = re.compile(
 )
 
 
-def _convert_to_dict(string):
+def generate_yaml_tests(directory):
+    """
+    Generate yaml test cases from the yaml files in the given directory
+    """
+    for yml_file in directory.glob("*/*.yml"):
+        data = yaml.safe_load(yml_file.read_text())
+        assert "cases" in data, "A fixture needs cases to be used in testing"
+
+        # Strip the parts of the directory to only get a name without
+        # extension and resolver directory
+        base_name = str(yml_file)[len(str(directory)) + 1:-4]
+
+        base = data.get("base", {})
+        cases = data["cases"]
+
+        for i, case_template in enumerate(cases):
+            case = base.copy()
+            case.update(case_template)
+
+            case[":name:"] = base_name
+            if len(cases) > 1:
+                case[":name:"] += "-" + str(i)
+
+            if case.pop("skip", False):
+                case = pytest.param(case, marks=pytest.mark.xfail)
+
+            yield case
+
+
+def id_func(param):
+    """
+    Give a nice parameter name to the generated function parameters
+    """
+    if isinstance(param, dict) and ":name:" in param:
+        return param[":name:"]
+
+    retval = str(param)
+    if len(retval) > 25:
+        retval = retval[:20] + "..." + retval[-2:]
+    return retval
+
+
+def convert_to_dict(string):
 
     def stripping_split(my_str, splitwith, count=None):
         if count is None:
@@ -89,7 +132,7 @@ def handle_install_request(script, requirement):
         message = result.stderr.rsplit("\n", 1)[-1]
 
         # XXX: There might be a better way than parsing the message
-        for match in re.finditer(message, _conflict_finder_re):
+        for match in re.finditer(message, _conflict_finder_pat):
             di = match.groupdict()
             retval["conflicting"].append(
                 {
@@ -119,7 +162,7 @@ def test_yaml_based(script, case):
     # XXX: This doesn't work because this isn't making an index of files.
     for package in available:
         if isinstance(package, str):
-            package = _convert_to_dict(package)
+            package = convert_to_dict(package)
 
         assert isinstance(package, dict), "Needs to be a dictionary"
 

tests/lib/scripttest.py

@@ -1,3 +0,0 @@
-from __future__ import absolute_import
-
-from . import PipTestEnvironment  # noqa

tests/lib/yaml_helpers.py

@@ -1,43 +0,0 @@
-"""
-"""
-
-import pytest
-import yaml
-
-
-def generate_yaml_tests(directory):
-    for yml_file in directory.glob("*/*.yml"):
-        data = yaml.safe_load(yml_file.read_text())
-        assert "cases" in data, "A fixture needs cases to be used in testing"
-
-        # Strip the parts of the directory to only get a name without
-        # extension and resolver directory
-        base_name = str(yml_file)[len(str(directory)) + 1:-4]
-
-        base = data.get("base", {})
-        cases = data["cases"]
-
-        for i, case_template in enumerate(cases):
-            case = base.copy()
-            case.update(case_template)
-
-            case[":name:"] = base_name
-            if len(cases) > 1:
-                case[":name:"] += "-" + str(i)
-
-            if case.pop("skip", False):
-                case = pytest.param(case, marks=pytest.mark.xfail)
-
-            yield case
-
-
-def id_func(param):
-    """Give a nice parameter name to the generated function parameters
-    """
-    if isinstance(param, dict) and ":name:" in param:
-        return param[":name:"]
-
-    retval = str(param)
-    if len(retval) > 25:
-        retval = retval[:20] + "..." + retval[-2:]
-    return retval

tests/unit/resolution_resolvelib/test_requirement.py

@@ -48,7 +48,7 @@ def test_cases(data):
     yield test_cases
 
 
-def test_rlr_requirement_has_name(test_cases, factory):
+def test_new_resolver_requirement_has_name(test_cases, factory):
     """All requirements should have a name"""
     for requirement, name, matches in test_cases:
         ireq = install_req_from_line(requirement)
@@ -56,7 +56,7 @@ def test_rlr_requirement_has_name(test_cases, factory):
         assert req.name == name
 
 
-def test_rlr_correct_number_of_matches(test_cases, factory):
+def test_new_resolver_correct_number_of_matches(test_cases, factory):
     """Requirements should return the correct number of candidates"""
     for requirement, name, matches in test_cases:
         ireq = install_req_from_line(requirement)
@@ -64,7 +64,7 @@ def test_rlr_correct_number_of_matches(test_cases, factory):
         assert len(req.find_matches()) == matches
 
 
-def test_rlr_candidates_match_requirement(test_cases, factory):
+def test_new_resolver_candidates_match_requirement(test_cases, factory):
     """Candidates returned from find_matches should satisfy the requirement"""
     for requirement, name, matches in test_cases:
         ireq = install_req_from_line(requirement)
@@ -74,7 +74,7 @@ def test_rlr_candidates_match_requirement(test_cases, factory):
             assert req.is_satisfied_by(c)
 
 
-def test_rlr_full_resolve(factory, provider):
+def test_new_resolver_full_resolve(factory, provider):
     """A very basic full resolve"""
     ireq = install_req_from_line("simplewheel")
     req = factory.make_requirement(ireq)

tests/unit/resolution_resolvelib/test_resolver.py

@@ -14,6 +14,7 @@ def resolver(preparer, finder):
     resolver = Resolver(
         preparer=preparer,
         finder=finder,
+        wheel_cache=None,
         make_install_req=mock.Mock(),
         use_user_site="not-used",
         ignore_dependencies="not-used",
@@ -57,7 +58,7 @@ def resolver(preparer, finder):
         ),
     ],
 )
-def test_rlr_resolver_get_installation_order(resolver, edges, ordered_reqs):
+def test_new_resolver_get_installation_order(resolver, edges, ordered_reqs):
     # Build graph from edge declarations.
     graph = DirectedGraph()
     for parent, child in edges:

tests/unit/test_req.py

@@ -74,7 +74,6 @@ class TestRequirementSet(object):
         make_install_req = partial(
             install_req_from_req_string,
             isolated=False,
-            wheel_cache=None,
             use_pep517=None,
         )
 
@@ -95,6 +94,7 @@ class TestRequirementSet(object):
                 preparer=preparer,
                 make_install_req=make_install_req,
                 finder=finder,
+                wheel_cache=None,
                 use_user_site=False, upgrade_strategy="to-satisfy-only",
                 ignore_dependencies=False, ignore_installed=False,
                 ignore_requires_python=False, force_reinstall=False,
@@ -176,9 +176,7 @@ class TestRequirementSet(object):
         command = create_command('install')
         with requirements_file('--require-hashes', tmpdir) as reqs_file:
             options, args = command.parse_args(['-r', reqs_file])
-            command.get_requirements(
+            command.get_requirements(args, options, finder, session)
-                args, options, finder, session, wheel_cache=None,
-            )
         assert options.require_hashes
 
     def test_unsupported_hashes(self, data):

tests/unit/test_vcs.py

@@ -5,7 +5,7 @@ import pytest
 from mock import patch
 from pip._vendor.packaging.version import parse as parse_version
 
-from pip._internal.exceptions import BadCommand
+from pip._internal.exceptions import BadCommand, InstallationError
 from pip._internal.utils.misc import hide_url, hide_value
 from pip._internal.vcs import make_vcs_requirement_url
 from pip._internal.vcs.bazaar import Bazaar
@@ -292,6 +292,21 @@ def test_version_control__get_url_rev_and_auth__missing_plus(url):
     assert 'malformed VCS url' in str(excinfo.value)
 
 
+@pytest.mark.parametrize('url', [
+    # Test a URL with revision part as empty.
+    'git+https://github.com/MyUser/myProject.git@#egg=py_pkg',
+])
+def test_version_control__get_url_rev_and_auth__no_revision(url):
+    """
+    Test passing a URL to VersionControl.get_url_rev_and_auth() with
+    empty revision
+    """
+    with pytest.raises(InstallationError) as excinfo:
+        VersionControl.get_url_rev_and_auth(url)
+
+    assert 'an empty revision (after @)' in str(excinfo.value)
+
+
 @pytest.mark.parametrize('url, expected', [
     # Test http.
     ('bzr+http://bzr.myproject.org/MyProject/trunk/#egg=MyProject',