Bug 1878908 - Review Request: rust-bootupd - bootloader updater
Summary: Review Request: rust-bootupd - bootloader updater
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Steve Milner
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2020-09-14 21:07 UTC by Colin Walters
Modified: 2021-01-29 17:03 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-01-29 17:03:14 UTC
Type: ---
Embargoed:
smilner: fedora-review+


Attachments (Terms of Use)

Description Colin Walters 2020-09-14 21:07:30 UTC
Spec URL: https://fedorapeople.org/~walters/rust-bootupd.spec
SRPM URL: https://fedorapeople.org/~walters/rust-bootupd-0.1.0-3.fc32.src.rpm
Description: bootloader updater
Fedora Account System Username: walters

Comment 1 Steve Milner 2020-09-15 20:21:04 UTC
First pass:

Using https://docs.fedoraproject.org/en-US/packaging-guidelines/ReviewGuidelines/

-----
? = not able to verify yet
+ = LGTM
- = Did not pass
-----


? spec rpmlint
Warning makes sense as the create is not released yet.

rust-bootupd.spec: W: invalid-url Source0: https://crates.io/api/v1/crates/bootupd/0.1.0/download#/bootupd-0.1.0.crate HTTP Error 404: Not Found
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

? source rpmlint
rust-bootupd.src: W: spelling-error Summary(en_US) Bootloader -> Boot loader, Boot-loader, Boatload
rust-bootupd.src: W: spelling-error %description -l en_US Bootloader -> Boot loader, Boot-loader, Boatload

"Bootloader" is valid (Eg: GRUB stands for GRand Unified Bootloader).

rust-bootupd.src: W: invalid-url URL: https://crates.io/crates/bootupd HTTP Error 404: Not Found
rust-bootupd.src: W: invalid-url Source0: https://crates.io/api/v1/crates/bootupd/0.1.0/download#/bootupd-0.1.0.crate HTTP Error 404: Not Found

Warning makes sense as the create is not released yet.

? rpm rpmlint

bootupd.x86_64: W: spelling-error Summary(en_US) Bootloader -> Boot loader, Boot-loader, Boatload
bootupd.x86_64: W: spelling-error %description -l en_US Bootloader -> Boot loader, Boot-loader, Boatload

"Bootloader" is valid (Eg: GRUB stands for GRand Unified Bootloader).

bootupd.x86_64: W: invalid-url URL: https://crates.io/crates/bootupd HTTP Error 404: Not Found

Expected as the release has not occured yet.

bootupd.x86_64: W: empty-%postun

Uses a macro which expands to empty

+ Naming
Rust is not called out directly in https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/. However https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/ does denote the use of `rust-` prefix.

+ Package matches spec name
+ License ASL 2.0
+ Code License matches https://github.com/coreos/bootupd/blob/master/LICENSE
+ License included in %license
+ spec is in American English
+ spec is legible
? sources match (not released yet)
+ ExclusiveArch set as required
+ Locales handled properly (none in use)
? Build dependencies listed in BuildRequires and %cargo_generate_buildrequires is in use. This means they are not explicitly listed in the spec since Cargo.toml is looked at.
+ %doc has nothing which is required for the application to run
+ filenames are valid utf-8
+ Does not require deprecated packages
+ System libraries are not bundled
+ Package is not relocatable
+ Package owns it's created directories
+ Permissions are sane

$ rpm -qplv ~/rpmbuild/RPMS/x86_64/bootupd-0.1.0-3.fc32.x86_64.rpm 
drwxr-xr-x    2 root     root                        0 Sep 15 18:48 /usr/lib/.build-id
drwxr-xr-x    2 root     root                        0 Sep 15 18:48 /usr/lib/.build-id/12
lrwxrwxrwx    1 root     root                       31 Sep 15 18:48 /usr/lib/.build-id/12/2d5ba52019605d94ffab884b2f35d10dcd3dd6 -> ../../../../usr/libexec/bootupd
-rw-r--r--    1 root     root                      105 Sep 15 18:48 /usr/lib/systemd/system/bootupd.service
-rw-r--r--    1 root     root                      101 Sep 15 18:48 /usr/lib/systemd/system/bootupd.socket
-rwxr-xr-x    1 root     root                  1311720 Sep 15 18:48 /usr/libexec/bootupd
drwxr-xr-x    2 root     root                        0 Sep 15 18:48 /usr/share/doc/bootupd
-rw-r--r--    1 root     root                     2786 Sep  9 18:57 /usr/share/doc/bootupd/README.md
drwxr-xr-x    2 root     root                        0 Sep 15 18:48 /usr/share/licenses/bootupd
-rw-r--r--    1 root     root                    11358 Jul 18 15:39 /usr/share/licenses/bootupd/LICENSE

