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

Tue Dec 1 11:12:48 UTC 2015

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.

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.

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.
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.

>>> 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.
On the other hand improving the py2 packages is not a goal of this
particular patch.

>> - 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?

> 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.

>>> 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.

>> 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.

-- 
Petr Viktorin

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0749.2-Package-ipapython-ipalib-ipaplatform-ipatests-for-Py.patch
Type: text/x-patch
Size: 24563 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151201/33265107/attachment.bin>