Bug 193103

Summary: Review Request: Listen
Product: [Fedora] Fedora Reporter: Haïkel Guémar <karlthered>
Component: Package ReviewAssignee: Kevin Fenzi <kevin>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: hugo, martin.sourada, panemade
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-10-09 11:15:12 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 192490, 192491, 193102    
Bug Blocks: 163779    

Description Haïkel Guémar 2006-05-25 10:47:16 UTC
Spec URL: http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SPECS/listen.spec
SRPM URL: http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SRPMS/listen-0.4.3-2.src.rpm
Description: Listen is a music manager and player for GNOME
Needs python-ogg, python-vorbis and python-mad
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=193102

I need a sponsor

Comment 1 Ville Skyttä 2006-05-25 11:12:30 UTC
If the dependency on python-mad can't be avoided, this cannot be included in FE.

Comment 2 Hugo Cisneiros 2006-05-25 11:46:51 UTC
I began packaging this, but I saw that it requires libmad and it contains mp3 
code in the tarball. This is not allowed in Extras, and a patch would be a 
little difficult to create (maybe talking with upstream). An alternative would 
be to put the package in a third-party repository.

BTW, python-ogg and python-vorbis will be commited shortly (I Hope ;)

Comment 3 Haïkel Guémar 2006-05-25 12:17:14 UTC
In fact, the dependency against libmad is not mandatory to build and use Listen
with free formats (for proprietary formats, just need to install the right
packages), just need to patch check.py to finish the build.
here are new spec and SRPMS
http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SPECS/listen.spec
http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SRPMS/listen-0.4.3-3.src.rpm

Comment 4 Hugo Cisneiros 2006-05-30 10:24:03 UTC
I just imported python-ogg and python-vorbis. Now it can have the proper 
dependencies satisfied.

Comment 5 Parag AN(पराग) 2006-06-01 07:09:54 UTC
No INSTALL , README, GPL/LICENSE file included in SPEC's %doc tag. Also Source
does not contain any INSTALL and README file.

Comment 6 Hugo Cisneiros 2006-06-01 07:28:13 UTC
Note: Generic compilation/installation files (INSTALL) should not be included 
in the %doc tag at all, as per PackagingGuidelines.

Comment 7 Haïkel Guémar 2006-06-01 17:46:09 UTC
Listen has no INSTALL, README files.
Anyway, I added licence files (gpl.txt and copy) and fixed some issues with rpmlint.
http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SPECS/listen.spec
http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SRPMS/listen-0.4.3-4.src.rpm

rpmlint output
$ rpmlint /home/karl/rpmbuild/RPMS/i386/listen-0.4.3-4.i386.rpm
W: listen unstripped-binary-or-object /usr/lib/listen/mmkeys.so

@Hugo Cisneiros: great news :)

Comment 8 Kevin Fenzi 2006-10-01 21:02:43 UTC
OK - Package name
OK - Spec file matches base package name.
OK - Meets Packaging Guidelines.
OK - License (GPL)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
See below - Sources match upstream md5sum:
1a008fbf898c4cfe2b0efd58905463a1  listen-0.4.3.tar.gz
998a9df094ee72efc84253f36a957b5c  listen-0.4.3.tar.gz.1
See below - Package compiles and builds on at least one arch.
Pending - BuildRequires correct
Pending - Package owns all the directories it creates.
Pending - Package has no duplicate files in %files.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
See below - Spec has consistant macro usage.
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package is a GUI app and has a .desktop file
Pending - Package doesn't own any directories other packages own.
Pending - No rpmlint output.

SHOULD Items:

OK - Should include License or ask upstream to include it.
See below - Should build in mock.

Issues:

1. Could you re-name the two patches to have 'listen-' on the front?
Makes them easier to find in a SOURCES directory.

2. The desktop file adds the following mime types:
MimeType=audio/x-musepack;application/x-musepack;audio/musepack;application/
musepack;application/x-ape;audio/ape;audio/x-ape;audio/x-musepack;application/x-
musepack;audio/musepack;application/musepack;application/x-ape;audio/ape;audio/
x-ape;audio/x-mp3;application/x-id3;audio/mpeg;audio/x-mpeg;audio/x-mpeg-
3;audio/mpeg3;audip/mp3;audio/x-mp3;application/x-id3;audio/mpeg;audio/x-
mpeg;audio/x-mpeg-3;audio/mpeg3;audip/mp3;audio/x-m4a;audio/x-m4a;audio/
mpc;audio/x-mpc;audio/mp;audio/x-mp;audio/mpc;audio/x-mpc;audio/mp;audio/x-
mp;application/ogg;application/x-ogg;audio/vorbis;audio/x-vorbis;audio/
ogg;audio/x-ogg;application/ogg;application/x-ogg;audio/vorbis;audio/x-
vorbis;audio/ogg;audio/x-ogg;audio/x-flac;application/x-flac;audio/flac;audio/x-
flac;application/x-flac;audio/flac;

