On 12/11/2015 09:36 AM, Martin Kosek wrote: > On 12/10/2015 05:09 PM, Martin Basti wrote: >> >> >> On 10.12.2015 15:49, Tomas Babej wrote: >>> >>> On 12/10/2015 11:23 AM, Martin Basti wrote: >>>> >>>> On 10.12.2015 09:13, Lukas Slebodnik wrote: >>>>> On (09/12/15 19:22), Martin Basti wrote: >>>>>> https://fedorahosted.org/freeipa/ticket/5535 >>>>>> >>>>>> Patch attached. >>>>> >From 8ef93485d61e8732166fb0c5b6c4559209740f3e Mon Sep 17 00:00:00 >>>>> 2001 >>>>>> From: Martin Basti <[email protected]> >>>>>> Date: Wed, 9 Dec 2015 18:53:35 +0100 >>>>>> Subject: [PATCH] Fix version comparison >>>>>> >>>>>> Use RPM library to compare vendor versions of IPA for redhat platform >>>>>> >>>>>> https://fedorahosted.org/freeipa/ticket/5535 >>>>>> --- >>>>>> freeipa.spec.in | 2 ++ >>>>>> ipaplatform/redhat/tasks.py | 19 +++++++++++++++++++ >>>>>> 2 files changed, 21 insertions(+) >>>>>> >>>>>> diff --git a/freeipa.spec.in b/freeipa.spec.in >>>>>> index >>>>>> 9f82b3695fb10c4db65cc31278364b3b34e26098..09feba7b8324f5e645da3e8010de86b6c3ee5ab9 >>>>>> >>>>>> >>>>>> 100644 >>>>>> --- a/freeipa.spec.in >>>>>> +++ b/freeipa.spec.in >>>>>> @@ -166,6 +166,8 @@ Requires: %{etc_systemd_dir} >>>>>> Requires: gzip >>>>>> Requires: python-gssapi >= 1.1.0 >>>>>> Requires: custodia >>>>>> +Requires: rpm-python >>>>>> +Requires: rpmdevtools >>>>> Could you explain why do you need the 2nd package? >>>>> It does not contains any python modules >>>>> and I cannot see usage of any binary in this patch >>>>> >>>>> LS >>>> Thanks for this catch, it is actually located in yum package, I rather >>>> copy stringToVersion function from there to IPA, to avoid dependency >>>> hell >>>> >>>> Updated patch attached. >>>> >>>> >>> Looking good. The __cmp__ function, however, is not available in Python >>> 3. As we will eventually support python3 in RHEL as well, maybe we >>> should make sure even platform-dependent parts are python3 compatible? >>> For the future's sake. >>> >>> Tomas >>> >> Thanks, >> >> python 3 compatible patch attached. > > I wonder why we have to add so much code here and reimplement RPM > version checking if it is already provided by rpmdevtools: > > ~~~ > $ /usr/bin/rpmdev-vercmp ipa-4.2.0-15.el7 4.2.0-15.el7_2.3; echo $? > WARNING: hyphen in release1: 4.2.0-15.el7 > > rpmdev-vercmp <epoch1> <ver1> <release1> <epoch2> <ver2> <release2> > rpmdev-vercmp <EVR1> <EVR2> > rpmdev-vercmp # with no arguments, prompt > > Exit status is 0 if the EVR's are equal, 11 if EVR1 is newer, and 12 if > EVR2 > is newer. Other exit statuses indicate problems. > > ipa-4.2.0-15.el7 < 4.2.0-15.el7_2.3 > 12 > ~~~ > > which could be directly called from __eq__ or __lt__, since we are in > platform specific code anyway already. > > Martin
Imho we should generally prefer reaching out to a Python library rather launching a subprocess to compare the versions, it's unnecessary overhead. That said, the main argument for the usage of rpm-python over rpmdevtools I see is that rpm-python is very likely to be present on the system (i.e. it is yum's own dependency), while rpmdevtools will not be present by default. >From the standpoint of trying to minimize the size of freeipa installation (i.e recent wget -> curl migration), we should keep the spirit here and do not introduce a dependency for a collection of developer tools. /2 cents Tomas -- 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
