Bug 1222484 - Review Request: rhythmbox-ampache - Ampache plugin for Rhythmbox
Summary: Review Request: rhythmbox-ampache - Ampache plugin for Rhythmbox
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Pete Walter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-05-18 10:54 UTC by Juan Orti Alcaine
Modified: 2016-06-09 18:16 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2016-06-09 18:16:00 UTC
Type: ---
Embargoed:
walter.pete: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Juan Orti Alcaine 2015-05-18 10:54:38 UTC
Spec URL: https://jorti.fedorapeople.org/rhythmbox-ampache/rhythmbox-ampache.spec
SRPM URL: https://jorti.fedorapeople.org/rhythmbox-ampache/rhythmbox-ampache-0-0.1.20150518git7c1d978.fc22.src.rpm
Description: The Rhythmbox Ampache Plugin is a plugin for the music player Rhythmbox that enables browsing the metadata and streaming music from an Ampache media server.
Fedora Account System Username: jorti

Comment 2 Pete Walter 2015-09-12 00:05:53 UTC
Looks good to me, except for two things:

1) how did you determine it is GPLv2-only and not GPLv2+ or even GPL+? I can't find any notes in the source code and it includes the standard GPL license text file which can mean anything.

https://fedoraproject.org/wiki/Licensing:FAQ#How_do_I_figure_out_what_version_of_the_GPL.2FLGPL_my_package_is_under.3F

2) you install files to %{_libdir}/rhythmbox/plugins/ampache, yet the package is marked as noarch. noarch packages cannot include files under _libdir, because those are architecture dependent.

Comment 3 Juan Orti Alcaine 2015-09-14 08:11:35 UTC
(In reply to Pete Walter from comment #2)
> Looks good to me, except for two things:
> 
> 1) how did you determine it is GPLv2-only and not GPLv2+ or even GPL+? I
> can't find any notes in the source code and it includes the standard GPL
> license text file which can mean anything.
> 
> https://fedoraproject.org/wiki/Licensing:
> FAQ#How_do_I_figure_out_what_version_of_the_GPL.2FLGPL_my_package_is_under.3F

It includes the license of GPLv2, so I assumed that. I have opened this bug: https://github.com/lotan/rhythmbox-ampache/issues/4

> 
> 2) you install files to %{_libdir}/rhythmbox/plugins/ampache, yet the
> package is marked as noarch. noarch packages cannot include files under
> _libdir, because those are architecture dependent.

rhythmbox loads its plugins from there. The main package also places there other python plugins like soundcloud, sendto, replaygain, rbzeitgeist, rb, python-console.

On the other hand, I've updated the package to use python3:
Spec URL: https://jorti.fedorapeople.org/rhythmbox-ampache/rhythmbox-ampache.spec
SRPM URL: https://jorti.fedorapeople.org/rhythmbox-ampache/rhythmbox-ampache-0-0.3.20150518git7415a69.fc22.src.rpm

