Bug 527549 - Review Request: osm2go - A simple openstreetmap editor
Summary: Review Request: osm2go - A simple openstreetmap editor
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Martin Gieseking
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2009-10-06 21:50 UTC by Fabian Affolter
Modified: 2011-03-14 10:24 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-03-14 10:24:46 UTC
Type: ---
Embargoed:
martin.gieseking: fedora-review?


Attachments (Terms of Use)

Description Fabian Affolter 2009-10-06 21:50:53 UTC
Spec URL: http://fab.fedorapeople.org/packages/SRPMS/osm2go.spec
SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/osm2go-0.7.19-1.fc11.src.rpm

Project URL: http://www.harbaum.org/till/maemo/index.shtml#osm2go

Description:
OSM2Go is an editor for openstreetmap.org map data. OSM2Go is designed
for simplicity and user friendlyness and not for maximim feature count.
It is meant for simple mapping on the road.

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

rpmlint output:
[fab@laptop016 SRPMS]$ rpmlint osm2go-0.7.19-1.fc11.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

[fab@laptop016 x86_64]$ rpmlint osm2go*
2 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 1 Martin Gieseking 2009-10-12 19:58:55 UTC
Here's my review of your package. I couldn't find any issues to be corrected. Just two minor remarks:

- version 0.7.21 has been released in the meantime
- you can drop BR: gettext because no locales are present


$ rpmlint /var/lib/mock/fedora-11-x86_64/result/osm2go-*
3 packages and 0 specfiles checked; 0 errors, 0 warnings.


---------------------------------
keys used in following checklist:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.

[+] MUST: The spec file name must match the base package %{name}.

[+] MUST: The package must meet the Packaging Guidelines.

[+] MUST: The package must be licensed with a Fedora approved license.
    - GPLv3+ according to source file headers

[+] MUST: The License field in the package spec file must match the actual license.

[+] MUST: The file containing the text of the license(s) for the package must be included in %doc.
    - COPYING added to %doc

[+] MUST: The spec file must be written in American English.

[+] MUST: The spec file for the package MUST be legible.

[+] MUST: The sources used to build the package must match the upstream source, as provided in the spec URL.
    $ sha1sum osm2go_0.7.19-maemo1.tar.gz*
    f9b081bf8a1c1974bb77c6c2386a21ddfba77ec8  osm2go_0.7.19-maemo1.tar.gz
    f9b081bf8a1c1974bb77c6c2386a21ddfba77ec8  osm2go_0.7.19-maemo1.tar.gz.1

[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
    - koji scratch build:
      https://koji.fedoraproject.org/koji/taskinfo?taskID=1742535

[.] MUST: If the package does not successfully compile, ...

[+] MUST: All build dependencies must be listed in BuildRequires.

[.] MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
    - no locales

[.] MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun.
    - no shared libs

[.] MUST: If the package is designed to be relocatable, ...
    - not relocatable

[+] MUST: A package must own all directories that it creates.

[+] MUST: A Fedora package must not list a file more than once in the spec file's %files listings.

[+] MUST: Permissions on files must be set properly.

[+] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot}.

[+] MUST: Each package must consistently use macros.

[+] MUST: The package must contain code, or permissable content.

[.] MUST: Large documentation files must go in a -doc subpackage.
    - no large docs

[+] MUST: If a package includes something as %doc, it must not affect the runtime of the application.

[.] MUST: Header files must be in a -devel package.
    - no header files packaged

[.] MUST: Static libraries must be in a -static package.
    - no static libs

[.] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
    - no .pc files

[.] MUST: Files that end in .so (without suffix) must go in a -devel package.
    - no .so files

[.] MUST: devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}
    - no devel package

[.] MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built.
    - no .la files

[+] MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section.
    - .desktop file present and properly installed
    - file content is valid
    - icon cache correctly updated

[+] MUST: Packages must not own files or directories already owned by other packages.

[+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}.

[+] MUST: All filenames in rpm packages must be valid UTF-8.


[+] SHOULD: The reviewer should test that the package builds in mock.
    - builds in mock

[+] SHOULD: The package should compile and build into binary rpms on all supported architectures.
    - builds in koji

[+] SHOULD: The reviewer should test that the package functions as described. 
    - seems to work properly

[+] SHOULD: If scriptlets are used, those scriptlets must be sane.

