https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #4 from giovanni.cabiddu@xxxxxxxxx --- Thanks for the review Carl. I uploaded the new spec at the same location. More comments below. > I tried to run fedora-review on this, but it failed to build (see the item below about missing build requirements). Here is a partial manual review of what I've noticed so far. > > - Using 0.1 for the Release field is fine during the review, but it should be raised to 1 before being imported into dist-git. After that it should always be 1 or higher [0], unless packaging a prerelease version or git snapshot. Done. The Release field has been changed to 1%{?dist}. > - Remove all instances of `(R)` from the Summary field and the %description sections [1]. Done. > - The INSTALL file indicates a complicated licensing situation [2]. All of these licenses must be reflected in the License field, using the combined scenario guidelines [3]. I also noticed that the INSTALL file mentions a LICENSE.GPL file, but that does not exist upstream. Could you assist in getting it added? The license of the project is BSD. The library that we are packaging uses (1) some Intel code that is BSD-3-Clause, (2) some Intel code which is dual license (BSD-3-Clause OR GPL-2.0-only) and (3) portions of OpenSSL. The sample codes, which are not included in this RPM but are built by default, use Intel code which is dual lincese (BSD-3-Clause OR GPL-2.0-only). These link against both OpenSSL and Zlib. Should the License field stay as BSD or reflect the internal components? Regarding the INSTALL file in [2], it is not correct. In this project we don't have code that is only GPLv2. We will be updating this file in the next release of the library. > - Source0 is not following the recommended format [4]. It should look like this: > > https://github.com/intel/%{name}/archive/%{version}/%{name}-%{version}.tar.gz Fixed. > - There are multiple missing build requirements. I can tell at least these are missing: > > autoconf automake libtool systemd-devel openssl-devel zlib-devel Fixed. > > - RPM will automatically adds requirements for several glibc virtual provides, so the explicit requires for glibc is redundant and must be removed [5]. Removed Glibc requirement. > - The requires for /sbin/ldconfig and invoking that during the %post/%preun/%postun scriptlets should be removed [6]. Removed /sbin/ldconfig requirement. > - The devel package's requirement on the base package must be arch-specific [7]. Fixed. > - Rather than conditionally running autogen.sh during %build, it would be better to always run `autoreconf -vif` during %prep. Fixed. > - There is a lot going on during %install. Is there a Makefile target we could use instead to improve spec file legibility [8]? Will be done as part of the next release of QATlib (20.10). We need to go through our internal release process in order to change the content of the repository. > - The %pre scriptlet should use the template for dynamic allocation [9]. Done > - Man pages must be referenced with a wildcard pattern to allow RPM to use its preferred compression format (which may not be gz in the future) [10]. Done. > - The `%files devel` section can be trimmed down by using just `%{_includedir}/qat` (which is recursive), rather than the directory and globbing all the files in the directory. Done. > - The version in the changelog entry (2010u) doesn't match the Version field. Fixed. > Adding a license in a comment of the spec file is only appropriate if you wish for the spec file itself to be available under a different license than the default MIT license specified by the FPCA [0]. This does not have to match the software being packaged. I'd recommend removing it as well for simplicity, but it's not strictly required. I need to consult the legal team to check if I can remove the license. -- 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 Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
