Bug 1998894 - Review Request: rust-automod - Pull in every source file in a directory as a module
Summary: Review Request: rust-automod - Pull in every source file in a directory as a ...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Davide Cavalca
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-08-29 19:46 UTC by Fabio Valentini
Modified: 2021-10-18 20:59 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2021-10-18 20:59:51 UTC
Type: ---
Embargoed:
davide: fedora-review+


Attachments (Terms of Use)

Description Fabio Valentini 2021-08-29 19:46:03 UTC
Spec URL: https://decathorpe.fedorapeople.org/rust-automod.spec
SRPM URL: https://decathorpe.fedorapeople.org/rust-automod-1.0.2-1.fc34.src.rpm

Description:
Pull in every source file in a directory as a module.

Fedora Account System Username: decathorpe

Comment 1 Fabio Valentini 2021-08-29 19:46:05 UTC
This package built on koji:  https://koji.fedoraproject.org/koji/taskinfo?taskID=74752934

Comment 2 Iago Rubio 2021-08-29 21:14:38 UTC
Hi Fabio, not a packager yet, just doing the review.

* rpmlint - no errors, it just issues erroneus warning (about the %autochangelog macro)
* rust2rpm package
* fedora-review - no warnings.
* manual review:
   - Package license is ok (ASL or MIT)
   - License files included in package
   - No bundled libraries
   - Changelog provided by macro, so good format expected.
   - Sources are only permisible code & content
   - No desktop file (crate package)
   - No hardcoded directories
   - Name accoroding Package Name Guidelines
   - It's not a rename. No obsoletes.
   - No Requires but BuildRequire on rust-packaging
   - Correct language on spec file
   - No systemd files needed
   - No excludearch. noarch package
   - No large doc, doc package not required (10240 bytes, 1 file)

Builds on mock (fedora-rawhide-x86_64).

Package looks good so far.

rpmbuild did not rebuilt on my rawhide devel system due to missing macro (crate(proc-macro2/default)). Package  rust-proc-macro2-devel did not gets pulled by rust-packaging nor cargo was able to resolve this dependency.

May be due a missconfiguration on this machine.

Just pointing to it, so you can have this into account.

Comment 3 Fabio Valentini 2021-08-29 21:21:52 UTC
(In reply to Iago Rubio from comment #2)
> Package looks good so far.

Thanks for the quick unofficial review!

> rpmbuild did not rebuilt on my rawhide devel system due to missing macro
> (crate(proc-macro2/default)). Package  rust-proc-macro2-devel did not gets
> pulled by rust-packaging nor cargo was able to resolve this dependency.
> 
> May be due a missconfiguration on this machine.
> 
> Just pointing to it, so you can have this into account.

This is not really a misconfiguration, but plain "rpmbuild -ba / -bb" do not support all new RPM features out-of-the box, like generated BuildRequires, which the Rust (and other new packaging stacks, like modern python packaging) use.

For this reason (and other reasons), I usually recommend new packagers not even to bother with plain "rpmbuild -ba / -bb" for building binary packages from source packages, but to start off by using "mock" (a build tool that basically runs everything in a clean Fedora container) instead. That's what official builds on Fedora build servers (koji) and unofficial community builds in COPR use anyway, so you'd need to learn to deal with mock anyway at some point.

Comment 4 Iago Rubio 2021-08-29 21:33:51 UTC
Thank you so much for the info Fabio. 

Helps a lot.

I knew something was wrong, as the package built in mock with no problems at all.

Comment 5 Davide Cavalca 2021-10-18 15:00:26 UTC
Taking this review

Comment 6 Davide Cavalca 2021-10-18 15:02:56 UTC
Package was generated with rust2rpm, simplifying the review.

- 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
- license file is included with %license in %files
- package complies with Rust Packaging Guidelines

Package APPROVED.

Comment 8 Gwyn Ciesla 2021-10-18 19:31:32 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-automod

Comment 9 Fabio Valentini 2021-10-18 20:59:51 UTC
Built for rawhide:
https://bodhi.fedoraproject.org/updates/FEDORA-2021-7a815bb724

Updates for F35 and F34 pending.


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