https://bugzilla.redhat.com/show_bug.cgi?id=2254109 --- Comment #4 from Carl George 🤠 <carl@xxxxxxxxxx> --- Cody asked me to take over this review from Jonathan. The license handling needs a few more tweaks, and fedora-review noticed a few other things I didn't on the first pass. ================================================================================ I saw that upstream added a license. We need to include that file in %files. %files +%license LICENSE %{_bindir}/%{name} %doc README.md ================================================================================ We also need to use the SPDX identifier in the License tag. -License: GPLv3 +License: GPL-3.0-only URL: https://github.com/Juve45/%{name} Source0: %{url}/archive/%{commit}/%{name}-%{shortcommit}.tar.gz ================================================================================ There is an rpmlint warning about PIE. batstat.x86_64: W: position-independent-executable-suggested /usr/bin/batstat I had to dig into this one a bit, and it seems what's happening is the upstream makefile is overriding the environment LDFLAGS. There's probably a way to patch the upstream makefile to respect the environment LDFLAGS, but I don't know it offhand. This fails to compile without the flags that are in the makefile, so we need to keep those. Sorry I didn't notice this in my initial suggestion to override the makefile's CCFLAGS. I think the best approach is to invoke it with the system flags combined with the flags in the makefile. Doing it this way gets rid of the rpmlint warning and gives us those debug symbols we need. %build -%make_build CCFLAGS="%{optflags}" +%make_build CCFLAGS="%{optflags} -std=c++11" LDFLAGS="%{build_ldflags} -lncurses -pthread" https://docs.fedoraproject.org/en-US/packaging-guidelines/#_pie ================================================================================ rpmlint is complaining about the 775 permissions set by the Makefile. batstat.x86_64: E: non-standard-executable-perm /usr/bin/batstat 775 This one can be fixed with a simple sed in %prep. %prep %autosetup -n batstat-%{commit} +sed -e 's/-m 775/-m 755/' -i Makefile ================================================================================ I think with those fixes this will be ready to approve. -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2254109 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202254109%23c4 -- _______________________________________________ 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, report it: https://pagure.io/fedora-infrastructure/new_issue
