Bug 493750

* Your home page points to a changed "URL: https://fedorahosted.org/netcf/"


* Better summary: Cross-platform network configuration library
(taken from README!)


* Fedora 10 build needs augeas >= 0.5.0 from updates-testing


* Build errors on Fedora 10:
[...]
checking for LIBAUGEAS... yes
checking for LIBXML... 
[...]

 => Missing BuildRequires: libxml2-devel


checking for LIBXSLT... 

 => Missing BuildRequires: libxslt-devel
(this requires libxml2-devel, too)


* In "src" directory, NETCF_CFLAGS duplicates the CFLAGS.


* netcf.pc  file is missing "Requires: augeas" because it contains "-laugeas".

> * netcf.pc  file is missing "Requires: augeas" because it
> contains "-laugeas".

Since libnetcf is linked with libaugeas already, it would be even better to drop "-laugeas" from Libs. 

Else, for static linking support in upstream tarball, add "Requires.private: augeas" to the pkgconfig file, move "-laugeas" from Libs to Libs.private, and add "Requires: augeas-devel >= 0.5.0" to the netcf-devel subpackage (for Fedora <= 10 which doesn't create automatic pkgconfig dependencies).

Sorry for the lag, somehow your review got filed into the wrong mail folder.

(In reply to comment #1)
> * Your home page points to a changed "URL: https://fedorahosted.org/netcf/"

Fixed.

> * Better summary: Cross-platform network configuration library
> (taken from README!)

Thanks; indeed much better.

> * Fedora 10 build needs augeas >= 0.5.0 from updates-testing

Yeah, it just got pushed to updates

> * Build errors on Fedora 10:
> [...]
> checking for LIBAUGEAS... yes
> checking for LIBXML... 
> [...]
> 
>  => Missing BuildRequires: libxml2-devel
> 
> 
> checking for LIBXSLT... 
> 
>  => Missing BuildRequires: libxslt-devel
> (this requires libxml2-devel, too)

Added those BR's

> * In "src" directory, NETCF_CFLAGS duplicates the CFLAGS.

I don't see that - do you mean the duplication of -I/usr/include/libxml2 ? That comes from using cflags from both libxml2 and libxslt; though since netcf uses API's from both, i.e. it doesn't just use libxml2 via libxslt, I'd rather leave the explicit LIBXML_CFLAGS in the Makefile.

> * netcf.pc  file is missing "Requires: augeas" because it contains "-laugeas".  

Ok, I am adding a Requires for augeas, libxslt, and libxml-2.0 to netcf.pc and taking out the -laugeas.

I'll post an updated spec/srpm in a minute.

Updated SRPM/spec:

SRPM: http://people.redhat.com/dlutter/netcf/download/netcf-0.0.2-1.fc10.src.rpm
Spec: http://people.redhat.com/dlutter/netcf/download/netcf.spec

Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1300743

Okay, packaging-wise the 0.0.2-1.fc10 src.rpm is fine:

  APPROVED

[...]

The remaining minor issues are within your upstream tarball:

* Duplicated CFLAGS. Quoting from:
http://koji.fedoraproject.org/koji/getfile?taskID=1300747&name=build.log

>>>
make[3]: Entering directory `/builddir/build/BUILD/netcf-0.0.2/src'
/bin/sh ../libtool --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. -I..    --std=gnu99 -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fasynchronous-unwind-tables -I ../gnulib/lib -Wall -Wformat -Wformat-security -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wextra -Wshadow -Wcast-align -Wwrite-strings -Waggregate-return -Wstrict-prototypes -Winline -Wredundant-decls -Wno-sign-compare -I/usr/include/libxml2   -I/usr/include/libxml2   -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i586 -mtune=generic -fasynchronous-unwind-tables -MT netcf.lo -MD -MP -MF .deps/netcf.Tpo -c -o netcf.lo netcf.c
<<<

Actually, it's _not_ duplicated CFLAGS as it seemed first, but that you AC_SUBST default NETCF_CFLAGS in the configure script (precisely: -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fasynchronous-unwind-tables), which are added a second time with Fedora's global optflags. I've expected Fedora's CFLAGS to override the defaults except for a hardcoded -fexceptions (which you want for interoperability with netcf API users written in C++).


* pkgconfig file issues. The result of adding the "Requires" to the .pc file is this:

$ pkg-config --cflags netcf
-I/usr/include/libxml2

$ pkg-config --libs netcf
-lnetcf -laugeas -lxslt -lz -lm -lxml2  

This is not needed for shared linking (the default with Fedora), because you don't need all those libs and headers when building with the libnetcf API. You only need them when linking statically (comment 2), and then you would prefer "Requires.private" and "Libs.private" and "pkg-config --static ...". The current pkgconfig inter-package dependencies lead to dependency-bloat.

And similar to my earlier comment 2, on Fedora <= 10, where there are no automatic pkgconfig RPM dependencies added by rpmbuild, the netcf.pc file's current dependencies lead to missing RPM Requires in netcf-devel (for the needed -devel pkgs). So, really better switch to "Requires.private" and "Libs.private".

I'll fix up the pkg-config file to use Requires.private in the next release.

As for the duplicate CFLAGS, I would realy like to keep NETCF_CFLAGS for development builds, and even if I build with CFLAGS then.

Thanks for the thorough review

New Package CVS Request
=======================
Package Name: netcf
Short Description: Cross-platform network configuration library
Owners: lutter
Branches: F-10 F-11 EL-5
InitialCC:

cvs done.