Bug 1415190 - Review Request: python-onionbalance - Load-balancing for Tor onion services
Summary: Review Request: python-onionbalance - Load-balancing for Tor onion services
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Simone Caronni
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-01-20 14:08 UTC by Marcel Haerry
Modified: 2017-02-20 01:50 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-02-17 16:21:09 UTC
Type: ---
negativo17: fedora-review+


Attachments (Terms of Use)

Description Marcel Haerry 2017-01-20 14:08:28 UTC
Spec URL: http://git.scrit.ch/srpm/python-onionbalance/plain/SPECS/python-onionbalance.spec
SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/3105/17343105/python-onionbalance-0.1.6-1.fc26.src.rpm
Description: OnionBalance provides load-balancing and redundancy for Tor
onion services by distributing requests to multiple back-end
Tor instances.
Fedora Account System Username: maha

This package is intended to be built for rawhide, as well as EPEL7.

Scratch build for Rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=17343105
Scratch build for F26: https://koji.fedoraproject.org/koji/taskinfo?taskID=17343163

There are some rpmlint warnings and errors, which I think do not apply for this package. My comments are:

>> python2-onionbalance.noarch: W: no-documentation
>> python2-onionbalance.noarch: W: no-manual-page-for-binary onionbalance-py2
>> python2-onionbalance.noarch: W: no-manual-page-for-binary onionbalance-config-py2

this is by intend, as the python3 package contains all the documentation and manpages. I wasn't sure whether this is the right way to package the python2 variant of the service.

>> python3-onionbalance.noarch: W: non-standard-gid /etc/onionbalance toranon
>> python3-onionbalance.noarch: E: non-standard-dir-perm /etc/onionbalance 750

This is by intend, as this directory will contain sensitive information.

>> python3-onionbalance.noarch: W: devel-file-in-non-devel-package /usr/bin/onionbalance-config

I'm not sure, why rpmlint thinks this is a devel file.

>> python3-onionbalance.noarch: W: non-standard-uid /var/lib/onionbalance onionbalance
>> python3-onionbalance.noarch: W: non-standard-gid /var/lib/onionbalance toranon
>> python3-onionbalance.noarch: E: non-standard-dir-perm /var/lib/onionbalance 750

This is by intend, as it might contain sensitive information.

>>> python3-onionbalance.noarch: W: non-standard-uid /var/log/onionbalance onionbalance
>> python3-onionbalance.noarch: W: non-standard-gid /var/log/onionbalance toranon
>> python3-onionbalance.noarch: E: non-standard-dir-perm /var/log/onionbalance 750

This is by intend, as the logfile might contain sensitive information.

>> python3-onionbalance.noarch: E: incoherent-logrotate-file /etc/logrotate.d/onionbalance.conf

This is by intend, as it then follows: https://fedoraproject.org/wiki/Packaging:Guidelines#Logrotate_config_file

What follows is the full output of rpmlint:

$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/python*
python2-onionbalance.noarch: W: no-documentation
python2-onionbalance.noarch: W: no-manual-page-for-binary onionbalance-config-py2
python2-onionbalance.noarch: W: no-manual-page-for-binary onionbalance-py2
python3-onionbalance.noarch: E: incoherent-logrotate-file /etc/logrotate.d/onionbalance.conf
python3-onionbalance.noarch: W: non-standard-uid /var/log/onionbalance onionbalance
python3-onionbalance.noarch: W: non-standard-gid /var/log/onionbalance toranon
python3-onionbalance.noarch: E: non-standard-dir-perm /var/log/onionbalance 750
python3-onionbalance.noarch: W: non-standard-uid /var/lib/onionbalance onionbalance
python3-onionbalance.noarch: W: non-standard-gid /var/lib/onionbalance toranon
python3-onionbalance.noarch: E: non-standard-dir-perm /var/lib/onionbalance 750
python3-onionbalance.noarch: W: devel-file-in-non-devel-package /usr/bin/onionbalance-config
python3-onionbalance.noarch: W: non-standard-gid /etc/onionbalance toranon
python3-onionbalance.noarch: E: non-standard-dir-perm /etc/onionbalance 750
3 packages and 0 specfiles checked; 4 errors, 9 warnings.

