[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 giovanni.cabi...@intel.com changed: What|Removed |Added Status|ASSIGNED|CLOSED Resolution|--- |NEXTRELEASE Last Closed||2020-12-14 17:12:41 -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #29 from giovanni.cabi...@intel.com --- Thanks Carl. I imported the SRPM into dist-git. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #28 from Carl George 鸞 --- Thanks for making those changes upstream. Yes, the current SRPM is ready to be imported into dist-git. One final piece of advice, it's a bad idea to force push and move tags in the upstream repo. It breaks a lot of expectations and some tools when a tarball is different if fetched at different times. Tagging a new release (such as 20.10.1) or applying patch files with the spec file is the recommended approach. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430
--- Comment #27 from giovanni.cabi...@intel.com ---
Is it ok if I push the src rpm to dist-git?
Just to summarize:
* The upstream release (20.10) was changed to use the default numbering scheme
produced by autotools/libtool (as you suggested) - see comment #24
* The spec file was updated to explicitly use %{libname_so_version} in the
%files section
* A bugzilla ticket was opened to track the ExcludeArch and a reference to it
was added to the spec
The updated spec file can be found here if you want to have a look:
Spec URL:
https://raw.githubusercontent.com/intel/qatlib/v20_10/rpm/qatlib.spec
SRPM URL:
https://raw.githubusercontent.com/intel/qatlib/v20_10/rpm/qatlib-20.10.0-1.fc33.src.rpm
Build on copr:
https://copr.fedorainfracloud.org/coprs/gcabiddu/qatlib/build/1772225/
Build on koji:
https://koji.fedoraproject.org/koji/taskinfo?taskID=55751536
--
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #26 from giovanni.cabi...@intel.com --- In the end changed the upstream release to use the names you suggested as the change was small (just remove the logic in the makefile to rename the library). We didn't include a symlink containing the version in the name. This will be included starting from the next upstream release. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430
--- Comment #25 from Carl George 鸞 ---
> BTW, I also checked other libraries and it seems that there isn't
> consistency. For e.g. this is the approach used by libc:
> libc.so.6 -> libc-2.32.so
> libc-2.32.so
> libc.so
The majority of libraries follow the pattern I described. I didn't claim 100%
consistency. :)
> In any case, is the realname of the library a problem?
> In the current schema the realname updates at every release and it is
> independent from the fully qualified soname (which includes the soversion).
> Applications will be using the fully qualified soname and not referring
> directly to the realname.
It's a problem if it can be confused with the soversion. And since
applications will reference the soversion, it's unnecessary to include the
software version in the library filename at all.
> In order to be able to detect the version we can add an additional symlink
> that includes in the name the version of the library:
> libqat-20.10.so -> libqat.so.0.0.0
I'm ok with a symlink like this because it avoids confusing the software
version with the soversion, similar to how libc is named. Make sure to add
those symlinks to the %files section.
%{_libdir}/libqat-%{version}.so
%{_libdir}/libusdm-%{version}.so
> If this option is preferred we need to go for an additional upstream release
> (I have a patch already for that). It might take a couple of days to get it
> approved due to the internal process.
Instead of waiting for the new upstream release, you can just include the patch
in the spec file (Patch0: .patch) and apply it during %prep. You're
already using %autosetup, you'll just need to add a -p1 flag (assuming the
strip offset of your patch is 1, which is pretty standard). Make sure to
include either a comment explaining the patch or a link to an upstream
issue/pull request/commit, as described in the patch guidelines [0].
[0]
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_patch_guidelines
--
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #24 from giovanni.cabi...@intel.com --- The upstream project has been updated so that the libraries produced are: libqat.so -> libqat.so.0.0.0 libqat.so.0 -> libqat.so.0.0.0 libqat.so.0.0.0 I also open a ticket for the Exclude arch and updated the spec: https://bugzilla.redhat.com/show_bug.cgi?id=1897661 The updated spec file can be found here: Spec URL: https://raw.githubusercontent.com/intel/qatlib/v20_10/rpm/qatlib.spec SRPM URL: https://raw.githubusercontent.com/intel/qatlib/v20_10/rpm/qatlib-20.10.0-1.fc33.src.rpm If you are ok with this, I will upload the spec to dist-git. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #23 from giovanni.cabi...@intel.com --- > Why does qatlib need to rename the library file to include the software > version? The main reason why this was done in the upstream project was to have a simple way to understand which version of the library is installed in the system by looking at /lib64/. This is mainly an issue when the library is built and installed from sources but is not when the RPM is installed since you can check what rpm is installed. The libraries were renamed as: libqat.so -> libqat.so.20.10.0 libqat.so.0 -> libqat.so.20.10.0 libqat.so.20.10.0 The makefile can be changed as you proposed to have libqat.so -> libqat.so.0.0.0 libqat.so.0 -> libqat.so.0.0.0 libqat.so.0.0.0 as this is the default produced by autotools. In order to be able to detect the version we can add an additional symlink that includes in the name the version of the library: libqat-20.10.so -> libqat.so.0.0.0 If this option is preferred we need to go for an additional upstream release (I have a patch already for that). It might take a couple of days to get it approved due to the internal process. BTW, I also checked other libraries and it seems that there isn't consistency. For e.g. this is the approach used by libc: libc.so.6 -> libc-2.32.so libc-2.32.so libc.so In any case, is the realname of the library a problem? In the current schema the realname updates at every release and it is independent from the fully qualified soname (which includes the soversion). Applications will be using the fully qualified soname and not referring directly to the realname. Regarding the component, I see now that qatlib has been added to Bugzilla. I can now create a ticket for tracking the ExcludeArch. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430
--- Comment #22 from Carl George 鸞 ---
> In the spec we were renaming .so.0.0.0 into .so.%{soversion}.%{version} and
> then creating symlinks to .so.%{soversion}
I'm not an expert in C libraries, but in a situation like this I'd prefer to
follow the pattern of other Fedora libraries. Here are a few examples.
curl-7.73.0-2.fc34:
libcurl.so -> libcurl.so.4.7.0
libcurl.so.4 -> libcurl.so.4.7.0
libcurl.so.4.7.0
yelp-3.38.1-1.fc34:
libyelp.so -> libyelp.so.0.0.0
libyelp.so.0 -> libyelp.so.0.0.0
libyelp.so.0.0.0
rpm-libs-4.16.0-3.fc34:
librpm.so -> librpm.so.9.1.0
librpm.so.9 -> librpm.so.9.1.0
librpm.so.9.1.0
libxcb-1.13.1-6.fc34:
libxcb.so -> libxcb.so.1.1.0
libxcb.so.1 -> libxcb.so.1.1.0
libxcb.so.1.1.0
You can see that the main library file with three digits is used as-is (not
renamed), with symlinks for the other names. They do not include the software
version in the library file name. Why does qatlib need to rename the library
file to include the software version? I think the following file layout is
what we should go with. Can the Makefile be adjusted to produce this?
libqat.so -> libqat.so.0.0.0
libqat.so.0 -> libqat.so.0.0.0
libqat.so.0.0.0
libusdm.so -> libusdm.so.0.0.0
libusdm.so.0 -> libusdm.so.0.0.0
libusdm.so.0.0.0
It's ok to glob these files as long as it doesn't conceal the major version, so
this is what I'd put in the %files section.
%{_libdir}/libqat.so.%{libqat_soversion}*
%{_libdir}/libusdm.so.%{libusdm_soversion}*
> The repo is now created, however the qatlib component is not visible yet in
> bugzilla.
I'm not sure if this is just a delay, or if it needs some other event first
such as the first build in koji. Check back on this task after the first build
is done.
--
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #21 from giovanni.cabi...@intel.com --- > Correct, it doesn't get created until you request the package source repo > with `fedpkg request-repo`. The repo is now created, however the qatlib component is not visible yet in bugzilla. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #20 from Gwyn Ciesla --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/qatlib -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430
--- Comment #19 from giovanni.cabi...@intel.com ---
> I tried a build reverting that myself, and something else looks off.
> Previously the build was creating the library files with this suffix:
>
> .so.%{soversion}.%{version}
>
> Now using the Makefile target it creates them with this suffix:
>
> .so.%{version}
>
> Is that a bug in the new Makefile target? This is the reason the guidelines
> state that that globs should not conceal the soname.
That is expected. The upstream library is now installing
.so.%{soversion}
.so.%{version}
Before, instead, the upstream library was not using the correct version in the
real name. So we had
.so.%{soversion}
.so.0.0.0
In the spec we were renaming .so.0.0.0 into .so.%{soversion}.%{version} and
then creating symlinks to .so.%{soversion}
In the new version of the spec I removed the glob in the %files section and
re-introduced a %global soversion with the difference that now there is a
soversion for each library we install since both might evolve independently.
Is this ok?
Spec URL:
https://raw.githubusercontent.com/intel/qatlib/v20_10/rpm/qatlib.spec
SRPM URL:
https://raw.githubusercontent.com/intel/qatlib/v20_10/rpm/qatlib-20.10.0-1.fc33.src.rpm
Regarding the git repo, I put a request for qatlib:
https://pagure.io/releng/fedora-scm-requests/issue/30688
--
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #18 from Carl George 鸞 --- > - When creating a ticket for tracking ExcludeArch, there isn't a component > yet for qatlib. Is this created when the repo gets created or there is > something I should do? Correct, it doesn't get created until you request the package source repo with `fedpkg request-repo`. Quoting from the guidelines I linked previously: > New packages will not have bugzilla entries during the review process, so > they should put this description in the comment until the package is > approved, then file the bugzilla entry, and replace the long explanation with > the bug number. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430
--- Comment #17 from Carl George 鸞 ---
I tried a build reverting that myself, and something else looks off.
Previously the build was creating the library files with this suffix:
.so.%{soversion}.%{version}
Now using the Makefile target it creates them with this suffix:
.so.%{version}
Is that a bug in the new Makefile target? This is the reason the guidelines
state that that globs should not conceal the soname.
--
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430
--- Comment #16 from Carl George 鸞 ---
Awesome, thanks for doing the work to get that debundled and add the makefile
target. Importing the latest version of the spec file into dist-git is fine.
However, in the latest version of the spec file you made an extra change that
needs to be reverted before it's imported.
-%{_libdir}/libqat.so.%{soversion}.%{version}
-%{_libdir}/libusdm.so.%{soversion}.%{version}
-%{_libdir}/libqat.so.%{soversion}
-%{_libdir}/libusdm.so.%{soversion}
+%{_libdir}/libqat.so.*
+%{_libdir}/libusdm.so.*
When listing shared library files, they should not be listed with a glob that
conceals the soname version [0].
[0]
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_listing_shared_library_files
--
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #15 from giovanni.cabi...@intel.com --- Thanks Carl. Two things: - We just released 20.10 which includes the removal of bundled OpenSSL/libcrypto and changes to the install target in the Makefile that allow to improve spec file legibility. Do I import to dist-git the new version of the spec or the version that matches 20.08 and then we update to 20.10? Here are the new spec and src rpm if you want to have a look: Spec URL: https://raw.githubusercontent.com/intel/qatlib/v20_10/rpm/qatlib.spec SRPM URL: https://raw.githubusercontent.com/intel/qatlib/v20_10/rpm/qatlib-20.10.0-1.fc33.src.rpm - When creating a ticket for tracking ExcludeArch, there isn't a component yet for qatlib. Is this created when the repo gets created or there is something I should do? -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #14 from Carl George 鸞 --- I just sponsored you as well. Let me know if you have any issues on the next steps. https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430
Carl George 鸞 changed:
What|Removed |Added
Flags|fedora-review? |fedora-review+
--- Comment #13 from Carl George 鸞 ---
The automated review looks mostly good, just a two items left that need to be
fixed.
1. Update the ExcludeArch tag as described in comment 12.
2. Add a comments explaining the license breakdown. Some examples are given in
the guidelines [0].
Those are simple enough that I won't hold up the review any longer. Just fix
them prior to importing the SRPM to distgit.
PACKAGE APPROVED.
[0]
https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_multiple_licensing_scenarios
Package Review
==
Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
= MUST items =
C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: If your application is a C or C++ application you must list a
BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.
Generic:
[x]: Package is licensed with an open-source compatible license and meets
other legal requirements as defined in the legal section of Packaging
Guidelines.
[x]: License field in the package spec file matches the actual license.
Note: Checking patched sources after %prep for licenses. Licenses
found: "Unknown or generated", "BSD 3-clause "New" or "Revised"
License", "BSD 4-clause "Original" or "Old" License Apache License
1.0", "BSD 3-clause "New" or "Revised" License GNU General Public
License, Version 2", "OpenSSL License BSD 3-clause "New" or "Revised"
License". 4 files have unknown license. Detailed output of
licensecheck in
/home/carl/packaging/reviews/1885430-qatlib/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[!]: If the package is under multiple licenses, the licensing breakdown
must be documented in the spec.
[x]: %build honors applicable compiler flags or justifies otherwise.
[!]: Package contains no bundled libraries without FPC exception.
Note: Bundled is now permitted by guidelines.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[x]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[!]: Package is not known to require an ExcludeArch tag.
Note: Failing arches noted in ExcludeArch and will have bugs opened
after package is imported.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
license(s) in its own file, then that file, containing the text of the
license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
provided
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430
--- Comment #12 from Carl George 鸞 ---
> app -> openssl:libcrypto_EVP -> qat_engine -> qat_lib ->
> openssl:libcrypto_EVP -> qat_engine -> qat_lib -> REPEAT
I still don't understand this, but I'm admittedly not a crypto expert.
Regardless, if it's only possible for this to function with bundled libcrypto,
it is permissible for the package to do that. We've got the bundled library
provides in place, so we are covered.
> I checked again and the guideline is to still leave the license on the file.
> In order to address the noise/visual clutter in the file we replaced the full
> header with the SPDX License Identifier: `# SPDX-License-Identifier: MIT`
> Is this ok?
It's still redundant, but it's a big improvement and I'll take it.
> Thanks. I haven't run the build yet any alternative architectures. Is it
> something I have to do? If yes, which architectures should I try?
When you build the package in Fedora it will attempt to build on all
architectures, unless they are excluded. I took your copr srpm and submitted
it as a scratch build in Fedora's build system with this command:
koji build --scratch rawhide qatlib-20.08.0-1.fc34.src.rpm
Here is the result where you can see all the architectures it attempted to
build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=55010398
Based on that, I think the appropriate exclusion will look like this:
ExcludeArch: %{arm} aarch64 %{power64} s390x
Remember after review you'll need to open bugzillas for each of those, mark
them as blocking the tracker bugs listed in the guidelines [0], and include
links to each of them in spec file comments.
Other than that slight adjustment to the ExcludeArch tag, I think this looks
good. I'm running fedora-review on it now.
[0]
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_build_failures
--
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #11 from giovanni.cabi...@intel.com --- I uploaded a new version of the SPEC and the RPM: Spec URL: https://raw.githubusercontent.com/intel/qatlib/v20_08/rpm/qatlib.spec SRPM URL: https://raw.githubusercontent.com/intel/qatlib/v20_08/rpm/qatlib-20.08.0-1.fc33.src.rpm I attempted a build in copr (https://copr.fedorainfracloud.org/coprs/gcabiddu/qatlib/build/1745809/) for the following targets: - centos-stream-aarch64 - centos-stream-x86_64 - fedora-eln-aarch64 - fedora-eln-s390x - fedora-eln-x86_64 - fedora-rawhide-aarch64 - fedora-rawhide-s390x - fedora-rawhide-x86_64 As I foreseen the build fails on aarch64 and s390x. I excluded those archs in the new revision of the spec. The build is also failing for fedora-eln-x86_64, however the failure seems to be related to a dnf problem not related to qatlib. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #10 from giovanni.cabi...@intel.com --- I uploaded a new version of the SPEC and the RPM: Spec URL: https://raw.githubusercontent.com/intel/qatlib/v20_08/rpm/qatlib.spec SRPM URL: https://raw.githubusercontent.com/intel/qatlib/v20_08/rpm/qatlib-20.08.0-1.fc33.src.rpm >Yes please, the guidelines are clear here. "If the bundled package also >exists separately in the distribution, use the name of that package." Use >`Provides: bundled(openssl) = 1.1.1c`. The idea is that if there is a >vulnerability discovered in a package, we can query for everything that >provides it as a bundled library. That only works when we standardize on a >name so we don't have to query for every possible variation or sub-library of >that package. Done. >I'm not sure I follow. Those are provided by the system openssl library, not >by qatengine. Where does it become circular? If it's debundled, the build >order for the distribution would be openssl, qatlib, then qatengine. The dependency is a run time, if qatlib uses the OpenSSL EVP API. In that case we would have: app -> openssl:libcrypto_EVP -> qat_engine -> qat_lib -> openssl:libcrypto_EVP -> qat_engine -> qat_lib -> REPEAT We resolved it by linking against OpenSSL/libcrypto and using the lower level (public) crypto APIs for the algorithms we need. This change is included in the next release. This will work for now, however we have to re-think at the approach for the future since for OpenSSL 3.0 those APIs are marked as DEPRECATED. E.g. https://github.com/openssl/openssl/blob/master/include/openssl/aes.h#L51 >That's odd, because qatengine got permission to remove their license header >(bug 1885495 comment 10). If the spec file will be under the MIT license (the >Fedora default for spec files), then the comment header is just noise and >needs to be removed to improve legibility. To be clear, the qatlib.spec.in >file in the upstream repo can keep the header, we're only discussing the spec >file that will be imported into Fedora package sources. I checked again and the guideline is to still leave the license on the file. In order to address the noise/visual clutter in the file we replaced the full header with the SPDX License Identifier: `# SPDX-License-Identifier: MIT` Is this ok? >The latter. There are many upstreams who don't test on alternative >architectures. Fedora basically is their alternative arch testing and QA >(which works better when there is an upstream test suite to run in %check). >Unless it has been established as fact that the software doesn't compile, >build, or work on an architecture, the package should build for that >architecture. Thanks. I haven't run the build yet any alternative architectures. Is it something I have to do? If yes, which architectures should I try? -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #9 from Carl George 鸞 --- > Based on [0] I added `Provides: bundled(libcrypto) = 1.1.1c`. If `openssl` is > preferred I can change it. Yes please, the guidelines are clear here. "If the bundled package also exists separately in the distribution, use the name of that package." Use `Provides: bundled(openssl) = 1.1.1c`. The idea is that if there is a vulnerability discovered in a package, we can query for everything that provides it as a bundled library. That only works when we standardize on a name so we don't have to query for every possible variation or sub-library of that package. > The reason why qatlib includes an implementation of MD5, SHA and AES [2] is > to avoid a potential circular dependency when QAT OpenSSL Engine [3] is in > the system. I'm not sure I follow. Those are provided by the system openssl library, not by qatengine. Where does it become circular? If it's debundled, the build order for the distribution would be openssl, qatlib, then qatengine. > Regarding the license on the spec file, I checked internally. We can change > the license from BSD to MIT however we cannot remove the header on the file. That's odd, because qatengine got permission to remove their license header (bug 1885495 comment 10). If the spec file will be under the MIT license (the Fedora default for spec files), then the comment header is just noise and needs to be removed to improve legibility. To be clear, the qatlib.spec.in file in the upstream repo can keep the header, we're only discussing the spec file that will be imported into Fedora package sources. > One question regarding the Arch. Qatlib was tested and validated only on > x86_64 platforms. Should we add `ExclusiveArch: x86_64` as described in [4] > or mention explicitly the architectures where the build does not work as > described in [5]? The latter. There are many upstreams who don't test on alternative architectures. Fedora basically is their alternative arch testing and QA (which works better when there is an upstream test suite to run in %check). Unless it has been established as fact that the software doesn't compile, build, or work on an architecture, the package should build for that architecture. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #8 from giovanni.cabi...@intel.com --- I uploaded a new version of the SPEC and the RPM: Spec URL: https://raw.githubusercontent.com/intel/qatlib/v20_08/rpm/qatlib.spec SRPM URL: https://raw.githubusercontent.com/intel/qatlib/v20_08/rpm/qatlib-20.08.0-1.fc32.src.rpm > If this package is going to bundle openssl (even if only partially), there > are two MUST requirements [0]. > > 1. Add the line `Provides: bundled(openssl) = `. Based on [0] I added `Provides: bundled(libcrypto) = 1.1.1c`. If `openssl` is preferred I can change it. > 2. Open an issue upstream to request it be possible to build against the > system openssl, and add a link to that issue as a comment in the spec file. Done. Opened a ticket [1] and added a comment before the line `Provides:`. A note on this. The reason why qatlib includes an implementation of MD5, SHA and AES [2] is to avoid a potential circular dependency when QAT OpenSSL Engine [3] is in the system. We are looking at a ways to remove the bundled libcrypto in qatlib and this change might be part of the next release. > On a related note, running rpmlint on the installed package generates the > following warnings. > > qatlib.x86_64: W: unused-direct-shlib-dependency > /usr/lib64/libqat.so.0.20.08.0 /lib64/libcrypto.so.1.1 > qatlib.x86_64: W: unused-direct-shlib-dependency > /usr/lib64/libusdm.so.0.20.08.0 /lib64/libcrypto.so.1.1 > > The wiki has some notes on how to resolve this [1]. Done. Regarding the license on the spec file, I checked internally. We can change the license from BSD to MIT however we cannot remove the header on the file. One question regarding the Arch. Qatlib was tested and validated only on x86_64 platforms. Should we add `ExclusiveArch: x86_64` as described in [4] or mention explicitly the architectures where the build does not work as described in [5]? [0] https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling [1] https://github.com/intel/qatlib/issues/2 [2] https://github.com/intel/qatlib/tree/main/quickassist/utilities/osal/src/linux/user_space/openssl [3] https://github.com/intel/QAT_Engine [4] https://fedoraproject.org/wiki/Architectures#ExcludeArch_.26_ExclusiveArch [5] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_build_failures -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #7 from Carl George 鸞 --- > as the code contains some snippets of OpenSSL libcrypto. If this package is going to bundle openssl (even if only partially), there are two MUST requirements [0]. 1. Add the line `Provides: bundled(openssl) = `. 2. Open an issue upstream to request it be possible to build against the system openssl, and add a link to that issue as a comment in the spec file. On a related note, running rpmlint on the installed package generates the following warnings. qatlib.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libqat.so.0.20.08.0 /lib64/libcrypto.so.1.1 qatlib.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libusdm.so.0.20.08.0 /lib64/libcrypto.so.1.1 The wiki has some notes on how to resolve this [1]. > On this, what level of detail would you like to see in the changelog for this > phase? > Should we just have a single entry for the initial version of the package? It's ok to consolidate all of the review work into a single changelog entry for the initial package, or you can keep each round of review changes as a separate entry. Your choice. [0] https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling [1] https://fedoraproject.org/wiki/Common_Rpmlint_issues#unused-direct-shlib-dependency -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430
--- Comment #6 from giovanni.cabi...@intel.com ---
Spec URL: https://raw.githubusercontent.com/intel/qatlib/v20_08/rpm/qatlib.spec
SRPM URL:
https://raw.githubusercontent.com/intel/qatlib/v20_08/rpm/qatlib-20.08.0-1.fc32.src.rpm
>The license field must reflect ALL of the licenses of the distributed
>software, including anything regarded as "internal". Libraries that are
>dynamically linked do not need to be referenced. Libraries that are bundled
>should be removed in favor of dynamically linking against the system copies.
>If this is not possible, the license field must also reference the license of
>those bundled libraries. There must also be special provides directives for
>the bundled libraries. Reference the guidelines [0] on how to handle bundled
>libraries correctly. Based on what you are describing, if you can debundled
>the OpenSSL parts, I think the correct license text should be:
>
>License: BSD and (BSD or GPLv2)
I updated the License to:
License: BSD and (BSD or GPLv2) and OpenSSL
as the code contains some snippets of OpenSSL libcrypto.
>This is still missing the scriptlet requirement on shadow-utils. Add this
>line near the other requirements in the spec file:
>
>Requires(pre):shadow-utils
Done.
>The redundant `%dir %{_includedir}/qat` line is still there, please remove it.
Done.
>While fixing the version, the release was dropped. Please add it back. It
>should look like this:
>
>* Fri Oct 16 2020 Giovanni Cabiddu - 20.08.0-1
Done.
On this, what level of detail would you like to see in the changelog for this
phase?
Should we just have a single entry for the initial version of the package?
>> I need to consult the legal team to check if I can remove the license.
>
>For reference, there is already plenty of Intel software in Fedora under
>various licenses that keep their respective spec file under the default MIT
>license.
I'm trying to understand internally what are the steps to change the license
for this file as the project was approved as BSD.
--
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430
--- Comment #5 from Carl George 鸞 ---
> I uploaded the new spec at the same location.
The review tool doesn't work with that link. It requires the spec URL to
return raw text. It also requires the current SRPM. Please upload the latest
spec file and SRPM somewhere online, and make a new comment that follows the
initial template.
Spec URL:
SRPM URL:
> 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.
The license field must reflect ALL of the licenses of the distributed software,
including anything regarded as "internal". Libraries that are dynamically
linked do not need to be referenced. Libraries that are bundled should be
removed in favor of dynamically linking against the system copies. If this is
not possible, the license field must also reference the license of those
bundled libraries. There must also be special provides directives for the
bundled libraries. Reference the guidelines [0] on how to handle bundled
libraries correctly. Based on what you are describing, if you can debundled
the OpenSSL parts, I think the correct license text should be:
License: BSD and (BSD or GPLv2)
Please review the license guidelines [1] yourself to ensure this matches the
state of the software.
> > - The %pre scriptlet should use the template for dynamic allocation [9].
> Done
This is still missing the scriptlet requirement on shadow-utils. Add this line
near the other requirements in the spec file:
Requires(pre):shadow-utils
> > - 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 redundant `%dir %{_includedir}/qat` line is still there, please remove it.
> > - The version in the changelog entry (2010u) doesn't match the Version
> > field.
> Fixed.
While fixing the version, the release was dropped. Please add it back. It
should look like this:
* Fri Oct 16 2020 Giovanni Cabiddu - 20.08.0-1
> I need to consult the legal team to check if I can remove the license.
For reference, there is already plenty of Intel software in Fedora under
various licenses that keep their respective spec file under the default MIT
license. Some examples:
- intel-clear-sans-fonts (ASL 2.0) [2]
- intel-gmmlib (MIT and BSD) [3]
- intel-mediasdk (MIT) [4]
- intel-mpi-benchmarks (CPL) [5]
- intel-undervolt (GPLv3+) [6]
- intelhex (BSD) [7]
[0] https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling
[1]
https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/
[2]
https://src.fedoraproject.org/rpms/intel-clear-sans-fonts/blob/master/f/intel-clear-sans-fonts.spec
[3]
https://src.fedoraproject.org/rpms/intel-gmmlib/blob/master/f/intel-gmmlib.spec
[4]
https://src.fedoraproject.org/rpms/intel-mediasdk/blob/master/f/intel-mediasdk.spec
[5]
https://src.fedoraproject.org/rpms/intel-mpi-benchmarks/blob/master/f/intel-mpi-benchmarks.spec
[6]
https://src.fedoraproject.org/rpms/intel-undervolt/blob/master/f/intel-undervolt.spec
[7] https://src.fedoraproject.org/rpms/intelhex/blob/master/f/intelhex.spec
--
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430
--- Comment #4 from giovanni.cabi...@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}.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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 --- Comment #3 from Carl George 鸞 --- I want to point out one thing that was brought up in bug 1885495. 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. [0] https://fedoraproject.org/wiki/Licensing:Main#License_of_Fedora_SPEC_Files -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 Carl George 鸞 changed: What|Removed |Added Blocks||1885495 Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=1885495 [Bug 1885495] Review Request: qatengine - Intel(R) QuickAssist Technology (QAT) OpenSSL Engine -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430
--- Comment #2 from Carl George 鸞 ---
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.
- Remove all instances of `(R)` from the Summary field and the %description
sections [1].
- 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?
- Source0 is not following the recommended format [4]. It should look like
this:
https://github.com/intel/%{name}/archive/%{version}/%{name}-%{version}.tar.gz
- There are multiple missing build requirements. I can tell at least these are
missing:
autoconf automake libtool systemd-devel openssl-devel zlib-devel
- RPM will automatically adds requirements for several glibc virtual provides,
so the explicit requires for glibc is redundant and must be removed [5].
- The requires for /sbin/ldconfig and invoking that during the
%post/%preun/%postun scriptlets should be removed [6].
- The devel package's requirement on the base package must be arch-specific
[7].
- Rather than conditionally running autogen.sh during %build, it would be
better to always run `autoreconf -vif` during %prep.
- There is a lot going on during %install. Is there a Makefile target we could
use instead to improve spec file legibility [8]?
- The %pre scriptlet should use the template for dynamic allocation [9].
- 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].
- 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.
- The version in the changelog entry (2010u) doesn't match the Version field.
[0]
https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_simple_versioning
[1]
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_trademarks_in_summary_or_description
[2] https://github.com/intel/qatlib/blob/20.08.0/INSTALL#L33-L46
[3]
https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_combined_dual_and_multiple_licensing_scenario
[4]
https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_git_tags
[5]
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_requires
[6]
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_shared_libraries
[7]
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_requiring_base_package
[8] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_spec_legibility
[9]
https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/#_dynamic_allocation
[10] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_manpages
--
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
[Bug 1885430] Review Request: qatlib - Intel® QuickAssist Technology Library
https://bugzilla.redhat.com/show_bug.cgi?id=1885430 Carl George 鸞 changed: What|Removed |Added CC||c...@redhat.com Assignee|nob...@fedoraproject.org|c...@redhat.com Doc Type|--- |If docs needed, set a value Flags||fedora-review? --- Comment #1 from Carl George 鸞 --- I'll handle this review. -- 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@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
