Bug 455137 - Review Request: soundmodem - Soundcard Packet Radio Modem
Summary: Review Request: soundmodem - Soundcard Packet Radio Modem
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-07-12 16:11 UTC by Lucian Langa
Modified: 2008-12-08 10:51 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-12-08 10:51:34 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)
remove spurious printf's (593 bytes, patch)
2008-12-06 17:28 UTC, Alan Crosswell
no flags Details | Diff
SPEC to apply spurious printf patch (2.11 KB, application/octet-stream)
2008-12-06 17:31 UTC, Alan Crosswell
no flags Details
applies soundmodem-spurious-printf.patch (2.81 KB, application/octet-stream)
2008-12-06 17:39 UTC, Alan Crosswell
no flags Details

Description Lucian Langa 2008-07-12 16:11:29 UTC
Spec URL: http://lucilanga.fedorapeople.org/soundmodem.spec
SRPM URL: http://lucilanga.fedorapeople.org/soundmodem-0.10-1.fc9.src.rpm
Description: This package contains the driver and the diagnostic utility for
userspace SoundModem. It allows you to use soundcards supported
by OSS/Free as Amateur Packet Radio modems.

Comment 1 Bill Nottingham 2008-07-17 14:51:33 UTC
Our drivers aren't OSS/Free (they're alsa), so that description could probably
use some rewording. Maybe just 's|supported by OSS/Free||'

Comment 2 Lucian Langa 2008-07-17 15:08:01 UTC
I've updated the description and bumped version

new files:

http://lucilanga.fedorapeople.org/soundmodem.spec
http://lucilanga.fedorapeople.org/soundmodem-0.10-2.fc9.src.rpm

Comment 3 Jason Tibbitts 2008-11-20 17:57:36 UTC
This failed to build for me on x86_64 with current rawhide:

+ ./configure --build=x86_64-unknown-linux-gnu --host=x86_64-unknown-linux-gnu --target=x86_64-redhat-linux-gnu --program-prefix= --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/usr/com --mandir=/usr/share/man --infodir=/usr/share/info
checking build system type...
Invalid configuration `x86_64-unknown-linux-gnu': machine `x86_64-unknown' not recognized
configure: error: /bin/sh ./config.sub x86_64-unknown-linux-gnu failed

I do not know what that means.  Maybe the configure script was simply generated by a really old autotools version?

Comment 4 Lucian Langa 2008-11-21 14:22:08 UTC
(In reply to comment #3)
> I do not know what that means.  Maybe the configure script was simply generated
> by a really old autotools version?
I've used autotools to update.

new ver:
http://lucilanga.fedorapeople.org/soundmodem.spec
http://lucilanga.fedorapeople.org/soundmodem-0.10-3.fc10.src.rpm

Comment 5 Jason Tibbitts 2008-12-03 01:53:45 UTC
Builds fine and rpmlint is silent.

The documentation files are duplicated between the main and -devel package.

Unfortunately, modem.h and simd.h are far too generic to place in /usr/include.

* source files match upstream.  sha256sum:
   e7a42c413a180b873ae76b2c252904a3e34c9807c2604f2315426443d9e28627  
   soundmodem-0.10.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint is silent.
* final provides and requires are sane:
  soundmodem-0.10-3.fc11.x86_64.rpm
   soundmodem = 0.10-3.fc11
   soundmodem(x86-64) = 0.10-3.fc11
  =
   /bin/sh
   chkconfig
   initscripts
   libX11.so.6()(64bit)
   libXext.so.6()(64bit)
   libXi.so.6()(64bit)
   libasound.so.2()(64bit)
   libasound.so.2(ALSA_0.9)(64bit)
   libasound.so.2(ALSA_0.9.0rc4)(64bit)
   libaudiofile.so.0()(64bit)
   libgdk-1.2.so.0()(64bit)
   libglib-1.2.so.0()(64bit)
   libgmodule-1.2.so.0()(64bit)
   libgtk-1.2.so.0()(64bit)
   libutil.so.1()(64bit)
   libutil.so.1(GLIBC_2.2.5)(64bit)
   libxml.so.1()(64bit)

  soundmodem-devel-0.10-3.fc11.x86_64.rpm
   soundmodem-devel = 0.10-3.fc11
   soundmodem-devel(x86-64) = 0.10-3.fc11
  =
   soundmodem = 0.10-3.fc11

* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
X generically named files
* scriptlets present OK (service management).
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headers are in the -devel package.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.

Comment 6 Lucian Langa 2008-12-03 10:59:09 UTC
(In reply to comment #5)
> The documentation files are duplicated between the main and -devel package.
Updated, there's no doc files for devel package.


> Unfortunately, modem.h and simd.h are far too generic to place in /usr/include.
Moved'em to /usr/include/soundmodem.

http://lucilanga.fedorapeople.org/soundmodem.spec
http://lucilanga.fedorapeople.org/soundmodem-0.10-4.fc10.src.rpm

Comment 7 Alan Crosswell 2008-12-06 17:28:16 UTC
Created attachment 326008 [details]
remove spurious printf's

I've removed two spurious printfs that were resulting in the syslog buffers getting filled up when soundmodem runs as daemon.  I've been running soundmodem in two production aprsdigi's for several years with this patch.  Without it, soundmodem would randomly exit.

Comment 8 Alan Crosswell 2008-12-06 17:31:21 UTC
Created attachment 326009 [details]
SPEC to apply spurious printf patch

This is the SPEC to apply the spurious printf's patch.

Comment 9 Alan Crosswell 2008-12-06 17:39:07 UTC
Created attachment 326010 [details]
applies soundmodem-spurious-printf.patch

Sorry, I patched against what is in the fedora SRPM.  Here's the update against the 10.4 spec.

Comment 10 Jason Tibbitts 2008-12-06 17:48:49 UTC
The issues I had with this review have been fixed.  It would be nice to address
the above mentioned code quality issue, which I have no way to test, but I'm
going to leave that up to Lucian.  Alan, if your issue is not addressed when
this package is imported, please consider filing a bug against it. 
Unfortunately you can't do that now because the review isn't finished and so
the bugzilla component doesn't exist yet.  It should be available soon after
the CVS request has been processed.

Anyway, I'm going to approve this and leave it to Lucian to decide what to do
with the above patch.

APPROVED

Comment 11 Lucian Langa 2008-12-06 21:20:46 UTC
Thank you, Jason.
Alan, thank you for the patch, I'm going to handle this after I check this in.

New Package CVS Request
=======================
Package Name: soundmodem
Short Description: Soundcard Packet Radio Modem
Owners: lucilanga
Branches: F-9 F-10 EL-5
InitialCC:

Comment 12 Kevin Fenzi 2008-12-07 03:30:33 UTC
cvs done.

Comment 13 Fedora Update System 2008-12-08 10:48:23 UTC
soundmodem-0.10-5.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/soundmodem-0.10-5.fc9

Comment 14 Fedora Update System 2008-12-08 10:49:35 UTC
soundmodem-0.10-5.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/soundmodem-0.10-5.fc10


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