Bug 461050 - Review Request: tucnak2 - VHF contest logging program
Summary: Review Request: tucnak2 - VHF contest logging program
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: 461007
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-09-03 17:41 UTC by Lucian Langa
Modified: 2009-03-23 06:39 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-03-23 06:39:56 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Lucian Langa 2008-09-03 17:41:02 UTC
Spec URL: http://lucilanga.fedorapeople.org/tucnak2.spec
SRPM URL: http://lucilanga.fedorapeople.org/tucnak2-2.13-2.fc9.src.rpm
Description: Tucnak2 is VHF/UHF/SHF log for hamradio contests. It supports multi
bands, free input, networking, voice and CW keyer, WWL database and
much more.

Comment 2 Jason Tibbitts 2008-11-20 21:24:41 UTC
Unfortunately this failed to build for me on x86_64, rawhide:

checking size of char...
configure: error: cannot compute sizeof (char), 77
See `config.log' for more details.

The end of config.log contains:

configure:5922: checking size of char
configure:6234: gcc -o conftest -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -I/usr/include/SDL -D_GNU_SOURCE=1 -D_REENTRANT -I/usr/include/libpng12         -z now conftest.c -lgpm  -lm -lutil -lglib-2.0   -pthread -lgthread-2.0 -lrt -lglib-2.0   -lSDL -lpthread -lpng12    -lsndfile   -lasound -lftdi -lusb    >&5
/usr/bin/ld: cannot find -lusb
collect2: ld returned 1 exit status
configure:6237: $? = 1
configure: program exited with status 1

I have no idea why libusb is needed to compute sizeof(char), but there it is.

Comment 3 Lucian Langa 2008-11-21 06:44:04 UTC
(In reply to comment #2)

> configure:6234: gcc -o conftest -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2
> -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
> -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -pthread
> -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -I/usr/include/SDL
> -D_GNU_SOURCE=1 -D_REENTRANT -I/usr/include/libpng12         -z now conftest.c
> -lgpm  -lm -lutil -lglib-2.0   -pthread -lgthread-2.0 -lrt -lglib-2.0   -lSDL
> -lpthread -lpng12    -lsndfile   -lasound -lftdi -lusb    >&5
> /usr/bin/ld: cannot find -lusb
> collect2: ld returned 1 exit status
> configure:6237: $? = 1
> configure: program exited with status 1
> 
> I have no idea why libusb is needed to compute sizeof(char), but there it is.

conftest is compiled against $LIBS which contains among other things -lusb.
Anyway it should be harmless as tucnak2 requires libftdi which requires libusb.
Updated BR...

new version:
http://lucilanga.fedorapeople.org/tucnak2.spec
http://lucilanga.fedorapeople.org/tucnak2-2.14-2.fc10.src.rpm

Comment 5 Lucian Langa 2008-12-03 09:56:20 UTC
Update bug dependency, Tucnak2 does not depend on ssbd.

Comment 6 Jason Tibbitts 2008-12-06 21:08:26 UTC
I noticed that /usr/bin/soundwrapper seemed a bit generic and did a search.  I note that nspluginwrapper will automatically try to call a program of that name if it exists.  I think it would be a pretty bad idea to package a program of that name.

DO you know what it is used for in this program?  Can it be removed or renamed?

Comment 7 Lucian Langa 2008-12-07 18:41:22 UTC
(In reply to comment #6)
> DO you know what it is used for in this program?  Can it be removed or renamed?
Upstream hasn't decided whether to rename of remove this program.
I decided to rename this to tucnak2-soundwrapper meanwhile.
Also updated to the latest version.

new version:
http://lucilanga.fedorapeople.org/tucnak2.spec
http://lucilanga.fedorapeople.org/tucnak2-2.21-1.fc10.src.rpm

Comment 8 Jason Tibbitts 2009-03-12 03:22:21 UTC
Builds fine and rpmlint is silent.

I believe the license of this program is GPLv2 (only); most of the source files just say "version 2" with no "or later" clause.

I note that a newer version is out.  I don't think it will significantly effect the packaging, but you can update if you like and I'll look it over.

Note that the touch call in your recode function is backwards, so you don't actually preserve the date.  It's not a big deal, but since you went to the effort....

I wonder about the files in /usr/share/tucnak2.  If they're not actually used by the problem, would they be better off packaged as documentation?  (Not that 100K of files really matter much, but I guess it's worth asking.)

The desktop file has an error:
  key "Categories" is a list and does not have a semicolon as trailing 
  character, fixing

Since this file comes from upstream, I don't really see a need to patch it but you might want to inform upstream about it.

I installed and ran this and it seemed to work, but I can get it to segfault repeatably by bringing up a map.  Honestly I have no clue at all how to use the software so I was just blindly poking keys.  That might be sufficiently crippling that it should be fixed before importing, but I don't really know.

* source files match upstream.  sha256sum:
   16ad9461034b4db7fc14848820f620bf978e523436547c67d6974ea36a730069  
   tucnak2-2.21.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.
X license field does not match the actual license.
* license is open source-compatible.
* license text included in package.
* 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:
   tucnak2 = 2.21-1.fc11
   tucnak2(x86-64) = 2.21-1.fc11
  =
   /usr/bin/perl
   libSDL-1.2.so.0()(64bit)
   libasound.so.2()(64bit)
   libasound.so.2(ALSA_0.9)(64bit)
   libasound.so.2(ALSA_0.9.0rc4)(64bit)
   libftdi.so.1()(64bit)
   libglib-2.0.so.0()(64bit)
   libgpm.so.2()(64bit)
   libgthread-2.0.so.0()(64bit)
   libhamlib.so.2()(64bit)
   libpng12.so.0()(64bit)
   libpng12.so.0(PNG12_0)(64bit)
   libsndfile.so.1()(64bit)
   libsndfile.so.1(libsndfile.so.1.0)(64bit)
   libusb-0.1.so.4()(64bit)
   libutil.so.1()(64bit)
   libutil.so.1(GLIBC_2.2.5)(64bit)

* 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.
* no generically named files
* no scriptlets present.
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.
o desktop files valid and installed properly (one desktop-file-complaint, should 
   be reported upstream).

Comment 9 Lucian Langa 2009-03-13 09:43:59 UTC
(In reply to comment #8)
> I believe the license of this program is GPLv2 (only); most of the source files
> just say "version 2" with no "or later" clause.

 
> I note that a newer version is out.  I don't think it will significantly effect
> the packaging, but you can update if you like and I'll look it over.
Updated to 2.25.


> Note that the touch call in your recode function is backwards, so you don't
> actually preserve the date.  It's not a big deal, but since you went to the
> effort....
Fixed.

> I wonder about the files in /usr/share/tucnak2.  If they're not actually used
> by the problem, would they be better off packaged as documentation?  (Not that
> 100K of files really matter much, but I guess it's worth asking.)
Well actually some of the files from that directory are used, the README file in that directory is outdated. I am also trying to send a patch upstream that enables lookup files from that directory as a fallback.

> The desktop file has an error:
>   key "Categories" is a list and does not have a semicolon as trailing 
>   character, fixing
Fixed.


> I installed and ran this and it seemed to work, but I can get it to segfault
> repeatably by bringing up a map.  Honestly I have no clue at all how to use the
> software so I was just blindly poking keys.  That might be sufficiently
> crippling that it should be fixed before importing, but I don't really know.
it shouldn't happen and it seems I cannot reproduce this.
there is an initial configuration dialog that asks among other things for callsign and WWL (world wide locator). It is possible that this field is not correctly validated. I wonder if this happens after update, could you send me the generated file ~/tucnac/tucnakrc?

new version:
http://lucilanga.fedorapeople.org/tucnak2.spec
http://lucilanga.fedorapeople.org/tucnak2-2.25-1.fc10.src.rpm

Comment 10 Jason Tibbitts 2009-03-16 22:21:36 UTC
The new package builds fine and the issues I had, save the crash are fixed.

As for the crash, I installed the new package on my rawhide guest and ran it work, displaying over the LAN and the map came up OK.  The initial page was filled with garbage but it went away when I scrolled or zoomed.  However, running the same package displaying over the network to my machine here at home does indeed crash.  The OS on both machines doing the display are pretty old, my home machine, where I get the crash is F7; the machine at work is FC5 of all things.

So I don't think this is an input validation thing; the same configuration was used on both runs.  But since it works for you and works for me at least part of the time, I would see this as something to be debugged and addressed later instead of holding up this review.  I really doubt this will be run often under the set of circumstances where I'm seeing the problem.

APPROVED

Comment 11 Lucian Langa 2009-03-17 05:46:54 UTC
Many thanks for the detailed review.

New Package CVS Request
=======================
Package Name: tucnak2
Short Description: VHF contest logging program
Owners: lucilanga
Branches: F-9 F-10 EL-5
InitialCC:

Comment 12 Kevin Fenzi 2009-03-18 03:37:06 UTC
cvs done.


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