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 220860
Summary: | Review Request: galternatives - Alternatives Configurator | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Deji Akingunola <dakingun> |
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | mtasaka, panemade |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-01-03 03:10:04 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: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Deji Akingunola
2006-12-28 04:12:54 UTC
Please check the packages you created by rpmlint Very Very quick note: -------------------------------------- E: galternatives no-binary E: galternatives non-executable-script /usr/lib/python2.5/site-packages/galternatives/main.py 0644 E: galternatives non-executable-script /usr/lib/python2.5/site-packages/galternatives/gadebug.py 0644 E: galternatives non-executable-script /usr/lib/python2.5/site-packages/galternatives/common.py 0644 E: galternatives non-executable-script /usr/lib/python2.5/site-packages/galternatives/__init__.py 0644 E: galternatives non-executable-script /usr/lib/python2.5/site-packages/galternatives/alternative.py 0644 E: galternatives-debuginfo empty-debuginfo-package ---------------------------------------- * This seems to be a noarch rpm * Scripts with shebang should have executable permission (or, if 0644 permissons are correct, shebangs should be removed) * .pyo files are not marked as ghosts due to SELinux issues. * Please check Requires - This package should require pam explicitly - And "include" line of pam configuration file means that requirement of pam is version-specific (pam >= 0.80) (In reply to comment #1) > Please check the packages you created by rpmlint > > * This seems to be a noarch rpm Yes, you're right, changed it to be so. > * Scripts with shebang should have executable permission > (or, if 0644 permissons are correct, shebangs should be removed) This is _strictly_ _not_ necessary (I've seen other reviews ignoring these kind of warning on python packages); anyways to make everyone happy, I've sed out the shebangs. > * .pyo files are not marked as ghosts due to SELinux issues. Right, fixed. > * Please check Requires > - This package should require pam explicitly It does already rightly requires usermode, which in turn explicitly requires pam. Besides other packages in Extras that uses consolehelper only requires usermode. New file with changes uploaded, thanks for the review. Spec URL: ftp://czar.eas.yorku.ca/pub/galternatives/galternatives.spec SRPM URL: ftp://czar.eas.yorku.ca/pub/galternatives/galternatives-0.13.4-2.src.rpm ... not a packaging issue, however... I get the following backtrace. ----------------------------------------------------- [root@softbank218114170036 ~]# LANG=C alternatives --display print print - status is auto. link currently points to /usr/bin/lpr.cups /usr/bin/lpr.cups - priority 40 slave print-cancel: /usr/bin/cancel.cups slave print-lp: /usr/bin/lp.cups slave print-lpq: /usr/bin/lpq.cups slave print-lprm: /usr/bin/lprm.cups slave print-lpstat: /usr/bin/lpstat.cups slave print-lpc: /usr/sbin/lpc.cups slave print-cancelman: /usr/share/man/man1/cancel-cups.1.gz slave print-lpman: /usr/share/man/man1/lp-cups.1.gz slave print-lpqman: /usr/share/man/man1/lpq-cups.1.gz slave print-lprman: /usr/share/man/man1/lpr-cups.1.gz slave print-lprmman: /usr/share/man/man1/lprm-cups.1.gz slave print-lpstatman: /usr/share/man/man1/lpstat-cups.1.gz slave print-lpcman: /usr/share/man/man8/lpc-cups.8.gz Current `best' version is /usr/bin/lpr.cups. [root@softbank218114170036 ~]# galternatives ========================================== (then select the item of print) ========================================== Traceback (most recent call last): File "/usr/lib/python2.5/site-packages/galternatives/main.py", line 364, in alternative_selected_cb self.update_options_tree () File "/usr/lib/python2.5/site-packages/galternatives/main.py", line 400, in update_options_tree self.PRIORITY, int(option['priority']), ValueError: invalid literal for int() with base 10: '40 cups' ---------------------------------------------------- Would you know why? Well, also I * launch galternatives * select one item on "alternatimes" column (in the left) * select one options * push the button "Properties" (then a new window titled "Details" pops up) * Break the windows which poped up newly * Then push again the button "Properties" --- Then the new window poped up is totally gray.. (In reply to comment #3) > I get the following backtrace. > ----------------------------------------------------- > [root@softbank218114170036 ~]# LANG=C alternatives --display print > print - status is auto. > link currently points to /usr/bin/lpr.cups > /usr/bin/lpr.cups - priority 40 > slave print-cancel: /usr/bin/cancel.cups > slave print-lp: /usr/bin/lp.cups > slave print-lpq: /usr/bin/lpq.cups > slave print-lprm: /usr/bin/lprm.cups > slave print-lpstat: /usr/bin/lpstat.cups > slave print-lpc: /usr/sbin/lpc.cups > slave print-cancelman: /usr/share/man/man1/cancel-cups.1.gz > slave print-lpman: /usr/share/man/man1/lp-cups.1.gz > slave print-lpqman: /usr/share/man/man1/lpq-cups.1.gz > slave print-lprman: /usr/share/man/man1/lpr-cups.1.gz > slave print-lprmman: /usr/share/man/man1/lprm-cups.1.gz > slave print-lpstatman: /usr/share/man/man1/lpstat-cups.1.gz > slave print-lpcman: /usr/share/man/man8/lpc-cups.8.gz > Current `best' version is /usr/bin/lpr.cups. > [root@softbank218114170036 ~]# galternatives > ========================================== > (then select the item of print) > ========================================== > Traceback (most recent call last): > File "/usr/lib/python2.5/site-packages/galternatives/main.py", line 364, in > alternative_selected_cb > self.update_options_tree () > File "/usr/lib/python2.5/site-packages/galternatives/main.py", line 400, in > update_options_tree > self.PRIORITY, int(option['priority']), > ValueError: invalid literal for int() with base 10: '40 cups' > ---------------------------------------------------- > > Would you know why? Ah... I found because /var/lib/alternatives/print says: ------------------------------------------------------ auto /usr/bin/lpr print-cancel <snip> /usr/bin/lpr.cups 40 cups <- THIS LINE /usr/bin/cancel.cups <snip> -------------------------------------------------- And.... -------------------------------------------------- [tasaka1@localhost ~]$ rpm -q --scripts cups postinstall scriptlet (using /bin/sh): <snip> /usr/sbin/alternatives --install /usr/bin/lpr print /usr/bin/lpr.cups 40 \ --slave /usr/bin/lp print-lp /usr/bin/lp.cups \ <snip> --initscript cups -------------------------------------------------- and --initscripts option is "a Red Hat Linux specific option" according to "man alternatives" Then a patch is needed * to treat initscripts options correctly * or to ignore initscripts options for now. Then a documentation (like README.fedora) which explains that this package ignores initscripts option currently. (In reply to comment #4) > Well, also I > * launch galternatives > * select one item on "alternatimes" column (in the left) > * select one options > * push the button "Properties" (then a new window titled "Details" pops up) > * Break the windows which poped up newly > * Then push again the button "Properties" > > --- Then the new window poped up is totally gray.. This is a longstanding upstream bug, its not resolved yet. I was just about point out the --initscript option issue, but you already figured it out, I see what I can do about it tomorrow (well, later today ;). > -------------------------------------------------- > and --initscripts option is "a Red Hat Linux specific option" > according to "man alternatives" > > Then a patch is needed > * to treat initscripts options correctly > * or to ignore initscripts options for now. Then a documentation > (like README.fedora) which explains that this package ignores > initscripts option currently. I've added a patch to treat the initscripts appropriately, and thus fixing the traceback. Updated spec and srpm below; Spec URL: ftp://czar.eas.yorku.ca/pub/galternatives/galternatives.spec SRPM URL: ftp://czar.eas.yorku.ca/pub/galternatives/galternatives-0.13.4-3.src.rpm Your patch works well, thanks. Then, first full review for galternatives * General notes for python related packages - Unlike shared libraries' dependencies, python related dependencies are not checked automatically by rpmbuild and these have to be looked into manually. Normally, this can be done by checking what modules this package has to import. For this package ------------------------------------------------------------ [tasaka1@localhost ~]$ grep import /usr/sbin/galternatives `rpm -ql galternatives | grep py$` /usr/sbin/galternatives:import os, sys /usr/sbin/galternatives:import galternatives /usr/sbin/galternatives:from galternatives import gtk /usr/sbin/galternatives:import gettext /usr/sbin/galternatives:from galternatives.common import PACKAGE /usr/lib/python2.5/site-packages/galternatives/__init__.py:from main import * /usr/lib/python2.5/site-packages/galternatives/alternative.py:from common import PACKAGE /usr/lib/python2.5/site-packages/galternatives/alternative.py:import os, gettext /usr/lib/python2.5/site-packages/galternatives/alternative.py:from gadebug import print_debug /usr/lib/python2.5/site-packages/galternatives/main.py:import pygtk /usr/lib/python2.5/site-packages/galternatives/main.py: import gtk, gobject /usr/lib/python2.5/site-packages/galternatives/main.py: from gtk import glade /usr/lib/python2.5/site-packages/galternatives/main.py:from common import PACKAGE /usr/lib/python2.5/site-packages/galternatives/main.py:import sys, os, gettext /usr/lib/python2.5/site-packages/galternatives/main.py:from alternative import Alternative ------------------------------------------------------------ This means that this package needs "Requires: pygtk2" Then A. http://fedoraproject.org/wiki/Packaging/Guidelines * Licensing - Please include the license document. For this package, debian/copyright seems the best - Also, adding debian/changelog seems useful and should be included in the package. B. http://fedoraproject.org/wiki/Packaging/ReviewGuidelines = This is okay, except for issues on A C. Other notes: - For upstream URL: Maybe http://packages.qa.debian.org/g/galternatives.html is more useful? - I think this package to be useful, however, how do you think of http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=365365 ? (In reply to comment #8) > This means that this package needs "Requires: pygtk2" > I think I ave this in before, don't know why I removed it, re-added. > Then > A. http://fedoraproject.org/wiki/Packaging/Guidelines > * Licensing > - Please include the license document. For this package, debian/copyright > seems the best > - Also, adding debian/changelog seems useful and should be included in > the package. > Done. > C. Other notes: > - For upstream URL: > Maybe http://packages.qa.debian.org/g/galternatives.html is more useful? > I'm not sure, I've changed to it though (at least the latest source package is found there). > - I think this package to be useful, however, how do you think of > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=365365 ? Yes, I've seen it. However, I also believe the package to be useful and as long as there is no security issue(s) with it, and no real alternative for it, then we can as well have it and maintain it for fedora. File with new changes below. Thanks for the thorough review. Spec URL: ftp://czar.eas.yorku.ca/pub/galternatives/galternatives.spec SRPM URL: ftp://czar.eas.yorku.ca/pub/galternatives/galternatives-0.13.4-4.src.rpm Now * This package meets ReviewGuidelines/Guidelines * Works functionally Okay ------------------------------------------------- This package (galternatives) is APPROVED by me |