Bug 2292549 - Review Request: rust-relm4 - Idiomatic GUI library inspired by Elm and based on gtk4-rs
Summary: Review Request: rust-relm4 - Idiomatic GUI library inspired by Elm and based ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL: https://crates.io/crates/relm4
Whiteboard:
Depends On: 2292548 2297995
Blocks: 2292541 2292545 2292546
TreeView+ depends on / blocked
 
Reported: 2024-06-15 23:14 UTC by Jonathan Steffan
Modified: 2024-09-13 13:38 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2024-09-13 13:38:48 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 8010930 to 8014833 (868 bytes, patch)
2024-09-12 15:36 UTC, Fedora Review Service
no flags Details | Diff

Description Jonathan Steffan 2024-06-15 23:14:00 UTC
Spec URL: https://jsteffan.fedorapeople.org/envision/rust-relm4.spec
SRPM URL: https://jsteffan.fedorapeople.org/envision/rust-relm4-0.8.1-1.fc39/rust-relm4-0.8.1-1.fc40.src.rpm
Description: Idiomatic GUI library inspired by Elm and based on gtk4-rs
Fedora Account System Username: jsteffan

Comment 1 Fedora Review Service 2024-06-15 23:16:31 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/7617822
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2292549-rust-relm4/fedora-rawhide-x86_64/07617822-rust-relm4/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
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.

Comment 3 Fabio Valentini 2024-07-15 15:48:10 UTC
Built packages don't all install successfully:

========================================

Problem 1: conflicting requests
  - nothing provides (crate(libpanel/default) >= 0.4.0 with crate(libpanel/default) < 0.5.0~) needed by rust-relm4+all-devel-0.8.1-1.fc41.noarch from @commandline
  - nothing provides (crate(libpanel/v1_4) >= 0.4.0 with crate(libpanel/v1_4) < 0.5.0~) needed by rust-relm4+all-devel-0.8.1-1.fc41.noarch from @commandline
 Problem 2: conflicting requests
  - nothing provides (crate(libpanel/default) >= 0.4.0 with crate(libpanel/default) < 0.5.0~) needed by rust-relm4+panel-devel-0.8.1-1.fc41.noarch from @commandline
 Problem 3: package rust-relm4+libpanel-devel-0.8.1-1.fc41.noarch from @commandline requires crate(relm4/panel) = 0.8.1, but none of the providers can be installed
  - conflicting requests
  - nothing provides (crate(libpanel/default) >= 0.4.0 with crate(libpanel/default) < 0.5.0~) needed by rust-relm4+panel-devel-0.8.1-1.fc41.noarch from @commandline

========================================

Looks like you will either need to remove the "all" feature and the libpanel dependency or package the Rust bindings for libpanel too.

Comment 4 Jonathan Steffan 2024-07-15 19:00:11 UTC
https://bugzilla.redhat.com/show_bug.cgi?id=2297995 and https://bugzilla.redhat.com/show_bug.cgi?id=2297994 added.

I went with the compatible version and not the latest. The latest requires existing package updates.

Comment 5 Jonathan Steffan 2024-07-16 19:01:24 UTC
Fixed any source issues. I now build the srpms in an isolated _sourcedir. Sigh.

Spec URL: https://jsteffan.fedorapeople.org/envision/rust-relm4.spec
SRPM URL: https://jsteffan.fedorapeople.org/envision/rust-relm4-0.8.1-1.fc39/rust-relm4-0.8.1-1.fc40.src.rpm

[fedora-review-service-build]

Comment 6 Jonathan Steffan 2024-09-07 17:51:53 UTC
Okay, this should be ready for re-review :-)

https://koji.fedoraproject.org/koji/taskinfo?taskID=123069614

Comment 7 Fabio Valentini 2024-09-09 21:13:10 UTC
> #error: failed to run custom build command for `relm4-icons v0.8.3`
> #Couldn't find `icons.toml` next to `Cargo.toml`: 
> # Os { code: 2, kind: NotFound, message: "No such file or directory"

This is just an error message copied into the spec file, it doesn't have any context *why* this means you can't run *any* tests at all?

Also, it looks like the error message is from the build script for relm4-icons, not from *this* crate. So this would point towards a packaging issue in relm4-icons instead.

