Bug 719103 - Review Request: media-explorer - Media centre application
Summary: Review Request: media-explorer - Media centre application
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Peter Robinson
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-07-05 19:10 UTC by Bastien Nocera
Modified: 2011-11-14 22:28 UTC (History)
7 users (show)

Fixed In Version: media-explorer-0.3.2-1.fc16
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-11-14 22:28:29 UTC
Type: ---
Embargoed:
pbrobinson: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Bastien Nocera 2011-07-05 19:10:44 UTC
Spec URL: http://people.fedoraproject.org/~hadess/mex/mex.spec
SRPM URL: http://people.fedoraproject.org/~hadess/mex/mex-0.1.2-1.fc15.src.rpm
Description: Media explorer is a media centre application for Linux, originally
targeted towards MeeGo.

scratch build for F15 at:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3181307

F16 build fails because of libmx bugs.

Comment 1 Bastien Nocera 2011-08-01 14:07:38 UTC
Update to 0.1.4:
http://people.fedoraproject.org/~hadess/mex/mex-0.1.4-1.fc15.src.rpm
http://people.fedoraproject.org/~hadess/mex/mex.spec

Scratch build on F15:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3243408

Should probably work on F16 after dust has settled there.

Comment 2 Volker Fröhlich 2011-08-23 16:13:59 UTC
rpmlint mex-0.1.4-1.fc17.x86_64.rpm 
mex.x86_64: W: spelling-error Summary(en_US) centre -> center, cent re, cent-re
mex.x86_64: W: spelling-error %description -l en_US centre -> center, cent re, cent-re
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/40mex-recommended.so ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/100mex-debug.so ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/20mex-gnome-dvb.so ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/40mex-queue.so ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/00mex-search.so ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/20mex-tracker.so ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/20mex-upnp.so ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/mex-webremote ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/mex ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/50mex-dbusinput.so ['/usr/lib64']
mex.x86_64: E: incorrect-fsf-address /usr/share/doc/mex-0.1.4/COPYING
mex.x86_64: E: postun-without-ldconfig /usr/lib64/libmex-0.2.so.0.0.0
mex.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libmex-0.2.so
mex.x86_64: W: no-manual-page-for-binary mex-webremote
mex.x86_64: W: no-manual-page-for-binary mex-debug
mex.x86_64: W: no-manual-page-for-binary mex

Get rid of the rpaths. See http://fedoraproject.org/wiki/Packaging:Guidelines#Removing_Rpath

Please contact the developer, so he can update the license file with the one up to date. You may opt to correct the address in the package.

Libs in standard directories need an ldconfig run. See http://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries

You'll need a devel sub-package: http://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages

Installing files to /usr/share/icons, you need this scriptlet: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache

Use the name macro instead of "mex".

Try to break the %configure line.

You can drop the defattr line. It's not necessary.

If you don't plan to go for EPEL5 or older, drop the clean section and the rm in the install section. If you want to go for EPEL5, define the buildroot.

Be more specific in your file section. For instance: all binaries start with %{name}.

