Fix package NVR comparison in fedabipkgdiff

When sorting two packages by their {Name Value Release} triplet to
select the latest one, just doing a string comparison of the NVRs is
wrong.  Take for example the packages foo-0.10-1.fc25 and
foo-0.2-1.fc25.  A basic string comparison will result in the string
"foo-0.10-1.fc25" being less than "foo-0.2-1.fc25", and thus
foo-0.2-1.fc25 will be selected as the latest package.  And that is
wrong, because the latest one is obviously foo-0.10-1.fc25.

So, after some research on this, I figured rpm.labelCompare is a
better choice to appropriately compare two NVRs.

Another reason why I chose rpm.labelCompare is because the latest
build in fedabipkgdiff means a build with the latest version.release
within a specific Fedora distribution such as fc23 or fc25.

	* configure.ac: Add new dependency.
	* tests/runtestfedabipkgdiff.py.in (builds): Add new builds for
	running tests to test selecting latest build from a package.
	(packages): Add new package gnutls.
	(GetPackageLatestBuildTest.{test_get_latest_one,
	test_cannot_find_a_latest_build_with_invalid_distro}): Use new
	builds of package gnutls to run tests.
	* tools/fedabipkgdiff (cmp_nvr): New function used to compare nvrs
	by Python built-in function sorted.
	(Brew.listBuilds): Use the new cmp_nvr function.

Signed-off-by: Chenxiong Qi <cqi@redhat.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>

configure.ac

@@ -291,7 +291,7 @@ if test x$CHECK_DEPS_FOR_FEDABIPKGDIFF = xyes; then
 
   REQUIRED_PYTHON_MODULES_FOR_FEDABIPKGDIFF="\
    argparse logging os re subprocess sys itertools urlparse \
-   unittest xdg koji mock StringIO"
+   unittest xdg koji mock StringIO rpm"
 
   if test -x$ENABLE_FEDABIPKGDIFF != xno; then
     AX_CHECK_PYTHON_MODULES([$REQUIRED_PYTHON_MODULES_FOR_FEDABIPKGDIFF],

tests/runtestfedabipkgdiff.py.in

@@ -88,6 +88,7 @@ BUILT_ABIPKGDIFF = '@top_builddir@/tools/abipkgdiff'
 # database, and methods of MockClientSession knows well how to read them.
 
 packages = [
+    {'id': 286, 'name': 'gnutls'},
     {'id': 612, 'name': 'dbus-glib'},
     {'id': 9041, 'name': 'nss-util'},
     ]
@@ -126,6 +127,20 @@ builds = [
      'name': 'nss-util', 'version': '3.24.0', 'release': '2.0.fc25',
      'package_id': 9041, 'package_name': 'nss-util', 'state': 1,
      },
+
+    # builds of package gnutls
+    {'build_id': 767306, 'nvr': 'gnutls-3.4.12-1.fc23',
+     'name': 'gnutls', 'release': '1.fc23', 'version': '3.4.12',
+     'package_id': 286, 'package_name': 'gnutls', 'state': 1,
+     },
+    {'build_id': 770965, 'nvr': 'gnutls-3.4.13-1.fc23',
+     'name': 'gnutls', 'release': '1.fc23', 'version': '3.4.13',
+     'package_id': 286, 'package_name': 'gnutls', 'state': 1,
+     },
+    {'build_id': 649701, 'nvr': 'gnutls-3.4.2-1.fc23',
+     'name': 'gnutls', 'release': '1.fc23', 'version': '3.4.2',
+     'package_id': 286, 'package_name': 'gnutls', 'state': 1,
+     },
     ]
 
 rpms = [
@@ -859,8 +874,8 @@ class GetPackageLatestBuildTest(unittest.TestCase):
     def test_get_latest_one(self):
         """Ensure to get latest build of a package"""
         session = fedabipkgdiff_mod.get_session()
-        build = session.get_package_latest_build('dbus-glib', 'fc23')
-        self.assertEquals('dbus-glib-0.106-1.fc23', build['nvr'])
+        build = session.get_package_latest_build('gnutls', 'fc23')
+        self.assertEquals('gnutls-3.4.13-1.fc23', build['nvr'])
 
     @patch('fedabipkgdiff.global_config', new=MockGlobalConfig)
     @patch('koji.ClientSession', new=MockClientSession)
@@ -872,7 +887,7 @@ class GetPackageLatestBuildTest(unittest.TestCase):
         session = fedabipkgdiff_mod.get_session()
         self.assertRaises(fedabipkgdiff_mod.NoCompleteBuilds,
                           session.get_package_latest_build,
-                          'dbus-glib', 'xxxx')
+                          'gnutls', 'xxxx')
 
 
 class DownloadRPMTest(unittest.TestCase):

tools/fedabipkgdiff

@@ -34,6 +34,7 @@ from itertools import groupby
 
 import xdg.BaseDirectory
 
+import rpm
 import koji
 
 # @file
@@ -152,6 +153,26 @@ def is_distro_valid(distro):
     return re.match(r'^(fc|el)\d{1,2}$', distro) is not None
 
 
+def cmp_nvr(left, right):
+    """Compare function for sorting a sequence of NVRs
+
+    This is the compare function used in sorted function to sort builds so that
+    fedabipkgdiff is able to select the latest build. Return value follows the
+    rules described in the part of paramter cmp of sorted documentation.
+
+    :param str left: left nvr to compare.
+    :param str right: right nvr to compare.
+    :return: -1, 0, or 1 that represents left is considered smaller than,
+    equal to, or larger than the right individually.
+    :rtype: int
+    """
+    left_nvr = koji.parse_NVR(left)
+    right_nvr = koji.parse_NVR(right)
+    return rpm.labelCompare(
+        (left_nvr['epoch'], left_nvr['version'], left_nvr['release']),
+        (right_nvr['epoch'], right_nvr['version'], right_nvr['release']))
+
+
 def log_call(func):
     """A decorator that logs a method invocation
 
@@ -451,8 +472,10 @@ class Brew(object):
         if order_by is not None:
             # FIXME: is it possible to sort builds by using opts parameter of
             # listBuilds
+            cmp_func = cmp_nvr if order_by == 'nvr' else None
             builds = sorted(builds,
                             key=lambda item: item[order_by],
+                            cmp=cmp_func,
                             reverse=reverse)
         if topone:
             builds = builds[0:1]