Comment 8 Jonathan Steffan 2024-09-10 01:13:18 UTC
Well, this doesn't actually pull in rust-relm4-icons/use it as a dependency. It has it's own source that's in this crate. The issue is that icons are not configured. I could go the route to try to get icons configured?

I'll try just skipping the tests that rely on this and see if that is suitable.

Comment 9 Fabio Valentini 2024-09-10 16:22:22 UTC
> Well, this doesn't actually pull in rust-relm4-icons/use it as a dependency.

What's this then?

"""
[dev-dependencies.relm4-icons]
version = "0.8.2"
"""

> I'll try just skipping the tests that rely on this and see if that is suitable.

That would work too ... but it would be great to know why the tests are failing to compile.

Comment 10 Jonathan Steffan 2024-09-12 05:46:19 UTC
I misremembered. It does need rust-relm4-icons, but it builds a custom build in the tests that needs an icons.toml.

So I've tried a few things.

1) The tests want criterion. Criterion wants a super specific version of tempfile. Instead of dealing with that mess, I just remove criterion in %prep:

# Remove dev dependencies for criterion, we wont run benchmarks
sed -i '/^\[dev-dependencies.criterion\]/{N;N;d}' Cargo.toml

2) I've very frailly copied the system icons.toml in %check and fixed it up for absolute paths so it is valid. The system one uses relative paths.

# Install icons.toml from rust-relm4-icons for tests
cp -pav /usr/share/cargo/registry/relm4-icons-*/icons.toml .
# Hack patch the path
sed -i 's@icons/fluentui-system-icons@/usr/share/cargo/registry/relm4-icons-0.8.3/icons/fluentui-system-icons@' icons.toml

3) This gets us further in the tests, but fails with the following in Rawhide:

error[E0433]: failed to resolve: use of undeclared crate or module `relm4_components`
 --> examples/state_management.rs:5:5
  |
