https://bugzilla.redhat.com/show_bug.cgi?id=1263941 Lubomir Rintel <lkundrak@xxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ --- Comment #17 from Lubomir Rintel <lkundrak@xxxxx> --- (In reply to Ingvar Hagelund from comment #14) > Hello, Lubomir. Thanks for taking the review :-) > > Updated src.rpm: > https://ingvar.fedorapeople.org/tayga/tayga-0.9.2-3.fc23.src.rpm > Updated specfile: https://ingvar.fedorapeople.org/tayga/tayga.spec > > (In reply to Lubomir Rintel from comment #12) > > 0.) License tag not correct > > > > > License: GPLv2 > > > > It seems to be "GPLv2+" (there's an or later version clause). > > Fixed. I have also asked upstream to add this to README or some other more > visible file. Atm, it's only visible in the manpage. Well, it's visible in the comments in the source code, which is what actually matters. > > 1.) Please add comments that would describe the upstreaming status of the > > patches > > > > > Patch0: tayga-0.9.2_redhat_initscripts_and_systemd.patch > > > Patch1: tayga-0.9.2_cflags_override.patch > > Done > > > Have you send them upstream? Is there a mailing list or bug tracker > > reference? > > Yes, they are sent upstream. I have not found any issue tracker nor mailing > list for tayga. I have asked upstream for this as well, but not received any > reply. Thanks. > > 2.) Why do you turn off PIE on ppc64? > > > > > # PIE hardening seems to fail on ppc64 > > > > If this is really the case, please add a more descriptive comment (error > > output). > > After a bit of debugging, I found this not to be a problem with PIE, but a > difference on Red Hat's ppc64 and x86_64 koji builders. The ppc64 one does > not take "-z now" in LDFLAGS, though the x86_64 one does. Changing to > "-znow" seems to work, so I have removed the workaround. Good job on this one, thanks. > > 3.) You're missing the %defattr tag > > > > It's not needed for current RPM, but seems like you're targetting old RHEL > > versions as well? > > Fixed. > > 4.) You're installing a service with name of a templated service: > > > > > ln -s %{_unitdir}/%{name}@.service %{buildroot}%{_unitdir}/%{name}@example.service > > > > This makes no sense. systemd would probably just ignore that. > > No, it treats it as a service. > > > 'systemctl enable tayga@example' would make the link in the proper place; > > just drop that line. > > Okay, dropped, but it would not be trivial for the non-seasoned systemd user > to know how to enable the service. Yes, but how does this help a non-seasoned user? Ideally upstream would should distribute the service files along with the documentation on how to use them. > Should I add a README.RedHat with a note > explaining this? Yes, I think that's okay. > Also, Will the %systemd_preun %{name}@.service remove both the unit file and > any generated symlinks? Of course not. The packages shouldn't touch /etc. If the user installs file in /etc, he's responsible for dealing with it. That's the same regardless of whether the service is templated or not. > > 5.) rpmlint complains: > > > > > tayga.x86_64: E: incorrect-fsf-address /usr/share/licenses/tayga/COPYING > > > The Free Software Foundation address in this file seems to be outdated or > > > misspelled. Ask upstream to update the address, or if this is a license file, > > > possibly the entire file with a new copy available from the FSF. > > > > This is something the upstream would need to fix; you may want to let them > > know. Obviously not a review blocker. > > I did notice upstream about this as well. Thank you. The package looks good to me now. APPROVED -- 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 _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review