lib: lua: fix LUA_VERSION_NUM check #8739
Conversation
The `LUA_VERSION_NUM` macro definition for Lua 5.2 is 502 and not 520. Reference: https://github.com/lua/lua/blob/e2fc2ce8dfe107d1e2742b459c2aaf137227bbc1/lua.h#L21 Signed-off-by: oz-rw <127842933+oz-rw@users.noreply.github.com>
oz-rw
requested review from
edsiper,
leonardo-albertovich,
fujimotos and
koleini
as code owners
|
cc: @tarruda |
| @@ -75,7 +75,7 @@ static inline int flb_lua_metadata_init(struct flb_lua_metadata *meta) | |||
| /* convert from negative index to positive index */ | |||
| static inline int flb_lua_absindex(lua_State *l , int index) | |||
| { | |||
| #if defined(LUA_VERSION_NUM) && LUA_VERSION_NUM < 520 | |||
| #if defined(LUA_VERSION_NUM) && LUA_VERSION_NUM < 502 | |||
There was a problem hiding this comment.
I suggest to just remove this test and assume the true condition.
Fluent-bit uses a vendored version of LuaJit (that has LUA_VERSION_NUM defined as 501) and AFAICT doesn't allow the user to build against a custom version, so it is not useful having this kind of test since we already know its definitions.
Also, the test doesn´t appear to be correct, since if LUA_VERSION_NUM is not defined (which is not in older lua versions) it will fallback to lua_absindex path, which only exists in lua 5.2+
There was a problem hiding this comment.
Fluent-bit uses a vendored version of LuaJit (that has LUA_VERSION_NUM defined as 501) and AFAICT doesn't allow the user to build against a custom version
We have a use for it, as we use Lua5.4 instead of LuaJIT. It was easier to add support for that in the CMake files, than to fixing cross compilation of luajit-cmake under Yocto/BitBake. I opened a discussion about it #8731.
Also, the test doesn´t appear to be correct, since if LUA_VERSION_NUM is not defined (which is not in older lua versions) it will fallback to lua_absindex path, which only exists in lua 5.2+
Then the check could be #if LUA_VERSION_NUM < 502, and it will be interpreted as 0 if not defined.
The
LUA_VERSION_NUMmacro definition for Lua 5.2 is 502 and not 520.Reference:
https://github.com/lua/lua/blob/e2fc2ce8dfe107d1e2742b459c2aaf137227bbc1/lua.h#L21
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.