URL: https://github.com/freeipa/freeipa/pull/254
Author: tiran
 Title: #254: Replace LooseVersion with parse_ipa_version()
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/254/head:pr254
git checkout pr254
From e49896c316bae2a6ea4f3b37973a024fd024a0a8 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Fri, 18 Nov 2016 12:24:09 +0100
Subject: [PATCH 1/3] Minor fixes for IPAVersion class

Py3: classes with __eq__ must provide __hash__ function or set __hash__
to None.
Comparison function like __eq__ must signal unsupported types by
returning NotImplemented. Python turns this in a proper TypeError.
Make the version member read-only and cache _bytes represention.

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipaplatform/redhat/tasks.py | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index dbe005a..5d627be 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -83,20 +83,26 @@ def selinux_enabled():
 class IPAVersion(object):
 
     def __init__(self, version):
-        self.version = version
+        self._version = version
+        self._bytes = version.encode('utf-8')
 
     @property
-    def _bytes(self):
-        return self.version.encode('utf-8')
+    def version(self):
+        return self._version
 
     def __eq__(self, other):
-        assert isinstance(other, IPAVersion)
+        if not isinstance(other, IPAVersion):
+            return NotImplemented
         return _librpm.rpmvercmp(self._bytes, other._bytes) == 0
 
     def __lt__(self, other):
-        assert isinstance(other, IPAVersion)
+        if not isinstance(other, IPAVersion):
+            return NotImplemented
         return _librpm.rpmvercmp(self._bytes, other._bytes) < 0
 
+    def __hash__(self):
+        return hash(self._version)
+
 
 class RedHatTaskNamespace(BaseTaskNamespace):
 

From dd47e974bb497c296ae505fbc320a95e3221c95e Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Fri, 18 Nov 2016 12:45:13 +0100
Subject: [PATCH 2/3] Break ipalib.x509 import cycle

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipaplatform/redhat/tasks.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 5d627be..74ca5f1 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -44,8 +44,6 @@
 from ipapython import ipautil
 import ipapython.errors
 
-from ipalib import x509 # FIXME: do not import from ipalib
-
 from ipaplatform.constants import constants
 from ipaplatform.paths import paths
 from ipaplatform.redhat.authconfig import RedHatAuthConfig
@@ -220,6 +218,9 @@ def reload_systemwide_ca_store(self):
             return True
 
     def insert_ca_certs_into_systemwide_ca_store(self, ca_certs):
+        # break import cycle
+        from ipalib import x509
+
         new_cacert_path = paths.SYSTEMWIDE_IPA_CA_CRT
 
         if os.path.exists(new_cacert_path):

From 87265d9a7d849ea78f8e1e5b416f5b934637b7cb Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Fri, 18 Nov 2016 09:26:18 +0100
Subject: [PATCH 3/3] Replace LooseVersion with tasks.parse_ipa_version

pylint is having a hard time with distutils.version in tox's virtual
envs. virtualenv uses some tricks to provide a virtual distutils
package, pylint can't cope with.

https://github.com/PyCQA/pylint/issues/73 suggests to use pkg_resources
instead. parse_version() can't handle some RPM-specific versions
correctly. Let's use our own version parser everywhere.

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipaclient/remote_plugins/compat.py         | 13 ++++++-------
 ipalib/capabilities.py                     |  6 +++---
 ipalib/frontend.py                         |  8 +++-----
 ipalib/plugable.py                         |  7 +++----
 ipaserver/install/krbinstance.py           |  6 +++---
 ipaserver/install/server/replicainstall.py |  7 +++----
 6 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/ipaclient/remote_plugins/compat.py b/ipaclient/remote_plugins/compat.py
index 984eecd..b40bafb 100644
--- a/ipaclient/remote_plugins/compat.py
+++ b/ipaclient/remote_plugins/compat.py
@@ -2,7 +2,6 @@
 # Copyright (C) 2016  FreeIPA Contributors see COPYING for license
 #
 
-from distutils.version import LooseVersion
 import importlib
 import os
 import re
@@ -12,6 +11,7 @@
 
 from ipaclient.frontend import ClientCommand, ClientMethod
 from ipalib.frontend import Object
+from ipaplatform.tasks import tasks
 
 if six.PY3:
     unicode = str
@@ -58,7 +58,7 @@ def get_package(server_info, client):
         server_info['version'] = server_version
         server_info.update_validity()
 
-    server_version = LooseVersion(server_version)
+    server_version = tasks.parse_ipa_version(server_version)
 
     package_names = {}
     base_name = __name__.rpartition('.')[0]
@@ -66,15 +66,14 @@ def get_package(server_info, client):
     for name in os.listdir(base_dir):
         package_dir = os.path.join(base_dir, name)
         if name.startswith('2_') and os.path.isdir(package_dir):
