Bug 754975

Just a brief look at the spec file:


> Summary:	Pion Network Library

Please sum up what it does not what it is called. For example:

Sumamry: C++ library for building lightweight HTTP interfaces


> %package	devel
> Summary:	Development files for pion-net
> Requires:	pion-net = %{version}-%{release}

https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package


> %package	doc
> Summary:	Documentation for pion-net
> Requires:	pion-net-devel = %{version}-%{release}

It's a bad habit to let documentation depend on -devel packages. It makes it more difficult (if not impossible) to create a collection of documentation packages, because they require growing -devel trees to be installable. Surely one can read the docs without installing the -devel files.


> %install
> rm -rf %{buildroot}

https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag


> %files
> %doc AUTHORS COPYING NEWS TODO
> %{_libdir}/libpion-*.so
> %{_libdir}/pion/plugins/*.so

https://fedoraproject.org/wiki/Packaging:UnownedDirectories

(In reply to comment #1)
> Just a brief look at the spec file:

Thank you for catching all these -- everything is fixed.

Updated spec URL:
http://jvcelak.fedorapeople.org/review/pion-net/4.0.7-2/pion-net.spec

Updated SRPM URL:
http://jvcelak.fedorapeople.org/review/pion-net/pion-net-4.0.7-2.fc17.src.rpm

* F-16 and F-15 builds complete for both x86_64 and i686.


* HTML documentation is also included in the base package. This is because the %doc macro for the base package installs the files in %_docdir/%name-%version, which is exactly the path you use in the -doc subpackage. In the same way, the -doc subpkg duplicates the %doc files from the base pkg %files list.


* -devel package is missing dependencies. In the C++ headers I see references to Boost headers, OpenSSL, log4cpp. It's likely one cannot include pion-net headers before installing missing -devel packages.


* The pkg-config file is broken in several ways:
 - on x86_64, it includes a hardcoded -L/usr/lib
 - its Libs definition duplicates the linker args that have been used to link pion-net itself, not what is necessary to link with pion-net
 - the Cflags line needs a second look, in particular where the hardcoded BOOST_FILESYSTEM_VERSION definition comes from (version 2 is deprecated)
 - $datarootdir and $datadir are undefined/unexpanded


* Source doesn't use Fedora's global compiler %optflags.

Update spec URL:
http://jvcelak.fedorapeople.org/review/pion-net/4.0.7-3/pion-net.spec

Updated SRPM url:
http://jvcelak.fedorapeople.org/review/pion-net/pion-net-4.0.7-3.fc17.src.rpm

(In reply to comment #3)
> * HTML documentation is also included in the base package. This is because the
> %doc macro for the base package installs the files in %_docdir/%name-%version,
> which is exactly the path you use in the -doc subpackage. In the same way, the
> -doc subpkg duplicates the %doc files from the base pkg %files list.

Resolved. Documentation is now in %{_docdir}/pion-net-doc-%{version}

> * -devel package is missing dependencies. In the C++ headers I see references
> to Boost headers, OpenSSL, log4cpp. It's likely one cannot include pion-net
> headers before installing missing -devel packages.

Resolved. Added the same -devel libs.

> * The pkg-config file is broken in several ways:
>  - on x86_64, it includes a hardcoded -L/usr/lib
>  - its Libs definition duplicates the linker args that have been used to link
> pion-net itself, not what is necessary to link with pion-net
>  - the Cflags line needs a second look, in particular where the hardcoded

I'm not a pkg-config guru. I have updated the .pc file to include only pion-net libraries in Libs. And added Libs.private to include the external libraries. I hope it is correct.

> BOOST_FILESYSTEM_VERSION definition comes from (version 2 is deprecated)
>  - $datarootdir and $datadir are undefined/unexpanded

That's right. The library is not compatible with new boost::filesystem. But I updated the affected parts, there were only a few. The updated package links against boost::filesystems v3 successfully.

> * Source doesn't use Fedora's global compiler %optflags.

Resolved, I patched the autoconf files not to clear the flags.

I found another issues, which were fixed as well:
- A typo in configure option (--with-pic).
- libicu was hardcoded in autoconf. I removed it and libicu-devel in BuildRequires is no longer needed.
- Unversioned .so files were moved into the -devel package.

(In reply to comment #3)
>

And thank you, Michael!

Michael, are you going to review this package? Or should I offer someone a review swap?

[pkg-config file changes]

> I hope it is correct.

Yes, it makes sense to move the external libs into Libs.private. Just that at Fedora we try to avoid static linking, so it might be that some of the libs in Libs.private are not available as -static packages.


> - A typo in configure option (--with-pic).

:)  Didn't make a difference, though. The default pic mode also compiled with -fPIC -DPIC.


> - libicu was hardcoded in autoconf. I removed it and libicu-devel in
> BuildRequires is no longer needed.

Confirmed. BuildRequires now match the list at
http://www.pion.org/projects/pion-network-library/libraries
however zlib and bzip2 dependency is a mystery, too. They seem to be unused but are linked with.

$ grep zlib * -R|grep '#include'
common/build/pion-config.inc:	AC_TRY_LINK([#include <zlib.h>],[ zlibVersion(); return(0); ],
common/build/pion-config.inc:	AC_TRY_LINK([#include <bzlib.h>],[ BZ2_bzlibVersion(); return(0); ],
configure:#include <zlib.h>
configure:#include <bzlib.h>


> - Unversioned .so files were moved into the -devel package.

Confirmed.


* Exlicit -devel Requires could be arch-specific using %{?_isa}.


* Upstream download section offers a shorter .tar.bz2 source archive.


* APPROVED

(In reply to comment #7)
> * APPROVED

Thank you.

New Package SCM Request
=======================
Package Name: pion-net
Short Description: C++ library for building lightweight HTTP interfaces
Owners: jvcelak
Branches: f15 f16
InitialCC:

Git done (by process-git-requests).

pion-net-4.0.7-4.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/pion-net-4.0.7-4.fc15

pion-net-4.0.7-4.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/pion-net-4.0.7-4.fc16

pion-net-4.0.7-4.fc15 has been pushed to the Fedora 15 stable repository.

pion-net-4.0.7-4.fc16 has been pushed to the Fedora 16 stable repository.