Bug 458785

Summary: Review Request: libev - High-performance event loop/event model with lots of features
Product: [Fedora] Fedora Reporter: Michal Nowak <mnowak>
Component: Package ReviewAssignee: Nicolas Chauvet (kwizart) <kwizart>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: cassmodiah, fedora-package-review, kwizart, notting, ohudlick, pertusus
Target Milestone: ---Flags: kwizart: fedora‑review+
tibbs: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-12-02 20:09:52 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 448613, 452427    

Description Michal Nowak 2008-08-12 05:50:30 EDT
Spec URL: http://mnowak.fedorapeople.org/libev/libev.spec
SRPM URL: http://mnowak.fedorapeople.org/libev/libev-3.43-1.fc9.src.rpm
Description:

Libev is modelled (very losely) after libevent and the Event perl
module, but is faster, scales better and is more correct, and also more
featureful. And also smaller.

This is dependency of bug 452427
Comment 1 Michal Nowak 2008-08-12 06:06:24 EDT
[root@dhcp-lab-192 SPECS]# rpmlint libev.spec /usr/src/redhat/SRPMS/libev-3.43-1.fc9.src.rpm /usr/src/redhat/RPMS/i386/libev-3.43-1.fc9.i386.rpm  /usr/src/redhat/RPMS/i386/libev-devel-3.43-1.fc9.i386.rpm /usr/src/redhat/RPMS/i386/libev-debuginfo-3.43-1.fc9.i386.rpm
libev-devel.i386: W: no-documentation
4 packages and 1 specfiles checked; 0 errors, 1 warnings.
Comment 2 Nicolas Chauvet (kwizart) 2008-08-12 09:17:04 EDT
- starting review -
Comment 3 Nicolas Chauvet (kwizart) 2008-08-12 10:08:52 EDT
+ License is GPLv2+ or BSD and fit the package Licensed field
+ The source tarball from the development site and the src.rpm are the same
 6dddea9189345f0c8c36c4eb8ecce441  libev-3.43.tar.gz
+ The spec file is readable
+ Package build in mock (local: F-8 x86_64) and Rawhide for all arches: 
http://koji.fedoraproject.org/koji/taskinfo?taskID=773760
+ Package use our CFLAGS


1/ Please use INSTALL="install -p" at make install step to avoid timstramps changes while installing headers.

2/ Package bundled a static library by default.
If really needed, it would be better to move this library to a -devel-static sub-package. If not, just add --disable-static at the configure step.

3/ There is a name clash with event.h from libevent. This suggest that the headers should be removed from the default include directory (ie  /usr/include) to be provided in a subdirectory by default (ie /usr/include/libev ). So the name clash doesn't occur anymore.
This change would be required for libevent eventually.

4/ Package lack to provide support of pkg-config for libev. It would be fine to have libev.pc for the dependant packages that will use libev.

+ no .pc file present.
+ no .la files.
+ Requires look sane
+ rpmlint on installed pacakge is quiet
+ package install/uninstall went fine

There is 4 points that can be improved.
Comment 4 Michal Nowak 2008-08-13 02:46:36 EDT
Thank for review, Nicolas.


* Mon Aug 12 2008 Michal Nowak <mnowak@redhat.com> - 3.43-2
- removed libev.a
- installing with "-p"
- event.h is removed intentionaly, because is there only for
  backward compatibility with libevent


Spec URL: http://mnowak.fedorapeople.org/libev/libev.spec
SRPM URL: http://mnowak.fedorapeople.org/libev/libev-3.43-2.fc9.src.rpm


Points 1. and 2. fixed.

Point 3: fixed by removing event.h because it's, according to upstream, only for bw compatibility with libevent -> N/A for me, IMHO.

"""
On Tue, Aug 12, 2008 at 12:16:40PM -0400, Michal Nowak <mnowak@redhat.com> wrote:
> Do you thing it could be possible to avoid such conflict on upstream
> basis?

Unlikely, the "conflict" is by design.

> Giving example, to install event.h, ev.h and ev++.h to /usr/include/libev
> by default?

That would break applications that expect to find it as event.h (basically
all libevent applications).

On Tue, Aug 12, 2008 at 01:06:27PM -0400, Michal Nowak <mnowak@redhat.com> wrote:
> ----- "Matt Tolton" <matt@tolton.com> wrote:
> > Why not just leave out event.h?  That's just for libevent
> > compatibility.
>
> Thanks, that was my original decision.

Why not do it like other distributions such as debian, where the common
header files are installed as alternatives, or optionally?

event.h is an alternative to the libevent event.h, it's not an unrelated
header file, it serves the same purpose in both libraries.
"""

