[Bug 889505] New: Review Request: libkqueue - A userspace implementation of the kqueue kernel event notification mechanism

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=889505

            Bug ID: 889505
           Summary: Review Request: libkqueue - A userspace implementation
                    of the kqueue kernel event notification mechanism
           Product: Fedora
           Version: rawhide
         Component: Package Review
          Severity: medium
          Priority: unspecified
          Reporter: eradman@xxxxxxxxxxxxxxx

+++ This bug was initially created as a clone of Bug #684446 +++

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

--- Additional comment from Rahul Sundaram on 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

--- Additional comment from Rahul Sundaram on 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

--- Additional comment from Aditya Patawari on 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

--- Additional comment from Rahul Sundaram on 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.

--- Additional comment from Michael Schwendt on 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

--- Additional comment from John Haxby on 2012-02-04 15:46:36 EST ---

Created attachment 559432
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.

--- Additional comment from John Haxby on 2012-02-04 15:50:23 EST ---

Created attachment 559433
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.

--- Additional comment from John Haxby on 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?

--- Additional comment from Mario Blättermann on 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?

--- Additional comment from Paul Wouters on 2012-09-28 19:00:26 EDT ---

I'm interested to get this package in, as I need it as a dependancy :)

--- Additional comment from Mario Blättermann on 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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=Irb8gBBdgs&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]