Bug 173368 - Review Request: planet
Review Request: planet
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Richard Dawe
Fedora Package Reviews List
http://www.planetplanet.org/
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2005-11-16 12:25 EST by Richard Dawe
Modified: 2014-01-21 17:53 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-03-02 01:03:58 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
Spec file tweaks (3.61 KB, patch)
2005-11-21 12:30 EST, Paul Howarth
no flags Details | Diff

  None (edit)
Description Richard Dawe 2005-11-16 12:25:39 EST
Spec Name or Url: http://homepages.nildram.co.uk/~phekda/richdawe/fedora/FC4/planetplanet.spec
SRPM Name or Url: http://homepages.nildram.co.uk/~phekda/richdawe/fedora/FC4/planetplanet-1.0-0.1.20051115arch.src.rpm
Description:

From Planet's README:

"Planet is a flexible feed aggregator, this means that it downloads feeds and aggregates their content together into a single combined feed with the latest news first.

It uses Mark Pilgrim's Ultra-liberal feed parser so can read from RDF, RSS and Atom feeds and Tomas Styblo's template library to output static files in unlimited formats based on a series of templates."

It's built from a snapshot taken from the GNU arch repository yesterday. Note that the planetplanet.org page lists the wrong branch (or whatever arch calls them). It should be:

jdub@perkypants.org--projects/planet--devel--1.0

I've called the package "planetplanet" to match the executable name "planetplanet". The program from upstream is actually called planet.py, but I had problems with planet.py not being able to find the "planet" module that went away when I renamed the script (see <http://lists.planetplanet.org/pipermail/devel/2005-November/000714.html>). I chose planetplanet to match the website.

There are some rpmlint warnings & errors (see below), but I believe they're spurious. In particular, the errors on .py seem quite selective - rpmlint doesn't warn about the other .py files in the same directory. "Python license" was copied from the list of licences in the rpmlint Python code itself!

[rpmbuild@katrina noarch]$ rpmlint planetplanet-1.0-0.1.20051115arch.noarch.rpm W: planetplanet invalid-license Python license
E: planetplanet non-executable-script /usr/lib/python2.4/site-packages/planet/__init__.py 0644
E: planetplanet non-executable-script /usr/lib/python2.4/site-packages/planet/feedparser.py 0644
E: planetplanet non-executable-script /usr/lib/python2.4/site-packages/planet/cache.py 0644

