https://bugzilla.redhat.com/show_bug.cgi?id=1386938 --- Comment #3 from Michael Schwendt <bugs.michael@xxxxxxx> --- Drive-by comments. Not a full review, because due to number of issues, this package will need some more work. Consider pointing the "fedora-review" tool at this bugzilla ticket. The tool performs test-builds and then runs *many* checks on them, which are relevant to both the package maintainer as well as potential reviewers. Some of the checks, such as the licensing related ones, are extremely helpful. > %define major 23 > %define libname libprelude%{major} > %define cppmajor 8 > %define libcpp libpreludecpp%{cppmajor} > %define libnamedevel libprelude-devel https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define > Summary: Prelude Hybrid Intrusion Detection System Library The summary could be made even shorter and more concise if not repeating the name of the library here. "Prelude" is an English word with various meanings that adds more ambiguity here than value. The %description expands on the name of this library framework. And there has yet to be a package tool that completely ignores the package %name when displaying a user's search results. Interestingly, the acronym "IDMEF" and its spelled out words don't appear anywhere in the spec file %summary or %description. > Group: System/Libraries The base group for runtime library packages has been "System Environment/Libraries" for ages. At Fedora, the Group tag is fully optional for a long time. So long that even the section in the packaging guidelines, which explained that, has been replaced: https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections > Patch0: libprelude-3.1.0-linking.patch https://fedoraproject.org/wiki/Packaging:Guidelines#Patch_Guidelines > %define major 23 > %define libname libprelude%{major} > %package -n %{libname} Ewwwh. As convenient as such versioned package names may be in some cases, doing it like this is a halfbaked solution. Whenever you would change %major, it would also change package names, and you would end up with orphaned packages in the repositories and on users' installations. Versioned packages make the most sense, if you actually wanted to release multiple parallel-installable APIs/ABIs to choose from. Then you would package older compatibility releases separately and with their own %name. > %package -n %{libname} > Provides: %{name} = %{version}-%{release} Useless, because incomplete. There are arch-specific automatic Provides these days, and this line would not be sufficient and would break any explicit arch-specific Requires: https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package > Group: Development/C This may be a group used by other distributions. It is "Development/Libraries" for Red Hat and Fedora for ages, but the Group tag is optional nowadays anyway. Not going to check all the other group tags. > Requires: %{libname} = %{version}-%{release} > Requires: %{libcpp} = %{version}-%{release} https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package > %{_libdir}/libprelude.so.%{major} > %{_libdir}/libprelude.so.%{major}.* https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries Also with regard to the other lib subpackages. > %files -n %{libnamedevel} > %doc %{_docdir}/%{libnamedevel} Anything in and below %_docdir is marked %doc by default (see "rpm -E %__docdir_path"). > %{_datadir}/%{name}/swig Nothing in the spec file seems to include %_datadir/%name yet. https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership https://fedoraproject.org/wiki/Packaging:UnownedDirectories > %doc AUTHORS ChangeLog README INSTALL https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text > %attr(0644,root,root) %config(noreplace) %{_sysconfdir}/prelude/default/*.conf Those are the default attributes for files. Consider adding a comment that explains whether and why you want to override something using %attr here. https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions -- 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