https://bugzilla.redhat.com/show_bug.cgi?id=2035576 --- Comment #1 from Carl George 🤠 <carl@xxxxxxxxxx> --- Good work on this so far. Here are the things I've noticed so far that should be corrected. ================================================================================ git doesn't appear to be needed to build. I removed it and a test build works fine. I understand it's needed by the generate-tarball.sh script, but that isn't run during the build. -BuildRequires: autoconf automake git +BuildRequires: autoconf automake ================================================================================ There are a few explicit requires that should be removed because they are already covered by the automatic library soname requirements (libSDL2-2.0.so.0, libSDL2_image-2.0.so.0, etc). -Requires: SDL2 SDL2_image SDL2_ttf SDL2_mixer ncurses hicolor-icon-theme +Requires: hicolor-icon-theme https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_requires ================================================================================ There is a requirement for shadow-utils for %pre scriptlets, but there are no %pre scriptlets. This line should be removed. -Requires(pre): shadow-utils ================================================================================ You can simplify the %prep section by using %autosetup. -%setup -q -%patch0 -p1 -%patch1 -p1 -%patch2 -p1 +%autosetup -p1 https://docs.fedoraproject.org/en-US/packaging-guidelines/#_autosetup ================================================================================ The spec file uses make, but there is no buildrequirement on make. It works right now because other buildrequirements pull in make as a dependency, but if that ever changes then the build will start failing. Best to specify it explicitly. +BuildRequires: make https://docs.fedoraproject.org/en-US/packaging-guidelines/#buildrequires ================================================================================ It's recommended to use the %make_build macro. -make %{?_smp_mflags} +%make_build https://docs.fedoraproject.org/en-US/packaging-guidelines/#_parallel_make ================================================================================ Currently angband-data doesn't require angband, which could result in that package being installed without the necessary copying.rst file. angband-data should include the license file also or require angband. +Requires: %{name} = %{version}-%{release} https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#subpackage-licensing ================================================================================ It appears that the only CC-BY licensed files are in /usr/share/angband/tiles/gervais/. Based on that, the top level license should be just GPLv2, and the -data subpackage should have a license of "GPLv2 and CC-BY". (top level) -License: GPLv2 and CC-BY +License: GPLv2 (data subpackage) +License: GPLv2 and CC-BY https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_multiple_licensing_scenarios ================================================================================ There are 14 documentation files in /usr/share/doc/angband. You should consider creating a -doc subpackage to own them (not mandatory). https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation ================================================================================ There are several rpmlint errors about the permissions structure. angband.x86_64: E: setgid-binary /usr/bin/angband games 2755 angband.x86_64: E: non-standard-executable-perm /usr/bin/angband 2755 angband.x86_64: E: non-standard-dir-perm /var/games/angband 775 angband.x86_64: E: non-standard-dir-perm /var/games/angband/archive 2775 angband.x86_64: E: non-standard-dir-perm /var/games/angband/save 2775 angband.x86_64: E: non-standard-dir-perm /var/games/angband/scores 2775 The upstream default is to store savefiles and scorefiles in each users' home directory. https://github.com/angband/angband/blob/4.2.3/configure.ac#L50-L54 Removing the --with-setgid flag will switch to this method, and then all the /var/games directories can be removed. ================================================================================ There is an rpmlint warning about a non-utf8 doc file. This can be fixed by running iconv on that file in %prep. angband.x86_64: W: file-not-utf8 /usr/share/doc/angband/version.rst https://fedoraproject.org/wiki/Common_Rpmlint_issues#file-not-utf8 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2035576 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
