Bug 427706 (python-urwid) - Review Request: python-urwid - console user interface for python
Summary: Review Request: python-urwid - console user interface for python
Keywords:
Status: CLOSED DUPLICATE of bug 510097
Alias: python-urwid
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2008-01-06 23:24 UTC by Niall Sheridan
Modified: 2009-07-07 16:53 UTC (History)
10 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-07-07 16:53:49 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Niall Sheridan 2008-01-06 23:24:24 UTC
Hi there.
This is my first package and I'm looking for a sponsor.

Spec URL: http://www.evil.ie/fedora/rpms/python-urwid/python-urwid.spec
SRPM URL: http://www.evil.ie/fedora/rpms/python-urwid/python-urwid-0.9.8.1-1.fc8.src.rpm
Description:
Urwid is a console user interface library for Python.
It includes many features useful for text console application developers:
- Fluid interface resizing (xterm window resizing / fbset on Linux console)
- Web application display mode using Apache and CGI
- Support for UTF-8, simple 8-bit and CJK encodings
- Multiple text alignment and wrapping modes built-in
- Ability create user-defined text layout classes
- Simple markup for setting text attributes
- Powerful list box that handles scrolling between different widget types
- List box contents may be managed with a user-defined class
- Flexible edit box for editing many different types of text
- Buttons, check boxes and radio boxes
- Customizable layout for all widgets
- Easy interface for creating HTML screen shots

Comment 1 Huzaifa S. Sidhpurwala 2008-01-09 10:19:47 UTC
Hi, 
I cannot sponsor your package, I can just review it:

[root@dhcp1-17 ~]# rpmlint python-urwid-0.9.8.1-1.fc8.src.rpm
python-urwid.src: W: summary-not-capitalized console user interface for python
python-urwid.src: W: invalid-license LGPL

See: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines


Also tmpl_tutorial.html should have gone to doc i guess

Comment 2 Huzaifa S. Sidhpurwala 2008-01-09 10:35:24 UTC
Also this package does not build for me, using rpmbuild:


running build_ext
error: invalid Python installation: unable to open
/usr/lib/python2.5/config/Makefile (No such file or directory)
error: Bad exit status from /var/tmp/rpm-tmp.56412 (%build)



Comment 3 Huzaifa S. Sidhpurwala 2008-01-09 10:51:32 UTC
I guess this means you add python-devel as BuildRoot if i am not wrong

Comment 4 Ruben Kerkhof 2008-01-20 13:19:06 UTC
You have to add python-setuptools-devel to the BuildRequires.

Comment 5 Niall Sheridan 2008-01-20 13:53:29 UTC
> python-urwid.src: W: summary-not-capitalized console user interface for python
> python-urwid.src: W: invalid-license LGPL

Fixed.

> Also tmpl_tutorial.html should have gone to doc i guess

I kept this file out, as it's only used during the build by docgen_tutorial.py
to generate tutorial.html. As such I didn't think it warranted an inclusion in doc.

> You have to add python-setuptools-devel to the BuildRequires.

Fixed.

Comment 6 Ruben Kerkhof 2008-01-20 20:52:02 UTC
Hi Niall,

Since this is your first package, you will need a sponsor.
You might want to do some reviews on your own, and post the bugzilla id's  over here, so potential 
sponsors can see what you're capable of.

There's more information about that on http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored



Comment 7 David Fraser 2008-03-26 22:53:29 UTC
Doing my own review of this as part of my sponsorship campaign...

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

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

Note: Shouldn't this be a BuildArch: noarch package? - it doesn't seem to
compile anything and the python should be cross-platform...

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines.
Suggestion: change Summary "Console user interface for python" to "Console user
interface library for python" for clarity
 [x] Spec file name must match the base package %{name}, in the format %{name}.spec.
 [x] Package meets the Packaging Guidelines.
 [x] Package successfully compiles and builds into binary rpms on at least one
supported architecture.
     Tested on: FC8/i386
 [!] Rpmlint output:
