On 12.01.2016 10:23, Martin Babinsky wrote:
On 01/11/2016 06:38 PM, Martin Basti wrote:On 11.01.2016 17:58, Jan Cholasta wrote:On 11.1.2016 16:29, Martin Babinsky wrote:On 01/11/2016 02:27 PM, Martin Babinsky wrote:On 01/11/2016 02:01 PM, Jan Cholasta wrote:On 11.1.2016 13:14, Martin Babinsky wrote:On 01/11/2016 07:47 AM, Jan Cholasta wrote:On 8.1.2016 18:30, Lukas Slebodnik wrote:On (08/01/16 17:56), Martin Babinsky wrote:On 01/08/2016 05:23 PM, Lukas Slebodnik wrote:On (08/01/16 16:59), Martin Babinsky wrote:Patch 0122 reimplements version checking and fixes https://fedorahosted.org/freeipa/ticket/5572 Patch 0123 contains unit test for version checking code. Thanks to Martin^1 for the idea of using CFFI for calling rpm C-API directly. -- Martin^3 Babinsky>From c7a5d8970d100d071597b4e1d7cef8a27b8cd485 Mon Sep 17 00:00:00 2001From: Martin Babinsky <[email protected]> Date: Fri, 8 Jan 2016 15:54:00 +0100 Subject: [PATCH 2/3] tests for package version comparison These tests will ensure that our package version handling code can correctly decide when to upgrade IPA master. https://fedorahosted.org/freeipa/ticket/5572 --- ipatests/test_ipaserver/test_version_comparsion.py | 47 ++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 ipatests/test_ipaserver/test_version_comparsion.pydiff --git a/ipatests/test_ipaserver/test_version_comparsion.pyb/ipatests/test_ipaserver/test_version_comparsion.py new file mode 100644 index0000000000000000000000000000000000000000..51d069b23ba389ffce39e948cdbc7a1faaa84563--- /dev/null +++ b/ipatests/test_ipaserver/test_version_comparsion.py @@ -0,0 +1,47 @@ +# +# Copyright (C) 2015 FreeIPA Contributors see COPYING for license +# + +""" +tests for correct RPM version comparison +""" + +from ipaplatform.tasks import tasks +import pytest + +version_strings = [ + ("3.0.el6", "3.0.0.el6", "older"), + ("3.0.0.el6", "3.0.0.el6_8.2", "older"), + ("3.0.0-42.el6", "3.0.0.el6", "newer"), + ("3.0.0-1", "3.0.0-42", "older"), + ("3.0.0-42.el6", "3.3.3.fc20", "older"), + ("4.2.0-15.el7", "4.2.0-15.el7_2.3", "older"), + ("4.2.0-15.el7_2.3", "4.2.0-15.el7_2.3", "equal"), + ("4.2.0-1.fc23", "4.2.1.fc23", "older"), + ("4.2.3-alpha.fc23", "4.2.3-2.fc23", "older"), # numeric version elements have + # precedence over letters + ("4.3.90.201601080923GIT55aeea7-0.fc23", "4.3.0-1.fc23", "newer") +] + + [email protected](params=version_strings) +def versions(request): + return request.param + +class TestVersionComparsion(object): + + def test_versions(self, versions):+ version_string1, version_string2, expected_comparison =versions + + ver1 = tasks.parse_ipa_version(version_string1) + ver2 = tasks.parse_ipa_version(version_string2) + + if expected_comparison == "newer": + assert ver1 > ver2 + elif expected_comparison == "older": + assert ver1 < ver2 + elif expected_comparison == "equal": + assert ver1 == ver2 + else: + raise TypeError( + "Unexpected comparison string: {}", expected_comparison) -- 2.5.0>From 9677e1a3ca2f5837f1b779780127adf27efa81df Mon Sep 17 00:00:00 2001From: Martin Babinsky <[email protected]> Date: Fri, 8 Jan 2016 14:17:14 +0100 Subject: [PATCH 1/3] use FFI call to rpmvercmp function for version comparison Stop using rpm-python to compare package versions since the implicit NSS initialization upon the module import breaks NSS handling in IPA code. Call rpm-libs C-API function via CFFI instead. Big thanks to Martin Kosek <[email protected]> for sharing the code snippet that spurred this patch. https://fedorahosted.org/freeipa/ticket/5572 --- freeipa.spec.in | 2 +- ipaplatform/redhat/tasks.py | 59 +++++++++++++++++++++------------------------ 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/freeipa.spec.in b/freeipa.spec.in index7e956538d0f6c24bab636579303e0c7b5eeec199..7f31d41d16b2a26b404c277595f0994a21123f80100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -214,7 +214,7 @@ Requires: python-pyasn1 Requires: dbus-python Requires: python-dns >= 1.11.1 Requires: python-kdcproxy >= 0.3 -Requires: rpm-python +Requires: rpm-devel/usr/lib64/librpm.so.7 is provided by package rpm-libs sh$ rpm -qf /usr/lib64/librpm.so.7 rpm-libs-4.13.0-0.rc1.7.fc23.x86_64 It's not very common to depend on devel packages. LSI was basically trying workaround this http://fpaste.org/308652/22677521/rpm-libs contains librpm.so.* and cffi is smart enough to load right library with C = ffi.dlopen("rpm")This likely won't be an issue here, but in general, we should use the versioned library name to have at least some API/ABI guarantee. If for example a function we depend on changed signature between versions, we would crash *hard* when it's called.So it's enough to add "Requires: rpm-libs"but it would be almost noop because rpm-libs is everytime availableod fedora/rhel :-) "Requires: rpm-devel" add unnecessary additional runtime dependencies on pkgconfig, popt-devellibrpm.so.7 belongs to rpm-libs and librpm.so to rpm-devel and I was lazy (hey it's friday) to add path to librpm.so.7 to paths and use it i LD loader.If you think that it is not optimal I will fix it, but let's waitfor some more feedback.Sure wait for another python related comments.NACK. The ffi.cdef() and ffi.dlopen() should be in the module scope,and the ffi.new() calls are unnecessary.Attaching updated patches. I have added specific versions of librpmshared lib to platform specific namespaces. Both of them were testedand work.You haven't tested on any 32 bit architecture, have you? I'm sure /lib64 will not work there. Note that when you dlopen just "librpm.so.$N", the correct path according to dynamic linker configuration will be used.I have also change dependency to rpm-libs.Given librpm is dlopened by full path, I think we should depend directly on the "librpm.so.$N()" soname rather than rpm-libs.Updated patches attached.Attaching another version.Patch 0122: ACK Pushed to: master: 7cd99e852053710c64dcb66cd5b15fc8ed4da5de ipa-4-2: be9af721536b7c05e2ba5ffd7d72cc3727d64ff5 ipa-4-3: 6b6a11fda76e70d390d5c55f3aaeba8f455458c0 Patch 0123: LGTMPatch 0123: + raise TypeError(+ "Unexpected comparison string: {}", expected_comparison)Did you mean: raise TypeError( "Unexpected comparison string: {}".format(expected_comparison)) ?Sorry I temporarily forgot how to python over there. Attaching updated patch.
Works for me, but shouldn't be the file named test_version_compaRISOn.py instead of test_version_compaRSIOn.py?
-- 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
