Spec URL: https://github.com/intel/QAT_Engine/blob/master/qatengine.spec Source is available at: https://github.com/intel/QAT_Engine https://github.com/intel/QAT_Engine/releases Description of problem: This package contains OpenSSL Engine providing cryptographic hardware acceleration using Intel(R) QuickAssist Technology which provides optimized implementations of cryptography on Intel(R) platforms. Dependency with qatlib - https://bugzilla.redhat.com/show_bug.cgi?id=1885430 Fedora Account System Username: dineshbx
Just some quick comments: - The URL for the SPEC file needs to point to a raw text for fedora-review to work. - Remove the license from the SPEC file header. - qatlib is not present in the Fedora Package Collection. - This package needs to provide a -devel subpackage
(In reply to Fabian Affolter from comment #1) > Just some quick comments: > > - The URL for the SPEC file needs to point to a raw text for fedora-review > to work. > - Remove the license from the SPEC file header. > - qatlib is not present in the Fedora Package Collection. > - This package needs to provide a -devel subpackage Thanks, Comments inline. - The URL for the SPEC file needs to point to a raw text for fedora-review to work. [YA] Do we need to upload the spec file in some place in Fedora system for review ? - Remove the license from the SPEC file header. [YA] Is it mandatory that license headers cannot be there. If yes we will remove it - qatlib is not present in the Fedora Package Collection. [YA] The review is still in progress for qatlib and SRPM can be downloaded from https://github.com/intel/qatlib/releases/download/20.08.0/qatlib-20.08.0-0.1.fc32.src.rpm - This package needsto provide a -devel subpackage [YA] There is no other entities that need QAT Engine headers, only library is sufficient so devel package is not included.
Uploading the files anywhere online is fine, as long as the spec file is marked as a plain text file so fedora-review works. If you build the package in copr [0], the build will include a copy of the spec file and SRPM that are easy to link to. Some people will upload those files to their fedorapeople.org space [1], but that requires already being part of at least one group other than the CLA group. 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 [2]. 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've marked this bug as depending on the qatlib review that I've already started, and assigning it to myself to do this full review later. [0] https://copr.fedorainfracloud.org [1] https://fedoraproject.org/wiki/Infrastructure/fedorapeople.org [2] https://fedoraproject.org/wiki/Licensing:Main#License_of_Fedora_SPEC_Files
(In reply to Carl George 🤠 from comment #3) > Uploading the files anywhere online is fine, as long as the spec file is > marked as a plain text file so fedora-review works. If you build the > package in copr [0], the build will include a copy of the spec file and SRPM > that are easy to link to. Some people will upload those files to their > fedorapeople.org space [1], but that requires already being part of at least > one group other than the CLA group. > > 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 [2]. 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've marked this bug as depending on the qatlib review that I've already > started, and assigning it to myself to do this full review later. > > [0] https://copr.fedorainfracloud.org > [1] https://fedoraproject.org/wiki/Infrastructure/fedorapeople.org > [2] > https://fedoraproject.org/wiki/Licensing:Main#License_of_Fedora_SPEC_Files Thanks Carl George. When you find time could you please complete this Spec file review and let us know your comments
It's already been pointed out that the review tool won't work correctly until: - it can download the raw text of the spec file - it can build the package, which requires qatlib to be available You also haven't posted a link to the SRPM. Please follow the template. Spec URL: <spec info here> SRPM URL: <srpm info here> Description: <description here> Fedora Account System Username: Also, can you clarify if the intent of the spec file license comment is to override the default MIT license specified by the FPCA? I still recommend removing that if you're willing to. It's completely acceptable for the spec file to be under the default MIT license, and the software it is building be under a different license.
(In reply to Carl George 🤠 from comment #5) > It's already been pointed out that the review tool won't work correctly > until: > > - it can download the raw text of the spec file > - it can build the package, which requires qatlib to be available > > You also haven't posted a link to the SRPM. Please follow the template. > > Spec URL: <spec info here> > SRPM URL: <srpm info here> > Description: <description here> > Fedora Account System Username: > > Also, can you clarify if the intent of the spec file license comment is to > override the default MIT license specified by the FPCA? I still recommend > removing that if you're willing to. It's completely acceptable for the spec > file to be under the default MIT license, and the software it is building be > under a different license. Unfortunately, the SRPM is not posted in public repo which we will take care from next release. Is it possible for you to generate SRPM from the specfile provided. Or if you can help us doing a manual review and give a go ahead with review comments we will address and post it in the QAT Engine Public Github Repo along with SRPM in the next release. And we intend to keep the license if it is not mandatory to remove it.
No. Follow the template, with direct links to the SRPM and a raw text spec file.
(In reply to Yogaraj Alamenda from comment #6) > > And we intend to keep the license if it is not mandatory to remove it. Please note that you are adding a burden for downstreams and other contributors if you override licenses. Please consider that when you make decisions like that.
It's also worth noting that 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) [0] - intel-gmmlib (MIT and BSD) [1] - intel-mediasdk (MIT) [2] - intel-mpi-benchmarks (CPL) [3] - intel-undervolt (GPLv3+) [4] - intelhex (BSD) [5] [0] https://src.fedoraproject.org/rpms/intel-clear-sans-fonts/blob/master/f/intel-clear-sans-fonts.spec [1] https://src.fedoraproject.org/rpms/intel-gmmlib/blob/master/f/intel-gmmlib.spec [2] https://src.fedoraproject.org/rpms/intel-mediasdk/blob/master/f/intel-mediasdk.spec [3] https://src.fedoraproject.org/rpms/intel-mpi-benchmarks/blob/master/f/intel-mpi-benchmarks.spec [4] https://src.fedoraproject.org/rpms/intel-undervolt/blob/master/f/intel-undervolt.spec [5] https://src.fedoraproject.org/rpms/intelhex/blob/master/f/intelhex.spec
(In reply to Carl George 🤠 from comment #5) > It's already been pointed out that the review tool won't work correctly > until: > > - it can download the raw text of the spec file > - it can build the package, which requires qatlib to be available > > You also haven't posted a link to the SRPM. Please follow the template. > > Spec URL: <spec info here> > SRPM URL: <srpm info here> > Description: <description here> > Fedora Account System Username: > > Also, can you clarify if the intent of the spec file license comment is to > override the default MIT license specified by the FPCA? I still recommend > removing that if you're willing to. It's completely acceptable for the spec > file to be under the default MIT license, and the software it is building be > under a different license. Spec URL: https://download.copr.fedorainfracloud.org/results/dineshbx/qatengine/srpm-builds/01715062/qatengine.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/dineshbx/qatengine/srpm-builds/01715062/qatengine-0.6.1-1.fc32.src.rpm Description: This package contains OpenSSL Engine providing cryptographic hardware acceleration using Intel(R) QuickAssist Technology which provides optimized implementations of cryptography on Intel(R) platforms. Fedora Account System Username: dineshbx Also, we will remove the license information from the spec file.
Spec URL: https://download.copr.fedorainfracloud.org/results/dineshbx/qatengine/srpm-builds/01715062/qatengine.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/dineshbx/qatengine/srpm-builds/01715062/qatengine-0.6.1-1.fc32.src.rpm Description: This package contains OpenSSL Engine providing cryptographic hardware acceleration using Intel(R) QuickAssist Technology which provides optimized implementations of cryptography on Intel(R) platforms. Fedora Account System Username: dineshbx
Thank you for attempting to build this in copr. I can point the review tool directly at a copr build to automate many of the required checks. However, there are two problems with copr build 1715062. 1. It was only built against the fedora-31-x86_64 target. Reviews should be done against a Rawhide build. Since copr allows you to build against multiple targets, I suggest also building for CentOS Stream and ELN to ensure the package builds cleanly for EL8 and EL9. Change the settings of the copr to enable all of these 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 2. It failed to build because qatlib is not available. The reason I suggested building in copr was because it would allow you to build qatlib and qatengine together prior to qatlib being included in Fedora. You can get the qatlib SRPM from bug 1885430 and rebuild it first in the copr so that qatlib-devel is available to the qatengine build. I went ahead and did a quick look over of the spec file, and here are some things I noticed that need to be fixed. - The license header is still there. - Remove all instances of `(R)` from the Summary field and the %description section [0]. - Remove the last sentence from the %description. It is redundant because the URL is already included in the package metadata. - Source0 is not following the recommended format [1]. It should look like this: https://github.com/intel/%{githubname}/archive/v%{version}/%{name}-%{version}.tar.gz - Build requiring openssl-devel and qatlib-devel is sufficient. The build requirements for openssl and qatlib are redundant. - Missing build requirements for autoconf, automake, and libtool. - The machinery in autogen.sh seems unnecessary. In general running shell scripts during an rpm build should be avoided. Can the files in the .tools directory be moved to the top level? Then you can avoid the script and just run `autoreconf -vif` directly. Also, this should happen in %build, not %prep. - There is a lot going on during %install. Is there a Makefile target we could use instead to improve spec file legibility [2]? - The %ldconfig_scriptlets macro should be removed. This happens automatically [3]. [0] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_trademarks_in_summary_or_description [1] https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_git_tags [2] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_spec_legibility [3] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_shared_libraries
Hello @carl, We have addressed most of the review comments. I have ran against different targets mentioned by you and below are my observations. qatlib is failed to build in many of the target and succeeded only in 'centos-stream-x86_64' and 'fedora-rawhide-x86_64'. Hence, qatengine able to build successfully only in this two target. qatlib build(failing in many target) : https://copr.fedorainfracloud.org/coprs/dineshbx/qatengine/build/1745585/ qatengine build: https://copr.fedorainfracloud.org/coprs/dineshbx/qatengine/build/1745663/ SRPM: https://copr-be.cloud.fedoraproject.org/results/dineshbx/qatengine/fedora-rawhide-x86_64/01745663-qatengine/qatengine-0.6.1-1.fc34.src.rpm SPEC: https://copr-be.cloud.fedoraproject.org/results/dineshbx/qatengine/fedora-rawhide-x86_64/01745663-qatengine/qatengine.spec - We will take care of removing the ./autogen.sh script and include 'autoreconf -vif' in next release into github. - In %install, we have only 5 lines. Hope it should be reasonable. Please review the SPEC file and let us know if anything needs to be corrected.
The review tool only works with an overall successful copr build. Go ahead and disable the targets that failed so we can get a clean build. I suggest waiting for the final SRPM out of bug 1885430 (qatlib review) because it's very close to being finished. You'll also need to duplicate the same ExcludeArch directive they are using, with a comment explaining that it must match qatlib's ExcludeArch. ExcludeArch: %{arm} aarch64 %{power64} s390x > Please review the SPEC file and let us know if anything needs to be corrected. Minor tweak, in the %files section you probably want to consistently use your %{soversion} macro by making this change: %{_libdir}/libqatengine.so.%{soversion}.%{version} -%{_libdir}/libqatengine.so.0 +%{_libdir}/libqatengine.so.%{soversion} %{enginesdir}/qatengine.so Otherwise you risk forgetting to change that 0 when the soversion definition eventually changes. > - We will take care of removing the ./autogen.sh script and include 'autoreconf -vif' in next release into github. I'll continue the review once this is done, in a successful copr build.
Addressed all the comments and copr build is successful. SRPM: https://copr-be.cloud.fedoraproject.org/results/dineshbx/qatengine/fedora-rawhide-x86_64/01767717-qatengine/qatengine-0.6.1-1.fc34.src.rpm SPEC: https://copr-be.cloud.fedoraproject.org/results/dineshbx/qatengine/fedora-rawhide-x86_64/01767717-qatengine/qatengine.spec Please run the automation test review for the qatengine spec file once the qatlib SRPM is available.
I ran fedora-review on this, and it found another issue. - Sources used to build the package match the upstream source, as provided in the spec URL. Note: Upstream MD5sum check error, diff is in /home/carl/packaging/other/qatengine/copr-build-1767717/review- qatengine/diff.txt See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ Source checksums ---------------- https://github.com/intel/QAT_Engine/archive/v0.6.1/qatengine-0.6.1.tar.gz : CHECKSUM(SHA256) this package : a63d4dd5f58a0856af8ff578f763cf489e85f7266ce703270171c895d6aa20f5 CHECKSUM(SHA256) upstream package : 8a738339f7743e1bd81ce58496c4b2ad6ec26c0124e70ef4f42ab1aadeaf57c7 diff -r also reports differences Did someone force push in the upstream repo? Please do another copr build with the actual upstream tarball. The tarball that is used in Fedora should be the unmodified sources available from upstream [0]. [0] https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
Hi Carl, There was no force push in the upstream repo and the Src RPM is containing a new version of code with review comments addressed. We will be upstreaming the changes to GitHub ASAP hoping there wont be further review comments to spec file with automated review tool.
That's not how this works. The Source0 archive in the SRPM needs to match what is downloaded from that URL. If you need to make changes to the source code that are not yet upstream, use patch files in the spec with comments regarding their upstream status, preferably including a link to an upstream pull request or commit. This is described in detail in the guidelines [0]. [0] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_patch_guidelines
The updated spec file can be found here: Spec URL: https://raw.githubusercontent.com/intel/QAT_Engine/v0.6.2/qatengine.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/dineshbx/qatengine/fedora-rawhide-x86_64/01780924-qatengine/qatengine-0.6.2-1.fc34.src.rpm Build on copr: https://copr.fedorainfracloud.org/coprs/dineshbx/qatengine/build/1780924/ Build on koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=55958832 * The spec file was changed incorporating the feedback from the qatlib review (bug 1885430) * The upstream project (QAT Engine) was changed to implement proper installation steps in ‘make install’ and the spec file was simplified. We released QAT Engine version v0.6.2 upstream today. * The spec file was updated to explicitly use %{soversion} in the %files section * The unversioned version(libqatengine.so) of the library is removed from the rpm and there is no devel rpm, since the library should not be linked directly by applications (only used via openSSL) * The RPM is including a symlink (unversioned) to the versioned library and that’s the way openSSL engines work.
I was able to run the review tool against that copr build. It pointed out that we have a complicated license situation, similar to qatlib. All licenses must be reflected in the License field, using the combined scenario guidelines [0]. Based on the output of the review tool's license check and the upstream description [1], I think this should cover it: License: BSD and OpenSSL and GPLv2 and (BSD or GPLv2) Additionally, all license files must be included in %files and marked as %license [2]. [0] https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_combined_dual_and_multiple_licensing_scenario [1] https://github.com/intel/QAT_Engine/blob/master/README.md#licensing [2] https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text
Thanks Carl for the review. 1. Although the below files under BSD/GPLv2 and GPLv2 are in the package, these are never used or built in the qatengine RPM library build. These files are only used for different build system in the qatengine other than rpm build. So these licenses need not to be included in License. Do we still these license to be included? If not, We will update the License to BSD and OpenSSL only. This needs update in the license file section to include LICENSE.OPENSSL as well which we are planning to make changes and provide SRPM with updated upstream package. BSD 3-clause "New" or "Revised" License GNU General Public License, Version 2 ----------------------------------------------------------------------------- QAT_Engine-0.6.2/qat/config/c3xxx/multi_process_event-driven_optimized/c3xxx_dev0.conf QAT_Engine-0.6.2/qat/config/c3xxx/multi_process_optimized/c3xxx_dev0.conf QAT_Engine-0.6.2/qat/config/c3xxx/multi_thread_event-driven_optimized/c3xxx_dev0.conf QAT_Engine-0.6.2/qat/config/c3xxx/multi_thread_optimized/c3xxx_dev0.conf QAT_Engine-0.6.2/qat/config/c6xx/multi_process_event-driven_optimized/c6xx_dev0.conf QAT_Engine-0.6.2/qat/config/c6xx/multi_process_event-driven_optimized/c6xx_dev1.conf QAT_Engine-0.6.2/qat/config/c6xx/multi_process_event-driven_optimized/c6xx_dev2.conf QAT_Engine-0.6.2/qat/config/c6xx/multi_process_optimized/c6xx_dev0.conf QAT_Engine-0.6.2/qat/config/c6xx/multi_process_optimized/c6xx_dev1.conf QAT_Engine-0.6.2/qat/config/c6xx/multi_process_optimized/c6xx_dev2.conf QAT_Engine-0.6.2/qat/config/c6xx/multi_thread_event-driven_optimized/c6xx_dev0.conf QAT_Engine-0.6.2/qat/config/c6xx/multi_thread_event-driven_optimized/c6xx_dev1.conf QAT_Engine-0.6.2/qat/config/c6xx/multi_thread_event-driven_optimized/c6xx_dev2.conf QAT_Engine-0.6.2/qat/config/c6xx/multi_thread_optimized/c6xx_dev0.conf QAT_Engine-0.6.2/qat/config/c6xx/multi_thread_optimized/c6xx_dev1.conf QAT_Engine-0.6.2/qat/config/c6xx/multi_thread_optimized/c6xx_dev2.conf QAT_Engine-0.6.2/qat/config/dh895xcc/multi_process_event-driven_optimized/dh895xcc_dev0.conf QAT_Engine-0.6.2/qat/config/dh895xcc/multi_process_optimized/dh895xcc_dev0.conf QAT_Engine-0.6.2/qat/config/dh895xcc/multi_thread_event-driven_optimized/dh895xcc_dev0.conf QAT_Engine-0.6.2/qat/config/dh895xcc/multi_thread_optimized/dh895xcc_dev0.conf GNU General Public License, Version 2 ------------------------------------- QAT_Engine-0.6.2/qat/LICENSE.GPL QAT_Engine-0.6.2/qat_contig_mem/LICENSE.GPL QAT_Engine-0.6.2/qat_contig_mem/qat_contig_mem.c QAT_Engine-0.6.2/qat_contig_mem/qat_contig_mem.h QAT_Engine-0.6.2/qat_contig_mem/qat_contig_mem_test.c 2.Hope using qatengine.so below (expected by OpenSSL) symlink to versioned library is fine and there are no other issues apart from this licensing. %{_libdir}/libqatengine.so.%{soversion}* %{enginesdir}/qatengine.so Please let us know if there are any other issues that needs change so that we can update the same in the new package.
We are planning to make one more change for library installation along with the license change where qatengine.so gets installed directly in the engines dir (/usr/lib64/engine-1.1) as the application that need to use engine can only be accessed via OpenSSL and there is no other application that can directly use qatengine. Makefile changes: qatengine_la_LDFLAGS = -module -no-undefined -avoid-version -shared This means there will be no versioned libary in the standard library dir. Could you please confirm having library installed directly in engines dir is fine as we see this approach being used by other engines in Redhat. Also please let us about license question on comment #21 because BSD/GPLv2 and GPLv2 are not used in the RPM. We are planning to do upstream release with changes based on your confirmation.
The updated spec file can be found here: Spec URL: https://raw.githubusercontent.com/intel/QAT_Engine/v0.6.3/qatengine.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/dineshbx/qatengine/fedora-rawhide-x86_64/01809699-qatengine/qatengine-0.6.3-1.fc34.src.rpm Build on copr: https://copr.fedorainfracloud.org/coprs/dineshbx/qatengine/build/1809699/ Build on koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=56635334 * The License tag in the spec file has been updated reporting BSD and OpenSSL and the OpenSSL license has been added to the upstream repository. The License tag does not include GPLv2 and (BSD or GPLv2) since the qat_contig_mem with license GPLv2 and the config files in qat/config with dual BSD/GPLv2 license are not used to generate the RPM or included in it. * The library(qatengine.so) is installed into openssl engines directory (/usr/lib64/engine-1.1)
I think installing qatengine.so directly in %{enginesdir} is the right choice. Thanks for making that change. On the licensing question, the guidelines answer this directly. > The License: field refers to the licenses of the contents of the binary rpm. I confirmed that the rpm build worked with the GPLv2 and BSD/GPLv2 code deleted during %prep, so you are correct to set the field to just BSD and OpenSSL. However, your license breakdown comment is insufficient. It needs to be clear which files are under which licenses, and furthermore should explain the situation with some of the unused code being under different licenses. This will prevent future packagers from "fixing" the license field incorrectly. I suggest: # Most of the source code is BSD, with the following exceptions: # - e_qat.txt, e_qat_err.c, and e_qat_err.h are OpenSSL # - qat/config/* are (BSD or GPLv2), but are not used during compilation # - qat_contig_mem/* are GPLv2, but are not used during compilation Once the license breakdown comment is fixed, I think this package will be ready to be approved.
I need some clarification on something else here. Who is the package maintainer going to be, Yogaraj or Dinesh? The FAS that has been referenced appears to be Dinesh (dineshbx), but Yogaraj has been more active during this package review. Considering I was asked to review this package because a sponsor is needed, I'm not comfortable with this situation. How can I sponsor Dinesh if Yogaraj did a substantial amount of the package review fixes?
Thanks a lot, Carl for the response. What's your expectation for the maintainer to do? We will discuss internally and reply back to you soon today.
Hi Carl, Dinesh is the maintainer for this time, and any change will let you know in the future. Thanks -Qihua
The spec file has been updated with clarifying the License. The updated spec file and srpm can be found here: Spec URL: https://copr-be.cloud.fedoraproject.org/results/dineshbx/qatengine/fedora-rawhide-x86_64/01811337-qatengine/qatengine.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/dineshbx/qatengine/fedora-rawhide-x86_64/01811337-qatengine/qatengine-0.6.3-1.fc34.src.rpm Build on copr: https://copr.fedorainfracloud.org/coprs/dineshbx/qatengine/build/1811337/ Build on koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=56764736 I will be the maintainer. Fedora Account System Username: dineshbx
> How can I sponsor Dinesh if Yogaraj did a substantial amount of the package review fixes? Myself and Dinesh have equal contribution to the packaging work. I discussed this today with Dinesh and Qihua(Engineering manager for QAT Engine in Intel) and if you prefer I can maintain the QAT Engine RPM. My Fedora Account System Username: yogaraj
(In reply to Carl George 🤠 from comment #24) > Once the license breakdown comment is fixed, I think this package will be > ready to be approved. I double-checked that the license breakdown comment is fixed in the .spec file from #c28. I've run a fedora-revew, the results are below. They are good, I would approve the package, but I don't wan to get into Carl's way. Dinesh, Yogaraj, please, note, after review you'll need to open bugzillas for each of those in ExcludeArch, mark them as blocking the tracker bugs listed in the guidelines [0], and include links to each of them in spec file comments. Please, see bz1885430#c12 and bz1897661 as a guidance. [0] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_build_failures ============== Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated, [ ] = Manual review needed ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [-]: Development (unversioned) .so files in -devel subpackage, if present. [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]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. 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: "BSD 3-clause "New" or "Revised" License", "BSD 4-clause "Original" or "Old" License Apache License 1.0", "Unknown or generated", "OpenSSL License", "OpenSSL License BSD 3-clause "New" or "Revised" License", "GNU Lesser General Public License", "GPL (v2)", "BSD 3-clause "New" or "Revised" License GPL (v2)". 9 files have unknown license. Detailed output of licensecheck in /root/qatengine/review-qatengine/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: 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. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: 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. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [!]: Package is not known to require an ExcludeArch tag. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 81920 bytes in 1 files. [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: No rpmlint messages. [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 in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [!]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Fully versioned dependency in subpackages if applicable. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: No rpmlint messages. [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Package should not use obsolete m4 macros [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: qatengine-0.6.3-1.fc34.x86_64.rpm qatengine-debuginfo-0.6.3-1.fc34.x86_64.rpm qatengine-debugsource-0.6.3-1.fc34.x86_64.rpm qatengine-0.6.3-1.fc34.src.rpm 4 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (debuginfo) ------------------- Checking: qatengine-debuginfo-0.6.3-1.fc34.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- 3 packages and 0 specfiles checked; 0 errors, 0 warnings. Unversioned so-files -------------------- qatengine: /usr/lib64/engines-1.1/qatengine.so Source checksums ---------------- https://github.com/intel/QAT_Engine/archive/v0.6.3/qatengine-0.6.3.tar.gz : CHECKSUM(SHA256) this package : 7787d8e328f0dafcfb78f47103387b3ee85cb803f6fd337757c967225138af5b CHECKSUM(SHA256) upstream package : 7787d8e328f0dafcfb78f47103387b3ee85cb803f6fd337757c967225138af5b Requires -------- qatengine (rpmlib, GLIBC filtered): libc.so.6()(64bit) libqat.so.0()(64bit) libusdm.so.0()(64bit) rtld(GNU_HASH) qatengine-debuginfo (rpmlib, GLIBC filtered): qatengine-debugsource (rpmlib, GLIBC filtered): Provides -------- qatengine: qatengine qatengine(x86-64) qatengine-debuginfo: debuginfo(build-id) qatengine-debuginfo qatengine-debuginfo(x86-64) qatengine-debugsource: qatengine-debugsource qatengine-debugsource(x86-64) Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10 Command line :/usr/bin/fedora-review -n qatengine
> What's your expectation for the maintainer to do? https://docs.fedoraproject.org/en-US/fesco/Package_maintainer_responsibilities/ > I double-checked that the license breakdown comment is fixed in the .spec file from #c28. > I've run a fedora-revew, the results are below. They are good, I would approve the package, > but I don't wan to get into Carl's way. Thanks for the assist Vladis. Last week and this week have been super busy. Package is approved. I've also sponsored Yogaraj into the packagers group, he can proceed with the repo request and import as documented here. https://fedoraproject.org/wiki/Package_Review_Process#Contributor > Dinesh, Yogaraj, please, note, after review you'll need to open bugzillas for each of those > in ExcludeArch, mark them as blocking the tracker bugs listed in the guidelines [0], and > include links to each of them in spec file comments. Please, see bz1885430#c12 and > bz1897661 as a guidance. Don't forget this part after the package repo is created.
Thanks for the approval. Repo request[1] created by me was closed as invalid due to the reason "The Bugzilla review bug creator didn't match the requester in Pagure." I have created new Review BZ https://bugzilla.redhat.com/show_bug.cgi?id=1908767 in my name. Could you please approve the BZ #1908767 to proceed with repo creation ? [1] https://pagure.io/releng/fedora-scm-requests/issue/31368
Done. Closing this one out so work can continue over there. *** This bug has been marked as a duplicate of bug 1908767 ***