Spec URL: http://www.gnutelephony.org/specs/sipwitch.spec SRPM URL: http://www.gnutelephony.org/specs/sipwitch-0.5.4-0.src.rpm Description: GNU SIP Witch is a pure SIP-based office telephone call server that supports generic phone system features like call forwarding, hunt groups and call distribution, call coverage and ring groups, holding, and call transfer, as well as offering SIP specific capabilities such as presence and messaging. It supports secure telephone extensions for making calls over the Internet, and intercept/decrypt-free peer-to-peer audio and video extensions. It is not a SIP proxy, a multi-protocol telephone server, or an IP-PBX, and does not try to emulate Asterisk, FreeSWITCH, or Yate. WARNING: This is a rather complex package, it includes plugins, python and php swig. Many different things and packaging issues are found within!
From very quick glance at your spec file: - Fedora suggests that one line in %description must not have more than 79 characters (please check rpmlint warnings) - Use macros consistently. ---------------------------------------------------- 31 Requires: %{name} = %{version}-%{release} 36 Requires: sipwitch = %{version}-%{release} ---------------------------------------------------- - Would you explain why some of the subpackage does not have the dependency for main package? - %{buildroot} is missing: ---------------------------------------------------- %{__rm} -f %{_libdir}/*.la %{__rm} -f %{_libdir}/sipwitch/*.la ---------------------------------------------------- - Perhaps "INSTALL" is not needed for rpm document files. - Files under %_mandir are automatically marked as %doc. - The main package must not own the directory %_sbindir, %_bindir themselves. - For initscripts related convention, * please follow https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_on_the_filesystem https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscript_packaging From rpmlint: * service should not be enabled by default (i.e. change ----------------------------------------------------- # chkconfig: 2345 95 15 ----------------------------------------------------- to ----------------------------------------------------- # chkconfig: - 95 15 ----------------------------------------------------- * init script should have "status" entry (please also check "$ rpmlint -I no-status-entry") * init script should put a lock file in %_localstatedir/lock/subsys . - {_datadir}/snmp{,/mibs} is not owned by the main package but it installs some files under these directories. These directories are owned by net-snmp-libs, currently this package (sipwitch) does not seem to require net-snmp-libs. Would you check if sipwitch should require net-snmp-libs? (or examine why some files are installed under %_datadir/snmp?) - Using %_libdir/python* is wrong (by the way this does not seem to work on 64 bit architecture). Please follow https://fedoraproject.org/wiki/Packaging/Python#System_Architecture - Because of some reasons (one of the reasons is to avoid selinux AVC denial), we always install byte-compiled .py{o,c} files altogether (note that these .py{o,c} files are automatically created by /usr/lib/rpm/brp-python-bytecompile). So - these files must appear in %files list (using sipwitch.py* is easier) - creating/removing byte-compiled python files in scriptlets (i.e. at %post or so) should be removed. - Usually configuration files should be marked as %config(noreplace) https://fedoraproject.org/wiki/Packaging/Guidelines#Configuration_files - When only /sbin/ldconfig is called on a scriptlet, use -p option to avoid unneeded shell call, like: --------------------------------------------------------- %postun -p /sbin/ldconfig --------------------------------------------------------- (however also please check initscripts scriptlets convention)
To do this I will also had to add a patch until changes make it into the next upstream release...but I have a question, on net-snmp. We are supplying a valid snmp MIB, and it really does belong in the mib directory. So I created sipwitch-snmp as a sub-package. Updated Spec: http://www.gnutelephony.org/specs/sipwitch.spec Current Patch: http://www.gnutelephony.org/specs/sipwitch.patch Updated SRPM: http://www.gnutelephony.org/specs/sipwitch-0.5.4-1.src.rpm
Created attachment 342909 [details] rpmlint on x86_64 (after fixing rpath and so on) For 0.5.4-1: * BR - "BR: pkgconfig" is not needed because ucommon-devel has "Requiers: pkgconfig" (Note that any packages containing pkgconfig .pc file should have "Requires: pkgconfig") - By the way would you check if some version specific dependency for BuildRequires (not Requires) is really needed? * Internal dependency - Is it safe that sipwitch-snmp subpackage does not depend on sipwitch? * Patch0 vs %patch - On rawhide %patch is rejected when you use "Patch0:". You should use "Patch: <-> %patch" or "Patch0: <-> %patch0". * rpath Ref: https://fedoraproject.org/wiki/Packaging/Guidelines#Beware_of_Rpath - On x86_64 the rebuilt binaries have unneeded rpath: ------------------------------------------------------------- sipwitch-plugin-forward.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/sipwitch/forward.so ['/usr/lib64'] sipwitch-plugin-rtpproxy.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/sipwitch/rtpproxy.so ['/usr/lib64'] sipwitch-plugin-scripting.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/sipwitch/scripting.so ['/usr/lib64'] sipwitch-plugin-subscriber.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/sipwitch/subscriber.so ['/usr/lib64'] sipwitch-plugin-zeroconf.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/sipwitch/zeroconf.so ['/usr/lib64'] ------------------------------------------------------------- Fedora requests to remove these rpaths. This can be removed by the below (I usually do the following) ------------------------------------------------------------- %prep %setup -q %patch -p0 sed -i.rpath -e \ '/sys_lib_dlsearch_path_spec/s|/usr/lib |/usr/lib /usr/lib64 /lib /lib64 |' \ configure .... .... %build .... %configure \ .... ------------------------------------------------------------- (note that the way recommended in the wiki often breaks linkage against libraries rebuilt from the same source) * stripping binaries - Some of the binaries are stripped: ------------------------------------------------------------- 1660 DEBUG: + /usr/bin/make DESTDIR=/builddir/build/BUILDROOT/sipwitch-0.5.4-1.1.fc11.x86_64 'INSTALL=install -p' swig-python ..... 1685 DEBUG: g++ -pthread -shared -lc -Wno-variadic-macros -Wno-long-long -fexceptions -DNEW_STDCPP -pthread -fno-check-new -finline -fvisibility=hidden -DUCOMMON_VISIBILITY=1 -I/builddir/build/BUILD/sipwitch-0.5.4/inc -I/builddir/build/BUILD/sipwitch-0.5.4 build/temp.linux-x86_64-2.6/wrapper.o -L/usr/lib64 -lpython2.6 -lucommon -o _sipwitch.so 1686 DEBUG: /bin/sh /builddir/build/BUILD/sipwitch-0.5.4/autoconf/install-sh -d /builddir/build/BUILDROOT/sipwitch-0.5.4-1.1.fc11.x86_64`python -c 'from distutils.sysconfig import get_python_lib; print get_python_lib()'` 1687 DEBUG: /bin/sh /builddir/build/BUILD/sipwitch-0.5.4/autoconf/install-sh -d /builddir/build/BUILDROOT/sipwitch-0.5.4-1.1.fc11.x86_64`python -c 'from distutils.sysconfig import get_python_lib; print get_python_lib(1)'` 1688 DEBUG: strip _sipwitch.so ..... 1692 DEBUG: + /usr/bin/make DESTDIR=/builddir/build/BUILDROOT/sipwitch-0.5.4-1.1.fc11.x86_64 'INSTALL=install -p' swig-php5 1698 DEBUG: g++ -shared -module -shared -avoid-version -o sipwitch.so wrapper.o -lucommon -lc 1699 DEBUG: /bin/sh /builddir/build/BUILD/sipwitch-0.5.4/autoconf/install-sh -d /builddir/build/BUILDROOT/sipwitch-0.5.4-1.1.fc11.x86_64`php-config --extension-dir` 1700 DEBUG: strip sipwitch.so ------------------------------------------------------------- To create debuginfo rpm correctly, binaries must not be stripped before %install ends. ref: https://fedoraproject.org/wiki/Packaging/Debuginfo For this package, the following will prevent this strip: ------------------------------------------------------------- %build export STRIP=/bin/true %configure --with-pkg-config --disable-static %{__make} %{?_smp_mflags} ------------------------------------------------------------- * Automated autotools call - At some point autotools are automatically called after configure -> make is executed: ------------------------------------------------------------- 1660 DEBUG: + /usr/bin/make DESTDIR=/builddir/build/BUILDROOT/sipwitch-0.5.4-1.1.fc11.x86_64 'INSTALL=install -p' swig-python 1661 DEBUG: (cd swig ; make python-swig) 1662 DEBUG: make[1]: Entering directory `/builddir/build/BUILD/sipwitch-0.5.4/swig' 1663 DEBUG: cd .. && /bin/sh /builddir/build/BUILD/sipwitch-0.5.4/autoconf/missing --run automake-1.10 --gnu swig/Makefile 1664 DEBUG: cd .. && /bin/sh ./config.status swig/Makefile 1665 DEBUG: config.status: creating swig/Makefile ------------------------------------------------------------- This modifies autotools-generated files and this must be done before configure/make is executed. For this package, this is because your patch modifies only swig/Makefile.am but does not modify swig/Makefile.in. Solution: - Patch against swig/Makefile.in, not against swig/Makefile.am This is recommended to avoid unneeded autotools call. - If you want to call autotools anyway, explicitly call autotools before configure/make is executed. However this is not recommended and should be avoided when possible. * %config(noreplace) - Would you explain why %{_sysconfdir}/logrotate.d/sipwitch is not marked as (noreplace)? * rpmlint issue See attached: ! Note that "sipwitch.src:151: E: hardcoded-library-path in /lib" is because of rpath fix suggested above and this can be ignored. - If %{python_sitelib}/sipwitch.py is to be executed directly from users, then this script should have 0755 permission. Otherwise the shebang on this script should be removed. * Macros - For the directory where php binary modules are installed, please use %php_extdir macro. This macro is defined in /etc/rpm/macros.php in php-devel. * Initscripts convention Again please check: https://fedoraproject.org/wiki/Packaging/SysVInitScript - Initscript must be installed under %_initrddir (/etc/rc.d/init.d, and please use this macro), not %_sysconfdir/init.d. - Some proper Requires(post) or so and scriptlets are needed. Please refer to https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_in_spec_file_scriptlets
(Removing NEEDSPONSOR)
I am going to look this over later tonight or maybe even over the weekend. There are some aspects that I may wish to do a new upstream release for also... In respect to init scripts, I usually like to keep the scripts themselves simple because they are going to run on different platforms...but most of this relates to the package part :). One concern I find in this is I wish to make sure that the upstream source picks the same directories as the packaging system :). For that, for things like python_sitearch, etc, I think I should add to configure.ac some stuff so I can do something like --with-python-sitearch=%{python_sitearch}..., --with-python-sitelib=..., --with-initrd-dir=... in %configure rather than simply depend on the upstream target directories happening to match :). So I want to think about that aspect later today... This in many ways is a good package for use in training, as it covers so many different aspects and issues.
It is safe for sipwitch-snmp to be separate because it may be separately installed on a network management station, for example to collect and decode traps. No sipwitch would be installed there :)
I decided to try and complete all changes on the current upstream release. Updated spec: http://www.gnutelephony.org/specs/sipwitch.spec Updated srpm: http://www.gnutelephony.org/specs/sipwitch-0.5.4-2.fc10.src.rpm Updated patch: http://www.gnutelephony.org/specs/sipwitch.patch And I will shift some changes upstream after this...
For -2: * BR - build.log shows: ------------------------------------------------------------- 714 DEBUG: checking gcrypt/gcrypt.h usability... no 715 DEBUG: checking gcrypt/gcrypt.h presence... no 716 DEBUG: checking for gcrypt/gcrypt.h... no 717 DEBUG: checking gcrypt.h usability... no 718 DEBUG: checking gcrypt.h presence... no 719 DEBUG: checking for gcrypt.h... no 720 DEBUG: checking openssl/md5.h usability... no 721 DEBUG: checking openssl/md5.h presence... no 722 DEBUG: checking for openssl/md5.h... no 723 DEBUG: checking sasl/md5global.h usability... no 724 DEBUG: checking sasl/md5global.h presence... no 725 DEBUG: checking for sasl/md5global.h... no ------------------------------------------------------------- Looking at configure.in, common/digest.cpp, and utils/sipdigest.cpp, it seems that adding "BuildRequires: libgcrypt-devel is preferable. After that, build.log shows: ------------------------------------------------------------- 722 DEBUG: checking gcrypt/gcrypt.h usability... no 723 DEBUG: checking gcrypt/gcrypt.h presence... no 724 DEBUG: checking for gcrypt/gcrypt.h... no 725 DEBUG: checking gcrypt.h usability... yes 726 DEBUG: checking gcrypt.h presence... yes 727 DEBUG: checking for gcrypt.h... yes 728 DEBUG: checking for gcry_control in -lgcrypt... yes ------------------------------------------------------------- * build failure - -2 srpm installs sipwitch initscript under %buildroot%_sysconfdir/init.d, not %buildroot%_initrddir and build fails there. Currently the following is needed: ------------------------------------------------------------- %{__mkdir_p} %{buildroot}%{_initrddir} %{__mv} -f %{buildroot}%{_sysconfdir}/init.d/* %{buildroot}%{_initrddir} rmdir %{buildroot}%{_sysconfdir}/init.d/ ------------------------------------------------------------- * Initscripts - Requires(postun): initscripts is still missing. - "service sipwitch restart" won't work like: ------------------------------------------------------------- # service sipwitch restart Restarting sipwitch: /etc/init.d/sipwitch: line 116: start-stop-daemon: command not found ------------------------------------------------------------- * rpmlint - Still rpmlint complains like: ------------------------------------------------------------- sipwitch-python-swig.i586: E: script-without-shebang /usr/lib/python2.6/site-packages/sipwitch.py ------------------------------------------------------------- Please check my previous comment: > - If %{python_sitelib}/sipwitch.py is to be executed directly from > users, then this script should have 0755 permission. > Otherwise the shebang on this script should be removed.
The sipwitch.py issue is partially related to swig, since it is generated by swig. I changed the patch to install using INSTALL_DATA, and maybe I will need to set explicit attrib too? I will test this a bit here... I see now yes, "upstart" based debian/ubuntu only have /etc/init.d, where rh/fedora symlink it to /etc/rc.d/init.d. In the next upstream release I am adding --with-initrddir option for configure, but for the current release, I will use your hack... The stop-start-daemon issue gets to the heart of some init scripting differences between debian & rh. I am not sure if this is best solved with a patch, since init scripts dont change much, so that would not be too hard to maintain, or to have an entirely separate source1 just to take along a fedora/rh specific init script. I have already worked on the other changes, and once I test them and resolve the debian vs rh init script issues remaining, I will do a new update. Probably I will go for extending the patch for now. Incidentally, if gcrypt is not used, sipwitch falls back to openssl libcrypto. This in fact should have been preferred since libeXosip2 already links in openssl, but it seems to fail to detect libcrypto support (even for md5), and that is strange. Also I tested mostly with the gcrypt crypto functions, so it is probably safer to keep using those for now. Anyway, at some point we all should try to converge on nss based stuff :).
rpmlint on sipwitch binary rpm itself: sipwitch.i386: E: non-readable /etc/sipwitch.conf 0660 sipwitch.i386: E: non-readable /etc/sipwitch.d/lab.xml 0660 sipwitch.i386: E: non-readable /etc/sysconfig/sipwitch 0660 sipwitch.i386: E: non-readable /etc/sipwitch.d/tests.xml 0660 sipwitch.i386: E: non-standard-dir-perm /etc/sipwitch.d 0770 In fact, I do NOT want those files to be "world" readable...and I do want them to be group and user r/w. So I feel these are actually correct, not error.
Hello: (In reply to comment #9) > The sipwitch.py issue is partially related to swig, since it is generated by > swig. I changed the patch to install using INSTALL_DATA, and maybe I will need > to set explicit attrib too? I will test this a bit here... - For this just using %attr(...) or using chmod at %install is enough. > Incidentally, if gcrypt is not used, sipwitch falls back to openssl libcrypto. > This in fact should have been preferred since libeXosip2 already links in > openssl, but it seems to fail to detect libcrypto support (even for md5), and > that is strange. Also I tested mostly with the gcrypt crypto functions, so it > is probably safer to keep using those for now. Anyway, at some point we all > should try to converge on nss based stuff :). - Well, on rawhide libeXosip2 is not linked against openssl library. Note that this is under GPLv3+, and strictly speaking openssl license conflicts with GPL (any version): https://fedoraproject.org/wiki/Licensing
(In reply to comment #10) > rpmlint on sipwitch binary rpm itself: > > sipwitch.i386: E: non-readable /etc/sipwitch.conf 0660 > sipwitch.i386: E: non-readable /etc/sipwitch.d/lab.xml 0660 > sipwitch.i386: E: non-readable /etc/sysconfig/sipwitch 0660 > sipwitch.i386: E: non-readable /etc/sipwitch.d/tests.xml 0660 > sipwitch.i386: E: non-standard-dir-perm /etc/sipwitch.d 0770 > > In fact, I do NOT want those files to be "world" readable...and I do want them > to be group and user r/w. So I feel these are actually correct, not error. - rpmlint always complains about these types of permission/attributes, however we usually ignores these "errors" if there is enough reason (By the way, there is no difference for this case, however I prefer "0640" or "0750" permission)
I actually wrote a patch (for upstream) to make openssl optional in exosip2 at configure awhile back. Yes, openssl licensing is...rather problematic. And nss has fips certification. I am doing another upstream patch for exosip2 upstream related to terminating with reason, and maybe I will try to also substitute nss in it as well then. I also often hear "openssl" falls under the "system library exception", a rather uncertain assertion though perhaps a bit easier to say on BSD. I re-wrote the init script in a generic way for the next upstream release, and have used that as a patch for this one, hence the patch file is updated and should solve these other issues. I now understand how condrestart relates to updates, and it makes a lot of sense to me. In debian, I did it an uglier way though achieving the same effect. I also replaced $(INSTALL) sipwitch.py with $(INSTALL_DATA) in the patch and that cleared the exec permission problem, since sipwitch.py generated by swig has no #! and should not be marked execute. I use 0660/0770 because I often create an "admin" group for the daemon, and users of that group have to be able to modify config, write the control fifo, etc. Perhaps that process should occur automatically in pre/post scripts and be "standardized"? Anyway here is what I have now: Updated Spec: http://www.gnutelephony.org/specs/sipwitch.spec Updated SRPM: http://www.gnutelephony.org/specs/sipwitch-0.5.4-3.fc10.src.rpm Updated Patch: http://www.gnutelephony.org/specs/sipwitch.patch
Well, ------------------------------------------------------ This package (sipwitch) is APPROVED by mtasaka ------------------------------------------------------ Now please follow http://fedoraproject.org/wiki/Package_Review_Process from "CVS admin requests" (also please write cvs admin request on ucommon review request)
New Package CVS Request ======================= Package Name: sipwitch Short Description: SIP telephony server for secure phone systems Owners: dyfet Branches: F-10 F-11 InitialCC: mtasaka
cvs done.
sipwitch-0.5.4-3.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/sipwitch-0.5.4-3.fc10
sipwitch-0.5.4-3.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/sipwitch-0.5.4-3.fc11
Now closing.
sipwitch-0.5.4-3.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.
sipwitch-0.5.4-3.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.