Bug 530021
| Summary: | Review Request: moovida - Media Center | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Alex Lancaster <alex> |
| Component: | Package Review | Assignee: | Nobody's working on this, feel free to take it <nobody> |
| Status: | CLOSED DUPLICATE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | che666, christoph.wickert, fedora-package-review, felix, ggillies, graeme.r.gillies, matthias, notting, thughes, tiagomatos, valent.turkovic, yulrottmann |
| Target Milestone: | --- | ||
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | 233598 | Environment: | |
| Last Closed: | 2010-01-12 06:48:47 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Alex Lancaster
2009-10-21 07:59:01 UTC
There are actually 3 additional plugin packages that need to be considered along with this release: moovida-plugins-bad (replaces elisa-plugins-bad): Spec: http://people.fluendo.com/matthias/moovida/fedora/11/i386/noarch/moovida-plugins-bad-1.0.6-1.fc11/moovida-plugins-bad.spec SRPM: http://people.fluendo.com/matthias/moovida/fedora/11/i386/noarch/moovida-plugins-bad-1.0.6-1.fc11/moovida-plugins-bad-1.0.6-1.fc11.src.rpm moovida-plugins-good (replaces elisa-plugins-good): Spec: http://people.fluendo.com/matthias/moovida/fedora/11/i386/noarch/moovida-plugins-good-1.0.6-1.fc11/moovida-plugins-good.spec SRPM: http://people.fluendo.com/matthias/moovida/fedora/11/i386/noarch/moovida-plugins-good-1.0.6-1.fc11/moovida-plugins-good-1.0.6-1.fc11.src.rpm moovida-plugins-ugly (replaces elisa-plugins-ugly): Spec: http://people.fluendo.com/matthias/moovida/fedora/11/i386/noarch/moovida-plugins-ugly-1.0.6-1.fc11/moovida-plugins-ugly.spec SRPM: http://people.fluendo.com/matthias/moovida/fedora/11/i386/noarch/moovida-plugins-ugly-1.0.6-1.fc11/moovida-plugins-ugly-1.0.6-1.fc11.src.rpm 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) 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. 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 (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. Oh, I see what you mean, you are talking about moovida and moovida-base within the moovida.spec file? (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. By the way, the original elisa review request is bug #233598. (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. (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. 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). 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 :) 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 ;) Taking review. 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 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. (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 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. 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! 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 (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. (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. 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 :) (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. (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. 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 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.
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).
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 :-) 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 Can't wait to install Moovida on Fedora 12! When do you expect rpms to be available in Fedora repos? (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! 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
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. 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. (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. 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. (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 *** Remove obsolete blockers. |