Spec URL: http://cedric.olivier.free.fr/rpms/josm.spec SRPM URL: http://cedric.olivier.free.fr/rpms/josm-1607-1.fc11.src.rpm Description: Hi, I just finished packaging josm, and I would appreciate a review. It's my first package, and I am seeking a sponsor. JOSM is an editor for OpenStreetMap (OSM) written in Java 1.5. Currently it supports loading stand alone GPX track data from the OSM database, loading and editing existing nodes, ways, metadata tags and relations.
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 *Set your email as : <cedric.olivier> >%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.
(In reply to comment #1) > > 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 AFAIK the above libraries are not yet in Fedora and therefore they must be packaged too. jfcUnit http://jfcunit.sourceforge.net/ gettext-commons http://code.google.com/p/gettext-commons/ josm-translation translations are here: https://translations.launchpad.net/josm/trunk/+pots/keys but I can't find a way to build the jar file metadata-extractor http://www.drewnoakes.com/code/exif/ Moreover, I think that the correct upstream site for JOSM is: http://josm.openstreetmap.de/ HTH, Andrea.
Ping.
Hello, Thanks for your review, I am updating this rpm with all your comments. following rpmlint traces : rpmlint SPECS/josm.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. rpmlint RPMS/noarch/josm-0-1788svn.fc11.noarch.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. rpmlint RPMS/noarch/josm-javadoc-0-1788svn.fc11.noarch.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. rpmlint SRPMS/josm-0-1788svn.fc11.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. http://cedric.olivier.free.fr/rpms/josm-1788/josm.spec http://cedric.olivier.free.fr/rpms/josm-1788/josm-0-1788svn.fc11.src.rpm Not yet realized : * remove .svn file and other uneeded stuff. * bash script for generating tarball from the subversion repository * package manual (I don't actually know how to do it) jfcUnit seems to not be used in josm, I have asked the josm-dev list to confirm it. I am packaging gettext-commons, I will submit soon a review for it. Bests regards. Cédric
I had a glance and I found the following issues: * Release tag is still wrong. It should be 0.X.1788svn%{?dist} and not 1788svn%{?dist}. The first 0 indicate that this is a pre-release version. The X is an incremented number (start at 1) you should bump at *each* new release. 1788svn is the svn version of the checkout you used. * This is still wrong: find -name '*.jar' -o -name '*.class' -exec rm -f '{}' \; It should be: find \( -name '*.jar' -o -name '*.class' \) -exec rm -f '{}' \; Or: find . -name '*.jar' -exec rm -f '{}' \; find . -name '*.class' -exec rm -f '{}' \; Therefore system jars are still not used, but I think you already knew this. * Please don't use %{_builddir}/%{buildsubdir}. You already are in %{_builddir}/%{buildsubdir}, so there is no need to specify it. * And you should install the icon under %{_datadir}/icons/hicolor/128x128/apps and not under %{_datadir}/pixmaps/ You must also add the following to require the related file system package: Requires: hicolor-icon-theme * There is no useful documentation in javadoc files you generate. * There is a duplicate BR: BuildRequires: java-devel You already have: BuildRequires: java-devel >= 1:1.6.0 * Another package you should make is bzip2 library: http://www.kohsuke.org/bzip2/ You should not use the local copy. * Please remove double spaces from summary and description. HTH, Andrea.
I just noticed that Debian has a new version of JOSM in unstable: http://packages.debian.org/squeeze/josm You may want to check what they are doing and if there are any useful patches.
(In reply to comment #4) > I am packaging gettext-commons, I will submit soon a review for it. I already had a more or less working package for gettext-commons, so I cleaned it up with the help of the Debian package and put it up for review: https://bugzilla.redhat.com/show_bug.cgi?id=515136
I also packaged metadata-extractor: https://bugzilla.redhat.com/show_bug.cgi?id=516343
About josm-translation, I found it at http://svn.openstreetmap.org/applications/editors/josm/i18n/ Is it necessary to do a separate package ? josm core source needed to build josm-translation. I am adding actually josm-translation in this josm package. Can I do it ?
(In reply to comment #9) > I am adding actually josm-translation in this josm package. Can I do it ? I think it is fine. There is no use of josm-translation outside josm. Debian does this too.
I also packaged gettext-ant-tasks at : https://bugzilla.redhat.com/show_bug.cgi?id=517776
I have updated the josm package with svn version 1788 and adding josm-translation. Spec URL : http://cedric.olivier.free.fr/rpms/josm-1788/release2/josm.spec SRPM URL : http://cedric.olivier.free.fr/rpms/josm-1788/release2/josm-0-1.1788svn.fc11.src.rpm
* To be consistent with other distributions (Debian ATM) you should use 0.0 instead of 0 as version. This is not mandatory though. * Release tag is still wrong. It should be 0.X.1788svn%{?dist} and not 1.1788svn%{?dist}. The first 0 indicate that this is a pre-release version. The X is an incremented number (start at 1) you should bump at *each* new release. 1788svn is the svn version of the checkout you used. * The version-release couple in the changelog is not consistent (0-1788svn) with the one declared at the beginning of the spec file. Always run rpmlint on the rpm produced by rpmbuild. Errors like this one can be easily caught in this way. * All patches should have an upstream bug link or comment https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment In general, it is a good practise to state what a patch does. * Description can be improved. The following is from the Debian package: JOSM is an editor for OpenStreetMap (OSM) written in Java. The current version supports stand alone GPX tracks, GPX track data from OSM database and existing nodes, line segments and metadata tags from the OSM database. OpenStreetMap is a project aimed squarely at creating and providing free geographic data such as street maps to anyone who wants them. The project was started because most maps you think of as free actually have legal or technical restrictions on their use, holding back people from using them in creative, productive or unexpected ways. * Adapt this Debian patch not to include the dependencies into the target jar file http://patch-tracking.debian.net/patch/series/view/josm/0.0.svn1529-1/10_build * After that add gettext-commons and metadata-extractor to Requires. * Please remove junk from the SPEC file. Like: #find . -name 'jfcunit.jar' -exec rm -f '{}' \; #find . -name 'josm-translation.jar' -exec rm -f '{}' \; #find . -name 'metadata-extractor-2.3.1-nosun.jar' -exec rm -f '{}' \; * I have some doubt about the following: install -p -m 644 dist/%{name}-custom.jar $RPM_BUILD_ROOT%{_javadir}/%{name}-%{version}.jar %{name}-%{version}.jar will be "josm-0" for a long time. I wonder if you should call it like upstream "josm-snapshot-svn_version". Maybe some more experienced java packager should help here :) * You should add an alias for josm.jar and use it to run the program without having to edit run script at each release. * Desktop file category you use is not right. "Network" is used by a "Network application such as a web browser": http://standards.freedesktop.org/menu-spec/latest/apa.html You should use "Categories=Education;Science;Geoscience;" * You should also add a comment line in the desktop file, like: Comment=Editor for OpenStreetMap.org This is not mandatory but it could help the final user. * A man page is available from Debain. Please include it: http://patch-tracking.debian.net/patch/debianonly/view/josm/0.0.svn1529-1 * In the generate-tarball script, please: - clean up temporary directories at the end of the script. - you could reuse the same tarball in more then one Fedora release therefore do not use Fedora tags in the filename. Call it something like "josm-0.0.svnXXXX.tar.gz". You can also generate the filename based on $JOSM_SVN_TAG. This means you can drop the $NAME_VERSION and $RELEASE variables. - are you sure plugins are needed? Debian do not check them out. - you could use "svn export" and not remove .svn directories later. - instead of creating a i18n directory and moving into it, you could do it all in one line, specifying i18n as the destination directory instead of the current directory ".". The same would apply to plugins if they are required. - do not bother to remove macosx directory. Moreover, it can be useful. It has an .icns file with icons at different resolutions. These can be used instead of a single 128x128 icon. The libicns-utils can be used to extract them. - IMHO it would be better to get rid of src/org/apache in the %prep section of the SPEC file. * Add the following files in %doc: Contribution gpl-2.0.txt gpl-2.0.txt is required because of the text of the license must be included in %doc: https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text
I have updated to last josm tested revision and tried to apply all your recommendations. http://cedric.olivier.free.fr/rpms/josm.spec http://cedric.olivier.free.fr/rpms/josm-snapshot-2221svn.1.fc11.src.rpm * rpmlint josm.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. * rpmlint josm-snapshot-2221svn.1.fc11.src.rpm josm.src: W: strange-permission generate-tarball.sh 0744 1 packages and 0 specfiles checked; 0 errors, 1 warnings. generate-tarball is a script used to download sources files, it's necessary to be executable. * ./RPMS/noarch/josm-javadoc-snapshot-2221svn.1.fc11.noarch.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. * /RPMS/noarch/josm-javadoc-snapshot-2221svn.1.fc11.noarch.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
I have updated to last josm tested revision and change $RPM_BUILD_ROOT by %{buildroot} in SPEC file. http://cedric.olivier.free.fr/rpms/josm.spec http://cedric.olivier.free.fr/rpms/josm-snapshot-2255svn.1.fc11.src.rpm
(In reply to comment #16) > I have updated to last josm tested revision and change $RPM_BUILD_ROOT by > %{buildroot} in SPEC file. > > http://cedric.olivier.free.fr/rpms/josm.spec > http://cedric.olivier.free.fr/rpms/josm-snapshot-2255svn.1.fc11.src.rpm You cannot use "snapshot" as a Version and Release is wrong. Please read again the following guideline: http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_packages
I was misunderstanding "%{name}-%{version}.jar will be "josm-0" for a long time. I wonder if you should call it like upstream "josm-snapshot-svn_version". It's now corrected : http://cedric.olivier.free.fr/rpms/josm.spec http://cedric.olivier.free.fr/rpms/josm-0-0.1.2255svn.fc11.src.rpm
Some notes: - man page should not be in %doc, and better glob extension as well - you need to update icon cache because you install a new icon: # ScriptletSnippet from Fedora Packaging on Icon Cache %post touch --no-create %{_datadir}/icons/hicolor &>/dev/null || : %postun if [ $1 -eq 0 ] ; then touch --no-create %{_datadir}/icons/hicolor &>/dev/null gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : fi %posttrans gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : - why aren't you keeping change history in %changelog? - you could set file permissions within install command instead of running chmod after it
Thanks. http://cedric.olivier.free.fr/rpms/josm-0-0.3.2255svn.fc11.src.rpm http://cedric.olivier.free.fr/rpms/josm.spec - Update icon cache added in spec file. - I wasn't kept change history in %changelog because version number wasn't correct. I have now updated all version of this rpm as there would be. - Is it better to run chmod after install command? why?
(In reply to comment #20) > - Is it better to run chmod after install command? why? Actually i was proposing the opposite, ie: install -Dp -m 644 %SOURCE4 %{buildroot}%{_mandir}/man1/%{name}.1 instead of: mkdir -p %{buildroot}%{_mandir}/man1 install -p %SOURCE4 %{buildroot}%{_mandir}/man1/ chmod -x %{buildroot}%{_mandir}/man1/%{name}.1 It's just a matter of style however.
Some quick notes. I hope to give a deeper look soon. * Summary: An editor for OpenStreetMap (OSM) ^^ There should be only one space. * Patches must be named %{name}-<summary>.patch. Even better they should be named %{name}-<version>-<summary>.patch and not <summary>.patch. * Almost the same rule apply for Sources. Therefore josm-generate-tarball.sh * You must not package plugins provided by http://svn.openstreetmap.org/applications/editors/josm/plugins/ with this package because they are independent. Please remove its checkout from the generate-tarball script. * Usually shell startup scripts are packaged with the .sh exension and installed in /usr/bin without it. * Please move %post, %postun and %posttrans sections after %clean. * It should be better to separate every changelog entry with a blank line. It would be much more readable. * Changelog format is slightly wrong: * Sat Nov 21 2009 Cedric OLIVIER <cedric.olivier> 0-0.3.2255svn ^^ There should be only one space. * You must keep the package updated to the latest JOSM stable (2561 at the time of writing). * Please add CONTRIBUTION among %doc * I still think that for josm it is not very useful to have a jar called %{name}-%{version}.jar because it will be "josm-0" for a long time. %{name}-snapshot-<svn_version>.jar as upstream does is better. I still have to examine your patches and some other things. Please follow this guide, otherwise you won't be sponsored: https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group Do especially informal reviews of other packages. This is a good way to convincing a sponsor that you have the required knowledge to become a packager.
I am actually off-line, without ADSL connexion. I hope to be connected soon. But it seems that will be difficult, I am in a new building and there isn't any telephone lines. I will update this review and following yours advices as soon as possible.
Some additional notes. * source file must not be called josm.tar.gz. It is too generic. It must have something that identifies the version and the release too. "When pulling from revision control, please remember to use a Name-version-release compatible with the Version and Release Guidelines. In particular, check the section on Naming Snapshots." https://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control A filename called like "josm-0.0.svn2255.tar.gz" would be a good choice. * Source file is not packaged correctly. There should be only one root directory. i18n directory should be one subdirectory. plugin directory must not be checked out (see comment #22). * package "manual" is not created. There is no file list. I don't know if this is an omission or manual is not required. * I editing/adding the following properties: version.entry.commit.revision version.entry.commit.date in patch0 and patch3 could be avoided by creating a REVISION file in the expected format (placing it among SOURCEs, with a name like josm-REVISION, and then copying it in the builddir in %prep) or using sed appropriately in %prep. In this way you should update these two patches much less often and not at each upstream release. BTW, the rest of both patches could be merged since the purpose is the same. * patch1 is not required. You already use build-classpath in %prep. Another, alternative, and perhaps more elegant way, to solve this issue is to use build-jar-repository: http://fedoraproject.org/wiki/Packaging:Java#build-jar-repository * This line: ln -s $(build-classpath ant) lib is not needed and wrong. There is no need to put ant in that dir. * Patch2 should not needed. * There is a cleaner way to do the following: install -p %SOURCE1 %{buildroot}%{_bindir}/%{name} chmod +x %{buildroot}%{_bindir}/%{name} just in one line: install -p -m 755 %SOURCE1 %{buildroot}%{_bindir}/%{name}
Ping. Any update about this?
Ping again. I will close this bug as NOTABUG if no response is received within one week.
Cédric OLIVIER said above he'd be having connectivity issues and it appears those aren't resolved. There are 15 people on the cc: list, so perhaps we need to find another maintainer.
Hello, I am sorry for this long time without update. My connectivity issues are resolved for two weeks ago. I have checked needs for update, there is a new dependence : signpost-core. It is a google code project : http://code.google.com/p/oauth-signpost/ I can't compile it because maven 2.0.11 seems needed.
I'm glad to hear you are back. It seems to me that Apache bzip2 is also required. How do you want to go on? Maybe some of the 15 people in CC can help in packaging those dependencies. Can you also update the package to fix the above issues? Anyway, you have to follow this guideline to get sponsored: https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group I suggest you to start ASAP :-)
> I can't compile it because maven 2.0.11 seems needed. Hrm, maven 2.0.9 was released two years ago, but 2.0.8 is tagged for f13 and has been maintained as recently as September. I suppose that means there's some reason Fedora isn't tracking maven2 upstream (?), which might be a problem. I'll drop the maintainer a message to get some feedback.
There is more information about maven update here : http://fedoraproject.org/wiki/MavenUpdate
Debian has a patch to disable oauth: http://patch-tracker.debian.org/package/josm/0.0.svn3094-1 Another possibility is to make a build.xml file for ant to build oauth. Unluckily I haven't the required skills to do it myself but maybe some in CC has them. BTW, the other Debian patches look interesting too.
Please do not disable OAuth, until OSM can get some SSL security going your OSM password is sent in the clear every time you upload some changes. With OAuth your password is only sent in the clear once when the OAuth tokens are set up, thus reducing the risk that your OSM password is compromised.
Created attachment 405625 [details] oauth build xml example build with ant -f build-core.xml Dont forget to build your classpath with export CLASSPATH=`build-classpath junit mockito commons-codec` Of course you need all those jars installed via yum. Place build-core.xml in ./oauth/signpost/ , resultant jar is in ./signpost-code/dist/lib/signpost-core.jar
Sorry, somehow this disappared from my above post: I built this under opensuse, as I am without a fedora box at the moment. There is no reasons it should be specific.
(In reply to comment #33) > Please do not disable OAuth, until OSM can get some SSL security going your > OSM password is sent in the clear every time you upload some changes. With > OAuth your password is only sent in the clear once when the OAuth tokens are > set up, thus reducing the risk that your OSM password is compromised. The password is still sent in clear text as you noticed, even if only once per session. I believe there is little value added until a secure channel (e.g. https) will be used. (In reply to comment #34) > Created an attachment (id=405625) [details] > oauth build xml example I tried this and it seems mockito is not yet available in Fedora. Anyway it seems that JOSM is using v1.1 while latest upstream is v1.2.1.1. Are these two API compatible? AFAIK Fedora should package the latest version.
(In reply to comment #36) > > I tried this and it seems mockito is not yet available in Fedora. > > Anyway it seems that JOSM is using v1.1 while latest upstream is v1.2.1.1. Are > these two API compatible? AFAIK Fedora should package the latest version. mockito itself depends on hamcrest (v1.1 already packaged) and objenesis (needs packager). Anyway, arent those tools only needed for code testing, ala junit? (pls correct me if i am wrong).
objenesis (www.objenesis.org) in turn depends on maven-plugin-timestamp (http://code.google.com/p/maven-timestamp-plugin): im working to package those two.
Review request for maven-timestamp-plugin: bug 581164
Sorry for this long silence, you can find an update with the last tested version : http://cedric.olivier.free.fr/rpms/josm-3208-1/josm-0-0.1.3208svn.fc12.src.rpm http://cedric.olivier.free.fr/rpms/josm-3208-1/josm.spec oauth is disabled until maven is updated on fedora or an other solution is found. org.apache.tools.bzip2 is removed from source file and it is ant package which is used. org.apache.commons.codec is removed from source file to use jakarta-commons-codec package. Actually theses jar files are added to josm final jar, is there a method to link dynamically theses jar files ? I don't know if it is possible to use %find_lang macro with this package. I don't know how, because lange file aren't names %{name}.lang, but lang.po rpmlint output : josm.src: W: spelling-error %description -l en_US metadata -> meta data, meta-data, metatarsus josm.src: W: strange-permission josm-generate-tarball.sh 0744 josm.src: W: invalid-url Source0: josm-0.3208svn.tar.gz 1 packages and 0 specfiles checked; 0 errors, 3 warnings. josm-generate-tarball is executable because it is needed to get source code invalid-url : source code only available on svn (without tag)
I'll review it. Cedric, what is your FAS account?
Ah, just found - Cedric, please, add yourself to 'Fedora Packages CVS Commit' group, and I'll sponsor you: https://admin.fedoraproject.org/accounts/group/view/packager
It's done.
Unblocking FE-NEEDSPONSOR - I just sponsored Cédric OLIVIER.
(In reply to comment #44) > Unblocking FE-NEEDSPONSOR - I just sponsored Cédric OLIVIER. OK, now that you are his sponsor, you need to complete the formal review. (All sponsors must be the reviewer of the sponsorees first package).
It was a little mistake in the install section. I was done an "install -d 755 Source". So a 755 directory was made wrongly. It's now corrected : http://cedric.olivier.free.fr/rpms/josm-3208-2/josm-0-0.2.3208svn.fc13.src.rpm http://cedric.olivier.free.fr/rpms/josm-3208-2/josm.spec
Hi, I've just tried that SPEC and it looks good. TODO, imho : - Please just remove the commented lines (macro could be interpreted anyways; ok that shouldn't be a problem in that spec.) - I guess josm-generate-tarball.sh should be 0755. - The file /usr/share/doc/josm-0/CONTRIBUTION should be fixed using sed -ie 's/\r//g' CONTRIBUTION Glad to see this package being included ! Here is the related rpmlint output : [tnorth@grouchy tmp]$ rpmlint /home/tnorth/rpmbuild/SRPMS/josm-0-0.2.3208svn.fc12.src.rpm josm.src: W: spelling-error %description -l en_US metadata -> meta data, meta-data, metatarsus josm.src: W: strange-permission josm-generate-tarball.sh 0744 josm.src:20: W: macro-in-comment %{name} josm.src:20: W: macro-in-comment %{svn_revision} josm.src:110: W: macro-in-comment %{buildroot} josm.src:110: W: macro-in-comment %{_bindir} josm.src:111: W: macro-in-comment %SOURCE1 josm.src:111: W: macro-in-comment %{buildroot} josm.src:111: W: macro-in-comment %{_bindir} josm.src:111: W: macro-in-comment %{name} josm.src:112: W: macro-in-comment %{buildroot} josm.src:112: W: macro-in-comment %{_bindir} josm.src:112: W: macro-in-comment %{name} josm.src:118: W: macro-in-comment %{buildroot} josm.src:118: W: macro-in-comment %{_mandir} josm.src:119: W: macro-in-comment %SOURCE4 josm.src:119: W: macro-in-comment %{buildroot} josm.src:119: W: macro-in-comment %{_mandir} josm.src:120: W: macro-in-comment %{buildroot} josm.src:120: W: macro-in-comment %{_mandir} josm.src:120: W: macro-in-comment %{name} josm.src: W: invalid-url Source0: josm-0.3208svn.tar.gz 1 packages and 0 specfiles checked; 0 errors, 22 warnings. [tnorth@grouchy tmp]$ rpmlint /home/tnorth/rpmbuild/RPMS/noarch/josm-0-0.2.3208svn.fc12.noarch.rpm josm.noarch: W: spelling-error %description -l en_US metadata -> meta data, meta-data, metatarsus josm.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/josm-0/CONTRIBUTION josm.noarch: W: file-not-utf8 /usr/share/doc/josm-0/CONTRIBUTION 1 packages and 0 specfiles checked; 0 errors, 3 warnings. [tnorth@grouchy tmp]$ rpmlint /home/tnorth/rpmbuild/RPMS/noarch/josm-javadoc-0-0.2.3208svn.fc12.noarch.rpm josm-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Javanese 1 packages and 0 specfiles checked; 0 errors, 1 warnings.
Thanks, It's now corrected : http://cedric.olivier.free.fr/rpms/josm-3208-3/josm-0-0.3.3208svn.fc13.src.rpm http://cedric.olivier.free.fr/rpms/josm-3208-3/josm.spec Here is the related rpmlint output : rpmlint SPECS/josm.spec SPECS/josm.spec: W: invalid-url Source0: josm-0.3208svn.tar.gz 0 packages and 1 specfiles checked; 0 errors, 1 warnings. rpmlint SRPMS/josm-0-0.3.3208svn.fc13.src.rpm josm.src: W: spelling-error %description -l en_US metadata -> meta data, meta-data, metatarsus josm.src: W: invalid-url Source0: josm-0.3208svn.tar.gz 1 packages and 0 specfiles checked; 0 errors, 2 warnings.
Koji scratchbuild for F-13: http://koji.fedoraproject.org/koji/taskinfo?taskID=2245550 One note - I'm suspecting that description and definition of 'manual' sub-package is a leftover, so it should be removed. REVIEW: Legend: + = PASSED, - = FAILED, 0 = Not Applicable + rpmlint is almost silent: Sulaco ~/Desktop: rpmlint josm-* josm.noarch: W: spelling-error %description -l en_US metadata -> meta data, meta-data, metatarsus josm.noarch: W: file-not-utf8 /usr/share/doc/josm-0/CONTRIBUTION josm-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Javanese 2 packages and 0 specfiles checked; 0 errors, 3 warnings. Sulaco ~/Desktop: + The package is named according to the Package Naming Guidelines. + The spec file name matches the base package %{name}, in the format %{name}.spec. +/- The package meets the Packaging Guidelines, except issue with sub-package 'manual' mentioned above. + The package is licensed with a Fedora approved license and meets the Licensing Guidelines. + The License field in the package spec file matches the actual license. + The file, containing the text of the license(s) for the package, is included in %doc. + The spec file is written in American English. + The spec file for the package is legible. + The sources used to build the package, match the upstream source (by checking the sources, generated from attached script) Sulaco ~/tmp: diff -ru josm-0 josm-0.from_script/ Sulaco ~/tmp: ls josm-0 josm-0.from_script Sulaco ~/tmp: + The package successfully compiles and builds into binary rpms on at least one primary architecture. + All build dependencies are listed in BuildRequires. 0 No need to handle locales. 0 No shared library files. + The package does NOT bundle copies of system libraries. + The package is not designed to be relocatable. + The package owns all directories that it creates. + The package does not list a file more than once in the spec file's %files listings. + Permissions on files are set properly. + The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). + The package consistently uses macros. + The package contains code, or permissible content. 0 No extremely large documentation files (except javadoc) + Anything, the package includes as %doc, does not affect the runtime of the application. 0 No header files. 0 No static libraries. 0 No pkgconfig(.pc) files. 0 The package doesn't contain library files with a suffix (e.g. libfoo.so.1.1). 0 No devel sub-package. + The package does NOT contain any .la libtool archives. + The package includes a %{name}.desktop file, and that file is properly installed with desktop-file-install in the %install section. + The package does not own files or directories already owned by other packages. + At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT). + All filenames in rpm packages are valid UTF-8. Please, fix/comment issue with 'manual' sub-package, and I'll continue.
Thanks! Oh well, I've introduced an small error in your SPEC with my previous sed line: sed -i 's/\r//g' CONTRIBUTION (no 'e' param, that's not perl !) Otherwise a file CONTRIBUTIONe is created and nothing happens. Sorry for that.
This: josm.noarch: W: file-not-utf8 /usr/share/doc/josm-0/CONTRIBUTION Can be fixed using this: http://fedoraproject.org/wiki/Common_Rpmlint_issues#file-not-utf8 (But the source encoding should be found, considering iso8859-1 will let a few weird chars)
Thanks, It's now changed : http://cedric.olivier.free.fr/rpms/josm-3208-4/josm-0-0.4.3208svn.fc13.src.rpm http://cedric.olivier.free.fr/rpms/josm-3208-4/josm.spec
Bogus "manual" sub-package's description still exists.
Package updated to the last tested josm revision. http://cedric.olivier.free.fr/rpms/josm-3329-1/josm-0-0.1.3329svn.fc13.src.rpm http://cedric.olivier.free.fr/rpms/josm-3329-1/josm.spec Manual sub-package's description has been removed. Here is the related rpmlint output : rpmlint SPECS/josm.spec SPECS/josm.spec: W: invalid-url Source0: josm-0.3329svn.tar.gz 0 packages and 1 specfiles checked; 0 errors, 1 warnings. rpmlint SRPMS/josm-0-0.1.3329svn.fc13.src.rpm josm.src: W: spelling-error %description -l en_US metadata -> meta data, meta-data, metatarsus josm.src: W: strange-permission josm-generate-tarball.sh 0755L josm.src: W: invalid-url Source0: josm-0.3329svn.tar.gz 1 packages and 0 specfiles checked; 0 errors, 3 warnings.
Ok, I don't see any other issues, so this package is APPROVED.
Two missing things before you'll request cvs (easy-to-fix): - you should increase (sub)version each time when you're updating svn revisions (look at specfile changelog): 0-0.1.3329svn < 0-0.4.3208svn should have been 0-0.5.3329svn - define -> %global (just cosmetic)
Update released : http://cedric.olivier.free.fr/rpms/josm-3329-5/josm-0-0.5.3329svn.fc13.src.rpm http://cedric.olivier.free.fr/rpms/josm-3329-5/josm.spec
New Package CVS Request ======================= Package Name: josm Short Description: An editor for OpenStreetMap (OSM) Owners: cquad Branches: F-11 F-12 F-13 InitialCC: cquad
CVS done (by process-cvs-requests.py). Note that we no longer create F-11 branches.
Trying josm from fedora updates-testing repo this morning On startup I get: "$ josm Warning: the revision file '/REVISION' is missing. Warning: no preference 'proxy.policy' found. The proxy will not be used." The side effect of this seems to be that the splash screen on startup says "Version UNKNOWN" and then the intial intro help window pane says "Strange version - You should update!" FYI, $ rpm -q josm josm-0-0.6.3329svn.fc13.noarch
It's done : https://admin.fedoraproject.org/updates/josm-0-0.7.3329svn.fc13
(In reply to comment #61) > It's done : > > https://admin.fedoraproject.org/updates/josm-0-0.7.3329svn.fc13 This works! Just for curiosity, do you know why the JAR installed (6 MB) is 150% bigger than the one generated by the authors (4 MB)? -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
Created attachment 426002 [details] Wrapper script for josm, with classpath definition
(In reply to comment #62) > Just for curiosity, do you know why the JAR installed (6 MB) is 150% bigger > than the one generated by the authors (4 MB)? Because this package is not valid, accorded to the guidelines. It embeds dependencies, whereas it is forbidden: https://fedoraproject.org/wiki/Packaging/Java#Pre-built_JAR_files_.2F_Other_bundled_software Upstream voluntarily embeds metadata-extractor.jar (see build.xml). This should not be the case. Moreover, I don't understand why you also embed commons-codec.jar and ant.jar with your patch josm-0-bzip2.patch. Please don't embed dependencies, you'd better modify your wrapper to set these dependencies in the classpath. By the way, is commons-codec really required? If ant is called in the code, I can't find any call to commons-codec. So: 1) remove the patch josm-0-bzip2.patch 2) try to remove (with a patch for example) the following line in build.xml: <zipfileset src="lib/metadata-extractor-2.3.1-nosun.jar" /> 3) replace your wrapper script with the one attached in this review (josm.wrapper) 4) add ant and metadata-extractor as Requires.
(In reply to comment #64) > (In reply to comment #62) > > Just for curiosity, do you know why the JAR installed (6 MB) is 150% bigger > > than the one generated by the authors (4 MB)? > Because this package is not valid, accorded to the guidelines. It embeds > dependencies, whereas it is forbidden: > > https://fedoraproject.org/wiki/Packaging/Java#Pre-built_JAR_files_.2F_Other_bundled_software That shouldn't be the case according to the review in comment #49, it claims: "+ The package does NOT bundle copies of system libraries." It appears that the reviewer may have missed that jars were still being bundled, even though it seems that from comment #2 some bundled jars were identified and (apparently) removed around comment #4, but some may have been added since. > 4) add ant and metadata-extractor as Requires. Actually the installation pulls in the metadata-extractor as a separate package so the "Requires" appear OK, it's just that the .class files for that package are duplicated inside josm.jar, so apparently the packager didn't remove them properly from the created josm.jar This needs to be fixed before this bug is closed as it appears the package passed review prematurely.
Just to show the duplication: $ jar tvf /usr/share/java/josm.jar |grep metadata|head -10 0 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/ 1128 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/DefaultTagDescriptor.class 12498 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/Directory.class 1947 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/Metadata.class 695 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/MetadataException.class 247 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/MetadataReader.class 2936 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/SampleUsage.class 1801 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/Tag.class 574 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/TagDescriptor.class 0 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/exif/ $ jar tvf /usr/share/java/metadata-extractor.jar|grep metadata|head -10 0 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/ 1128 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/DefaultTagDescriptor.class 12498 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/Directory.class 1947 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/Metadata.class 695 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/MetadataException.class 247 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/MetadataReader.class 2936 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/SampleUsage.class 1801 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/Tag.class 574 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/TagDescriptor.class 0 Tue Jan 01 00:00:00 EST 1980 com/drew/metadata/exif/
Likewise issues with commons-codec (provided within the jakarta-commons-codec package): $ jar tvf josm.jar |grep apache|head -10 0 Tue Jan 01 00:00:00 EST 1980 org/apache/ 0 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/ 0 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/ 268 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/BinaryDecoder.class 268 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/BinaryEncoder.class 588 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/CharEncoding.class 248 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/Decoder.class 822 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/DecoderException.class 248 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/Encoder.class 822 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/EncoderException.class $ jar tvf /usr/share/java/commons-codec.jar|grep apache|head -10 0 Tue Jan 01 00:00:00 EST 1980 org/apache/ 0 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/ 0 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/ 268 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/BinaryDecoder.class 268 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/BinaryEncoder.class 588 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/CharEncoding.class 248 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/Decoder.class 822 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/DecoderException.class 248 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/Encoder.class 822 Tue Jan 01 00:00:00 EST 1980 org/apache/commons/codec/EncoderException.class
Thanks a lot for this report. jakarta-commons-codec is included in the official josm-tested.jar, but it seems to be never used : $ find . -name '*' -exec grep org.apache.commons.codec '{}' \; | wc -l 0 I removed josm-0-bzip2.patch and metadata-extractor-2.3.1-nosun.jar in build.xml Josm wrapper script has been improved as you suggested $ rpmlint SRPMS/josm-0-0.8.3329svn.fc13.src.rpm josm.src: W: spelling-error %description -l en_US metadata -> meta data, meta-data, metatarsus josm.src: W: strange-permission josm-generate-tarball.sh 0755L josm.src: W: invalid-url Source0: josm-0.3329svn.tar.gz 1 packages and 0 specfiles checked; 0 errors, 3 warnings. http://cedric.olivier.free.fr/rpms/josm-3329-8/josm.spec http://cedric.olivier.free.fr/rpms/josm-3329-8/josm-0-0.8.3329svn.fc13.src.rpm
josm-0-0.8.3329svn.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/josm-0-0.8.3329svn.fc13
josm-0-0.8.3329svn.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/josm-0-0.8.3329svn.fc12
josm-0-0.8.3329svn.fc13 has been pushed to the Fedora 13 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 josm'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/josm-0-0.8.3329svn.fc13
josm-0-0.8.3329svn.fc12 has been pushed to the Fedora 12 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 josm'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/josm-0-0.8.3329svn.fc12
josm-0-0.8.3329svn.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.
josm-0-0.8.3329svn.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.
Package Change Request ====================== Package Name: josm New Branches: epel7 Owners: cquad mcepl
Git done (by process-git-requests).