Bug 684446

Summary: Review Request: libkqueue - A userspace implementation of the kqueue kernel event notification mechanism
Product: [Fedora] Fedora Reporter: Aditya Patawari <adimania>
Component: Package ReviewAssignee: Paul Wouters <pwouters>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: eradman, fedora-package-review, jch, mail, metherid, notting, panemade, pwouters
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 889505 (view as bug list) Environment:
Last Closed: 2013-07-22 07:19:03 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Description Flags
Updated .spec file
Compilation errors patch none

Description Aditya Patawari 2011-03-12 17:09:08 UTC
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

Comment 1 Rahul Sundaram 2011-03-12 19:04:50 UTC
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


* 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  


* Header files and pkgconfig files belong in a -devel package
* You should use a %check and run the in-built test suite

Comment 2 Rahul Sundaram 2011-03-14 07:36:02 UTC
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

Comment 3 Aditya Patawari 2011-04-01 15:40:10 UTC
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

Comment 4 Rahul Sundaram 2011-04-02 19:10:35 UTC
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.

Comment 5 Michael Schwendt 2011-05-03 08:08:42 UTC
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:

> %package devel


> %{_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


> %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

Comment 6 John Haxby 2012-02-04 20:46:36 UTC
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.

Comment 7 John Haxby 2012-02-04 20:50:23 UTC
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.

Comment 8 John Haxby 2012-02-04 20:51:21 UTC
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?

Comment 9 Mario Blättermann 2012-09-17 19:37:35 UTC
@Aditya, you didn't response here for more than a year. Are you still interested in to keep this review request alive?

Comment 10 Paul Wouters 2012-09-28 23:00:26 UTC
I'm interested to get this package in, as I need it as a dependancy :)

Comment 11 Mario Blättermann 2012-10-08 18:05:29 UTC
(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.

Comment 12 Parag AN(पराग) 2013-07-22 07:19:03 UTC
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 ***