Bug 187351 - Review Request: bmpx - Media player with the WinAmp GUI
Review Request: bmpx - Media player with the WinAmp GUI
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Thorsten Leemhuis (ignored mailbox)
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2006-03-30 06:43 EST by Matthias Saou
Modified: 2007-11-30 17:11 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-11-13 09:42:22 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Matthias Saou 2006-03-30 06:43:49 EST
Spec Name or Url: http://svn.rpmforge.net/svn/trunk/rpms/bmpx/bmpx.spec
SRPM Name or Url: http://ftp.freshrpms.net/pub/freshrpms/fedora/linux/5/bmpx/
Description:
BMPx is an audio player that features support for specifications like XDS DnD,
XSPF and DBus. BMPx is highly interoperable and integrates well with other
applications and a variety of desktop environments.

Note: This is "yet another xmms evolution", but goes much further than bmp and audacious, as it uses gstreamer for its backend, making adding new supported formats very easy and removing the need to patch out mp3 support etc.
Comment 1 Eric Tanguy 2006-04-04 01:47:28 EDT
I'm trying to test it but i can' achieve to fill in the music library. I select
a directory containing ogg files (maybe all not tagged correctly) and try to add
these. The system seems to look at he files and the end add nothing.
Another thing, when you insert an audio cd and you try to read the files in, you
can't see the gnome-mounted cd.
Comment 2 Matthias Saou 2006-04-10 07:56:23 EDT
Yeah, there are definitely issues with file adding, like playlist icons not
working, while right click is, or errors and playback stop when .jpg files are
found...
The program is still maturing, working "enough" for many people, and could
possibly benefit from a wider testing if it was available in Extras, so this
should be a review about the package more than about the bugs remaining in the
software... which should go here : http://bugs.beep-media-player.org/
Comment 3 Eric Tanguy 2006-04-10 16:18:12 EDT
Review for release 1.fc5:
* RPM name is OK
* Source bmpx-0.14.3.tar.bz2 is the same as upstream
* Builds fine in mock
* rpmlint of bmpx-devel looks OK
* rpmlint of bmpx looks OK
* File list of bmpx-devel looks OK
* INSERT RESULT OF RUN TEST