Comment 4 Pete Walter 2015-09-14 10:22:47 UTC
(In reply to Juan Orti from comment #3)
> (In reply to Pete Walter from comment #2)
> > 2) you install files to %{_libdir}/rhythmbox/plugins/ampache, yet the
> > package is marked as noarch. noarch packages cannot include files under
> > _libdir, because those are architecture dependent.
> 
> rhythmbox loads its plugins from there. The main package also places there
> other python plugins like soundcloud, sendto, replaygain, rbzeitgeist, rb,
> python-console.

Right, the issue is that on x86_64, _libdir expands to /usr/lib64, but on i686, it expands to /usr/lib. If you mark the package as noarch it needs to work on all architectures. In this case, it can't work because of the lib64 vs lib mismatch.

Comment 5 Juan Orti Alcaine 2015-09-14 10:45:26 UTC
(In reply to Pete Walter from comment #4)
> (In reply to Juan Orti from comment #3)
> > (In reply to Pete Walter from comment #2)
> > > 2) you install files to %{_libdir}/rhythmbox/plugins/ampache, yet the
> > > package is marked as noarch. noarch packages cannot include files under
> > > _libdir, because those are architecture dependent.
> > 
> > rhythmbox loads its plugins from there. The main package also places there
> > other python plugins like soundcloud, sendto, replaygain, rbzeitgeist, rb,
> > python-console.
> 
> Right, the issue is that on x86_64, _libdir expands to /usr/lib64, but on
> i686, it expands to /usr/lib. If you mark the package as noarch it needs to
> work on all architectures. In this case, it can't work because of the lib64
> vs lib mismatch.

I don't see the problem.

On i686, rhythmbox will use the folder /usr/lib/rhythmbox/plugins, so this plugin must be installed there.
On x86_64 it will use /usr/lib64/rhythmbox/plugins, so everything works, doesn't it?

Comment 6 Pete Walter 2015-09-14 11:14:54 UTC
No, it expands _libdir during build time and depending on which builder it happened to be built on, the final rpm is going to have either /usr/lib/rhythmbox/plugins or /usr/lib64/rhythmbox/plugins hardcoded in it.

If you have a noarch package that installs the plugin to /usr/lib/rhythmbox/plugins but you install it on x86_64 where rhythmbox only searches /usr/lib64/rhythmbox/plugins, it just won't load.

Comment 7 Juan Orti Alcaine 2015-09-14 12:06:56 UTC
(In reply to Pete Walter from comment #6)
> No, it expands _libdir during build time and depending on which builder it
> happened to be built on, the final rpm is going to have either
> /usr/lib/rhythmbox/plugins or /usr/lib64/rhythmbox/plugins hardcoded in it.
> 
> If you have a noarch package that installs the plugin to
> /usr/lib/rhythmbox/plugins but you install it on x86_64 where rhythmbox only
> searches /usr/lib64/rhythmbox/plugins, it just won't load.

Thanks for the explanation, now I got it.
So, I guess I should make the package arched to avoid this problem, right?

Comment 8 Pete Walter 2015-09-14 12:18:39 UTC
Yes, I would say so.

Comment 9 Juan Orti Alcaine 2015-09-14 20:32:17 UTC
Ok, here is the arched package. I have also used the new Python macros:

Spec URL: https://jorti.fedorapeople.org/rhythmbox-ampache/rhythmbox-ampache.spec
SRPM URL: https://jorti.fedorapeople.org/rhythmbox-ampache/rhythmbox-ampache-0-0.4.20150518git7415a69.fc22.src.rpm

Comment 10 Pete Walter 2015-09-14 21:26:37 UTC
It fails to build:

error: Empty %files file /builddir/build/BUILD/rhythmbox-ampache-7415a69d019da8cf973cfb9e1727156ad901e7c1/debugfiles.list

Probably needs something like '%global debug_package %{nil}' to turn off debuginfo generation now that it is no longer noarch?

http://koji.fedoraproject.org/koji/taskinfo?taskID=11085092

Comment 11 Pete Walter 2015-09-14 21:39:43 UTC
Anyway, you can add '%global debug_package %{nil}' to the top of the file to fix it up to properly build before importing.

rpmlint output:

rhythmbox-ampache.x86_64: W: spelling-error %description -l en_US metadata -> meta data, meta-data, metatarsal
rhythmbox-ampache.x86_64: E: no-binary
rhythmbox-ampache.src: W: spelling-error %description -l en_US metadata -> meta data, meta-data, metatarsal
2 packages and 0 specfiles checked; 1 errors, 2 warnings.

fedora-review has nothing interesting to say and I won't post its output here.

Everything seems fine to me, so the package is:

APPROVED

Comment 13 Juan Orti Alcaine 2015-09-15 05:39:43 UTC
New Package SCM Request
=======================
Package Name: rhythmbox-ampache
Short Description: Ampache plugin for Rhythmbox
Upstream URL: https://github.com/lotan/rhythmbox-ampache
Owners: jorti
Branches: f21 f22 f23
InitialCC:

Comment 14 Gwyn Ciesla 2015-09-15 13:22:41 UTC
Git done (by process-git-requests).

Comment 15 Fedora Update System 2015-09-15 15:20:41 UTC
rhythmbox-ampache-0-0.5.20150518git7415a69.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-15912

Comment 16 Fedora Update System 2015-09-15 15:47:07 UTC
rhythmbox-ampache-0-0.5.20150518git7415a69.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-15919

Comment 17 Fedora Update System 2015-09-15 22:56:11 UTC
rhythmbox-ampache-0-0.5.20150518git7415a69.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.\nIf you want to test the update, you can install it with \n su -c 'yum --enablerepo=updates-testing update rhythmbox-ampache'. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-15919

Comment 18 Fedora Update System 2015-09-16 04:52:59 UTC
rhythmbox-ampache-0-0.5.20150518git7415a69.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.\nIf you want to test the update, you can install it with \n su -c 'yum --enablerepo=updates-testing update rhythmbox-ampache'. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-15912

Comment 19 Fedora Update System 2015-09-24 05:12:23 UTC
rhythmbox-ampache-0-0.5.20150518git7415a69.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

Comment 20 Fedora Update System 2015-09-24 08:24:48 UTC
rhythmbox-ampache-0-0.5.20150518git7415a69.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.


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