Bug 654374 - Review Request: navit - Car navigation system with routing engine
Review Request: navit - Car navigation system with routing engine
Status: CLOSED DEFERRED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michel Alexandre Salim
Fedora Extras Quality Assurance
stalledsubmitter
:
: navit (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-11-17 12:58 EST by viji
Modified: 2011-06-04 06:09 EDT (History)
9 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-06-04 06:09:34 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
michel: fedora‑review?


Attachments (Terms of Use)

  None (edit)
Description viji 2010-11-17 12:58:29 EST
Spec URL: http://viji.fedorapeople.org/SPECS/navit.spec
SRPM URL: http://viji.fedorapeople.org/SRPMS/navit-0.1.2-0.4.20101117svn3684.fc14.src.rpm

Description: Navit is a car navigation system with routing engine. Its modular
design is capable of using vector maps of various formats for routing
and rendering of the displayed map. It's even possible to use multiple
maps at a time.

The GTK+ or SDL user interfaces are designed to work well with touch
screen displays. Points of Interest of various formats are displayed
on the map.

The current vehicle position is either read from 'gpsd' or directly from
NMEA GPS sensors.

The routing engine not only calculates an optimal route to your
destination, but also generates directions and even speaks to you.
Comment 1 Jason Tibbitts 2010-11-17 13:12:32 EST
*** Bug 485652 has been marked as a duplicate of this bug. ***
Comment 2 viji 2010-11-17 13:15:31 EST
$ rpmlint ../SRPMS/navit-0.1.2-0.4.20101117svn3684.fc14.src.rpm

navit.src: W: spelling-error %description -l en_US gpsd -> Gypsy, gypsy, gramps
navit.src: W: invalid-url Source0: http://downloads.sourceforge.net/navit/navit-3684.tar.bz2 HTTP Error 404: Not Found
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

$ rpmlint ../RPMS/x86_64/navit-0.1.2-0.4.20101117svn3684.fc14.x86_64.rpm 
navit.x86_64: W: spelling-error %description -l en_US gpsd -> Gypsy, gypsy, gramps
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

$ rpmlint ../RPMS/x86_64/navit-graphics-sdl-0.1.2-0.4.20101117svn3684.fc14.x86_64.rpm 
navit-graphics-sdl.x86_64: W: spelling-error %description -l en_US xml -> XML, cml, ml
navit-graphics-sdl.x86_64: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

$ rpmlint ../RPMS/x86_64/navit-graphics-qt-0.1.2-0.4.20101117svn3684.fc14.x86_64.rpm 
navit-graphics-qt.x86_64: W: spelling-error %description -l en_US xml -> XML, cml, ml
navit-graphics-qt.x86_64: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

I am not sure what to do with the "invalid-url" warning, the package is an svn checkout.

F14 Scratch
-----------
http://koji.fedoraproject.org/koji/taskinfo?taskID=2606951

F13 Scratch
-----------
http://koji.fedoraproject.org/koji/taskinfo?taskID=2606966
Comment 3 Valent Turkovic 2010-11-20 15:30:27 EST
When will Navit package be in Fedora 14 repos?
Comment 4 viji 2010-11-20 15:39:18 EST
I have contacted my mentor to review this package, he is out of town for couple of days (mostly it will be done by Monday)

If everything goes fine will push to testing and then stable. May be two weeks from now you can do a "yum install navit"
Comment 5 Michel Alexandre Salim 2010-11-26 05:40:26 EST
Hi Viji,

We need the instructions to generate the tarball to have reproducible results; so
the safest way is to use the particular VCS's archiving functionality, *and* specify the desired revision.

In this case, the following works:

svn co https://navit.svn.sourceforge.net/svnroot/navit/trunk/navit
(cd navit ; svn export -r %%{svn} . ../%{name}-%%{svn})
tar cvjf %%{name}-%%{svn}.tar.bz2 %%{name}-%%{svn}

and adjust the %dirname accordingly; I had to add %{svn} otherwise it conflicts with either the directory created by the checkout (navit) or the ./navit subdirectory inside it -- it's quite annoying that SVN does not let one
output directly to stdout, and thus just pipe it directly to tar without having to
create a temporary directory on the FS!

Right now, the MD5sums don't match between your tarball and the one I generate (even if I output to a directory called "navit" without the SVN revision). Creating the archive using svn export, while not guaranteed, would probably be more reliable as the files would be exported in the same order, and so hopefully the filesystem will list it in the same order for tar! Not as good as being able to just stream the output to tar, definitely.
Comment 6 viji 2010-11-26 05:49:14 EST
I will be removing this "svn co" completely as the upstream released 0.2.0 day before yesterday. Will be moving to the same.

will update the srpm and spec in a while.
Comment 7 Michel Alexandre Salim 2010-11-26 06:13:25 EST
Ah, OK. I have a bit of a concern over the way navit comes bundled with so many third-party support libraries (including glib and espeak) -- unfortunately the Koji build logs have been garbage-collected so I need to do another scratch build just to inspect the logs (please provide a new Koji build with the next update).

One thing to note is that we cannot enable espeak support, as it is GPLv3+ whereas navit is GPLv2 only. Could you perhaps bring this up with the developers? If they can relicense to GPLv2+ it would avoid a lot of headaches down the road (I can promise you there will be bug reports to the tune of "Fedora's navit does not have TTS support!").

Using GPL for libraries meant to be reusable is a rather strong-arm tactic, IMHO -- I package bti, developed by a kernel developer who is (like many of them) adamant to stick with GPLv2, and naturally readline was license-upgraded to GPLv3+ ... (leaving the option of compiling against compat-readline v5 or compiling against the non-Unicode-supporting BSD-licensed libedit).
Comment 8 viji 2010-11-26 14:38:23 EST
I have contacted the upstream developers and they are asking re-licensing the same to GPLv2+ would be sufficient or not..

There are some compiling issues, working on it, will be updating the new package soon.

Thank you so much for your support
Comment 9 viji 2010-11-26 15:12:28 EST
I have contacted the upstream developers and they are asking re-licensing the same to GPLv2+ would be sufficient or not..

There are some compiling issues, working on it, will be updating the new package soon.

Thank you so much for your support
Comment 10 viji 2010-11-26 15:57:12 EST
Updated the packages, please review.

Spec URL: http://viji.fedorapeople.org/SPECS/navit.spec
SRPM URL : http://viji.fedorapeople.org/SRPMS/navit-0.2.0-1.fc14.src.rpm

F14 Scratch
-----------
http://koji.fedoraproject.org/koji/taskinfo?taskID=2628765
Comment 11 Ralf Corsepius 2010-11-27 01:20:56 EST
Not a formal review, just some comments.

* I am not sure whether bundling this sample-map is a good idea, because most users will want to remove it and replace it with a map which suites their needs

i.e. I'd propose to either
* entirely remove this map from this package.
* package it as a separate package.
* package it as a "noarch" subpackage of navit.


* I am observing an inpressive amount of warnings when running navit, similar to this one:
navit:displayitem_draw:failed to load icon '/usr/share/navit/xpm/parking.png'


* Your package contains a larger number of *la's.
Are they really needed? I haven't checked, but I am having doubts.
Comment 12 viji 2010-11-27 03:23:45 EST
@Michel

Please ignore the previous spec file and SRPM, I mean the upstream release (0.2.0). We need to use the latest SVN export itself as the 0.2.0 release has many issue with qt, it breaks.

I will be updating a new spec file and srpm with the svn export, the "svn" related issues which you have mentioned will be taken care of.

Thank you so much.
Comment 13 viji 2010-11-27 03:26:58 EST
@Ralf

Thank you so much for your suggestions, I will defiantly have a look at it. I am updating the same to the latest svn export as this release has many issues with qt.
Comment 14 viji 2010-11-28 13:11:32 EST
Bumped to the latest SVN, 0.5.0.

Please do the review and let me know your comments

Spec URL: http://viji.fedorapeople.org/SPECS/navit.spec

SRPM URL: http://viji.fedorapeople.org/SRPMS/navit-0.5.0-0.1.20101128svn3732.fc14.src.rpm

Koji F14 Scratch
----------------
http://koji.fedoraproject.org/koji/taskinfo?taskID=2630789


Issues which Ralf pointed out:

- The map can be on a different package, but I would suggest for the time being we will keep this sample maps as a part of the main package itself so that we can identify the bugs quickly, Eg: icon issues. Also, if we are providing a map data then we need to think about what else can be provided with it and some samples needs to be there for testing.

- The icon issues are fixed, this is not a complete fix. The navit icons comes in different formats, xpm, svg and png. The default /etc/navit/navit.xml has some issues with the file name/format. If you face any issues just check the icon folder and replace the same with the one which is available in navit.xml. This is actually not a bug.

- The .la files are required
Comment 15 viji 2010-11-28 13:58:57 EST
I mean:

- The .la files are NOT required.
Comment 16 viji 2010-11-30 06:00:59 EST
Regarding espeak:

The integrated espeak is only used for win32 or wince. On linux navit just calls the espeak binary. It is automatically disabled on Linux also. Do we really need to think about the re-licensing here?

Also, They upstream guys are not that much happy with GPLv2+, FSF could release a GPLv4 with conditions which they may not like. They are working towards a clean solution, considering

- Re-licensing navit with GPLv2 or GPLv3. 
- Using a downgraded GPLv2 espeak library
Comment 17 Michel Alexandre Salim 2010-12-04 14:45:34 EST
In fact the .la files should be stripped.

I do notice that espeak support is disabled by default anyway, so in that case, no, it's not a problem for *us* (at least at the moment). It's still proper to let upstream know, especially because, since they do turn it on for Win32/WinCE that means those packages are technically illegal!

They don't need to do GPLv2+ -- the other alternatives if they want to use espeak by default on some platforms is to either switch to GPLv3 (only), or dual-license with GPLv2/GPLv3 (if the GPLv3 terms are acceptable, and they just worry about what GPLv4 might look like). *Or* ask espeak to grant them an exception.

Again, GPL-based libraries meant for linking against are just *evil*
Comment 18 viji 2010-12-05 01:11:01 EST
The .la files are being removed already.

I have informed the upstream regarding espeak, waiting for a clean solution.
 
How do we go ahead now? Can we continue with review?
Comment 20 Michel Alexandre Salim 2010-12-05 16:34:49 EST
Package seems almost ready, with one major issue and several minor ones remaining.

Major: source tarball does not match result of SVN export.
  The reordering is not a big deal, but the checkout I get uses lowercase
  is_nan whereas yours use uppercase is_nan. Since this is referred to in the
  patch files, it probably should be fixed

  (The file ordering problem can probably only be fixed by using git-svn instead
   of svn, and then using git archive instead of svn export, but as long as the
   recursive diff matches I'd consider the source tarball to match)

Minor: incomplete directory ownership. See below.


* TODO Review [54%]
** DONE Names [3/3]
*** DONE Package name
*** DONE Spec name
*** DONE Package version [2/2]
**** DONE Version number
**** DONE Release tag
** DONE Meets [[http://fedoraproject.org/wiki/Packaging/Guidelines][guidelines]]
** FAIL Source files match upstream
   Contents do not match:
$ diff -ru navit-3732 ~/rpmbuild/SOURCES/navit-3732
diff -ru navit-3732/navit/vehicle/gpsd_dbus/vehicle_gpsd_dbus.c /home/michel/rpmbuild/SOURCES/navit-3732/navit/vehicle/gpsd_dbus/vehicle_gpsd_dbus.c
--- navit-3732/navit/vehicle/gpsd_dbus/vehicle_gpsd_dbus.c	2010-11-22 15:51:07.994297000 +0100
+++ /home/michel/rpmbuild/SOURCES/navit-3732/navit/vehicle/gpsd_dbus/vehicle_gpsd_dbus.c	2010-11-28 09:11:53.000000000 +0100
@@ -86,9 +86,9 @@
 			priv->geo.lat=latitude;
 			priv->geo.lng=longitude;
 		}
-		if (!is_nan(track))
+		if (!is_NaN(track))
 			priv->track=track;
-		if (!is_nan(speed))
+		if (!is_NaN(speed))
 			priv->speed=speed;
 		if (!isnan(altitude))
 			priv->altitude=altitude;

** TODO [[http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries][No bundled libraries]]

** DONE License [4/4]
*** DONE License is Fedora-approved
*** DONE No licensing conflict
    Remember not to enable espeak support
*** DONE License field accurate
*** DONE License included iff packaged by upstream
** TODO rpmlint [/]
*** TODO on src.rpm
$ rpmlint ~/Downloads/navit-0.5.0-0.1.20101128svn3732.fc14.src.rpm 
navit.src: W: spelling-error %description -l en_US gpsd -> Gypsy, gypsy, gramps
navit.src:6: W: macro-in-comment %{name}
=> can be ignored

navit.src:17: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 17)
=> should be fixed

navit.src: W: invalid-url Source0: http://downloads.sourceforge.net/navit/navit-3732.tar.bz2 HTTP Error 404: Not Found
=> probably comment out this Source line when using a snapshot tarball

1 packages and 0 specfiles checked; 0 errors, 4 warnings.

*** TODO on x86_64.rpm
$ rpmlint ~/Downloads/navit-*.x86_64*.rpm
navit.x86_64: W: spelling-error %description -l en_US gpsd -> Gypsy, gypsy, gramps
navit-graphics-qt.x86_64: W: spelling-error %description -l en_US xml -> XML, cml, ml
navit-graphics-qt.x86_64: W: no-documentation
navit-graphics-sdl.x86_64: W: spelling-error %description -l en_US xml -> XML, cml, ml
navit-graphics-sdl.x86_64: W: no-documentation
4 packages and 0 specfiles checked; 0 errors, 5 warnings.

=> All harmless

** DONE Language & locale [3/3]
*** DONE Spec in US English
*** DONE Spec legible
*** DONE Use %find_lang to handle locale files
** TODO Build [2/3]
*** DONE Koji results
*** DONE BRs complete
*** FAIL Directory ownership
    - Nobody owns %dir %{_sysconfdir}/navit
    - Should Require: hicolor-icon-theme for /usr/share/icons
** TODO Spec inspection [8/9]
*** N/A ldconfig for libraries
*** DONE No duplicate files
*** DONE File permissions
*** DONE Filenames must be UTF-8
*** WAIT no BuildRoot ([[https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag][except if targeting EPEL5]])
    BuildRoot can be removed, but it's harmless
*** DONE Has %clean section
    (except F-13+:
    https://fedoraproject.org/wiki/Packaging/Guidelines#.25clean)
*** DONE %buildroot cleaned on %install
*** DONE Macro usage consistent
*** DONE Other subpackages
** DONE Desktop file validation
** DONE [[http://fedoraproject.org/wiki/Packaging/ScriptletSnippets][Scriptlets]] [4/4]
*** N/A desktop-database (desktop entry has MimeType)
*** DONE icon cache (icons in %{_datadir}/icons/)
*** N/A info files
*** N/A mimeinfo (file in %{_datadir}/mime/packages)
Comment 21 Ralf Corsepius 2010-12-05 22:52:26 EST
Further remarks:

- tarballs contains an uses a bundled library: navit/fib-1.1
Googling for it indicates nobody is shipping this library.

- the svn snapshot tarballs contain bundled libraries and java binaries (android java, licensed Apache-2.0 == GPLv2 incompatible)
=> Your source tarballs are non-GPLv2 compatible

- the svn snapshot tarballs contain a copy of the Liberation-fonts.
== unnecessary bloat

The official stable source tarball doesn't contain the android code nor the liberation fonts. Therefore, I am inclined to think your snapshot tarballs are improperly cut or upstream might be heading an unhealthy development direction.

That said, I can't avoid to ask a direct question: Why are you trying to package an unstable developments branch instead of what upstream calls "stable"?


Finally: Package still contains the sample map.
Comment 22 viji 2010-12-06 14:34:21 EST
Michel, 

Thank you so much for your review. Will update the ticket with the new spec and srpm after fixing all the issues mentioned.
Comment 23 viji 2010-12-06 14:39:14 EST
Ralf, 

Thank you so much for your observations. The tarball is just an svn checkout, no repackaging. The files you have mentioned are included by the upstream. Anyway I will check it and discuss with them, also will compare with the latest SVN.

Hope you have gone through  my previous comments and change-logs, the reason behind the SVN version:

- The stable branch is not that much "stable" with all supported Fedora versions like 13/14/15, specifically qt breaks.
- The bug fixes are been made to SVN and not porting properly back to the stable release.
- Navit upstream also recommend to use the SVN version
- Since this project is very rapidly evolving users are very keen to test the latest features and enhancements. 

Conclusion: Will Switch back to stable after 1.0 release.

Maps are not on a separate package now, because:

- For quick testing, navit is very rapidly evolving.
- People expect to see "some" maps quickly after the installation
- The maps directory contains the POI icons as well, in different formats - xpm, svg, png etc, need to differentiate and repack all of them properly

Conclusion: Up to 1.0 we will keep this in the same package
Comment 24 Ralf Corsepius 2010-12-06 23:23:18 EST
(In reply to comment #23)
> Ralf, 
> 
> Thank you so much for your observations. The tarball is just an svn checkout,
> no repackaging.
Then let me rephrase it: Your tarballs violate the GPLv2.

> The files you have mentioned are included by the upstream.
Right, but it's you who is tarring up their sources.

> Anyway I will check it and discuss with them, also will compare with the
> latest SVN.
There actually are several issues at once:
- Them shipping Apache-v2-licensed works (android) bundled with GPLv2 licenced works violates the GPLv2.
- Them bundling fonts and android code is a mistake.
 
> Hope you have gone through  my previous comments and change-logs, the reason
> behind the SVN version:
> 
> - The stable branch is not that much "stable" with all supported Fedora
> versions like 13/14/15, specifically qt breaks.
> - The bug fixes are been made to SVN and not porting properly back to the
> stable release.
> - Navit upstream also recommend to use the SVN version
> - Since this project is very rapidly evolving users are very keen to test the
> latest features and enhancements. 
> 
> Conclusion: Will Switch back to stable after 1.0 release.
My conclusion: ATM, this package is too immature for inclusion into a distro.

> Maps are not on a separate package now, because:
> 
> - For quick testing, navit is very rapidly evolving.
Seems as if we are facing a miscommunication:
I want you to package them as a noarch _subpackage_ of navit.

(With my FPC head on) You currently are shipping a big, optional data file as part of an application package. 

> - People expect to see "some" maps quickly after the installation
Yes, the way upstream currently is shipping maps is not really useful. They need to do something about it.

> - The maps directory contains the POI icons as well, in different formats -
> xpm, svg, png etc, need to differentiate and repack all of them properly
Again, I am talking about the actual map file.

>Conclusion: Up to 1.0 we will keep this in the same package
Under these circumstances, you can consider this review failed.
Comment 25 viji 2010-12-06 23:43:27 EST
We need this package in the distro, as there is a huge amount of work is going on to have a "Geo" spin for Fedora 15. There are many people interested to see this package in Fedora, just check the "CC" list of this bug, OSGeo mailing list, Fedora mail thread - "Fedora 15 exciting plans". The upstream guys are very responsive and fixing the bugs quickly, so far there is no "instability" reported regarding the "svn" package.

Regarding the GPL violations, I will remove these "violating" stuffs and repack them properly. Let me check the comments from upstream guys also.

The maps can be on a separate sub-package, will do it on the next review-update

Can I go ahead with the changes and review?
Comment 26 Ralf Corsepius 2010-12-07 12:00:43 EST
(In reply to comment #25)
> We need this package in the distro, as there is a huge amount of work is going
> on to have a "Geo" spin for Fedora 15. 
Well, many kids want a pony, but ...

> There are many people interested to see
> this package in Fedora, just check the "CC" list of this bug, 
I am interested, too, however, the unpleasant truth is, this package has a lot of issues which _need_ to be resolved before it can go into Fedora.

#1 issue is upstream's apparent carelessness about legal issues
(A simple solution would be them to license their works under GPLv2+ or GPLv3),
#2 issue is this package's apparent immaturity.

> The maps can be on a separate sub-package, will do it on the next review-update
Thanks.

> Can I go ahead with the changes and review?
Certainly. 

The next think you could do is to try working around the legal issues on your part, by refining your tarball cutting procedure (exclude the android code, exclude the fonts; If necessary, patch the sources to make them buildable without these subdirs in place.)
Comment 27 Ralf Corsepius 2011-06-04 06:09:34 EDT
Closing - No activity from submitter for almost half a year.

In case you're still interested, feel encouraged to resubmit another review request or to reopen this one.

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