Bug 2237084 (espanso)

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

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_sources

If it helps, I can try to provide a WIP spec file for an all-in-one package.

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.

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

sorry, Typo:

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

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.

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

This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time, but it seems
that the review is still being working out by you. If this is right, please
respond to this comment clearing the NEEDINFO flag and try to reach out the
submitter to proceed with the review.

If you're not interested in reviewing this ticket anymore, please clear the
fedora-review flag and reset the assignee, so that a new reviewer can take
this ticket.

Without any reply, this request will shortly be resetted.

This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.