Bug 2214269
| Summary: | Review requested: openssl-pkcs11-sign-provider - OpenSSL 3.0 provider for pkcs#11 (private key operations only) | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Holger Dengler <dengler> | ||||||||||
| Component: | Package Review | Assignee: | Dan Horák <dan> | ||||||||||
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||||||
| Severity: | unspecified | Docs Contact: | |||||||||||
| Priority: | unspecified | ||||||||||||
| Version: | rawhide | CC: | dan, jschmidb, package-review, pemensik, tstaudt | ||||||||||
| Target Milestone: | --- | Flags: | dan:
fedora-review+
|
||||||||||
| Target Release: | --- | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | Linux | ||||||||||||
| Whiteboard: | |||||||||||||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |||||||||||
| Doc Text: | Story Points: | --- | |||||||||||
| Clone Of: | Environment: | ||||||||||||
| Last Closed: | 2023-06-13 15:07:01 UTC | Type: | Bug | ||||||||||
| Regression: | --- | Mount Type: | --- | ||||||||||
| Documentation: | --- | CRM: | |||||||||||
| Verified Versions: | Category: | --- | |||||||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||||||
| Embargoed: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Holger Dengler
2023-06-12 12:44:35 UTC
Created attachment 1970418 [details]
Source RPM
I think it looks good (a formal review will follow), but I have a question about the tests. There are BuildRequires listed for tests, but the %check section is missing. If I understand it right, then the tests need the pkcsslotd service running, which can't be expected during the regular rpm build. I think there are ~3 options - comment out (or remove) the test related BRs - add %check with commented out "make check" with some comment about running pkcsslotd in advance - hide both the test BRs and %check behind a "with_test" condition, again with comment about pkcsslotd I suppose starting pkcsslotd from the tests is not possible ... Holger, what do you think? You're right, for executing the tests, pkcsslotd is (currently) required, as well as a working opencryptoki and p11-kit setup. For the moment, I would like to go forward with option 2. As soon as there are other test-modules available (as the test-framework allow it), I can remove the comments and activate `make check` during package build. Created attachment 1970647 [details]
Specfile (with empty %check section)
Created attachment 1970648 [details]
Source RPM
formal review is here, see the notes explaining OK* and BAD statuses below: OK source files match upstream: 4e28e1e21ee93bd15c8ae9fe6cad565d417bc0f2 openssl-pkcs11-sign-provider-1.0.0.tar.gz OK package meets naming and versioning guidelines. OK* specfile is properly named, is cleanly written and uses macros consistently. OK dist tag is present. OK license field matches the actual license. OK license is open source-compatible (Apache-2.0). License text included in package. OK latest version is being packaged. OK BuildRequires are proper. OK compiler flags are appropriate. OK package builds in mock (Rawhide/ppc64le). OK debuginfo package looks complete. OK rpmlint is silent. OK final provides and requires look sane. N/A %check is present and all tests pass. OK no shared libraries are added to the regular linker search paths. OK owns the directories it creates. OK doesn't own any directories it shouldn't. OK no duplicates in %files. OK file permissions are appropriate. OK no scriptlets present. OK code, not content. OK documentation is small, so no -docs subpackage is necessary. OK %docs are not necessary for the proper functioning of the package. OK no headers. OK no pkgconfig files. OK no libtool .la droppings. OK not a GUI app. - I think you can switch to %autochangelog in addition to %autorelease before importing, see https://fedoraproject.org/wiki/Changes/rpmautospec for some details The package is APPROVED and I will sponsor you into the packager group as well. I would recommend using %bcond_with tests. That way it can enabled during build time. Makes it easy to do test local build by: fedpkg local --with tests But will stay disabled by default, where it requires something extra to pass. I did something similar on cpp-httplib package with %bcond_with online: https://src.fedoraproject.org/rpms/cpp-httplib/blob/rawhide/f/cpp-httplib.spec#_5 That makes it possible to change behaviour slightly by defining --with parameter. fedpkg mockbuild allows it too, in case it can work even from chroot. The Pagure repository was created at https://src.fedoraproject.org/rpms/openssl-pkcs11-sign-provider FEDORA-2023-42193d9705 has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-42193d9705 FEDORA-2023-42193d9705 has been pushed to the Fedora 39 stable repository. If problem still persists, please make note of it in this bug report. FEDORA-2023-76b638f02b has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-76b638f02b FEDORA-2023-128b5cecf0 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2023-128b5cecf0 FEDORA-2023-76b638f02b has been pushed to the Fedora 38 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-76b638f02b \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-76b638f02b See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates. FEDORA-2023-128b5cecf0 has been pushed to the Fedora 37 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-128b5cecf0 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-128b5cecf0 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates. FEDORA-2023-128b5cecf0 has been pushed to the Fedora 37 stable repository. If problem still persists, please make note of it in this bug report. FEDORA-2023-76b638f02b has been pushed to the Fedora 38 stable repository. If problem still persists, please make note of it in this bug report. |