Bug 684446 - Review Request: libkqueue - A userspace implementation of the kqueue kernel event notification mechanism
Review Request: libkqueue - A userspace implementation of the kqueue kernel e...
Status: CLOSED DUPLICATE of bug 889505
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Paul Wouters
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-12 12:09 EST by Aditya Patawari
Modified: 2013-07-22 03:19 EDT (History)
8 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 889505 (view as bug list)
Environment:
Last Closed: 2013-07-22 03:19:03 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Updated .spec file (1.41 KB, text/plain)
2012-02-04 15:46 EST, John Haxby
no flags Details
Compilation errors patch (964 bytes, patch)
2012-02-04 15:50 EST, John Haxby
no flags Details | Diff

  None (edit)
Description Aditya Patawari 2011-03-12 12:09:08 EST
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 14:04:50 EST
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
Comment 2 Rahul Sundaram 2011-03-14 03:36:02 EDT
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 11:40:10 EDT
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 15:10:35 EDT
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 04:08:42 EDT
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
Comment 6 John Haxby 2012-02-04 15:46:36 EST
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 15:50:23 EST
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 15:51:21 EST
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 15:37:35 EDT
@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 19:00:26 EDT
I'm interested to get this package in, as I need it as a dependancy :)
Comment 11 Mario Blättermann 2012-10-08 14:05:29 EDT
(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 03:19:03 EDT
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 ***

Note You need to log in before you can comment on or make changes to this bug.