5 | use relm4_components::open_dialog::{
  |     ^^^^^^^^^^^^^^^^ use of undeclared crate or module `relm4_components`

error[E0433]: failed to resolve: use of undeclared crate or module `relm4_components`
 --> examples/state_management.rs:8:5
  |
8 | use relm4_components::save_dialog::{
  |     ^^^^^^^^^^^^^^^^ use of undeclared crate or module `relm4_components`

error[E0425]: cannot find value `TAG_OUTLINE_ADD` in module `icon_names`
  --> examples/state_management.rs:98:48
   |
98 |                     set_icon_name: icon_names::TAG_OUTLINE_ADD,
   |                                                ^^^^^^^^^^^^^^^ not found in `icon_names`

Some errors have detailed explanations: E0425, E0433.
For more information about an error, try `rustc --explain E0425`.
error: could not compile `relm4` (example "state_management") due to 3 previous errors

Okay, so we need relm4_components. However, rust-relm4-components needs rust-relm4 so when trying to build rust-relm4-components first:

Problem 1: nothing provides requested (crate(relm4) >= 0.8.0 with crate(relm4) < 0.9.0~)
Problem 2: nothing provides requested (crate(relm4/macros) >= 0.8.0 with crate(relm4/macros) < 0.9.0~)

And oddly, also failed with a different error in F40:

error[E0425]: cannot find value `MINUS` in module `icon_names`
  --> examples/icons.rs:37:48
   |
37 |                     set_icon_name: icon_names::MINUS,
   |                                                ^^^^^ not found in `icon_names`

For more information about this error, try `rustc --explain E0425`.
error: could not compile `relm4` (example "icons") due to 1 previous error

I've uploaded this current situation:

Spec URL: https://jsteffan.fedorapeople.org/envision/rust-relm4.spec
SRPM URL: https://jsteffan.fedorapeople.org/envision/rust-relm4-0.8.1-1.fc40/rust-relm4-0.8.1-1.fc40.src.rpm

However, after documenting what is going on it seems the examples are what is failing to build. Should/can we just turn those off?

Comment 11 Jonathan Steffan 2024-09-12 05:50:15 UTC
Umm... and somehow it just built. I'm so confused.

Comment 12 Fedora Review Service 2024-09-12 06:08:18 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8010930
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2292549-rust-relm4/fedora-rawhide-x86_64/08010930-rust-relm4/fedora-review/review.txt

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.

Comment 13 Fabio Valentini 2024-09-12 10:02:35 UTC
Ok, thanks a lot for investigating!

To me, this looks like a case of "the tests are set up in a way that makes them basically only runnable from within the upstream git repository". In this case (and especially because it would introduce a dependency loop with relm4-components), turning off tests entirely is obviously acceptable. This is a very common cause of "not able to run tests, sorry" (https://src.fedoraproject.org/rpms/rust-serde/blob/rawhide/f/rust-serde.spec#_2-3).

In this case, your build probably passed because you dropped the check %bcond entirely, with it not being defined being equivalent to turning it off.

Comment 14 Jonathan Steffan 2024-09-12 15:11:03 UTC
AHA! That's what I get for working on this so late. Now it make sense. I was cleaning up the comments and removed too much.


[scripts.prep]
post = [
    "# Copy LICENSE files",
    "cp -pav %{SOURCE100} %{SOURCE101} .",
    "# Remove dev dependencies for criterion, we wont run benchmarks",
    "sed -i '/^\[dev-dependencies.criterion\]/{N;N;d}' Cargo.toml"
]

Spec URL: https://jsteffan.fedorapeople.org/envision/rust-relm4.spec
SRPM URL: https://jsteffan.fedorapeople.org/envision/rust-relm4-0.8.1-1.fc40/rust-relm4-0.8.1-1.fc42.src.rpm

Comment 15 Fedora Review Service 2024-09-12 15:36:35 UTC
Created attachment 2046584 [details]
The .spec file difference from Copr build 8010930 to 8014833

Comment 16 Fedora Review Service 2024-09-12 15:36:37 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8014833
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2292549-rust-relm4/fedora-rawhide-x86_64/08014833-rust-relm4/fedora-review/review.txt

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.

Comment 17 Fabio Valentini 2024-09-13 09:19:15 UTC
Thanks, looks good to me now!

> # Remove dev dependencies for criterion, we wont run benchmarks
> sed -i '/^\[dev-dependencies.criterion\]/{N;N;d}' Cargo.toml

Technically this isn't needed if you flip the check bcond to "off", but it doesn't hurt to keep it.

===

Package was generated with rust2rpm, simplifying the review.

✅ package contains only permissible content
✅ package builds and installs without errors on rawhide
✅ test suite is run and all unit tests pass
🫤 latest version of the crate is packaged (version matches dependencies in Fedora)
✅ license matches upstream specification and is acceptable for Fedora
✅ license files are included with %license in %files (temporarily manually included from upstream)
✅ package complies with Rust Packaging Guidelines

Package APPROVED.

===

Recommended post-import rust-sig tasks:

- 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

- add @rust-sig with "commit" access as package co-maintainer
  (should happen automatically)

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

- track package in koschei for all built branches
  (should happen automatically once rust-sig is co-maintainer)

Comment 18 Jonathan Steffan 2024-09-13 13:16:00 UTC
(In reply to Fabio Valentini from comment #17)
> Thanks, looks good to me now!
> 
> > # Remove dev dependencies for criterion, we wont run benchmarks
> > sed -i '/^\[dev-dependencies.criterion\]/{N;N;d}' Cargo.toml
> 
> Technically this isn't needed if you flip the check bcond to "off", but it
> doesn't hurt to keep it.

I intended to keep trying to enable tests every update or file bugs, and we don't want to package criterion right now :-)

Thanks!

Comment 19 Fedora Admin user for bugzilla script actions 2024-09-13 13:18:50 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-relm4

Comment 20 Fedora Update System 2024-09-13 13:34:23 UTC
FEDORA-2024-48c55740eb (rust-relm4-0.8.1-2.fc42) has been submitted as an update to Fedora 42.
https://bodhi.fedoraproject.org/updates/FEDORA-2024-48c55740eb

Comment 21 Fedora Update System 2024-09-13 13:38:48 UTC
FEDORA-2024-48c55740eb (rust-relm4-0.8.1-2.fc42) has been pushed to the Fedora 42 stable repository.
If problem still persists, please make note of it in this bug report.


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