[
https://issues.apache.org/jira/browse/THRIFT-6189?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6189.
--------------------------------
Fix Version/s: 0.26.0
Assignee: Jens Geyer
Resolution: Fixed
> 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
> Assignee: Jens Geyer
> Priority: Major
> Labels: Breaking-Change
> Fix For: 0.26.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> 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.
> _Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens
> Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
