From 0f762de90d4ae465274ce2430e9f55e56f416bd3 Mon Sep 17 00:00:00 2001
From: Bo Maryniuk <bo@suse.de>
Date: Wed, 30 Mar 2016 12:14:21 +0200
Subject: [PATCH 37/37] Force-sort the RPM output to ensure latest version of
 the multi-package on top of the list.

- Remove version_cmp from the yumpkg and use just a lowpkg alias
- Remove version_cmp from Zypper module and use just lowpkg alias
- Merge yumpkg's and zypper's version_cmp for a common use
- Sort installed pkgs data by version_cmp
- Move "string to EVR" function to the utilities
- Remove suse/redhat checks, refactor code.
- Fix condition from returning None on 0
- Remove tests from the zypper_test that belongs to rpm_test
- Add lowpkg tests for version comparison
- Fix lint
- Fix the documentation

Conflicts:
	salt/modules/yumpkg.py
---
 salt/modules/rpm.py               | 60 +++++++++++++++++++++++++++++++++++++--
 salt/modules/yumpkg.py            | 30 ++------------------
 salt/modules/zypper.py            | 58 +------------------------------------
 salt/utils/__init__.py            | 35 +++++++++++++++++++++++
 tests/unit/modules/rpm_test.py    | 21 ++++++++++++++
 tests/unit/modules/zypper_test.py | 22 --------------
 6 files changed, 118 insertions(+), 108 deletions(-)

diff --git a/salt/modules/rpm.py b/salt/modules/rpm.py
index 00cbd5d..e75bfa4 100644
--- a/salt/modules/rpm.py
+++ b/salt/modules/rpm.py
@@ -17,6 +17,19 @@ import salt.utils.pkg.rpm
 # pylint: disable=import-error,redefined-builtin
 from salt.ext.six.moves import shlex_quote as _cmd_quote
 from salt.ext.six.moves import zip
+
+try:
+    import rpm
+    HAS_RPM = True
+except ImportError:
+    HAS_RPM = False
+
+try:
+    import rpmUtils.miscutils
+    HAS_RPMUTILS = True
+except ImportError:
+    HAS_RPMUTILS = False
+
 # pylint: enable=import-error,redefined-builtin
 from salt.exceptions import CommandExecutionError, SaltInvocationError
 
@@ -490,7 +503,7 @@ def info(*packages, **attr):
     else:
         out = call['stdout']
 
-    ret = dict()
+    _ret = list()
     for pkg_info in re.split(r"----*", out):
         pkg_info = pkg_info.strip()
         if not pkg_info:
@@ -537,6 +550,49 @@ def info(*packages, **attr):
         if attr and 'description' in attr or not attr:
             pkg_data['description'] = os.linesep.join(descr)
         if pkg_name:
-            ret[pkg_name] = pkg_data
+            pkg_data['name'] = pkg_name
+            _ret.append(pkg_data)
+
+    # Force-sort package data by version,
+    # pick only latest versions
+    # (in case multiple packages installed, e.g. kernel)
+    ret = dict()
+    for pkg_data in reversed(sorted(_ret, cmp=lambda a_vrs, b_vrs: version_cmp(a_vrs['version'], b_vrs['version']))):
+        pkg_name = pkg_data.pop('name')
+        if pkg_name not in ret:
+            ret[pkg_name] = pkg_data.copy()
 
     return ret
+
+
+def version_cmp(ver1, ver2):
+    '''
+    .. versionadded:: 2015.8.9
+
+    Do a cmp-style comparison on two packages. Return -1 if ver1 < ver2, 0 if
+    ver1 == ver2, and 1 if ver1 > ver2. Return None if there was a problem
+    making the comparison.
+
+    CLI Example:
+
+    .. code-block:: bash
+
+        salt '*' pkg.version_cmp '0.2-001' '0.2.0.1-002'
+    '''
+    try:
+        if HAS_RPM:
+            cmp_func = rpm.labelCompare
+        elif HAS_RPMUTILS:
+            cmp_func = rpmUtils.miscutils.compareEVR
+        else:
+            cmp_func = None
+        cmp_result = cmp_func is None and 2 or cmp_func(salt.utils.str_version_to_evr(ver1),
+                                                        salt.utils.str_version_to_evr(ver2))
+        if cmp_result not in (-1, 0, 1):
+            raise Exception("Comparison result '{0}' is invalid".format(cmp_result))
+
+        return cmp_result
+    except Exception as exc:
+        log.warning("Failed to compare version '{0}' to '{1}' using RPM: {2}".format(ver1, ver2, exc))
+
+    return salt.utils.version_cmp(ver1, ver2)
diff --git a/salt/modules/yumpkg.py b/salt/modules/yumpkg.py
index 1d29928..1df5258 100644
--- a/salt/modules/yumpkg.py
+++ b/salt/modules/yumpkg.py
@@ -38,13 +38,7 @@ try:
 except ImportError:
     from salt.ext.six.moves import configparser
     HAS_YUM = False
