fix version sort in PackageFinder

pip/index.py

@@ -156,7 +156,7 @@ def mkurl_pypi_url(url):
             logger.fatal('Could not find any downloads that satisfy the requirement %s' % req)
             raise DistributionNotFound('No distributions at all found for %s' % req)
         if req.satisfied_by is not None:
-            found_versions.append((req.satisfied_by.parsed_version, Inf, req.satisfied_by.version))
+            found_versions.append((req.satisfied_by.parsed_version, InfLink, req.satisfied_by.version))
         if file_versions:
             file_versions.sort(reverse=True)
             logger.info('Local files found: %s' % ', '.join([url_to_path(link.url) for parsed, link, version in file_versions]))
@@ -168,11 +168,13 @@ def mkurl_pypi_url(url):
                 logger.info("Ignoring link %s, version %s doesn't match %s"
                             % (link, version, ','.join([''.join(s) for s in req.req.specs])))
                 continue
-            applicable_versions.append((link, version))
-        applicable_versions = sorted(applicable_versions, key=lambda v: pkg_resources.parse_version(v[1]), reverse=True)
-        existing_applicable = bool([link for link, version in applicable_versions if link is Inf])
+            applicable_versions.append((parsed_version, link, version))
+        applicable_versions = [(link, version) for parsed_version, link, version in self._sort_versions(applicable_versions)]
+        existing_applicable = bool([link for link, version in applicable_versions if link is InfLink])
         if not upgrade and existing_applicable:
-            if applicable_versions[0][1] is Inf:
+            if applicable_versions[0][1] is InfLink:
+                #FIXME: [0][1] will never find InfLink
+                #this should not be casually fixed, as this has been baked in like this for awhile
                 logger.info('Existing installed version (%s) is most up-to-date and satisfies requirement'
                             % req.satisfied_by.version)
                 raise BestVersionAlreadyInstalled
@@ -184,7 +186,7 @@ def mkurl_pypi_url(url):
             logger.fatal('Could not find a version that satisfies the requirement %s (from versions: %s)'
                          % (req, ', '.join([version for parsed_version, link, version in found_versions])))
             raise DistributionNotFound('No distributions matching the version for %s' % req)
-        if applicable_versions[0][0] is Inf:
+        if applicable_versions[0][0] is InfLink:
             # We have an existing version, and its the best version
             logger.info('Installed version (%s) is most up-to-date (past versions: %s)'
                         % (req.satisfied_by.version, ', '.join([version for link, version in applicable_versions[1:]]) or 'none'))
@@ -194,6 +196,14 @@ def mkurl_pypi_url(url):
                         (applicable_versions[0][1], ', '.join([version for link, version in applicable_versions])))
         return applicable_versions[0][0]
 
+    def _sort_versions(self, versions):
+        """
+        Return version tuples sorted in reverse order.
+        If a tuple containing InfLink is tied as the latest version,
+        then that tuple will be sorted as first.
+        """
+        return sorted(versions, reverse=True)
+
     def _find_url_name(self, index_url, url_name, req):
         """Finds the true URL name of a package, when the given name isn't quite correct.
         This is usually used to implement case-insensitivity."""
@@ -586,14 +596,20 @@ def __str__(self):
         if self.comes_from:
             return '%s (from %s)' % (self.url, self.comes_from)
         else:
-            return self.url
+            return str(self.url)
 
     def __repr__(self):
         return '<Link %s>' % self
 
     def __eq__(self, other):
         return self.url == other.url
 
+    def __lt__(self, other):
+        return self.url < other.url
+
+    def __gt__(self, other):
+        return self.url > other.url
+
     def __hash__(self):
         return hash(self.url)
 
@@ -649,6 +665,9 @@ def hash_name(self):
     def show_url(self):
         return posixpath.basename(self.url.split('#', 1)[0].split('?', 1)[0])
 
+#An "Infinite Link" that compares greater than other links
+InfLink = Link(Inf)
+
 
 def get_requirement_from_url(url):
     """Get a requirement from the URL, if possible.  This looks for #egg

tests/test_index.py

@@ -1,4 +1,6 @@
-from pip.index import package_to_requirement, HTMLPage
+from pip.index import package_to_requirement, HTMLPage, Link, InfLink
+from pip.index import PackageFinder
+from pkg_resources import parse_version
 
 
 def test_package_name_should_be_converted_to_requirement():
@@ -26,3 +28,41 @@ def test_html_page_should_be_able_to_scrap_rel_links():
     assert len(links) == 1
     assert links[0].url == 'http://supervisord.org/'
 
+
+def test_inflink_greater():
+    """Test InfLink compares greater."""
+    assert InfLink > Link(object())
+
+
+def test_version_sort():
+    """Test version sorting."""
+    finder = PackageFinder(None, None)
+    versions = []
+    versions.append((parse_version('3.0'), Link('link3'), '3.0'))
+    versions.append((parse_version('2.0'), Link('link2'), '2.0'))
+    assert finder._sort_versions(versions)[0][2] == '3.0'
+    versions.reverse()
+    assert finder._sort_versions(versions)[0][2] == '3.0'
+
+
+def test_version_sort_inflink_latest():
+    """Test version sorting with InfLink tied as latest version."""
+    finder = PackageFinder(None, None)
+    versions = []
+    versions.append((parse_version('2.0'), Link('link'), '2.0'))
+    versions.append((parse_version('2.0'), InfLink, '2.0'))
+    assert finder._sort_versions(versions)[0][1] is InfLink
+    versions.reverse()
+    assert finder._sort_versions(versions)[0][1] is InfLink
+
+
+def test_version_sort_inflink_not_latest():
+    """Test version sorting with InfLink not latest version."""
+    finder = PackageFinder(None, None)
+    versions = []
+    versions.append((parse_version('3.0'), Link('link'), '3.0'))
+    versions.append((parse_version('2.0'), InfLink, '2.0'))
+    assert finder._sort_versions(versions)[0][2] == '3.0'
+    versions.reverse()
+    assert finder._sort_versions(versions)[0][2] == '3.0'
+

tests/test_util.py

@@ -8,6 +8,7 @@
 from nose.tools import eq_
 from tests.path import Path
 from pip.util import egg_link_path
+from pip.util import Inf
 
 
 class Tests_EgglinkPath:
@@ -138,3 +139,7 @@ def test_noegglink_in_sitepkgs_venv_global(self):
         self.mock_isfile.return_value = False
         eq_(egg_link_path(self.mock_dist), None)
 
+
+def test_inf_greater():
+    """Test Inf compares greater."""
+    assert Inf > object()