Bug 607778 - Review Request: xnoise-plugins-core - Core plugins for xnoise
Review Request: xnoise-plugins-core - Core plugins for xnoise
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michael Schwendt
Fedora Extras Quality Assurance
:
Depends On: 607777
Blocks:
  Show dependency treegraph
 
Reported: 2010-06-24 15:04 EDT by Michel Alexandre Salim
Modified: 2010-07-16 14:49 EDT (History)
3 users (show)

See Also:
Fixed In Version: xnoise-plugins-core-0.1.6-2.fc13
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-07-16 14:49:09 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
bugs.michael: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Michel Alexandre Salim 2010-06-24 15:04:19 EDT
Spec URL: http://salimma.fedorapeople.org/specs/gnome/xnoise-plugins-core.spec
SRPM URL: http://salimma.fedorapeople.org/specs/gnome/xnoise-plugins-core-0.1.6-1.fc13.src.rpm
Description:
This package contains the core plugins for xnoise. Their usage with
xnoise is optional.
Comment 1 Michael Schwendt 2010-06-25 06:50:58 EDT
"sqlite3-devel" isn't found here. Will see how far it goes with "sqlite-devel".
Comment 2 Michel Alexandre Salim 2010-06-25 07:43:31 EDT
Yes, that should be sqlite-devel. I added the sqlite dependency after the last local SRPM rebuild, and ought to have tested the final SRPM too. Apologies.
Comment 3 Michael Schwendt 2010-06-26 16:17:05 EDT
$ sha1sum xnoise-plugins-core-0.1.6.tar.gz 
8eb208d9d3cc263e36904216d17520bc00b89a67  xnoise-plugins-core-0.1.6.tar.gz


* "BuildRequires: intltool" is missing


* MediaWatcher plugin crashes:
  1) start xnoise
  2) open Preferences > Plugins
  3) enable MediaWatcher
  4) disable MediaWatcher
  5) enable MediaWatcher
  6) GLib-GObject warnings on stderr + segfault


* LastfmCovers plugin crashes on exit:
  1) start xnoise
  2) enable LastfmCovers plugin
  3) quit
  4) segfault


* Notifications plugin prints "<b> ... </b>"


