Bug 669010 - Review Request: libfap - C port of Ham::APRS::FAP APRS Parser
Review Request: libfap - C port of Ham::APRS::FAP APRS Parser
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
low Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Extras Quality Assurance
Depends On:
Blocks: 670915
  Show dependency treegraph
Reported: 2011-01-12 07:58 EST by Andrew Elwell
Modified: 2013-01-22 20:52 EST (History)
3 users (show)

See Also:
Fixed In Version: libfap-1.0-3.fc13
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2011-02-09 12:25:05 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
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 Andrew Elwell 2011-01-12 07:58:16 EST
Spec URL: http://dl.dropbox.com/u/6594808/Fedora/libfap.spec
SRPM URL: http://dl.dropbox.com/u/6594808/Fedora/libfap-0.9-1.fc14.src.rpm
Description: libfap is a C port of the Ham::APRS::FAP Finnish APRS Parser (Fabulous APRS Parser) Perl module. As the original Perl code, libfap parses normal, mic-e and compressed location packets, NMEA location packets, objects, items, messages, telemetry and most weather packets. For more description, see the Perl module.

1) It's a dual licenced library (Artistic or GPL) -- Although Artistic 1.0 is NOT allowed as a fedora licence on its own, but IS allowed as a Perl licence (from the table https://fedoraproject.org/wiki/Licensing:Main#Good_Licenses )

I have asked upstream (Tapio Aaltonen) to clarify this but no email response yet (only sent today) as they state in COPYING "Libfap may be copied only under the terms of either the Artistic License or the GNU General Public License. These licenses can be found in the licences directory of this package." without explicitly mentioning modification.

2) not all the text files included with the source are packaged (NEWS provides no information and INSTALL is the generic one -- as per https://fedoraproject.org/wiki/Packaging/Guidelines#Documentation)

3) there is a test application 'smoketest' that is optional (make check) -- again should this be packaged with the relevant parts of the Makefile? It doesn't look like its a useful standalone application.

4) rpmlint output:
$ rpmlint libfap*
1 packages and 1 specfiles checked; 0 errors, 0 warnings.

I am still looking for a sponsor.
Comment 1 g.trentalancia 2011-01-12 10:35:12 EST
Here is my advice on possible ways to improve the package:

- you should modify the "License" tag to GPLv3 or GPLv3+ (the authors should provide you with further information on whether it's only GPL version 3 or also any later version). Remove Artistic because this is not a Perl module and, as you said, Artistic 1.0 is not allowed in Fedora
- permission of libfap.spec is 0664, this should be changed to 0644
Comment 2 Andrew Elwell 2011-01-17 04:05:11 EST
Updates from upstream author about licencing:

>> I'm packaging up libfap into an RPM for Fedora, but I notice all the
>> web pages mention 'copying' under GPL / artistic licence, but can you
>> explicitly mention modification?
> Sure, my mistake. Now it is mentioned.
>> also can you say what version of the artistic licence is used
>> (clarified or 2.0) (as v 1.0 isn't acceptable apparently)
> My intention is to follow the licensing of the original Perl code, which
> uses v. 1.0. This is now also mentioned.
> Many thanks for pointing out those issues and especially packing libfap as
> an RPM! Is it available somewhere on the Internets, so that I could link to
> it?"

and then again:
> Hi again,
> On 12.01.2011 10:43:18, Andrew Elwell wrote:
>> also can you say what version of the artistic licence is used
>> (clarified or 2.0) (as v 1.0 isn't acceptable apparently)
> I forgot to mention that if the dual licensing model is an issue, drop the
> Artistic License. You can also use more recent version of the GPL if you
> want. The licensing terms allow both actions.
> If libfap needs to be modified in order to make an RPM of it, it is totally
> ok with me to release the modifications only under GPLv3 for example. But if
> you make other changes, it would be nice to have them available under the
> original licensing terms, so that they can be migrated back if wanted. But
> that is of course for you to decide.
Comment 3 Andrew Elwell 2011-01-20 04:23:40 EST
Upstream released v1.0 on the 17th. Updated spec and srpm available:

Spec URL: http://dl.dropbox.com/u/6594808/Fedora/libfap.spec
SRPM URL: http://dl.dropbox.com/u/6594808/Fedora/libfap-1.0-1.fc14.src.rpm

Mock build OK
State Changed: build
INFO: Done(/home/aelwell/rpmbuild/SRPMS/libfap-1.0-1.fc14.src.rpm) Config(default) 1 minutes 8 seconds
INFO: Results and/or logs in: /var/lib/mock/fedora-14-x86_64/result

rpmlint output looks OK:
[aelwell@pcitgtelwell libfap]$ rpmlint ~/Dropbox/Public/Fedora/libfap-1.0-1.fc14.src.rpm ~/Dropbox/Public/Fedora/libfap.spec 
1 packages and 1 specfiles checked; 0 errors, 0 warnings.
Comment 4 Jason Tibbitts 2011-01-20 21:09:19 EST
Builds fine and rpmlint is completely silent.  (But note that it's not sufficient to run rpmlint on the source package; you need to run it on the generated binary packages as well.)

As before, you can dispense with BuildRoot, %clean and the first line of %install if you don't intend to target el4 or el5 with the same spec.

I'm not sure there's much point in mentioning the Perl module in the Summary: or in the %description.  Why not just describe what the package does?  Especially in the %description, there's not much point in referring people to the Perl module when we don't even package it.

I'm pretty sure that the current License: tag is OK.  It's rare to see non-Perl code under that License, but I see no problem with it.  I'm not sure what was up with comment #1, since there's no mention of GPLv3 anywhere.  We do permit code under the Artistic license as long as it's dual-licensed with something acceptable, and we do continue to list Artistic as one of the licenses in that case.

Several documentation files are duplicated between the main and -devel packages.  This needs fixing.

fap.h looks relatively generic, but I did some searching and didn't find anything else it might conflict with, so it seems OK.

* source files match upstream.  sha256sum:
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
X summary could use work.
X description could use work.
* dist tag is present.
* license field matches the actual license.
* license is open source-compatible.
* license texts included in package.
* latest version is being packaged.
* BuildRequires are proper (none).
* compiler flags are appropriate.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint is silent.
* final provides and requires are sane:
   libfap = 1.0-1.fc15
   libfap(x86-64) = 1.0-1.fc15

   libfap-devel = 1.0-1.fc15
   libfap-devel(x86-64) = 1.0-1.fc15
   libfap = 1.0-1.fc15

* no bundled libraries.
* shared libraries installed:
   ldconfig is called properly.
   unversioned .so files are in the -devel package.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
X several duplicates in %files.
* file permissions are appropriate.
* no generically named files.
* scriptlets are OK (ldconfig).
* 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 static libraries.
* no libtool .la files.
Comment 5 g.trentalancia 2011-01-20 21:34:53 EST
If you look at version 0.9, which was available at the time I made my comment, then you will discover that the package was carrying GPLv3 (file licenses/GPL.txt). This is no longer true with the version 1.0 of the same package, but my comment was made about ten days ago before this new version came out.

Apparently now the license is GPL version 1 or greater. Hopefully they won't change their mind again in the future...
Comment 6 Andrew Elwell 2011-01-21 03:21:00 EST
Updated spec and rebuilt. 

Spec URL: http://dl.dropbox.com/u/6594808/Fedora/libfap.spec
SRPM URL: http://dl.dropbox.com/u/6594808/Fedora/libfap-1.0-2.fc14.src.rpm

build cleanly in mock (F14).

libfap-devel now simply contains
$ rpm -qlp ~/rpmbuild/RPMS/x86_64/libfap-devel-1.0-2.fc14.x86_64.rpm 

I'm leaving in the BuildRoot etc as I'd like it to be available in EPEL5 (and I've just tested it builds OK under mock, subject to building the srpm on F14 with rpmbuild -bs --define _source_filedigest_algorithm=1 libfap.spec(

I've not expanded the acronymn APRS in the Summary as it'll take up too many characters, but have in the Description.

rpmlint now throws a warning on the -devel about no documentation - can I ignore this as the main package (which is a requires: for -devel) pulls it in? (I think this was the reason I originally duplicated those files!)

[aelwell@pcitgtelwell ~]$ rpmlint ~/rpmbuild/RPMS/x86_64/libfap-*1.0-2*
libfap-devel.x86_64: W: no-documentation
3 packages and 0 specfiles checked; 0 errors, 1 warnings.
Comment 7 Jason Tibbitts 2011-01-21 14:56:10 EST
Yes, it's perfectly OK that rpmlint complain about no-documentation when you don't actually have any documentation.  You shouldn't invent or duplicate some just to quite rpmlint.  Sometimes you can decide that some documentation should go in the -devel package and some in the main package, but that doesn't seem to be the case here.

Generally for summaries (and especially for something like a library that users will rarely install on their own anyway) there's not much in indicating what language the package is written in.  It's certainly not worth arguing over, though.  Just imagine that you installed some application and it pulled in this library.  Does the summary provide enough information for you to make a quick judgment about whether you really want that install to go ahead?

Otherwise this looks OK to me.  However, I just now noticed your message about smoketest.c.  If at all possible it would be nice to get that built so it could be run in a %check section.  Just adding

make check

is sufficient and gives a bunch of useful information.

Anyway, with that, this is about done.  I need to look over your pre-reviews and then I'll push the necessary buttons.
Comment 8 Andrew Elwell 2011-01-21 15:46:07 EST
Great, didn't know about make check. Added to spec, and it passed on F14. 
Mock build went OK and ran all the tests sucessfully.

Spec URL: http://dl.dropbox.com/u/6594808/Fedora/libfap.spec
SRPM URL: http://dl.dropbox.com/u/6594808/Fedora/libfap-1.0-3.fc14.src.rpm
Comment 9 Jason Tibbitts 2011-01-21 18:27:09 EST
Looks great; APPROVED

I've already updated your privileges in the account system; that may take a bit of time to propagate but once it has you'll be able to make an SCM request and set the fedora-cvs flag.  See https://fedoraproject.org/wiki/Package_SCM_admin_requests for more info on that.
Comment 10 Andrew Elwell 2011-01-22 04:51:33 EST
New Package SCM Request
Package Name: libfap
Short Description: Amateur Radio APRS parser
Owners: elwell
Branches: f13 f14 el6
InitialCC: hams-sig
Comment 11 Jason Tibbitts 2011-01-22 10:50:06 EST
Git done (by process-git-requests).
Comment 12 Fedora Update System 2011-01-24 07:33:53 EST
libfap-1.0-3.fc14 has been submitted as an update for Fedora 14.
Comment 13 Fedora Update System 2011-01-24 07:38:17 EST
libfap-1.0-3.fc13 has been submitted as an update for Fedora 13.
Comment 14 Fedora Update System 2011-01-24 07:40:54 EST
libfap-1.0-3.el6 has been submitted as an update for Fedora EPEL 6.
Comment 15 Fedora Update System 2011-01-24 13:02:58 EST
libfap-1.0-3.el6 has been pushed to the Fedora EPEL 6 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 libfap'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/libfap-1.0-3.el6
Comment 16 Fedora Update System 2011-02-09 12:24:58 EST
libfap-1.0-3.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 17 Fedora Update System 2011-02-09 15:23:48 EST
libfap-1.0-3.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 18 Fedora Update System 2011-03-21 16:56:04 EDT
libfap-1.0-3.fc13 has been pushed to the Fedora 13 stable repository.

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