Comment 3 Bastien Nocera 2011-09-12 10:36:46 UTC
(In reply to comment #2)
> rpmlint mex-0.1.4-1.fc17.x86_64.rpm 
> mex.x86_64: W: spelling-error Summary(en_US) centre -> center, cent re, cent-re
> mex.x86_64: W: spelling-error %description -l en_US centre -> center, cent re,
> cent-re

Fixed.

> mex.x86_64: E: binary-or-shlib-defines-rpath
> /usr/lib64/mex/plugins/40mex-recommended.so ['/usr/lib64']
> mex.x86_64: E: binary-or-shlib-defines-rpath
> /usr/lib64/mex/plugins/100mex-debug.so ['/usr/lib64']
> mex.x86_64: E: binary-or-shlib-defines-rpath
> /usr/lib64/mex/plugins/20mex-gnome-dvb.so ['/usr/lib64']
> mex.x86_64: E: binary-or-shlib-defines-rpath
> /usr/lib64/mex/plugins/40mex-queue.so ['/usr/lib64']
> mex.x86_64: E: binary-or-shlib-defines-rpath
> /usr/lib64/mex/plugins/00mex-search.so ['/usr/lib64']
> mex.x86_64: E: binary-or-shlib-defines-rpath
> /usr/lib64/mex/plugins/20mex-tracker.so ['/usr/lib64']
> mex.x86_64: E: binary-or-shlib-defines-rpath
> /usr/lib64/mex/plugins/20mex-upnp.so ['/usr/lib64']
> mex.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/mex-webremote
> ['/usr/lib64']
> mex.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/mex ['/usr/lib64']
> mex.x86_64: E: binary-or-shlib-defines-rpath
> /usr/lib64/mex/plugins/50mex-dbusinput.so ['/usr/lib64']
> mex.x86_64: E: incorrect-fsf-address /usr/share/doc/mex-0.1.4/COPYING
> mex.x86_64: E: postun-without-ldconfig /usr/lib64/libmex-0.2.so.0.0.0
> mex.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libmex-0.2.so
> mex.x86_64: W: no-manual-page-for-binary mex-webremote
> mex.x86_64: W: no-manual-page-for-binary mex-debug
> mex.x86_64: W: no-manual-page-for-binary mex
> 
> Get rid of the rpaths. See
> http://fedoraproject.org/wiki/Packaging:Guidelines#Removing_Rpath

Fixed.

> Please contact the developer, so he can update the license file with the one up
> to date. You may opt to correct the address in the package.

https://github.com/media-explorer/media-explorer/issues/140

> Libs in standard directories need an ldconfig run. See
> http://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries

Yep.

> You'll need a devel sub-package:
> http://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages

Nope. The files are only there for introspection purposes. There is no support for 3rd-party plugins.

> Installing files to /usr/share/icons, you need this scriptlet:
> http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache

Done.

> Use the name macro instead of "mex".

Done where it made sense.

> Try to break the %configure line.

Done.

> You can drop the defattr line. It's not necessary.

Done.

> If you don't plan to go for EPEL5 or older, drop the clean section and the rm
> in the install section. If you want to go for EPEL5, define the buildroot.

Done.

> Be more specific in your file section. For instance: all binaries start with
> %{name}.

Done.

Comment 5 Bastien Nocera 2011-09-12 10:53:46 UTC
Scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3344432

Comment 6 Volker Fröhlich 2011-09-12 14:22:50 UTC
From the build log:

configure: WARNING: unrecognized options: --disable-rpath

Do you intend to use the internal thumbnailer instead of tumbler?

 • Applets:
        Displayconf : no
        Rebinder    : no
        Networks    : no
        Webremote   : yes

Do you leave them out on purpose?

The makefile is not verbose.

mex.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libmex-0.2.so

Comment 7 Bastien Nocera 2011-09-17 22:51:41 UTC
(In reply to comment #6)
> From the build log:
> 
> configure: WARNING: unrecognized options: --disable-rpath

I'll fix that.

> Do you intend to use the internal thumbnailer instead of tumbler?

Yes, we don't want tumbler there.

>  • Applets:
>         Displayconf : no
>         Rebinder    : no
>         Networks    : no
>         Webremote   : yes
> 
> Do you leave them out on purpose?

Yes, they're needed if you intend mex to take over the whole desktop, such as on set-top boxes.

> The makefile is not verbose.

Don't understand that.

> mex.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libmex-0.2.so

Again, as mentioned above, it's needed for GObject introspection to work, not for development purposes.

Comment 8 Bastien Nocera 2011-09-17 22:55:19 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > From the build log:
> > 
> > configure: WARNING: unrecognized options: --disable-rpath
> 
> I'll fix that.

It's "fixed" by the autoreconf -f which pulled in a non-broken libtool (I couldn't get rpmlint to show me the same error you got).

Comment 10 Jason Tibbitts 2011-09-17 23:48:14 UTC
I just wanted to point out that /usr/bin/mex has for a very long time (i.e. decades) an executable associated with Matlab.  A web search also shows conflicts between Matlab's mex and something related to TeX.  Probably not a good idea for yet another project to try to use that executable as well.

Comment 11 Bastien Nocera 2011-09-22 12:58:17 UTC
(In reply to comment #10)
> I just wanted to point out that /usr/bin/mex has for a very long time (i.e.
> decades) an executable associated with Matlab.  A web search also shows
> conflicts between Matlab's mex and something related to TeX.  Probably not a
> good idea for yet another project to try to use that executable as well.

I would hope that the name clash with a proprietary piece of software doesn't block this review, but I've made the upstream aware of the problem in:
https://github.com/media-explorer/media-explorer/issues/147

Volker, do you want to finish the review?

Comment 13 Jason Tibbitts 2011-09-22 21:13:30 UTC
The mex executable in texlive is not, to my understanding, proprietary.  And in any case, stepping over something that has been around for a very long time, proprietary or not, seems problematic.  Arch, at least, appears to rename the executable from this package to mexp.

Comment 14 Bastien Nocera 2011-09-22 23:13:12 UTC
(In reply to comment #13)
> The mex executable in texlive is not, to my understanding, proprietary.

I missed that bit. It's been renamed now.

Comment 15 Peter Robinson 2011-09-24 08:04:38 UTC
I'll review this

Comment 16 Peter Robinson 2011-10-03 21:58:58 UTC
Bastien: Update the package to the new 0.2.0 and I'll get this done this week

Comment 17 Bastien Nocera 2011-10-06 16:37:55 UTC
Needs a new libmx, which is building now.

Comment 18 Bastien Nocera 2011-10-06 16:52:22 UTC
No scratch build as libmx 1.3.2 is just an update (candidate) right now.

http://people.fedoraproject.org/~hadess/mex/mex.spec
http://people.fedoraproject.org/~hadess/mex/mex-0.2.0-1.fc16.src.rpm

Comment 19 Peter Robinson 2011-10-09 15:25:28 UTC
(In reply to comment #18)
> No scratch build as libmx 1.3.2 is just an update (candidate) right now.

Should be test building in rawhide anyway, and pushing 1.3.2 to rawhide as well! :)

Comment 20 Peter Robinson 2011-10-09 15:34:45 UTC
A few bits need fixing

- rpmlint output

rpmlint mex-0.2.0-1.fc16.src.rpm mex-0.2.0-1.fc17.x86_64.rpm mex.spec
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/40mex-recommended.so ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/20mex-upnp.so ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/20mex-gnome-dvb.so ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/40mex-queue.so ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/00mex-search.so ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/libexec/mex-thumbnailer ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/20mex-tracker.so ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/mex-webremote ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/mex/plugins/50mex-dbusinput.so ['/usr/lib64']
mex.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/media-explorer ['/usr/lib64']
mex.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libmex-0.2.so
mex.x86_64: W: no-manual-page-for-binary mex-webremote
mex.x86_64: W: no-manual-page-for-binary media-explorer
2 packages and 1 specfiles checked; 10 errors, 3 warnings.

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
+ license matches the actual package license
+ latest version packaged

+ %doc includes license file
+ spec file written in American English
+ spec file is legible
+ upstream sources match sources in the srpm
  66eda375fdede3e0df652de7024aeff0  mex-0.2.0.tar.bz2
+ package successfully builds on at least one architecture
  tested using koji scratch build
+ BuildRequires list all build dependencies
+ %find_lang instead of %{_datadir}/locale/*
+ binary RPM with shared library files must call ldconfig in %post and %postun+ does not use Prefix: /usr
+ package owns all directories it creates
+ no duplicate files in %files
+ Package perserves timestamps on install
  Permissions on files must be set properly 
+ %defattr line
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package runtime 
n/a header files should be in -devel
n/a static libraries should be in -static
n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
- libfoo.so must go in -devel
n/a devel must require the fully versioned base
+ packages should not contain libtool .la files
+ packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ filenames must be valid UTF-8

Optional:

+ if there is no license file, packager should query upstream to include it
n/a translations of description and summary for non-English languages, if available
+ reviewer should build the package in mock/koji
n/a the package should build into binary RPMs on all supported architectures
n/a review should test the package functions as described
+ scriptlets should be sane
  non -devel packages should require fully versioned base
n/a pkgconfig files should go in -devel
+ shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or /usr/sbin
  Package should have man files

Comment 21 Bastien Nocera 2011-11-01 10:13:24 UTC
The application changed name to "media-explorer":

http://people.fedoraproject.org/~hadess/mex/media-explorer.spec
http://people.fedoraproject.org/~hadess/mex/media-explorer-0.3.2-1.fc16.src.rpm

I already explained why the .so file wasn't in a separate package, though I'll just remove it instead (along with the introspection support) if you feel it's actually a problem to not follow guidelines that are wrong :)

Scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3476342

I don't get any rpath problems with this build. Let me know if I'm missing something.

Comment 22 Peter Robinson 2011-11-01 13:28:02 UTC
The 0.3.2 release looks much better:

$rpmlint media-explorer*
media-explorer.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libmex-0.2.so
media-explorer.x86_64: W: no-manual-page-for-binary mex-webremote
media-explorer.x86_64: W: no-manual-page-for-binary media-explorer
media-explorer-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/media-explorer-0.3.2/applets/webremote/tracker-client.h
media-explorer-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/media-explorer-0.3.2/applets/webremote/tracker-client.c
3 packages and 0 specfiles checked; 2 errors, 3 warnings.

Rest of the changes look sane

APPROVED

Comment 23 Bastien Nocera 2011-11-02 11:23:33 UTC
New Package SCM Request
=======================
Package Name: media-explorer
Short Description: Media center application
Owners: hadess
Branches: f16
InitialCC:

Comment 24 Gwyn Ciesla 2011-11-02 12:51:44 UTC
Git done (by process-git-requests).

Comment 25 Fedora Update System 2011-11-02 22:10:58 UTC
media-explorer-0.3.2-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/media-explorer-0.3.2-1.fc16

Comment 26 Fedora Update System 2011-11-03 21:52:01 UTC
media-explorer-0.3.2-1.fc16 has been pushed to the Fedora 16 testing repository.

Comment 27 Fedora Update System 2011-11-14 22:28:29 UTC
media-explorer-0.3.2-1.fc16 has been pushed to the Fedora 16 stable repository.


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