Bug 468631 - Review Request: libgarmin - C library to parse and use Garmin image files
Summary: Review Request: libgarmin - C library to parse and use Garmin image files
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: 461849 navit
TreeView+ depends on / blocked
 
Reported: 2008-10-26 23:13 UTC by Fabian Affolter
Modified: 2009-05-28 08:08 UTC (History)
7 users (show)

Fixed In Version: 0-0.6.20090212svn.fc11
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-04-27 21:18:36 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Fabian Affolter 2008-10-26 23:13:54 UTC
Spec URL: http://fab.fedorapeople.org/packages/SRPMS/libgarmin.spec
SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/libgarmin-0-0.1.20081026svn.fc9.src.rpm

URL: http://libgarmin.sourceforge.net/

Description:
Libgarmin is an in C written library for Garmin image format maps. The 
library can be used to parse Garmin image files.

Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=904966

[rpm@laptop024 SRPMS]$ rpmlint -i libgarmin*
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

[rpm@laptop024 i386]$ rpmlint libgarmin*
3 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 1 Ralf Corsepius 2008-10-27 02:17:01 UTC
Just a couple of remarks:

(In reply to comment #0)
> [rpm@laptop024 i386]$ rpmlint libgarmin*
> 3 packages and 0 specfiles checked; 0 errors, 0 warnings.
Interesting, rpmlint is worse than I had thought:)

MUSTFIX
- libgarmin.a is missing from *-devel
Without, this *-devel rpm is widely useless.

- package doesn't support shared libraries
=> Must add *-static magic to *-devel

- Wrong License:
Your spec says: License:        GPLv2+
Several files (e.g. libgarmin.h and COPYING) inside of the sources say:
 GPLv2 *only*.

Comment 2 Fabian Affolter 2008-10-29 00:17:20 UTC
Thanks Ralf for the comments

