[jira] [Created] (THRIFT-6189) Add a GitHub Actions CI job for the Lua library

["Jens Geyer (Jira)" <ji...@apache.org>]

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)