Comment 1 Simone Caronni 2017-01-20 14:57:52 UTC
(In reply to Marcel Haerry from comment #0)
> >> python2-onionbalance.noarch: W: no-documentation
> >> python2-onionbalance.noarch: W: no-manual-page-for-binary onionbalance-py2
> >> python2-onionbalance.noarch: W: no-manual-page-for-binary onionbalance-config-py2
> 
> this is by intend, as the python3 package contains all the documentation and
> manpages. I wasn't sure whether this is the right way to package the python2
> variant of the service.

Can you make the package conditionally add the manpages to the "base" package depending on the distribution? Python 2 is the default on all supported distributions so far. And in the EPEL 7 build the man pages are not there.

Have you tried to put man pages into both packages? Man pages are considered documentation and they show up with "rpm -qd <package>". If they are identical while installing I think the packages will not complain.

$ rpm -qf /usr/share/doc/glibc
glibc-2.24-4.fc25.x86_64
glibc-2.24-4.fc25.i686

If you are unsure maybe ask on the mailing list?

The only thing that I see that is wrong in the package is the license field. It's not BSD but GPLv3:

https://github.com/DonnchaC/onionbalance/blob/develop/COPYING

Comment 2 Simone Caronni 2017-01-20 15:37:38 UTC
Nice touch in the hardening in systemd unit.
I should find the time to do the same in my packages.

Comment 3 Marcel Haerry 2017-01-20 15:47:15 UTC
> > this is by intend, as the python3 package contains all the documentation and
> > manpages. I wasn't sure whether this is the right way to package the python2
> > variant of the service.
> 
> Can you make the package conditionally add the manpages to the "base"
> package depending on the distribution?

I'll try that one.

> Python 2 is the default on all
> supported distributions so far.

Well if I read
https://fedoraproject.org/wiki/Packaging:Python#Python_Version_Support

then I understand it that the default target is Python 3 and Python 2 is nice-to-have.

I also thought about only building a python3 version for Fedora and a python 2 version for EPEL.

I tried to understand the whole Python packaging guidelines, but I think lots of things are in transition atm. and I'm not sure what the current best-practice is in all the edge-cases.

For example I also did not include any service files or so in the python2 package on Fedora.

So one could really question, what the benefit from the python2 package is and whether we should build a python2 package at all.

It's kinda easy for pure libraries, but binaries and services make it a bit more complex.

So I would suggest to then build only the python3 version.

> And in the EPEL 7 build the man pages are
> not there.

The problem is, that the version of sphinx in EPEL 7 is not new enough to build the man pages. That's why I excluded them.

How would you go forward in such a case? From the update guidelines, I think it's not feasible to get the new version sphinx in EPEL, as it has breaking changes, afair.

> The only thing that I see that is wrong in the package is the license field.
> It's not BSD but GPLv3:
> 
> https://github.com/DonnchaC/onionbalance/blob/develop/COPYING

Good catch! Fixed.

So atm, I got a regression from a recently updated library, which makes the build failing atm. :/

I need to look into that one more in detail before being able to do a new test build. Nevertheless, we can already discuss the situation above.

Comment 4 Marcel Haerry 2017-01-20 16:26:01 UTC
Ok, so the latest python-sphinxcontrib-autoprogram version (updated today in rawhide) has a bug that makes it not working on python 2: https://bitbucket.org/birkenfeld/sphinx-contrib/issues/168/autoprogram-013-fails-on-python-27-due-to

I can backport the fix to the autoprogram package first, but this is only urgent if we want to keep the build of the python2 package.

Comment 5 Simone Caronni 2017-01-22 18:06:05 UTC
(In reply to Marcel Haerry from comment #3)
> Well if I read
> https://fedoraproject.org/wiki/Packaging:Python#Python_Version_Support
> 
> then I understand it that the default target is Python 3 and Python 2 is
> nice-to-have.

Ok, I missed that, sorry. Then I would say your approach is perfectly fine.

> I also thought about only building a python3 version for Fedora and a python
> 2 version for EPEL.
> 
> So I would suggest to then build only the python3 version.

Then yeah, I would say so.

> The problem is, that the version of sphinx in EPEL 7 is not new enough to
> build the man pages. That's why I excluded them.

Fine, you can still add them later if the code comes in to build with an older sphinx.

> How would you go forward in such a case? From the update guidelines, I think
> it's not feasible to get the new version sphinx in EPEL, as it has breaking
> changes, afair.

It was decided (don't know exactly when) that it was ok also to rebase stuff in EPEL (for example Nagios jumped from 2 to 4). After all, RHEL itself contains huge rebases of everything (DRM, X and libraries, the whole GNOME, libvirt and friends).

If you can push the changes and update all the various components, then fine. Maybe I can help you there.

If your rebases require touch in RHEL packages, then you need a customer account (hint!) to get stuff changed, normal bugzilla tickets are rarely taken into consideration.

> I need to look into that one more in detail before being able to do a new
> test build. Nevertheless, we can already discuss the situation above.

I think all your points are valid. Just make the python 3 only version in Fedora and the python 2 version only in EPEL 7 without the man pages.

(In reply to Marcel Haerry from comment #4)
> Ok, so the latest python-sphinxcontrib-autoprogram version (updated today in
> rawhide) has a bug that makes it not working on python 2:
> https://bitbucket.org/birkenfeld/sphinx-contrib/issues/168/autoprogram-013-
> fails-on-python-27-due-to
> 
> I can backport the fix to the autoprogram package first, but this is only
> urgent if we want to keep the build of the python2 package.

Well, in this case, just build the package for Fedora with python 3, but request all branches including epel 7 even if at the moment you can't build the package. You can always work on it later or in subsequent upstream releases.

Package approved.

Comment 6 Marcel Haerry 2017-02-03 11:47:31 UTC
Ok, so I built the packages based on our discussion:

F26: https://koji.fedoraproject.org/koji/taskinfo?taskID=17564793
EPEL7: https://koji.fedoraproject.org/koji/taskinfo?taskID=17564730

Going to ask for the SCM entries.

Comment 7 Gwyn Ciesla 2017-02-03 13:25:49 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/python-onionbalance

Comment 8 Marcel Haerry 2017-02-03 14:18:50 UTC
Package uploaded to Rawhide and EL7

Comment 9 Fedora Update System 2017-02-04 10:11:36 UTC
python-onionbalance-0.1.6-1.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-56450e4920

Comment 10 Fedora Update System 2017-02-04 10:12:07 UTC
python-onionbalance-0.1.6-1.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-a77047740b

Comment 11 Fedora Update System 2017-02-05 02:21:21 UTC
python-onionbalance-0.1.6-1.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-56450e4920

Comment 12 Fedora Update System 2017-02-05 05:50:25 UTC
python-onionbalance-0.1.6-1.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-a77047740b

Comment 13 Fedora Update System 2017-02-17 16:21:09 UTC
python-onionbalance-0.1.6-1.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.

Comment 14 Fedora Update System 2017-02-20 01:50:08 UTC
python-onionbalance-0.1.6-1.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.