https://bugzilla.redhat.com/show_bug.cgi?id=2051062
--- Comment #3 from Aleksei Bavshin alebastr89@gmail.com --- (In reply to Jakub Kadlčík from comment #1)
Hello Aleksei, thank you for the package.
Thanks for the review!
I tried to install and run it, and I think it works - I got a blue screen and a mouse cursor. Maybe if you are a river developer, you might want to describe the default key bindings in some README because I don't know how to launch any applications :D. But that's outside of the package-review scope.
The upstream developer strongly believe that the user should write their own configuration based on the example they provide and don't have any support for the default config (in fact support for the default system config was removed at some point just before the initial release). Which means there's no default keybindings :(
README.md describes how to start in the Usage section, but I agree that it raises the entry barrier a bit higher than usual even for tiling WMs.
Just for the record, the package fails to build for F35 but it will be EOL soon, so I think we don't care about it anymore.
I assume that's because f35 does not have required version of wlroots. All wlroots releases so far have been API- and ABI-breaking and the compositors based on the library or the language bindings to wlroots support only an exact minor version.
Source100: %{name}.desktop
Why not Source3?
No particular reason, just wanted a clean separation from the sources provided by upstream. I'll change that to Source3.
# bundled sources Provides: bundled(zig-pixman) Provides: bundled(zig-wayland) Provides: bundled(zig-wlroots) Provides: bundled(zig-xkbcommon)
My apologies, I have zero experience with Zig programming language and with compiled programming languages overall, for that matter.
Is it absolutely necessary to bundle these? Packaging guidelines https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling tells us to make every effort to avoid bundling.
Ah, here comes the complicated part: zig, being still actively developed and designed, does not have any support for sharing the code yet. Think rust with its unstable ABI, but without a package manager or a standard library location.
So all the bundled code is embedded to the repo as git submodules and the paths are hardcoded in the buildsystem. Plus some details of the build system which make it even more inconvenient.
Tl;dr: I don't want to deal with that until the Zig toolchain evolves to offer some package/code reuse solution. And until we have support for that in zig-rpm-macros.
%description
I just wanted to say that I like the well-written description
%zig_build \ -Dxwayland
%zig_install \ -Dxwayland
A bit unnecessary to wrap the linen there but I don't mind
%license LICENSE %doc README.md
Maybe we can add %doc CONTRIBUTING.md as well?
I did not see any value for the end user in this file; any prospective contributor to the project will find the same info in the git repository.
Issues:
- Package installs a %{name}.desktop using desktop-file-install or desktop- file-validate if there is such a file.
The fedora-review tool complains about this. I think we need to do something like
%check desktop-file-validate
%{buildroot}%{_datadir}/wayland-sessions/%{name}.desktop
I assumed that we don't do that for desktop session files, but sure, will add.
NTP License (legal disclaimer) Historical Permission Notice and Disclaimer - sell variant
/home/jkadlcik/2051062-river/upstream-unpacked/Source0/river-0.1.3/deps/zig-wlroots/protocol/wlr-layer-shell-unstable-v1.xml /home/jkadlcik/2051062-river/upstream-unpacked/Source0/river-0.1.3/protocol/wlr-layer-shell-unstable-v1.xml
This license isn't mentioned in the License field but I am not sure if it is necessary.
In the Callaway licensing scheme this license was handled as one of the variants of MIT. I should update the tag to
GPL-3.0-or-later AND ISC AND HPND-sell-variant
and ask upstream about the license of the code generated with zig-wayland from the protocol descriptions.
[ ]: Package must own all directories that it creates. Note: Directories without known owners: /usr/share/river
There is
%{_datadir}/%{name}/init.example
in the %files section. We need to change it or add a new line for the whole directory.
Will add %dir entry, thanks for catching that!