https://bugzilla.redhat.com/show_bug.cgi?id=1371158 Petr Pisar <ppisar@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ --- Comment #6 from Petr Pisar <ppisar@xxxxxxxxxx> --- > FIX: While the library is LGPLv2, the examples files have different license > (GPLv2+). You must add `License: LGPLv2 and GPLv2+' to the devel sub-package. > %package devel [...] +License: LGPLv2 and GPLv2+ Ok. > FIX: Add a full-stop symbol at the end of the descriptions. > -data without having to deal with memory allocation +data without having to deal with memory allocation. Ok. > FIX: Build-require `make' (ebtree.spec:41). > FIX: Build-require `coreutils' (ebtree.spec:42). > FIX: Build-require `gcc' (Makefile:13). > +BuildRequires: make +BuildRequires: coreutils +BuildRequires: gcc Ok. > FIX: Do not package LICENSE file twice (both %doc and %license). Use only > %license. > -%doc VERSION LICENSE README +%doc VERSION README Ok. > FIX: Export LDFLAGS for make. The linker command must use distribution flags > from %{__global_ldflags} macro. > %build [...] +export LDFLAGS="%{__global_ldflags}" + +# Some hardening on epel5,6 +%if 0%{?rhel} == 5 || 0%{?rhel} == 6 +export CFLAGS="%{optflags} -fPIE" +export LDFLAGS="-Wl,-z,relro -z,now" +%endif TODO: Here you still ignores %{__global_ldflags} for EPEL 5 and 6. The same applies to %check section. The macro is maybe empty there but still for the sake of consistency and those who redefines it locally it should be respected. TODO: I recommend to add -fPIC -DPIC compiler options when building the shared library object files. They are required for building a shared library. The build passes only because /usr/lib/rpm/redhat/redhat-hardened-cc1 hardening configuration supplies -fPIE. TODO: I recommend to build the tests in %build section. +%check > TODO: Execute tests from %check section. Upstream has tests. Actually it > would be great if the tests used the dynamic library so they checked the > bits the RPM package will deliver to users. They uses the static library now. > FIX: Build-require util-linux because of uuidgen in tests.sh. > TODO: The library SONAME should be two-digit because this not an upstream > versioning. It also does not make sense to start with "6". You could start > with "0.1". +SOMIN = 0 +SOREL = 1 [...] +libebtree.so: $(OBJS) + $(CC) $(LDFLAGS) -shared -Wl,-soname,libebtree.so.$(SOMIN) -o libebtree.so.$(SOMIN).$(SOREL) $(OBJS) + ln -sf libebtree.so.$(SOMIN).$(SOREL) libebtree.so.$(SOMIN) + ln -sf libebtree.so.$(SOMIN) libebtree.so + TODO: This is not a 2-digit SONAME. SONAME is what you pass to -soname linker option. > FIX: You have to call ldconfig from post and postun scriptlets > <https://fedoraproject.org/wiki/Packaging:Guidelines#SharedLibraries>. > +%post +/sbin/ldconfig + +%postun +/sbin/ldconfig TODO: You are missing dependency on /sbin/ldconfig for the scripts. Please use the "%post -p /sbin/ldconfig" notation the fixes it. > I don't like the compiler.h directly in /usr/include. The file name does not > sound ebtree specific. I recommend to move the header files into a > subdirectory /usr/include/ebtree. > -install -D -p -m 0644 -t %{buildroot}%{_includedir} *.h +install -D -p -m 0644 -t %{buildroot}%{_includedir}/%{name} *.h Ok. > FIX: Run-require exact ebtree package (NEVRA) from ebtree-devel package to > keep them matching. > -Requires: %{name} +Requires: %{name}%{?_isa} = %{version}-%{release} Ok. All tests pass. Ok. $ rpmlint ebtree.spec ../SRPMS/ebtree-6.0.8-3.fc26.src.rpm ../RPMS/x86_64/ebtree-*-3.fc26.* ebtree.x86_64: W: one-line-command-in-%post /sbin/ldconfig ebtree.x86_64: W: one-line-command-in-%postun /sbin/ldconfig ebtree-devel.x86_64: W: only-non-binary-in-usr-lib 4 packages and 1 specfiles checked; 0 errors, 3 warnings. TODO: Use the %post -p /sbin/ldoconfig. Your code requires another shell execution. The tests uses the dynamic library. You are right. I probably read the unpatched Makefile. Package builds in F26 (http://koji.fedoraproject.org/koji/taskinfo?taskID=16788376). Ok. Please consider fixing the `TODO' items before building this package. Resolution: Package 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 To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx