[Bug 583327] Review Request: clementine - A music player and library organiser

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=583327

Mattias Ellert <mattias.ellert at fysast.uu.se> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-review?

--- Comment #8 from Mattias Ellert <mattias.ellert at fysast.uu.se> 2010-07-20 10:51:53 EDT ---
Fedora Review clementine 2010-07-20

rpmlint output:

$ rpmlint clementine-0.4.2-3.fc14.src.rpm \
          clementine-0.4.2-3.fc14.x86_64.rpm \
          clementine-debuginfo-0.4.2-3.fc14.x86_64.rpm 
clementine.src: W: invalid-url Source0:
http://clementine-player.googlecode.com/files/clementine-0.4.2.tar.gz HTTP
Error 404: Not Found
clementine.x86_64: W: no-manual-page-for-binary clementine
3 packages and 0 specfiles checked; 0 errors, 2 warnings.

Googlecode is infamous for triggering false 404 warnings in rpmlint.
Not quite sure why - wget seems to work without problem every time I try.


+ Package is named according to guidelines
+ Specfile is named after the package

- Guidelines say cmake projects should use "make VERBOSE=1":
  https://fedoraproject.org/wiki/Packaging/cmake

+ Package License tag GPLv3 and GPLv2+ is a Fedora approved license

- All the source files say "or (at your option) any later version", so it
  would make sense to say GPLv3+ and GPLv2+

The universalchardet (bundled code) is MPLv1.1 or GPLv2+ or LGPLv2+.
I guess we can choose to use the GPLv2+ in this case, which is alredy
covered in the tag.

+ License file (COPYING) is included as %doc (this is the GPLv3 text)
+ Specfile is written in legible English
+ Source matches upstream:

$ md5sum srpm/clementine-0.4.2.tar.gz clementine-0.4.2.tar.gz 
c6819b0d2a8324f1d686fb5a3b1d287b  srpm/clementine-0.4.2.tar.gz
c6819b0d2a8324f1d686fb5a3b1d287b  clementine-0.4.2.tar.gz

+ Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=2329614

- libprojectM-devel is listed twice as BuildRequires (once with a
  version and once without)


The build log says:
-- Building engines: gst
-- Skipping engines: vlc xine qt-phonon

So you are currently not building the xine engine - but you have
xine-lib-devel as a BuildRequires - is it needed in this case?

If you try to enable more engines you get a big warning:

> The following engines are NOT supported by clementine developers:
>  xine qt-phonon
>
> Don't post any bugs if you use them, fix them yourself!

So I guess you have a reason for not building them for Fedora.

Some other BuildRequires seems redundant too - did you remove those
that are only required to build the bundled libraries you do not build
anymore? E.g. glew-devel seems to only be used inside the libprojectM
code.


+ No locale files installed. (The translation files are mangled into
  the main binary by Qt.)
+ No shared libraries


This package must have been a nightmare w.r.t. bundled libraries. You
have done an excellent job unbundling the source. Very good job and
congratualations.


What remains is the universalchardet code. This is tricky. Did you
discuss this with some Fedora packaging people?

Googling a bit shows that this code is used by quite many projects,
and they all bundle it. It would be interesting to know how many
packages that exist in Fedora and bundle the code. I don't know any
reasonably fast way to figure that out though.

By doing a repoquery I found one package that installs the
universalchardet headers. In that package (codeblocks-devel) the code
is not compiled into a separate library, but bundled with a lot of
other stuff into a big library that has very many dependencies - so it
is not really a good option to use if you only want to use the
universalchardet. Installing the headerfiles for a bundled library the
way codeblocks-devel does seems wrong anyway.

In a perfect world, I think this would be best compiled as a shared
library in a separate package, having a common maintained codebase for
all users of the code. But you might have some arguments for treating
this case special.


? How do you ensure ownership of /usr/share/icons/hicolor/64x64/apps?

+ No duplicate files
+ Permissions are sane and %file has %defattr
+ Macros are used consistently in the Specfile
+ %doc is not runtime essential
+ No headerfiles
+ No static libraries
+ No libtool archive
+ .desktop file verified using desktop-file-validate
+ Package does not own other's directories
+ Installed files have valid UTF8 filenames

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.