https://bugzilla.redhat.com/show_bug.cgi?id=1885430
--- Comment #4 from giovanni.cabiddu(a)intel.com ---
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}.ta...
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