https://bugzilla.redhat.com/show_bug.cgi?id=1322168 --- Comment #2 from Michal Schmidt <mschmidt@xxxxxxxxxx> --- > %global _hardened_build 1 This is actually the default setting since Fedora 23: https://fedoraproject.org/wiki/Changes/Harden_All_Packages I see the Packaging Guidelines don't mention that fact, but anyway, it's certainly OK to keep it in the spec file as well. > # libibacmp.so is a plugin for the ibacm daemon, not a public library. > # Do not advertise it in RPM metadata: > %global _privatelibs libibacmp[.]so.* > %global __provides_exclude ^(%{_privatelibs})$ > %global __requires_exclude ^(%{_privatelibs})$ I see this matches the style described in "Filtering provides and requires after scanning" in https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering. I would just suggest moving these lines down, right before the %description, to satisfy the recommendation in "Location of macro invocation" on the page. > Name: ibacm > Version: 1.2.0 > Release: 1%{?dist} > Summary: InfiniBand Communication Manager Assistant > Group: System Environment/Daemons The Group: tag is unnecessary. > License: GPLv2 or BSD > Url: http://www.openfabrics.org/ > Source: http://downloads.openfabrics.org/downloads/rdmacm/%{name}-%{version}.tar.gz > Source1: ibacm.service It would be nice to have a systemd unit file in upstream. /me looks inside ibacm.service and src/acm.c ... I can see the daemonization is implemented carelessly. In such a case Type=forking does not buy us anything. We may as well use Type=simple and ExecStart=/usr/bin/ibacm -P WantedBy=network.target is unusual and likely wrong. I'll look into improving this later myself (will file a BZ to myself once we have the component created). This is not a problem for the review. > Patch0001: 0001-Coverity-and-compile-warning-fixes.patch It would be nice to get rid of the patch by getting the changes to upstream. I may as well do this when I'm sending the systemd-related changes. Not a problem for the review. > BuildRequires: libibverbs-devel >= 1.2.0 > BuildRequires: libibumad-devel >= 1.3.10.2 > BuildRequires: systemd Should also add "BuildRequires: gcc" according to https://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B#BuildRequires_and_Requires > Requires(post): systemd > Requires(preun): systemd > Requires(postun): systemd These 3 lines can be expressed by: %{?systemd_requires} as suggested in https://fedoraproject.org/wiki/Packaging:Scriptlets#Systemd > [...] > %build > # ./autogen.sh > %configure CFLAGS="$CXXFLAGS -fno-strict-aliasing" LIBS=-lpthread Setting CFLAGS from CXXFLAGS is surely suspicious. A more customary way to add to CFLAGS is: CFLAGS="%{optflags} -fno-strict-aliasing" %configure I don't know why the setting of LIBS is needed. Maybe it really isn't? -- 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