https://bugzilla.redhat.com/show_bug.cgi?id=1058812 --- Comment #2 from Jakub Hrozek <jhrozek@xxxxxxxxxx> --- The specfile mostly looks good and both build and the binary works as advertised. The package builds cleanly in Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=6470246 I have some comments and questions, though: * You shouldn't Require httpd itself, but rather httpd-mmn. "mmn" stands for "module magic number" and will ensure that you only Require the version of httpd that has compatible module API. httpd-mmn is provided by httpd, it's just versioned, so the dependency tree stays the same. You can use the following one-liner: %{!?_httpd_mmn: %{expand: %%global _httpd_mmn %%(cat %{_includedir}/httpd/.mmn || echo 0-0)}} And then: Requires: httpd-mmn = %{_httpd_mmn} * Why do you filter the auto-provides? I think the comment should also explain "why", not only "what". * I think the description could be improved, too. Currently it says: """ mod_lookup_identity can retrieve GECOS and group information about user authenticated in Apache httpd server. """ I don't think the descrption should pitch GECOS in particular, but rather only talk about generic attributes, as it does later. * Most importantly -- you don't use the standard Fedora flags during %build, doesn't that also omit flags that improve security? I think an Apache module should be compiled in total paranoia mode.. For a local build, the following worked for me: %{_httpd_apxs} -c -Wc,"%{optflags} -Wall -pedantic $(pkg-config --cflags dbus-1)" $(pkg-config --libs dbus-1) mod_lookup_identity.c (I also moved the source file name to the end to separate the flags from the source on the build command line.) * When the sssd-ifp package is in Fedora, the Requires should change from dbus-libs to sssd-ifp. It's OK to keep the module as-is for now, but it's something we should keep in mind for the future. -- 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