source RPM: empty
binary RPM: 
python-urwid.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/urwid/canvas.py 0644
python-urwid.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/urwid/graphics.py 0644
python-urwid.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/urwid/old_str_util.py 0644
python-urwid.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/urwid/font.py 0644
python-urwid.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/urwid/raw_display.py 0644
python-urwid.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/urwid/web_display.py 0644
python-urwid.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/urwid/html_fragment.py 0644
python-urwid.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/urwid/listbox.py 0644
python-urwid.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/urwid/widget.py 0644
python-urwid.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/urwid/escape.py 0644
python-urwid.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/urwid/util.py 0644
python-urwid.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/urwid/__init__.py 0644
python-urwid.i386: E: non-executable-script
/usr/lib/python2.5/site-packages/urwid/curses_display.py 0644

This is basically because the Python modules contain a #!/usr/bin/python shebang.

 [x] Package is not relocatable.
 [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: LGPLv2+
 [x] 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.
     SHA1SUM of tar.gz: 0a0b6e716ff6794900475463a0aaf8a9b4458ca0
 [x] Package is not known to require ExcludeArch
 [!] All build dependencies are listed in BuildRequires, except for any that are
listed in the exceptions section of Packaging Guidelines.

Are you sure this should be BuildRequires ncurses, not BuildRequires
ncurses-devel? I can't even get rpmbuild to run without ncurses (even Python
seems to require it). But building without ncurses-devel seems to run fine, so
not sure anything is needed here...

 [-] 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.
 [?] Permissions on files are set properly.