-            package_version = name.replace('_', '.')
+            package_version = tasks.parse_ipa_version(name.replace('_', '.'))
             package_names[package_version] = '{}.{}'.format(base_name, name)
 
     package_version = None
-    for version in sorted(package_names, key=LooseVersion):
-        if (package_version is None or
-                LooseVersion(package_version) < LooseVersion(version)):
+    for version in sorted(package_names):
+        if package_version is None or package_version < version:
             package_version = version
-        if LooseVersion(version) >= server_version:
+        if version >= server_version:
             break
 
     package_name = package_names[package_version]
diff --git a/ipalib/capabilities.py b/ipalib/capabilities.py
index 7ddaea2..4c62308 100644
--- a/ipalib/capabilities.py
+++ b/ipalib/capabilities.py
@@ -25,7 +25,7 @@
 versions they were introduced in.
 """
 
-from distutils import version
+from ipaplatform.tasks import tasks
 
 VERSION_WITHOUT_CAPABILITIES = u'2.51'
 
@@ -64,6 +64,6 @@ def client_has_capability(client_version, capability):
     :param client_version: The API version string reported by the client
     """
 
-    version_tuple = version.LooseVersion(client_version)
+    version = tasks.parse_ipa_version(client_version)
 
-    return version_tuple >= version.LooseVersion(capabilities[capability])
+    return version >= tasks.parse_ipa_version(capabilities[capability])
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index c94d174..e870319 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -20,9 +20,7 @@
 """
 Base classes for all front-end plugins.
 """
-
-from distutils import version
-
+from ipaplatform.tasks import tasks
 import six
 
 from ipapython.version import API_VERSION
@@ -770,8 +768,8 @@ def verify_client_version(self, client_version):
         If the client minor version is less than or equal to the server
         then let the request proceed.
         """
-        server_ver = version.LooseVersion(API_VERSION)
-        ver = version.LooseVersion(client_version)
+        server_ver = tasks.parse_ipa_version(API_VERSION)
+        ver = tasks.parse_ipa_version(client_version)
         if len(ver.version) < 2:
             raise VersionError(cver=ver.version, sver=server_ver.version, server= self.env.xmlrpc_uri)
         client_major = ver.version[0]
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index 76fb9fd..1189c4b 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -24,8 +24,6 @@
 you are unfamiliar with this Python feature, see
 http://docs.python.org/ref/sequence-types.html
 """
-
-from distutils.version import LooseVersion
 import operator
 import sys
 import threading
@@ -44,6 +42,7 @@
 from ipalib.util import classproperty
 from ipalib.base import ReadOnly, lock, islocked
 from ipalib.constants import DEFAULT_CONFIG
+from ipaplatform.tasks import tasks
 from ipapython import ipautil
 from ipapython.ipa_log_manager import (
     log_mgr,
@@ -725,8 +724,8 @@ def finalize(self):
                 except KeyError:
                     pass
                 else:
-                    version = LooseVersion(plugin.version)
-                    default_version = LooseVersion(default_version)
+                    version = tasks.parse_ipa_version(plugin.version)
+                    default_version = tasks.parse_ipa_version(default_version)
                     if version < default_version:
                         continue
             self.__default_map[plugin.name] = plugin.version
diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index b5cfd79..02ecee2 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -24,6 +24,7 @@
 import os
 import pwd
 import socket
+
 import dns.name
 
 from ipaserver.install import service
@@ -38,7 +39,6 @@
 from ipaserver.install import ldapupdate
 
 from ipaserver.install import certs
-from distutils import version
 from ipaplatform.constants import constants
 from ipaplatform.tasks import tasks
 from ipaplatform.paths import paths
@@ -291,8 +291,8 @@ def __configure_instance(self):
                              raiseonerr=False, capture_output=True)
         if result.returncode == 0:
             verstr = result.output.split()[-1]
-            ver = version.LooseVersion(verstr)
-            min = version.LooseVersion(MIN_KRB5KDC_WITH_WORKERS)
+            ver = tasks.parse_ipa_version(verstr)
+            min = tasks.parse_ipa_version(MIN_KRB5KDC_WITH_WORKERS)
             if ver >= min:
                 workers = True
         # Write down config file
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index a7b333c..aabc98a 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -4,7 +4,6 @@
 
 from __future__ import print_function
 
-from distutils.version import LooseVersion
 import dns.exception as dnsexception
 import dns.name as dnsname
 import dns.resolver as dnsresolver
@@ -510,9 +509,9 @@ def check_remote_version(api):
     finally:
         client.disconnect()
 
-    remote_version = env['version']
-    version = api.env.version
-    if LooseVersion(remote_version) > LooseVersion(version):
+    remote_version = tasks.parse_ipa_version(env['version'])
+    api_version = tasks.parse_ipa_version(api.env.version)
+    if remote_version > api_version:
         raise RuntimeError(
             "Cannot install replica of a server of higher version ({}) than"
             "the local version ({})".format(remote_version, version))
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to