Perhaps remove the ones it can no longer handle without libmad?

3. The sources don't match upstream. It appears your source has a link from
listen to listen.py that the upstream doesn't have. You should use the
upstream source and only add things in the build process.

4. You are mixing usage of the %{buildroot} and $RPM_BUILD_ROOT macros.
Pick one and use only that one.

5. It doesn't seem to build under mock. It gets to the Xvfb stuff, but never
past it. Does it work for you under mock? I know other packages that have
needed an X server have used this, so you might check other packages in
extras for working magic on this.


Comment 9 Haïkel Guémar 2006-10-05 12:54:37 UTC
This package is a bit outdated, I updated to 0.5 beta 1.
The 0.4.x branch is no more maintained, and the last "stable" release is more
buggy than the 0.5 beta 1

http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SPECS/listen.spec
http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SRPMS/listen-0.5b1-2.src.rpm

1. No more patches, everything was fixed upstream, some Ubuntu-isms in the
Makefile are corrected by sed one-liners.
2. Listen comes now with a .desktop file.
3. fixed upstream
4. A mistake from the old days, corrected.
5. I found why
a) it needs gtk2-devel
b) then, Xvfb should be called with " -nolisten tcp -ac -terminate "
Now, it builds fine under Mock for fc5 & fc6 i386

-> rpmlint output 
$ rpmlint -i listen-0.5b1-2.fc5.i386.rpm
E: listen script-without-shebang /usr/share/listen/trackedit.glade
This text file has executable bits set or is located in a path dedicated
for executables, but lacks a shebang and cannot thus be executed.  If the file
is meant to be an executable script, add the shebang, otherwise remove the
executable bits or move the file elsewhere.

Comment 10 Martin Sourada 2006-10-07 16:08:32 UTC
Some time ago I also made a listen package, which is used by people on
fedoraforums.org.

The spec file can be found here:
http://forums.fedoraforum.org/forum/attachment.php?attachmentid=9292
and source rpms (gziped) here:
http://forums.fedoraforum.org/forum/attachment.php?attachmentid=9295

I have this rpmlint output:
rpmlint listen-0.5-0.b1.4.i386.rpm
W: listen unstripped-binary-or-object /usr/lib/listen/mmkeys.so

Comment 11 Kevin Fenzi 2006-10-08 03:07:06 UTC
1. Ok. 

2. Yeah, but it still has mime types for things like "audio/mpeg3", which it 
can't play if it doesn't have python-mad, right? 

3 - 4. ok. 

5. Yeah, it builds ok in mock here now too. ;) 

I think your version/release is not correct... 
Look at: 

http://fedoraproject.org/wiki/Packaging/NamingGuidelines#NonNumericRelease

perhaps something like release: 0.5 version: 0.2.b1 ?

You might also take a look at Martins sepc from comment #10 and see if he 
has any improvements. ;) 

Comment 12 Haïkel Guémar 2006-10-08 21:33:13 UTC
http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SPECS/listen.spec
http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SRPMS/listen-0.5-3.beta1.src.rpm

mpeg3 playback in Listen uses Gstreamer, mp3 can be read without python-mad if
you have installed the right gstreamer plugins from third party repositories or
Fluendo mp3 plugin.
Found a new desktop-file-install trick: --remove-mime-type , audio/mp3 & cie
mimetypes were removed.
The rights issue with trackedit.glade was fixed thanks to Martin.
-> $ rpmlint -i listen-0.5-3.beta1.fc5.i386.rpm
$

@Martin Sourada: debug infos aren't correctly stripped, you should fix the
rights of mmkeys.so
chmod +x $RPM_BUILD_ROOT%{_libdir}/%{name}/mmkeys.so
You should think using sed one-liners instead of patches for small fixes, it
will ease the maintenance of your package :)

