Bug 1953508 - Review Request: fwupd-efi - Firmware update EFI binaries
Summary: Review Request: fwupd-efi - Firmware update EFI binaries
Keywords:
Status: MODIFIED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Neal Gompa
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-04-26 09:26 UTC by Richard Hughes
Modified: 2021-05-07 09:58 UTC (History)
2 users (show)

Fixed In Version: fwupd-efi-1.0-1.fc35
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
ngompa13: fedora-review+


Attachments (Terms of Use)

Description Richard Hughes 2021-04-26 09:26:34 UTC
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.

Comment 1 Neal Gompa 2021-04-26 14:38:09 UTC
Taking this review.

Comment 2 Neal Gompa 2021-04-27 01:00:10 UTC
>     -Defi_sbat_distro_version="%{version}" \

I think this should probably be "%{version}-%{release}" instead, since it would matter when you patch it.

Comment 3 Richard Hughes 2021-04-27 10:00:59 UTC
> 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.

Comment 4 Neal Gompa 2021-04-27 11:12:03 UTC
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...

Comment 5 Richard Hughes 2021-04-27 13:06:29 UTC
> Is there a reason we aren't going to have the DistTag on this?

Err, no, unintentional! Fixed.

Comment 6 Neal Gompa 2021-04-27 22:04:42 UTC
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.

Comment 7 Richard Hughes 2021-04-28 09:25:23 UTC
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?

Comment 8 Neal Gompa 2021-04-28 09:48:03 UTC
(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...

Comment 9 Richard Hughes 2021-04-28 11:45:57 UTC
> 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 :)

Comment 10 Gwyn Ciesla 2021-04-28 13:46:13 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/fwupd-efi

Comment 11 Richard Hughes 2021-04-28 14:54:34 UTC
Waiting for https://pagure.io/fedora-infrastructure/issue/9912 and then will build.

Comment 12 Richard Hughes 2021-05-07 09:58:24 UTC
Built as fwupd-efi-1.0-1.fc35, will rebuild when the signing stuff is fixed.


Note You need to log in before you can comment on or make changes to this bug.