[Bug 1396232] Review Request: libkeepalive - Enable TCP keepalive in dynamic binaries

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

 



https://bugzilla.redhat.com/show_bug.cgi?id=1396232



--- Comment #3 from Phil Sutter <psutter@xxxxxxxxxx> ---
Hi again,

(In reply to Phil Sutter from comment #2)
> (In reply to Igor Gnatenko from comment #1)
> > * CFLAGS/LDFLAGS are not used
> >   export CFLAGS="%{optflags}"
> >   export LDFLAGS="%{__global_ldflags}"
> >   but it still requires some patching of Makefile `=` -> `?=`
> > * Missing BuildRequires: gcc
> > * You should try to version library (though I'm not sure how much it matters
> > here, probably doesn't at all)

I didn't do this. To my knowledge, versioning shared objects makes sense only
if programs are expected to link against them so it's possible to distinguish
between minor and major upgrades. In this case, the library is meant to be
injected into the program using LD_PRELOAD, so no versioning is required.

> > * echo is not needed in %build
> > * instead of make you should use %make_build (e.g. %make_build -C src)
> > * there are some tests, so try to build / run them
> > * instead of doing mkdir, you can pass `-D` to install
> > 
> > Apart from that, looks good ;)
> 
> Thanks for the elaborate review! I'll try to get things sorted ASAP then
> upload fresh SRPM and spec file.

All fixed apart from the missing versioning, updated spec file and SRPM
available here:

Spec URL: https://psutter.fedorapeople.org/libkeepalive.spec
SRPM URL: https://psutter.fedorapeople.org/libkeepalive-0.3-2.fc25.src.rpm

Thanks, Phil

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




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