[Bug 2237084] New: Review Request: rust-espanso - Cross-platform Text Expander written in Rust

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: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]
  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

[Bug 2237084] Review Request: rust-espanso - Cross-platform Text Expander written in Rust

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

Fabio Valentini  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
           Doc Type|---                         |If docs needed, set a value

--- Comment #1 from Fabio Valentini  ---
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).

[Bug 2237084] Review Request: rust-espanso - Cross-platform Text Expander written in Rust

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

--- Comment #2 from Fabio Valentini  ---
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.

[Bug 2237084] Review Request: rust-espanso - Cross-platform Text Expander written in Rust

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

--- Comment #3 from Robert-André Mauchin 🐧  ---
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.

[Bug 2237084] Review Request: rust-espanso - Cross-platform Text Expander written in Rust

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

--- Comment #4 from Fabio Valentini  ---
That definitely shouldn't happen. Can you report a bug against cargo2rpm with
the reproduction steps please? https://pagure.io/fedora-rust/csrgo2rpm

[Bug 2237084] Review Request: rust-espanso - Cross-platform Text Expander written in Rust

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

--- Comment #5 from Fabio Valentini  ---
sorry, Typo:

https://pagure.io/fedora-rust/cargo2rpm

[Bug 2237084] Review Request: espanso - Cross-platform Text Expander written in Rust

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

--- Comment #6 from Robert-André Mauchin 🐧  ---
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.

[Bug 2237084] Review Request: espanso - Cross-platform Text Expander written in Rust

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

Fabio Valentini  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends On|                            |2237104 (rust-markdown),
                   |                            |2237101 (rust-include_dir),
                   |                            |2237103 (rust-log-panics)
              Flags|                            |fedora-review?
           Assignee|[e-mail]    |[e-mail]

--- Comment #7 from Fabio Valentini  ---
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)

[Bug 2237084] Review Request: espanso - Cross-platform Text Expander written in Rust

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

needinfo canceled: [Bug 2237084] Review Request: espanso - Cross-platform Text Expander written in Rust

Product: Fedora
Version: rawhide
Component: Package Review

Fabio Valentini  has canceled Package Review
's request for Fabio Valentini
's needinfo:
Bug 2237084: Review Request: espanso - Cross-platform Text Expander written in
Rust
https://bugzilla.redhat.com/show_bug.cgi?id=2237084