(In reply to comment #1)
> Just a couple of remarks:
> 
> (In reply to comment #0)
> MUSTFIX
> - libgarmin.a is missing from *-devel
> Without, this *-devel rpm is widely useless.

fixed

> - package doesn't support shared libraries
> => Must add *-static magic to *-devel

fixed.  I added '--enable-static=no' to %configure
 
> - Wrong License:
> Your spec says: License:        GPLv2+
> Several files (e.g. libgarmin.h and COPYING) inside of the sources say:
>  GPLv2 *only*.

fixed

SPEC: http://fab.fedorapeople.org/packages/SRPMS/libgarmin.spec
SRPM: http://fab.fedorapeople.org/packages/SRPMS/libgarmin-0-0.2.20081026svn.fc9.src.rpm

Comment 3 Michael Schwendt 2008-11-10 00:38:20 UTC
> %files
> %defattr(-,root,root,-)
> %doc AUTHORS COPYING INSTALL README TODO

Absolutely no need to include the same %doc files also in
the -devel pkg.

> %{_bindir}/gar*
> %{_datadir}/%{name}/garmintypes.txt

Don't forget the corresponding directory entry!
Add:  %dir %{_datadir}/%{name}

> %files devel
> %defattr(-,root,root,-)
> %doc AUTHORS COPYING INSTALL README TODO
> %{_datadir}/%{name}/doc/*

Here either replace your line with

   %{_datadir}/%{name}/doc/

to add that directory and its contents recursively, or add:

  %dir %{_datadir}/%{name}/doc

to the -devel pkg files section.

> %{_libdir}/pkgconfig/%{name}.pc

You put a file in there, so "Requires: pkgconfig".

Comment 4 Fabian Affolter 2008-11-10 20:06:28 UTC
(In reply to comment #3)
> > %files
> > %defattr(-,root,root,-)
> > %doc AUTHORS COPYING INSTALL README TODO
> 
> Absolutely no need to include the same %doc files also in
> the -devel pkg.

fixed

> > %{_bindir}/gar*
> > %{_datadir}/%{name}/garmintypes.txt
> 
> Don't forget the corresponding directory entry!
> Add:  %dir %{_datadir}/%{name}

fixed

> > %files devel
> > %defattr(-,root,root,-)
> > %doc AUTHORS COPYING INSTALL README TODO
> > %{_datadir}/%{name}/doc/*
> 
> Here either replace your line with
> 
>    %{_datadir}/%{name}/doc/
> 
> to add that directory and its contents recursively, or add:
> 
>   %dir %{_datadir}/%{name}/doc
> 
> to the -devel pkg files section.

fixed

> > %{_libdir}/pkgconfig/%{name}.pc
> 
> You put a file in there, so "Requires: pkgconfig".

fixed

SPEC: http://fab.fedorapeople.org/packages/SRPMS/libgarmin.spec
SRPM:
http://fab.fedorapeople.org/packages/SRPMS/libgarmin-0-0.3.20081026svn.fc9.src.rpm

Comment 5 Fabian Affolter 2008-12-14 20:22:57 UTC
* Sun Dec 14 2008 Fabian Affolter <fabian> - 0-0.4.20081214svn
- Updated from revision 307 to revision 315

SPEC: http://fab.fedorapeople.org/packages/SRPMS/libgarmin.spec
SRPM:
http://fab.fedorapeople.org/packages/SRPMS/libgarmin-0-0.4.20081214svn.fc9.src.rpm

Comment 6 Balint Cristian 2009-01-03 17:15:28 UTC
Folks,

- I think this package violates Garmin (TM) proprietary format and their patents.We should be careful by adding such a package to Fedora, Garmin (TM) never released any specifications of their format, everything is a pure reverse 
engineering. On top of all it contains some patent subjected (not sure which one) which allows very smart routing information store.

- Legal folk, can state some points over this package ?

Comment 7 Peter Lemenkov 2009-01-03 18:03:09 UTC
> I think this package violates Garmin (TM) proprietary format and their
patents

You're not the Garmin's legal representative to make such statement.

Comment 8 Fabian Affolter 2009-01-03 18:44:08 UTC
Add Legal Blocker.  Let's see what's Fedora Legal says.

Comment 9 Tom "spot" Callaway 2009-01-12 20:46:35 UTC
I can find no patents in a preliminary search that this would violate. While the format is clearly not documented publicly, there is no problem with a reverse engineered implementation, especially since this is being done for the explicit reason of providing interoperability with Linux environments.

Lifting FE-Legal.

Comment 11 Tom "spot" Callaway 2009-02-12 21:13:10 UTC
Fabian, please keep the spec file name the same as the SRPM name.

Comment 13 Jason Tibbitts 2009-03-12 02:01:18 UTC
Builds fine and rpmlint is silent.

Since this package has only static libraries, it's OK that they're in the devel package but you must provide libgarmin-static (and any packages that need to link against the .a need build dependencies on it).

Most files don't seem to have a license block; those that do are GPLv2 (only) and the author adds a note at the head of the COPYING file which clarifies v2 only for the rest.

The description could use a little cleanup. I would suggest something like:
  Libgarmin is a library used to parse IMG files from Garmin GPS devices.

The COPYING, REAMDE and TODO files are duplicated.  You should choose one package to contain them.


* source files match upstream (manually compared with checkout).
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
X description could use a bit of grammar 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.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint is silent.
X final provides and requires:

  libgarmin-0-0.5.20090212svn.fc11.x86_64.rpm
   libgarmin = 0-0.5.20090212svn.fc11
   libgarmin(x86-64) = 0-0.5.20090212svn.fc11
  =
   (none)

  libgarmin-devel-0-0.5.20090212svn.fc11.x86_64.rpm
   pkgconfig(libgarmin) = 0.1
   libgarmin-devel = 0-0.5.20090212svn.fc11
   libgarmin-devel(x86-64) = 0-0.5.20090212svn.fc11
X  needs libgarmin-static 
  =
   /usr/bin/pkg-config
   libgarmin = 0-0.5.20090212svn.fc11
   pkgconfig

* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
X 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.
* headers are in the -devel package.
* pkgconfig files are in the -devel package; pkgconfig dependency is present.
X static libraries are in the -devel package, but there is no -static provide.
* no libtool .la files.

The package review process needs reviewers!  If you haven't done any package
reviews recently, please consider doing one.

Comment 14 Fabian Affolter 2009-04-08 09:12:00 UTC
Jason, thanks for your input.  I will try to update this package soon.

Comment 15 Michael Schwendt 2009-04-08 11:58:32 UTC
> Group:          Applications/Productivity

System Environment/Libraries


> %configure --enable-static=no

Wrong or not recognised option it seems. Library is static-only, and as soon as it would build shared libs, build would fail anyway due to installed files not included in %files.


> %doc AUTHORS COPYING INSTALL README TODO

In case this is the standard "INSTALL" file, it is irrelevant to RPM package
users and typically is not included.

Comment 17 Jason Tibbitts 2009-04-16 18:00:45 UTC
Removing the duplicated documentation caused a no-documentation complaint from rpm on the -devel package, but that's not an issue.

Otherwise I think this is fine now.  APPROVED

Comment 18 Fabian Affolter 2009-04-16 22:12:07 UTC
New Package CVS Request
=======================
Package Name: libgarmin
Short Description: C library to parse and use Garmin image files
Owners: fab
Branches: F-9 F-10 F-11
InitialCC:

Comment 19 Kevin Fenzi 2009-04-17 16:23:57 UTC
cvs done.

Comment 20 Fedora Update System 2009-04-17 19:43:02 UTC
libgarmin-0-0.6.20090212svn.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/libgarmin-0-0.6.20090212svn.fc10

Comment 21 Fedora Update System 2009-04-17 19:43:07 UTC
libgarmin-0-0.6.20090212svn.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/libgarmin-0-0.6.20090212svn.fc11

Comment 22 Fedora Update System 2009-04-17 19:43:12 UTC
libgarmin-0-0.6.20090212svn.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/libgarmin-0-0.6.20090212svn.fc9

Comment 23 Fedora Update System 2009-04-22 00:50:37 UTC
libgarmin-0-0.6.20090212svn.fc9 has been pushed to the Fedora 9 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-newkey update libgarmin'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2009-3770

Comment 24 Fedora Update System 2009-04-22 00:55:08 UTC
libgarmin-0-0.6.20090212svn.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 libgarmin'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-3799

Comment 25 Fedora Update System 2009-04-27 21:18:28 UTC
libgarmin-0-0.6.20090212svn.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 26 Fedora Update System 2009-04-27 21:24:25 UTC
libgarmin-0-0.6.20090212svn.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 27 Fedora Update System 2009-05-28 08:08:11 UTC
libgarmin-0-0.6.20090212svn.fc11 has been pushed to the Fedora 11 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.