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=510743 --- Comment #14 from Andrea Musuruane <musuruan@xxxxxxxxx> 2009-08-21 09:03:47 EDT --- This is not a formal review. I miss the basic knowledge of how aranym works. Nonetheless, I hope this will help in getting aranym approved. OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistent macro usage. OK - Meets Packaging Guidelines. GPLv2+ - License OK - License field in spec matches OK - License file included in package OK - Spec in American English SEE BELOW - Spec is legible. OK - Sources match upstream md5sum: 8a9fd404c8d72b1a2a23ea866d322132 aranym-0.9.8beta.tar.gz 8a9fd404c8d72b1a2a23ea866d322132 aranym-0.9.8beta.tar.gz.orig NA - Package needs ExcludeArch SEE BELOW - BuildRequires correct NA - Spec handles locales/find_lang NA - Package is relocatable and has a reason to be. SEE BELOW - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Package has correct buildroot %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) OK - Package is code or permissible content. NA - Doc subpackage needed/used. - Packages %doc files don't affect runtime. NA - Headers/static libs in -devel subpackage. NA - Spec has needed ldconfig in post and postun NA - .pc files in -devel subpackage/requires pkgconfig NA - .so files in -devel subpackage. NA - -devel package Requires: %{name} = %{version}-%{release} NA - .la files are removed. SEE BELOW - Package is a GUI app and has a .desktop file OK - Package compiles and builds on at least one arch. Fedora 11/i386 OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. SEE BELOW - No rpmlint output. OK - final provides and requires are sane SHOULD Items: SEE BELOW - Should build in mock. SEE BELOW - Should build on all supported archs - Should function as described. OK - Should have sane scriptlets. NA - Should have subpackages require base package with fully versioned depend. OK - Should have dist tag OK - Should package latest version Issues: 1. The layout of the SPEC file, although formally correct, isn't very readable. Please move the BuildRequires and Requires at the bottom of Header Information, just after the BuildRoot. Even better, please do follow the layout of the SPEC template Fedora uses: https://fedoraproject.org/wiki/Packaging:Guidelines#Writing_a_package_from_scratch It is very handy to have one BuildRequires/Requires entry per line. Thus when diff'ing cvs revisions, it is very clear what have been added/removed. 2. Rename Patch to Patch0 3. Patch0 should patch configure too and not only configure.ac thus making autoreconf no longer needed. Autoreconf shouldn't be used in the %prep or %build sections of a package's spec file: https://fedoraproject.org/wiki/PackagingDrafts/AutoConf 4. You require autoreconf but you don't have autoconf among the BR. This is wrong and it implies the package cannot be built using mock. But please solve this issue getting rid of autoreconf as stated above. 5. Submit Patch1 upstream: https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment 6. Remove #Suggests: afros Make a file named README.Fedora and place it among the docs: https://fedoraproject.org/wiki/Packaging:Guidelines#summary State there that aranym needs afros to be useful and where to find it. Debian does something like this, if you need inspiration for the text: http://patch-tracking.debian.net/patch/debianonly/view/aranym/0.9.8beta-1 7. Ignoring verifying the file mode because aratapif needs to be setuid root manually by the user is wrong: %verify(not mode) %attr(755,root,root) %{_bindir}/aratapif Install it with setuid root _and_ check that SELinux won't complain about it. If it complains at runtime, try to solve the issue. Do not delegate this to the final user. 8. The %{_docdir}/aranym directory is not the one where Fedora users expect to find the documentation in. It should be %{_docdir}/%{name}-%{version}. You include INSTALL and you must not. I have some doubts about changelog too: it is too detailed to be useful to the final user, it points to the source code a lot and there is already a more readable NEWS file. https://fedoraproject.org/wiki/Packaging:Guidelines#Documentation You could remove %{_docdir}/aranym in %build and specify the doc files in %files. 9. Try to build aranym as the following sniplet does. It is much more readable and you shouldn't get rpmlint warnings. # JIT only works on i586 %ifarch %ix86 %configure --enable-jit-compiler make %{?_smp_mflags} mv aranym aranym-jit make clean %endif %configure --enable-addressing=direct --enable-fullmmu --enable-lilo make %{?_smp_mflags} mv aranym aranym-mmu make clean %configure --enable-addressing=direct make %{?_smp_mflags} BTW, why do you disable the (default) native debugger? If it is really needed, consider adding a comment. 10. Preserve time stamps with "touch -r" in the loop you are doing to fix char encoding. https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps 11. Please add a semicolon at the end of the "Categories" key, in the desktop file. Report this problem upstream. desktop-file-install --dir=/home/fedora/rpmbuild/BUILDROOT/aranym-0.9.8-0.2.beta.fc11.i386/usr/share/applications contrib/aranym.desktop contrib/aranym.desktop: key "Categories" is a list and does not have a semicolon as trailing character, fixing 12. These icons will not be used by desktop file because their names are not correct: /usr/share/icons/hicolor/32x32/apps/icon-32.png /usr/share/icons/hicolor/48x48/apps/icon-48.png Files must be named "aranym.png" according to the desktop file. 13. You must also install desktop files for aranym-mmu and aranym-jit https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files 14. Do not use %define: %define pre beta Use %global: https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define 15. As I said I don't really know how aranym works and I saw that there are some executables and documentation in usr/share/aranym/. Why? -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review