https://bugzilla.redhat.com/show_bug.cgi?id=2143919

Fabio Valentini decathorpe@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |decathorpe@gmail.com

--- Comment #1 from Fabio Valentini decathorpe@gmail.com --- Two quick comments from Rust packaging POV:

Please don't just downgrade dependencies, but let other Rust packagers know what you need. I already updated Fedora packages for env_logger 0.9.3, libc 0.2.137 a few days ago, and the update for regex 1.7.0 will happen soon.

As for the updates to API-breaking newer versions of libudev and rust-ini, please comment on the bugs for these versions, or mark them as blocking this review request. Updates to API-breaking versions are only done on an as-needed basis, so unless I know something *needs* the new version, they won't be updated.

Since this package also contains a Rust binary, you'll need to include a breakdown of statically linked components and their licenses. Luckily for you, I've recently worked on a macro to do that. You can add `%{cargo_license} > LICENSE.dependencies` after %cargo_build to automatically generate a file that contains all statically linked components and their licenses, which you can then include with `%license LICENSE.dependencies` in %files. You'll also need to add a separate License tag for the %{crate} package to reflect all listed licenses.

https://bugzilla.redhat.com/show_bug.cgi?id=2143919 Bug 2143919 depends on bug 2056606, which changed state.

Bug 2056606 Summary: rust-rust-ini-0.18.0 is available https://bugzilla.redhat.com/show_bug.cgi?id=2056606

What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA

https://bugzilla.redhat.com/show_bug.cgi?id=2143919

--- Comment #3 from Michal Sekletar msekleta@redhat.com --- I finally found some to time to return to this. Dropped version downgrade patch as well as API break workaround patches and applied your suggestions regarding licenses. Updated spec,

https://copr-dist-git.fedorainfracloud.org/cgit/msekleta/rust-prefixdevname/...

Fabio, thanks a lot for updating the dependencies and all the help so far!

https://bugzilla.redhat.com/show_bug.cgi?id=2143919

Fabio Valentini decathorpe@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? | |needinfo?(msekleta@redhat.c | |om) Assignee|nobody@fedoraproject.org |decathorpe@gmail.com

https://bugzilla.redhat.com/show_bug.cgi?id=2143919

Jakub Kadlčík jkadlcik@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- URL| |https://crates.io/crates/pr | |efixdevname

--- Comment #6 from Jakub Kadlčík jkadlcik@redhat.com --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5729306 (succeeded)

Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...

Please take a look if any issues were found.

--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.

https://bugzilla.redhat.com/show_bug.cgi?id=2143919

Fabio Valentini decathorpe@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(decathorpe@gmail. | |com) |

--- Comment #7 from Fabio Valentini decathorpe@gmail.com --- From LICENSE.dependencies:

(MIT OR Apache-2.0) AND Unicode-DFS-2016: regex-syntax v0.6.29 MIT OR Apache-2.0: ahash v0.7.6 MIT OR Apache-2.0: cfg-if v1.0.0 MIT OR Apache-2.0: env_logger v0.9.3 MIT OR Apache-2.0: getrandom v0.2.8 MIT OR Apache-2.0: hashbrown v0.12.3 MIT OR Apache-2.0: humantime v2.1.0 MIT OR Apache-2.0: lazy_static v1.4.0 MIT OR Apache-2.0: libc v0.2.140 MIT OR Apache-2.0: log v0.4.17 MIT OR Apache-2.0: once_cell v1.17.1 MIT OR Apache-2.0: regex v1.7.3 MIT: atty v0.2.14 MIT: dlv-list v0.3.0 MIT: libudev v0.3.0 MIT: libudev-sys v0.1.4 MIT: ordered-multimap v0.4.3 MIT: prefixdevname v0.2.0 MIT: rust-ini v0.18.0 Unlicense OR MIT: aho-corasick v0.7.20 Unlicense OR MIT: memchr v2.5.0 Unlicense OR MIT: termcolor v1.2.0

According to the latest guidance from Red Hat Legal, the License tag of the subpackage should look something like this:

