SPEC: https://pbrobinson.fedorapeople.org/fido-device-onboard.spec SRPM: https://pbrobinson.fedorapeople.org/fido-device-onboard-0.4.6-1.fc37.src.rpm Description: A rust implementation of the FIDO Device Onboard Specification FAS: pbrobinson koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=928906109
Quick comments from Rust packaging POV: 1) %global __cargo_skip_build 0 Where did you get this? This macro hasn't been used for ages and doesn't do anything. 2) %global __cargo_is_lib() false This should not be necessary. 3) Building against vendored dependencies is forbidden for a Fedora package. At the very least (for ELN / RHEL), they need to be listed as "Provides: bundled(crate(foo)) = 1.2.3" alongside their licenses. 4) > # Needs, at least, tss bindings regen / ExcludeArch: s390x i686 %{power64} This probably needs to be fixed in the rust-tss-esapi-sys package? 5) %{__cargo} build --release --features openssl-kdf/deny_custom This looks very weird, why are you using this custom build command? You should be able to pass features to "%cargo_build" easily enough with something like "%cargo_build -f openssl-kdf/deny_custom". Using standard "%cargo_build" should also make setting "profile.release.debug = true" unnecessary, since the correct flags are set by %cargo_build already. 6) %bcond_without check This does nothing if you don't use %cargo_generate_buildrequires, since there's not even a %check section. 7) License tag does not reflect bundled / statically linked components. General question: The package name is fido-*, the summary mentions FIDO, but the subpackages are fdo* and only mention FDO? Is that the same thing or are there typos in some places?
> General question: The package name is fido-*, the summary mentions FIDO, but > the subpackages are fdo* and only mention FDO? Is that the same thing or are > there typos in some places? FDO = FIDO Device Onboarding. FIDO is the organisation, it had more than one standard (the web auth standard is the best known, this is not that).
> 3) Building against vendored dependencies is forbidden for a Fedora package. Is it? There's a number of other packages that do it and I've had discussions with you about it in the past.
(In reply to Peter Robinson from comment #3) > > Is it? There's a number of other packages that do it and I've had > discussions with you about it in the past. Yes, it is. Just because other people ignore Fedora rules doesn't mean they're not there: https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/#_bundled_dependencies https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling In particular: > "All packages whose upstreams allow them to be built against system libraries MUST be built against system libraries." TL;DR: If possible to do so, packages must be built without bundled / vendored dependencies. This is not a Rust-specific rule.
> TL;DR: If possible to do so, packages must be built without bundled / > vendored dependencies. This is not a Rust-specific rule. It's an order of magnitude more work to do individual packages and the cat herding for every single upgrade, I've done that before and I don't have the time or the interest in doing it again and as there's already precedent for bundling/vendoring (in nodejs too) I would prefer to go this way so I don't need to spend the little bit of free time I actually have for Fedora work cat herding tedious package updates.
You mean the NodeJS precedent that caused most applications to be removed from Fedora, because nobody could claim to have verified that the resulting vendor tarballs didn't contain pre-compiled or otherwise objectionable content? I manage to do the cat herding for 1000+ Rust packages in my free time, so I'm sure you can manage too.
> I manage to do the cat herding for 1000+ Rust packages in my free time, so > I'm sure you can manage too. I'm sorry, I don't believe it's for you to state how much free time I have and what I use it for in Fedora.
If pbrobinson wants to vendor this package's dependencies, he is free to do so. But I will not approve this review if it violates our rules and guidelines, and I assume other Rust SIG members will not do so, either. If somebody else wants to approve this package despite this, they are free to do so.
I've worked through the backlog of missing updates over the past few weeks, and it looks like everything you need should be present in Fedora repositories now?
(In reply to Fabio Valentini from comment #9) > I've worked through the backlog of missing updates over the past few weeks, > and it looks like everything you need should be present in Fedora > repositories now? Yes, we're almost there, I have a few more out of tree deps to package. I will post the latest rev of the spec/srpm shortly.
> But I will not approve this review if it violates our rules and guidelines, So I think we're finally there, for this release at least, interested in reviewing or at least providing feedback? I *think* I have addressed everything but there's also a lot changed since the last rev. SPEC: https://pbrobinson.fedorapeople.org/fido-device-onboard.spec SRPM: https://pbrobinson.fedorapeople.org/fido-device-onboard-0.4.9-1.fc38.src.rpm koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=101015497
Thanks! I appreciate the work you've put into this. I am travelling until ~this Sunday, so I will not be able to get to it until Sunday or Monday. After just a quick glance at the spec file I see no obvious show-stoppers from a Rust POV (other than License tag not taking into account statically linked Rust deps), so if somebody else wants to review the package before I get to it, that's fine with me. For the license stuff, I recommend to use the %cargo_license macro to generate the license breakdown, and %cargo_license_summary to list the individual licenses of all components that end up linked into shipped binaries. For a usage example, you can look at the spec file for rust-rpm-sequoia (or any other Rust application that has been updated in the last few months). I'm planning to document these macros in the Rust Packaging Guidelines ASAP (after applying a small but necessary fix to make them work with %bcond check being disabled).
> from a Rust POV (other than License tag not taking into account statically > linked Rust deps) Yes, I knew that would be an issue > For the license stuff, I recommend to use the %cargo_license macro to > generate the license breakdown, and %cargo_license_summary to list the Will these work in RHEL-8/9?
(In reply to Peter Robinson from comment #13) > > For the license stuff, I recommend to use the %cargo_license macro to > > generate the license breakdown, and %cargo_license_summary to list the > > Will these work in RHEL-8/9? They work in EPEL9 since yesterday, but the lobotomized Rust toolchain available in RHEL almost certainly doesn't include them. :( But on RHEL it should be possible to use the underlying mechanism directly (i.e. the "cargo tree | sed | sort" commands that the %cargo_license* macros expand to).
> They work in EPEL9 since yesterday, but the lobotomized Rust toolchain > available in RHEL almost certainly doesn't include them. :( > But on RHEL it should be possible to use the underlying mechanism directly > (i.e. the "cargo tree | sed | sort" commands that the %cargo_license* macros > expand to). May just generate it out of band and include it for each upstream release to mitigate that. Thx
YMMV. That may or may not produce accurate results - either due to different versions of crates upstream / in Fedora, downstream patches, or because upstream metadata is wrong but the one in Fedora was corrected.
(PS: But of course this would work fine when generated list is only used for RHEL builds, and the dynamic list from %cargo_license macros is used for Fedora.)
Sorry for the delay, here's an initial review pass. === This package no longer builds after the recent stratisd / stratis-cli / rust-libcryptsetup-rs updates: https://bodhi.fedoraproject.org/updates/?search=rust-libcryptsetup-rs-0.7.0 Error: Problem 1: nothing provides requested (crate(libcryptsetup-rs/default) >= 0.6.1 with crate(libcryptsetup-rs/default) < 0.7.0~) Problem 2: nothing provides requested (crate(libcryptsetup-rs/mutex) >= 0.6.1 with crate(libcryptsetup-rs/mutex) < 0.7.0~) (try to add '--skip-broken' to skip uninstallable packages or '--nobest' to use not only best candidate packages) Not sure if there are API changes between libcryptsetup-rs v0.6 and v0.7 that affect FDO. Options to resolve this are either to 1. bump dependency from 0.6.1 to 0.7 (assuming there are no API changes that affect FDO), or 2. report this issue upstream and port FDO to the new version. Creating "compat" packages in Fedora for v0.6 is not really an option in this case - it can have unintended consequences (i.e. build failures with obscure error messages) if there are two different crates that link the same shared library (in this case, libcryptsetup-rs-sys, which links libcryptsetup.so.12). === Other things that I can check without building the package: 1. Do not include source / patch files depending on the build environment. This results in non-portable SRPM files, and is forbidden by the packaging guidelines. 2. "ExclusiveArch: %{rust_arches}" should be removed, it is a noop. It is no longer necessary, and in fact no longer mentioned in the Rust packaging guidelines. 3. I would recommend not to use the %forge macros for simple packages like this. There's discussions in the FPC about deprecating them, given that they have been unmaintained for years. Especially in simple cases like this (i.e. tarball for a tag on GitHub), they don't have any advantages compared to the standard Source URL. c.f. https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_git_tags 5. The package must use SPDX identifier for the License tag. "BSD" is ambiguous in this regard, but it looks like the upstream license is "BSD-3-Clause". 6. It looks like the package contains a Go wrapper around the Rust component (there's also BuildRequires: golang) - so I assume the Go part is built, but I don't see it being installed or shipped in a package. Is this intentional? 7. The thing that's still missing from the package is the license of the statically linked binaries. The %cargo_license / %cargo_license_summary macros can be used for this purpose for this on Fedora and EPEL9, but they are not available in RHEL. The output of the %cargo_license_summary macro can be used to collect the License tag that needs to be applied to all subpackages that contain statically linked Rust binaries. You can take a look at rust-sequoia-octopus-librnp how the current "best practice" around this looks like: - listing licenses and specifying License tag in subpackage (using SPDX expressions and according to latest Red Hat Legal guidance): https://src.fedoraproject.org/rpms/rust-sequoia-octopus-librnp/blob/rawhide/f/rust-sequoia-octopus-librnp.spec#_43-61 - including the license breakdown for all components: https://src.fedoraproject.org/rpms/rust-sequoia-octopus-librnp/blob/rawhide/f/rust-sequoia-octopus-librnp.spec#_72 - printing license summary during the build and dynamically generating the breakdown during the build: https://src.fedoraproject.org/rpms/rust-sequoia-octopus-librnp/blob/rawhide/f/rust-sequoia-octopus-librnp.spec#_85-86 You can also take a look at recent changes to Rust packages to make them build with vendored dependencies on ELN, which incorporate similar changes in a way to make the package build in both Fedora and ELN. For example, in rust-rpm-sequoia: https://src.fedoraproject.org/rpms/rust-rpm-sequoia/c/997d42deeb441597944cbeb30ce9af303ff0d799?branch=eln
> Sorry for the delay, here's an initial review pass. No issues, comments below > This package no longer builds after the recent stratisd / stratis-cli / > rust-libcryptsetup-rs updates: > https://bodhi.fedoraproject.org/updates/?search=rust-libcryptsetup-rs-0.7.0 > > Error: > Problem 1: nothing provides requested (crate(libcryptsetup-rs/default) >= > 0.6.1 with crate(libcryptsetup-rs/default) < 0.7.0~) > Problem 2: nothing provides requested (crate(libcryptsetup-rs/mutex) >= > 0.6.1 with crate(libcryptsetup-rs/mutex) < 0.7.0~) > (try to add '--skip-broken' to skip uninstallable packages or '--nobest' to > use not only best candidate packages) > > Not sure if there are API changes between libcryptsetup-rs v0.6 and v0.7 > that affect FDO. > Options to resolve this are either to 1. bump dependency from 0.6.1 to 0.7 > (assuming there are no API changes that affect FDO), or 2. report this issue > upstream and port FDO to the new version. > > Creating "compat" packages in Fedora for v0.6 is not really an option in > this case - it can have unintended consequences (i.e. build failures with > obscure error messages) if there are two different crates that link the same > shared library (in this case, libcryptsetup-rs-sys, which links > libcryptsetup.so.12). > > === > > Other things that I can check without building the package: > > 1. Do not include source / patch files depending on the build environment. > This results in non-portable SRPM files, and is forbidden by the packaging > guidelines. > > 2. "ExclusiveArch: %{rust_arches}" should be removed, it is a noop. > It is no longer necessary, and in fact no longer mentioned in the Rust > packaging guidelines. > > 3. I would recommend not to use the %forge macros for simple packages like > this. > There's discussions in the FPC about deprecating them, given that they have > been unmaintained for years. > Especially in simple cases like this (i.e. tarball for a tag on GitHub), > they don't have any advantages compared to the standard Source URL. > c.f. > https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ > #_git_tags > > 5. The package must use SPDX identifier for the License tag. > "BSD" is ambiguous in this regard, but it looks like the upstream license is > "BSD-3-Clause". All of the above has been resolved. > 6. It looks like the package contains a Go wrapper around the Rust component > (there's also BuildRequires: golang) - so I assume the Go part is built, but > I don't see it being installed or shipped in a package. Is this intentional? Yes, and no, they're still under development, it's useful to build them in Fedora for some build coverage, will be including them in the next major bump soon. They're not important for the initial use cases in Fedora. I'm still working on the below. I would like to avoid things we can't reuse with RHEL because that increases ongoing workload maintenance. > 7. The thing that's still missing from the package is the license of the > statically linked binaries. > The %cargo_license / %cargo_license_summary macros can be used for this > purpose for this on Fedora and EPEL9, but they are not available in RHEL. > > The output of the %cargo_license_summary macro can be used to collect the > License tag that needs to be applied to all subpackages that contain > statically linked Rust binaries. You can take a look at > rust-sequoia-octopus-librnp how the current "best practice" around this > looks like: > > - listing licenses and specifying License tag in subpackage (using SPDX > expressions and according to latest Red Hat Legal guidance): > > https://src.fedoraproject.org/rpms/rust-sequoia-octopus-librnp/blob/rawhide/ > f/rust-sequoia-octopus-librnp.spec#_43-61 > - including the license breakdown for all components: > > https://src.fedoraproject.org/rpms/rust-sequoia-octopus-librnp/blob/rawhide/ > f/rust-sequoia-octopus-librnp.spec#_72 > - printing license summary during the build and dynamically generating the > breakdown during the build: > > https://src.fedoraproject.org/rpms/rust-sequoia-octopus-librnp/blob/rawhide/ > f/rust-sequoia-octopus-librnp.spec#_85-86 > > You can also take a look at recent changes to Rust packages to make them > build with vendored dependencies on ELN, which incorporate similar changes > in a way to make the package build in both Fedora and ELN. For example, in > rust-rpm-sequoia: > https://src.fedoraproject.org/rpms/rust-rpm-sequoia/c/ > 997d42deeb441597944cbeb30ce9af303ff0d799?branch=eln I will add that to my to-do list but I'd like to complete this review, and I don't believe that's blocking? spec: https://pbrobinson.fedorapeople.org/fido-device-onboard.spec srpm: https://pbrobinson.fedorapeople.org/fido-device-onboard-0.4.9-2.fc38.src.rpm koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=101678980
Copr build: https://copr.fedorainfracloud.org/coprs/build/5992345 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2133551-fido-device-onboard/fedora-rawhide-x86_64/05992345-fido-device-onboard/fedora-review/review.txt Please take a look if any issues were found. --- 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.
Sorry, but a wrong license tag certainly is a blocking issue. At the very least, the spec file will need to contain - a list of the licenses of all dependencies that get statically linked into shipped binaries, and - the resulting License tag (i.e. SPDX expression resulting from AND-ing them all together) You've already included RPM macros that don't work in RHEL in the spec file (i.e. %cargo_license_summary and %cargo_license) - so I don't understand why copying the output of %cargo_license_summary from the build log into the spec file and AND-ing everything together to form the value of the License tag is an additional problem?
(In reply to Fabio Valentini from comment #21) > Sorry, but a wrong license tag certainly is a blocking issue. I don't see where I said that it wasn't a blocking issue. I said I was still working on it. > At the very least, the spec file will need to contain > - a list of the licenses of all dependencies that get statically linked into > shipped binaries, and > - the resulting License tag (i.e. SPDX expression resulting from AND-ing > them all together) > > You've already included RPM macros that don't work in RHEL in the spec file > (i.e. %cargo_license_summary and %cargo_license) - so I don't understand why > copying the output of %cargo_license_summary from the build log into the > spec file and AND-ing everything together to form the value of the License > tag is an additional problem?
Sorry, I must have misinterpreted this line (or associated with the wrong thing): > I will add that to my to-do list but I'd like to complete this review, and I don't believe that's blocking?
(In reply to Fabio Valentini from comment #23) > Sorry, I must have misinterpreted this line (or associated with the wrong > thing): > > > I will add that to my to-do list but I'd like to complete this review, and I don't believe that's blocking? I meant the use of the macros isn't blocking, but I think in the short term I'm going to just use it and sort out something that works for both Fedora and RHEL in parallel and update it some time in the future.
OK, I've reviewed the licenses of all the dependencies, and I think the license tag for this package should be: Apache-2.0 AND (Apache-2.0 OR BSL-1.0) AND (Apache-2.0 OR ISC OR MIT) AND (Apache-2.0 OR MIT) AND ((Apache-2.0 OR MIT) AND BSD-3-Clause) AND (Apache-2.0 ITH LLVM-exception OR Apache-2.0 OR MIT) AND BSD-2-Clause AND BSD-3-Clause AND (CC0-1.0 OR Apache-2.0) AND (CC0-1.0 OR MIT-0 OR Apache 2.0) AND ISC AND MIT AND ((MIT OR Apache-2.0) AND Unicode-DFS-2016) AND (Apache-2.0 OR MIT OR Zlib) AND MPL-2.0 AND (Unlicense OR MIT)
spec: https://pbrobinson.fedorapeople.org/fido-device-onboard.spec srpm: https://pbrobinson.fedorapeople.org/fido-device-onboard-0.4.9-3.fc38.src.rpm Updated for the licensing, added the cargo_license macros and the LICENSE.dependencies as well as Jared's license overview. This should now be complete..... yes I know it is currently failing to build, some deps in the rust merry-go-round have changed which I'm reviewing (I don't thing those block this remaining review item though).
It was another bump in libcryptsetup-rs, updated, testing a build
Created attachment 1970585 [details] The .spec file difference from Copr build 5992345 to 6072492
Copr build: https://copr.fedorainfracloud.org/coprs/build/6072492 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2133551-fido-device-onboard/fedora-rawhide-x86_64/06072492-fido-device-onboard/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- 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.
It builds on F-38: https://koji.fedoraproject.org/koji/taskinfo?taskID=102090348 I am working to get the bump for libcryptsetup-rs as it needs code changes.
spec: https://pbrobinson.fedorapeople.org/fido-device-onboard.spec srpm: https://pbrobinson.fedorapeople.org/fido-device-onboard-0.4.9-4.fc38.src.rpm koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=102135541 OK, rawhide build fixed, I believe all things are now addressed. Fabio can you give this a final review, I'd like to get this closed out :)
Created attachment 1970818 [details] The .spec file difference from Copr build 6072492 to 6080294
Copr build: https://copr.fedorainfracloud.org/coprs/build/6080294 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2133551-fido-device-onboard/fedora-rawhide-x86_64/06080294-fido-device-onboard/fedora-review/review.txt Please take a look if any issues were found. --- 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.
RPM cannot parse the linked spec file - the %{gittag} macro is undefined (and it looks like it's no longer used). Additionally, at first glance, "%global debug_package %{nil}" is a red flag. It should not be needed - but if it *is* needed to make RPM happy, then the package is built without debuginfo, i.e. with nonstandard compiler flags. I can't see any indication of custom cargo profiles, so likely "%global debug_package %{nil}" can just be removed to restore debuginfo subpackage generation.
Note: I see that the License tag is now included in all subpackages. You could make updating it easier by deduplicating it, i.e. defining something like %global binary_license Apache-2.0 AND (Apache-2.0 OR BSL-1.0) AND ... at the top of the spec file and using "License: %binary_license" in subpackages.
(In reply to Fabio Valentini from comment #34) > RPM cannot parse the linked spec file - the %{gittag} macro is undefined > (and it looks like it's no longer used). It was cut and paste directly from the packaging guidelines you suggested in comment 18, point 3. I will remove. > Additionally, at first glance, "%global debug_package %{nil}" is a red flag. > It should not be needed - but if it *is* needed to make RPM happy, then the > package is built without debuginfo, i.e. with nonstandard compiler flags. I > can't see any indication of custom cargo profiles, so likely "%global > debug_package %{nil}" can just be removed to restore debuginfo subpackage > generation. Probabky a left over of the 100 revisions. Can't those just be fixed on commit?
(In reply to Fabio Valentini from comment #35) > Note: I see that the License tag is now included in all subpackages. You > could make updating it easier by deduplicating it, i.e. defining something > like > > %global binary_license Apache-2.0 AND (Apache-2.0 OR BSL-1.0) AND ... > > at the top of the spec file and using "License: %binary_license" in > subpackages. Sure, but I was just copying what rust-rpm-sequoia has as you had suggested, again I see this as a minor point that can be fixed on commit??
(In reply to Peter Robinson from comment #36) > (In reply to Fabio Valentini from comment #34) > > RPM cannot parse the linked spec file - the %{gittag} macro is undefined > > (and it looks like it's no longer used). > > It was cut and paste directly from the packaging guidelines you suggested in > comment 18, point 3. I will remove. Yes, but those guidelines should also mention that you need to actually define those macros somewhere :) In this case, I'm pretty sure you can just replace %{gittag} with version. > > Additionally, at first glance, "%global debug_package %{nil}" is a red flag. > > It should not be needed - but if it *is* needed to make RPM happy, then the > > package is built without debuginfo, i.e. with nonstandard compiler flags. I > > can't see any indication of custom cargo profiles, so likely "%global > > debug_package %{nil}" can just be removed to restore debuginfo subpackage > > generation. > > Probabky a left over of the 100 revisions. Can't those just be fixed on > commit? Sure. (In reply to Peter Robinson from comment #37) > (In reply to Fabio Valentini from comment #35) > > Note: I see that the License tag is now included in all subpackages. You > > could make updating it easier by deduplicating it, i.e. defining something > > like > > > > %global binary_license Apache-2.0 AND (Apache-2.0 OR BSL-1.0) AND ... > > > > at the top of the spec file and using "License: %binary_license" in > > subpackages. > > Sure, but I was just copying what rust-rpm-sequoia has as you had suggested, > again I see this as a minor point that can be fixed on commit?? rpm-sequoia has only one subpackage, so there's nothing to de-duplicate. But yes, this can be fixed post-approval-pre-import. In the meantime, I'll try to make fedora-review work in the case where the linked spec file is invalid ...
Ok, this works: > Source0: %{url}/archive/v%{version}/%{name}-rs-%{version}.tar.gz (Note the leading "v", since upstream tags releases "vX.Y.Z".)
(In reply to Fabio Valentini from comment #39) > Ok, this works: > > > Source0: %{url}/archive/v%{version}/%{name}-rs-%{version}.tar.gz > > (Note the leading "v", since upstream tags releases "vX.Y.Z".) Yep, I already have that locally :)
Thanks, I've done the full review now. Issues are listed in detail, and the fedora-review checklist and rpmlint output are included at the bottom. I'd like to have fixes or clarifications (maybe rpmlint has some false positives) for points 4, 8, and 9 before I approve the package. The others can be fixed post-approval / pre-import. ====================================================================== 1. Unowned directories: /usr/lib/dracut /usr/lib/dracut/modules.d /usr/lib/dracut/modules.d/52fdo /usr/libexec/fdo /usr/share/doc/fdo - The subpackages which install binaries into %{dracutlibdir} need to either have "Requires: dracut" or co-own the first two directories. - The subpackages which install binaries into "%{_libexecdir}/fdo/foo" should also own "%dir %{_libexecdir}/fdo". - The fdo-init subpackage should own "%dir %{dracutlibdir}/modules.d/52fdo". - All packages that contain files under "%{_docdir}/fdo" should co-own "%dir %{_docdir}/fdo". ====================================================================== 2. No debuginfo subpackages / unstripped binaries: fdo-admin-cli.x86_64: W: unstripped-binary-or-object /usr/bin/fdo-admin-tool fdo-client.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-client-linuxapp fdo-init.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-manufacturing-client fdo-manufacturing-server.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-manufacturing-server fdo-owner-cli.x86_64: W: unstripped-binary-or-object /usr/bin/fdo-owner-tool fdo-owner-onboarding-server.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-owner-onboarding-server fdo-owner-onboarding-server.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-serviceinfo-api-server fdo-rendezvous-server.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-rendezvous-server Removing "%global debug_package %nil" should resolve this. ====================================================================== 3. Source0 URL invalid. Use "%{url}/archive/v%{version}/%{name}-rs-%{version}.tar.gz" instead. ====================================================================== 4. Unversioned cross-subpackage dependencies For example: Requires: fdo-manufacturing-server Requires: fdo-init Requires: fdo-client Requires: fdo-rendezvous-server Requires: fdo-owner-onboarding-server Requires: fdo-owner-cli Should these have "= %{version}-%{release}" restriction, or is having mismatched versions of these packages fine? ====================================================================== 5. Latest version is 0.4.10, packaged version is 0.4.9. This can be updated at a later point. ====================================================================== 6. The package contains unit tests, but they are not run. Not sure if they can work in containerized environment, but it would be good to check whether adding %if %{with check} %check %cargo_test %endif After the %install scriptlet would work or not. If the tests cannot be run, then that's fine, but it would be good to have a comment about that in the spec file. ====================================================================== 7. Please use %global dracutlibdir %{_prefix}/lib/dracut" instead of "%define dracutlibdir %{_prefix}/lib/dracut". ====================================================================== 8. rpmlint complains about crypto policy non-compliance in multiple binaries: fdo-admin-cli.x86_64: W: crypto-policy-non-compliance-openssl /usr/bin/fdo-admin-tool SSL_CTX_set_cipher_list fdo-client.x86_64: W: crypto-policy-non-compliance-openssl /usr/libexec/fdo/fdo-client-linuxapp SSL_CTX_set_cipher_list fdo-init.x86_64: W: crypto-policy-non-compliance-openssl /usr/libexec/fdo/fdo-manufacturing-client SSL_CTX_set_cipher_list fdo-owner-onboarding-server.x86_64: W: crypto-policy-non-compliance-openssl /usr/libexec/fdo/fdo-owner-onboarding-server SSL_CTX_set_cipher_list Not sure if this is a problem that needs to be investigated. ====================================================================== 9. Unused library dependencies? fdo-admin-cli.x86_64: W: unused-direct-shlib-dependency /usr/bin/fdo-admin-tool /lib64/libtss2-esys.so.0 fdo-admin-cli.x86_64: W: unused-direct-shlib-dependency /usr/bin/fdo-admin-tool /lib64/libtss2-tctildr.so.0 fdo-admin-cli.x86_64: W: unused-direct-shlib-dependency /usr/bin/fdo-admin-tool /lib64/libtss2-mu.so.0 fdo-rendezvous-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-rendezvous-server /lib64/libtss2-esys.so.0 fdo-rendezvous-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-rendezvous-server /lib64/libtss2-tctildr.so.0 fdo-rendezvous-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-rendezvous-server /lib64/libtss2-mu.so.0 fdo-manufacturing-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-manufacturing-server /lib64/libtss2-esys.so.0 fdo-manufacturing-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-manufacturing-server /lib64/libtss2-tctildr.so.0 fdo-manufacturing-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-manufacturing-server /lib64/libtss2-mu.so.0 fdo-owner-onboarding-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-owner-onboarding-server /lib64/libtss2-esys.so.0 fdo-owner-onboarding-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-owner-onboarding-server /lib64/libtss2-tctildr.so.0 fdo-owner-onboarding-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-owner-onboarding-server /lib64/libtss2-mu.so.0 fdo-owner-onboarding-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-serviceinfo-api-server /lib64/libtss2-esys.so.0 fdo-owner-onboarding-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-serviceinfo-api-server /lib64/libtss2-tctildr.so.0 fdo-owner-onboarding-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-serviceinfo-api-server /lib64/libtss2-mu.so.0 fdo-owner-cli.x86_64: W: unused-direct-shlib-dependency /usr/bin/fdo-owner-tool /lib64/libtss2-esys.so.0 fdo-owner-cli.x86_64: W: unused-direct-shlib-dependency /usr/bin/fdo-owner-tool /lib64/libtss2-tctildr.so.0 Not sure what these are about. ====================================================================== Full fedora-review checklist included below. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== 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. [x]: License file installed when any subpackage combination is installed. [!]: Package requires other packages for directories it uses. [!]: Package must own all directories that it creates. Note: Directories without known owners: /usr/lib/dracut/modules.d/52fdo, /usr/libexec/fdo, /usr/lib/dracut/modules.d, /usr/share/doc/fdo, /usr/lib/dracut [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. [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. [!]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of 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: 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 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]: systemd_post is invoked in %post, systemd_preun in %preun, and systemd_postun in %postun for Systemd service files. Note: Systemd service file(s) in fdo-owner-onboarding-server, fdo- rendezvous-server, fdo-manufacturing-server, fdo-client, fdo-admin-cli [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [!]: Sources can be downloaded from URI in Source: tag [-]: 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). [?]: Fully versioned dependency in subpackages if applicable. [?]: Package functions as described. [!]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [x]: Scriptlets must be sane, if used. [x]: SourceX tarball generation or download is documented. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. [ ]: 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. [!]: Spec use %global instead of %define unless justified. [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]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: SourceX is a working URL. ===== EXTRA items ===== Generic: [x]: Spec file according to URL is the same as in SRPM. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. Rpmlint ------- Checking: fdo-init-0.4.9-4.fc39.x86_64.rpm fdo-owner-onboarding-server-0.4.9-4.fc39.x86_64.rpm fdo-rendezvous-server-0.4.9-4.fc39.x86_64.rpm fdo-manufacturing-server-0.4.9-4.fc39.x86_64.rpm fdo-client-0.4.9-4.fc39.x86_64.rpm fdo-owner-cli-0.4.9-4.fc39.x86_64.rpm fdo-admin-cli-0.4.9-4.fc39.x86_64.rpm fido-device-onboard-0.4.9-4.fc39.src.rpm ============================ rpmlint session starts ============================ rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml rpmlintrc: [PosixPath('/tmp/tmp7amwp7gx')] checks: 31, packages: 8 fdo-admin-cli.x86_64: W: unstripped-binary-or-object /usr/bin/fdo-admin-tool fdo-client.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-client-linuxapp fdo-init.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-manufacturing-client fdo-manufacturing-server.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-manufacturing-server fdo-owner-cli.x86_64: W: unstripped-binary-or-object /usr/bin/fdo-owner-tool fdo-owner-onboarding-server.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-owner-onboarding-server fdo-owner-onboarding-server.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-serviceinfo-api-server fdo-rendezvous-server.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-rendezvous-server fdo-init.x86_64: W: summary-not-capitalized dracut module for device initialization fdo-admin-cli.x86_64: W: no-manual-page-for-binary fdo-admin-tool fdo-owner-cli.x86_64: W: no-manual-page-for-binary fdo-owner-tool fdo-admin-cli.x86_64: W: no-documentation fdo-client.x86_64: W: no-documentation fdo-init.x86_64: W: no-documentation fdo-owner-cli.x86_64: W: no-documentation fdo-admin-cli.x86_64: W: crypto-policy-non-compliance-openssl /usr/bin/fdo-admin-tool SSL_CTX_set_cipher_list fdo-client.x86_64: W: crypto-policy-non-compliance-openssl /usr/libexec/fdo/fdo-client-linuxapp SSL_CTX_set_cipher_list fdo-init.x86_64: W: crypto-policy-non-compliance-openssl /usr/libexec/fdo/fdo-manufacturing-client SSL_CTX_set_cipher_list fdo-owner-onboarding-server.x86_64: W: crypto-policy-non-compliance-openssl /usr/libexec/fdo/fdo-owner-onboarding-server SSL_CTX_set_cipher_list 8 packages and 0 specfiles checked; 0 errors, 61 warnings, 0 badness; has taken 1.1 s Rpmlint (installed packages) ---------------------------- /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) ============================ rpmlint session starts ============================ rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 31, packages: 7 fdo-admin-cli.x86_64: W: unused-direct-shlib-dependency /usr/bin/fdo-admin-tool /lib64/libtss2-esys.so.0 fdo-admin-cli.x86_64: W: unused-direct-shlib-dependency /usr/bin/fdo-admin-tool /lib64/libtss2-tctildr.so.0 fdo-admin-cli.x86_64: W: unused-direct-shlib-dependency /usr/bin/fdo-admin-tool /lib64/libtss2-mu.so.0 fdo-rendezvous-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-rendezvous-server /lib64/libtss2-esys.so.0 fdo-rendezvous-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-rendezvous-server /lib64/libtss2-tctildr.so.0 fdo-rendezvous-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-rendezvous-server /lib64/libtss2-mu.so.0 fdo-manufacturing-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-manufacturing-server /lib64/libtss2-esys.so.0 fdo-manufacturing-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-manufacturing-server /lib64/libtss2-tctildr.so.0 fdo-manufacturing-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-manufacturing-server /lib64/libtss2-mu.so.0 fdo-owner-onboarding-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-owner-onboarding-server /lib64/libtss2-esys.so.0 fdo-owner-onboarding-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-owner-onboarding-server /lib64/libtss2-tctildr.so.0 fdo-owner-onboarding-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-owner-onboarding-server /lib64/libtss2-mu.so.0 fdo-owner-onboarding-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-serviceinfo-api-server /lib64/libtss2-esys.so.0 fdo-owner-onboarding-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-serviceinfo-api-server /lib64/libtss2-tctildr.so.0 fdo-owner-onboarding-server.x86_64: W: unused-direct-shlib-dependency /usr/libexec/fdo/fdo-serviceinfo-api-server /lib64/libtss2-mu.so.0 fdo-owner-cli.x86_64: W: unused-direct-shlib-dependency /usr/bin/fdo-owner-tool /lib64/libtss2-esys.so.0 fdo-owner-cli.x86_64: W: unused-direct-shlib-dependency /usr/bin/fdo-owner-tool /lib64/libtss2-tctildr.so.0 fdo-admin-cli.x86_64: W: unstripped-binary-or-object /usr/bin/fdo-admin-tool fdo-rendezvous-server.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-rendezvous-server fdo-client.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-client-linuxapp fdo-manufacturing-server.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-manufacturing-server fdo-owner-onboarding-server.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-owner-onboarding-server fdo-owner-onboarding-server.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-serviceinfo-api-server fdo-init.x86_64: W: unstripped-binary-or-object /usr/libexec/fdo/fdo-manufacturing-client fdo-owner-cli.x86_64: W: unstripped-binary-or-object /usr/bin/fdo-owner-tool fdo-init.x86_64: W: summary-not-capitalized dracut module for device initialization fdo-admin-cli.x86_64: W: no-manual-page-for-binary fdo-admin-tool fdo-owner-cli.x86_64: W: no-manual-page-for-binary fdo-owner-tool fdo-admin-cli.x86_64: W: no-documentation fdo-client.x86_64: W: no-documentation fdo-init.x86_64: W: no-documentation fdo-owner-cli.x86_64: W: no-documentation fdo-admin-cli.x86_64: W: crypto-policy-non-compliance-openssl /usr/bin/fdo-admin-tool SSL_CTX_set_cipher_list fdo-client.x86_64: W: crypto-policy-non-compliance-openssl /usr/libexec/fdo/fdo-client-linuxapp SSL_CTX_set_cipher_list fdo-owner-onboarding-server.x86_64: W: crypto-policy-non-compliance-openssl /usr/libexec/fdo/fdo-owner-onboarding-server SSL_CTX_set_cipher_list fdo-init.x86_64: W: crypto-policy-non-compliance-openssl /usr/libexec/fdo/fdo-manufacturing-client SSL_CTX_set_cipher_list 7 packages and 0 specfiles checked; 0 errors, 78 warnings, 0 badness; has taken 0.8 s Requires -------- fdo-init (rpmlib, GLIBC filtered): /usr/bin/bash ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libcrypto.so.3()(64bit) libcrypto.so.3(OPENSSL_3.0.0)(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libgcc_s.so.1(GCC_4.2.0)(64bit) libm.so.6()(64bit) libssl.so.3()(64bit) libssl.so.3(OPENSSL_3.0.0)(64bit) libtss2-esys.so.0()(64bit) libtss2-mu.so.0()(64bit) libtss2-tctildr.so.0()(64bit) openssl-libs rtld(GNU_HASH) fdo-owner-onboarding-server (rpmlib, GLIBC filtered): /bin/sh ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libcrypto.so.3()(64bit) libcrypto.so.3(OPENSSL_3.0.0)(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libgcc_s.so.1(GCC_4.2.0)(64bit) libm.so.6()(64bit) libssl.so.3()(64bit) libssl.so.3(OPENSSL_3.0.0)(64bit) libtss2-esys.so.0()(64bit) libtss2-mu.so.0()(64bit) libtss2-tctildr.so.0()(64bit) openssl-libs rtld(GNU_HASH) fdo-rendezvous-server (rpmlib, GLIBC filtered): /bin/sh ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libcrypto.so.3()(64bit) libcrypto.so.3(OPENSSL_3.0.0)(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libgcc_s.so.1(GCC_4.2.0)(64bit) libm.so.6()(64bit) libssl.so.3()(64bit) libssl.so.3(OPENSSL_3.0.0)(64bit) libtss2-esys.so.0()(64bit) libtss2-mu.so.0()(64bit) libtss2-tctildr.so.0()(64bit) rtld(GNU_HASH) fdo-manufacturing-server (rpmlib, GLIBC filtered): /bin/sh ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libcrypto.so.3()(64bit) libcrypto.so.3(OPENSSL_3.0.0)(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libgcc_s.so.1(GCC_4.2.0)(64bit) libm.so.6()(64bit) libssl.so.3()(64bit) libssl.so.3(OPENSSL_3.0.0)(64bit) libtss2-esys.so.0()(64bit) libtss2-mu.so.0()(64bit) libtss2-tctildr.so.0()(64bit) openssl-libs rtld(GNU_HASH) fdo-client (rpmlib, GLIBC filtered): /bin/sh clevis clevis-luks clevis-pin-tpm2 cryptsetup ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libcrypto.so.3()(64bit) libcrypto.so.3(OPENSSL_3.0.0)(64bit) libcryptsetup.so.12()(64bit) libcryptsetup.so.12(CRYPTSETUP_2.0)(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libgcc_s.so.1(GCC_4.2.0)(64bit) libm.so.6()(64bit) libssl.so.3()(64bit) libssl.so.3(OPENSSL_3.0.0)(64bit) libtss2-esys.so.0()(64bit) libtss2-mu.so.0()(64bit) libtss2-tctildr.so.0()(64bit) openssl-libs rtld(GNU_HASH) fdo-owner-cli (rpmlib, GLIBC filtered): ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libcrypto.so.3()(64bit) libcrypto.so.3(OPENSSL_3.0.0)(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libgcc_s.so.1(GCC_4.2.0)(64bit) libm.so.6()(64bit) libssl.so.3()(64bit) libssl.so.3(OPENSSL_3.0.0)(64bit) libtss2-esys.so.0()(64bit) libtss2-mu.so.0()(64bit) libtss2-tctildr.so.0()(64bit) rtld(GNU_HASH) fdo-admin-cli (rpmlib, GLIBC filtered): /bin/sh fdo-client fdo-init fdo-manufacturing-server fdo-owner-cli fdo-owner-onboarding-server fdo-rendezvous-server ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libcrypto.so.3()(64bit) libcrypto.so.3(OPENSSL_3.0.0)(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libgcc_s.so.1(GCC_4.2.0)(64bit) libm.so.6()(64bit) libssl.so.3()(64bit) libssl.so.3(OPENSSL_3.0.0)(64bit) libtss2-esys.so.0()(64bit) libtss2-mu.so.0()(64bit) libtss2-tctildr.so.0()(64bit) rtld(GNU_HASH) Provides -------- fdo-init: fdo-init fdo-init(x86-64) fdo-owner-onboarding-server: fdo-owner-onboarding-server fdo-owner-onboarding-server(x86-64) fdo-rendezvous-server: fdo-rendezvous-server fdo-rendezvous-server(x86-64) fdo-manufacturing-server: fdo-manufacturing-server fdo-manufacturing-server(x86-64) fdo-client: fdo-client fdo-client(x86-64) fdo-owner-cli: fdo-owner-cli fdo-owner-cli(x86-64) fdo-admin-cli: fdo-admin-cli fdo-admin-cli(x86-64)
Small correction: > [x]: Package should compile and build into binary rpms on all supported > architectures. The scratch build for rawhide was successful: https://koji.fedoraproject.org/koji/taskinfo?taskID=102141567
> 5. Latest version is 0.4.10, packaged version is 0.4.9. This can be updated > at a later point. This is awaiting updates to rust dependencies in Fedora :-/
(In reply to Peter Robinson from comment #43) > > 5. Latest version is 0.4.10, packaged version is 0.4.9. This can be updated > > at a later point. > > This is awaiting updates to rust dependencies in Fedora :-/ If you tell me which ones are currently too old, I can prioritize them. Alternatively, commenting on the bugzillas for the new versions will make them bubble to the top of my todo list.
> positives) for points 4, 8, and 9 before I approve the package. > The others can be fixed post-approval / pre-import. > > ====================================================================== > > 1. Unowned directories: > All fixed > ====================================================================== > > 2. No debuginfo subpackages / unstripped binaries: > > Removing "%global debug_package %nil" should resolve this. Fixed. > ====================================================================== > > 3. Source0 URL invalid. Use > "%{url}/archive/v%{version}/%{name}-rs-%{version}.tar.gz" instead. Fixed > ====================================================================== > > 4. Unversioned cross-subpackage dependencies They are actually independent, for the fdo-admin-cli which is the only one with the deps it likely makes sense to enforce this so I've added it. > ====================================================================== > > 5. Latest version is 0.4.10, packaged version is 0.4.9. This can be updated > at a later point. This is due to some deps awaiting bump in Fedora, the upstream is only minor updates so it's not a priority and can come when those deps happen. > ====================================================================== > > 6. The package contains unit tests, but they are not run. Not sure if they > can work in containerized environment, but it would be good to check whether > adding > > %if %{with check} > %check > %cargo_test > %endif > > After the %install scriptlet would work or not. If the tests cannot be run, > then that's fine, but it would be good to have a comment about that in the > spec file. I think we'd need to be able to run up services, I'll work with the team on this. I've added it for completeness but they're not enabled. > ====================================================================== > > 7. Please use %global dracutlibdir %{_prefix}/lib/dracut" instead of > "%define dracutlibdir %{_prefix}/lib/dracut". Updated. > ====================================================================== > > 8. rpmlint complains about crypto policy non-compliance in multiple binaries: > > fdo-admin-cli.x86_64: W: crypto-policy-non-compliance-openssl > /usr/bin/fdo-admin-tool SSL_CTX_set_cipher_list > fdo-client.x86_64: W: crypto-policy-non-compliance-openssl > /usr/libexec/fdo/fdo-client-linuxapp SSL_CTX_set_cipher_list > fdo-init.x86_64: W: crypto-policy-non-compliance-openssl > /usr/libexec/fdo/fdo-manufacturing-client SSL_CTX_set_cipher_list > fdo-owner-onboarding-server.x86_64: W: crypto-policy-non-compliance-openssl > /usr/libexec/fdo/fdo-owner-onboarding-server SSL_CTX_set_cipher_list > > Not sure if this is a problem that needs to be investigated. I can add it as an internal option but the protocol is extremely specific on it's crypto, we've had to get openssl updates upstream to deal with some of this in the past. I would like this to not block the review, it has already been going on long enough. > ====================================================================== > > 9. Unused library dependencies? > > > Not sure what these are about. A few of the FDO components use the TPM2 for storing/retrieving things, maybe the rust linker is linking this into all the compoents even if individual components (eg the services) don't actually use the TPM2 side of things. > ====================================================================== > > Full fedora-review checklist included below. > > Package Review > ============== > > Legend: > [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated > [ ] = Manual review needed > > > > ===== MUST items ===== > > 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. > [x]: License file installed when any subpackage combination is installed. > [!]: Package requires other packages for directories it uses. > [!]: Package must own all directories that it creates. > Note: Directories without known owners: > /usr/lib/dracut/modules.d/52fdo, /usr/libexec/fdo, > /usr/lib/dracut/modules.d, /usr/share/doc/fdo, /usr/lib/dracut > [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. > [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. > [!]: Useful -debuginfo package or justification otherwise. > [x]: Package is not known to require an ExcludeArch tag. > [-]: Large documentation must go in a -doc subpackage. Large could be size > (~1MB) or number of 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: 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 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]: systemd_post is invoked in %post, systemd_preun in %preun, and > systemd_postun in %postun for Systemd service files. > Note: Systemd service file(s) in fdo-owner-onboarding-server, fdo- > rendezvous-server, fdo-manufacturing-server, fdo-client, fdo-admin-cli > [x]: File names are valid UTF-8. > [x]: Packages must not store files under /srv, /opt or /usr/local > > ===== SHOULD items ===== > > Generic: > [!]: Sources can be downloaded from URI in Source: tag > [-]: 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). > [?]: Fully versioned dependency in subpackages if applicable. > [?]: Package functions as described. > [!]: Latest version is packaged. > [x]: Package does not include license text files separate from upstream. > [x]: Patches link to upstream bugs/comments/lists or are otherwise > justified. > [x]: Scriptlets must be sane, if used. > [x]: SourceX tarball generation or download is documented. > [-]: Sources are verified with gpgverify first in %prep if upstream > publishes signatures. > [ ]: 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. > [!]: Spec use %global instead of %define unless justified. > [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]: Packager, Vendor, PreReq, Copyright tags should not be in spec file > [x]: SourceX is a working URL. > > ===== EXTRA items ===== > > Generic: > [x]: Spec file according to URL is the same as in SRPM. > [x]: Rpmlint is run on all installed packages. > Note: There are rpmlint messages (see attachment). > [x]: Large data in /usr/share should live in a noarch subpackage if package > is arched. I think those are all fixed now. spec: https://pbrobinson.fedorapeople.org/fido-device-onboard.spec srpm: https://pbrobinson.fedorapeople.org/fido-device-onboard-0.4.9-5.fc38.src.rpm koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=102142841
Copr build: https://copr.fedorainfracloud.org/coprs/build/6081317 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2133551-fido-device-onboard/srpm-builds/06081317/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- 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.
> If you tell me which ones are currently too old, I can prioritize them. > Alternatively, commenting on the bugzillas for the new versions will make > them bubble to the top of my todo list. I'm aware of that, we don't need the new version yet, there will be another new version RSN, I really just want to get this into Fedora so we can unblock other work for the team in Fedora, as long as this isn't available it blocks the entire team working on things with Fedora as the upstream. It was at least pem 2.0 (up from 1.1) (rhbz 2160078) and another major bump I don't remember.
(In reply to Peter Robinson from comment #45) > (snip) > I can add it as an internal option but the protocol is extremely specific on it's crypto, we've had to get openssl updates upstream to deal with some of this in the past. > I would like this to not block the review, it has already been going on long enough. Yes, I agree. "The crypto is really finicky" is clarification enough for me. :) > I think those are all fixed now. > > spec: https://pbrobinson.fedorapeople.org/fido-device-onboard.spec > srpm: > https://pbrobinson.fedorapeople.org/fido-device-onboard-0.4.9-5.fc38.src.rpm > > koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=102142841 It looks like the upload for these files failed - the spec file is the previous one, and the srpm file URL is HTTP 404?
> It looks like the upload for these files failed - the spec file is the > previous one, and the srpm file URL is HTTP 404? Should be there now: spec: https://pbrobinson.fedorapeople.org/fido-device-onboard.spec srpm: https://pbrobinson.fedorapeople.org/fido-device-onboard-0.4.9-5.fc38.src.rpm koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=102143391
Copr build: https://copr.fedorainfracloud.org/coprs/build/6081415 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2133551-fido-device-onboard/fedora-rawhide-x86_64/06081415-fido-device-onboard/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- 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.
The package fails to build now with missing dependencies (not sure why) ... switching back to "%bcond_without check" and removing the %check section (since you're not going to run tests anyway) should resolve the build failure. Other issues are resolved as far as I can tell, so package is now officially APPROVED. Just fix the aforementioned %check stuff before importing. Thanks for sticking with this!
The Pagure repository was created at https://src.fedoraproject.org/rpms/fido-device-onboard
(In reply to Fabio Valentini from comment #51) > The package fails to build now with missing dependencies (not sure why) ... > switching back to "%bcond_without check" and removing the %check section > (since you're not going to run tests anyway) should resolve the build > failure. If you actually remove the entire "+%bcond_without check" line plus the associated %check pieces it also fails, very strange.
(In reply to Peter Robinson from comment #53) > (In reply to Fabio Valentini from comment #51) > > The package fails to build now with missing dependencies (not sure why) ... > > switching back to "%bcond_without check" and removing the %check section > > (since you're not going to run tests anyway) should resolve the build > > failure. > > If you actually remove the entire "+%bcond_without check" line plus the > associated %check pieces it also fails, very strange. Not strange at all, just maybe a bit unexpected ... the "check" %bcond affects the BuildRequires generator (i.e. %cargo_generate_buildrequires), which is why my suggestion above was very specific: > switching back to "%bcond_without check" and removing the %check section (since you're not going to run tests anyway) should resolve the build failure.
FEDORA-2023-d1097102e9 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-d1097102e9
FEDORA-2023-d1097102e9 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-d1097102e9 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-d1097102e9 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2023-d1097102e9 has been pushed to the Fedora 38 stable repository. If problem still persists, please make note of it in this bug report.