Bug 1953508
| Summary: | Review Request: fwupd-efi - Firmware update EFI binaries | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Richard Hughes <rhughes> |
| Component: | Package Review | Assignee: | Neal Gompa <ngompa13> |
| Status: | CLOSED CURRENTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | jan.public, ngompa13, package-review |
| Target Milestone: | --- | Flags: | ngompa13:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | fwupd-efi-1.0-1.fc35 | Doc Type: | If docs needed, set a value |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2022-06-20 07:08:54 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Richard Hughes
2021-04-26 09:26:34 UTC
Taking this review. > -Defi_sbat_distro_version="%{version}" \
I think this should probably be "%{version}-%{release}" instead, since it would matter when you patch it.
> I think this should probably be "%{version}-%{release}" instead, since it would matter when you patch it.
Completely agree, good catch! New .spec and srpm uploaded with this fix. Many thanks for the speed review.
I'm going to ignore the goop around the pesign stuff (which just reminds myself that I need to figure out how to make that less stupid...) Is there a reason we aren't going to have the DistTag on this? This doesn't seem to have a Microsoft signing requirement... > Is there a reason we aren't going to have the DistTag on this?
Err, no, unintentional! Fixed.
Review notes: * Package follows Fedora Packaging Guidelines * Package licensing is correct and license files are correctly installs * Package builds and installs with no serious issues from rpmlint There was one issue though: * Missing gcc and ninja-build BRs: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ However, I'm okay with you fixing this on import. PACKAGE APPROVED. Thanks Neal, much appreciated.
> * Missing gcc and ninja-build BRs
Totally agree on gcc, my bad, fixed. Ninja is a hard requirement of meson -- so it is required in this package too?
(In reply to Richard Hughes from comment #7) > Thanks Neal, much appreciated. > > > * Missing gcc and ninja-build BRs > > Totally agree on gcc, my bad, fixed. Ninja is a hard requirement of meson -- > so it is required in this package too? I guess not, then. But it's a bit weird, since Meson is a generator program that supports different build file formats... > But it's a bit weird, since Meson is a generator program that supports different build file formats
True; but on the flip side fwupd doesn't actually require ninja; we just call %meson_build -- if meson switched to Makefiles rather than ninja.build I don't think fwupd would mind at all :)
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/fwupd-efi Waiting for https://pagure.io/fedora-infrastructure/issue/9912 and then will build. Built as fwupd-efi-1.0-1.fc35, will rebuild when the signing stuff is fixed. Package is available in repositories, closing. |