[.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
    - no subpackages

[.] SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg.
    - no .pc files

Comment 2 Fabian Affolter 2009-10-13 18:53:13 UTC
(In reply to comment #1)
> - version 0.7.21 has been released in the meantime

Thanks, updated

> - you can drop BR: gettext because no locales are present

Removed all preparation stuff for translation

Here are the updated files:

Spec URL: http://fab.fedorapeople.org/packages/SRPMS/osm2go.spec
SRPM URL:
http://fab.fedorapeople.org/packages/SRPMS/osm2go-0.7.21-1.fc11.src.rpm

Comment 3 Martin Gieseking 2009-10-14 09:36:19 UTC
$ rpmlint /var/lib/mock/fedora-11-i386/result/osm2go-*
3 packages and 0 specfiles checked; 0 errors, 0 warnings.


The koji scratch build for F-12 fails, probably because of interface changes in GTK2. In src/project.c the function gtk_widget_get_sensitive is reimplemented and its signature doesn't match those in gtkwidget.h.

http://koji.fedoraproject.org/koji/taskinfo?taskID=1745746

Building for F-11 succeeds, though:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1745751

Maybe you should ask upstream to have a look into this issue.

Comment 4 Fabian Affolter 2009-11-13 21:37:09 UTC
I reported this issue upstream.

Comment 5 Martin Gieseking 2009-11-14 18:55:57 UTC
Ok, fine. I think it's better to wait for an upstream release that also works with the upcoming F-12 and the devel branch rather than approving the current package for F-11 only. 
Please clear the whiteboard when a new release/revision is ready to be reviewed.

Comment 6 Valent Turkovic 2009-12-02 17:34:27 UTC
I would love to see this great app working in F11 and F12.

Comment 7 Valent Turkovic 2009-12-02 17:56:28 UTC
I created rpm from your src.rpm and installed it on Fedora 12. It runs nice from what I could see.

Comment 8 Fabian Affolter 2010-01-04 09:50:54 UTC
Upstream release 0.8.0 for maemo.  Unfortunately there is no source tarball available at the moment.

Comment 9 Valent Turkovic 2010-01-10 21:47:34 UTC
isn't osm2go opensource? how come they didn't release source code?!? Seams really strange...

Comment 10 Fabian Affolter 2010-02-09 09:39:42 UTC
(In reply to comment #9)
> isn't osm2go opensource? how come they didn't release source code?!? Seams
> really strange...    

Of course is osm2go open source but the main platform is maemo.

For 0.8.3 the source is available.  I will make an update as soon as possible.

Comment 11 Fabian Affolter 2010-03-28 11:56:35 UTC
Here are the updated files:

* Wed Feb 24 2010 Fabian Affolter <fabian> - 0.8.3-1
- Added patch for DSO linking 
- Updated source location
- Added todo file
- Updated to new upstream version 0.8.3

Spec URL: http://fab.fedorapeople.org/packages/SRPMS/osm2go.spec
SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/osm2go-0.8.3-1.fc13.src.rpm

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

Comment 12 Martin Gieseking 2010-03-29 08:50:25 UTC
The package looks pretty clean to me. Nonetheless two further remarks:

- you can drop --prefix=%{_prefix} from %configure

- Directory %{_datadir}/osm2go/ contains a file COPYING with the GPL 2.0 license text. You should ask upstream whether the images are actually intended to be licensed under GPLv2. If so, the License tag must be "GPLv3+ and GPLv2"

Comment 13 Fabian Affolter 2010-04-28 11:11:40 UTC
Thanks Martin your comments.

Comment 14 Valent Turkovic 2010-05-06 15:25:52 UTC
Cool, seams this packages will soon be in Fedora repos, jay!

Comment 15 Martin Gieseking 2010-05-06 15:56:51 UTC
Fabian, did you already get any information about the license from upstream? If so, please let me know.

Comment 16 Fabian Affolter 2010-06-06 14:04:10 UTC
Unfortunately not. I will get in touch again with upstream.

https://garage.maemo.org/pipermail/osm2go-users/2010-June/000265.html

Comment 17 Fabian Affolter 2010-06-15 11:15:02 UTC
According to upstream everything is GPLv3+.

https://garage.maemo.org/pipermail/osm2go-users/2010-June/000266.html

Comment 18 Martin Gieseking 2010-06-15 18:10:39 UTC
Thanks for clarifying the license issue.

Please update the package with the latest upstream tarball. Something seems to have changed in version 0.8.3:

$ md5sum osm2go_0.8.3-maemo2.tar.gz*
058c9dd98dd1cca8fdc7b7a14f817a4d  osm2go_0.8.3-maemo2.tar.gz
13f7fa4d89de2ac72d4ad876c6e59447  osm2go_0.8.3-maemo2.tar.gz.upstream

Comment 19 Fabian Affolter 2010-06-27 21:39:36 UTC
Can you please tell provide a link to the *maemo2.tar.gz source?  It seams that upstream changed the location of the source tarball.

Comment 20 Martin Gieseking 2010-07-03 16:38:35 UTC
The site was temporarily down but seems to be available again:
http://masymasymas.freemoe.org/repository.maemo.org/extras-devel/pool/fremantle/free/source/o/osm2go/

Comment 21 Fabian Affolter 2010-08-15 09:53:07 UTC
The new package with the new source.

Here are the new files:
Spec URL: http://fab.fedorapeople.org/packages/SRPMS/osm2go.spec
SRPM URL:
http://fab.fedorapeople.org/packages/SRPMS/osm2go-0.8.3-3.2.fc13.src.rpm

Comment 22 Martin Gieseking 2010-08-15 15:34:55 UTC
Sorry, Fabian, I've still some things to nag about. :) But as far as I see, that's all, and the package should be ready afterwards.

Remove file COPYING from %{_datadir}/%{name}/


The following typos reported by rpmlint should be fixed:
  friendlyness -> friendliness
  maximim -> maximum


I suggest to replace 
  %{_mandir}/man*/%{name}*.*
with the more specific variant
  %{_mandir}/man1/%{name}.1*
since there's only one manpage to add.


The source tarball is available on the maemo website again:
http://repository.maemo.org/extras/pool/fremantle/free/source/o/osm2go/
I recommend to use this original location in Source0 instead of the mirror. Also remove the preceding comment so that the rpmlint warning goes away:
osm2go.src:12: W: macro-in-comment %{version}

Comment 23 Fabian Affolter 2010-08-30 21:46:20 UTC
At the moment there is an issue with the 'Wizard' because 'Maemo Mapper' is not available.

* Mon Aug 30 2010 Fabian Affolter <fabian> - 0.8.3-4.2
- Changed source URL
- Typos fixed in description
- Changed path for the man page
- Removed the second license file

Spec URL: http://fab.fedorapeople.org/packages/SRPMS/osm2go.spec
SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/osm2go-0.8.3-4.2.fc13.src.rpm

Comment 24 Martin Gieseking 2010-08-31 14:21:21 UTC
(In reply to comment #23)
> At the moment there is an issue with the 'Wizard' because 'Maemo Mapper' is not
> available.

Hm, I don't really understand the upstream development strategy. It's also strange that they don't provide source tarballs at a prominent place (maybe we picked the wrong tarball to build this package).
Fabian, could you please contact the developer and clarify what happened to the "Maemo Mapper", and how to get a working Linux version of osm2go? 
I think we shouldn't add the package to Fedora with broken functionality.

Comment 25 Fabian Affolter 2010-09-01 10:19:37 UTC
(In reply to comment #24)
> (In reply to comment #23)
> > At the moment there is an issue with the 'Wizard' because 'Maemo Mapper' is not
> > available.
> 
> Hm, I don't really understand the upstream development strategy. It's also
> strange that they don't provide source tarballs at a prominent place (maybe we
> picked the wrong tarball to build this package).

A lot of source tarballs are hosted at http://repository.maemo.org/ and others are available at their development web space.  The Maemo community is very similar to the OLPC/Sugar group. A lot of websites, wikis, bugtrackers, and ftp servers...it's hard to overview. 

> Fabian, could you please contact the developer and clarify what happened to the
> "Maemo Mapper", and how to get a working Linux version of osm2go? 

'Maemo Mapper' is used by the wizard to get the mapping area and an independent project. osm2go is working without 'Maemo Mapper' functionality but is not that comfortable. I think that I will make a patch to remove the 'Maemo Mapper' option/requirement from the wizard.

> I think we shouldn't add the package to Fedora with broken functionality.

The functionality is not broken, it's just not working ;-). I agree that at the moment we shouldn't add the package to the Package Collection but when the 'Maemo Mapper' option is gone, we can reconsider our point of view.

Comment 26 Martin Gieseking 2010-09-01 12:18:21 UTC
Ah OK. Thanks for the clarification. I thought, the wizard did work in a former version, but maybe I'm wrong. Currently, the wizard seems to be completely useless because the third option "Specify area manually" doesn't work either. I just get a default page showing the label "Page 2". However, I only tested the program superficially yet.

Comment 27 viji 2010-11-17 18:34:15 UTC
Any update on this?

Comment 28 Valent Turkovic 2010-11-20 20:25:56 UTC
When will Fedora 14 package be ready?

Comment 29 viji 2010-11-20 20:41:00 UTC
will try to contact the requester, if he is busy will take over :)

Comment 30 Fabian Affolter 2010-11-21 09:23:10 UTC
There is a patch or patches needed to get rid of the Maemo Mapper because this tool is, like osm2go, initially developed for Maemo and depends indirectly on other parts that are available for the Maemo platform.  If you want to help, please provide the needed patches.

Comment 31 Valent Turkovic 2010-11-21 11:26:47 UTC
I managed to compile and run osm2go on Fedora 14 without issues with these steps:

yum install goocanvas-devel libgnome-devel libsoup-devel libsoup22-devel

svn checkout https://vcs.maemo.org/svn/osm2go/trunk

cd trunk

./configure

make

./src/osm2go

Comment 32 Fabian Affolter 2010-11-21 11:46:04 UTC
(In reply to comment #31)
> I managed to compile and run osm2go on Fedora 14 without issues with these
> steps:

The package is working fine. The issue is that the Wizard has no functionality due the missing Maemo Mapper and missing implementation. This is the blocker. Martin has done a review.  So if you are going to review the package and have no problem with the missing 'Wizard', the package will be in Fedora soon. If not, the package is going nowhere without the patches.

Comment 33 Valent Turkovic 2010-11-21 11:50:16 UTC
Ok, thank you for explaining.

Comment 34 Fabian Affolter 2011-03-14 10:24:46 UTC
I will close this review but leave the files in place.


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