Needs work:
* BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
  (wiki: PackagingGuidelines#BuildRoot)
* Spec file: some paths are not replaced with RPM macros
  (wiki: QAChecklist item 7)
* The BuildRoot must be cleaned at the beginning of %install
* BuildRequires: gettext is missing (required by the %find_lang macro)
* The package should contain the text of the license
  (wiki: Packaging/ReviewGuidelines)
* Desktop file: vendor should be fedora
  (wiki: PackagingGuidelines#desktop)
* Desktop file: the Categories tag should contain X-Fedora
  (wiki: PackagingGuidelines#desktop)
* Scriptlets: missing update-desktop-database
  (wiki: ScriptletSnippets)

Minor:
* Duplicate BuildRequires: dbus-devel (by hal-devel), gtk2-devel (by
libglade2-devel)
Comment 4 Matthias Saou 2006-04-11 05:17:52 EDT
About the needs work :
- That build root is plain silly with the chroot builds we have nowadays, and is
the "preferred" according to the wiki. It makes mach builds fail, so it's not
the one I use.
- Please detail what paths aren't replaced, as I can't identify any at a first
glance.
- %{__rm} -rf %{buildroot} is already present right after %install...
- If you look closer, gettext-devel is build required already...
- COPYING added... it was missing from the tarballs some time ago IIRC.
- I understand the guidelines as "if there is no desktop files, then include one
and install it this way", but since the included desktop file is fine IMHO, I
didn't touch it. If my interpretation is wrong, alright, but the Wiki section
will need some more details.
- update-desktop-database calls will be added, good catch!

Updated package 0.14.3-2 available now.
Comment 5 Aurelien Bompard 2006-04-11 08:05:49 EDT
Yeah, my fedora-qa script tends to give a lot of false positives, so it should
be double-checked by human eye. It's much worse that rpmlint in this regard.
I believe it can be useful though.

I've fixed the problem detecting gettext-devel. The "absolute path" check greps
for /usr and /var in the spec file, but I can't find that in the spec file from
svn. You'll find the updated script at the usual place.
Comment 6 Eric Tanguy 2006-04-11 09:10:25 EDT
Yes you are right. I had to put my human eye but i had not enough time so i
apologize. I will look more carefully to this next time.
(In reply to comment #4)
> About the needs work :
> - That build root is plain silly with the chroot builds we have nowadays, and is
> the "preferred" according to the wiki. It makes mach builds fail, so it's not
> the one I use.

Ok

> - Please detail what paths aren't replaced, as I can't identify any at a first
> glance.

You had already modified their : these was the lines about gtk-update-icon-cache

> - %{__rm} -rf %{buildroot} is already present right after %install...

you are right.

> - If you look closer, gettext-devel is build required already...

you are right also

> - COPYING added... it was missing from the tarballs some time ago IIRC.

Ok

> - I understand the guidelines as "if there is no desktop files, then include one
> and install it this way", but since the included desktop file is fine IMHO, I
> didn't touch it. If my interpretation is wrong, alright, but the Wiki section
> will need some more details.

I agree that the wiki section is not very clear about what to if a desktop file
is already included in the source file but when i had this question i modified
myself the desktop file to be according to the wiki. Maybe someone else could
give more details about this or i will ask the question to extras list to make
clarification in this wiki section

> - update-desktop-database calls will be added, good catch!
> 
> Updated package 0.14.3-2 available now.

I will see the new version when i will be bak home
Comment 7 Ralf Corsepius 2006-04-11 09:46:55 EDT
(In reply to comment #4)
> About the needs work :
> - That build root is plain silly with the chroot builds we have nowadays,
What? The purpose is to protect users rebuilding the package. chroot's are
completely irrelevant wrt. this.

> It makes mach builds fail, so it's not the one I use.
Irrelevant for FE. FE's buildsystem and normal user environments count.
What you say only meany a bug in mach.
Comment 8 Matthias Saou 2006-04-11 09:55:40 EDT
So let's just say that this (silly IMHO) %(id etc.) suffix to the build root is
the "preferred" according to the wiki, meaning it is encouraged to use it, but
not mandatory. I don't like it, so I don't use it.
BTW, it's totally useless when building with mock, which is the Extras way of
building packages :-)
Comment 9 Eric Tanguy 2006-04-12 01:49:01 EDT
Review for release 2:
* RPM name is OK
* Source bmpx-0.14.3.tar.bz2 is the same as upstream
* Builds fine in mock
* rpmlint of bmpx-devel looks OK
* rpmlint of bmpx looks OK
* File list of bmpx-devel looks OK
* File list of bmpx looks OK

Needs work:
* BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
  (wiki: PackagingGuidelines#BuildRoot)
* Desktop file: vendor should be fedora
  (wiki: PackagingGuidelines#desktop)
* Desktop file: the Categories tag should contain X-Fedora
  (wiki: PackagingGuidelines#desktop)

Minor:
* Duplicate BuildRequires: dbus-devel (by hal-devel), gtk2-devel (by
libglade2-devel)
* Maybe you could add %{?dist} in the release. It could be usefull for next fedora
version (ie fc6)
Comment 10 Michael Schwendt 2006-04-12 09:21:14 EDT
$ rpmqfcheck.pl /home/qa/tmp/rpm/RPMS/bmpx-0.14.3-1.fc5.i386.rpm
Orphaned dir: /usr/share/dbus-1
Orphaned dir: /usr/share/locale/af
Orphaned dir: /usr/share/locale/af/LC_MESSAGES
Orphaned dir: /usr/share/locale/et
Comment 11 Matthias Saou 2006-05-03 16:01:52 EDT
> $ rpmqfcheck.pl /home/qa/tmp/rpm/RPMS/bmpx-0.14.3-1.fc5.i386.rpm
> Orphaned dir: /usr/share/dbus-1
> Orphaned dir: /usr/share/locale/af
> Orphaned dir: /usr/share/locale/af/LC_MESSAGES
> Orphaned dir: /usr/share/locale/et

Since /usr/share/dbus-1/services/ is owned by dbus, I think you've found a bug
in dbus, which should own the /usr/share/dbus-1/ directory.
The af and et directories should be owned by glibc-common if I'm not mistaken,
as many other translations are already there too.

Did you file these bugs already, or do you want me to do it?
Comment 12 Michael Schwendt 2006-05-04 04:33:47 EDT
I haven't filed them. Btw, glibc-common does not include plain "af"
and "et" locales at all.
Comment 13 Matthias Saou 2006-05-04 05:01:40 EDT
Looking further at all the other directories under /usr/share/locale/, I'm
really not sure what the consensus is, as MANY aren't owned by any package.
Probably a subject for the packaging-list before any bug gets filed.
Comment 14 Ville Skyttä 2006-05-04 11:49:18 EDT
IMO %find_lang or the glibc-common are things that should be looked into,
they're things that could provide a centralized fix for unowned locale dirs.
Comment 15 Matthias Saou 2006-07-11 12:32:24 EDT
I'm not sure what to do here... bmpx has evolved quite fast. It's still somewhat
buggy from my experience, and the current 0.20.2 release optionally uses faad2
(MP4/AAC decoder...) to read some tags, so it cannot be shipped full-featured in
Extras.

This does seem wrong though, since using gstreamer for all the tags would be the
proper thing to do... *sigh*

It also has a dependency on a version of libnotify that isn't even in FC
development yet (see bug #191731).
Comment 16 Ville Skyttä 2006-07-11 12:51:55 EDT
Does it really require faad2 for reading the tags, or would libmp4v2 (bug
191036) be enough?
Comment 17 Matthias Saou 2006-07-11 12:56:52 EDT
Thanks for the pointer. Indeed, only libmp4v2 should be enough. Still the
libnotify requirement left... then wait for legal... then continue with the
submission. Might be a while...
Comment 18 Matthias Saou 2006-11-13 09:42:22 EST
Well, I've clearly lost interest in maintaing bmpx for Extras : My perception is
that upstream doesn't really know what they want (they went for an xmms fork to
a Rhythmbox-like player written in C++...), and seem more interested in working
on beta releases with plenty of new features than releasing stable and solid
versions of their player.

I'll withdraw this submission and welcome anyone to re-submit bmpx if they're
interested in maintaining it.
Comment 19 Michel Alexandre Salim 2006-11-13 17:16:58 EST
That's a shame. bmpx has the nicest Last.fm client for Linux I've seen so far.
Are you still going to maintain it in your own repository?

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