Bug 530021 - Review Request: moovida - Media Center
Review Request: moovida - Media Center
Status: CLOSED DUPLICATE of bug 554243
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-10-21 03:59 EDT by Alex Lancaster
Modified: 2010-01-12 01:56 EST (History)
12 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 233598
Environment:
Last Closed: 2010-01-12 01:48:47 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 Alex Lancaster 2009-10-21 03:59:01 EDT
Spec URL: http://people.fluendo.com/matthias/moovida/fedora/11/i386/noarch/moovida-1.0.6-1.fc11/moovida.spec
SRPM URL: http://people.fluendo.com/matthias/moovida/fedora/11/i386/noarch/moovida-1.0.6-1.fc11/moovida-1.0.6-1.fc11.src.rpm
Description: Media center solution using the GStreamer multimedia framework.

This is a rename of elisa to moovida (see bug #510965)
Comment 2 Rudolf Kastl 2009-10-21 09:13:10 EDT
after trying it out i figured the following general issues:

1. right after starting moovida it updates itsself to 1.07
2. there seem to be unmet dependencies according to the output before actually updating the main program or its plugins:

moovida 
Launcher core version: 1.0.6
/usr/lib64/python2.6/site-packages/twisted/internet/_sslverify.py:5: DeprecationWarning: the md5 module is deprecated; use hashlib instead
  import itertools, md5
Current core version: 1.0.6
WARN  MainThread      plugin_registry             Okt 21 13:11:26  plugin elisa-plugin-coherence has the following unmet dependencies: Twisted>=2.5.0 (elisa/core/plugin_registry.py:362)
WARN  MainThread      plugin_registry             Okt 21 13:11:26  plugin luci has the following unmet dependencies: Catwalk>=2.0.2 (elisa/core/plugin_registry.py:362)
WARN  MainThread      plugin_registry             Okt 21 13:11:26  plugin Coherence has the following unmet dependencies: Twisted>=2.5.0 (elisa/core/plugin_registry.py:362)
Comment 3 Felix Kaechele 2009-10-25 08:52:57 EDT
Well for me this violates
MUST: The spec file for the package MUST be legible.

I don't find that the spec file is easily legible.
Maybe you could clean it up a bit with indentation etc.
Comment 4 Christoph Wickert 2009-10-25 09:18:39 EDT
What's the use of splitting this into two packages? Only for building the plugins or can the moovida-base package actually be used without movida?
Even without splitting the package I cannot see the dependency loop: The plugins BuildRequire the movida package, but at runtime movida requires (at least) some plugins. So where is the loop? Note the difference between BuildRequires and Requires here.

%description is a little short and not really useful. Users don't care if this is based on gstreamer or something else, but they want to know what the package actually does.

I doubt moovida-base needs "Requires: python-setuptools".

The package installs a dbus service so I guess it needs to require dbus-x11 to provide dbus-launch.

You are not validating the moovida.desktop
Comment 5 Alex Lancaster 2009-10-25 15:24:08 EDT
(In reply to comment #4)
> What's the use of splitting this into two packages? Only for building the
> plugins or can the moovida-base package actually be used without movida?
> Even without splitting the package I cannot see the dependency loop: The
> plugins BuildRequire the movida package, but at runtime movida requires (at
> least) some plugins. So where is the loop? Note the difference between
> BuildRequires and Requires here.

I'm not the maintainer (I just posted the review to start the rename reviewing going) so I don't know exactly why this is necessary (hopefully Matthias will chime in soon), but I will point out that this was the way that it was done when the package was called elisa.  I believe this split also mirrors the way gstreamer is split: gstreamer, gstreamer-plugins-good, -plugins-bad etc.
Comment 6 Alex Lancaster 2009-10-25 15:29:56 EDT
Oh, I see what you mean, you are talking about moovida and moovida-base within the moovida.spec file?
Comment 7 Alex Lancaster 2009-10-25 15:33:51 EDT
(In reply to comment #4)
> What's the use of splitting this into two packages? Only for building the
> plugins or can the moovida-base package actually be used without movida?
> Even without splitting the package I cannot see the dependency loop: The
> plugins BuildRequire the movida package, but at runtime movida requires (at
> least) some plugins. So where is the loop? Note the difference between
> BuildRequires and Requires here.

I think the problem is that if the plugins were to just BuildRequire: moovida (without base), then when that BR (moovida) is installed, it would also attempt to install the plugins which haven't yet been built.  That's the loop.
Comment 8 Alex Lancaster 2009-10-25 15:37:47 EDT
By the way, the original elisa review request is bug #233598.
Comment 9 Christoph Wickert 2009-10-25 15:41:53 EDT
(In reply to comment #7)
> I think the problem is that if the plugins were to just BuildRequire: moovida
> (without base), then when that BR (moovida) is installed, it would also attempt
> to install the plugins which haven't yet been built.  That's the loop.  

But splitting wont help us here, since movida and movida-base are build in a singe build. This is no dependency loop but a chicken and egg problem.
Comment 10 Alex Lancaster 2009-10-25 15:43:46 EDT
(In reply to comment #9)
> (In reply to comment #7)
> > I think the problem is that if the plugins were to just BuildRequire: moovida
> > (without base), then when that BR (moovida) is installed, it would also attempt
> > to install the plugins which haven't yet been built.  That's the loop.  
> 
> But splitting wont help us here, since movida and movida-base are build in a
> singe build. This is no dependency loop but a chicken and egg problem.  

The problem isn't for moovida, but for the noovida-plugins-* packages.  They need to only be able to install the moovida-base package.
Comment 11 Alex Lancaster 2009-11-12 18:03:08 EST
Graeme Gillies will be taking over this review as per bug #510965 comment #13.  I will step up and take the review (taking bug, but not marking review flag until package submitted).
Comment 12 Graeme Gillies 2009-11-15 21:14:01 EST
Hi,

I have had a look at the moovida RPMS supplied by Matthias and the comments on this issue and have refactored the spec files and SRPMS for moovida, as well as updating them to the latest release (1.0.8) and they are available for review at

moovida.spec:
http://sites.google.com/site/ggilliesrpms/home/moovida.spec?attredirects=0&d=1

moovida-1.0.8-1.fc11.src.rpm:
http://sites.google.com/site/ggilliesrpms/home/moovida-1.0.8-1.fc11.src.rpm?attredirects=0&d=1

moovida-plugins-good.spec:
http://sites.google.com/site/ggilliesrpms/home/moovida-plugins-good.spec?attredirects=0&d=1

moovida-plugins-good-1.0.8-1.fc11.src.rpm:
http://sites.google.com/site/ggilliesrpms/home/moovida-plugins-good-1.0.8-1.fc11.src.rpm?attredirects=0&d=1

moovida-plugins-bad.spec:
http://sites.google.com/site/ggilliesrpms/home/moovida-plugins-bad.spec?attredirects=0&d=1

moovida-plugins-bad-1.0.8-1.fc11.src.rpm:
http://sites.google.com/site/ggilliesrpms/home/moovida-plugins-bad-1.0.8-1.fc11.src.rpm?attredirects=0&d=1

Sorry about the hosting it was all I had :)
Comment 13 Felix Kaechele 2009-11-16 02:55:03 EST
If you have a FAS account (which I assume you have because you are submitting a package for Fedora) you get some webspace with it. See http://fedoraproject.org/wiki/Infrastructure/fedorapeople.org

So no need for Google sites ;)
Comment 14 Alex Lancaster 2009-11-17 00:40:42 EST
Taking review.
Comment 15 Alex Lancaster 2009-11-17 00:51:12 EST
As a @redhat.com person have you have already been sponsored?    I can't find any packages you own on PackageDB:

https://admin.fedoraproject.org/pkgdb/users/packages/ggillies
Comment 16 Graeme Gillies 2009-11-17 00:55:47 EST
Sorry no I totally forgot to mention that this is my first package submission and I have not been sponsored yet. Sorry for the confusion.
Comment 17 Alex Lancaster 2009-11-17 01:40:09 EST
(In reply to comment #16)
> Sorry no I totally forgot to mention that this is my first package submission
> and I have not been sponsored yet. Sorry for the confusion.  

Ah, so then unfortunately your first review needs to be done by a sponsor, which I am not, see:

https://fedoraproject.org/wiki/Package_Review_Process#Reviewer

I can do an informal review, but can't do the official one. :(    So I'm unassigning this and setting the blocker bug FE-NEEDSPONSOR. Perhaps you can pester some redhat colleagues who are also sponsors.  Meanwhile I will make some quick remarks here and on a followup comment:

Summmary: should not include the package name and should use upstream (US) spelling as described on http://www.moovida.com/ so:

Summary: Media center
Comment 18 Alex Lancaster 2009-11-17 01:46:55 EST
For the moovida-base package though, it would probably make sense to include the name "moovida" in the Summary since it isn't really the one that's usually installed by end users.
Comment 19 Alex Lancaster 2009-11-17 02:02:11 EST
OK, to start the ball rolling here's the output for rpmlint for the binary packages:

# rpmlint *
moovida.noarch: W: name-repeated-in-summary Moovida
moovida-base.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/elisa/core/launcher.py 0644 /usr/bin/python
moovida-base.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/elisa/core/utils/mswin32/structures.py 0644 /usr/bin/python
moovida-base.noarch: E: wrong-script-end-of-line-encoding /usr/lib/python2.6/site-packages/elisa/core/utils/mswin32/structures.py
moovida-base.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/elisa/core/tests/test_launcher.py 0644 /usr/bin/python
moovida-plugins-bad.noarch: W: no-documentation
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/database/i18n/cs/LC_MESSAGES/elisa-plugin-database.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/database/i18n/de/LC_MESSAGES/elisa-plugin-database.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/database/i18n/es/LC_MESSAGES/elisa-plugin-database.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/database/i18n/fr/LC_MESSAGES/elisa-plugin-database.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/database/i18n/gl/LC_MESSAGES/elisa-plugin-database.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/database/i18n/hu/LC_MESSAGES/elisa-plugin-database.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/database/i18n/it/LC_MESSAGES/elisa-plugin-database.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/database/i18n/pl/LC_MESSAGES/elisa-plugin-database.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/database/i18n/pt_BR/LC_MESSAGES/elisa-plugin-database.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/dvd/i18n/cs/LC_MESSAGES/elisa-plugin-dvd.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/dvd/i18n/de/LC_MESSAGES/elisa-plugin-dvd.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/dvd/i18n/es/LC_MESSAGES/elisa-plugin-dvd.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/dvd/i18n/fr/LC_MESSAGES/elisa-plugin-dvd.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/dvd/i18n/gl/LC_MESSAGES/elisa-plugin-dvd.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/dvd/i18n/hu/LC_MESSAGES/elisa-plugin-dvd.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/dvd/i18n/it/LC_MESSAGES/elisa-plugin-dvd.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/dvd/i18n/pl/LC_MESSAGES/elisa-plugin-dvd.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/dvd/i18n/pt_BR/LC_MESSAGES/elisa-plugin-dvd.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/poblesec/i18n/cs/LC_MESSAGES/elisa-plugin-poblesec.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/poblesec/i18n/de/LC_MESSAGES/elisa-plugin-poblesec.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/poblesec/i18n/es/LC_MESSAGES/elisa-plugin-poblesec.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/poblesec/i18n/fr/LC_MESSAGES/elisa-plugin-poblesec.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/poblesec/i18n/gl/LC_MESSAGES/elisa-plugin-poblesec.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/poblesec/i18n/hu/LC_MESSAGES/elisa-plugin-poblesec.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/poblesec/i18n/it/LC_MESSAGES/elisa-plugin-poblesec.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/poblesec/i18n/pl/LC_MESSAGES/elisa-plugin-poblesec.mo
moovida-plugins-bad.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/poblesec/i18n/pt_BR/LC_MESSAGES/elisa-plugin-poblesec.mo
moovida-plugins-good.noarch: W: no-documentation
moovida-plugins-good.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/base/i18n/cs/LC_MESSAGES/elisa-plugin-base.mo
moovida-plugins-good.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/base/i18n/de/LC_MESSAGES/elisa-plugin-base.mo
moovida-plugins-good.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/base/i18n/es/LC_MESSAGES/elisa-plugin-base.mo
moovida-plugins-good.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/base/i18n/fr/LC_MESSAGES/elisa-plugin-base.mo
moovida-plugins-good.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/base/i18n/gl/LC_MESSAGES/elisa-plugin-base.mo
moovida-plugins-good.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/base/i18n/hu/LC_MESSAGES/elisa-plugin-base.mo
moovida-plugins-good.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/base/i18n/it/LC_MESSAGES/elisa-plugin-base.mo
moovida-plugins-good.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/base/i18n/pl/LC_MESSAGES/elisa-plugin-base.mo
moovida-plugins-good.noarch: W: file-not-in-%lang /usr/lib/python2.6/site-packages/elisa/plugins/base/i18n/pt_BR/LC_MESSAGES/elisa-plugin-base.mo
4 packages and 0 specfiles checked; 4 errors, 39 warnings.

Some issues that need to be fixed are:

1. addressing the file permissions
2. one line encoding issue
3. handling the locale files correctly, see: https://fedoraproject.org/wiki/Packaging/Guidelines#Handling_Locale_Files

The no documentation thing can probably be ignored.  It also points out the problem with the duplicate name in summary that I already pointed out.

The SRPMS pretty much pass the tests, with the exception of the Summary already noted:

# rpmlint *.src.rpm
moovida.src: W: name-repeated-in-summary Moovida
3 packages and 0 specfiles checked; 0 errors, 1 warnings.

And, yes, it would be great if you could use your webspace on fedorapeople.org to post the updated .specs and srpms for the subsequent iteration.  Thanks!
Comment 20 Graeme Gillies 2009-11-19 00:28:37 EST
Hi I have made the requested changes. I wasn't able to use %find_lang to correctly mark all the locale data unfortunately because it's in a non-standard directory, I'm not sure if there is a better way to mark it than what I have done.

moovida.spec:
http://ggillies.fedorapeople.org/moovida.spec

moovida-1.0.8-2.fc11.src.rpm:
http://ggillies.fedorapeople.org/moovida-1.0.8-2.fc11.src.rpm

moovida-plugins-good.spec:
http://ggillies.fedorapeople.org/moovida-plugins-good.spec

moovida-plugins-good-1.0.8-2.fc11.src.rpm:
http://ggillies.fedorapeople.org/moovida-plugins-good-1.0.8-2.fc11.src.rpm

moovida-plugins-bad.spec:
http://ggillies.fedorapeople.org/moovida-plugins-bad.spec

moovida-plugins-bad-1.0.8-2.fc11.src.rpm:
http://ggillies.fedorapeople.org/moovida-plugins-bad-1.0.8-2.fc11.src.rpm
Comment 21 Graeme Gillies 2009-11-19 00:34:01 EST
(In reply to comment #20)
> Hi I have made the requested changes. I wasn't able to use %find_lang to
> correctly mark all the locale data unfortunately because it's in a non-standard
> directory, I'm not sure if there is a better way to mark it than what I have
> done.
> 
> moovida.spec:
> http://ggillies.fedorapeople.org/moovida.spec
> 
> moovida-1.0.8-2.fc11.src.rpm:
> http://ggillies.fedorapeople.org/moovida-1.0.8-2.fc11.src.rpm
> 
> moovida-plugins-good.spec:
> http://ggillies.fedorapeople.org/moovida-plugins-good.spec
> 
> moovida-plugins-good-1.0.8-2.fc11.src.rpm:
> http://ggillies.fedorapeople.org/moovida-plugins-good-1.0.8-2.fc11.src.rpm
> 
> moovida-plugins-bad.spec:
> http://ggillies.fedorapeople.org/moovida-plugins-bad.spec
> 
> moovida-plugins-bad-1.0.8-2.fc11.src.rpm:
> http://ggillies.fedorapeople.org/moovida-plugins-bad-1.0.8-2.fc11.src.rpm  

Apologies I was accidentaly logged into my old bugzilla account when I posted that.
Comment 22 Alex Lancaster 2009-11-19 01:33:42 EST
(In reply to comment #21)
> (In reply to comment #20)
> > Hi I have made the requested changes. I wasn't able to use %find_lang to
> > correctly mark all the locale data unfortunately because it's in a non-standard
> > directory, I'm not sure if there is a better way to mark it than what I have
> > done.
> > 
> > moovida.spec:
> > http://ggillies.fedorapeople.org/moovida.spec
> > 
> > moovida-1.0.8-2.fc11.src.rpm:
> > http://ggillies.fedorapeople.org/moovida-1.0.8-2.fc11.src.rpm
> > 
> > moovida-plugins-good.spec:
> > http://ggillies.fedorapeople.org/moovida-plugins-good.spec
> > 
> > moovida-plugins-good-1.0.8-2.fc11.src.rpm:
> > http://ggillies.fedorapeople.org/moovida-plugins-good-1.0.8-2.fc11.src.rpm
> > 
> > moovida-plugins-bad.spec:
> > http://ggillies.fedorapeople.org/moovida-plugins-bad.spec
> > 
> > moovida-plugins-bad-1.0.8-2.fc11.src.rpm:
> > http://ggillies.fedorapeople.org/moovida-plugins-bad-1.0.8-2.fc11.src.rpm  
> 
> Apologies I was accidentaly logged into my old bugzilla account when I posted
> that.  

Thanks.  This doesn't need a new spec bump right away, but when you update for the next round could you use "center" (US spelling) rather than "centre" in the Summary.  Although I generally use centre myself (being from a Commonwealth country), upstream uses "center" http://www.moovida.com/ so we should stick with that.

Did you have any luck with finding a sponsor?  The best way to get a sponsor is to help out post suggestions/on other reviews which shows you know the packaging guidelines and to become more generally known on the packaging side of the Fedora community.
Comment 23 Graeme Gillies 2009-11-19 22:49:56 EST
I have rolled back the spec bump, changed the spelling, and uploaded new specs/SRPMS (now built on my F12 machine).

moovida.spec:
http://ggillies.fedorapeople.org/moovida.spec

moovida-1.0.8-1.fc12.src.rpm:
http://ggillies.fedorapeople.org/moovida-1.0.8-1.fc12.src.rpm

moovida-plugins-good.spec:
http://ggillies.fedorapeople.org/moovida-plugins-good.spec

moovida-plugins-good-1.0.8-1.fc12.src.rpm:
http://ggillies.fedorapeople.org/moovida-plugins-good-1.0.8-1.fc12.src.rpm

moovida-plugins-bad.spec:
http://ggillies.fedorapeople.org/moovida-plugins-bad.spec

moovida-plugins-bad-1.0.8-1.fc12.src.rpm:
http://ggillies.fedorapeople.org/moovida-plugins-bad-1.0.8-1.fc12.src.rpm

Still trying to track down a sponsor, thanks for the suggestion about assisting with other package reviews, I will try that :)
Comment 24 Alex Lancaster 2009-11-19 23:20:01 EST
(In reply to comment #23)
> I have rolled back the spec bump, changed the spelling, and uploaded new
> specs/SRPMS (now built on my F12 machine).

Actually when I said "This doesn't need a new spec bump right away", I meant that there was no need to up the release tag and create and post new .specs and .srpms until after the next major review (i.e. just keep of a note of it until you do the next major revision).  I didn't mean rolling back the release tag to version 1.  Sorry if I wasn't clearer about that. 

Actually you should *always* update the release tag when you post a new version.  So actually technically this should have been version 1.0.8-3.
Comment 25 Alex Lancaster 2009-11-19 23:22:17 EST
(In reply to comment #24)
> (In reply to comment #23)
> > I have rolled back the spec bump, changed the spelling, and uploaded new
> > specs/SRPMS (now built on my F12 machine).
> 
> Actually when I said "This doesn't need a new spec bump right away", I meant
> that there was no need to up the release tag and create and post new .specs and
> .srpms until after the next major review (i.e. just keep of a note of it until
> you do the next major revision).  I didn't mean rolling back the release tag to
> version 1.  Sorry if I wasn't clearer about that. 
> 
> Actually you should *always* update the release tag when you post a new
> version.  So actually technically this should have been version 1.0.8-3.  

And of course for each new release tag, document that in the %changelog, then it makes it easier to diff the old and new specs.
Comment 26 Graeme Gillies 2009-12-02 20:59:01 EST
Hi,

I have updated the specs/SRPMS to moovida 1.0.9 which came out recently. I am still trying to track down a sponsor for review

moovida.spec:
http://ggillies.fedorapeople.org/moovida.spec

moovida-1.0.9-1.fc12.src.rpm:
http://ggillies.fedorapeople.org/moovida-1.0.9-1.fc12.src.rpm

moovida-plugins-good.spec:
http://ggillies.fedorapeople.org/moovida-plugins-good.spec

moovida-plugins-good-1.0.9-1.fc12.src.rpm:
http://ggillies.fedorapeople.org/moovida-plugins-good-1.0.9-1.fc12.src.rpm

moovida-plugins-bad.spec:
http://ggillies.fedorapeople.org/moovida-plugins-bad.spec

moovida-plugins-bad-1.0.9-1.fc12.src.rpm:
http://ggillies.fedorapeople.org/moovida-plugins-bad-1.0.9-1.fc12.src.rpm
Comment 27 Matthias Saou 2009-12-10 05:06:53 EST
I'll look into becoming your sponsor if needed. For now, a few comments about the moovida.spec file :
 * You should remove the first line (the python dir comment)
 * You're mixing $RPM_BUILD_ROOT and %{buildroot} when you shouldn't
 * You're inconsistently skipping one or two lines between sections
 * Putting all (build)requirements on a single line makes them hard to read... I personally prefer putting one per line (or a few more if they're related), not going over 80 columns and keeping the same order for requirements and buildrequirements.
 * The plain explicit "python" requirement is clearly not needed
 * The "%doc COPYING LICENSE*" line should be below the %defattr for consistency
 * Why are some files being forced to mode 755? If there is a reason, you should add a comment above, because doing that is at least partly wrong since they end up listed twice, possibly with different modes (and this might be an undefined behaviour, though it's certainly the last one who wins). The proper fix here if those files never need to be executed directly might be to remove the "#!" line instead.
Comment 28 Graeme Gillies 2009-12-10 19:51:53 EST
Hi Matthias,

Thanks heaps for helping me with this and sponsoring me, much appreciated. I'm going through you comments now (all good points) and making the necessary changes and testing them in mock, just wanted to get your opinion about the last point (in moovida 1.0.9 it seems they fixed most the files with the problem, but only 2 remain). Originally I didn't have the mode 755 on them at all, but when I ran rpmlint over the rpms I would get

moovida-base.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/elisa/core/launcher.py 0644 /usr/bin/python
moovida-base.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/elisa/core/tests/test_launcher.py 0644 /usr/bin/python

While I can exectute the scripts themselves and they work, if they are left without execute permissions moovida still works fine. Should I create a patch that removes the #!/usr/bin/python line from these scripts, or would it be better if I re-worked my %files section so instead of blindly including %{python_sitelib}/* I explicitly indicate all files/directories that need to be included (and thus leave the defattr settings on these files).
Comment 29 Matthias Saou 2009-12-11 05:59:28 EST
In this particular case, I think the rpmlint errors should be treated as false positives. That check is meant to find scripts meant to be executed directly which would be lacking +x. Here, I seriously doubt these python files are meant to be executed "standalone", so it's just the developers who have introduced a useless though harmless "#!/usr/bin/python" at the top of the file.

Various overlapping suggestions :
 * Poke upstream about it if you want, it could be easily fixed in the next release
 * Put a simple comment above the relevant line of %files with something like "# Some *.py files have a useless hashbang line we should just ignore"
 * Include a patch which would remove those lines and silence rpmlinl

I think the comment should be fine. Since those are *launcher.py files, maybe they are occasionally run directly by some to debug things. Anyway, in the end, quite a minor issue if you ask me :-)
Comment 30 Graeme Gillies 2009-12-14 23:48:10 EST
Ah ok thanks :)

I have updated the spec files and rebuilt the SRPMs (made the corrections you outlined to all three packages)

moovida.spec:
http://ggillies.fedorapeople.org/moovida.spec

moovida-1.0.9-2.fc12.src.rpm:
http://ggillies.fedorapeople.org/moovida-1.0.9-2.fc12.src.rpm

moovida-plugins-good.spec:
http://ggillies.fedorapeople.org/moovida-plugins-good.spec

moovida-plugins-good-1.0.9-2.fc12.src.rpm:
http://ggillies.fedorapeople.org/moovida-plugins-good-1.0.9-2.fc12.src.rpm

moovida-plugins-bad.spec:
http://ggillies.fedorapeople.org/moovida-plugins-bad.spec

moovida-plugins-bad-1.0.9-2.fc12.src.rpm:
http://ggillies.fedorapeople.org/moovida-plugins-bad-1.0.9-2.fc12.src.rpm
Comment 31 Valent Turkovic 2009-12-20 14:45:16 EST
Can't wait to install Moovida on Fedora 12! When do you expect rpms to be available in Fedora repos?
Comment 32 Alex Lancaster 2009-12-20 17:22:19 EST
(In reply to comment #31)
> Can't wait to install Moovida on Fedora 12! When do you expect rpms to be
> available in Fedora repos?  

Only once Matthias has approved the package (which hopefully will be soon) and agreed to sponsor Graeme (which he appears to be willing to do), then the package can be built.  The bottleneck is now getting Matthias' final approval, until then be patient Valent.

Matthias: could you please set the review flag and ASSIGN to bug to yourself to indicate that you are actively doing the review as per the standard practice of reviewing?   If you find yourself unable to finish this off in timely fashion (say within the next week or so, since it's already been 2 weeks since you initiated the review), please let us know and we can recommence the search for a new sponsor.  Thanks!
Comment 33 Tim Hughes 2009-12-29 19:19:09 EST
This is a bit of a FYI so that someone else doesnt need to work it out when they come across the same issue

I have built these rpms against a clean install of F12 and found that they used 100% of a single cpu core

A strace gives the following repeated infinitely 

gettimeofday({1262131136, 457154}, NULL) = 0
poll([{fd=6, events=POLLIN}, {fd=10, events=POLLIN}, {fd=9, events=POLLIN}, {fd=15, events=POLLIN}, {fd=23, events=POLLIN}, {fd=24, events=POLLIN}, {fd=8, events=POLLIN}, {fd=29, events=POLLIN}, {fd=31, events=POLLIN}, {fd=35, events=POLLIN}, {fd=33, events=POLLIN}, {fd=27, events=POLLIN}, {fd=36, events=POLLIN}, {fd=38, events=POLLIN}, {fd=32, events=POLLIN}, {fd=26, events=POLLIN}, {fd=3, events=POLLIN}, {fd=13, events=POLLIN}, {fd=34, events=POLLIN}, {fd=37, events=POLLIN}], 20, 9) = 1 ([{fd=9, revents=POLLIN}])
gettimeofday({1262131136, 457349}, NULL) = 0
read(10, 0x9da6ba0, 4096)               = -1 EAGAIN (Resource temporarily unavailable)


Appears to be a known bug with both pygobject and pygtk. Looks like a patch has been pushed to pygobject back at 2009-10-21 but maybe not to pygtk. 

Relevant bugs from other projects:  
https://bugs.launchpad.net/moovida/+bug/233734
http://twistedmatrix.com/trac/ticket/3096
https://bugzilla.gnome.org/show_bug.cgi?id=481569
Comment 34 Alex Lancaster 2010-01-07 01:06:42 EST
Matthias: ping?  Will you still be able to sponsor Graeme or should we perhaps search for a new one if you don't have time to do the review.
Comment 35 Jason Tibbitts 2010-01-08 16:51:56 EST
Who is actually submitting this package now?  The "sponsorship needed" list shows Alex Lancaster: http://fedoraproject.org/PackageReviewStatus/NEEDSPONSOR.html.  If that's not correct then you now understand why we ask that if someone else is submitting a review, they open their own review ticket.  Otherwise we have no real way to tell what's going on without actually reading all of the tickets, and there are too many to do that.
Comment 36 Alex Lancaster 2010-01-08 17:20:26 EST
(In reply to comment #35)
> Who is actually submitting this package now?  The "sponsorship needed" list
> shows Alex Lancaster:
> http://fedoraproject.org/PackageReviewStatus/NEEDSPONSOR.html.  If that's not
> correct then you now understand why we ask that if someone else is submitting a
> review, they open their own review ticket.  Otherwise we have no real way to
> tell what's going on without actually reading all of the tickets, and there are
> too many to do that.  

Graeme is submitting the package, originally I was going to do the review, but since Graeme is a new package, he needs to be sponsored.  Is there no way to change the reporter or otherwise indicate that a different person is submitting the review than open a whole new bug?

If not: Graeme can you open up a new bug and close this as a duplicate of the new bug?  Make sure to Cc Matthias as a potential sponsor.
Comment 37 Graeme Gillies 2010-01-10 22:50:50 EST
I have created bug #554243 (https://bugzilla.redhat.com/show_bug.cgi?id=554243) for the new review request. Unfortunately though, I can't seem to mark this bug as closed/duplicate of that bug, someone with more permissions than I will have to.
Comment 38 Alex Lancaster 2010-01-12 01:48:47 EST
(In reply to comment #37)
> I have created bug #554243 (https://bugzilla.redhat.com/show_bug.cgi?id=554243)
> for the new review request. Unfortunately though, I can't seem to mark this bug
> as closed/duplicate of that bug, someone with more permissions than I will have
> to.    

Since I opened it, I closed it as a duplicate.

All others: please Cc yourself on the new review request if you wish to track progress and/or comment on the package.

*** This bug has been marked as a duplicate of bug 554243 ***
Comment 39 Alex Lancaster 2010-01-12 01:56:30 EST
Remove obsolete blockers.

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