https://bugzilla.redhat.com/show_bug.cgi?id=2327650
--- Comment #13 from wojnilowicz lukasz.wojnilowicz@gmail.com --- (In reply to Fabio Valentini from comment #12)
Some comments:
- You don't need to put the whole output of %cargo_license into the spec
file, that is what the output of %cargo_license_summary is for.
Done.
- The output of %cargo_license should be written to a file, and that file
included in the package. (rust2rpm should do it this way?)
Done.
- You have disabled running tests. Why? Please document why you can't run
tests, or ideally, run at least *some* tests.
A mistake due to the confusing %bcond_with behavior.
- The package name is a bit strange. The executable is just "aw-server",
you could just name the package "aw-server" too, it wouldn't conflict with any existing package. (and it would also mirror aw-sync executable being in the aw-sync package). If you want the source package name to remain "aw-server-rust" to match the upstream project, that's fine, but I would recommend to make the built packages just have the names "aw-server" and "aw-sync".
There is also https://github.com/ActivityWatch/aw-server which is Python based. The source package "aw-server-rust" has the binary "aw-server" (without the rust suffix) because aw-qt (UI for it) calls it that way. It can work with aw-server (Python based) and aw-server-rust (Rust based). I'm not sure if the Python based form would make it to Fedora, so if that's not a problem, then I would leave the names as they are right now.
- The order of sections in the spec file is very unusual - I would
recommend to do <packages> <scriptlets> <file lists>.
I believe the order is good now.
- This is more complicated than it needs to be:
install -Dm 0755 %{_builddir}/%{name}-%{commit}/target/rpm/{aw-server,aw-sync} -t %{buildroot}%{_bindir}
Just do
install -Dm 0755 target/rpm/{aw-server,aw-sync} -t %{buildroot}%{_bindir}
Done.
- You've included a systemd user session unit, but haven't added the
necessary scriptlets for it: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/ #_user_units
Skipped initially, because I thought I don't need them. I'm still not sure of their effect.
- Patch 1 looks strange. Is it OK to just remove the openssl crate
dependency entirely? I would have just removed the "vendored" feature from it. I wonder why this still compiles with the dependency removed entirely ...
No mention of openssl usage in aw-sync. It wasn't there initially at all, and isn't present for other platforms. It appeared with the bug at https://github.com/ActivityWatch/aw-server-rust/issues/478 The app is distributed as AppImage, and I believe that openssl is included for some side effect that it gives, and that makes this distribution form working.
- In general, I would recommend to split patches into logical units, not
split by which file they touch.
Done.
- The aw-webui subdirectory is empty (which might explain why things fail)
- it's an uninitialized git submodule. I suspect that you will actually need
to include those sources separately.
This is expected. I already include those sources separately from nodejs-aw-webui and point to them through export AW_WEBUI_DIR="%{_datadir}/aw-webui/dist/"
Spec URL: https://wojnilowicz.fedorapeople.org/aw-server-rust.spec SRPM URL: https://wojnilowicz.fedorapeople.org/aw-server-rust-0.13.1%5E20241216.a0cdef...