If you want to give planetplanet a whirl, I can give you an example config. Just let me know.
Comment 1 Paul Howarth 2005-11-16 12:45:06 EST
(In reply to comment #0)
> There are some rpmlint warnings & errors (see below), but I believe they're
spurious. In particular, the errors on .py seem quite selective - rpmlint
doesn't warn about the other .py files in the same directory.

I'd hazard a guess that the .py files complained about start with shebangs, and
the others don't.

> "Python license" was copied from the list of licences in the rpmlint Python
code itself!

That's a good point!

> If you want to give planetplanet a whirl, I can give you an example config.
Just let me know.

I'd suggest including a sample config in the package, perhaps as a %doc

I'd also suggest looking at /usr/share/fedora/spectemplate-python.spec from the
fedora-rpmdevtools package regarding the packaging of python packages in general.
Comment 2 Ville Skyttä 2005-11-16 13:19:15 EST
The list of licenses rpmlint considers valid are listed 
in /etc/rpmlint/config. 
Comment 3 Richard Dawe 2005-11-16 13:54:21 EST
(In reply to comment #1)
> (In reply to comment #0)
[snip]
> > If you want to give planetplanet a whirl, I can give you an example config.
> Just let me know.
> 
> I'd suggest including a sample config in the package, perhaps as a %doc

Already does:

[rich@katrina i386]$ rpm -qpl planetplanet-1.0-0.2.20051115arch.noarch.rpm |
grep examples
/usr/share/doc/planetplanet-1.0/examples
/usr/share/doc/planetplanet-1.0/examples/atom.xml.tmpl
/usr/share/doc/planetplanet-1.0/examples/basic
/usr/share/doc/planetplanet-1.0/examples/basic/config.ini
/usr/share/doc/planetplanet-1.0/examples/basic/index.html.tmpl
/usr/share/doc/planetplanet-1.0/examples/fancy
/usr/share/doc/planetplanet-1.0/examples/fancy/config.ini
/usr/share/doc/planetplanet-1.0/examples/fancy/index.html.tmpl
/usr/share/doc/planetplanet-1.0/examples/foafroll.xml.tmpl
/usr/share/doc/planetplanet-1.0/examples/opml.xml.tmpl
/usr/share/doc/planetplanet-1.0/examples/rss10.xml.tmpl
/usr/share/doc/planetplanet-1.0/examples/rss20.xml.tmpl

I can give you a tarball of a configuration + stuff that works, if this isn't
enough.

> I'd also suggest looking at /usr/share/fedora/spectemplate-python.spec
> from the fedora-rpmdevtools package regarding the packaging
> of python packages in general.

Thanks for the tip!
Comment 4 Richard Dawe 2005-11-16 13:56:31 EST
Paul and Ville: Thanks for your help.

I have new packages based on the Python spec template. These also now rpmlint
cleanly. They're here:

Spec Name or Url:
http://homepages.nildram.co.uk/~phekda/richdawe/fedora/FC4/planetplanet.spec

SRPM Name or Url:
http://homepages.nildram.co.uk/~phekda/richdawe/fedora/FC4/planetplanet-1.0-0.2.20051115arch.src.rpm
Comment 5 Paul Howarth 2005-11-21 12:30:44 EST
Created attachment 121304 [details]
Spec file tweaks

Suggestions:

- no need to define python_sitearch macro since it's not used
- defining macros %{name}, %{version}, and %{release} is redundant and clutters
the specfile - just fill in the values for the appropriate RPM headers
- try to avoid using the project name in the summary, using something more
generic if possible; I'd suggest "Flexible feed aggregator"
- I'd normally suggest using a full URL for Source0 but upstream don't appear
to provide one; this is something to bear in mind though once a full 1.0
release appears
- include the INSTALL file as a document; normally that's a bad idea but in
this case there's lots of useful information for the package user in there
- specifically own the directories %{python_sitelib}/planet/ and
%{python_sitelib}/planet/compat_logging/ so that they get deleted if the
package is removed from the system

Other than that, it looks good to me (at first glance - this isn't a formal
review).
Comment 6 Richard Dawe 2005-12-09 15:57:07 EST
(In reply to comment #5)
> Created an attachment (id=121304) [edit]
> Spec file tweaks
> 
> Suggestions:
> 
> - no need to define python_sitearch macro since it's not used
> - defining macros %{name}, %{version}, and %{release} is redundant and clutters
> the specfile - just fill in the values for the appropriate RPM headers
> - try to avoid using the project name in the summary, using something more
> generic if possible; I'd suggest "Flexible feed aggregator"
> - I'd normally suggest using a full URL for Source0 but upstream don't appear
> to provide one; this is something to bear in mind though once a full 1.0
> release appears
> - include the INSTALL file as a document; normally that's a bad idea but in
> this case there's lots of useful information for the package user in there
> - specifically own the directories %{python_sitelib}/planet/ and
> %{python_sitelib}/planet/compat_logging/ so that they get deleted if the
> package is removed from the system
> 
> Other than that, it looks good to me (at first glance - this isn't a formal
> review).

Thanks. I've placed updated packages here:

http://homepages.nildram.co.uk/~phekda/richdawe/fedora/FC4/planet.spec
http://homepages.nildram.co.uk/~phekda/richdawe/fedora/FC4/planet-1.0-0.3.20051115arch.src.rpm

Note that I've renamed them from planetplanet at the request of the upstream
author, Jeff Waugh.
Comment 7 Kevin Fenzi 2006-02-05 16:11:02 EST
A review:

MUST items:

OK - No rpmlint output
OK - Package name.
OK - Spec file name matches.
OK - Package guidelines.
SEE #1 BELOW - License. (Python) (but see item #1 below)
OK - License field matches in spec.
OK - License included in files.
OK - Spec in american english.
SEE #2 BELOW - md5sum of source from upstream
OK - Compiles and builds on one arch at least.
OK - No forbidden buildrequires included
OK - Owns all directories it creates.
OK - No duplicate files in %files listing.
OK - Permissions on files correct.
OK - Clean section correct.
OK - Macros consistant.
OK - Code not content.
OK - Doesn't own any files/dirs that are already owned by other packages.

Items needing attention:

1. The package is licensed under the Python license, but the planet/htmltmpl.py
file appears to be under the GPL. Doesn't that mean the entire package has
to be released under the GPL?

2. Wanted to check the md5sum against upstream, but it's unclear how to
get the specific version you are using out of their arch/bzr setup.
Can you provide a bzr command to get that version?
Also, you might want to upgrade to the latest.

SHOULD Items:

OK - Package builds in mock.
OK - Binary rpms on all arches. (x86 and x86_64 at least, don't have ppc)
OK - Check for functionality. Seems to work fine here.

Additionally, since you are seeking a sponsor, you should consider
commenting on others packages to show your good understanding of the
package guidelines. You can find the list of packages awaiting review at:

https://bugzilla.redhat.com/bugzilla/showdependencytree.cgi?id=FE-NEW&hide_resolved=1

and the ones in review and seeking more comments at:

https://bugzilla.redhat.com/bugzilla/showdependencytree.cgi?id=FE-REVIEW&hide_resolved=1
Comment 8 Richard Dawe 2006-02-18 10:13:01 EST
Thanks for the further review!

Response to items needing attention:

1. The Python licence is listed as GPL-compatible by the FSF -- see
<http://www.fsf.org/licensing/licenses/index_html#GPLCompatibleLicenses>. So no
problem there.

2. I've updated my rpms:

http://homepages.nildram.co.uk/~phekda/richdawe/fedora/FC4/planet.spec
http://homepages.nildram.co.uk/~phekda/richdawe/fedora/FC4/planet-1.0-0.5.20060218pre.src.rpm
http://homepages.nildram.co.uk/~phekda/richdawe/fedora/FC4/i386/planet-1.0-0.5.20060218pre.noarch.rpm

These are built off planet--devel--1.0--patch-20, which I got using:

baz register-archive http://www.gnome.org/~jdub/arch
baz get jdub@perkypants.org--projects/planet--devel--1.0--patch-20

I've put MD5 sums of the files I downloaded in planet--devel--1.0--patch-20 here:

http://homepages.nildram.co.uk/~phekda/richdawe/fedora/FC4/MD5SUM.planet--devel--1.0--patch-20

The 1.0 release was due to appear two weeks ago...but hasn't.
Comment 9 Kevin Fenzi 2006-02-26 13:34:39 EST
1. ok. 

2. ok. I have checked the md5sum's and they match up fine. 

I see 1.0 still isn't out yet. They did push out a test 1.0 tar/zip file, but
that was only a test release, not something you would want to base a package on. 

I ran a mockbuild and some testing on the packages in comment #8 and everything
looks good, so this package is APPROVED. 

I will be happy to sponsor you. You should move forward from the 
"Get a Fedora Account" section of the
http://fedoraproject.org/wiki/Extras/Contributors page. 

If you have any questions at all, feel free to drop me an email. 
Comment 10 Richard Dawe 2006-03-02 01:10:10 EST
Oops, I seem to have accidentally closed this bug by taking ownership of it. I'm
in the process of submitting a build, so I'll change the resolution to
NEXTRELEASE when that's done.
Comment 11 Christian Iseli 2006-10-18 08:57:05 EDT
Normalize summary field for easy parsing

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