https://bugzilla.redhat.com/show_bug.cgi?id=2237084
Bug ID: 2237084 Summary: Review Request: rust-espanso - Cross-platform Text Expander written in Rust Product: Fedora Version: rawhide Status: NEW Component: Package Review Assignee: nobody@fedoraproject.org Reporter: zebob.m@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://eclipseo.fedorapeople.org/for-review/rust-espanso.spec SRPM URL: https://eclipseo.fedorapeople.org/for-review/rust-espanso-2.2.0-1.fc38.src.r...
Description: Cross-platform Text Expander written in Rust. A text expander is a program that detects when you type a specific keyword and replaces it with something else. This is useful in many ways: - Save a lot of typing, expanding common sentences. - Create system-wide code snippets. - Execute custom scripts - Use emojis like a pro.
Fedora Account System Username: eclipseo
To build it against the dependencies, use the following COPR in your rawhide mock.cfg:
[copr:copr.fedorainfracloud.org:eclipseo:espanso] name=Copr repo for espanso owned by eclipseo baseurl=https://download.copr.fedorainfracloud.org/results/eclipseo/espanso/fedora-r... type=rpm-md skip_if_unavailable=True gpgcheck=1 gpgkey=https://download.copr.fedorainfracloud.org/results/eclipseo/espanso/pubkey.g... repo_gpgcheck=0 enabled=1 enabled_metadata=1
https://bugzilla.redhat.com/show_bug.cgi?id=2237084
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |decathorpe@gmail.com Doc Type|--- |If docs needed, set a value
--- Comment #1 from Fabio Valentini decathorpe@gmail.com --- Why are you packaging all this project's components separately? They are *not even allowed to* be packaged separately as libraries if they're not published on crates.io ...
Rust packaging macros support workspaces since version 24, so just packaging from GitHub tarball download should work. In this case, drop the "rust-" prefix from the package, and just package it as "espanso" (and use upstream tarball).
https://bugzilla.redhat.com/show_bug.cgi?id=2237084
--- Comment #2 from Fabio Valentini decathorpe@gmail.com --- You can look at several other projects in Fedora (or pending import) that have already adopted this approach:
- firecracker - ntpd-rs (https://github.com/decathorpe/ntpd-rs-rpms/blob/main/ntpd-rs/ntpd-rs.spec) - system76-keyboard-configurator (does not use %cargo_generate_buildrequires with workspace support yet)
You *can* use rust2rpm for workspace projects (download sources from GitHub, extract them, and run "rust2rpm ./Cargo.toml" in the unpacked sources), but it does not automate patching Cargo.toml files.
But the way it's done right now is not correct, i.e. packaging private components / unpublished workspace members is forbidden: https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/#_package_sou...
If it helps, I can try to provide a WIP spec file for an all-in-one package.
https://bugzilla.redhat.com/show_bug.cgi?id=2237084
--- Comment #3 from Robert-André Mauchin 🐧 zebob.m@gmail.com --- Thanks for noticing,
I've got a basic issue:
+ cd espanso-2.1.8 + cd espanso + /usr/bin/cargo2rpm --path Cargo.toml buildrequires --features default,wayland --with-check Traceback (most recent call last): File "/usr/bin/cargo2rpm", line 8, in <module> sys.exit(main()) ^^^^^^ File "/usr/lib/python3.12/site-packages/cargo2rpm/__main__.py", line 111, in main action_buildrequires(args) File "/usr/lib/python3.12/site-packages/cargo2rpm/__main__.py", line 50, in action_buildrequires brs = workspace_buildrequires(metadata, flags, args.with_check) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.12/site-packages/cargo2rpm/rpm.py", line 234, in workspace_buildrequires member_flags = metadata.get_feature_flags_for_workspace_members(flags) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.12/site-packages/cargo2rpm/metadata.py", line 617, in get_feature_flags_for_workspace_members required_features[package.name].update(reqs) ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^ KeyError: 'espanso'
I don't get why I have this error. I have it locally too when I run cargo2rpm --path Cargo.toml buildrequires. It was working correctly before.
https://bugzilla.redhat.com/show_bug.cgi?id=2237084
--- Comment #4 from Fabio Valentini decathorpe@gmail.com --- That definitely shouldn't happen. Can you report a bug against cargo2rpm with the reproduction steps please? https://pagure.io/fedora-rust/csrgo2rpm
https://bugzilla.redhat.com/show_bug.cgi?id=2237084
--- Comment #5 from Fabio Valentini decathorpe@gmail.com --- sorry, Typo:
https://pagure.io/fedora-rust/cargo2rpm
https://bugzilla.redhat.com/show_bug.cgi?id=2237084
--- Comment #6 from Robert-André Mauchin 🐧 zebob.m@gmail.com --- Spec URL: https://eclipseo.fedorapeople.org/for-review/espanso.spec SRPM URL: https://eclipseo.fedorapeople.org/for-review/espanso-2.1.8-1.fc39.src.rpm
Ok, so the whole thing was rewritten with workspaces in mind:
%build %cargo_build -- --package={espanso,espanso-clipboard,espanso-config,espanso-detect,espanso-engine,espanso-info,espanso-inject,espanso-ipc,espanso-kvs,espanso-mac-utils,espanso-match,espanso-migrate,espanso-modulo,espanso-package,espanso-path,espanso-render,espanso-ui} --bin espanso --features="default" --bin espanso-wayland --features="wayland"
%{cargo_license_summary} %{cargo_license} > LICENSE.dependencies
%install install -m 0755 -vd %{buildroot}%{_bindir} install -m 0755 -vp target/release/espanso %{buildroot}%{_bindir}/ install -m 0755 -vp target/release/espanso-wayland %{buildroot}%{_bindir}/
All patches have been taken from the previous split packages.
https://bugzilla.redhat.com/show_bug.cgi?id=2237084
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |2237104 (rust-markdown), | |2237101 (rust-include_dir), | |2237103 (rust-log-panics) Flags| |fedora-review? Assignee|nobody@fedoraproject.org |decathorpe@gmail.com
--- Comment #7 from Fabio Valentini decathorpe@gmail.com --- Thanks! I have a few quick comments before I do a full review:
------------------------------------------------------------
The upstream tarball bundles a copy of wxWidgets. It needs to be removed in %prep:
%prep ... # drop vendored wxWidgets sources rm -r espanso-modulo/vendor
------------------------------------------------------------
%cargo_build -- --package={espanso,espanso-clipboard,espanso-config,espanso-detect,espanso-engine,espanso-info,espanso-inject,espanso-ipc,espanso-kvs,espanso-mac-utils,espanso-match,espanso-migrate,espanso-modulo,espanso-package,espanso-path,espanso-render,espanso-ui} --bin espanso --features="default" --bin espanso-wayland --features="wayland"
This should not be necessary, and I also think it doesn't do what you actually want. This should be enough to achieve the same thing:
%cargo_build -f wayland
------------------------------------------------------------
Similarly, for %cargo_test, the following should be equivalent to what you're doing manually:
%cargo_test -- -- --skip ipc_multiple_clients
------------------------------------------------------------
If you want to actually have distinct License tags for espanso and espanso-wayland, you also need to call %cargo_license / %cargo_license_summary with different arguments for both crates:
# for "default" %{cargo_license_summary} %{cargo_license} > LICENSE.dependencies
# for "wayland" %{cargo_license_summary -f wayland} %{cargo_license -f wayland} > LICENSE.dependencies.wayland
Otherwise one or the other will not be accurate.
In general, you should apply the same "-a" / "-f" flags to *all* %cargo_* macros (except %cargo_prep) to avoid inconsistent results or weird errors.
------------------------------------------------------------
Package also doesn't built yet due to missing dependencies:
- include_dir (pending review) - log-panics (approved, pending unretirement) - markdown (pending review)
There's also "yaml-rust-terzi", which is a fork of the "yaml-rust" crate. The code for it will need to be bundled in this package, and the dependency replaced with a `path = "./path/to/yaml-rust"`-style dependency).
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2237101 [Bug 2237101] Review Request: rust-include_dir - Embed the contents of a directory in your binary https://bugzilla.redhat.com/show_bug.cgi?id=2237103 [Bug 2237103] Review Request: rust-log-panics - Panic hook which logs panic messages rather than printing them https://bugzilla.redhat.com/show_bug.cgi?id=2237104 [Bug 2237104] Review Request: rust-markdown - Native Rust library for parsing Markdown and (outputting HTML)
https://bugzilla.redhat.com/show_bug.cgi?id=2237084 Bug 2237084 depends on bug 2237103, which changed state.
Bug 2237103 Summary: Review Request: rust-log-panics - Panic hook which logs panic messages rather than printing them https://bugzilla.redhat.com/show_bug.cgi?id=2237103
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA
Product: Fedora Version: rawhide Component: Package Review
Fabio Valentini decathorpe@gmail.com has canceled Package Review package-review@lists.fedoraproject.org's request for Fabio Valentini decathorpe@gmail.com's needinfo: Bug 2237084: Review Request: espanso - Cross-platform Text Expander written in Rust https://bugzilla.redhat.com/show_bug.cgi?id=2237084
package-review@lists.fedoraproject.org