https://bugzilla.redhat.com/show_bug.cgi?id=1885430



--- Comment #4 from [email protected] ---
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 -- [email protected]
To unsubscribe send an email to [email protected]
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/[email protected]

Reply via email to