+ No files listed more than once in %files
+ Macro use is consistent
+ Package contains code
+ No large docs requiring a -doc sub package
+ Not a static package/binary
+ No devel package produced
+ No .la archives
+ Not a GUI application

SHOULDS:
+ Package builds on x86_64
+ Package results execute as expected
- (Not required) SHOULD: your package should contain man pages



Continuing with https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/

+ Is named rust-%{crate}
+ Uses BuildRequires: rust-packaging
? build-dependencies are not denoted in Cargo.toml, though dependencies are set and are required at build time (https://github.com/coreos/bootupd/blob/master/Cargo.toml#L15-L31)
+ Package explicitly sets arch to x86_64 so %{rust_arches} is not needed
+ %cargo_prep is used
+ crates are versioned and the tool is explicitly written to target Fedora/RHEL based systems

SHOULDS:
+ scriptlets are mainly macros and seem sane
- (not required) SHOULD: your package should contain man pages for binaries/scripts. If it doesn’t, work with upstream to add them where they make sense

Comment 2 Colin Walters 2020-09-15 20:28:43 UTC
I'm not honestly sure I'm going to upload it to the official crates.io - the sources here can be gathered by using `cargo publish --dry-run` from git.

bootupd is much like afterburn in that it's not something you can usefully `cargo install`.  But...I guess since afterburn is there we might as well.  Doing it now.

Comment 3 Steve Milner 2020-09-16 16:54:17 UTC
tl;dr: Some non-required SHOULDs are still outstanding but all MUSTs look covered.

Update:

+ spec rpmlint

rpmlint rust-bootupd.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

+ source rpmlint

rpmlint rust-bootupd-0.1.0-3.fc32.src.rpm 
rust-bootupd.src: W: spelling-error Summary(en_US) Bootloader -> Boot loader, Boot-loader, Boatload
rust-bootupd.src: W: spelling-error %description -l en_US Bootloader -> Boot loader, Boot-loader, Boatload
rust-bootupd.src: W: invalid-url URL: https://crates.io/crates/bootupd HTTP Error 404: Not Found
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

Clicking the link does show it's not a 404.


+ rpm rpmlint

rpmlint /root/rpmbuild/RPMS/x86_64/bootupd-0.1.0-3.fc32.x86_64.rpm
warning: Found bdb Packages database while attempting sqlite backend: using bdb backend.
bootupd.x86_64: W: spelling-error Summary(en_US) Bootloader -> Boot loader, Boot-loader, Boatload
bootupd.x86_64: W: spelling-error %description -l en_US Bootloader -> Boot loader, Boot-loader, Boatload
bootupd.x86_64: W: invalid-url URL: https://crates.io/crates/bootupd HTTP Error 404: Not Found
bootupd.x86_64: W: empty-%postun
1 packages and 0 specfiles checked; 0 errors, 4 warnings.

Clicking the link does show it's not a 404.

+ sources match

$ sha256sum rust-bootupd-0.1.0-3.fc32.src/bootupd-0.1.0.crate
4c90182e11829eae328c2914b50dc2d73cbbeb79e7e3ea559477acf8f1a93926  rust-bootupd-0.1.0-3.fc32.src/bootupd-0.1.0.crate
$ curl -L -O https://crates.io/api/v1/crates/bootupd/0.1.0/download#/bootupd-0.1.0.crate
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100 29649  100 29649    0     0  33201      0 --:--:-- --:--:-- --:--:-- 33201
$ sha256sum bootupd-0.1.0.crate 
4c90182e11829eae328c2914b50dc2d73cbbeb79e7e3ea559477acf8f1a93926  bootupd-0.1.0.crate


+ Build dependencies listed in BuildRequires and %cargo_generate_buildrequires is in use. This means they are not explicitly listed in the spec since Cargo.toml is looked at.

Considering this good because it's following the pattern which is provided by the documentation.

+ build-dependencies are not denoted in Cargo.toml, though dependencies are set and are required at build time (https://github.com/coreos/bootupd/blob/master/Cargo.toml#L15-L31)

Considering this good because the results work as expected by noting the dependencies in Cargo.toml

Comment 5 Gwyn Ciesla 2020-09-16 18:15:38 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-bootupd


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