Hide Forgot
SPEC URL: http://adimania.fedorapeople.org/libkqueue.spec SRPM URL: http://adimania.fedorapeople.org/libkqueue-1.0.1-1.src.rpm Description: libkqueue is a userspace implementation of the kqueue(2) kernel event notification mechanism. Initial efforts are focused on porting to the Linux 2.6 kernel. libkqueue acts as a translator between the kevent structure and the native kernel facilities of the host machine. Koji Scratch Build URL: http://koji.fedoraproject.org/koji/taskinfo?taskID=2906645
Not a full review yet but here is some initial feedback: " Initial efforts are focused on porting to the Linux 2.6 kernel" * Drop this from the description. * You don't need to define the buildroot or have a %clean section http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag http://fedoraproject.org/wiki/Packaging:Guidelines#.25clean * Does %setup really require passing name and version to it? * You need to use %configure macro and not use ./configure * Must use %defattr(-,root,root,-) * Should not package .la file http://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries * Header files and pkgconfig files belong in a -devel package * You should use a %check and run the in-built test suite
src/common/tree.h and include/sys/event.h are under the BSD license (2 clause) and hence the license tag must be MIT and BSD
The bugs have been fixed and devel package has been created. The package has been updated to latest version. Spec: http://adimania.fedorapeople.org/specs/libkqueue.spec SRPM: http://adimania.fedorapeople.org/src.rpms/libkqueue-1.0.2-1.src.rpm Koji Build URL: http://koji.fedoraproject.org/koji/taskinfo?taskID=2966742
Run rpmlint on the spec file, srpm and binary rpms and try to fix any warnings and errors. Files which should be in -devel subpackage are not. You shouldn't package libtool archive (.la) and static library (.a). Fix the summary as well.
The package would violate the static library packaging guidelines anyway. If shared and static lib are present, the static lib would need to be moved into a libkqueue-static subpackage: https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries > %package devel https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages > %{_mandir}/man2/kqueue.2.gz > %{_mandir}/man2/kevent.2.gz Manual pages are compressed on-the-fly by rpmbuild with a compression that may change, so including with wildcard .* instead of .gz is the way to go. > %files devel > %defattr(-,root,root,-) > %{_includedir}/kqueue/sys/event.h https://fedoraproject.org/wiki/Packaging:UnownedDirectories > %post > /sbin/ldconfig > > %postun > /sbin/ldconfig The better form here is to not run ldconfig from within a /bin/sh script, but directly: %post -p /sbin/ldconfig %postun -p /sbin/ldconfig
Created attachment 559432 [details] Updated .spec file This updated .spec file addresses most of the comments above. There is one remaining issue: the debuginfo rpm contains no files: I think this is because libkqueue.so is actually a symbolic link to libkqueue.so.0.0. The .spec file includes new links for the home page and source and this .spec file is for 1.0.4. The next attachment is a patch file to address two compiler warnings that were treated as errors.
Created attachment 559433 [details] Compilation errors patch This patch addresses two compilation warnings. One is for an ignored read(2) result that is simply passed up to the caller: all the callers I could find do check the result. The other warning is for an ignored write(2) in a signal handler. There is nothing much you can do with this (as it's in a signal handler) so I just wrapped it in an assert; it should never fail anyway.
I don't suppose this is going to make it into Fedora 17 is it? Along with the other libraries that make up Apple's Grand Central Dispatch?
@Aditya, you didn't response here for more than a year. Are you still interested in to keep this review request alive?
I'm interested to get this package in, as I need it as a dependancy :)
(In reply to comment #10) > I'm interested to get this package in, as I need it as a dependancy :) Paul, if Aditya doesn't response within a reasonable time frame, feel free to open a new review request for libkqueue and mark the current one as a duplicate. I'll do the review then.
I will close this as I see we already have fresh review request for this package. *** This bug has been marked as a duplicate of bug 889505 ***