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

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

Reply via email to