Bug 2143583 - Review Request: lua-ev - Lua integration with libev
Summary: Review Request: lua-ev - Lua integration with libev
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jonny Heggheim
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2022-11-17 10:04 UTC by Benson Muite
Modified: 2023-01-08 20:00 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2023-01-08 20:00:30 UTC
Type: Bug
Embargoed:
hegjon: fedora-review+


Attachments (Terms of Use)

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


Note You need to log in before you can comment on or make changes to this bug.