Spec URL: https://jjelen.fedorapeople.org/kryoptic.spec SRPM URL: https://jjelen.fedorapeople.org/kryoptic-0.1.0%5e20241113.git14a7c8f9-1.fc42.src.rpm Description: A PKCS #11 software token written in Rust. Fedora Account System Username: jjelen Copr builds are available here: https://copr.fedorainfracloud.org/coprs/jjelen/kryoptic/ Note, that this is pre-release build from latest git snapshot. We are planning to get the 1.0.0 version release upstream soon so we will likely end up importing only the final release, which should not differ much from the current version.
Copr build: https://copr.fedorainfracloud.org/coprs/build/8255132 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2325933-kryoptic/fedora-rawhide-x86_64/08255132-kryoptic/fedora-review/review.txt Found issues: - Unversioned so-files directly in %_libdir. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages - Not a valid SPDX expression 'GPL-3.0'. Read more: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1 Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
> - Unversioned so-files directly in %_libdir. > Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages This is intended -- the pkcs11 modules are not for linking, but for dlopen-ing and have fixed ABI. > - Not a valid SPDX expression 'GPL-3.0'. Read more: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1 Fixed to GPL-3.0-or-later Updated the links: Spec URL: https://jjelen.fedorapeople.org/kryoptic.spec SRPM URL: https://jjelen.fedorapeople.org/kryoptic-0.1.0%5e20241113.git14a7c8f9-1.fc42.src.rpm
Created attachment 2057673 [details] The .spec file difference from Copr build 8255132 to 8257407
Copr build: https://copr.fedorainfracloud.org/coprs/build/8257407 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2325933-kryoptic/fedora-rawhide-x86_64/08257407-kryoptic/fedora-review/review.txt Found issues: - Unversioned so-files directly in %_libdir. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Updated to current version copr: https://copr.fedorainfracloud.org/coprs/jjelen/kryoptic/build/8414994/ Spec URL: https://jjelen.fedorapeople.org/kryoptic.spec SRPM URL: https://jjelen.fedorapeople.org/kryoptic-0.1.0%5e20241219.git6166f28a-1.fc42.src.rpm
Created attachment 2063304 [details] The .spec file difference from Copr build 8257407 to 8422625
Copr build: https://copr.fedorainfracloud.org/coprs/build/8422625 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2325933-kryoptic/fedora-rawhide-x86_64/08422625-kryoptic/fedora-review/review.txt Found issues: - Not a valid SPDX expression 'GPL-3.0'. Read more: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1 Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Uploaded wrong spec/srpm files. Second try: Spec URL: https://jjelen.fedorapeople.org/kryoptic.spec SRPM URL: https://jjelen.fedorapeople.org/kryoptic-0.1.0%5e20241219.git6166f28a-1.fc42.src.rpm
Created attachment 2063328 [details] The .spec file difference from Copr build 8422625 to 8426087
Copr build: https://copr.fedorainfracloud.org/coprs/build/8426087 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2325933-kryoptic/fedora-rawhide-x86_64/08426087-kryoptic/fedora-review/review.txt Found issues: - Unversioned so-files directly in %_libdir. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Spec URL: https://jjelen.fedorapeople.org/kryoptic.spec SRPM URL: https://jjelen.fedorapeople.org/kryoptic-0.1.0%5e20250225.gitb37f7ee1-1.fc43.src.rpm Copr build: https://copr.fedorainfracloud.org/coprs/jjelen/kryoptic/build/8698149/
Created attachment 2077751 [details] The .spec file difference from Copr build 8426087 to 8698150
Copr build: https://copr.fedorainfracloud.org/coprs/build/8698150 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2325933-kryoptic/fedora-rawhide-x86_64/08698150-kryoptic/fedora-review/review.txt Found issues: - Unversioned so-files directly in %_libdir. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
> - Unversioned so-files directly in %_libdir. > Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages The PKCS#11 modules are not versioned and do not come in devel sub-package: https://docs.fedoraproject.org/en-US/packaging-guidelines/Pkcs11Support/
Package was generated with rust2rpm, simplifying the review. ✅ package contains only permissible content ✅ package builds and installs without errors on rawhide ✅ test suite is run and all unit tests pass ✅ latest version of the crate is packaged ✅ license matches upstream specification and is acceptable for Fedora ✅ license file is included with %license in %files ✅ package complies with Rust Packaging Guidelines Two midly annoying reports from rpmlint: kryoptic.x86_64: E: unused-direct-shlib-dependency /usr/lib64/libkryoptic_pkcs11.so /lib64/libm.so.6 kryoptic.x86_64: W: no-soname /usr/lib64/libkryoptic_pkcs11.so Package APPROVED. === Recommended post-import rust-sig tasks: - set up package on release-monitoring.org: project: $crate homepage: https://crates.io/crates/$crate backend: crates.io version scheme: semantic version (*NOT* pre-release) filter: alpha;beta;rc;pre distro: Fedora Package: rust-$crate - add @rust-sig with "commit" access as package co-maintainer (should happen automatically) - set bugzilla assignee overrides to @rust-sig (optional) - track package in koschei for all built branches (should happen automatically once rust-sig is co-maintainer)
The Pagure repository was created at https://src.fedoraproject.org/rpms/kryoptic
Thank you! Repo request: https://pagure.io/releng/fedora-scm-requests/issue/72680 Fedora 42 branch request: https://pagure.io/releng/fedora-scm-requests/issue/72681 release-monitoring: https://release-monitoring.org/project/376956/ (will likely have to adjust after the final release) Built the pre-release now: https://koji.fedoraproject.org/koji/taskinfo?taskID=129607188 We should get a final release soon.
There's several things that are strange and / or wrong in the spec file: 1. The SourceLicense is GPL-3.0-or-later, but this license doesn't show up in the License tag for the built package. That shouldn't be possible. 2. You're using the wrong GitHub archive (i.e. zip), the correct ones (.tar.gz) are documented here: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_commit_revision i.e. it should be https://github.com/latchset/kryoptic/archive/%{revision}/kryoptic-%{revision}.tar.gz 2. What is "BuildRequires: openssl-devel" for? This should not be necessary if this uses the Rust OpenSSL bindings. 3. I'm not sure if this actually works: > CONFDIR=%{_sysconfdir} %cargo_build -f dynamic,nssdb,standard since %cargo_build uses a subshell. I would recommend to do this instead: > export CONFDIR=%{_sysconfdir} > %cargo_build -f dynamic,nssdb,standard 4. You can also just skip %cargo_install macro and the "rm .../conformance" calls entirely - building and installing the executable just to delete it again is pointless. 5. The unversioned .so in %{_libdir} is not a minor annoyance, it's wrong. Even the pkcs11 docs say to install it under %{_libdir}/pkcs11/. > install -Dp target/rpm/libkryoptic_pkcs11.so $RPM_BUILD_ROOT/%{_libdir}/libkryoptic_pkcs11.so > %{_libdir}/libkryoptic_pkcs11.so Should be > install -Dp target/rpm/libkryoptic_pkcs11.so $RPM_BUILD_ROOT/%{_libdir}/pkcs11/libkryoptic_pkcs11.so > %{_libdir}/pkcs11/libkryoptic_pkcs11.so see https://docs.fedoraproject.org/en-US/packaging-guidelines/Pkcs11Support/#_registering_the_modules_system_wide
Side note for point 4) in the previous comment: If you skip %cargo_install, you can drop setting "%global cargo_install_lib 0" too, since that only affects %cargo_install.
> 1. The SourceLicense is GPL-3.0-or-later, but this license doesn't show up in the License tag for the built package. That shouldn't be possible. This is the license of the project itself: https://github.com/latchset/kryoptic/blob/main/LICENSE.txt Seems like I did not manage to put it into the License field correctly. In generated LICENSE.dependencies I can see it is correctly. I will update it. > 2. You're using the wrong GitHub archive (i.e. zip), the correct ones (.tar.gz) are documented here: > https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_commit_revision > i.e. it should be https://github.com/latchset/kryoptic/archive/%{revision}/kryoptic-%{revision}.tar.gz Thanks for the pointer! Will fix with the next update! > 2. What is "BuildRequires: openssl-devel" for? This should not be necessary if this uses the Rust OpenSSL bindings. It does not use the Rust OpenSSL bindings. It is linking to the openssl directly for reasons. > 3. I'm not sure if this actually works: > > > CONFDIR=%{_sysconfdir} %cargo_build -f dynamic,nssdb,standard > > since %cargo_build uses a subshell. > > I would recommend to do this instead: > > > export CONFDIR=%{_sysconfdir} > > %cargo_build -f dynamic,nssdb,standard Thank you! Will change! > 4. You can also just skip %cargo_install macro and the "rm .../conformance" calls entirely - building and installing the executable just to delete it again is pointless. We still install the %{_bindir}/softhsm_migrate which should land from this command. > 5. The unversioned .so in %{_libdir} is not a minor annoyance, it's wrong. Even the pkcs11 docs say to install it under %{_libdir}/pkcs11/. > > > install -Dp target/rpm/libkryoptic_pkcs11.so $RPM_BUILD_ROOT/%{_libdir}/libkryoptic_pkcs11.so > > %{_libdir}/libkryoptic_pkcs11.so > > Should be > > > install -Dp target/rpm/libkryoptic_pkcs11.so $RPM_BUILD_ROOT/%{_libdir}/pkcs11/libkryoptic_pkcs11.so > > %{_libdir}/pkcs11/libkryoptic_pkcs11.so > > see https://docs.fedoraproject.org/en-US/packaging-guidelines/Pkcs11Support/#_registering_the_modules_system_wide Thank you! Will fix.
Updated the spec file in https://src.fedoraproject.org/rpms/kryoptic/c/2e0b4c80c9b6bc5eb79420530394f11cc91ccea5 let me know if I missed some of your comments.
Apparently yes :D You can drop these four lines: > # prevent library files from being installed > %global cargo_install_lib 0 > %cargo_install -f dynamic,nssdb,standard > rm -f $RPM_BUILD_ROOT/%{_bindir}/conformance If you don't call %cargo_install, the rest is unnecessary. You're already installing the .so file manually, so %cargo_install does *only* things you need to manually prevent or un-do. So ... just don't call %cargo_install xD You also added a new issue into the License tag. The "GPL-3.0" identifier is not a valid SPDX identifier in Fedora. It is deprecated in SPDX upstream, and should be either "GPL-3.0-or-later" or "GPL-3.0-only". Given that the SourceLicense is GPL-3.0-or-later, I assume it should be the former. I don't know where you copied the deprecated identifier from, because whereever it came from, it should have matched where GPL-3.0-or-later for SourceLicense came from. So there appears to be either an inconsistency or a copy-paste-error happened.
(In reply to Fabio Valentini from comment #22) > Apparently yes :D > > You can drop these four lines: > > > # prevent library files from being installed > > %global cargo_install_lib 0 > > %cargo_install -f dynamic,nssdb,standard > > rm -f $RPM_BUILD_ROOT/%{_bindir}/conformance > > If you don't call %cargo_install, the rest is unnecessary. > You're already installing the .so file manually, so %cargo_install does > *only* things you need to manually prevent or un-do. > So ... just don't call %cargo_install xD What about the `%{_bindir}/softhsm_migrate`? It is still needed to be installed. > You also added a new issue into the License tag. > > The "GPL-3.0" identifier is not a valid SPDX identifier in Fedora. It is > deprecated in SPDX upstream, and should be either "GPL-3.0-or-later" or > "GPL-3.0-only". Given that the SourceLicense is GPL-3.0-or-later, I assume > it should be the former. Thanks for double-checking. Fixed. > I don't know where you copied the deprecated identifier from, because > whereever it came from, it should have matched where GPL-3.0-or-later for > SourceLicense came from. So there appears to be either an inconsistency or a > copy-paste-error happened. Let me try to backtrace the issue. I think I took it from the output of `%cargo_license_summary`. Running `mockbuild results in the following output, which I used to update the licence from: ``` + /usr/bin/env CARGO_HOME=.cargo RUSTC_BOOTSTRAP=1 'RUSTFLAGS=-Copt-level=3 -Cdebuginfo=2 -Ccodegen-units=1 -Cstrip=none -Cforce-frame-pointers=yes -Clink-arg=-specs=/usr/lib/rpm/redhat/redhat-package-notes --cap-lints=warn' /usr/bin/cargo tree -Z avoid-dev-deps --workspace --offline --edges no-build,no-dev,no-proc-macro --no-dedupe --target all --features dynamic,nssdb,standard --prefix none --format '# {l}' + sed -e 's: / :/:g' -e 's:/: OR :g' + sort -u # Apache-2.0 # Apache-2.0 OR BSL-1.0 # Apache-2.0 OR MIT # BSD-2-Clause OR Apache-2.0 OR MIT # BSD-3-Clause # GPL-3.0 # MIT # MIT OR Apache-2.0 # MIT-0 OR Apache-2.0 # Unlicense OR MIT ```
> What about the `%{_bindir}/softhsm_migrate`? It is still needed to be installed. Ah, true. I missed that one. You could install that in the same way manually as you install the .so file, but in that case %cargo_install is not *entirely* useless :) > Let me try to backtrace the issue. I think I took it from the output of `%cargo_license_summary`. Running `mockbuild results in the following output, which I used to update the licence from: (...) This means that some crate in the dependency tree as `package.license = "GPL-3.0"` in its Cargo.toml. The LICENSE.dependencies file should tell you which one.
And obviously, its our package: # grep GPL-3 LICENSE.dependencies GPL-3.0: kryoptic v1.0.0 https://github.com/latchset/kryoptic/blob/main/Cargo.toml#L8 Will get it fixed. Thanks for pointing this out!