Spec URL: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/08909843-rust-virt-firmware/rust-virt-firmware.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/08909843-rust-virt-firmware/rust-virt-firmware-0.20250411-1.fc43.src.rpm Description: virt firmware rust tools and efi apps Fedora Account System Username: kraxel
*** Bug 2359079 has been marked as a duplicate of this bug. ***
*** Bug 2359081 has been marked as a duplicate of this bug. ***
*** Bug 2359085 has been marked as a duplicate of this bug. ***
*** Bug 2359088 has been marked as a duplicate of this bug. ***
Copr build: https://copr.fedorainfracloud.org/coprs/build/8909995 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2360119-rust-virt-firmware/fedora-rawhide-x86_64/08909995-rust-virt-firmware/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.
Thanks - this looks more straightforward than half a dozen separate packages. Some quick notes before dependencies are ready: - Since you're no longer building from the crates on crates.io, you *must* drop the "rust-" prefix for the package name: https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/#_package_naming_3 - Please use the documented patterns for Version / Source URL / %autosetup etc. for packaging git snapshots: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_commit_revision https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snapshots - Don't glob-own *everything* in %{_bindir}. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_lists - "%{_datadir}/%{name}/%{efiarch}/*.efi" causes the folders to be unowned. You either need to own "%{_datadir}/%{name}/" entirely, or list both *.efi files *and* the directory structure as %dir entries separately. https://docs.fedoraproject.org/en-US/packaging-guidelines/UnownedDirectories/
spec url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/08952149-virt-firmware-rs/virt-firmware-rs.spec srpm url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/08952149-virt-firmware-rs/virt-firmware-rs-0.20250411-1.fc43.src.rpm first round of changes, sorting the versioning TBD still
Copr build: https://copr.fedorainfracloud.org/coprs/build/8952240 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2360119-virt-firmware-rs/fedora-rawhide-x86_64/08952240-virt-firmware-rs/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.
spec url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/08954316-virt-firmware-rs/virt-firmware-rs.spec srpm url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/08954316-virt-firmware-rs/virt-firmware-rs-25.4-1.fc43.src.rpm
Created attachment 2086656 [details] The .spec file difference from Copr build 8952240 to 8954894
Copr build: https://copr.fedorainfracloud.org/coprs/build/8954894 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2360119-virt-firmware-rs/fedora-rawhide-x86_64/08954894-virt-firmware-rs/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.
spec url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/08988999-virt-firmware-rs/virt-firmware-rs.spec srpm url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/08988999-virt-firmware-rs/virt-firmware-rs-25.4-1.fc43.src.rpm * enabled igvm-tools, now that igvm landed in rawhide. * made efi-apps build optional (will be needed anyway, rustc supports uefi target on some archs only). * turned off efi-apps everywhere while rust-uefi is waiting in package review still.
Created attachment 2088095 [details] The .spec file difference from Copr build 8954894 to 8989036
Copr build: https://copr.fedorainfracloud.org/coprs/build/8989036 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2360119-virt-firmware-rs/fedora-rawhide-x86_64/08989036-virt-firmware-rs/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ 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://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/09104441-virt-firmware-rs/virt-firmware-rs.spec srpm url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/09104441-virt-firmware-rs/virt-firmware-rs-25.5.1-1.fc43.src.rpm changes: - update to v25.5.1 - turn on efi app builds for x86_64 and aarch64
Created attachment 2092242 [details] The .spec file difference from Copr build 8989036 to 9104805
Copr build: https://copr.fedorainfracloud.org/coprs/build/9104805 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2360119-virt-firmware-rs/fedora-rawhide-x86_64/09104805-virt-firmware-rs/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ 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.
Thanks for the updates - I re-ran local checks now that rust-uefi is available too. 1. There are some warnings from rpmlint that are actually actionable: a) W: summary-not-capitalized virt firmware rust tools and efi apps Would be great to have something better here, maybe "Tools for EFI and virtual machine firmware"? Adding "written in Rust" is mostly just noise when it's either irrelevant for the user or already indicated by the package name (here: -rs suffix). Same goes for the %description which is very bare-bones and just a copy of the Summary at the moment. b) E: spelling-error ('efi', 'Summary(en_US) efi -> fie, eff') Maybe capitalize this as EFI consistently? c) W: no-documentation There are at least README files for the whole project and some of the subprojects that might be nice to include. d) W: unused-direct-shlib-dependency {/usr/bin/generate-boot-csv,/usr/bin/uefi-boot-menu) /lib64/libudev.so.1 Not sure why this is happening - do those subprojects have a dependency on the udev bindings but don't actually use them? But this might be a harmless warning in either case. 2. There is mixed use of %define and %global in the package. Current guidance is that you should use %global unless you *know* that you need %define. I would also rewrite the build_efi_apps conditional as ``` %ifarch x86_64 aarch64 %bcond build_efi_apps 1 %else %bcond build_efi_apps 0 %endif ``` Or something similar to avoid redefinition. Alternatively, now that all the dependencies needed are present, why not drop the conditionals for "do not build_efi_apps" entirely? 3. Please add some comments around what this is for / why it is there: ``` sed -i Cargo.toml -e '/varstore/d' %if !%{build_efi_apps} sed -i Cargo.toml -e '/efi-apps/d' %endif ``` 4. You generate the LICENSE.dependencies file in %build but then don't install it. Add "%license LICENSE.dependencies" to the %files list. 5. The License tag is MIT, but this should be the content of the "SourceLicense" tag, if anything. The License tag needs to reflect statically linked dependencies too - you can consult the output of the %cargo_license_summary macro from the build.log for a handy summary. 5. It's a bit strange to see architecture-dependent executables getting installed under /usr/share ? It might be OK to do this for EFI executables since they're not meant to be executed on the host OS (I assume), but it's still ... weird. Just making sure this is intentional. Otherwise, the package looks pretty good!
Oh, forgot one thing: > %global debug_package %{nil} This is wrong and should not be necessary. If it *is* necessary to build the package, then something else is going wrong.
> d) W: unused-direct-shlib-dependency > {/usr/bin/generate-boot-csv,/usr/bin/uefi-boot-menu) /lib64/libudev.so.1 > > Not sure why this is happening - do those subprojects have a dependency on > the udev bindings but don't actually use them? > But this might be a harmless warning in either case. libefi has a module which uses udev. Both binaries use the library but not the module. Apparently rust links in libudev nevertheless. > Alternatively, now that all the dependencies needed are present, why not > drop the conditionals for "do not build_efi_apps" entirely? It doesn't build on ppc and s390 ... > 5. It's a bit strange to see architecture-dependent executables getting > installed under /usr/share ? > It might be OK to do this for EFI executables since they're not meant to be > executed on the host OS (I assume), but it's still ... weird. Just making > sure this is intentional. It's common practice for firmware. edk2-ovmf.rpm places firmware images and efi binaries in /usr/share/edk2/ovmf too. I've also moved the efi binaries to a noarch sub-rpm, to allow installing it on non-native archs and run the efi apps in qemu (again following common packaging practice for firmware). spec url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/09121431-virt-firmware-rs/virt-firmware-rs.spec srpm url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/09121431-virt-firmware-rs/virt-firmware-rs-25.5.1-1.fc43.src.rpm
Created attachment 2092802 [details] The .spec file difference from Copr build 9104805 to 9121585
Copr build: https://copr.fedorainfracloud.org/coprs/build/9121585 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2360119-virt-firmware-rs/fedora-rawhide-x86_64/09121585-virt-firmware-rs/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ 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://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/09155757-virt-firmware-rs/virt-firmware-rs.spec srpm url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/09155757-virt-firmware-rs/virt-firmware-rs-25.6-1.fc43.src.rpm update to version 25.6
Created attachment 2093770 [details] The .spec file difference from Copr build 9121585 to 9156411
Copr build: https://copr.fedorainfracloud.org/coprs/build/9156411 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2360119-virt-firmware-rs/fedora-rawhide-x86_64/09156411-virt-firmware-rs/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ 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.
Thanks for the update - some more comments: 1) The Summary and %description are nice now, there's just a typo in the Summary ("firmwarea") Additionally, it's a bit jarring that the definition of the %{name}-%{efiarch} subpackage happens between metadata for the "main" package and the definition of the %descripion for the "main" package. I would suggest to move the subpackage definition *below* the %description for the "main" package for clarity (even if that might mean having the "%if %{with efi_apps}" conditional in two places). 2) You generate the list of statically linked dependencies with "%{cargo_license} > LICENSE.dependencies" but then don't use that file anywhere. You'll need to include it as "%license LICENSE.dependencies" in both %files lists. 3) The statically linked dependencies need to be reflected in the License tag as well. I would suggest moving "MIT" to the "SourceLicense" tag, and use the output of %cargo_license_summary for populating the License tag (the "%shrink" macro can help so you don't end up with a really long line that is hard to maintain and keep up-to-date): ``` SourceLicense: MIT # Apache-2.0 # Apache-2.0 OR BSL-1.0 # Apache-2.0 OR MIT # Apache-2.0 WITH LLVM-exception OR Apache-2.0 OR MIT # BSD-2-Clause OR Apache-2.0 OR MIT # MIT # MIT OR Apache-2.0 # MPL-2.0 # Unlicense OR MIT License: %{shrink: MIT AND Apache-2.0 AND MPL-2.0 AND (Apache-2.0 OR BSL-1.0) AND (Apache-2.0 OR MIT) AND (Apache-2.0 WITH LLVM-exception OR Apache-2.0 OR MIT) AND (BSD-2-Clause OR Apache-2.0 OR MIT) AND (Unlicense OR MIT) } ``` PS: > libefi has a module which uses udev. Both binaries use the library > but not the module. Apparently rust links in libudev nevertheless. I don't understand the distinction between "module" and "library" here. But if the Rust crate uses pkg-config to probe for libudev, then that gets an "-ludev" argument unconditionally from the pkg-config file from systemd-devel.
> Additionally, it's a bit jarring that the definition of the > %{name}-%{efiarch} subpackage happens between metadata for the "main" > package and the definition of the %descripion for the "main" package. I > would suggest to move the subpackage definition *below* the %description for > the "main" package for clarity (even if that might mean having the "%if > %{with efi_apps}" conditional in two places). rpm is a bit picky where it allows %description being placed. Placing it before the sub-rpm definition did not work for me. Placing it before the sub-rpm %description worked, and yes, that needs the condition twice. Phew. Took me a while to make that build. > > libefi has a module which uses udev. Both binaries use the library > > but not the module. Apparently rust links in libudev nevertheless. > > I don't understand the distinction between "module" and "library" here. As defined by rust. 'library' is lib.rs and everything included by it. 'module' is one 'pub mod $name;'. Specifically the efifile.rs module uses udev, the other modules in the library do not. > But > if the Rust crate uses pkg-config to probe for libudev, then that gets an > "-ludev" argument unconditionally from the pkg-config file from > systemd-devel. spec url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/09203447-virt-firmware-rs/virt-firmware-rs.spec srpm url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/09203447-virt-firmware-rs/virt-firmware-rs-25.6-1.fc43.src.rpm
> rpm is a bit picky where it allows %description being placed. The order of things you have now is even weirder than what was there before :D But whatever works for you ... (The *only* thing you need to be careful about with %description is that there are no Tags after it, because they would be subsumed into the description text, so RPM isn't "picky", it just needs to known how to parse the .spec file.) > As defined by rust. 'library' is lib.rs and everything included by it. > 'module' is one 'pub mod $name;'. Specifically the efifile.rs module > uses udev, the other modules in the library do not. It doesn't make sense to distinguish between modules. There is only one unit for compilation and linking - and that's *the whole crate*. "Modules" only exist on the source code level / file system, but they are *not* separate compilation units. So if the *crate* links a library, *all* targets from that crate link it, there is no granularity here (how would cargo even know?). If an executable target *shouldn't* link with a library (because it doesn't need it), it needs to be moved to a separate crate that doesn't have this dependency. === I am just confused by the "alternatives" stuff you added in the most recent version of the package. Is using alternatives really necessary? They're an old and creaky system that doesn't even work on Atomic systems. It also doesn't look as if it confirms to the guidelines here: https://docs.fedoraproject.org/en-US/packaging-guidelines/Alternatives/ Do these two packages really need to be parallel-installable with runtime selection between the two? Or would making uefi-boot-rs available as ... uefi-boot-rs be enough?
(In reply to Fabio Valentini from comment #28) > (The *only* thing you need to be careful about with %description is that > there are no Tags after it, because they would be subsumed into the > description text, so RPM isn't "picky", it just needs to known how to parse > the .spec file.) Ah. Which is exactly what happened. I had it before the BuildRequires, so that effectively dropped the cargo macros from the build root, leading to strange build failures. > > As defined by rust. 'library' is lib.rs and everything included by it. > > 'module' is one 'pub mod $name;'. Specifically the efifile.rs module > > uses udev, the other modules in the library do not. > > It doesn't make sense to distinguish between modules. There is only one unit > for compilation and linking - and that's *the whole crate*. "Modules" only > exist on the source code level / file system, but they are *not* separate > compilation units. So if the *crate* links a library, *all* targets from > that crate link it, there is no granularity here (how would cargo even > know?). If an executable target *shouldn't* link with a library (because it > doesn't need it), it needs to be moved to a separate crate that doesn't have > this dependency. Problem is the library links udev, so anyone using the library gets the udev dependency even if the functionality actually needing udev is not used ... Maybe I can hide the udev dependeny behind a feature flag, so we can have two variants of the library, with and without udev ... > I am just confused by the "alternatives" stuff you added in the most recent > version of the package. > Is using alternatives really necessary? They're an old and creaky system > that doesn't even work on Atomic systems. Will remove.
spec url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/09210283-virt-firmware-rs/virt-firmware-rs.spec srpm url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/09210283-virt-firmware-rs/virt-firmware-rs-25.6-1.fc43.src.rpm
[fedora-review-service-build]
Created attachment 2096770 [details] The .spec file difference from Copr build 9156411 to 9256831
Copr build: https://copr.fedorainfracloud.org/coprs/build/9256831 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2360119-virt-firmware-rs/fedora-rawhide-x86_64/09256831-virt-firmware-rs/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ 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://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/09266872-virt-firmware-rs/virt-firmware-rs.spec srpm url: https://download.copr.fedorainfracloud.org/results/kraxel/rust.misc/fedora-rawhide-x86_64/09266872-virt-firmware-rs/virt-firmware-rs-25.7-1.fc43.src.rpm update to version 25.7
> Problem is the library links udev, so anyone using the library gets the udev > dependency even if the functionality actually needing udev is not used ... > > Maybe I can hide the udev dependeny behind a feature flag, so we can have > two variants of the library, with and without udev ... You don't need two variants, just specify which executable needs the udev feature and which doesn't. Then building the executables with only the features they need should avoid the over-linking. And I see that you've basically done this in the latest version, and the overlinking warning from rpmlint is gone. Great! === Changes look good to me, some minor things left (that doesn't block the review): 1. The typo in the Summary is back, it's "firmwarea" again. 2. Mixed use of tabs and spaces - please indent lines 22-30 with spaces, not tabs. 3. You wrapped the %check section in an %if %{with check} conditional, but don't actually define this bcond. Please add "%bcond check 1" (or "0" if you can't run tests, and document *why*) at the top of the spec file. I don't see any other remaining issues, so I'll mark the package as APPROVED (but please fix these three before importing the spec file). === ✅ 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 ✅ licenses of statically linked dependencies are correctly taken into account ✅ license file is included with %license in %files ✅ package complies with Rust Packaging Guidelines Package APPROVED.
The Pagure repository was created at https://src.fedoraproject.org/rpms/virt-firmware-rs
> Please add "%bcond check 1" (or "0" if you can't run tests, and document *why*) at the top of the spec file. I didn't write it like that for funsies. Some of the RPM macros from cargo-rpm-macros *rely* on the bcond being named "check". For example, the fact that the %cargo_generate_buildrequires macro currently works with the "run_tests" bcond is purely accidental. So I would recommend to either use a "check" bcond instead, *or* pass "-t" to %cargo_generate_buildrequires to unconditionally include test-only dependencies.
FEDORA-2025-16b3e8af23 (virt-firmware-rs-25.7-4.fc42) has been submitted as an update to Fedora 42. https://bodhi.fedoraproject.org/updates/FEDORA-2025-16b3e8af23
FEDORA-2025-16b3e8af23 has been pushed to the Fedora 42 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2025-16b3e8af23 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2025-16b3e8af23 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2025-16b3e8af23 (virt-firmware-rs-25.7-4.fc42) has been pushed to the Fedora 42 stable repository. If problem still persists, please make note of it in this bug report.