Bug 226061

considered OK
=============
- silent rpmlint
- sane summary/description
- valid project URL and source URL
- upstream tarball is the same as the one from look-aside cache
- md5 hash matches the sources
- dist tag OK
- license tag OK
- BuildRoot tag OK
- configure options OK
- %clean OK
- %files OK
- %defattr OK
- %doc OK


may be better
=============
- duplicated BuildRequires for pkgconfig (already required by openssl-devel)
- patch wvstreams-4.5-noxplctarget.patch has no link to the upstream thread
- make LIBS_DBUS=/%{_lib}/libdbus-1.so deserves a comment
- make should either use %{?_smp_mflags} or there should be a comment why the parallel build does not work

- -fpermissive should not appear in COPTS:
cc1: warning: command line option "-fpermissive" is valid for C++/ObjC++ but not for C

Thanks for review...

I would prefer to keep BuildRequires for pkgconfig - although it is almost sure it will be required in future by openssl-devel, someone may modify the spec file, throw away with-openssl configure option and openssl-devel requirement - and missing dependency will appear. It is harmless as it (slightly) affects only build speed/dependency resolving...

Patch is missing link to upstream thread as it probably was not reported, it seems I forgot to do that. I'll do that tomorrow.

make LIBS_DBUS=/%{_lib}/libdbus-1.so commented - upstream searches for .a library and and without hardcoding LIBS_DBUS it caused build failures.

parallel build will be used, "-fpermissive" from COPTS thrown away.

Commited to CVS, building as libwvstreams-4.6.1-2.fc13 .

(In reply to comment #2)
> I would prefer to keep BuildRequires for pkgconfig

fine by me

> make LIBS_DBUS=/%{_lib}/libdbus-1.so commented - upstream searches for .a
> library and and without hardcoding LIBS_DBUS it caused build failures.

great, the added comment makes it more obvious

> Commited to CVS, building as libwvstreams-4.6.1-2.fc13 .    

I'll check it tomorrow if nothing urgent happens.

Created attachment 383429 [details]
failure of parallel build

I realized that adding %{?_smp_mflags} wasn't the best idea.  Perhaps I should have tested it myself, before writing the review :-)  Attached is the build log from my 8-core box:

$ rpmbuild --rebuild libwvstreams-4.6.1-2.fc13.src.rpm 2>&1 | tee build.log

I think the change should be reverted and commented in the specfile and maybe reported upstream if the fix is non-trivial.  I haven't investigated further.

Created attachment 383456 [details]
patching making it possible to build in parallel

Comment on attachment 383456 [details]
patching making it possible to build in parallel

looks sane...

Created attachment 383497 [details]
patch making it possible to build in parallel (V2)

add libwvdbus.so to the list of dependencies when building with dbus support

Thanks Kamil. Finally built as libwvstreams-4.6.1-2.fc13.

fedora‑review+, package is sane ... this bug may be closed

Ok, closing... thanks for review.

Comment on attachment 383497 [details]
patch making it possible to build in parallel (V2)

patch proposed upstream:

http://groups.google.com/group/wvstreams-devel/browse_thread/thread/a76875e0c904f04a