Bug 446841 - Review Request: python-sippy - B2BUA SIP call controlling component
Review Request: python-sippy - B2BUA SIP call controlling component
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-05-16 08:13 EDT by Peter Lemenkov
Modified: 2009-06-15 22:41 EDT (History)
3 users (show)

See Also:
Fixed In Version: 0-0.8.20090429cvs.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-06-15 22:08:55 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Peter Lemenkov 2008-05-16 08:13:28 EDT
Spec URL: http://peter.fedorapeople.org/sippy.spec
SRPM URL: http://peter.fedorapeople.org/sippy-0-1.20080515cvs.fc9.src.rpm
Description: The B2BUA is a SIP call controlling component. Unlike a SIP proxy server,
which only maintains transaction state, the B2BUA maintains complete call
state and participates in all call requests. For this reason it can perform
number of functions that are not possible to implement using SIP proxy,
such as for example accurate call accounting, pre-paid rating and billing,
fail over call routing etc. Unlike PBX-type solutions such as Asterisk for
example, the B2BUA doesn't perform any media relaying or processing,
therefore it doesn't introduce any additional packet loss, delay or jitter
into the media path.


Few remarks:

* This is not a B2BUA itself, but rather a library for creating ones (two examples included).
* This package still not released as tarball
* I'm in doubts about naming scheme. Although it looks ugly but we use python-foo naming scheme, so maybe I should rename it to python-sippy?
* I placed working examples into %docs/examples (and therefore raising rpmling warnings) but maybe I must place them into %bindir or %sbindir?


Every comments and suggestions will be highly appreciated.
Comment 1 Jason Tibbitts 2008-06-29 15:37:57 EDT
This did not build for me:

