Red Hat Bugzilla – Bug 237817
Merge review request for pam_thinkfinger
Last modified: 2007-11-30 17:12:02 EST
Spec URL: http://belial.dead.li/~jmp/thinkfinger/Fedora/thinkfinger.spec SRPM URL: http://belial.dead.li/~jmp/thinkfinger/Fedora/SRPMS/pam_thinkfinger-0.3-2.src.rpm Description: ThinkFinger is a driver for the UPEK/SGS Thomson Microelectronics fingerprint reader (USB ID 0483:2016). The device is being found either as a standalone USB device, built into USB keyboards or built into laptops. The following laptop vendors are using the device: - Dell - IBM/Lenovo - Toshiba Toshiba is shipping their laptops either with the UPEK/SGS Thomson Microelectronics fingerprint reader or with a fingerprint reader built by AuthenTec. The AuthenTec fingerprint reader is *not* supported by ThinkFinger. SONY laptops with the UPEK/SGS Thomson Microelectronics fingerprint reader are not supported.
A few random questions regarding the design of the program itself, rather than the package: - Why does pam_thinkfinger need uinput when pam_usb (http://www.pamusb.org/) doesn't? - Why does it need threading (which is likely to cause problems with PAM) (apart from a bad library design?)
Bastien, I was expecting these :-) - uinput has been added by the maintainer to send \n after the swiped image. Now this was working fine with the code from 0.2.. To me this is just a gadget and I will add --with-uinput in configure to give the choice. Ubuntu,debian and other folks already spoke to me about this. - threading... same as above! a choice from the maintainer and that I will find time sometime to get them out. Feel free to assist ;-) Jose
Isn't this a duplicate of the already available/approved 'thinkfinger' package? https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229291 Perhaps you could talk to the maintainer of that package and offer improvements and/or help co-maintain it?
(In reply to comment #3) > Isn't this a duplicate of the already available/approved 'thinkfinger' package? > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229291 > > Perhaps you could talk to the maintainer of that package and offer improvements > and/or help co-maintain it? It is indeed. As Jose is also helping upstream, it would be good if the original package could include some of Jose's patches.
Also the guy is aware of this, CCing him.
Moving to thinkfinger. Julian, could you check with Jose which patches you'd want to include in the package? The README might be useful as well.
I will include the readme as soon as F7 is released, since rawhide is at devel freeze right now and update from 0.2 to 0.3 would probably need Release Engineering Team approval. As for the uinput dependency, I have been investigating how to make 0.2 -> 0.3 update, as well as install, require no config files editing from the users. I was suggested that creating an udev rule is the best solution. Locally I am running a version that loads uinput regardless of the devices present, with snippet borrowed from, nomen omen, udev. BTW, perhaps I should even hand the maintainership to you, given your coding skills?
Oh, one more thing. IIRC there is a problem that whenever system-config-auth is run, pam_thinkfinger will be removed from the config. I don't know if this can be solved at thinkfinger side.
Julian, in which case you need the patches at : * http://belial.dead.li/~jmp/thinkfinger/0.2.3/patches/ unless the version you currently packaged is 0.2.2? these have been comitted upstream by Timo. I don't mind sharing or maintaining :-) We should also envisage to rename the package as I've done in : * http://belial.dead.li/~jmp/thinkfinger/Fedora/ Jose
For the udev rules, that was an idea I had, however... nah :-) I rather prefered to pass this onto the patch that opens different locations. Also, what Bastien was suggesting was to remove uinput and put back the write on the stdin from the terminal. I will work on a simple patch that adds into configure.in the --without-uinput, so in Fedora and Ubuntu/Debian we can call that and bypass the usage of uinput if we agree on this. Also, note that 0.2.3 is a version pulled out from a devel cvs... it has never been released :-/; I bet the one you are using is 0.2.2, for this, also check: http://belial.dead.li/~jmp/thinkfinger/0.2.2/ Jose For the threads, that needs some time... And I'm not sure Timo will like the idea. Jose
Answering to comment 8, nop not a thinkfinger problem.
OK, email to Release Engineering Team sent.
The team have suggested to wait for F7. I'll add you as comaintainer in the meantime.
OK, Fedora 7 is branched, so I will push this update to devel soonish.
One thing bothers me: with your patch .bir files are stored elsewhere, in security subdir. How to handle that nicely with an update, so that users won't have to re-enroll?
Created attachment 155035 [details] Updated spec with merged changes I have merged your changes with some minor tweaks. Until we figure out how to handle .bir relocation nicehow to handly, patch is disabled. Also, as you promised to work on dropping uinput requirement, I left my hacky solution in - I don't know nothing about udev and given the circumstances it's not worth my time to learn it. Rename deferred until I figure it how to handle it in CVS.
Changes now committed for rawhide.
Jose, why did you change the .bir files storage location at all?
Built for devel.
Hello! /etc/security was my first choice, however after reading FHS, no binaries should be stored under /etc/, therefore none of these directories are correct for this package. I would recommend /var/lib/ as OpenLDAP does to store it's data. * http://www.pathname.com/fhs/pub/fhs-2.3.html#PURPOSE31 But this is just a detail where to store the bir files - yes an upgrade could be problematic. Keep the original for the time being.
OK, everything seems to be clear now, so I'm planning to close this bug once new version gets built for all active branches, which will happen after Fedora 7 release. As you are a maintainer now, if you have any changes, feel free to commit them to CVS directly. Cheers.