-
-try:
-    import rpmUtils.miscutils
-    HAS_RPMUTILS = True
-except ImportError:
-    HAS_RPMUTILS = False
-# pylint: enable=import-error
+# pylint: enable=import-error,redefined-builtin
 
 # Import salt libs
 import salt.utils
@@ -592,26 +586,8 @@ def version_cmp(pkg1, pkg2):
 
         salt '*' pkg.version_cmp '0.2-001' '0.2.0.1-002'
     '''
-    if HAS_RPMUTILS:
-        try:
-            cmp_result = rpmUtils.miscutils.compareEVR(
-                rpmUtils.miscutils.stringToVersion(pkg1),
-                rpmUtils.miscutils.stringToVersion(pkg2)
-            )
-            if cmp_result not in (-1, 0, 1):
-                raise Exception(
-                    'cmp result \'{0}\' is invalid'.format(cmp_result)
-                )
-            return cmp_result
-        except Exception as exc:
-            log.warning(
-                'Failed to compare version \'{0}\' to \'{1}\' using '
-                'rpmUtils: {2}'.format(pkg1, pkg2, exc)
-            )
-    # Fall back to distutils.version.LooseVersion (should only need to do
-    # this for RHEL5, or if an exception is raised when attempting to compare
-    # using rpmUtils)
-    return salt.utils.version_cmp(pkg1, pkg2)
+
+    return __salt__['lowpkg.version_cmp'](pkg1, pkg2)
 
 
 def list_pkgs(versions_as_list=False, **kwargs):
diff --git a/salt/modules/zypper.py b/salt/modules/zypper.py
index 1c6b31d..445e423 100644
--- a/salt/modules/zypper.py
+++ b/salt/modules/zypper.py
@@ -17,12 +17,6 @@ import os
 import salt.ext.six as six
 from salt.ext.six.moves import configparser
 from salt.ext.six.moves.urllib.parse import urlparse as _urlparse
-
-try:
-    import rpm
-    HAS_RPM = True
-except ImportError:
-    HAS_RPM = False
 # pylint: enable=import-error,redefined-builtin,no-name-in-module
 
 from xml.dom import minidom as dom
@@ -346,40 +340,6 @@ def version(*names, **kwargs):
     return __salt__['pkg_resource.version'](*names, **kwargs) or {}
 
 
-def _string_to_evr(verstring):
-    '''
-    Split the version string into epoch, version and release and
-    return this as tuple.
-
-    epoch is always not empty.
-    version and release can be an empty string if such a component
-    could not be found in the version string.
-
-    "2:1.0-1.2" => ('2', '1.0', '1.2)
-    "1.0" => ('0', '1.0', '')
-    "" => ('0', '', '')
-    '''
-    if verstring in [None, '']:
-        return ('0', '', '')
-    idx_e = verstring.find(':')
-    if idx_e != -1:
-        try:
-            epoch = str(int(verstring[:idx_e]))
-        except ValueError:
-            # look, garbage in the epoch field, how fun, kill it
-            epoch = '0'  # this is our fallback, deal
-    else:
-        epoch = '0'
-    idx_r = verstring.find('-')
-    if idx_r != -1:
-        version = verstring[idx_e + 1:idx_r]
-        release = verstring[idx_r + 1:]
-    else:
-        version = verstring[idx_e + 1:]
-        release = ''
-    return (epoch, version, release)
-
-
 def version_cmp(ver1, ver2):
     '''
     .. versionadded:: 2015.5.4
@@ -394,23 +354,7 @@ def version_cmp(ver1, ver2):
 
         salt '*' pkg.version_cmp '0.2-001' '0.2.0.1-002'
     '''
-    if HAS_RPM:
-        try:
-            cmp_result = rpm.labelCompare(
-                _string_to_evr(ver1),
-                _string_to_evr(ver2)
-            )
-            if cmp_result not in (-1, 0, 1):
-                raise Exception(
-                    'cmp result \'{0}\' is invalid'.format(cmp_result)
-                )
-            return cmp_result
-        except Exception as exc:
-            log.warning(
-                'Failed to compare version \'{0}\' to \'{1}\' using '
-                'rpmUtils: {2}'.format(ver1, ver2, exc)
-            )
-    return salt.utils.version_cmp(ver1, ver2)
+    return __salt__['lowpkg.version_cmp'](ver1, ver2)
 
 
 def list_pkgs(versions_as_list=False, **kwargs):
diff --git a/salt/utils/__init__.py b/salt/utils/__init__.py
index 8c8309e..828a312 100644
--- a/salt/utils/__init__.py
+++ b/salt/utils/__init__.py
@@ -2882,3 +2882,38 @@ def split_input(val):
         return [x.strip() for x in val.split(',')]
     except AttributeError:
         return [x.strip() for x in str(val).split(',')]
+
+
+def str_version_to_evr(verstring):
+    '''
+    Split the package version string into epoch, version and release.
+    Return this as tuple.
+
+    The epoch is always not empty. The version and the release can be an empty
+    string if such a component could not be found in the version string.
+
+    "2:1.0-1.2" => ('2', '1.0', '1.2)
+    "1.0" => ('0', '1.0', '')
+    "" => ('0', '', '')
+    '''
+    if verstring in [None, '']:
+        return '0', '', ''
+
+    idx_e = verstring.find(':')
+    if idx_e != -1:
+        try:
+            epoch = str(int(verstring[:idx_e]))
+        except ValueError:
+            # look, garbage in the epoch field, how fun, kill it
+            epoch = '0'  # this is our fallback, deal
+    else:
+        epoch = '0'
+    idx_r = verstring.find('-')
+    if idx_r != -1:
+        version = verstring[idx_e + 1:idx_r]
+        release = verstring[idx_r + 1:]
+    else:
+        version = verstring[idx_e + 1:]
+        release = ''
+
+    return epoch, version, release
diff --git a/tests/unit/modules/rpm_test.py b/tests/unit/modules/rpm_test.py
index 8bfce9b..f180736 100644
--- a/tests/unit/modules/rpm_test.py
+++ b/tests/unit/modules/rpm_test.py
@@ -95,6 +95,27 @@ class RpmTestCase(TestCase):
             self.assertDictEqual(rpm.owner('/usr/bin/python', '/usr/bin/vim'),
                                  ret)
 
+    @patch('salt.modules.rpm.HAS_RPM', True)
+    def test_version_cmp_rpm(self):
+        '''
+        Test package version is called RPM version if RPM-Python is installed
+
+        :return:
+        '''
+        rpm.rpm = MagicMock(return_value=MagicMock)
+        with patch('salt.modules.rpm.rpm.labelCompare', MagicMock(return_value=0)):
+            self.assertEqual(0, rpm.version_cmp('1', '2'))  # mock returns 0, which means RPM was called
+
+    @patch('salt.modules.rpm.HAS_RPM', False)
+    def test_version_cmp_fallback(self):
+        '''
+        Test package version is called RPM version if RPM-Python is installed
+
+        :return:
+        '''
+        rpm.rpm = MagicMock(return_value=MagicMock)
+        with patch('salt.modules.rpm.rpm.labelCompare', MagicMock(return_value=0)):
+            self.assertEqual(-1, rpm.version_cmp('1', '2'))  # mock returns -1, a python implementation was called
 
 if __name__ == '__main__':
     from integration import run_tests
diff --git a/tests/unit/modules/zypper_test.py b/tests/unit/modules/zypper_test.py
index 5c4eb67..67cf52a 100644
--- a/tests/unit/modules/zypper_test.py
+++ b/tests/unit/modules/zypper_test.py
@@ -301,28 +301,6 @@ class ZypperTestCase(TestCase):
                 self.assertFalse(zypper.upgrade_available(pkg_name))
             self.assertTrue(zypper.upgrade_available('vim'))
 
-    @patch('salt.modules.zypper.HAS_RPM', True)
-    def test_version_cmp_rpm(self):
-        '''
-        Test package version is called RPM version if RPM-Python is installed
-
-        :return:
-        '''
-        with patch('salt.modules.zypper.rpm', MagicMock(return_value=MagicMock)):
-            with patch('salt.modules.zypper.rpm.labelCompare', MagicMock(return_value=0)):
-                self.assertEqual(0, zypper.version_cmp('1', '2'))  # mock returns 0, which means RPM was called
-
-    @patch('salt.modules.zypper.HAS_RPM', False)
-    def test_version_cmp_fallback(self):
-        '''
-        Test package version is called RPM version if RPM-Python is installed
-
-        :return:
-        '''
-        with patch('salt.modules.zypper.rpm', MagicMock(return_value=MagicMock)):
-            with patch('salt.modules.zypper.rpm.labelCompare', MagicMock(return_value=0)):
-                self.assertEqual(-1, zypper.version_cmp('1', '2'))  # mock returns -1, a python implementation was called
-
     def test_list_pkgs(self):
         '''
         Test packages listing.
-- 
2.1.4