License: MIT AND (MIT OR Apache-2.0) AND (Unlicense OR MIT) AND Unicode-DFS-2016

c.f. https://docs.fedoraproject.org/en-US/legal/license-field/#_license_expressio...

https://bugzilla.redhat.com/show_bug.cgi?id=2143919

Michal Sekletar msekleta@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(decathorpe@gmail. | |com)

--- Comment #9 from Michal Sekletar msekleta@redhat.com --- Hi Fabio,

Do you think this is ready now and you see anything else that needs to be addressed?

https://bugzilla.redhat.com/show_bug.cgi?id=2143919

Fabio Valentini decathorpe@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+

--- Comment #11 from Fabio Valentini decathorpe@gmail.com --- Thanks, looks good to me now Sorry for the delay, the previous notification email seems to have been lost in my inbox somewhere :( I have three minor non-blocking complaints (included below), and one blocking complaint.

===

Two minor complaints that you can fix before importing the package:

%{_prefix}/lib/dracut/modules.d/71prefixdevname %{_prefix}/lib/dracut/modules.d/71prefixdevname/* %{_prefix}/lib/dracut/modules.d/71prefixdevname-tools %{_prefix}/lib/dracut/modules.d/71prefixdevname-tools/*

These cause RPM to match the directories themselves twice. If you want to have the packages to own these two directories *and* all their contents, then you can just use:

%{_prefix}/lib/dracut/modules.d/71prefixdevname/ %{_prefix}/lib/dracut/modules.d/71prefixdevname-tools/

Which is more concise and does not cause RPM to complain.

The Patch0001 file looks ok to me, but it would be great to document *why* this is necessary, or if this patch is a candidate for upstreaming (or if it's even a backport of an upstream change).

Additionally, you might want to use "rust-packaging >= 23" instead of ">= 21", since the %cargo_license macro was only added in v23.

===

I also ran a scratch build of the latest version: https://koji.fedoraproject.org/koji/taskinfo?taskID=101875804

The build failed due to the tests failing on i686. This might indicate an issue that's specific to 32-bit architectures. I assume you do not need prefixdevname on i686 since it's not a multilib capable package at all, so I would be fine if you just added "ExcludeArch: %{ix86}".

https://bugzilla.redhat.com/show_bug.cgi?id=2143919

Fabio Valentini decathorpe@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags|fedora-review? |fedora-review+ |needinfo?(decathorpe@gmail. | |com) |

--- Comment #13 from Fabio Valentini decathorpe@gmail.com --- Thanks, looks good to me now. Sorry that this has taken so long. Package APPROVED

===

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 - licenses of statically linked components are reflected in license tag - license file is included with %license in %files - package complies with Rust Packaging Guidelines

===

Recommended post-import rust-sig tasks:

- add @rust-sig with "commit" access as package co-maintainer

- 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 distro: Fedora Package: rust-$crate

- track package in koschei for all built branches

https://bugzilla.redhat.com/show_bug.cgi?id=2143919

Michal Sekletar msekleta@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(msekleta@redhat.c | |om) |

--- Comment #15 from Michal Sekletar msekleta@redhat.com --- (In reply to Fabio Valentini from comment #14)

Hi, it's been a few weeks since this has been approved, but the package wasn't imported yet. Please comment here if you are still interested in this package, since I will likely need to refresh the "fedora-review" tag before you can request the dist-git repo.

I am still interested in the package. I was about to request the repo...

https://bugzilla.redhat.com/show_bug.cgi?id=2143919

--- Comment #17 from Fabio Valentini decathorpe@gmail.com --- subtle *ping* to actually import the package :)

https://bugzilla.redhat.com/show_bug.cgi?id=2143919

Michal Sekletar msekleta@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(msekleta@redhat.c | |om) |

--- Comment #19 from Michal Sekletar msekleta@redhat.com --- Build for Fedora 40 is now available and package update has been submitted to Bodhi.

https://koji.fedoraproject.org/koji/buildinfo?buildID=2336517 https://bodhi.fedoraproject.org/updates/FEDORA-2023-e8effe84aa