Jens Geyer created THRIFT-6189:
----------------------------------
Summary: Add a GitHub Actions CI job for the Lua library
Key: THRIFT-6189
URL: https://issues.apache.org/jira/browse/THRIFT-6189
Project: Thrift
Issue Type: New Feature
Components: Build Process, Lua - Library
Reporter: Jens Geyer
No CI job builds or tests the Lua binding.
.github/workflows/build.yml has jobs for php, go, java-kotlin, netstd, haxe,
rust, python, nodejs, cpp and ruby, but none for Lua, and both build.yml (line
32) and sca.yml (line 29) pass --without-lua in their shared
CONFIG_ARGS_FOR_LIBS. The docker.yml workflow only builds and validates the
official images; it runs no language test suite. So a change to lib/lua gets no
automated coverage at all.
lib/lua ships enabled by default -- configure.ac line 276 has
AX_THRIFT_LIB(lua, [Lua], yes) and lib/Makefile.am line 88 has SUBDIRS += lua
-- so this is code that reaches users with no regression coverage behind it.
h3. The historical reason for --without-lua no longer applies
The three ubuntu Dockerfiles carry the comment "need to update our luasocket
code, lua doesn't have luaL_openlib any more". That update happened:
THRIFT-4386 (eb684d348, 2024-07-28) added the version switch, so the C sources
now call luaL_setfuncs on Lua >= 5.2 and luaL_register only below it. The
comment and the --without-lua flags are stale and should go with this change.
h3. Verified in the thrift:jammy image before filing
* All six C sources under lib/lua/src compile clean against Lua 5.4.4.
* ./configure --with-lua reports "Building Lua Library ......... : yes", using
/usr/bin/lua, Lua 5.4.4.
* make -C lib/lua builds all four shared libraries (libluasocket,
liblualongnumber, libluabpack, libluabitwise) without warnings.
* lib/lua/test/test_recursion_depth.lua reports 30 passed, 0 failed, after
generating its stubs.
* test/lua/test_basic_server.lua and test_basic_client.lua run against each
other successfully (client exits 0) over the default buffered/binary
combination.
h3. Most of what a job needs already exists
* build/docker/ubuntu-{jammy,noble}/Dockerfile install lua5.4 and
liblua5.4-dev; ubuntu-focal installs lua5.2 and lua5.2-dev. configure requires
>= 5.2, so all three are usable, and /usr/bin/lua exists as an alternative (the
launchpad bug referenced in the Dockerfile comment affected 5.3, not 5.4).
* configure.ac already locates the interpreter, headers and libraries via
AX_PROG_LUA, AX_LUA_HEADERS and AX_LUA_LIBS.
* test/tests.json already carries a complete lua entry -- client, server,
transports (buffered, framed, http) and protocols (binary, compact, json) -- so
the cross-test wiring exists and has simply never been exercised. Its "TODO:
Add dll to LUA_CPATH" note is also stale; the built libraries load from
../../lib/lua/.libs without further work.
h3. One prerequisite, the same shape as THRIFT-6172 for Dart
lib/lua/Makefile.am defines no check target. "make -C lib/lua check" answers
"Nothing to be done for 'check-am'", which means
lib/lua/test/test_recursion_depth.lua has never been executed by any build
since it was added. A check-local that generates the stubs with $(THRIFT) and
runs the test should land before, or with, the job -- otherwise the job goes
green without running the one test the binding has.
For the same reason the job should assert that configure actually enabled the
binding, for instance by grepping the configure summary for "Building Lua
Library ......... : yes". configure silently falls back to disabled when it
cannot find Lua, so a job that only runs "make check" would pass while testing
nothing.
h3. Worth knowing when writing the job
The Lua server socket's default timeout is 1000, and lib/lua/src/usocket.c
treats it as milliseconds (socket_wait computes timeout/1000 seconds). So
test_basic_server.lua exits with "Timeout" roughly one second after start if
nothing connects. The cross-test runner is fine with this because
test/crossrunner/run.py polls the port every 0.1 s and starts the client as
soon as it is open, using the tests.json "delay" only as an upper bound -- but
a fixed sleep before the client would kill the server first.
Compare THRIFT-6002 (netstd), THRIFT-6003 (haxe), THRIFT-6170 (D), THRIFT-6171
(Erlang) and THRIFT-6172 (Dart), which are the same gap in other bindings.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
