This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 1050744 - Review Request: belle-sip - Linphone SIP stack
Review Request: belle-sip - Linphone SIP stack
Status: ASSIGNED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Tom "spot" Callaway
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-Legal 1116582
  Show dependency treegraph
 
Reported: 2014-01-08 20:19 EST by nucleo
Modified: 2016-07-26 11:43 EDT (History)
11 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
tcallawa: fedora‑review+


Attachments (Terms of Use)
Fix typo in comparison code (523 bytes, patch)
2016-07-08 10:36 EDT, Tom "spot" Callaway
no flags Details | Diff
Replace readdir_r with readdir (1.30 KB, patch)
2016-07-25 14:33 EDT, Tom "spot" Callaway
no flags Details | Diff
Replace readdir_r with readdir and catch errno (1.36 KB, patch)
2016-07-26 11:39 EDT, Tom "spot" Callaway
no flags Details | Diff

  None (edit)
Description nucleo 2014-01-08 20:19:59 EST
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.
Comment 1 nucleo 2014-01-08 20:20:48 EST
$ 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.
Comment 2 nucleo 2014-01-08 20:23:56 EST
Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6376909
Comment 3 Christopher Meng 2014-01-09 05:19:54 EST
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 ;) ?
Comment 4 nucleo 2014-01-11 19:48:47 EST
- 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.
Comment 5 Christopher Meng 2014-01-12 00:45:36 EST
Please feedback the results later here.

Thanks.
Comment 6 nucleo 2014-01-12 09:27:44 EST
(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.
Comment 7 Christopher Meng 2014-01-12 09:33:45 EST
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.
Comment 8 nucleo 2014-01-12 09:35:34 EST
(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,
Comment 9 nucleo 2014-01-17 22:07:20 EST
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
Comment 10 nucleo 2014-01-17 22:31:59 EST
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.
Comment 12 nucleo 2014-01-18 07:24:13 EST
Upstream informed.
I can revert sed if review will be finished after this.
Comment 13 Christopher Meng 2014-01-24 02:27:44 EST
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
Comment 14 Rex Dieter 2014-01-24 20:11:38 EST
Fwiw, GPLv2+ + BSD/MIT = GPLv2+, pretty sure the same can be said of libpng/zlib too
Comment 15 nucleo 2014-02-20 19:46:09 EST
(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
Comment 16 Christopher Meng 2014-03-26 10:07:19 EDT
Can you read carefully:

https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#cite_note-1

See "cite-note"
Comment 17 nucleo 2014-03-26 17:41:18 EDT
(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?
Comment 18 Jan Kratochvil 2014-07-05 10:53:21 EDT
(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.
Comment 19 Christopher Meng 2014-07-06 09:34:05 EDT
(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.
Comment 20 nucleo 2014-09-08 14:36:20 EDT
Can someone help fix compilation error from comment 15?
Comment 21 Christopher Meng 2014-09-10 01:04:37 EDT
Well...

_DEFAULT_SOURCE switch is bad IMO... From recent glibc I mean.

You can escape that by dropping -Werror=cpp in the cflags.
Comment 22 Christopher Meng 2014-09-10 01:06:53 EDT
(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.
Comment 23 Tom "spot" Callaway 2014-11-14 14:41:53 EST
Status check here?
Comment 24 nucleo 2014-11-14 14:45:04 EST
Help with fixing compilation error needed.
Comment 25 Tom "spot" Callaway 2014-11-21 09:16:51 EST
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.
Comment 26 nucleo 2015-02-15 01:46:07 EST
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
Comment 27 Christopher Meng 2015-03-09 05:34:35 EDT
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.
Comment 29 Tom "spot" Callaway 2015-03-13 15:36:54 EDT
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.
Comment 30 nucleo 2015-03-13 16:15:14 EDT
(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.
Comment 31 Tom "spot" Callaway 2015-03-13 16:21:29 EDT
Well, it's either that or patch belle-sip to use modern antlr. Which one is less work is up to you. :)
Comment 32 Orion Poplawski 2015-08-31 22:15:10 EDT
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.
Comment 33 nucleo 2015-09-04 06:14:27 EDT
(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
Comment 34 nucleo 2015-09-04 06:15:42 EDT
F22 Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=10953249
Comment 35 nucleo 2015-09-18 22:03:54 EDT
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
Comment 36 Orion Poplawski 2015-09-18 23:05:41 EDT
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.
Comment 37 nucleo 2015-09-19 18:10:15 EDT
Copr repos with linphone based on belle-sip
https://copr.fedoraproject.org/coprs/nucleo/linphone/
Comment 39 Orion Poplawski 2015-11-13 17:16:37 EST
Chris - are you still up for reviewing this?
Comment 40 Tom "spot" Callaway 2016-07-08 10:36 EDT
Created attachment 1177694 [details]
Fix typo in comparison code
Comment 41 Tom "spot" Callaway 2016-07-08 10:40:44 EDT
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).
Comment 42 Jon Ciesla 2016-07-19 11:25:12 EDT
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/belle-sip
Comment 43 nucleo 2016-07-19 13:23:51 EDT
(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,
            ^~~~~~~~~
Comment 44 Orion Poplawski 2016-07-19 17:06:03 EDT
readdir_r is deprecated, just use readdir everywhere.
Comment 45 nucleo 2016-07-19 18:18:08 EDT
(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);
Comment 46 Orion Poplawski 2016-07-19 18:25:46 EDT
Well, yeah.  They are different function with different arguments.
Comment 47 Tom "spot" Callaway 2016-07-25 14:33 EDT
Created attachment 1183912 [details]
Replace readdir_r with readdir

I _think_ this patch is right. Would appreciate other people reviewing it for sanity.
Comment 48 Orion Poplawski 2016-07-25 18:53:15 EDT
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.
Comment 49 Tom "spot" Callaway 2016-07-26 11:39 EDT
Created attachment 1184318 [details]
Replace readdir_r with readdir and catch errno

How about this one?
Comment 50 Orion Poplawski 2016-07-26 11:43:25 EDT
Comment on attachment 1184318 [details]
Replace readdir_r with readdir and catch errno

Seems reasonable to me.

Note You need to log in before you can comment on or make changes to this bug.