[Freeipa-devel] [PATCH] 0749 Package ipapython, ipalib, ipaplatform, ipatests for Python 3

Tue Dec 1 13:37:11 UTC 2015

On 1.12.2015 12:12, Petr Viktorin wrote:
> Hello,
> I noticed I didn't attach an updated patch last time, which probably
> affected the discussion here. Sorry for that; it's attached this time.
>
> We seem to have a disconenct here: there are two different strategies to
> do the packaging.
>
> Your idea of the port seems to be to have the py2 and py3 packages "in
> sync", so they're almost exact copies of each other. This keeps things
> consistent, but I'm not sure this specific kind of consistency is better
> than the alternative.
>
> My approach is to make new, better packages for Python 3. I treat this
> as a bit of a sandbox, a new chance to do things right. My patch tries
> to keep the existing Python 2 packages untouched where it's feasible, to
> minimize regressions.
> (I did add new Provides based on your review, though, so users can use
> the "consistent" names.)
> When the py3 packages are finalized, the changes can be applied to the
> py2 ones as well. (Or not, if it's decided it's a case of fixing things
> that ain't broke.)
>
> So, two paths to get to the same goal. I'm not saying yours is bad, but
> I fail to see how it's objectively better. If it is, could you explain
> why? Otherwise please take this patch as an incremental improvement with
> priorities that are a bit different than yours.

Developer and user convenience. If your usage of the packages goes 
beyond "yum install package", being self-consistent should prevent 
raised eyebrows.

>
>
> On 11/30/2015 09:59 AM, Jan Cholasta wrote:
>> On 27.11.2015 13:46, Petr Viktorin wrote:
>>> On 11/26/2015 11:52 AM, Jan Cholasta wrote:
>>>> 1) The freeipa-common subpackage is not necessary: /etc/ipa/dnssec
>>>> should be owned by freeipa-server and everything else in /etc/ipa
>>>> currently owned by freeipa-python should be owned by freeipa-client.
>>>
>>> If the common files are in freeipa-server or freeipa-client, then the
>>> Python 3 packages would need to depend on those. I'd like to avoid that.
>>
>> I'm not sure I follow. The files do not belong to freeipa-python, since
>> they are used only by freeipa-server and freeipa-client. What Python 3
>> packages would need to depend on them?
>
> python3-ipaserver and python3-ipaclient, eventually.

freeipa-server and freeipa-client already own some non-Python files and 
you will have to deal with them either way.

>
> Anyway, /etc/ipa/default.conf is used directly by ipapython.config, so
> at least that should be owned by a common package. And once there's a
> common package, I don't think there's a problem with it holding the
> other tiny files as well.

/etc/ipa/default.conf is managed by freeipa-client and thus should be 
owned by it.

This is a common pattern in other packages (even other FreeIPA 
sub-packages) and I don't see any reason not to follow it here as well.

> Also, a lot of the other files are mentioned in ipaplatform.base.paths,
> which is part of the freeipa-python, so it seems fitting that that's
> what should require them, rather than the -client or -server packages.

Following this logic, every non-Python file in every sub-package should 
be owned by freeipa-python (or freeipa-common), which is definitely not 
something we want.

>
>>>> This should be a separate patch. I have prepared one, see attachment.
>>>>
>>>>
>>>> 2) IMO this patch does too much, it should only add the Python 3
>>>> equivalent of freeipa-python - python3-ipalib - but none of the other
>>>> packages/provides.
>>>
>>> My reasoning for the new packages:
>>> - python3-ipalib is the main point
>>
>> Right.
>>
>>> - freeipa-common contains files that both the py2 and py3 versions need
>>
>> See above.
>>
>>> - python-ipap11helper has compiled code: with this pulled out,
>>> python3-ipalib can be noarch
>>
>> This is not the goal here, but if you insist on doing it, do it for
>> Python 2 as well.
>
> It is definitely the goal of this patch to make the py3 packages as good
> as possible. That includes making them noarch.

It is completely unnecessary for the initial py3 support.

I would rather maintain internal consistency than make the py3 packages 
"perfect".

> On the other hand improving the py2 packages is not a goal of this
> particular patch.

Which is exactly the reason I have provided patches for py2 packages myself.

>
>>> - python3-ipatests is needed if we want to start testing the py3
>>> packages in  CI
>>
>> Right.
>>
>>>
>>> As for new provides, Fedora's Python packaging guidelines say:
>>>
>>> """
>>> Using a fictional module named "example", the subpackage containing
>>> the python2 version must provide python2-example. This is of course
>>> always the case if the subpackage is named python2-example [...]
>>> If the subpackage has some other name then then Provides: python2-example
>>> must be added explicitly (but see the %python_provide macro below).
>>>
>>> The python3 subpackage must provide python3-example. However, as the
>>> naming guidelines mandate that the python3 subpackage be named
>>> python3-example, this will happen automatically.
>>> """
>>>
>>> so I'm now adding Provides for the top-level modules.
>>
>> The goal of this work is to add support for Python 3, not to comply with
>> Fedora packaging guidelines. FreeIPA on Fedora uses its own spec file
>> anyway.
>
> The goal of this patch is to add new packages that support Python 3.
> Yes, the Fedora spec is different, but it's heavily based on the
> upstream one, and this is a good thing. I consider the Fedora guidelines
> the standard in Python RPM packaging. If IPA uses different packaging
> guidelines, can you point me to them?

FreeIPA never fully complied to Fedora packaging guidelines AFAIK and I 
don't see any reason to start now, since nobody seemed to care so far. 
Following them in just py3 sub-packages does not improve the state of 
FreeIPA as a whole and only brings inconsistency into it, so there's no 
benefit in doing it at all.

>
>> Again, if you insist on doing this, do it for Python 2 as well.
>
> Improving the py2 packages would be great, but again it is not the goal
> of this patch.

Like it or not, whatever you do for py3 support must integrate nicely in 
FreeIPA, and that might mean touching py2 stuff as well.

>
>>>> 3) Speaking of freeipa-python, it should be renamed to python-ipalib,
>>>> for consistency.
>>>>
>>>> This should also be a separate patch, again see attachment.
>>>
>>> I think that would just be unnecessary churn. Python 3 gives us a chance
>>> for a fresh start, so I'm taking advantage of that, but renaming
>>> existing packages is a pain.
>>
>> Something similar was done in SSSD not long ago and according to Lukáš
>> (CCd), it was no pain.
>
> OK. But again, not the goal here.

Which is again exactly the reason I provided the patch myself.

>
>>> With the right Provides, "dnf install python-ipalib" (or python2-ipalib)
>>> will do the right thing, and I thing that's enough.
>>
>> I don't think that's good enough, since it creates an inconsistency.
>>
>>>
>>>> 4) There should be a python3-devel (and also other python3- equivalents
>>>> of python- packages) BuildRequires when Python 3 support is enabled.
>>>
>>> I did miss that somehow. Thanks for checking.
>>> I added python3-devel; the others are unnecessary because pylint is not
>>> currently run on the Python3 version.
>>
>> OK, I think that will do for now. Note that many of them are required
>> for makeapi and makeaci as well.
>>
>> When can we expect a Python 3 pylint patch which adds the remaining
>> BuildRequires?
>
> That's something we'll need to talk about in the future: do we want to
> run python2-pylint and python3-pylint on every build?
> Anyway, I'd like to see the Python3 tests for ipapython and ipalib run
> in CI first.

Sure, no problem.

-- 
Jan Cholasta