Processing files: sippy-0-1.20080515cvs.fc10
Executing(%doc): /bin/sh -e /var/tmp/rpm-tmp.38465
error: File not found by glob:
/var/tmp/sippy-0-1.20080515cvs.fc10-root-mockbuild/usr/lib64/python2.5/site-packages/*

and later:
  Checking for unpackaged file(s): /usr/lib/rpm/check-files 
   /var/tmp/sippy-0-1.20080515cvs.fc10-root-mockbuild
  error: Installed (but unpackaged) file(s) found:
     /usr/lib/python2.5/site-packages/sippy-0.0-py2.5.egg-info
     /usr/lib/python2.5/site-packages/sippy/CCEvents.py
     /usr/lib/python2.5/site-packages/sippy/CCEvents.pyc
and so on, for every installed file.

Looks like you're using sitearch when you should be using sitelib, since this is
a noarch package.  I can't imagine that this package could ever actually build,
but somehow you got rpmlint output.

As you currently have things, this is just a python module and should be called
python-sippy.  Even if it has some scripts but is still mainly used as a module,
I'd name it as a module.  But if it's an application that happens to bundle
modules for its own use, then name it after the application.

If you expect that the examples will actually need to be called by people during
regular use then they should be in _bindir.  Otherwise they should be packaged
as documentation, and generally not be made executable.  Although it's not
really a problem (i.e. a review blocker) for them to be executable as long as
they don't pull in dependencies that the package wouldn't have were they not
executable.
Comment 2 Peter Lemenkov 2008-06-30 04:12:39 EDT
> I can't imagine that this package could ever actually build

...but it does :)

Thanks for hint with sitelib - I'll apply this and other suggestions.
Comment 3 Peter Lemenkov 2008-06-30 05:23:58 EDT
> Looks like you're using sitearch when you should be using sitelib,

Fixed.

> As you currently have things, this is just a python module and should be called
python-sippy.

Renamed.

> If you expect that the examples will actually need to be called by people during
regular use then they should be in _bindir.  Otherwise they should be packaged
as documentation, and generally not be made executable.

Moved to %_bindir

New version:

http://peter.fedorapeople.org/python-sippy.spec
http://peter.fedorapeople.org/python-sippy-0-2.20080627cvs.fc9.src.rpm
Comment 4 Jason Tibbitts 2008-12-06 21:50:27 EST
I'm sorry for not looking at this ticket in so many months.  The review queue is just so long....

Unfortunately the package in comment 3 doesn't build for me:

Executing command: ['bash', '--login', '-c', 'rpmbuild -bs --target x86_64 --nodeps builddir/build/SPECS/python-sippy.spec']
error: %patch without corresponding "Patch:" tag

You have Patch0: but call %patch instead of %patch0.  rpm these days doesn't permit that.

You can probably drop the BuildRequires conditional for Fedora 8; new branches for that release aren't accepted any longer so you only have to consider F-9 and newer.

If I fix the %patch problem, the package builds and rpmlint says:
  python-sippy.src:13: W: unversioned-explicit-provides sippy
You should essentially never use an unversioned Provides: like that, because it makes it impossible to bring back a package with that name.  See https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Renaming.2Freplacing_existing_packages for more information on how to rename packages.
Comment 5 Peter Lemenkov 2008-12-07 10:38:38 EST
(In reply to comment #4)

> You have Patch0: but call %patch instead of %patch0.  rpm these days doesn't
> permit that.

Fixed.

> You can probably drop the BuildRequires conditional for Fedora 8; new branches
> for that release aren't accepted any longer so you only have to consider F-9
> and newer.

Actually, I can't - I plan to submit this package for EPEL as well.

> If I fix the %patch problem, the package builds and rpmlint says:
>   python-sippy.src:13: W: unversioned-explicit-provides sippy
> You should essentially never use an unversioned Provides: like that, because it
> makes it impossible to bring back a package with that name.  See
> https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Renaming.2Freplacing_existing_packages
> for more information on how to rename packages.

Fixed.

http://peter.fedorapeople.org/python-sippy.spec
http://peter.fedorapeople.org/python-sippy-0-3.20081202cvs.fc10.src.rpm

Koji scratchbuild:

http://koji.fedoraproject.org/koji/taskinfo?taskID=985384
Comment 6 Jason Tibbitts 2008-12-19 17:35:14 EST
Actually you most certainly can remove the conditional for %fedora >= 8; you will need a different conditional for EPEL, since %fedora isn't even defined there.  (Unless you're assuming for some reason that anything not Fedora behaves a certain way, in which case RHEL6 will definitely surprise you.)  Of course, you also always have the option of just not using the same spec in EPEL.

Since this is a prerelease, the Release: tag must be less than 1.  Should be 0.3.20081202cvs%{?dist}

The %description doesn't seem to talk about this package at all; it just talks about what a B2BUA does.  If it had a sentence such as "This module provides a B2BUA (bullhorn to beef unstructured accountancy)" at the beginning, it would make more sense.  Also, you probably want to use the correct definition for "B2BUA", as I have no idea what it is.

* source files match upstream (manually verified)
X package does not meet versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
X description needs a bit of work.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* rpmlint is silent.
* final provides and requires are sane:
   sippy = 0-3.20081202cvs.fc11
   python-sippy = 0-3.20081202cvs.fc11
  =
   /usr/bin/python
   python(abi) = 2.6
   python-twisted-core

* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
Comment 7 Peter Lemenkov 2009-04-09 11:33:50 EDT
New package version:

http://peter.fedorapeople.org/python-sippy.spec
http://peter.fedorapeople.org/python-sippy-0-0.4.20090409cvs.fc10.src.rpm

* New cvs-snapshot, in which upstream  satisfies some our wishes, so I was able to drop almost all patches and workarounds from this package.
* Changed versioning scheme a little.
* Changed description.

Koji scratchbuild for EL-5:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1287562

Koji scratchbuild for F-10:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1287566
Comment 8 Jason Tibbitts 2009-04-16 16:05:09 EDT
This failed to build for me on the current F11 snapshot.

Processing files: python-sippy-0-0.4.20090409cvs.fc11.noarch
Executing(%doc): /bin/sh -e /var/tmp/rpm-tmp.pHzHPI
error: File not found by glob: /builddir/build/BUILDROOT/python-sippy-0-0.4.20090409cvs.fc11.x86_64/usr/bin/*.pyc
error: File not found by glob: /builddir/build/BUILDROOT/python-sippy-0-0.4.20090409cvs.fc11.x86_64/usr/bin/*.pyo

Looks like bug 182498 has been fixed.  You'll need to touch those two files so that you can exclude them, or condititionalize the two exclude lines.  I personally use the former approach.
Comment 9 Peter Lemenkov 2009-04-17 03:53:19 EDT
Thanks! Fixed.

Koji scratchbuild for F-11:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1303532

http://peter.fedorapeople.org/python-sippy.spec
http://peter.fedorapeople.org/python-sippy-0-0.6.20090409cvs.fc10.src.rpm

I also added missing "Requires:" - radiusclient-ng-utils.
Comment 11 Jason Tibbitts 2009-05-16 19:52:35 EDT
OK, this does build on F11, rpmlint is clean, the versioning is proper and %description looks OK to me.  However, you still have the conditional
  %if 0%{?fedora} >= 8
which will always be satisfied.  Is there some reason you can't remove that?  It will not help your epel builds, because %fedora is never defined there.
Comment 12 Peter Lemenkov 2009-05-17 08:37:18 EDT
(In reply to comment #11)
> OK, this does build on F11, rpmlint is clean, the versioning is proper and
> %description looks OK to me.  However, you still have the conditional
>   %if 0%{?fedora} >= 8
> which will always be satisfied.  Is there some reason you can't remove that? 
> It will not help your epel builds, because %fedora is never defined there.  

I just revised this spec-file and found that the requirement for python-setuptools is superfluous - I removed it (along with pkgconfig - another leftover)


Koji scratch builds for F-11 and EL-5

http://koji.fedoraproject.org/koji/taskinfo?taskID=1358724
http://koji.fedoraproject.org/koji/taskinfo?taskID=1358726

http://peter.fedorapeople.org/python-sippy.spec
http://peter.fedorapeople.org/python-sippy-0-0.8.20090429cvs.fc10.src.rpm
Comment 13 Jason Tibbitts 2009-05-27 17:05:26 EDT
OK, looks good to me.

APPROVED
Comment 14 Peter Lemenkov 2009-05-27 23:58:26 EDT
Thanks!

New Package CVS Request
=======================
Package Name: python-sippy
Short Description: B2BUA (back-to-back user agent) SIP call controlling component
Owners: peter
Branches: F-10 F-11 EL-4 EL-5
InitialCC: peter
Comment 15 Jason Tibbitts 2009-05-28 12:04:14 EDT
CVS done.
Comment 16 Fedora Update System 2009-05-28 13:21:16 EDT
python-sippy-0-0.8.20090429cvs.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/python-sippy-0-0.8.20090429cvs.fc10
Comment 17 Fedora Update System 2009-05-28 13:21:24 EDT
python-sippy-0-0.8.20090429cvs.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/python-sippy-0-0.8.20090429cvs.fc11
Comment 18 Fedora Update System 2009-05-29 22:27:51 EDT
python-sippy-0-0.8.20090429cvs.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update python-sippy'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-5653
Comment 19 Fedora Update System 2009-05-29 22:35:27 EDT
python-sippy-0-0.8.20090429cvs.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update python-sippy'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-5692
Comment 20 Fedora Update System 2009-06-15 22:08:48 EDT
python-sippy-0-0.8.20090429cvs.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 21 Fedora Update System 2009-06-15 22:41:03 EDT
python-sippy-0-0.8.20090429cvs.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

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