Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 229651 - Review Request: Democracy - A internet TV video player
Summary: Review Request: Democracy - A internet TV video player
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-02-22 14:43 UTC by Thorsten Scherf
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-02-24 10:39:39 UTC
rdieter: fedora-review+
dennis: fedora-cvs+


Attachments (Terms of Use)

Description Thorsten Scherf 2007-02-22 14:43:08 UTC
Spec URL: http://people.redhat.com/tscherf/rpms/fedora/Democracy.spec
SRPM URL: http://people.redhat.com/tscherf/rpms/fedora/Democracy-0.9.5-1.src.rpm
Description: Democracy player is a free application that turns your computer into an internet TV video player.

Comment 1 Rex Dieter 2007-02-22 14:48:18 UTC
Awesome!

A couple quick comments:
1. No need to %ghost .pyo files anymore, as a matter of fact, it is 
discouraged:
http://fedoraproject.org/wiki/Packaging/Python

2.  Use %find_lang instead of manually including locale bits:
%{_datadir}/locale/*/LC_MESSAGES/democracyplayer.mo

3.  You include
%post
update-desktop-database %{_datadir}/applications
but not for %postun ?

Comment 2 Gianluca Sforna 2007-02-22 14:59:53 UTC
(In reply to comment 
> A couple quick comments:
> 1. No need to %ghost .pyo files anymore, as a matter of fact, it is 
> discouraged:
> http://fedoraproject.org/wiki/Packaging/Python

moreover, I think the python-abi require line could go away.

> 3.  You include
> %post
> update-desktop-database %{_datadir}/applications
> but not for %postun ?

and those lines should probably end with "&> /dev/null ||:" as specified in:
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets


Moreover, do they fixed the x86_64 issues present in earlier versions?



Comment 3 Rex Dieter 2007-02-22 15:05:34 UTC
> Moreover, do they fixed the x86_64 issues present in earlier versions?

"they" being who?  democracy devs?
"the x86_64 issues" being what exactly?  didn't build?  didn't run properly?

Comment 4 Xavier Lamien 2007-02-22 15:11:26 UTC
please, use %{_mandir}/man1/ instead of %{_datadir}/man/man1/

> # Include files and dirs below %{python_sitelib} (for noarch packages) 

Where ?

the use of cd platform/gtk-x11 looks good.
personnaly, i prefer to use pushd-command to enter in some directory and
popd-command to exit this one (same thing for %build section).
such as:
----
pushd gtk-x11
python setup.py install -O1 --skip-build --root $RPM_BUILD_ROOT
popd
----
i think it's better for a clean review.

Comment 5 Gianluca Sforna 2007-02-22 15:25:30 UTC
(In reply to comment #3)
> > Moreover, do they fixed the x86_64 issues present in earlier versions?
> 
> "they" being who?  democracy devs?
> "the x86_64 issues" being what exactly?  didn't build?  didn't run properly?

sorry for the out-of-context comment.
I was referring to this:
https://www.redhat.com/archives/fedora-extras-list/2006-November/msg00104.html


Comment 6 Thorsten Scherf 2007-02-22 16:39:48 UTC
1) removed %ghost from files section
2) used find_lang 
3) created a postun 
4) added "&> /dev/null ||:" to the Scriptlet
5) AFAIK the x86_64 bug should be fixed, can't test it myself, maybe someone
else can verify this

http://people.redhat.com/tscherf/rpms/fedora/Democracy.spec
http://people.redhat.com/tscherf/rpms/fedora/Democracy-0.9.5-2.src.rpm


Comment 7 Rex Dieter 2007-02-22 16:54:04 UTC
Looking good, I can review this.

1. SHOULD simplify things and just include in %files:
%{python_sitearch}/democracy/
instead of all the %dirs and * globbing.

2.  Requires: firefox
Other apps that build against firefox-devel need/use a *versioned* requires 
here, in effect Require'ing the same version of firefox they were built 
against.  Is that the case here?  (or maybe not worry about it (: )

3.  Requires: python-abi ...
shouldn't be explictly required (when using python >= 2.4 anyway).

4.  Requires:	xine-lib gnome-python2-gtkmozembed libfame gnome-python2-gconf 
dbus-python
Are *all* of these really explicitly required?  In particular, xine-lib 
libfame should get auto'req'd by rpm (if not, it's ok to keep the Requires).

Comment 8 Thorsten Scherf 2007-02-22 17:04:28 UTC
1) simplified the files section
3) removed python-abi req
4) rpm should take care about this, true, removed the req

about 2) not sure if we need a versioned firefox requirement. I tested this
package on 2 difeerent machines with different firefox versions without
problems, so it should work. could you verify this?

http://people.redhat.com/tscherf/rpms/fedora/Democracy.spec
http://people.redhat.com/tscherf/rpms/fedora/Democracy-0.9.5-3.src.rpm


Comment 9 Rex Dieter 2007-02-22 17:19:31 UTC
mock build failed on Democracy-0.9.5-3.src.rpm
...
Package config error:
pkg-config --list-all outputted the following error:
Package xfixes was not found in the pkg-config search path.
Perhaps you should add the directory containing `xfixes.pc'
to the PKG_CONFIG_PATH environment variable
Package 'xfixes', required by 'Xcursor', not found

error: Bad exit status from /var/tmp/rpm-tmp.89920 (%build)

Either a missing BR or an X-dep missing/bug.

Comment 10 Rex Dieter 2007-02-22 17:58:02 UTC
after adding 
BuildRequires: libXcursor-devel libXfixes-devel
now I see
RuntimeError: pkg-config --cflags --libs gtk+-2.0 glib-2.0 pygtk-2.0
firefox-gtkmozembed firefox-xpcom outputted the following error:
Package gtk+-2.0 was not found in the pkg-config search path.
Perhaps you should add the directory containing `gtk+-2.0.pc'
to the PKG_CONFIG_PATH environment variable
No package 'gtk+-2.0' found

adding 
BuildRequires: gtk2-devel
iterating again...  finished build.

So, looks like we need to add:
BuildRequires: libXcursor-devel libXfixes-devel
BuildRequires: gtk2-devel

Comment 11 Rex Dieter 2007-02-22 18:06:17 UTC
These look mostly harmless:

$ rpmlint Democracy-0.9.5-3.fc7.i386.rpm
E: Democracy non-executable-script
/usr/lib/python2.5/site-packages/democracy/coverage.py 0644
E: Democracy non-executable-script
/usr/lib/python2.5/site-packages/democracy/timetemplates.py 0644
W: Democracy wrong-file-end-of-line-encoding /usr/share/doc/Democracy-0.9.5/CREDITS
E: Democracy non-executable-script
/usr/lib/python2.5/site-packages/democracy/feedparser.py 0644

Add the missing BR's and looks like we have a winner.

Comment 12 Thorsten Scherf 2007-02-22 18:15:33 UTC
Looks like we figured out the same thing during the same time. :) Was also
looking  for addional BR after Mock building failed. Added the necessary BR and
now it builds in Mock without problems.

http://people.redhat.com/tscherf/rpms/fedora/Democracy.spec
http://people.redhat.com/tscherf/rpms/fedora/Democracy-0.9.5-4.src.rpm

Comment 13 Rex Dieter 2007-02-22 18:19:09 UTC
Democracy rules, APPROVED.

Be careful about removing some of the python runtime Req's, I didn't verify if 
those are still Required or not:  gnome-python2-gtkmozembed 
gnome-python2-gconf dbus-python

Comment 14 Thorsten Scherf 2007-02-22 21:02:13 UTC
New Package CVS Request
=======================
Package Name: Democracy
Short Description: Internet TV and video player
Owners: tscherf@redhat.com
Branches: FC-5 FC-6 devel
InitialCC: 

Comment 15 Dennis Gilmore 2007-02-23 13:03:10 UTC
branched

Comment 16 Dennis Gilmore 2007-02-23 13:03:29 UTC
branched

Comment 17 Michael Schwendt 2007-02-23 22:46:24 UTC
Is there a good reason why the spec does unusual manual installation
of documentation via %_defaultdocdir instead of simply doing

%doc README license.txt CREDITS

in the %files section?


Comment 18 Rex Dieter 2007-02-24 00:06:16 UTC
> Is there a good reason...

not really, is it a problem?

Comment 19 Michael Schwendt 2007-02-24 02:35:56 UTC
It makes installation of additional files via short %doc impossible,
because both techniques conflict with eachother. Using the %doc macro
to include files located in the extracted tarball in $RPM_BUILD_DIR
deletes %_defaultdocdir.


Comment 20 Thorsten Scherf 2007-02-24 10:39:39 UTC
I corrected the doc installation style. updated package (0.9.5.1-3) is now in
devel and fc6 branch available. 

will close here.
 


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