Comment 13 Kevin Fenzi 2006-10-09 01:33:21 UTC
Excellent. All the blockers I saw are fixed, so this package is APPROVED. 

Don't forget to close this bug NEXTRELEASE once it's been imported and built. 

Also, consider reviewing another package thats waiting for review to help 
spread out the reviewing load. 

Comment 14 Haïkel Guémar 2006-10-09 11:15:12 UTC
The build failed on x86_64, since listen installs everything in /usr/lib instead
of /usr/lib64. 
I've increased release tag and moved /usr/lib/listen to /usr/lib64 for arch
x86_64, after many builds (!), it succeeded.I'll be waiting feedbacks from
x86_64 users.
I'll see if I can help in reviewing :)

Comment 15 Kevin Fenzi 2006-10-09 15:06:39 UTC
:( Sorry about that. I usually run file and directory checks at the beginning 
of a review, and because this didn't build in mock for me, I didn't do that, 
then forgot before approval. ;( 

As mschwent pointed out also, you need to own: 
%{_datadir}/%{name}

Sorry again, I should have caught that and the x86_64 issue. :( 

Comment 16 Haïkel Guémar 2006-10-09 16:13:26 UTC
No worries, the matter is that I don't have any x86_64 machine to test build and
mock fails each time I use another hardware profile than x86 on my workstation :|

Comment 17 Martin Sourada 2006-10-10 19:51:31 UTC
Thanks for the mmkeys.so trick and patches tip:) I'm looking for the time, when
your listen package will be in extras.

Comment 18 Martin Sourada 2006-10-10 20:17:18 UTC
Just few hints about your spec file:
From what I saw in listen sources, musicbrainz support is only runtime, so you
should change 
# MusicBrainz support
BuildRequires: python-musicbrainz2 
BuildRequires: libtunepimp 
to
# MusicBrainz support
Requires:	python-musicbrainz2 
Requires:	/usr/bin/puid 
Requires:	/usr/lib/libtunepimp.so.5
I think.
Maybe the last line is optional... since libtunepimp should be loaded due to
puid dependency.
Next, python-vorbis package seems to be no longer needed in the new version.
Maybe the BuildRequires and Requires sections need further review (to look if
all optional dependencies aren't omitted during build or in runtime by mistake
and if there are not any dependencies no longer needed)?

And one question: is it possible to change listen translation files (*.mo)
directory from /usr/lib/listen/po/ to fedora default one (/usr/share/locale/)
and use then the %find_lang macro?

Comment 19 Haïkel Guémar 2006-10-12 07:21:24 UTC
right, python-ogg and python-vor aren't needed in 0.5.x branch, I'll lock
further about R & BR. I'm not sure that optionnal stuff should be required, some
people will complaint about unecessary downloads.
About translation files, it shouldn't be too difficult to move them, you can
either modify the path in Makefile (ie:
$(PREFIX)/share/locale/$$lang/LC_MESSAGES/ instead of
$(PREFIX)/lib/listen/po/$$lang/LC_MESSAGES/), or move them during %install time.
I'll try to push an update before the end of the week.

Comment 20 Martin Sourada 2006-10-13 21:25:35 UTC
Ok, I installed your package in FCpre6 (almost clear installation) and this is
what I got:
ImportError: No module named pygst
so I installed gstreamer-python. Next I got:
ImportError: No module named mutagen.apev2
so I installed python-mutagen. Next I got:
ImportError: No module named gtkhtml2
so I installed gnome-python2-gtkhtml2. Next I got:
ImportError: No module named gtkmozembed
so I installed gnome-python2-gtkmozembed.

So for the runtime dependences you should at least add to you spec file this
section:
Requires: gstreamer-python
Requires: python-mutagen
Requires: gnome-python2-gtkhtml2
Requires: gnome-python2-gtkmozembed

As for the run-time optional dependencies: It says during startup this:
'No musicbrainz2 support' and 'No Audio cd support' - that can be solved by
installing python-musicbrainz2 and libtunepimp (>=0.5 since it needs
/usr/bin/puid). Also one optional dependency (both build- and run-time) is
python-gpod, but as far as I know this part of libgpod is not yet included in
fedora :(

Comment 21 Haïkel Guémar 2006-10-14 16:10:39 UTC
Check the new package listen-0.5-6.beta1 or better wait listen-0.5-7.beta1 build
, I had updated the spec with more requires. It only lacks
gnome-python2-gtkhtml2, which will be added now.