Bug 2143583

Summary: Review Request: lua-ev - Lua integration with libev
Product: [Fedora] Fedora Reporter: Benson Muite <benson_muite>
Component: Package ReviewAssignee: Jonny Heggheim <hegjon>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: hegjon, package-review
Target Milestone: ---Flags: hegjon: fedora-review+
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2023-01-08 20:00:30 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Comment 1 Jonny Heggheim 2022-11-17 22:39:01 UTC
The module is not loading, the install location seems strange.

$ rpm -ql lua-ev
/usr/lib/.build-id
/usr/lib/.build-id/f2
/usr/lib/.build-id/f2/c3f640879d3e057d274a2318e4db45eda2237b
/usr/lib64/lua/5.45.4/cmod/ev.so
/usr/share/doc/lua-ev
/usr/share/doc/lua-ev/README
/usr/share/doc/lua-ev/example.lua
/usr/share/licenses/lua-ev
/usr/share/licenses/lua-ev/LICENSE

$ lua -e 'require "ev"'
lua: (command line):1: module 'ev' not found:
	no field package.preload['ev']
	no file '/usr/share/lua/5.4/ev.lua'
	no file '/usr/share/lua/5.4/ev/init.lua'
	no file '/usr/lib64/lua/5.4/ev.lua'
	no file '/usr/lib64/lua/5.4/ev/init.lua'
	no file './ev.lua'
	no file './ev/init.lua'
	no file '/usr/lib64/lua/5.4/ev.so'
	no file '/usr/lib64/lua/5.4/loadall.so'
	no file './ev.so'
stack traceback:
	[C]: in function 'require'
	(command line):1: in main chunk
	[C]: in ?

Comment 2 Jonny Heggheim 2022-11-17 22:48:55 UTC
This steps/changes are working on my laptop:

---

%build
%cmake -DINSTALL_CMOD=%{lua_libdir}
%cmake_build

%install
%cmake_install

%check
LUA_CPATH=%{buildroot}%{lua_libdir}/?.so \
lua example.lua

%files
%license LICENSE
%doc README
%doc example.lua
%{lua_libdir}/ev.so

---


$ lua -e 'ev = require "ev"; print(ev.version())'
4.0	33.0

Comment 3 Jonny Heggheim 2022-11-17 22:55:26 UTC
$ lua /usr/share/doc/lua-ev/example.lua
Register build_all_timers callback
Run the event loop
Run build_all_timers callback
Thu Nov 17 23:54:44 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:44 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:45 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:45 2022	CB 2	interval: 2.0
Thu Nov 17 23:54:45 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:46 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:46 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:47 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:47 2022	CB 2	interval: 2.0
Thu Nov 17 23:54:47 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:48 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:48 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:48 2022	CB 1	interval: 5.0
Thu Nov 17 23:54:49 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:49 2022	CB 2	interval: 2.0
Thu Nov 17 23:54:49 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:50 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:50 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:51 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:51 2022	CB 2	interval: 2.0
Thu Nov 17 23:54:51 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:52 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:52 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:53 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:53 2022	CB 2	interval: 2.0
Thu Nov 17 23:54:53 2022	CB 3	interval: 0.5
Thu Nov 17 23:54:53 2022	CB 1	interval: 5.0
Thu Nov 17 23:54:53 2022	CB 4	interval: 10.0

Comment 4 Jonny Heggheim 2022-11-18 11:45:13 UTC
I will start again at the review once a functional package have been re-submitted

Comment 6 Jonny Heggheim 2022-12-05 23:01:10 UTC
> License:        Apache-2.0

The license file and README says it is MIT licensed.

You could use the README as %license, since the license/copyright is in-lined.

Not having a verbatim URL is annoying, I can not open the link directly and have to manual string replacement.

Comment 8 Jonny Heggheim 2022-12-08 21:50:26 UTC
Approved!

Please fix %changelog before import:

> lua-ev.x86_64: W: incoherent-version-in-changelog 1.5-1 ['1.5-2.fc38', '1.5-2']



Suggestion, feel free to keep as is:
I think it would be nice to also include the readme in the %doc (duplicated, in addition to %license). Users wanting to save space can always install with --nodocs.

I do not think there is a best practice, but we had some discussion in the review of lua-cosmo bug 2142671#c4

Comment 9 Benson Muite 2022-12-09 13:28:46 UTC
A softlink from license directory to docs directory seems ok.

Given that SUSE has managed to integrate lua_rocks into their
build process, it would be good to do the same for Fedora. It
would also make package maintenance easier.

Comment 10 Gwyn Ciesla 2022-12-09 15:26:09 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/lua-ev