Bug 220775 - Review Request: exaile - A music player
Summary: Review Request: exaile - A music 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: Parag AN(पराग)
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-12-26 15:58 UTC by Deji Akingunola
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: 2006-12-29 06:10:46 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Deji Akingunola 2006-12-26 15:58:47 UTC
Spec URL: ftp://czar.eas.yorku.ca/pub/exaile/exaile.spec
SRPM URL: ftp://czar.eas.yorku.ca/pub/exaile/exaile-0.2.6-1.src.rpm
Description: Exaile is a media player aiming to be similar to KDE's AmaroK, but for GTK+.
It incorporates many of the cool things from AmaroK (and other media players)
like automatic fetching of album art, handling of large libraries, lyrics
fetching, artist/album information via the wikipedia, last.fm support, optional
iPod support (assuming you have python-gpod installed).

In addition, Exaile also includes a built in shoutcast directory browser,
tabbed playlists (so you can have more than one playlist open at a time),
blacklisting of tracks (so they don't get scanned into your library),
downloading of guitar tablature from fretplay.com, and submitting played tracks
on your iPod to last.fm

Comment 1 Parag AN(पराग) 2006-12-27 04:24:35 UTC
pacakaging looks nice.
as its pygtk application no need for -devel for included .so file.
desktop file even worked well.
But mock build failed with
In file included from mmkeys.override:6:
/usr/include/pygtk-2.0/pygobject.h:20: error: expected specifier-qualifier-list
before 'PyObject'
/usr/include/pygtk-2.0/pygobject.h:27: error: expected specifier-qualifier-list
before 'PyObject_HEAD'
/usr/include/pygtk-2.0/pygobject.h:38: error: expected specifier-qualifier-list
before 'PyObject_HEAD'
/usr/include/pygtk-2.0/pygobject.h:48: error: expected specifier-qualifier-list
before 'PyObject_HEAD'
/usr/include/pygtk-2.0/pygobject.h:60: error: expected specifier-qualifier-list
before 'PyObject_HEAD'
/usr/include/pygtk-2.0/pygobject.h:67: error: expected declaration specifiers or
'...' before 'PyTypeObject'
/usr/include/pygtk-2.0/pygobject.h:68: error: expected '=', ',', ';', 'asm' or
'__attribute__' before '*' token
/usr/include/pygtk-2.0/pygobject.h:76: error: expected ')' before '*' token
/usr/include/pygtk-2.0/pygobject.h:78: error: expected ';' before 'void'
mmkeys.c:16: error: expected '=', ',', ';', 'asm' or '__attribute__' before '*'
token
mmkeys.c:21: warning: data definition has no type or storage class
mmkeys.c:21: error: expected ',' or ';' before 'PyMmKeys_Type'
mmkeys.c:30: error: expected declaration specifiers or '...' before 'PyObject'
mmkeys.c:30: error: expected declaration specifiers or '...' before 'PyObject'
mmkeys.c: In function '_wrap_mmkeys_new':
mmkeys.c:34: error: 'args' undeclared (first use in this function)
mmkeys.c:34: error: (Each undeclared identifier is reported only once
mmkeys.c:34: error: for each function it appears in.)
mmkeys.c:34: error: 'kwargs' undeclared (first use in this function)
mmkeys.c:39: error: 'struct _PyGObject_Functions' has no member named
'pygobject_constructv'
mmkeys.c:40: error: 'PyGObject' has no member named 'obj'
mmkeys.c:42: error: 'PyExc_RuntimeError' undeclared (first use in this function)
mmkeys.c: At top level:
mmkeys.c:49: warning: data definition has no type or storage class
mmkeys.c:49: error: expected ',' or ';' before 'PyMmKeys_Type'
mmkeys.c:98: error: expected '=', ',', ';', 'asm' or '__attribute__' before
'mmkeys_functions'
mmkeys.c:104: error: expected ')' before '*' token
make[1]: *** [mmkeyspy.o] Error 1
make[1]: Leaving directory `/builddir/build/BUILD/exaile_0.2.6/mmkeys'
make: *** [mmkeys.so] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.14826 (%install)

Maybe you need to add python-devel as BR.

Comment 2 Deji Akingunola 2006-12-27 13:28:04 UTC
(In reply to comment #1)
> 
> Maybe you need to add python-devel as BR.

I already did add python-devel as BR, and I also built it succesfully in rawhide
mock. However, I think i know where the problem is. The updated version below
should be ok.
Spec URL: ftp://czar.eas.yorku.ca/pub/exaile/exaile.spec
SRPM URL: ftp://czar.eas.yorku.ca/pub/exaile/exaile-0.2.6-2.src.rpm

Thanks for doing the review.

Comment 3 Parag AN(पराग) 2006-12-28 07:39:22 UTC
Same mock build errors i got.
Then i removed patch and it worked well in mock build

Comment 4 Parag AN(पराग) 2006-12-28 07:48:19 UTC
Also add license.txt to %doc
then will do final review.

Comment 5 Deji Akingunola 2006-12-28 16:12:48 UTC
The patch is necessary for build on fedora devel. I've reworked the patch, and
tested it on FC6 so it should be o.k. now
I don't think adding license.txt to %doc is necessary, it just repeating what's
been already specified in the spec, that the package is under GPL license.

Spec URL: ftp://czar.eas.yorku.ca/pub/exaile/exaile.spec
SRPM URL: ftp://czar.eas.yorku.ca/pub/exaile/exaile-0.2.6-3.src.rpm

Comment 6 Parag AN(पराग) 2006-12-29 04:48:39 UTC
IMHO, AFAIK %doc is used to install files in /usr/share/doc right? So if you
include license.txt to %doc then other users who don't have SPEC can see which
license this package is using.
Review Guidelines said that if a source package contains license text included
as separate file then that file must be added to %doc


Comment 7 Deji Akingunola 2006-12-29 05:45:35 UTC
(In reply to comment #6)
> IMHO, AFAIK %doc is used to install files in /usr/share/doc right? So if you
> include license.txt to %doc then other users who don't have SPEC can see which
> license this package is using.
one can always do 'rpm -qi exaile | grep License'

> Review Guidelines said that if a source package contains license text included
> as separate file then that file must be added to %doc
Note it says 'license text'; that file, license.txt, does not contain the
license text, it only expresses what the license is.
Thanks. 



Comment 8 Parag AN(पराग) 2006-12-29 05:52:54 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > IMHO, AFAIK %doc is used to install files in /usr/share/doc right? So if you
> > include license.txt to %doc then other users who don't have SPEC can see which
> > license this package is using.
> one can always do 'rpm -qi exaile | grep License'
  ahh i forgot that option. Thanks.
> 
> > Review Guidelines said that if a source package contains license text included
> > as separate file then that file must be added to %doc
> Note it says 'license text'; that file, license.txt, does not contain the
> license text, it only expresses what the license is.
> Thanks. 

 I got this point now thanks for explaining this to me.

Comment 9 Parag AN(पराग) 2006-12-29 05:55:56 UTC
Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM and RPMS.
+ source files match upstream.
05f8ad394f872f24c201d51687c96890  exaile_0.2.6.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.  License text is not included in package.
+ %doc is small; no -doc subpackage required.
+ %doc does not affect runtime.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code, not content.
+ no headers or static libraries.
+ no .pc file present.
+ no -devel subpackage exists
+ no .la files.
+ no translations are available
+ Dose owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ Desktop file installed successfully
+ Desktop file is handled correctly in SPEC file.
+ GUI app
APPROVED.


Comment 10 Parag AN(पराग) 2006-12-29 05:57:41 UTC
Don't Forget to CLOSE this Review once package will be imported in CVS

Comment 11 Deji Akingunola 2006-12-29 06:10:46 UTC
Imported into CVS. 
Thanks Parag.

Comment 12 Christian Iseli 2007-01-02 23:29:25 UTC
Changed summary for tracking purposes.



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