(Not yet in mail archive http://lists.schmorp.de/pipermail/libev/)

Point 4:

No interest from upstream, probably because when it's in /usr/lib/ no special magic is necessary. When it's not upstream -> projects won't use it -> useless to have it in Fedora.

What you think?
Comment 5 Michal Nowak 2008-08-18 11:49:13 EDT
Ping?

* http://www.stud.fit.vutbr.cz/~xnowak01/Fedora/libev/libev.spec
* http://www.stud.fit.vutbr.cz/~xnowak01/Fedora/libev/libev-3.43-3.fc9.src.rpm
(temporarily here due to Fedora infrastructure problems)


* Mon Aug 18 2008 Michal Nowak <mnowak@redhat.com> - 3.43-3
- renamed $$RPM_BUILD_ROOT to %%{buildroot}
- commenting .pc file & related
- installing back to /usr/include from /usr/include/%%{name}
Comment 6 Nicolas Chauvet (kwizart) 2008-09-02 09:07:07 EDT
(In reply to comment #4)
> Point 3: fixed by removing event.h because it's, according to upstream, only
> for bw compatibility with libevent -> N/A for me, IMHO.
> 
> """
> On Tue, Aug 12, 2008 at 12:16:40PM -0400, Michal Nowak <mnowak@redhat.com>
> wrote:
> > Do you thing it could be possible to avoid such conflict on upstream
> > basis?
> 
> Unlikely, the "conflict" is by design.
I disagree. AFAIK using a special sub-directory path
> > Giving example, to install event.h, ev.h and ev++.h to /usr/include/libev
> > by default?
> 
> That would break applications that expect to find it as event.h (basically
> all libevent applications).
Thoses can be fixed by choosing which pkgconfig files they will use.
> On Tue, Aug 12, 2008 at 01:06:27PM -0400, Michal Nowak <mnowak@redhat.com>
> wrote:
> > ----- "Matt Tolton" <matt@tolton.com> wrote:
> > > Why not just leave out event.h?  That's just for libevent
> > > compatibility.
> >
> > Thanks, that was my original decision.
> 
> Why not do it like other distributions such as debian, where the common
> header files are installed as alternatives, or optionally?
> 
> event.h is an alternative to the libevent event.h, it's not an unrelated
> header file, it serves the same purpose in both libraries.
> """
event.h from libev and libevent.h are clearly not the same.
if anyone wants to link against libev instead of libevent, the compatibility layer will not be provided by the Fedora package once event.h is removed.

> (Not yet in mail archive http://lists.schmorp.de/pipermail/libev/)
> 
> Point 4:
> 
> No interest from upstream, probably because when it's in /usr/lib/ no special
> magic is necessary. When it's not upstream -> projects won't use it -> useless
> to have it in Fedora.
>
> What you think?
I think pkgconfig support would solve the co-installation problem along with giving a accurate value for linking dependant application. (that can pick which libev.pc/libevent.pc to link with, as a configure option.)
Now if upstream think it is not necessary,then I don't mind. This won't prevent this package to be good for Fedora. Might be important to check rpmlint output on installed files for the applications using libev... (to check for undefined-non-weak-symbol).

But as soon as pkgconfig support would be merged upstream, the dependent apps can be fixed and patches against dependent applications will have a chance to be merged. Once done, there is a chance to avoid problems with libev/libevent -devel been installed on the same system.

So to conclude: I don't want to prevent this package to be approved.
So I just wait to have your answer back.

ps: about your lib.pc , your have picked a bad example. You can see pkgconfig as an abstraction layer over hardcoded configuration path.
quicklook on this:
http://kwizart.fedorapeople.org/SRPMS/libev-3.43-4.fc8.kwizart.src.rpm
Comment 7 Nicolas Chauvet (kwizart) 2008-09-15 09:34:35 EDT
Any news from the previous comment and email ?
Please tell me if you disagree, i don't want to hold the review...
Comment 8 Michal Nowak 2008-09-16 04:22:22 EDT
(In reply to comment #6)
> I disagree. AFAIK using a special sub-directory path

Agree.

> Thoses can be fixed by choosing which pkgconfig files they will use.

Agree.

> event.h from libev and libevent.h are clearly not the same.
> if anyone wants to link against libev instead of libevent, the compatibility
> layer will not be provided by the Fedora package once event.h is removed.

Agree.

Even though I did diff of both, libev's libevent.h and libevent's libeven.h, header files, and have seen they are different, I thought it's just old/new version problem. Didn't even thought it might be evolved one in libev.

> I think pkgconfig support would solve the co-installation problem along with
> giving a accurate value for linking dependant application. (that can pick which
> libev.pc/libevent.pc to link with, as a configure option.)

Sure.

> Now if upstream think it is not necessary,then I don't mind. This won't prevent
> this package to be good for Fedora. 

That's the point.

I am aware pkg-config it the right solution, but upstream showed no interest in merging this (thay said it's "by design").

Let's coordinate on this.

I'll ask upstream to provide pkg-config support. If they denies it what to do next?

A) We remove the libevent.h from libev-devel and install the libev.h (and friends) to %{_includedir}. The positive on this is that we stay on the same way upstream is, and that apps linking against libev.h will know where to find it. The con is that we vaporize possibility to link against libev's libevent.h.


B) We can go our own way, and provide pkg-config support and have libev.h, libevent.h (and friends) in %{_includedir}/%{name} and then we will fix other libev dependent packages (it won't be much of them at the moment).


> Might be important to check rpmlint output
> on installed files for the applications using libev... (to check for
> undefined-non-weak-symbol).

I was not hit by this, not never seen it so far, can you please explain "undefined-non-weak-symbol" to me, or provide some useful link?

> But as soon as pkgconfig support would be merged upstream, the dependent apps
> can be fixed and patches against dependent applications will have a chance to
> be merged. Once done, there is a chance to avoid problems with libev/libevent
> -devel been installed on the same system.
> 
> So to conclude: I don't want to prevent this package to be approved.
> So I just wait to have your answer back.

> ps: about your lib.pc , your have picked a bad example. You can see pkgconfig
> as an abstraction layer over hardcoded configuration path.
> quicklook on this:
> http://kwizart.fedorapeople.org/SRPMS/libev-3.43-4.fc8.kwizart.src.rpm

Thanks for the .pc file in private mail, I'll base the "official" one on this, if we decide we need it/will provide it :-).
Comment 9 Nicolas Chauvet (kwizart) 2008-09-18 12:12:45 EDT
>I'll ask upstream to provide pkg-config support. If they denies it what to do next?
Please attach me in copy of the email (or point us to the mail list).
Comment 10 Nicolas Chauvet (kwizart) 2008-10-01 12:24:51 EDT
I haven't received any email from upstream.
Again i don't want to block this review... What can we do ?
Comment 12 Nicolas Chauvet (kwizart) 2008-10-14 04:46:29 EDT
Package has built fine in Rawhide:
http://koji.fedoraproject.org/koji/taskinfo?taskID=879581
Package built fine in local workstation
rpmlint on installed file is quiet
Source from src.rpm and upstream matches
  f47a18735d3a6ebe4cc113c107e657f250e135da  libev-3.44.tar.gz
  429297 sep 29 05:30 libev-3.44.tar.gz
Package is multilibs compliant.
OK


---------------

This package (libev) is APPROVED by me

---------------
Comment 13 Michal Nowak 2008-10-19 13:30:30 EDT
New Package CVS Request
=======================
Package Name: libev
Short Description: High-performance event loop/event model with lots of features
Owners: mnowak
Branches: F-8 F-9 F-10
InitialCC: mnowak
Comment 14 Kevin Fenzi 2008-10-19 18:45:28 EDT
cvs done.
Comment 15 Nicolas Chauvet (kwizart) 2008-10-27 22:24:03 EDT
This package doesn't seems imported - Why ?
perl-EV has been updated to 3.45, does libev plans to be updated ?
(upstream website is down currently).
Comment 16 Fedora Update System 2008-11-09 08:32:02 EST
libev-3.48-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/libev-3.48-1.fc10
Comment 17 Fedora Update System 2008-11-09 08:33:09 EST
libev-3.48-1.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/libev-3.48-1.fc9
Comment 18 Fedora Update System 2008-11-09 08:34:09 EST
libev-3.48-1.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/libev-3.48-1.fc8
Comment 19 Fedora Update System 2008-11-11 21:59:05 EST
libev-3.48-1.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update libev'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-9594
Comment 20 Fedora Update System 2008-11-11 22:00:02 EST
libev-3.48-1.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update libev'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F8/FEDORA-2008-9598
Comment 21 Fedora Update System 2008-11-22 11:54:24 EST
libev-3.48-1.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update libev'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/f10/FEDORA-2008-10000
Comment 22 Fedora Update System 2008-12-02 20:09:49 EST
libev-3.48-1.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 23 Fedora Update System 2008-12-02 20:26:32 EST
libev-3.48-1.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 24 Fedora Update System 2008-12-02 20:30:27 EST
libev-3.48-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 25 Simon 2010-07-01 06:50:21 EDT
Package Change Request
======================
Package Name: libev
New Branches: EL-6
Owners: cassmodiah

quote from a friendly colloquy via mail with Michal:
<<<>>>
I am not interested in maintaining libev in RHEL-6, feel free to take
ownership for this branch.

Michal
<<<>>>
Comment 26 Jason Tibbitts 2010-07-01 13:33:04 EDT
CVS done (by process-cvs-requests.py).