Bug 522613

I wrote a spec for myself which includes the C module (for Python 2.5 users).

I thought I might as well throw is up here just in case anyone else was interested.

http://silassewell.googlecode.com/svn/trunk/projects/packages/rpms/python-tornado/python-tornado.spec

This is an unofficial review for the original package and spec file:

+: ok
!: needs to be fixed
-: not applicable

MUST Items:
[+] rpmlint comes out clean on every package
[+] package name meets Package Naming Guidelines
[+] spec file name must match base package name
[+] the package license (ASL 2.0) is correct and allowed in Fedora
[+] spec file is legible and written in American English
[+] SOURCE url points to packaged source archive
[+] package md5sum matches upstream (69d6c60c4eca3a32de23aa5e3717b6f2)
[+] BuildRequires are correctly specified as per Python Guidelines (see below)
[+] package builds fine in koji
[-] locales are properly handled
[+] no system libraries are bundled
[-] if package installs libraries in default paths run ldconfig in %post/%postun
[!] package owns the directories it creates

%{python_sitelib}/%{name}/* is not sufficient, see http://fedoraproject.org/wiki/Packaging:UnownedDirectories#Common_Mistakes
This way you would not own %{python_sitelib}/%{name}

[+] no file is listed twice
[+] permissions on files are explictly set (via defattr)
[+] package must contain %clean with rm -rf %{buildroot} (or $RPM_BUILD_ROOT)
[!] macros are consistently used

You should decide to either use %{buildroot} or $RPM_BUILD_ROOT - either one is fine, but mixing them should be avoided.

[+] the package has permissable code / content
[-] large documentations must be put into -doc subpackage
[+] files included in %doc must not be essential for the application to work
[-] header files must be in -devel package
[-] static libraries must be in a -static package
[-] packages containing pkgconfig(.pc) files must "Requires: pkgconfig"
[-] library files without a suffix (foo.so) must go into -devel subpackage if libraries with a suffix (foo.so.0.0) are present.
[-] %{name}-devel packages must specify a fully versioned dependency on the %{name} package
[-] package must not contain any libtool (.la) archives
[-] (most) GUI applications need to include a %{name}.desktop file
[+] package must not own any file or directory already owned by another package
[+] first command in %install must be rm -rf %{buildroot} (or $RPM_BUILD_ROOT)
[+] all filesnames in package are valid UTF-8


SHOULD-Items:
[!] if source package does not include license text as seperate file, packager should query upstream to include it
[-] if available,  description and summary in spec file should contain translations for non-english languages
[+] package builds fine in mock
[+] package should compile on all supported architectures
[+] package does work during a short test
[+] scriptlets - if used - must be sane
[-] non-devel subpackages should require the base package
[-] pkgconfig(.pc) files should be placed in -devel package
[-] if package does require a file outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin, packager should require the package which provides the file (not the file alone)


Python-specific MUST-Items:
[+] package must (almost always) require python-devel
[+] if package wants to install into the global site_packages directory, python_sitelib must be defined according to Python Guidelines
[-] python eggs must be built from source
[-] python eggs must not be downloading dependencies during build process
[+] egg-info files must be included in the package if available
[-] compat packages must install using easy_install -m
[-] if building multiple versions (compat packages), one package must contain a default version usable via "import modulename"
[+] all python source files except those in %{_bindir} and %{_sbindir} must be byte-compiled into *.pyo and both source and compiled version must be included in the package


Summary:
* package does not own every directory it creates
* mixed usage of %{buildroot} and $RPM_BUILD_ROOT

Note: This package will not compile for Fedora < 11, as the setup script will try to compile epoll.c as F10 has only Python 2.5, but this is not a blocker.

Other Questions:
Why do you use a script to remove the shebangs from files? Creating a patch may be better to verify the changes you make to the source - especially if you only want to make rpmlint happy.

You should also consider to update your package to match the latest upstream version (0.2).

Thank you, Rene!

I've updated the package to fix these issues:
http://mapleoin.fedorapeople.org/pkgs/tornado/tornado.spec.1
http://mapleoin.fedorapeople.org/pkgs/tornado/tornado-0.2-1.fc11.src.rpm

I've also refactored the way the spurious permissions are handled.

I'm using the script to remove the shebangs as mentioned here:
https://fedoraproject.org/wiki/Packaging_tricks#Remove_shebang_from_files
Though either method seems fair to me.

I've also contacted upstream for the inclusion of the LICENSE file:
http://groups.google.com/group/python-tornado/browse_thread/thread/2045077b2c70dbbf

Inspired by Silas Sewell above, I would also like to rename this package to python-tornado.

(In reply to comment #3)
> Inspired by Silas Sewell above, I would also like to rename this package to
> python-tornado.  

Good. Go ahead. :)

Package Review
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [?] Package is named according to the Package Naming Guidelines.
     When the name will be python-tornado and the spec renamed, yes
 [?] Spec file name must match the base package %{name}, in the format %{name}.spec.
     also checking later
 [x] Package meets the Packaging Guidelines
 [x] Package successfully compiles and builds into binary rpms on at least one supported architecture.
     Tested on: 
       [] devel/i386 
       [] devel/x86_64
       [] F11/i386 
       [x] F11/x86_64
 [x] Rpmlint output:
     $ rpmlint tornado.spec tornado-0.2-1.fc11.src.rpm noarch/tornado-0.2-1.fc11.noarch.rpm 
2 packages and 1 specfiles checked; 0 errors, 0 warnings.

 [x] Buildroot is correct
     (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n))
 [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines.
 [x] License field in the package spec file matches the actual license.
     License type: ASL 2.0
 [-] If (and only if) the source package includes the text of the license(s) in
its own file, then that file, containing the text of the license(s) for the
package is included in %doc.
 [x] Spec file is legible and written in American English.
 [x] Sources used to build the package matches the upstream source, as provided in the spec URL.
     Upstream source: 4704cbf8baab2562c1e648c76de87b60
     Build source:    4704cbf8baab2562c1e648c76de87b60
 [x] Package is not known to require ExcludeArch
 [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines.
     BuildRequires: python-setuptools is missing, so it should not build in koji, but it does:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1755511
 [-] The spec file handles locales properly.
 [-] ldconfig called in %post and %postun if required.
 [x] Package must own all directories that it creates.
 [x] Package requires other packages for directories it uses.
 [x] Package does not contain duplicates in %files.
 [x] Permissions on files are set properly.
     But: find demos -name "*.py" | xargs chmod -x
     would be better readable.
 [x] Package has a %clean section, which contains rm -rf %{buildroot}.
 [x] Package consistently uses macros.
 [!] Large documentation files are in a -doc subpackage, if required.
     The demos are 1.1 MB big and the main files, which will be used are 0.2 MB, please put the demos in a -doc subpackage
 [x] Package uses nothing in %doc for runtime.
 [-] Header files in -devel subpackage, if present.
 [-] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [-] Development .so files in -devel subpackage, if present.
 [!] Fully versioned dependency in subpackages, if present.
     TBD
 [x] Package does not contain any libtool archives (.la).
 [-] Package contains a properly installed %{name}.desktop file if it is a GUI application.
 [x] Package does not own files or directories owned by other packages.

Python specific musts are fullfiled, too.


=== SUGGESTED ITEMS ===
 [x] Latest version is packaged.
 [!] Package does not include license text files separate from upstream.
     upstream already queried
 [-] Description and summary sections in the package spec file contains translations for supported Non-English languages, if available.
 [x] Reviewer should test that the package builds in mock.
http://koji.fedoraproject.org/koji/taskinfo?taskID=1755511
 [x] Package functions as described (no hardware to test with).
     helloworld does ;)
 [x] Scriptlets must be sane, if used.
     comment about permissions above
 [-] The placement of pkgconfig(.pc) files is correct.


____________________


Todo:
- rename to python-tornado (summary of this bug, too)
- split the demos into a -doc subpackage

Lifting FE-LEGAL, because I'm unsure if the name 'facebook' (and maybe 'django') may be in the demos subdir.

There is no picture or something like that in the sources, but the example 'demos/facebook' seems to redirect to the offical website and the name is called several times…
What are the naming rights like?

The example itself is ASL 2.0.

Is that a legal issue?

Don't think so, but want to be sure, anyway ;)

No real issue here, as the code isn't claiming to be "facebook, the web site". Lifting FE-Legal.

thanks a lot, Thomas!

Here are the updates:
http://mapleoin.fedorapeople.org/pkgs/python-tornado/python-tornado.spec
http://mapleoin.fedorapeople.org/pkgs/python-tornado/python-tornado-0.2-2.fc11.src.rpm

Checking items, left behind last time, because of the rename:

 [x] Package is named according to the Package Naming Guidelines.
 [x] Spec file name must match the base package %{name}, in the format 
     %{name}.spec.

 [x] Large documentation files are in a -doc subpackage, if required


Close to finish, but the rename introduced some new things ;)

- use %global instead of %define,
  see: https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define

- Group: Development/Libraries should be for libraries.
  These examples should be better in: Documentation

Thanks again, Thomas!

http://mapleoin.fedorapeople.org/pkgs/python-tornado/python-tornado.spec
http://mapleoin.fedorapeople.org/pkgs/python-tornado/python-tornado-0.2-3.fc11.src.rpm

There's quite an omission I made when opening this ticket it seems. Sorry. I haven't yet been accepted in the packager group, though I'm in the process of being sponsored.

It would be nice to know that before starting a review ;)
e.g. with blocking FE-NEEDSPONSOR like in the 5 / 7 other review requests!!


The two issues, mentioned in comment #8 are resolved, so I'll approve this, when you are sponsored.


Remove the block to FE-NEEDSPONSOR, once you are sponsored...

   APPROVED

New Package CVS Request
=======================
Package Name: python-tornado
Short Description: Scalable, non-blocking web server and tools
Owners: mapleoin
Branches: F-11 F-12
InitialCC:

cvs done.

python-tornado-0.2-3.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update python-tornado'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-11024

python-tornado-0.2-3.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: python-tornado
New Branches: el5 el6
Owners: mapleoin

I'd like to make this package available for EPEL. Thanks.

Git done (by process-git-requests).

*** Bug 617834 has been marked as a duplicate of this bug. ***

Package Change Request
======================
Package Name: python-tornado
New Branches: epel7
Owners: tomspur orion

Git done (by process-git-requests).