Bug 2030857 - Review Request: rust-ignition-config - Data structures for reading/writing Ignition configs
Summary: Review Request: rust-ignition-config - Data structures for reading/writing Ig...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Unspecified
unspecified
high
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-12-09 21:45 UTC by Sohan Kunkerkar
Modified: 2021-12-14 19:44 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-12-14 19:44:01 UTC
Type: Bug
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)

Description Sohan Kunkerkar 2021-12-09 21:45:58 UTC
Spec URL: https://sohank2602.fedorapeople.org/rust-ignition-config/rust-ignition-config.spec
SRPM URL: https://sohank2602.fedorapeople.org/rust-ignition-config/rust-ignition-config-0.2.0-1.fc35.src.rpm
Description: Data structures for reading/writing Ignition configs
Fedora Account System Username: sohank2602


Koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=79767419

Comment 1 Fabio Valentini 2021-12-10 15:38:57 UTC
Taking this review.

Comment 2 Fabio Valentini 2021-12-10 15:50:45 UTC
You're missing a dependency: schemafy_lib.
It leads to two of the subpackages not being installable:

Error: 
 Problem 1: conflicting requests
  - nothing provides (crate(schemafy_lib/default) >= 0.6.0 with crate(schemafy_lib/default) < 0.7.0~) needed by rust-ignition-config+regenerate-devel-0.2.0-1.fc36.noarch
 Problem 2: conflicting requests
  - nothing provides (crate(schemafy_lib/default) >= 0.6.0 with crate(schemafy_lib/default) < 0.7.0~) needed by rust-ignition-config+schemafy_lib-devel-0.2.0-1.fc36.noarch

It looks like that is used to generate code from JSON schemas at build time?
And if that's possible to regenerate generated sources during RPM builds, that should be done.

Can you check if that's possible and works (turn on the "regenerate" feature by default with a Cargo.toml patch), i.e. adding

```
[features]
default = ["regenerate"]
```

or by modifying the build.rs script such that the code for regenerating the sources is run whether the "regenerate" feature is enabled or not?

If that does not work or is not possible, you can drop the "regenerate" feature and its optional dependencies (including "schemafy_lib").

Comment 3 Benjamin Gilbert 2021-12-10 16:09:50 UTC
"regenerate" is a development feature and isn't intended to be invoked by other crates.  Regenerating the sources on every build could cause API incompatibility with upstream if there's version skew between upstream's pinned schemafy_lib and Fedora's.  So I'd recommend dropping the feature and its deps.

Comment 4 Fabio Valentini 2021-12-10 16:57:37 UTC
Sounds good to me.

Using "rust2rpm -p" to generate a small patch to remove the optional dependencies and the [features] section entirely should be enough.

Comment 5 Sohan Kunkerkar 2021-12-10 18:00:48 UTC
(In reply to Fabio Valentini from comment #4)
> Sounds good to me.
> 
> Using "rust2rpm -p" to generate a small patch to remove the optional
> dependencies and the [features] section entirely should be enough.

 Thanks!
 
 As suggested, I have created a small patch (https://sohank2602.fedorapeople.org/rust-ignition-config/ignition-config-fix-metadata.diff) to address the issue and updated the specfile accordingly.

 Koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=79799566

Comment 6 Fabio Valentini 2021-12-10 21:13:10 UTC
Looks good to me, but please add a short descriptive comment between lines 16 and 17 of the .spec file, something like:

# * disable development-only feature for regenerating sources

===

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.

===

Recommended post-import rust-sig tasks:

- add package to rust-sig with "commit" access

- set bugzilla assignee overrides to @rust-sig (optional)

- set up package on release-monitoring.org:
  project: $crate
  homepage: https://crates.io/crates/$crate
  backend: crates.io
  version scheme: semantic
  version filter: alpha;beta;rc;pre

- track package in koschei for all built branches

Comment 7 Sohan Kunkerkar 2021-12-13 14:05:05 UTC
Requested a git repo: https://pagure.io/releng/fedora-scm-requests/issue/39335

Comment 8 Gwyn Ciesla 2021-12-13 15:35:05 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-ignition-config

Comment 9 Sohan Kunkerkar 2021-12-13 18:22:02 UTC
release-monitoring request: https://release-monitoring.org/project/241658/

Koschei package: https://koschei.fedoraproject.org/package/rust-ignition-config?

Comment 10 Benjamin Gilbert 2021-12-13 19:14:32 UTC
This bug should stay assigned to the approver, I believe.

Comment 11 Fabio Valentini 2021-12-14 19:23:41 UTC
(In reply to Benjamin Gilbert from comment #10)
> This bug should stay assigned to the approver, I believe.

Yes. Otherwise the dist-git repository request is closed as invalid.

BTW, it looks like you also built the package for f35 and f34, but forgot to submit those builds to bodhi?

Comment 12 Sohan Kunkerkar 2021-12-14 19:41:12 UTC
(In reply to Fabio Valentini from comment #11)
> (In reply to Benjamin Gilbert from comment #10)
> > This bug should stay assigned to the approver, I believe.
> 
> Yes. Otherwise the dist-git repository request is closed as invalid.
> 
> BTW, it looks like you also built the package for f35 and f34, but forgot to
> submit those builds to bodhi?

ah, right

F-35: https://bodhi.fedoraproject.org/updates/FEDORA-2021-a9ed18c2c3

F-34: https://bodhi.fedoraproject.org/updates/FEDORA-2021-e95cbf8a90

Comment 13 Fabio Valentini 2021-12-14 19:44:01 UTC
Thanks!

Since this package is now already stable in rawhide, we can close this bug.

https://bodhi.fedoraproject.org/updates/FEDORA-2021-01adcb8a03


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