Bug 771851 - Review Request: python-ladon - Multiprotocol approach to creating a webservice
Summary: Review Request: python-ladon - Multiprotocol approach to creating a webservice
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Bohuslav "Slavek" Kabrda
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-01-05 09:03 UTC by Matthias Runge
Modified: 2012-01-23 21:57 UTC (History)
3 users (show)

Fixed In Version: python-ladon-0.6.5-2.fc16
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-01-23 21:57:29 UTC
Type: ---
Embargoed:
bkabrda: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Matthias Runge 2012-01-05 09:03:49 UTC
Spec URL: http://www.matthias-runge.de/fedora/python-ladon.spec
SRPM URL: http://www.matthias-runge.de/fedora/python-ladon-0.6.5-1.fc16.src.rpm
Description: Ladon is a framework for exposing methods to several internet service
protocols. Once a method is ladonized it is automatically served through
all the interfaces that your ladon installation contains. Ladon's interface
implemetations are added in a modular fashion making it very easy extend
Ladon's protocol support.


[mrunge@mrungexp SPECS]$ rpmlint -i /home/mrunge/rpmbuild/SRPMS/python-ladon-0.6.5-1.fc16.src.rpm /home/mrunge/rpmbuild/RPMS/noarch/python-ladon-0.6.5-1.fc16.noarch.rpm ./python-ladon.spec 
python-ladon.src: W: spelling-error Summary(en_US) Multiprotocol -> Multiprocessor
The value of this tag appears to be misspelled. Please double-check.

python-ladon.src: W: spelling-error Summary(en_US) webservice -> web service, web-service, service
The value of this tag appears to be misspelled. Please double-check.

python-ladon.src: W: spelling-error %description -l en_US ladonized -> lionized
The value of this tag appears to be misspelled. Please double-check.

python-ladon.src: W: spelling-error %description -l en_US implemetations -> implementations, implementation, supplementation
The value of this tag appears to be misspelled. Please double-check.

python-ladon.noarch: W: spelling-error Summary(en_US) Multiprotocol -> Multiprocessor
The value of this tag appears to be misspelled. Please double-check.

python-ladon.noarch: W: spelling-error Summary(en_US) webservice -> web service, web-service, service
The value of this tag appears to be misspelled. Please double-check.

python-ladon.noarch: W: spelling-error %description -l en_US ladonized -> lionized
The value of this tag appears to be misspelled. Please double-check.

python-ladon.noarch: W: spelling-error %description -l en_US implemetations -> implementations, implementation, supplementation
The value of this tag appears to be misspelled. Please double-check.

python-ladon.noarch: W: no-manual-page-for-binary ladon2.7ctl
Each executable in standard binary directories should have a man page.

python-ladon.noarch: W: no-manual-page-for-binary ladon2.7ctl.py
Each executable in standard binary directories should have a man page.

2 packages and 1 specfiles checked; 0 errors, 10 warnings.

Comment 1 Bohuslav "Slavek" Kabrda 2012-01-06 09:18:14 UTC
I'm taking this one for a review.

Comment 2 Bohuslav "Slavek" Kabrda 2012-01-06 10:02:32 UTC
- BuildRequires: python-devel should be BuildRequires: python2-devel, see [1].
- When packaging for Fedora 13 or greater, you don't need to define %python_sitelib yourself, because its in a macro file in python2-devel, see [2].
- Could you please specify why you need the two identical files in %{_bindir}, only with different names? It is an upstream decision, but maybe you could query them about this?
- The package seems to have a bundled library python-chardet, which is already in Fedora. You should unbundle it and make the Fedora package runtime dependency. (Perhaps query the upstream not to bundle it at all?)
- You probably don't need to use the CFLAGS in %build section, as your package doesn't contain any binary extensions.
- "rm -rf $RPM_BUILD_ROOT" is not needed, if you don't plan on getting the package to EPEL.
- Could you please clarify the LICENSE? Your specfile says GPLv2+, but PKG-INFO only says GPL.

[1] https://fedoraproject.org/wiki/Packaging:Python#BuildRequires
[2] https://fedoraproject.org/wiki/Packaging:Python#Macros

Comment 3 Matthias Runge 2012-01-06 10:09:11 UTC
Thank you for your review. I'll resolve those questions/issues.