* /usr/lib64/xnoise/*.la files are not needed


* xnoise-plugins-core.x86_64: E: incorrect-locale-subdir /usr/share/locale/default/LC_MESSAGES/xnoise-plugins-core.mo


* %{_libdir}/xnoise ownership : currently owned by the -plugins-core package, which is optional, however. Would separate xnoise plugin packages need to own %{_libdir}/xnoise  or do you think it would be better to make the base "xnoise" package own that directory?


* xnoise-plugins-core-devel : What's the purpose of this package? The current %description can't be correct. ;-)


* Rest is fine.
Comment 4 Michel Alexandre Salim 2010-07-03 09:52:57 EDT
(In reply to comment #3)

Updated SRPM:
http://salimma.fedorapeople.org/specs/gnome/xnoise-plugins-core-0.1.6-2.fc13.src.rpm

> * "BuildRequires: intltool" is missing
Fixed
> 
> 
> * MediaWatcher plugin crashes:
>   1) start xnoise
>   2) open Preferences > Plugins
>   3) enable MediaWatcher
>   4) disable MediaWatcher
>   5) enable MediaWatcher
>   6) GLib-GObject warnings on stderr + segfault
Confirmed; reporting upstream and disabling for now
> 
> 
> * LastfmCovers plugin crashes on exit:
>   1) start xnoise
>   2) enable LastfmCovers plugin
>   3) quit
>   4) segfault
Confirmed. Looks like something broke between the last development version I tried before 0.1.6 (0.1.2 or so), and I did not notice because my albums already have their cover images cached

Disabling for now -- is this the error you get?
Failed to open file '/home/michel/.config/xnoise/album_images/kate_walsh/tim's_house/tim's_house_medium': No such file or directory
Failed to open file '/home/michel/.config/xnoise/album_images/kate_walsh/tim's_house/tim's_house_medium': No such file or directory
*** glibc detected *** xnoise: malloc(): smallbin double linked list corrupted: 0x0000000001006cf0 ***


> * Notifications plugin prints "<b> ... </b>"
Fixed; patch sent upstream

> * /usr/lib64/xnoise/*.la files are not needed
Done
> 
> 
> * xnoise-plugins-core.x86_64: E: incorrect-locale-subdir
> /usr/share/locale/default/LC_MESSAGES/xnoise-plugins-core.mo
> 
> 
> * %{_libdir}/xnoise ownership : currently owned by the -plugins-core package,
> which is optional, however. Would separate xnoise plugin packages need to own
> %{_libdir}/xnoise  or do you think it would be better to make the base "xnoise"
> package own that directory?
That is true. I'll make xnoise owns it, but since xnoise is already in the override tag, the switch could probably happen after the initial xnoise/xnoise-plugins-core combo is released.

> * xnoise-plugins-core-devel : What's the purpose of this package? The current
> %description can't be correct. ;-)
Actually .. I'm not sure. the .pc file does not actually specify where plugins should be installed, but it does at least specify how to find the xnoise package. I have adjust the description.
Comment 5 Michael Schwendt 2010-07-04 16:59:10 EDT
Okay. The segfaults in the plugins will be left to you anyway. There are more, e.g. when starting from scratch and enabling "Leoslyrics". ABRT doesn't catch them:

$ rm -rf ~/.config/xnoise
$ xnoise
        LastfmCovers plugin failed to activate!
        Mediawatcher plugin failed to activate!
*** glibc detected *** xnoise: malloc(): smallbin double linked list corrupted: 0x00000000015f5f50 ***

$ rm -rf ~/.config/xnoise
$ xnoise
        LastfmCovers plugin failed to activate!
        Mediawatcher plugin failed to activate!
closing...
Segmentation fault (core dumped)


JFTR, the review guidelines say:

| SHOULD: The reviewer should test that the package functions as
| described. A package should not segfault instead of running, for example.

We could go on with even more disabled plugins. Eventually, no plugin may be left. ;) Or old/new segfaults might return with one of the next updates after package approval.

Whether 0.1.6 is ready for Rawhide, I leave this decision to you. 

[...]

About the .pc file: Consider merging this dubious -devel subpackage into the xnoise-plugins-core package. Possibly with a virtual -devel pkg or only when you learn about something that would need such a BuildRequires. Rationale: xnoise-plugins-core builds the plugins with the .pc file in xnoise-devel, so external plugins will likely use that .pc file, too.

[...]

Other issues have been dealt with. As the xnoise-plugins-core-devel pkg may return after the review process or be killed again in future updates, it isn't necessary to insist on its removal during this review.

APPROVED
Comment 6 Michel Alexandre Salim 2010-07-04 17:51:30 EDT
New Package CVS Request
=======================
Package Name: xnoise-plugins-core
Short Description: Core plugins for xnoise
Owners: salimma
Branches: F-12 F-13
InitialCC: 

(In reply to comment #5)
> JFTR, the review guidelines say:
> 
> | SHOULD: The reviewer should test that the package functions as
> | described. A package should not segfault instead of running, for example.
> 
Apologies there. The plugins were in working order at some point before 0.1.6 came out, but I still should have not assumed that's still the case with the released versions. I guess xnoise is still in flux and the plugins lag a bit behind xnoise itself.

> We could go on with even more disabled plugins. Eventually, no plugin may be
> left. ;) Or old/new segfaults might return with one of the next updates after
> package approval.
> 
> Whether 0.1.6 is ready for Rawhide, I leave this decision to you. 
It's ready for Rawhide, I guess, but I'd probably keep it there and not push it to F-12 and F-13 until they are more stable.

> 
> [...]
> 
> About the .pc file: Consider merging this dubious -devel subpackage into the
> xnoise-plugins-core package. Possibly with a virtual -devel pkg or only when
> you learn about something that would need such a BuildRequires. Rationale:
Good idea; will delete it for now
Comment 7 Kevin Fenzi 2010-07-04 21:44:31 EDT
CVS done (by process-cvs-requests.py).
Comment 8 Fedora Update System 2010-07-08 07:47:53 EDT
xnoise-plugins-core-0.1.6-2.fc13,xnoise-0.1.6-2.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/xnoise-plugins-core-0.1.6-2.fc13,xnoise-0.1.6-2.fc13
Comment 9 Fedora Update System 2010-07-08 14:29:49 EDT
xnoise-plugins-core-0.1.6-2.fc13, xnoise-0.1.6-2.fc13 has been pushed to the Fedora 13 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update xnoise-plugins-core xnoise'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/xnoise-plugins-core-0.1.6-2.fc13,xnoise-0.1.6-2.fc13
Comment 10 Fedora Update System 2010-07-16 14:48:59 EDT
xnoise-plugins-core-0.1.6-2.fc13, xnoise-0.1.6-2.fc13 has been pushed to the Fedora 13 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.