https://bugzilla.redhat.com/show_bug.cgi?id=1953508
Bug ID: 1953508 Summary: Review Request: fwupd-efi - Firmware update EFI binaries Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: rhughes@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://people.freedesktop.org/~hughsient/temp/fwupd-efi.spec SRPM URL: https://people.freedesktop.org/~hughsient/temp/fwupd-efi-1.0-1.src.rpm Description:
fwupd is a project to allow updating device firmware, and this package provides the EFI binary that is used for updating using UpdateCapsule. This has been split out of the main fwupd package so it can be released asynchronously from the library and daemon, as additional signing requirements are required.
Fedora Account System Username: rhughes $ rpmlint SPECS/fwupd-efi* SRPMS/fwupd-efi* RPMS/fwupd-efi* fwupd-efi.x86_64: E: no-binary 2 packages and 1 specfiles checked; 1 errors, 0 warnings.
Note: the "binary" in this case is an EFI binary, rather than an ELF binary in the sense that rpmlint is looking for.
https://bugzilla.redhat.com/show_bug.cgi?id=1953508
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |ngompa13@gmail.com Assignee|nobody@fedoraproject.org |ngompa13@gmail.com Flags| |fedora-review?
--- Comment #1 from Neal Gompa ngompa13@gmail.com --- Taking this review.
https://bugzilla.redhat.com/show_bug.cgi?id=1953508
--- Comment #2 from Neal Gompa ngompa13@gmail.com ---
-Defi_sbat_distro_version="%{version}" \
I think this should probably be "%{version}-%{release}" instead, since it would matter when you patch it.
https://bugzilla.redhat.com/show_bug.cgi?id=1953508
Richard Hughes rhughes@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(ngompa13@gmail.co | |m)
--- Comment #3 from Richard Hughes rhughes@redhat.com ---
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.
https://bugzilla.redhat.com/show_bug.cgi?id=1953508
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(ngompa13@gmail.co |needinfo?(rhughes@redhat.co |m) |m)
--- Comment #4 from Neal Gompa ngompa13@gmail.com --- 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...
https://bugzilla.redhat.com/show_bug.cgi?id=1953508
Richard Hughes rhughes@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(rhughes@redhat.co |needinfo?(ngompa13@gmail.co |m) |m)
--- Comment #5 from Richard Hughes rhughes@redhat.com ---
Is there a reason we aren't going to have the DistTag on this?
Err, no, unintentional! Fixed.
https://bugzilla.redhat.com/show_bug.cgi?id=1953508
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ |needinfo?(ngompa13@gmail.co | |m) |
--- Comment #6 from Neal Gompa ngompa13@gmail.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=1953508
--- Comment #7 from Richard Hughes rhughes@redhat.com --- 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?
https://bugzilla.redhat.com/show_bug.cgi?id=1953508
--- Comment #8 from Neal Gompa ngompa13@gmail.com --- (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...
https://bugzilla.redhat.com/show_bug.cgi?id=1953508
--- Comment #9 from Richard Hughes rhughes@redhat.com ---
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 :)
https://bugzilla.redhat.com/show_bug.cgi?id=1953508
--- Comment #10 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/fwupd-efi
https://bugzilla.redhat.com/show_bug.cgi?id=1953508
--- Comment #11 from Richard Hughes rhughes@redhat.com --- Waiting for https://pagure.io/fedora-infrastructure/issue/9912 and then will build.
https://bugzilla.redhat.com/show_bug.cgi?id=1953508
Richard Hughes rhughes@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED Fixed In Version| |fwupd-efi-1.0-1.fc35
--- Comment #12 from Richard Hughes rhughes@redhat.com --- Built as fwupd-efi-1.0-1.fc35, will rebuild when the signing stuff is fixed.
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com