Comment 4 Matthias Runge 2012-01-06 19:46:16 UTC
updated version:
SPEC: http://www.matthias-runge.de/fedora/python-ladon.spec
(old version, for reference: http://www.matthias-runge.de/fedora/python-ladon-1.spec )
SRPM: http://www.matthias-runge.de/fedora/python-ladon-0.6.5-2.fc16.src.rpm


Upstream clarified, license is lgpv3+, updated PKG-INFO on http://pypi.python.org/pypi/ladon

Bundled library is kept upstream for missing reference for chardet (chardet's upstream is gone). It should be save to remove ladons bundled lib chardet_py2 and replace the only reference to bundled lib to fedoras version of chardet.

CFLAGS removed, buildroot-removal deleted.


for easier reference:

[mrunge@sofja SPECS]$ diff -u python-ladon-1.spec python-ladon.spec --- python-ladon-1.spec	2012-01-06 20:31:08.574775992 +0100
+++ python-ladon.spec	2012-01-06 20:44:47.248475349 +0100
@@ -1,18 +1,17 @@
-%{!?python_sitelib: %global python_sitelib %(%{__python} -c "from distutils.sysconfig import get_python_lib; print(get_python_lib())")}
-
 Name:           python-ladon
 Version:        0.6.5
-Release:        1%{?dist}
+Release:        2%{?dist}
 Summary:        Multiprotocol approach to creating a webservice
 
-License:        GPLv2
+License:        LGPLv3+
 URL:            http://ladonize.org
 Source0:        http://pypi.python.org/packages/source/l/ladon/ladon-%{version}.tar.gz
 # #md5=07dd1d395d0d7123448d7710bb3da32a
 
 BuildArch:      noarch
-BuildRequires:  python-devel
+BuildRequires:  python2-devel
 Requires:       python-jinja2
+Requires:       python-chardet
 
 %description
 Ladon is a framework for exposing methods to several internet service 
@@ -24,22 +23,33 @@
 %prep
 %setup -q -n ladon-%{version}
 
+# replace reference to bundled lib with fedora's reference
+sed -i 's/chardet_py2/chardet/' src/ladon/ladonizer/collection.py
 
 %build
-CFLAGS="$RPM_OPT_FLAGS" %{__python} setup.py build
+%{__python} setup.py build
 
 
 %install
-rm -rf $RPM_BUILD_ROOT
 %{__python} setup.py install -O1 --skip-build --root $RPM_BUILD_ROOT
-chmod 755 /%{buildroot}/%{python_sitelib}/ladon/clients/jsonwsp.py
+chmod 755 %{buildroot}/%{python_sitelib}/ladon/clients/jsonwsp.py
+
+# remove file included for windows-builds
+rm %{buildroot}/%{_bindir}/ladon2.7ctl.py
+
+# remove bundled library
+rm -rf %{buildroot}/%{python_sitelib}/chardet_py2
  
 %files
 %doc PKG-INFO
 %{python_sitelib}/*
 %{_bindir}/ladon2.7ctl
-%{_bindir}/ladon2.7ctl.py
 
 %changelog
+* Fri Jan 06 2012 Matthias Runge <mrunge> - 0.6.5-2
+- remove unecessary definition, buildroot cleaning
+- remove bundled library chardet_py2, correct reference to use fedora's version
+- correct build requirements
+
 * Wed Jan 04 2012 Matthias Runge <mrunge> - 0.6.5-1
 - initial fedora package

Comment 5 Bohuslav "Slavek" Kabrda 2012-01-09 06:42:03 UTC
Looks good now, package is


APPROVED

Comment 6 Matthias Runge 2012-01-09 07:29:40 UTC
Bohuslav, thanks for the review. It has been a pleasure for me!

New Package SCM Request
=======================
Package Name: python-ladon
Short Description: Multiprotocol approach to creating a webservice
Owners: mrunge
Branches: f16, el6

Comment 7 Gwyn Ciesla 2012-01-09 13:08:59 UTC
Git done (by process-git-requests).

Comment 8 Fedora Update System 2012-01-10 08:17:15 UTC
python-ladon-0.6.5-2.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/python-ladon-0.6.5-2.fc16

Comment 9 Fedora Update System 2012-01-10 09:02:28 UTC
python-ladon-0.6.5-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/python-ladon-0.6.5-2.el6

Comment 10 Fedora Update System 2012-01-11 05:59:06 UTC
python-ladon-0.6.5-2.fc16 has been pushed to the Fedora 16 testing repository.

Comment 11 Fedora Update System 2012-01-23 21:57:29 UTC
python-ladon-0.6.5-2.fc16 has been pushed to the Fedora 16 stable repository.


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