See the above issue with shebangs from rpmlint
 [x] Package has a %clean section, which contains rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
 [x] Package consistently uses macros.
 [x] Package contains code, or permissible content.
 [-] Large documentation files are in a -doc subpackage, if required.
 [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.                    
 [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.
 [x] All filenames present are valid UTF-8

=== SUGGESTED ITEMS ===
 [x] Latest version is packaged.
 [x] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
 [?] Reviewer should test that the package builds in mock.
Not tested...
 [?] Package should compile and build into binary rpms on all supported
architectures.
     Tested on: i386
 [x] Package functions as described.
 [-] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.

Hope the above is helpful...

Comment 8 manuel wolfshant 2008-03-27 12:48:07 UTC
1. it's not noarch:
gcc -pthread -DNDEBUG -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -D_G
NU_SOURCE -fPIC -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -fPIC -I/u
sr/include/python2.5 -c source/str_util.c -o
build/temp.linux-x86_64-2.5/source/str_util.o
2. ncurses does not seem to be needed at all, the package builds fine locally in
mock


Comment 9 David Timms 2008-06-20 12:42:09 UTC
(In reply to comment #5)
> > python-urwid.src: W: summary-not-capitalized console user interface for python
> > python-urwid.src: W: invalid-license LGPL
> 
> Fixed.
Niall:
Did you get a chance to update your spec with the review points already mentioned ?
If so, please provide the location of your .spec and .src.rpm

Comment 10 Niall Sheridan 2008-07-06 12:43:15 UTC
Hi

I think I've made all the recommended changes. The spec and src.rpm are at
http://www.evil.ie/fedora/rpms/python-urwid/

Comment 11 David Timms 2008-07-08 13:45:51 UTC
Niall: I am not a sponsor, but will help out of I can.

1. It is expected that when you have made requested changes to your package that
you post the actual clickable URLs of the spec and .src.rpm. 
  - Additionally, it can make a reviewers job easier to be able to see the
earlier .spec and .src.rpm's so that it is easier to follow the improvements
that get made. This is easy enough to do by renaming the earlier .spec {and
leaving it publically accessible} to include the version that it was eg
python-urwid-1.2-5.spec

2. Even in package review it is required to show your knowledge of how to handle
package changelogs within the .spec {and hence rpms} by describing your changes
to the .spec in the spec, and always incrementing the Release field:
https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs
  - preferably add changelogs for each updates that have been made already.

Comment 12 David Timms 2008-07-08 13:48:42 UTC
Also, please state the output of rpmlint {or quiet} for both the .src.rpm and
the .rpms build from it. Consider it good practice to always use this tool for
picking up things before the reviewer has to...

Comment 13 Niall Sheridan 2008-07-08 19:24:07 UTC
My apologies.
rpmlint output:
3 packages and 1 specfiles checked; 0 errors, 0 warnings.

srpm: http://www.evil.ie/fedora/rpms/python-urwid/python-urwid-0.9.8.2-1.fc9.src.rpm
spec: http://www.evil.ie/fedora/rpms/python-urwid/python-urwid.spec

I managed to lose the earlier spec and srpm by scping over them. I've fixed up
the changelog to reflect reality better.

Thanks for your feedback.

Comment 14 Jason Tibbitts 2008-10-17 05:12:23 UTC
I'm looking at some of the older tickets and thought I'd take care of this one.  I'm sorry you've had to wait so long.  Are you still interested in getting this package into Fedora?  If so, I'll review it, but I need to note a couple of things.  First off, rpmlint does spew a couple of complaints:
  python-urwid.x86_64: E: non-standard-executable-perm 
   /usr/lib64/python2.5/site-packages/urwid/str_util.so 0775
This one is some weird bug somewhere that has nothing to do with your package; occasionally the permissions on .so files built by python come out wrong.  I have no idea what causes it, but we're just ignoring it as the packages generated by the buildsystem seem to come out OK.

  python-urwid.x86_64: W: incoherent-version-in-changelog 0.9.8.2-2 
   0.9.8.2-1.fc10
You have an entry for 0.9.8.2-2 (i.e release 2) but the spec has Release: 1.

Also, it seems that 0.9.8.3 is out; would you like to update the package?

It seems reasonable to package up the examples (those .py files in the top level which aren't setup.py) as documentation.  Generally I'd recommend that you make them not executable but that's not necessary as long as they don't drag in additional dependencies.

There is an actual test suite included as test_urwid.py which, surprisingly, seems to run standalone.  You should call it in a %check section:
  %check
  %{__python} test_urwid.py
We always want to run test suites when possible, since we want as many sanity checks as possible.

Comment 15 Jason Tibbitts 2008-10-27 16:52:19 UTC
Any response to the above commentary?

Comment 16 Jason Tibbitts 2008-11-17 18:54:31 UTC
OK, it's been a month since I posted comment 14 and there's been no response.  I will close this ticket soon if nothing further happens.

Comment 17 Jason Tibbitts 2008-11-28 18:29:52 UTC
I guess not.  Closing.

Comment 18 Konstantinos 2009-07-07 12:32:34 UTC
Hi,
This is my first package. 
I checked the package and the spec and I updated them using the comments of Comment 14 and playing with rpmlint. Here is the spec and srpm.
 
SPEC URL:http://tartufo.dyndns.org/temp/python-urwid.spec
SRPM URL:http://tartufo.dyndns.org/temp/python-urwid-0.9.8.4-1.fc11.src.rpm

Comment 19 Dimitris Glezos 2009-07-07 12:36:31 UTC
Reopening, as we've got a new spec/srpm from Konstantinos who hasn't got bugzilla edit permissions. Hope this is OK.

Comment 20 David Timms 2009-07-07 13:41:09 UTC
Firstly, thanks for stepping up to work on the package. However, the original and current submitter need to decide who is wanting to become the package maintainer. If it is not the original submitter of the review request, I think the new packager should begin a new review request bug, referencing this (closed) bug. This makes for clearer reading of the work performed by the potential packager, for sponsors deciding to approve and sponsor the package.

Comment 21 Jason Tibbitts 2009-07-07 15:44:05 UTC
Yes, please open a new review request for the new package and close this one as a duplicate.

Comment 22 Dimitris Glezos 2009-07-07 16:53:49 UTC
Done. #510097

*** This bug has been marked as a duplicate of bug 510097 ***


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