https://bugzilla.redhat.com/show_bug.cgi?id=1027770
--- Comment #5 from Alec Leamas <leamas.a...@gmail.com> ---
Hi again!
I'm no sponsor, but I noticed some issues (there are certainly more) while
skimming through your spec:
First, there are some things which are not needed unless you intend to use this
on EPEL. If not, just remove them:
- rm -rf %{buildroot}
- %clean (whole section)
- %defattr(-,root,root,-)
OTOH, if you are heading for EPEL you need a Buildroot: tag.
You are using /etc/pam.d without requiring the owner of this dir. Also,
you have a file dependency on /etc/pamd.d/system-auth. Replacing that Requires:
with Requires: pam solves both problems (since pam owns bot the dir and the
file).
Is the BuildRequires: autogen really needed?
You have placed the ocserv state files in /var/ocserv. This is not really as
intended, use /var/lib/ocserv instead. See http://www.pathname.com/fhs.
Thou shall not use %makeinstall [1]
You have a lot of licenses, not just GPLv2 such as GPLv3+, LGPL, LGPL2.1 and
X11. Use the licensecheck tool to get the complete story, and look into [2] to
write a proper license tag.
Add the disttag to Release: It's not mandatory, but should be :) [3]
If you cannot use %{?_smp_mflags}, add a comment line above explaining
the situation.
Add the -p option to all install commands in order to preserve modification
times.
Use a /etc/ocserv dir to hold ocserve.conf. There's a link about that
somewhere, I don't find. It's just a little cleaner.
Use a wildcard for the manpage file types (the compression might change) like
in %{_mandir}/man8/ocpasswd.8*
Add Requires(pre): shadow-utils for the %pre snippet, and remove Requires:
/usr/sbin/useradd.
What are you trying to achieve with the truggerun macro? Triggers are normally
used to execute code in one package when some other package is (un)installed.
Here, both trigger and target package is ocserv, which does not really make
sense. Furthermore, the net effect of that macro seems to be that you start the
service by default which requires a FESCO permission [4].
Did I say that this is a good spec? It is. Don't take these remarks as
something else :)
BTW: Don't forget to update the changelog after fixing these remarks!
[1]
http://fedoraproject.org/wiki/Packaging:Guidelines#Why_the_.25makeinstall_macro_should_not_be_used
[2] http://fedoraproject.org/wiki/Packaging:LicensingGuidelines
[3] http://fedoraproject.org/wiki/Packaging:DistTag
[4] https://fedoraproject.org/wiki/Starting_services_by_default
--
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
