Spec URL: http://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip.spec SRPM URL: http://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip-1.2.4-1.fc20.src.rpm Description: Belle-sip is an object oriented c written SIP stack used by Linphone. Fedora Account System Username: nucleo Required for Linphone 3.7.0.
$ rpmlint belle-sip-1.2.4-1.fc20.i686.rpm belle-sip-1.2.4-1.fc20.src.rpm belle-sip-debuginfo-1.2.4-1.fc20.i686.rpm belle-sip-devel-1.2.4-1.fc20.i686.rpm belle-sip.spec belle-sip.i686: W: spelling-error Summary(en_US) Linphone -> Lin phone, Lin-phone belle-sip.i686: E: incorrect-fsf-address /usr/share/doc/belle-sip/COPYING belle-sip.src: W: spelling-error Summary(en_US) Linphone -> Lin phone, Lin-phone belle-sip-devel.i686: W: no-documentation 4 packages and 1 specfiles checked; 1 errors, 3 warnings.
Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6376909
1. In -devel, should be: Requires: %{name}%{?_isa} = %{version}-%{release} 2. autoreconf -i -f --> autoreconf -fiv 3. Hint: find %{buildroot} -name '*.la' -exec rm -f {} ';' --> find %{buildroot} -name '*.la' -delete 4. From your koji log: CC libbellesip_la-clock_gettime.lo CC libbellesip_la-port.lo CC libbellesip_la-belle_sip_headers_impl.lo CC libbellesip_la-belle_sip_uri_impl.lo CC libbellesip_la-belle_sip_utils.lo CC libbellesip_la-belle_sip_object.lo CC libbellesip_la-belle_sip_loop.lo CC libbellesip_la-belle_sip_resolver.lo CC libbellesip_la-belle_sip_parameters.lo CC libbellesip_la-belle_sdp_impl.lo CC libbellesip_la-transaction.lo CC libbellesip_la-listeningpoint.lo CC libbellesip_la-sipstack.lo CC libbellesip_la-provider.lo CC libbellesip_la-channel.lo CC libbellesip_la-message.lo CC libbellesip_la-md5.lo CC libbellesip_la-auth_helper.lo CC libbellesip_la-siplistener.lo CC libbellesip_la-ict.lo CC libbellesip_la-ist.lo CC libbellesip_la-nict.lo CC libbellesip_la-nist.lo CC libbellesip_la-dialog.lo CC libbellesip_la-auth_event.lo CC libbellesip_la-udp_listeningpoint.lo CC libbellesip_la-udp_channel.lo CC libbellesip_la-stream_channel.lo CC libbellesip_la-stream_listeningpoint.lo CC libbellesip_la-refresher.lo CC libbellesip_la-dns.lo CC libbellesip_la-belle_sip_dict.lo CC libbellesip_generated_la-belle_sip_messageParser.lo CC libbellesip_generated_la-belle_sip_messageLexer.lo CC libbellesip_generated_la-belle_sdpParser.lo Well, it's better to not use silent build so we can find if there has any problem during building, thus appending "--disable-silent-rules" to %configure is needed. 5. Notify upstream about the rpmlint error: E: incorrect-fsf-address /usr/share/doc/belle-sip/COPYING 6. Licensecheck: BSD (3 clause) -------------- belle-sip-1.2.4/src/clock_gettime.c GPL (v2 or later) ----------------- belle-sip-1.2.4/include/belle-sip/auth-helper.h belle-sip-1.2.4/include/belle-sip/belle-sdp.h belle-sip-1.2.4/include/belle-sip/dialog.h belle-sip-1.2.4/include/belle-sip/list.h belle-sip-1.2.4/include/belle-sip/listeningpoint.h belle-sip-1.2.4/include/belle-sip/mainloop.h belle-sip-1.2.4/include/belle-sip/refresher.h belle-sip-1.2.4/ltmain.sh belle-sip-1.2.4/src/auth_helper.c belle-sip-1.2.4/src/belle_sdp_impl.c belle-sip-1.2.4/src/belle_sip_dict.c belle-sip-1.2.4/src/belle_sip_utils.c belle-sip-1.2.4/src/clock_gettime.h belle-sip-1.2.4/src/port.c belle-sip-1.2.4/src/siplistener.c belle-sip-1.2.4/src/transports/stream_channel.h belle-sip-1.2.4/src/transports/tunnel_channel.c belle-sip-1.2.4/src/transports/tunnel_listeningpoint.c belle-sip-1.2.4/src/transports/tunnel_wrapper.cc belle-sip-1.2.4/tester/auth_helper_tester.c belle-sip-1.2.4/tester/belle_sdp_tester.c belle-sip-1.2.4/tester/belle_sip_dialog_tester.c belle-sip-1.2.4/tester/belle_sip_headers_tester.c belle-sip-1.2.4/tester/belle_sip_message_tester.c belle-sip-1.2.4/tester/belle_sip_refresher_tester.c belle-sip-1.2.4/tester/belle_sip_tester.c belle-sip-1.2.4/tester/belle_sip_tester.h belle-sip-1.2.4/tester/belle_sip_uri_tester.c belle-sip-1.2.4/tester/cast_test.c belle-sip-1.2.4/tester/parse.c GPL (v3 or later) ----------------- belle-sip-1.2.4/include/belle-sip/belle-sip.h belle-sip-1.2.4/include/belle-sip/defs.h belle-sip-1.2.4/include/belle-sip/dict.h belle-sip-1.2.4/include/belle-sip/headers.h belle-sip-1.2.4/include/belle-sip/listener.h belle-sip-1.2.4/include/belle-sip/message.h belle-sip-1.2.4/include/belle-sip/object.h belle-sip-1.2.4/include/belle-sip/parameters.h belle-sip-1.2.4/include/belle-sip/provider.h belle-sip-1.2.4/include/belle-sip/resolver.h belle-sip-1.2.4/include/belle-sip/sipstack.h belle-sip-1.2.4/include/belle-sip/transaction.h belle-sip-1.2.4/include/belle-sip/utils.h belle-sip-1.2.4/src/auth_event.c belle-sip-1.2.4/src/belle_sip_headers_impl.c belle-sip-1.2.4/src/belle_sip_internal.h belle-sip-1.2.4/src/belle_sip_loop.c belle-sip-1.2.4/src/belle_sip_object.c belle-sip-1.2.4/src/belle_sip_parameters.c belle-sip-1.2.4/src/belle_sip_resolver.c belle-sip-1.2.4/src/belle_sip_uri_impl.c belle-sip-1.2.4/src/channel.c belle-sip-1.2.4/src/channel.h belle-sip-1.2.4/src/dialog.c belle-sip-1.2.4/src/ict.c belle-sip-1.2.4/src/ist.c belle-sip-1.2.4/src/listeningpoint.c belle-sip-1.2.4/src/listeningpoint_internal.h belle-sip-1.2.4/src/message.c belle-sip-1.2.4/src/nict.c belle-sip-1.2.4/src/nist.c belle-sip-1.2.4/src/port.h belle-sip-1.2.4/src/provider.c belle-sip-1.2.4/src/refresher.c belle-sip-1.2.4/src/sipstack.c belle-sip-1.2.4/src/transaction.c belle-sip-1.2.4/src/transports/stream_channel.c belle-sip-1.2.4/src/transports/stream_listeningpoint.c belle-sip-1.2.4/src/transports/tls_channel_polarssl.c belle-sip-1.2.4/src/transports/tls_listeningpoint_polarssl.c belle-sip-1.2.4/src/transports/udp_channel.c belle-sip-1.2.4/src/transports/udp_listeningpoint.c belle-sip-1.2.4/tester/belle_sip_core_tester.c belle-sip-1.2.4/tester/belle_sip_register_tester.c belle-sip-1.2.4/tester/belle_sip_resolver_tester.c belle-sip-1.2.4/tester/describe.c MIT/X11 (BSD like) ------------------ belle-sip-1.2.4/src/dns.c belle-sip-1.2.4/src/dns.h Unknown or generated -------------------- belle-sip-1.2.4/include/belle-sip/uri.h belle-sip-1.2.4/tester/register_tester.h zlib/libpng ----------- belle-sip-1.2.4/src/md5.c belle-sip-1.2.4/src/md5.h It has bundled code. --------------- Small request, is it possible of you to change your name in Bugzilla from nucleo to your real name ;) ?
- add %%{?_isa} in -devel Requires - add verbose option to autoreconf - verbose build output Spec URL: http://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip.spec SRPM URL: http://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip-1.2.4-2.fc20.src.rpm Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6391276 I sent email about COPYING to linphone-users list.
Please feedback the results later here. Thanks.
(In reply to Christopher Meng from comment #5) > Please feedback the results later here. > > Thanks. You consider license file problem as blocker? We never get feedback from Linphone developers, they never replied on previous emails with compilation and other problems, patches.
Not a blocker. But I'm afraid there might have some questionable things. Have you tried linphone-developers list? BTW I blocked FE-Legal, hope Tom can give us some explanation.
(In reply to Christopher Meng from comment #7) > Not a blocker. But I'm afraid there might have some questionable things. > > Have you tried linphone-developers list? No, because they require signing contributor agreement for this,
They changed in git license headers to GPLv2+ in all sources where it was GPLv2+ but FSF address still incorrect. I can fix address it in spec with sed sed -i s/'59 Temple Place, Suite 330, Boston, MA 02111-1307 USA'/'51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA'/ COPYING
GPLv3+ in sources was mistake or typo, so I changed License field to GPLv2+. - License: GPLv2+ - fix FSF address in COPYING Spec URL: http://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip.spec SRPM URL: http://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip-1.2.4-3.fc20.src.rpm Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6423030 $ rpmlint belle-sip-1.2.4-3.fc20.i686.rpm belle-sip-1.2.4-3.fc20.src.rpm belle-sip-debuginfo-1.2.4-3.fc20.i686.rpm belle-sip-devel-1.2.4-3.fc20.i686.rpm belle-sip.i686: W: spelling-error Summary(en_US) Linphone -> Lin phone, Lin-phone belle-sip.src: W: spelling-error Summary(en_US) Linphone -> Lin phone, Lin-phone belle-sip-devel.i686: W: no-documentation 4 packages and 0 specfiles checked; 0 errors, 3 warnings.
https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address
Upstream informed. I can revert sed if review will be finished after this.
How can you explain other BSD/zlib sources? BSD (3 clause) -------------- belle-sip-1.2.4/src/clock_gettime.c MIT/X11 (BSD like) ------------------ belle-sip-1.2.4/src/dns.c belle-sip-1.2.4/src/dns.h Unknown or generated -------------------- belle-sip-1.2.4/include/belle-sip/uri.h belle-sip-1.2.4/tester/register_tester.h zlib/libpng ----------- belle-sip-1.2.4/src/md5.c belle-sip-1.2.4/src/md5.h ******************* Also for the md5 part you should check: https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
Fwiw, GPLv2+ + BSD/MIT = GPLv2+, pretty sure the same can be said of libpng/zlib too
(In reply to Christopher Meng from comment #13) > How can you explain other BSD/zlib sources? Have no any idea about this, no reply on my two emails sent. > Also for the md5 part you should check: > > https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries So this is blocker? If so I don't know how to fix this. Linphone 3.7.0 released (and belle-sip-1.3.0) http://lists.nongnu.org/archive/html/linphone-users/2014-02/msg00036.html but without belle-sip update impossible. Spec URL: http://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip.spec SRPM URL: http://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip-1.3.0-1.fc20.src.rpm - belle-sip-1.3.0 - revert fix FSF address in COPYING Compilation of 1.3.0 failed with error: /usr/include/features.h:145:3: error: #warning "_BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE" [-Werror=cpp] http://koji.fedoraproject.org/koji/taskinfo?taskID=6554670
Can you read carefully: https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#cite_note-1 See "cite-note"
(In reply to Christopher Meng from comment #16) > Can you read carefully: > > https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#cite_note-1 > > See "cite-note" Can you describe what should be done?
(In reply to nucleo from comment #17) > Can you describe what should be done? add: # The version is used from src/md5.c line: # /* $Id: md5.c,v 1.6 2002/04/13 19:20:28 lpd Exp $ */ Provides: bundled(md5-deutsch) = 1.6 (In reply to Christopher Meng from comment #13) > How can you explain other BSD/zlib sources? use: License: GPLv2+ and BSD and BSD with advertising and MIT > Unknown or generated > -------------------- > belle-sip-1.2.4/include/belle-sip/uri.h - no longer exists > belle-sip-1.2.4/tester/register_tester.h - belle-sip-1.3.0 has it GPLv2+ marked already > zlib/libpng > ----------- > belle-sip-1.2.4/src/md5.c > belle-sip-1.2.4/src/md5.h Why do you think so? I would say just BSD.
(In reply to Jan Kratochvil from comment #18) > (In reply to nucleo from comment #17) > > Can you describe what should be done? > > add: > # The version is used from src/md5.c line: > # /* $Id: md5.c,v 1.6 2002/04/13 19:20:28 lpd Exp $ */ > Provides: bundled(md5-deutsch) = 1.6 Thanks, finally have a smart people. > (In reply to Christopher Meng from comment #13) > > How can you explain other BSD/zlib sources? > > use: > License: GPLv2+ and BSD and BSD with advertising and MIT > > > > Unknown or generated > > -------------------- > > belle-sip-1.2.4/include/belle-sip/uri.h > - no longer exists > > belle-sip-1.2.4/tester/register_tester.h > - belle-sip-1.3.0 has it GPLv2+ marked already > > > > zlib/libpng > > ----------- > > belle-sip-1.2.4/src/md5.c > > belle-sip-1.2.4/src/md5.h > > Why do you think so? I would say just BSD. As Rex has pointed out, mark this package as GPLv2+ should be OK. The rest are noted in Provides of the bundled lib. Please fix the spec. Finally before the approval, please explain why you disable the tests. If no reason could be given, enable it in the %check.
Can someone help fix compilation error from comment 15?
Well... _DEFAULT_SOURCE switch is bad IMO... From recent glibc I mean. You can escape that by dropping -Werror=cpp in the cflags.
(In reply to Christopher Meng from comment #21) > Well... > > _DEFAULT_SOURCE switch is bad IMO... From recent glibc I mean. > > You can escape that by dropping -Werror=cpp in the cflags. This is the simplest way. Or you can hack the code as well.
Status check here?
Help with fixing compilation error needed.
Your compilation errors are due to antlr-C being at 3.5 in Fedora. belle-sip doesn't support antlr revisions other than 3.2 and 3.4, neither of which are completely packaged in Fedora anymore. This one isn't going to be easy to work around.
I added antlr3.jar version 3.4 to SRPM sources for build time use only. - belle-sip-1.3.3 - added atlr3 3.4 SOURCE1 - License: GPLv2+ and BSD and BSD with advertising and MIT - Provides: bundled(md5-deutsch) = 1.6 Spec URL: https://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip.spec SRPM URL: https://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip-1.3.3-1.fc23.src.rpm Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=8938157
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated Issues: ======= - Sources used to build the package match the upstream source, as provided in the spec URL. Note: Upstream MD5sum check error, diff is in /home/cawk/Downloads/belle- sip/diff.txt See: http://fedoraproject.org/wiki/Packaging/SourceURL ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Header files in -devel subpackage, if present. [x]: ldconfig called in %post and %postun if required. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. [x]: Development (unversioned) .so files in -devel subpackage, if present. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "GPL (v2 or later)", "GPL (v3 or later)", "Unknown or generated", "MIT/X11 (BSD like)", "zlib/libpng", "BSD (3 clause)". 14 files have unknown license. Detailed output of licensecheck: BSD (3 clause) -------------- belle-sip-1.3.3/src/clock_gettime.c GPL (v2 or later) ----------------- belle-sip-1.3.3/include/belle-sip/auth-helper.h belle-sip-1.3.3/include/belle-sip/belle-sdp.h belle-sip-1.3.3/include/belle-sip/belle-sip.h belle-sip-1.3.3/include/belle-sip/defs.h belle-sip-1.3.3/include/belle-sip/dialog.h belle-sip-1.3.3/include/belle-sip/dict.h belle-sip-1.3.3/include/belle-sip/headers.h belle-sip-1.3.3/include/belle-sip/list.h belle-sip-1.3.3/include/belle-sip/listener.h belle-sip-1.3.3/include/belle-sip/listeningpoint.h belle-sip-1.3.3/include/belle-sip/mainloop.h belle-sip-1.3.3/include/belle-sip/message.h belle-sip-1.3.3/include/belle-sip/object.h belle-sip-1.3.3/include/belle-sip/parameters.h belle-sip-1.3.3/include/belle-sip/provider.h belle-sip-1.3.3/include/belle-sip/refresher.h belle-sip-1.3.3/include/belle-sip/resolver.h belle-sip-1.3.3/include/belle-sip/sip-uri.h belle-sip-1.3.3/include/belle-sip/sipstack.h belle-sip-1.3.3/include/belle-sip/transaction.h belle-sip-1.3.3/include/belle-sip/utils.h belle-sip-1.3.3/src/auth_event.c belle-sip-1.3.3/src/auth_helper.c belle-sip-1.3.3/src/backgroundtask.m belle-sip-1.3.3/src/belle_sdp_impl.c belle-sip-1.3.3/src/belle_sip_dict.c belle-sip-1.3.3/src/belle_sip_headers_impl.c belle-sip-1.3.3/src/belle_sip_internal.h belle-sip-1.3.3/src/belle_sip_loop.c belle-sip-1.3.3/src/belle_sip_object.c belle-sip-1.3.3/src/belle_sip_parameters.c belle-sip-1.3.3/src/belle_sip_resolver.c belle-sip-1.3.3/src/belle_sip_uri_impl.c belle-sip-1.3.3/src/belle_sip_utils.c belle-sip-1.3.3/src/channel.c belle-sip-1.3.3/src/channel.h belle-sip-1.3.3/src/clock_gettime.h belle-sip-1.3.3/src/dialog.c belle-sip-1.3.3/src/ict.c belle-sip-1.3.3/src/ist.c belle-sip-1.3.3/src/listeningpoint.c belle-sip-1.3.3/src/listeningpoint_internal.h belle-sip-1.3.3/src/message.c belle-sip-1.3.3/src/nict.c belle-sip-1.3.3/src/nist.c belle-sip-1.3.3/src/port.c belle-sip-1.3.3/src/port.h belle-sip-1.3.3/src/provider.c belle-sip-1.3.3/src/refresher.c belle-sip-1.3.3/src/siplistener.c belle-sip-1.3.3/src/sipstack.c belle-sip-1.3.3/src/transaction.c belle-sip-1.3.3/src/transports/stream_channel.c belle-sip-1.3.3/src/transports/stream_channel.h belle-sip-1.3.3/src/transports/stream_listeningpoint.c belle-sip-1.3.3/src/transports/tls_channel_polarssl.c belle-sip-1.3.3/src/transports/tls_listeningpoint_polarssl.c belle-sip-1.3.3/src/transports/tunnel_channel.c belle-sip-1.3.3/src/transports/tunnel_listeningpoint.c belle-sip-1.3.3/src/transports/tunnel_wrapper.cc belle-sip-1.3.3/src/transports/udp_channel.c belle-sip-1.3.3/src/transports/udp_listeningpoint.c belle-sip-1.3.3/tester/auth_helper_tester.c belle-sip-1.3.3/tester/belle_sdp_tester.c belle-sip-1.3.3/tester/belle_sip_core_tester.c belle-sip-1.3.3/tester/belle_sip_dialog_tester.c belle-sip-1.3.3/tester/belle_sip_headers_tester.c belle-sip-1.3.3/tester/belle_sip_message_tester.c belle-sip-1.3.3/tester/belle_sip_refresher_tester.c belle-sip-1.3.3/tester/belle_sip_register_tester.c belle-sip-1.3.3/tester/belle_sip_resolver_tester.c belle-sip-1.3.3/tester/belle_sip_tester.c belle-sip-1.3.3/tester/belle_sip_tester.h belle-sip-1.3.3/tester/belle_sip_uri_tester.c belle-sip-1.3.3/tester/cast_test.c belle-sip-1.3.3/tester/describe.c belle-sip-1.3.3/tester/get.c belle-sip-1.3.3/tester/parse.c belle-sip-1.3.3/tester/register_tester.h GPL (v3 or later) ----------------- belle-sip-1.3.3/include/belle-sip/bodyhandler.h belle-sip-1.3.3/include/belle-sip/generic-uri.h belle-sip-1.3.3/include/belle-sip/http-listener.h belle-sip-1.3.3/include/belle-sip/http-message.h belle-sip-1.3.3/include/belle-sip/http-provider.h belle-sip-1.3.3/include/belle-sip/types.h belle-sip-1.3.3/src/bodyhandler.c belle-sip-1.3.3/src/generic-uri.c belle-sip-1.3.3/src/http-listener.c belle-sip-1.3.3/src/http-message.c belle-sip-1.3.3/src/http-provider.c belle-sip-1.3.3/src/parserutils.h belle-sip-1.3.3/tester/belle_generic_uri_tester.c belle-sip-1.3.3/tester/belle_http_tester.c MIT/X11 (BSD like) ------------------ belle-sip-1.3.3/src/dns.c belle-sip-1.3.3/src/dns.h Unknown or generated -------------------- belle-sip-1.3.3/autogen.sh belle-sip-1.3.3/build/android/config.h belle-sip-1.3.3/build/wp8/belle-sip-tester-wp8/belle-sip-tester-native/belle-sip-tester-native.cpp belle-sip-1.3.3/build/wp8/belle-sip-tester-wp8/belle-sip-tester-native/belle-sip-tester-native.h belle-sip-1.3.3/build/wp8/belle-sip-tester-wp8/belle-sip-tester-wp8/App.xaml.cs belle-sip-1.3.3/build/wp8/belle-sip-tester-wp8/belle-sip-tester-wp8/LocalizedStrings.cs belle-sip-1.3.3/build/wp8/belle-sip-tester-wp8/belle-sip-tester-wp8/MainPage.xaml.cs belle-sip-1.3.3/build/wp8/belle-sip-tester-wp8/belle-sip-tester-wp8/Properties/AssemblyInfo.cs belle-sip-1.3.3/build/wp8/belle-sip-tester-wp8/belle-sip-tester-wp8/Resources/AppResources.Designer.cs belle-sip-1.3.3/build/wp8/belle-sip-tester-wp8/belle-sip-tester-wp8/TestCasePage.xaml.cs belle-sip-1.3.3/build/wp8/belle-sip-tester-wp8/belle-sip-tester-wp8/TestResultPage.xaml.cs belle-sip-1.3.3/include/belle-sip/wakelock.h belle-sip-1.3.3/src/wakelock.c belle-sip-1.3.3/src/wakelock_internal.h zlib/libpng ----------- belle-sip-1.3.3/src/md5.c belle-sip-1.3.3/src/md5.h [x]: License file installed when any subpackage combination is installed. [x]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Only use %_sourcedir in very specific situations. Note: %_sourcedir/$RPM_SOURCE_DIR is used. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. Note: Test run failed [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Test run failed [x]: Packages must not store files under /srv, /opt or /usr/local Note: Test run failed [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package do not use a name that already exist [x]: Package is not relocatable. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. ===== SHOULD items ===== Generic: [!]: Sources can be downloaded from URI in Source: tag Note: Could not download Source0: http://download.savannah.gnu.org/releases/linphone/belle-sip/belle- sip-1.3.3.tar.gz See: http://fedoraproject.org/wiki/Packaging:Guidelines#Tags [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Scriptlets must be sane, if used. [x]: SourceX tarball generation or download is documented. Note: Package contains tarball without URL, check comments [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [!]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: Dist tag is present (not strictly required in GL). [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Fully versioned dependency in subpackages if applicable. [x]: Uses parallel make %{?_smp_mflags} macro. [x]: The placement of pkgconfig(.pc) files are correct. [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [-]: Large data in /usr/share should live in a noarch subpackage if package is arched. Note: Test run failed [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Package should not use obsolete m4 macros Rpmlint ------- Checking: belle-sip-1.3.3-1.fc23.x86_64.rpm belle-sip-devel-1.3.3-1.fc23.x86_64.rpm belle-sip-1.3.3-1.fc23.src.rpm belle-sip.x86_64: W: spelling-error Summary(en_US) Linphone -> Lin phone, Lin-phone belle-sip.x86_64: E: incorrect-fsf-address /usr/share/doc/belle-sip/COPYING belle-sip-devel.x86_64: W: only-non-binary-in-usr-lib belle-sip-devel.x86_64: W: no-documentation belle-sip.src: W: spelling-error Summary(en_US) Linphone -> Lin phone, Lin-phone belle-sip.src:32: E: use-of-RPM_SOURCE_DIR belle-sip.src: W: invalid-url Source0: http://download.savannah.gnu.org/releases/linphone/belle-sip/belle-sip-1.3.3.tar.gz HTTP Error 403: Forbidden 3 packages and 0 specfiles checked; 2 errors, 5 warnings. Rpmlint (installed packages) ---------------------------- Cannot parse rpmlint output: Requires -------- belle-sip-devel (rpmlib, GLIBC filtered): /usr/bin/pkg-config belle-sip(x86-64) belle-sip (rpmlib, GLIBC filtered): /sbin/ldconfig libantlr3c.so()(64bit) libc.so.6()(64bit) libdl.so.2()(64bit) libgcc_s.so.1()(64bit) libm.so.6()(64bit) libpolarssl.so.7()(64bit) libpthread.so.0()(64bit) librt.so.1()(64bit) rtld(GNU_HASH) Provides -------- belle-sip-devel: belle-sip-devel belle-sip-devel(x86-64) pkgconfig(belle-sip) belle-sip: belle-sip belle-sip(x86-64) bundled(md5-deutsch) libbellesip.so.0()(64bit) Source checksums ---------------- https://github.com/antlr/website-antlr3/blob/gh-pages/download/antlr-3.4-complete.jar : CHECKSUM(SHA256) this package : 9d3e866b610460664522520f73b81777b5626fb0a282a5952b9800b751550bf7 CHECKSUM(SHA256) upstream package : 66301f492ee5ad0372703a6902210e78d509e9a2914963dd6605dd05f03de570 diff -r also reports differences Generated by fedora-review 0.5.2 (63c24cb) last change: 2014-07-14 Command line :/usr/bin/fedora-review -rvn belle-sip-1.3.3-1.fc23.src.rpm Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api, C/C++ Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG And, please use %license for license text.
Spec URL: https://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip.spec SRPM URL: https://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip-1.4.0-1.fc23.src.rpm Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=9224894 Changelog: - belle-sip-1.4.0 - update Source0 URL - use %%license macro
This has antlr3 bundled for building. While I understand why you're doing this, I don't think this is kosher. You'll need to properly build the java bits of antlr3 in a separate source package and BuildRequires that.
(In reply to Tom "spot" Callaway from comment #29) > This has antlr3 bundled for building. While I understand why you're doing > this, I don't think this is kosher. You'll need to properly build the java > bits of antlr3 in a separate source package and BuildRequires that. Too much work for maintaining outdated antlr3 3.4 package.
Well, it's either that or patch belle-sip to use modern antlr. Which one is less work is up to you. :)
It really looks like the C parser generator is broken in antlr 3.5. See https://github.com/antlr/antlr3/issues/175 for one. Also, cvc4 is the only other anltr C project that I could find in Fedora. It ships pre-generated parser files so it appears to be fine, but if I remove the generated files, I see similar errors there as we do with belle-sip. I think an acceptable work around may be to generate the needed parser files with anltr 3.4 and then use those for the build, which is what cvc4 is doing. The current bundling of antlr3.4 jar is clearly forbidden.
(In reply to Orion Poplawski from comment #32) > It really looks like the C parser generator is broken in antlr 3.5. See > https://github.com/antlr/antlr3/issues/175 for one. Also, cvc4 is the only > other anltr C project that I could find in Fedora. It ships pre-generated > parser files so it appears to be fine, but if I remove the generated files, > I see similar errors there as we do with belle-sip. > > I think an acceptable work around may be to generate the needed parser files > with anltr 3.4 and then use those for the build, which is what cvc4 is > doing. The current bundling of antlr3.4 jar is clearly forbidden. Thanks, your suggestion really works. But unfortunately belle-sip detects only mbedtls 1.3.11 (in F22) but 2.0.0 don't (in F23, F24). See F24 scratch build http://koji.fedoraproject.org/koji/taskinfo?taskID=10953238 Maybe this patch from git should be modified somehow https://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip-1.4.1-libmbedtls.patch Spec URL: https://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip.spec SRPM URL: https://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip-1.4.1-1.fc22.src.rpm F22 Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=10953238 - belle-sip-1.4.1 - removed atlr3 3.4 SOURCE1 - added patch with files generated by antlr-3.4-complete.jar - BR: mbedtls-devel instead of polarssl-devel
F22 Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=10953249
Looks like the only possible solution for now is disable TLS for F23+ because of mbedtls 2 incompatibility. Spec URL: https://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip.spec SRPM URL: https://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip-1.4.1-2.fc24.src.rpm F24 Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=11139600
Yeah, looks like belle-sip really hasn't kept up with mbedtls development. It's actually making use of 1.2 functions that are deprecated in 1.3. So it's really not close to being able to use 2.0.
Copr repos with linphone based on belle-sip https://copr.fedoraproject.org/coprs/nucleo/linphone/
belle-sip-1.4.2 Spec URL: https://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip.spec SRPM URL: https://nucleo.fedorapeople.org/pkg-reviews/belle-sip/belle-sip-1.4.2-1.fc24.src.rpm Copr build: https://copr.fedoraproject.org/coprs/nucleo/linphone/build/137730/
Chris - are you still up for reviewing this?
Created attachment 1177694 [details] Fix typo in comparison code
With the patch in Comment 40 applied, this package builds cleanly on Fedora 24 (the patch fixes an obvious typo in a comparison that wasn't caught by earlier gcc revisions). All the items noted in Chris's previous review are resolved in 1.4.2-1, so I'm approving this package (please be sure to apply that patch before committing).
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/belle-sip
(In reply to Tom "spot" Callaway from comment #41) > With the patch in Comment 40 applied, this package builds cleanly on Fedora > 24 (the patch fixes an obvious typo in a comparison that wasn't caught by > earlier gcc revisions). > > All the items noted in Chris's previous review are resolved in 1.4.2-1, so > I'm approving this package (please be sure to apply that patch before > committing). Thanks for the patch but it builds fine only in F24 and fails in Rawhide: belle_sip_utils.c: In function 'belle_sip_parse_directory': belle_sip_utils.c:1241:2: error: 'readdir_r' is deprecated [-Werror=deprecated-declarations] res = readdir_r(dir, ent, &result); ^~~ In file included from belle_sip_utils.c:31:0: /usr/include/dirent.h:183:12: note: declared here extern int readdir_r (DIR *__restrict __dirp, ^~~~~~~~~ belle_sip_utils.c:1249:3: error: 'readdir_r' is deprecated [-Werror=deprecated-declarations] res = readdir_r(dir, ent, &result); ^~~ In file included from belle_sip_utils.c:31:0: /usr/include/dirent.h:183:12: note: declared here extern int readdir_r (DIR *__restrict __dirp, ^~~~~~~~~
readdir_r is deprecated, just use readdir everywhere.
(In reply to Orion Poplawski from comment #44) > readdir_r is deprecated, just use readdir everywhere. After I replaced readdir_r with readdir: belle_sip_utils.c: In function 'belle_sip_parse_directory': belle_sip_utils.c:1241:8: error: too many arguments to function 'readdir' res = readdir(dir, ent, &result); ^~~~~~~ In file included from belle_sip_utils.c:31:0: /usr/include/dirent.h:162:23: note: declared here extern struct dirent *readdir (DIR *__dirp) __nonnull ((1)); ^~~~~~~ belle_sip_utils.c:1241:6: error: assignment makes integer from pointer without a cast [-Werror=int-conversion] res = readdir(dir, ent, &result); ^ belle_sip_utils.c:1249:9: error: too many arguments to function 'readdir' res = readdir(dir, ent, &result); ^~~~~~~ In file included from belle_sip_utils.c:31:0: /usr/include/dirent.h:162:23: note: declared here extern struct dirent *readdir (DIR *__dirp) __nonnull ((1)); ^~~~~~~ belle_sip_utils.c:1249:7: error: assignment makes integer from pointer without a cast [-Werror=int-conversion] res = readdir(dir, ent, &result);
Well, yeah. They are different function with different arguments.
Created attachment 1183912 [details] Replace readdir_r with readdir I _think_ this patch is right. Would appreciate other people reviewing it for sanity.
Comment on attachment 1183912 [details] Replace readdir_r with readdir From man 3 readdir: On success, readdir() returns a pointer to a dirent structure. (This structure may be statically allocated; do not attempt to free(3) it.) If the end of the directory stream is reached, NULL is returned and errno is not changed. If an error occurs, NULL is returned and errno is set appropriately. So I think you want to check errno for errors.
Created attachment 1184318 [details] Replace readdir_r with readdir and catch errno How about this one?
Comment on attachment 1184318 [details] Replace readdir_r with readdir and catch errno Seems reasonable to me.
@Tom "spot" Callaway Imho that patch won't work as expected. You are using ent in comparison, but this is not longer set by any function. Correct should be using result->d_name instead of ent->d_name and you could drop name_max detection and the ent variable at all.
Created attachment 1231249 [details] use result->d_name Good catch. I didn't take the NAME_MAX stuff out because it feels like that could be done in a separate patch, as it is less tied to the readdir_r bugfix and more of a cleanup.
This review is not proceeding. I'm abandoning my side of it.
This package seems to be in the repo - should this be closed? The current released version is 1.6.3. It still has the wrong fsf address.
Yes, this was imported long-ago apparently (or was dup'd?) commit c8e9350a41bd74d1a3ab6631284a1486f7e9ed74 (origin/f25, origin/f24, origin/f23) Author: nucleo <nucleo> Date: Tue Jul 19 19:09:52 2016 +0300 import belle-sip commit 0a08a5522c173933baf4578b40425036711d1a22 Author: Fedora Release Engineering <rel-eng.org> Date: Tue Jul 19 15:28:03 2016 +0000 Initial setup of the repo