Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
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
>> 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
> rpmdev-vercmp
> 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
The version comparing was discussed again as current way causes some issues.
JFTR, the example above is not correct, this is the right way of calling it:
$ rpmdev-vercmp 4.2.0-15.el7 4.2.0-15.el7_2.3; echo $?
4.2.0-15.el7 < 4.2.0-15.el7_2.3
12
One thing discussed is that rpmdevtools require Perl. Looking at currenet
FreeIPA dependencies, we do require it already, but in general, it would be
nice to get rid of it. But as for now, we did not come up with a better idea
for above other than reimplementing it all in FreeIPA python code.
Martin
--
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
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
On 22.12.2015 14:10, Martin Basti wrote:
On 22.12.2015 14:04, Petr Spacek wrote:
On 14.12.2015 14:29, Tomas Babej wrote:
On 12/14/2015 10:21 AM, Martin Basti wrote:
On 14.12.2015 09:24, Martin Kosek wrote:
On 12/14/2015 07:21 AM, Jan Cholasta wrote:
On 11.12.2015 19:01, Tomas Babej wrote:
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
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
rpmdev-vercmp
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.
I would not be too worried about miliseconds longer execution on a
function
called during RPM upgrade.
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
+1, also a single function is hardly "much code".
Ok then. If you all want to add yet another N-V-R parsing function
in the
FreeIPA code, I can live with it (with raised eyebrows though).
Rebased patch attached.
I tested the patch, and it works fine - so conditional ACK from me for
the current iteration of the patch, given developer consensus which was
not reached yet.
There's a split of opinions (external binary camp vs. copy&paste camp),
so we need to decide if we both camps are OK with proceeding.
Further inspection shows that rpmdevtools depends on Perl stack which
seems to
be too much for such a simple thing. So, I'm hesitantly changing my
NACK to
ACK for copy&paste camp.
It seems that copy&paste camp won.
Pushed to:
master: 91913c5ba7c380fe6456e1c3e35fcbfbecef5ff1
ipa-4-3: a249de3b00f20956214a6ee0c1d614b972826637
Patch for ipa-4-2 needs rebase ... wait for it please
Martin^2
Pushed to ipa-4-2: dccdade484698d5ef553f03ce6aab5d9db7a7cf0
--
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
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
On 22.12.2015 14:04, Petr Spacek wrote:
On 14.12.2015 14:29, Tomas Babej wrote:
On 12/14/2015 10:21 AM, Martin Basti wrote:
On 14.12.2015 09:24, Martin Kosek wrote:
On 12/14/2015 07:21 AM, Jan Cholasta wrote:
On 11.12.2015 19:01, Tomas Babej wrote:
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
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
rpmdev-vercmp
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.
I would not be too worried about miliseconds longer execution on a
function
called during RPM upgrade.
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
+1, also a single function is hardly "much code".
Ok then. If you all want to add yet another N-V-R parsing function in the
FreeIPA code, I can live with it (with raised eyebrows though).
Rebased patch attached.
I tested the patch, and it works fine - so conditional ACK from me for
the current iteration of the patch, given developer consensus which was
not reached yet.
There's a split of opinions (external binary camp vs. copy&paste camp),
so we need to decide if we both camps are OK with proceeding.
Further inspection shows that rpmdevtools depends on Perl stack which seems to
be too much for such a simple thing. So, I'm hesitantly changing my NACK to
ACK for copy&paste camp.
It seems that copy&paste camp won.
Pushed to:
master: 91913c5ba7c380fe6456e1c3e35fcbfbecef5ff1
ipa-4-3: a249de3b00f20956214a6ee0c1d614b972826637
Patch for ipa-4-2 needs rebase ... wait for it please
Martin^2
--
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
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
On 14.12.2015 14:29, Tomas Babej wrote:
>
>
> On 12/14/2015 10:21 AM, Martin Basti wrote:
>>
>>
>> On 14.12.2015 09:24, Martin Kosek wrote:
>>> On 12/14/2015 07:21 AM, Jan Cholasta wrote:
On 11.12.2015 19:01, Tomas Babej wrote:
>
> 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
>>> 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
>> rpmdev-vercmp
>> 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.
>>> I would not be too worried about miliseconds longer execution on a
>>> function
>>> called during RPM upgrade.
>>>
> 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
+1, also a single function is hardly "much code".
>>> Ok then. If you all want to add yet another N-V-R parsing function in the
>>> FreeIPA code, I can live with it (with raised eyebrows though).
>>
>> Rebased patch attached.
>
> I tested the patch, and it works fine - so conditional ACK from me for
> the current iteration of the patch, given developer consensus which was
> not reached yet.
>
> There's a split of opinions (external binary camp vs. copy&paste camp),
> so we need to decide if we both camps are OK with proceeding.
Further inspection shows that rpmdevtools depends on Perl stack which seems to
be too much for such a simple thing. So, I'm hesitantly changing my NACK to
ACK for copy&paste camp.
--
Petr^2 Spacek
--
Manage your subscription for the Freeipa-dev
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
On 12/14/2015 10:21 AM, Martin Basti wrote:
>
>
> On 14.12.2015 09:24, Martin Kosek wrote:
>> On 12/14/2015 07:21 AM, Jan Cholasta wrote:
>>> On 11.12.2015 19:01, Tomas Babej wrote:
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
>> 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
> rpmdev-vercmp
> 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.
>> I would not be too worried about miliseconds longer execution on a
>> function
>> called during RPM upgrade.
>>
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
>>> +1, also a single function is hardly "much code".
>> Ok then. If you all want to add yet another N-V-R parsing function in the
>> FreeIPA code, I can live with it (with raised eyebrows though).
>
> Rebased patch attached.
I tested the patch, and it works fine - so conditional ACK from me for
the current iteration of the patch, given developer consensus which was
not reached yet.
There's a split of opinions (external binary camp vs. copy&paste camp),
so we need to decide if we both camps are OK with proceeding.
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
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
On 14.12.2015 09:24, Martin Kosek wrote:
On 12/14/2015 07:21 AM, Jan Cholasta wrote:
On 11.12.2015 19:01, Tomas Babej wrote:
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
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
rpmdev-vercmp
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.
I would not be too worried about miliseconds longer execution on a function
called during RPM upgrade.
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
+1, also a single function is hardly "much code".
Ok then. If you all want to add yet another N-V-R parsing function in the
FreeIPA code, I can live with it (with raised eyebrows though).
Rebased patch attached.
From c15bb5a71a8a1fb065b4ffdaee0de0554569ec9f Mon Sep 17 00:00:00 2001
From: Martin Basti
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 | 1 +
ipaplatform/redhat/tasks.py | 53 +
2 files changed, 54 insertions(+)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 09bd62a4f8ad7fd739d7422750bc0054a3afef9d..4002666df1952af59115da7f36386afc580f2ff4 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -204,6 +204,7 @@ Requires: python-pyasn1
Requires: dbus-python
Requires: python-dns >= 1.11.1
Requires: python-kdcproxy >= 0.3
+Requires: rpm-python
%description -n python2-ipaserver
IPA is an integrated solution to provide centrally managed Identity (users,
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 94d2cb4e906965a20bcfdd55f38854005091c26f..c90e55f28cd492d43a98e4e0ee0476a23db8a099 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -30,11 +30,13 @@ import stat
import socket
import sys
import base64
+from functools import total_ordering
from subprocess import CalledProcessError
from nss.error import NSPRError
from pyasn1.error import PyAsn1Error
from six.moves import urllib
+import rpm
from ipapython.ipa_log_manager import root_logger, log_mgr
from ipapython import ipautil
@@ -47,6 +49,35 @@ from ipaplatform.redhat.authconfig import RedHatAuthConfig
from ipaplatform.base.tasks import BaseTaskNamespace
+
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
On 14.12.2015 09:24, Martin Kosek wrote:
> On 12/14/2015 07:21 AM, Jan Cholasta wrote:
>> On 11.12.2015 19:01, Tomas Babej wrote:
>>>
>>>
>>> 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
> 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
rpmdev-vercmp
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.
>
> I would not be too worried about miliseconds longer execution on a function
> called during RPM upgrade.
>
>>> 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
>>
>> +1, also a single function is hardly "much code".
>
> Ok then. If you all want to add yet another N-V-R parsing function in the
> FreeIPA code, I can live with it (with raised eyebrows though).
+1
We already tried and failed in custom parsing (presumably caused by request to
not add any new dependency). I agree with Martin that copy&pasting code does
not look like a proper fix.
--
Petr^2 Spacek
--
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
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
On 12/14/2015 07:21 AM, Jan Cholasta wrote:
> On 11.12.2015 19:01, Tomas Babej wrote:
>>
>>
>> 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
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
>>> rpmdev-vercmp
>>> 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.
I would not be too worried about miliseconds longer execution on a function
called during RPM upgrade.
>> 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
>
> +1, also a single function is hardly "much code".
Ok then. If you all want to add yet another N-V-R parsing function in the
FreeIPA code, I can live with it (with raised eyebrows though).
--
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
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
On 11.12.2015 19:01, Tomas Babej wrote:
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
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
rpmdev-vercmp
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
+1, also a single function is hardly "much code".
--
Jan Cholasta
--
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
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
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
>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
--
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
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
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
>> 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
> rpmdev-vercmp
> 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
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
On 11.12.2015 09:36, 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
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
rpmdev-vercmp
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
I do not like the idea of calling external tool, but I have no other
objections except my personal feeling.
Martin^2
--
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
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
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
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
rpmdev-vercmp
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
--
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
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
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
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.
From 0e5c42ac282f47138e106e4884e11bc5ff7ab12b Mon Sep 17 00:00:00 2001
From: Martin Basti
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 | 1 +
ipaplatform/redhat/tasks.py | 53 +
2 files changed, 54 insertions(+)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 9f82b3695fb10c4db65cc31278364b3b34e26098..7a5565c497b0dd20ed73af3112a37ac25d2abe6e 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -166,6 +166,7 @@ Requires: %{etc_systemd_dir}
Requires: gzip
Requires: python-gssapi >= 1.1.0
Requires: custodia
+Requires: rpm-python
Provides: %{alt_name}-server = %{version}
Conflicts: %{alt_name}-server
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 94d2cb4e906965a20bcfdd55f38854005091c26f..c90e55f28cd492d43a98e4e0ee0476a23db8a099 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -30,11 +30,13 @@ import stat
import socket
import sys
import base64
+from functools import total_ordering
from subprocess import CalledProcessError
from nss.error import NSPRError
from pyasn1.error import PyAsn1Error
from six.moves import urllib
+import rpm
from ipapython.ipa_log_manager import root_logger, log_mgr
from ipapython import ipautil
@@ -47,6 +49,35 @@ from ipaplatform.redhat.authconfig import RedHatAuthConfig
from ipaplatform.base.tasks import BaseTaskNamespace
+# copied from rpmUtils/miscutils.py
+def stringToVersion(verstring):
+if verstring in [None, '']:
+return (None, None, None)
+i = verstring.find(':')
+if i != -1:
+try:
+epoch = str(long(verstring[:i]))
+except ValueError:
+# look, garbage in the epoch field, how fun, kill it
+epoch = '0' # this is our fallback, deal
+else:
+epoch = '0'
+j = verstring.find('-')
+if j != -1:
+if verstring[i + 1:j] == '':
+version = None
+else:
+version = verstring[i + 1:j]
+release = verstring[j + 1:]
+else:
+if verstring[i + 1:] == '':
+version = None
+else:
+version = verstring[i + 1:]
+release = None
+return (epoch, version, release)
+
+
log = log_mgr.get_logger(__name__)
@@ -66,6 +97,21 @@ def selinux_enabled():
return False
+@total_ordering
+class IPAVersion(object):
+
+def __init__(self, version):
+self.version_tuple = stringToVersion(version)
+
+def __eq__(self, other):
+assert isinstance(other, IPAVersion)
+return rpm.labelCompare(self.version_tuple, other.version_tuple) == 0
+
+def __lt__(self, other):
+assert isinstance(other, IPAVersion)
+return rpm.labelCompare(self.version_tuple, other.version_tuple) == -1
+
+
class RedHatTaskNamespace(BaseTaskNamespace):
def restore_context(self, filepath, restorecon=paths.SBIN_RESTORECON):
@@ -423,5 +469,12 @@ class RedHatTaskNamespace(BaseTaskNamespace):
super(RedHatTaskNamespace, self).create_system_user(name, group,
homedir, shell, uid, gid, comment, create_homedir)
+def parse_ipa_version(self, version):
+"""
+:param version: textu
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
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
>>> 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
--
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
Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
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
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.
From 2cce3c5d0aa7de88e7a0a69af8a0e8e803bd8305 Mon Sep 17 00:00:00 2001
From: Martin Basti
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 | 1 +
ipaplatform/redhat/tasks.py | 47 +
2 files changed, 48 insertions(+)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 9f82b3695fb10c4db65cc31278364b3b34e26098..7a5565c497b0dd20ed73af3112a37ac25d2abe6e 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -166,6 +166,7 @@ Requires: %{etc_systemd_dir}
Requires: gzip
Requires: python-gssapi >= 1.1.0
Requires: custodia
+Requires: rpm-python
Provides: %{alt_name}-server = %{version}
Conflicts: %{alt_name}-server
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 94d2cb4e906965a20bcfdd55f38854005091c26f..9c975f9627ebd7207c1390aab264338972dbaff6 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -30,6 +30,7 @@ import stat
import socket
import sys
import base64
+import rpm
from subprocess import CalledProcessError
from nss.error import NSPRError
@@ -47,6 +48,35 @@ from ipaplatform.redhat.authconfig import RedHatAuthConfig
from ipaplatform.base.tasks import BaseTaskNamespace
+# copied from rpmUtils/miscutils.py
+def stringToVersion(verstring):
+if verstring in [None, '']:
+return (None, None, None)
+i = verstring.find(':')
+if i != -1:
+try:
+epoch = str(long(verstring[:i]))
+except ValueError:
+# look, garbage in the epoch field, how fun, kill it
+epoch = '0' # this is our fallback, deal
+else:
+epoch = '0'
+j = verstring.find('-')
+if j != -1:
+if verstring[i + 1:j] == '':
+version = None
+else:
+version = verstring[i + 1:j]
+release = verstring[j + 1:]
+else:
+if verstring[i + 1:] == '':
+version = None
+else:
+version = verstring[i + 1:]
+release = None
+return (epoch, version, release)
+
+
log = log_mgr.get_logger(__name__)
@@ -66,6 +96,16 @@ def selinux_enabled():
return False
+class IPAVersion(object):
+
+def __init__(self, version):
+self.version_tuple = stringToVersion(version)
+
+def __cmp__(self, other):
+assert isinstance(other, IPAVersion)
+return rpm.labelCompare(self.version_tuple, other.version_tuple)
+
+
class RedHatTaskNamespace(BaseTaskNamespace):
def restore_context(self, filepath, restorecon=paths.SBIN_RESTORECON):
@@ -423,5 +463,12 @@ class RedHatTaskNamespace(BaseTaskNamespace):
super(RedHatTaskNamespace, self).create_system_user(name, group,
homedir, shell, uid, gid, comment, create_homedir)
+def parse_ipa_version(self, version):
+"""
+:param version: textual version
+:return: object implementing proper __cmp__ method for version compare
+"""
+return IPAVersion(version)
+
tasks = RedHatTaskNamespace()
--
2.5.0
--
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
[Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison
https://fedorahosted.org/freeipa/ticket/5535
Patch attached.
From 8ef93485d61e8732166fb0c5b6c4559209740f3e Mon Sep 17 00:00:00 2001
From: Martin Basti
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
Provides: %{alt_name}-server = %{version}
Conflicts: %{alt_name}-server
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 94d2cb4e906965a20bcfdd55f38854005091c26f..0debae1f39924b608190ef7a7f9ba5ebe1b13dfc 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -30,11 +30,13 @@ import stat
import socket
import sys
import base64
+import rpm
from subprocess import CalledProcessError
from nss.error import NSPRError
from pyasn1.error import PyAsn1Error
from six.moves import urllib
+from rpmUtils.miscutils import stringToVersion
from ipapython.ipa_log_manager import root_logger, log_mgr
from ipapython import ipautil
@@ -66,6 +68,16 @@ def selinux_enabled():
return False
+class IPAVersion(object):
+
+def __init__(self, version):
+self.version_tuple = stringToVersion(version)
+
+def __cmp__(self, other):
+assert isinstance(other, IPAVersion)
+return rpm.labelCompare(self.version_tuple, other.version_tuple)
+
+
class RedHatTaskNamespace(BaseTaskNamespace):
def restore_context(self, filepath, restorecon=paths.SBIN_RESTORECON):
@@ -423,5 +435,12 @@ class RedHatTaskNamespace(BaseTaskNamespace):
super(RedHatTaskNamespace, self).create_system_user(name, group,
homedir, shell, uid, gid, comment, create_homedir)
+def parse_ipa_version(self, version):
+"""
+:param version: textual version
+:return: object implementing proper __cmp__ method for version compare
+"""
+return IPAVersion(version)
+
tasks = RedHatTaskNamespace()
--
2.5.0
--
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
