Fedora Merge Review: libwvstreams http://cvs.fedora.redhat.com/viewcvs/devel/libwvstreams/ Initial Owner: harald
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