Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 180164 - Review Request: muine
Summary: Review Request: muine
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jeremy Katz
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On: 181824 183694
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-02-06 15:14 UTC by Sindre Pedersen Bjørdal
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-03-14 22:26:51 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Sindre Pedersen Bjørdal 2006-02-06 15:14:31 UTC
Spec Name or Url: http://folk.ntnu.no/sindrb/packages/muine.spec
SRPM Name or Url: http://folk.ntnu.no/sindrb/packages/muine-0.8.4-1.src.rpm

Description: 

Muine is a new music player for GNOME using some new UI ideas. 
The idea is that it will be much easier and comfortable 
to use than the iTunes model, which is used by most
GNOME music players. It is written in C and C#, 
using GStreamer for music playback.

Comment 1 Jeremy Katz 2006-02-15 22:45:59 UTC
Hmm, at this point, gstreamer08 is no longer included in Core and hasn't yet
been imported into Extras.  Checking upstream, there's not yet a release that
builds against gstreamer-0.10, although the support is in SVN.

Other preliminary thoughts
* Why package the trayicon and dashboard support separately?  They're not going
to be big and people will probably expect the trayicon at least to just work if
they install the package
* Does it make sense to include the dashboard plugin when dashboard isn't
shipped anywhere yet?

Comment 2 Sindre Pedersen Bjørdal 2006-02-16 21:32:59 UTC
Guess the missing gstreamer08 means we're waiting for the package to arrive in
Extras, or for a new relase of muine that uses gstreamer 0.10, which ever comes
first. 

1. Not including the tray icon by default was a decision made upstream. My
opinion is that the tray icon is shouldn't be included in the Fedora package
when it's not included by default upstream, I can be convinced otherwise though. 

2. Removing the dashboard plugin for now is fine by me. 

Updates spec, without the dashboard stuff:
http://folk.ntnu.no/sindrb/packages/muine.spec

SRPM: 
http://folk.ntnu.no/sindrb/packages/muine-0.8.4-2.src.rpm



Comment 3 Brian Pepple 2006-02-16 22:03:22 UTC
I just submitted the gstreamer 0.8 package for review, since I also need it for
gnomebaker.  Any reviews would be welcome.

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

Comment 4 Jeremy Katz 2006-02-20 20:17:49 UTC
Even if upstream packages them separately, I think we're far better off
packaging them together.  From a user experience perspective, it's going to be
far better.

I'll try to do the more full review checklist later today.

Comment 5 Brian Pepple 2006-03-07 15:44:50 UTC
I can't sponser you, but here's a review of your package:

MD5Sums:
215dab4b1a0022deadb848b646640514  muine-0.8.4.tar.gz

Good:
* Upstream source tarball verified
* Package name conforms to the Fedora Naming Guidelines
* Buildroot has all required elements
* All paths begin with macros
* All directories are owned by this or other packages
* No deprecated fields used
* All necessary BuildRequires listed
* All desired features are enabled
* Make succeeds even when %{_smp_mflags} is defined

Bad:
* Drop duplicate BuildRequires: gstreamer08-devel (provided by
gstreamer08-plugins-devel)
* Didn't disable schema installation during build.  Refer to
http://fedoraproject.org/wiki/ScriptletSnippets#head-ff64cd482595764f672082d5a3b83e1fc22962e8
* Drop the 'killall -HUP gconfd-2 || :' from your scriptlets since it's no
longer needed.
* rpmlint errors that must be fixed:
  W: muine incoherent-version-in-changelog yet. 0.8.4-2
  W: muine devel-file-in-non-devel-package /usr/lib/pkgconfig/muine-plugin.pc
  W: muine devel-file-in-non-devel-package /usr/lib/muine/libmuine.a
  W: muine devel-file-in-non-devel-package /usr/lib/pkgconfig/muine-dbus.pc
  W: muine devel-file-in-non-devel-package /usr/lib/muine/libinotifyglue.a
* I agree with Jeremy in comment #4 that the trayicon should be included with
the package.
* Drop Requires on gstreamer08, devel pacakge soname pulls this in.
 

Comment 6 Sindre Pedersen Bjørdal 2006-03-07 17:26:11 UTC
TrayIcon included by default now
Fixed rpmlint issues by creating -devel package and fixing typo
Fixed the GConf-scriptlets
Removed duplicate Buildrequires
Added gettext dependency for %find_lang
Removed Requires on gstreamer08 and gstreamer08-plugins

Updated spec: http://folk.ntnu.no/sindrb/packages/muine.spec

SRPM: 
http://folk.ntnu.no/sindrb/packages/muine-0.8.4-3.src.rpm

Comment 7 Jeremy Katz 2006-03-13 19:30:02 UTC
Looking fairly good.  Tested on my x86_64 box, though, and it's breaking due to
things being in %{_libdir}/mono/gac instead of /usr/lib/mono/gac.  Talking with
some people who know more about mono than I do, it seems that the files should
always end up in /usr/lib right now (they're always 32-bit i386 PE executables).
 Probably post-FC5, we'll be working to move them into %{_datadir} or similar.

But if you fix that up, I'll sponsor you and approve the package 

Comment 8 Christopher Aillon 2006-03-13 20:33:44 UTC
Just a followup, make sure it is %{_prefix}/lib/mono/gac instead of simply
/usr/lib/mono/gac.

Comment 9 Sindre Pedersen Bjørdal 2006-03-13 23:01:36 UTC
Fixed the issues breaking x86_64

Updated spec: http://folk.ntnu.no/sindrb/packages/muine.spec

SRPM: 
http://folk.ntnu.no/sindrb/packages/muine-0.8.4-4.src.rpm

Comment 10 Jeremy Katz 2006-03-13 23:16:37 UTC
This just changes the spec file and doesn't actually fix where the package is
trying to install things.  It looks like patching configure along these lines
should work around the problem

[katzj@orodruin BUILD]$ diff -ur muine-0.8.4/configure muine/configure
--- muine-0.8.4/configure       2006-01-29 23:51:08.000000000 -0500
+++ muine/configure     2006-03-13 18:19:27.000000000 -0500
@@ -21724,7 +21724,7 @@
    { (exit 1); exit 1; }; }
 fi

-GACUTIL_FLAGS='/package muine /gacdir $(libdir) /root $(DESTDIR)$(libdir)'
+GACUTIL_FLAGS='/package muine /gacdir $(prefix)/lib /root $(DESTDIR)$(prefix)/lib'


Comment 11 Sindre Pedersen Bjørdal 2006-03-14 20:05:21 UTC
Applied patch

Updated spec: http://folk.ntnu.no/sindrb/packages/muine.spec

SRPM: 
http://folk.ntnu.no/sindrb/packages/muine-0.8.4-5.src.rpm

Comment 12 Jeremy Katz 2006-03-14 22:26:51 UTC
Tested on my workstation, looks good.  Remaining rpmlint stuff is caused by mono
more than anything.

Foolish -- I've gone ahead and moved this to FE-ACCEPT.  If you request
sponsorship for the cvsextras group, I'll go ahead and do so and then you should
be good to check in and build the package.  Congrats!


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