Spec URL: http://fedora.mkdir.name/packages/razertool-0.0.7/razertool.spec SRPM URL: http://fedora.mkdir.name/packages/razertool-0.0.7/razertool-0.0.7-1.fc10.src.rpm Description: RazerTool is an unofficial tool for controlling Razer Copperhead mice in Linux. It has both command-line and gtk+-2.x versions.
I've reviewed your package and it looks quite good. There is one TODO item and I have a small usage question (see below). * rpmlint: OK rpmlint SPECS/razertool.spec RPMS/i386/razertool-* SRPMS/razertool-0.0.7-1.fc10.src.rpm 3 packages and 1 specfiles checked; 0 errors, 0 warnings. * naming: OK * spec file name: OK * License: OK - GPLv2+: acceptable - matches the actual license - license file packaged * spec file in English and legible: OK * Source0: OK - newest upstream version used - source file matches upstream - md5sum: dad8236b2fface39f054a7ca6d1a448c razertool_0.0.7.orig.tar.gz - spectool -g works * compilation: OK - mock (F10) - koji: F11, F10 and F9 - %{optflags} are used correctly - smpflags used * locale handling: OK * debuginfo: OK - debuginfo files are not empty and can be used in gdb * handling of shared/static libraries, header files, pkgconfig files: OK (n/a) * directory ownership: OK - no directories created - only directories used which are included in the base filesystem * no files listed twice: OK * file permissions and ownership: OK * %clean section: OK * macro usage: OK * code vs. content: OK (only code) * subpackage for large documentation: OK (n/a) * %doc: OK * rm -rf in %install: OK * filenames UTF8: OK * desktop file: OK - it may be debatable in which menu the razortool appears (Preferences vs. Administration), but since it changes the configuration values globally (in the mouse), Administration seems the best option... * buildrequires: OK * scriptlets: OK * final requires: OK * functional test: TODO - programs run without segfault ;-) - since I don't have such a device, I can't do a full functional test - razertool-gtk doesn't find its icon: ** (razertool-gtk:12930): WARNING **: Couldn't find pixmap file: razertool-icon.png This is caused by moving the pixmaps in the spec file (the program will search the icon in the old place). I would suggest the following: - don't move %{_datadir}/%{name}/pixmaps, but package %{_datadir}/%{name} completly - use the full path to the icon in the *.desktop file (That's OK according to http://fedoraproject.org/wiki/Packaging/Guidelines#Icon_tag_in_Desktop_Files) Additionally I have one (optional) question: I've seen that libusb tries to open the files in /dev/bus/usb/*/*. On my system the user has no write permissions to any of these files (not even the one of the mouse). Is there a need to run this program as root or do you have to explicitly change the permissions of these files?
Spec URL: http://fedora.mkdir.name/packages/razertool-0.0.7/razertool.spec SRPM URL: http://fedora.mkdir.name/packages/razertool-0.0.7/razertool-0.0.7-2.fc10.src.rpm I fixed the pixmaps issue. Concerning that question regarding access rights... You are right, due to the permissions the tool has to be either run as root or a rule has to be added to udev. As I'm no fan of the adding-a-rule scenario, I'm wondering how to realize a password request here. Any ideas?
With pam: /etc/security/console.apps/razertool: . config-util PROGRAM=/usr/sbin/razertool SESSION=true In order to install razertool to /usr/sbin instead of /usr/bin, configure with %configure --bindir=%{_sbindir} /etc/pam.d/razertool: #%PAM-1.0 auth include config-util account include config-util session include config-util If you choose the pam approach, don't forget to require usermode-gtk. I would however prefer not to run this tool as root. Are these per-user or global settings? Running as root is ok if it's golbal, but as downside a weakness in the code could be exploited with root privileges. On the other hand I don't like the idea of making _all_ the usb device files writable. Does the program really need write access or is it just probing the devices? Or is there a special device for the razer mice? If so, a file in /etc/security/console.perms.d/ might suffice.
Well, I'm pretty unsure. The risk when using pam is clear to me... and should be avoided. <snip> ACTION=="add", BUS=="usb", SYSFS{idVendor}=="1532", SYSFS{idProduct}=="0101", MODE="0660", GROUP="plugdev" ACTION=="add", BUS=="usb", SYSFS{idVendor}=="1532", SYSFS{idProduct}=="0009", MODE="0660", GROUP="plugdev" </snip> This is the udev example rulefile. As you can see, this rulefile restricts the permission changes to Razer-only devices. There is no special device for Razer mice. The changes are rather hardware-side, as they are directly transmitted to the Razer (mouse) hardware.
In this case udev should IMO be preferred.
Then I shall go for this. One last question remains though: in the example udev case, the permissions are assigned to a group. As on Fedora, the user is -- by default -- in no group, should permissions to write be set for everyone here (equaling 0666)?
(In reply to comment #2) > Spec URL: > http://fedora.mkdir.name/packages/razertool-0.0.7/razertool.spec > > SRPM URL: > http://fedora.mkdir.name/packages/razertool-0.0.7/razertool-0.0.7-2.fc10.src.rpm > > I fixed the pixmaps issue. Thanks! Yes, that fixed the problem. A minor issue has leaked in with the latest fix: Since you've used %{_datadir}/%{name}/* to package the files in /usr/share/razertool/*, only the contents of the directory but not the directory itself is packaged. This lead to an directory which isn't owned by any package: rpm -qf /usr/share/razertool file /usr/share/razertool is not owned by any package The fix is quite easy, just use %{_datadir}/%{name}/ or %{_datadir}/%{name} to package the complete directory. Just for completness: please also removed the unused line: #rm -rf %{buildroot}%{_datadir}/%{name}/ from the spec file. ;-)
No correction of Christian's comment, just a more elaborate explanation for our new contributor: (In reply to comment #7) > %{_datadir}/%{name}/ > or > %{_datadir}/%{name} I would prefer %{_datadir}/%{name}/. It doesn't make a difference from the technical point of view, but another human that reads the spec will realize that it's a dir and not a file.
Spec URL: http://fedora.mkdir.name/packages/razertool-0.0.7/razertool.spec SRPM URL: http://fedora.mkdir.name/packages/razertool-0.0.7/razertool-0.0.7-3.fc10.src.rpm * Thu Apr 16 2009 Andreas Osowski <th0br0> - 0.0.7-3 - Code cleanup - Permission fix - Added the rules file for udev
Christoph Wickert is sponsoring me, FE-NEEDSPONSOR removed.
(In reply to comment #9) I've looked at the newest package and it looks quite good besides the following enhancement of the permission management for the mouse's device file. > - Added the rules file for udev Instead of the udev rule which sets the permissions of the device file to 0666 I would suggest the following: Add a hal rule to /usr/share/hal/fdi/policy/10osvendor: - let hal set for the specific vendor/device ID of the mice additional permissions for the local user (it is already done for mice for the /dev/input/event* device) for the "linux.device_file" (/dev/bus/usb/*/*). - make sure the number is lower than 20 (which defines the acl actions) I've attached a sample file which changes the permissions of linux's device file associated with a specific vendor and device id (which must be filled out).
Created attachment 340650 [details] sample hal policy file
Hello, first of all sorry for the bi~g delay. I added the hal policy and removed the udev rule. Spec URL: http://fedora.mkdir.name/packages/razertool-0.0.7/razertool.spec SRPM URL: http://fedora.mkdir.name/packages/razertool-0.0.7/razertool-0.0.7-4.fc10.src.rpm
Hi Andreas, Thanks for the new pacakge - we're very close to approval now. ;-) Unfortunately one minor issue appeared by adding the hal rule: Since the package contains a file in /usr/share/hal/fdi/policy/10osvendor/ and this directory belongs to hal, razertool must require hal: http://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership Best regards, Christian
Oh, that's true. I totally forgot about that requirement :) Spec URL: http://fedora.mkdir.name/packages/razertool-0.0.7/razertool.spec SRPM URL: http://fedora.mkdir.name/packages/razertool-0.0.7/razertool-0.0.7-5.fc10.src.rpm
Thanks for the update! All open issues were addressed in the last package: -> APPROVED
thanks a lot for your review New Package CVS Request ======================= Package Name: razertool Short Description: Tool for controlling Razer Copperhead mice Owners: th0br0 Branches: F-10 F-11 InitialCC:
cvs done.
razertool-0.0.7-5.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/razertool-0.0.7-5.fc10
razertool-0.0.7-5.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/razertool-0.0.7-5.fc11
razertool-0.0.7-5.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.
razertool-0.0.7-5.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.