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=699843 --- Comment #9 from Martin Gieseking <martin.gieseking@xxxxxx> 2011-10-19 10:38:47 EDT --- Hi Damian, you spec file doesn't look bad, it just needs some streamlining. > -I have set the platform to be i386, the lowest common denominator. Can this be > the default for compatible platforms, i.e. i686? Just remove the BuildArch field completely. It's set by rpmbuild depending on the selected target system. By default, koji creates both i686 and x86_64 packages, so you shouldn't hard-code the build arch here. > -I have used macros throughout, apart from CFLAGS="$RPM_OPT_FLAGS" does this > need to be changed? Using the variable instead of the corresponding macro %{buildarch} is fine. However, the definition of CFLAGS is not required here. It's already part of the %configure macro and therefore recognized by make. You can verify it with rpm --eval %configure > -BuildRequires and Requires is present, with requirements, is this correct? The BuildRequires (BR) field is almost correct. The package fails to build in mock because of the missing BR desktop-file-utils. I also recommend to use multiple BR lines with a single package name each rather than one line in order to improve legibility. You can drop all Requires as these dependencies are recognized automatically (due to the corresponding linked shared libraries). > -Is a man page required? No, it's optional but nice to have if one is available and if it contains some useful information. > "dsi.i386: E: world-writable /var/lib/games/dsi/hiscore 0666L > A file or directory in the package is installed with world writable > permissions, which is most likely a security issue." > > should I change this? Yes, I think that's a good idea. I suggest to assign this file to group "games" like the data files of gnuchess, and set the permissions to 0664. All users who want to play dsi have to add themselves to the games group first. Here are some further things I recognized so far: - According to the source file headers, the license is GPLv2+. The "+" indicates the addition "or (at your option) any later version". Thus, update the License field accordingly. - I suggest to run "make install" prior to the additional cp/install commands. Also, remove all the appended variable definitions except DESTDIR. As far as I can see, none of them is required. - Remove all the install lines as they are not necessary. You can install the icon and the hiscore file with install -D alien.png %{buildroot}%{_datadir}/pixmaps/alien.png install -D hiscore %{buildroot}%{_localstatedir}/lib/games/%{name}/hiscore - The spec addresses two different .desktop files: that one provided by the tarball and another one created with "cat". Since dsi already ships a .desktop file, you don't need to create one. * remove "cp -av DSI.desktop ..." * remove the heredoc creating dsi.desktop * rename DSI.desktop to dsi.desktop * install it with desktop-file-install - You should also remove the .png suffix from the Icon entry in the .desktop file. It's determined automatically and considered an error by desktop-file-validate. Also, remove the deprecated Encoding entry. - Drop "%define _unpackaged_files_terminate_build 0". rpmbuild should fail if some of the installed files are not packaged. - Please be a bit more specific in %files and avoid plain asterisks. It improves legibility and helps to prevent adding unwanted files. %{_bindir}/* => %{_bindir}/DSI - Add "%dir %{_localstatedir}/lib/games/%{name}/" to the %file section for proper directory ownership. - Add ChangeLog and TODO to the %doc line in %files.(In reply to comment #8) -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review