[Bug 508351] Review Request: josm - java openstreetmap editor

Sun Jul 19 11:58:35 UTC 2009

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=508351

D Haley <mycae at yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mycae at yahoo.com

--- Comment #1 from D Haley <mycae at yahoo.com>  2009-07-19 07:58:34 EDT ---
I have made a few comments below:

> Version:        1607
*This is not the recommended way to do SVN revision numbering, and may cause a
problem if you ever switch to a release package (See
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_packages),
so maybe use something like 0.1607svn%{?dist}. Also for your next build please
update to at least 1788, which is labelled as current tested build. 

>#sources construite avec la commande suivante
*Please only use english in spec file. I assume it means "Source may be built
using the follwing commands"

>Summary:
>written in  Java 1.5. 

*Please remove Java version number. It is incorrect and not required, the java
VM they use is not the same VM we use. 

>From LICENSE:
>Earlier contributions to JOSM did not specifiy a version of that license,
>but the license included in the distribution was version 2. 
>All contributions made on or after 15 April 2008 are explicitly "GPL
>v2 or later"

*Please contact upstream and see if this has been resolved. Otherwise you must
set the licence to GPLv2.

>$ rpmlint ../SRPMS/josm-1607-1.fc11.src.rpm 
>josm.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 10)
>1 packages and 0 specfiles checked; 0 errors, 1 warnings.

*Personally, I don't care about this, but policy and rpmlint says you need to
fix this. 

>$ rpmlint josm-javadoc-1607-1.fc10.noarch.rpm 
>josm-javadoc.noarch: W: non-standard-group Development Documentation
>1 packages and 0 specfiles checked; 0 errors, 1 warnings.

*Again, please fix.

*Make sure that you run rpmlint on the .spec file, the .srpm file, all .rpm
files (debug, subpackages, etc) and on the installed rpm, prior to sumitting to
review. This output should be posted to the review page.

> find -name '*.jar' -o -name '*.class' -exec rm -f '{}' \;

*Break this into two lines, as your find command does not remove the jar files,
shown below.

$ find -name '*.jar' -o -name '*.class' -exec rm -f '{}' \;
$ find ./ -name '*.jar' -o -name '*.class'
./lib/jfcunit.jar
./lib/gettext-commons-0.9.6.jar
./lib/josm-translation.jar
./lib/metadata-extractor-2.3.1-nosun.jar

*If you are going to use a source build, it might be an idea to remove these
prior to construction of the Source0 file. You can include a bash script for
generating the tarball from the subversion repository, and use it as a SOURCEx,
isntead, as in
(https://fedoraproject.org/wiki/Packaging/SourceURL#When_Upstream_uses_Prohibited_Code).
You could also remove the .svn file and other uneeded stuff (./macosx) if you
choose to do so.

As you are providing an SVN checkout, I am unable to confirm the md5 without
the removal of the .svn entries. Please remove .svn directories in svn
checkout, using SOURCEx method noted above.
$  mkdir josm-1607 && cd josm-1607 && svn -r 1607 co
http://josm.openstreetmap.de/svn/trunk/ . && cd ../ && tar zcvf
josm-1607.tar.gz josm-1607
...
$ md5sum rpmbuild/SOURCES/josm-1607.tar.gz 
8909c0b2bbf13cebad0e47c39e1adafa  rpmbuild/SOURCES/josm-1607.tar.gz
$ md5sum josm-1607.tar.gz 
45de92c7984c6873b8c9702ea2006c4a  josm-1607.tar.gz

Looking at this source1 I note:

>#!/bin/sh
>LIBXCB_ALLOW_SLOPPY_LOCK=1 java -Xms32M -Xmx512M -XX:+UseParallelGC -XX:+UseAdaptiveSizePolicy -jar /usr/share/java/josm-tested.jar "$@"

*Which is not going to work without josm-tested.jar, use %{javadir}/%{name}.jar
instead (with substitution obviously :) ).

>Source3:	%{name}.png

*Use images/logo.png in preference to Source3 (same file).

> %doc README
>%doc LICENSE
This can go on one line (%doc README LICENSE)

>cedric.olivier at free.fr

*Set your email as : <cedric.olivier at free.fr> 

>%package manual
*Are there any files for this subpackage?

> install %SOURCE1 $RPM_BUILD_ROOT%{_bindir}/%{name}
*Please preserve timestamps during copy and install (see install(1) manpage).
Its not as critical for SVN checkouts, but it is nice :)

*Please provide a koji scratch build for both F-10 and F-11 if possible. 

> [exec] Execute failed: java.io.IOException: Cannot run program "svn": java.io.IOException: error=13, Permission denied
*Why is ant calling SVN during build? You do not have network access during the
build. Please patch out this behaviour from build.xml.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.