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 <[email protected]> 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 <[email protected]> --- 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 <[email protected]> Date: Fri, 18 Nov 2016 12:45:13 +0100 Subject: [PATCH 2/3] Break ipalib.x509 import cycle Signed-off-by: Christian Heimes <[email protected]> --- 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 <[email protected]> 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 <[email protected]> --- 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
