From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Igor Munkin <imun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit 1/3] test: add PUC-Rio Lua 5.1 test suite
Date: Fri, 19 Mar 2021 14:26:37 +0300 [thread overview]
Message-ID: <20210319112637.GC28016@root> (raw)
In-Reply-To: <20210317145538.GB29703@tarantool.org>
Igor,
Thanks for the review!
On 17.03.21, Igor Munkin wrote:
> Sergey,
>
> Thanks for the patch!
>
> On 12.03.21, Sergey Kaplun wrote:
> > This patch adds PUC-Rio Lua 5.1 test suite as a part of the LuaJIT
> > test suite. Source code taken verbatim from
>
> Unfortunately, this is not true. Consider the following:
> | $ md5sum lua5.1-tests.tar.gz
> | b376d315ada7bd9d379ec820d6cc27ed lua5.1-tests.tar.gz
> | <...>
> | $ pwd
> | /lua5.1-tests
> | $ find . -type f | sort | xargs md5sum > ~/vanilla
> | <...>
> | $ pwd
> | /tarantool-luajit/test/PUC-Lua-5.1-tests
> | $ git remote -v
> | origin git@github.com:tarantool/luajit (fetch)
> | origin git@github.com:tarantool/luajit (push)
> | $ git lo -1
> | 19e526e (HEAD) test: add PUC-Rio Lua 5.1 test suite
> | $ find . -type f | sort | xargs md5sum > ~/tarantool
> | $ diff ~/vanilla ~/tarantool
> | 2,3c2,3
> | < 70085c31a4cd70096dea1636e8fb3d9d ./api.lua
> | < b863b536cfb6114ec70d846c2f2c1a5f ./attrib.lua
> | ---
> | > 9e9888c13bce8de45b470370cb6f5d11 ./api.lua
> | > f2e3148cbdacf6bd8a06c1744dea3a16 ./attrib.lua
> | 7,12c7,13
> | < 428b214012380dbb8922b70f8e3a174f ./closure.lua
> | < 580aeb1bf3efd526c8b62efcbb7b7ff2 ./code.lua
> | < f11d5bc5a7809d5d34a4fe0b0a86905b ./constructs.lua
> | < 29c56553a51f06ec14ebd45ea2789eaa ./db.lua
> | < 62ef206224896aac4e5998f78305083a ./errors.lua
> | < 2b2a4688858d7b01ead2d94217705c4f ./etc/ltests.c
> | ---
> | > 2fe3dc8ae11397c08c8343cad6f251ab ./closure.lua
> | > cbeb7f7013161dd2d558a7b1adc77327 ./CMakeLists.txt
> | > d72539840341a07aae459cac9991e41f ./code.lua
> | > 0f0bdf9e2e0cf758e0e76600f7afc700 ./constructs.lua
> | > 596f4340c66fd9f0ff6ecacef955ade0 ./db.lua
> | > a2b968868f2d4b7165f561076639c98b ./errors.lua
> | > 2b901df9ba9024f0279a7bbfeaf47f1f ./etc/ltests.c
> | 15c16
> | < fae357bb29cc0ea5ec380a2678674330 ./files.lua
> | ---
> | > 77d39dbfac5abc7a9dcd8e9595ccf517 ./files.lua
> | 18c19
> | < b0536a009fd46b3c54759ce447883cd8 ./libs/lib1.c
> | ---
> | > 1efd07d7bd4efe1b334cc3e11ce83115 ./libs/lib1.c
> | 20,21c21
> | < e27b1cfafb3a4f60cee764beaa761059 ./libs/lib2.c
> | < aa964093a6483e66e434a5ae22816a24 ./libs/makefile
> | ---
> | > b2ac9185c54826707cce0ab1bbfb1289 ./libs/lib2.c
> | 23,24c23,24
> | < e3c8a3668564606e8387ce695229c24f ./locals.lua
> | < a83fe79ec8c8c21dac7ee4f882344178 ./main.lua
> | ---
> | > e3fdc6da784973cf96d19861f79908b3 ./locals.lua
> | > f99dc272fedc3e9bf21344f06e6fa6b7 ./main.lua
> | 26c26
> | < 41119d8062a9f66b89c30b4ff87a7a1f ./nextvar.lua
> | ---
> | > 52e9e6bbac229d72a46672aff5e84874 ./nextvar.lua
>
> Well, as you mentioned in the cover letter, you removed the trailing
> whitespace. Just for the history, I collected everything to the list:
> * api.lua: 1 occurrence
> * attrib.lua: 1 occurrence
> * closure.lua: 1 occurrence
> * code.lua: 1 occurrence
> * constructs.lua: 2 occurrences
> * db.lua: 4 occurrences
> * errors.lua: 2 occurrences
> * etc/ltests.c: 3 occurrences
> * files.lua: 3 occurrences
> * lib/lib1.c: 2 occurrences + luaL_reg fix
> * lib/lib2.c: luaL_reg fix
> * locals.lua: 2 occurrences
> * main.lua: 5 occurrences
> * nextvar.lua: 2 occurrences
>
> IMHO, this is so neglible, that we can move suite intact, but if it
> makes you happier, I believe we can relax the adopting rule for the case
> with such tiny whitespace changes. At the same time there are also a
> couple of *functional* changes: you adjusted auxiliary C sources
> regarding LuaJIT specifics. This violates the rule we discussed so much.
>
> As you mentioned below, tests still fail after this commit, so please
> move all functional changes to a separate patch.
Yep, moved to the separate patch.
>
> > https://www.lua.org/tests/lua5.1-tests.tar.gz.
> >
> > <lib1.c> and <lib2.c> is slightly modified to be consistent with the
> > current LuaJIT's LuaC API.
> >
> > Some tests may fail after this commit. They will be disabled
> > or adapted in the next patches.
> >
> > Part of tarantool/tarantool#5845
> > Part of tarantool/tarantool#4473
> > ---
> > .luacheckrc | 5 +-
> > test/CMakeLists.txt | 4 +-
> > test/PUC-Lua-5.1-tests/CMakeLists.txt | 89 ++
> > test/PUC-Lua-5.1-tests/README | 41 +
> > test/PUC-Lua-5.1-tests/all.lua | 137 +++
> > test/PUC-Lua-5.1-tests/api.lua | 711 +++++++++++++++
> > test/PUC-Lua-5.1-tests/attrib.lua | 339 ++++++++
> > test/PUC-Lua-5.1-tests/big.lua | 381 ++++++++
> > test/PUC-Lua-5.1-tests/calls.lua | 294 +++++++
> > test/PUC-Lua-5.1-tests/checktable.lua | 77 ++
> > test/PUC-Lua-5.1-tests/closure.lua | 422 +++++++++
> > test/PUC-Lua-5.1-tests/code.lua | 143 +++
> > test/PUC-Lua-5.1-tests/constructs.lua | 240 ++++++
> > test/PUC-Lua-5.1-tests/db.lua | 499 +++++++++++
> > test/PUC-Lua-5.1-tests/errors.lua | 250 ++++++
> > test/PUC-Lua-5.1-tests/etc/ltests.c | 1147 +++++++++++++++++++++++++
> > test/PUC-Lua-5.1-tests/etc/ltests.h | 92 ++
> > test/PUC-Lua-5.1-tests/events.lua | 360 ++++++++
> > test/PUC-Lua-5.1-tests/files.lua | 324 +++++++
> > test/PUC-Lua-5.1-tests/gc.lua | 312 +++++++
> > test/PUC-Lua-5.1-tests/libs/lib1.c | 40 +
> > test/PUC-Lua-5.1-tests/libs/lib11.c | 18 +
> > test/PUC-Lua-5.1-tests/libs/lib2.c | 28 +
> > test/PUC-Lua-5.1-tests/libs/lib21.c | 18 +
> > test/PUC-Lua-5.1-tests/literals.lua | 176 ++++
> > test/PUC-Lua-5.1-tests/locals.lua | 127 +++
> > test/PUC-Lua-5.1-tests/main.lua | 159 ++++
> > test/PUC-Lua-5.1-tests/math.lua | 208 +++++
> > test/PUC-Lua-5.1-tests/nextvar.lua | 396 +++++++++
> > test/PUC-Lua-5.1-tests/pm.lua | 273 ++++++
> > test/PUC-Lua-5.1-tests/sort.lua | 74 ++
> > test/PUC-Lua-5.1-tests/strings.lua | 176 ++++
> > test/PUC-Lua-5.1-tests/vararg.lua | 126 +++
> > test/PUC-Lua-5.1-tests/verybig.lua | 100 +++
> > 34 files changed, 7783 insertions(+), 3 deletions(-)
> > create mode 100644 test/PUC-Lua-5.1-tests/CMakeLists.txt
> > create mode 100644 test/PUC-Lua-5.1-tests/README
> > create mode 100755 test/PUC-Lua-5.1-tests/all.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/api.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/attrib.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/big.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/calls.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/checktable.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/closure.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/code.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/constructs.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/db.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/errors.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/etc/ltests.c
> > create mode 100644 test/PUC-Lua-5.1-tests/etc/ltests.h
> > create mode 100644 test/PUC-Lua-5.1-tests/events.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/files.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/gc.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/libs/lib1.c
> > create mode 100644 test/PUC-Lua-5.1-tests/libs/lib11.c
> > create mode 100644 test/PUC-Lua-5.1-tests/libs/lib2.c
> > create mode 100644 test/PUC-Lua-5.1-tests/libs/lib21.c
> > create mode 100644 test/PUC-Lua-5.1-tests/literals.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/locals.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/main.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/math.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/nextvar.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/pm.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/sort.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/strings.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/vararg.lua
> > create mode 100644 test/PUC-Lua-5.1-tests/verybig.lua
> >
>
> <snipped>
>
> > diff --git a/test/PUC-Lua-5.1-tests/CMakeLists.txt b/test/PUC-Lua-5.1-tests/CMakeLists.txt
> > new file mode 100644
> > index 0000000..08bee36
> > --- /dev/null
> > +++ b/test/PUC-Lua-5.1-tests/CMakeLists.txt
> > @@ -0,0 +1,89 @@
> > +# Test suite that has been added from PUC-Rio Lua 5.1 test archive
> > +# in scope of https://github.com/tarantool/tarantool/issues/4473.
> > +
> > +# See the rationale in the root CMakeLists.txt.
> > +cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
> > +
> > +set(TEST_RUNNER ${CMAKE_CURRENT_SOURCE_DIR}/all.lua)
>
> I guess this variable is excess: there is no special variable for runner
> in LuaJIT-tests/CMakeLists.txt, so let's also follow this practice here.
>
> Moreover, the following part (from here to the line of dashes) should be
> moved to a new CMakeLists.txt in libs subdirectory.
Fixed, thanks.
>
> > +set(LIB_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/libs)
> > +set(TESTLIB_PATH ${CMAKE_CURRENT_BINARY_DIR}/libs)
>
> After moving this part into a separate CMakeLists.txt this variables
> become excess.
Yep, done.
>
> > +
> > +# XXX: -fPIC is required to linking with static library.
> > +if(NOT BUILDMODE STREQUAL "static")
>
> What does block you from building the libs against the static build? I
> don't get the reason and it looks like everything works fine (at least
> on my working station).
Yes, if we don't need linking with library instead dynamic lookup.
Fixed.
>
> > + # Build additional C libraries for tests.
> > + macro(build_lib lib sources)
>
> Consider CMake macro naming convention in LuaJIT.
Yep, done.
>
> > + add_library(${lib} SHARED EXCLUDE_FROM_ALL ${sources})
> > + target_include_directories(${lib} PRIVATE
> > + ${LUAJIT_SOURCE_DIR}
> > + )
> > + set_target_properties(${lib} PROPERTIES
> > + LIBRARY_OUTPUT_DIRECTORY "${TESTLIB_PATH}"
> > + PREFIX ""
> > + )
> > + if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
> > + set_target_properties(${lib} PROPERTIES
> > + LINK_FLAGS "-undefined dynamic_lookup"
> > + )
> > + endif()
> > + target_link_libraries(${lib} PRIVATE libluajit_shared)
>
> This looks to be excess, doesn't it?
Yes, static check gone too.
>
> > + list(APPEND TESTLIBS ${lib})
> > + endmacro()
> > +
> > + build_lib(lib1 ${LIB_SOURCES}/lib1.c)
> > + build_lib(lib11 ${LIB_SOURCES}/lib1.c ${LIB_SOURCES}/lib11.c)
> > + build_lib(lib2 ${LIB_SOURCES}/lib2.c)
> > + build_lib(lib21 ${LIB_SOURCES}/lib2.c ${LIB_SOURCES}/lib21.c)
> > +
> > + set(LIB2COPY "${TESTLIB_PATH}/lib2${CMAKE_SHARED_LIBRARY_SUFFIX}")
> > + set(LIB_COPY "${TESTLIB_PATH}/-lib2${CMAKE_SHARED_LIBRARY_SUFFIX}")
> > +
> > + add_custom_command(
> > + COMMENT "Copping lib2 to -lib2 for PUC-Rio Lua 5.1 tests"
> > + OUTPUT ${LIB_COPY}
> > + DEPENDS ${TESTLIBS}
> > + COMMAND ${CMAKE_COMMAND} -E copy ${LIB2COPY} ${LIB_COPY}
> > + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
> > + )
> > + list(APPEND TESTLIBS ${LIB_COPY})
> > +endif()
> > +
> > +set(CUSTOM_TEST_DIR ${TESTLIB_PATH}/P1)
> > +add_custom_command(
> > + COMMENT "Create directory for PUC-Rio Lua 5.1 tests"
> > + OUTPUT ${CUSTOM_TEST_DIR}
> > + COMMAND ${CMAKE_COMMAND} -E make_directory ${CUSTOM_TEST_DIR}
> > + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
> > +)
>
> --------------------------------------------------------------------------------
>
> > +
> > +set(LUA_PATH "?\;${CMAKE_CURRENT_SOURCE_DIR}/?.lua")
>
> It's worth to mention right here, why did you choose this way instead of
> the "better yet" (and the LUA_INIT problem). I see you referred to the
> well-known issue below, but IMHO it's better to group the rationale
> right here.
OK, fixed.
>
> > +
> > +# TODO: PUC-Rio Lua 5.1 test suite also has special header
> > +# <ltests.h> and <ltests.c> translation unit to check some
> > +# internal behaviour of the Lua implementation (see etc/
> > +# directory). It modifies realloc function to check memory
> > +# consistency and also contains tests for yield in hooks
> > +# and for the Lua C API.
> > +# But, unfortunately, <ltests.c> depends on specific PUC-Rio Lua 5.1
> > +# internal headers and should be adopted for LuaJIT.
>
> Typo: s/adopted/adapted/ or simply use "adjusted" here.
Fixed.
>
> > +
> > +add_custom_target(PUC-Lua-5.1-tests
> > + DEPENDS ${LUAJIT_TEST_BINARY} ${TESTLIBS} ${CUSTOM_TEST_DIR}
> > +)
> > +
> > +add_custom_command(TARGET PUC-Lua-5.1-tests
> > + COMMENT "Running PUC-Rio Lua 5.1 tests"
> > + COMMAND
> > + env
> > + # Tarantool doesn't support LUA_INIT and most likely it
> > + # never will.
> > + # See https://github.com/tarantool/tarantool/issues/5744
> > + # for more info.
> > + # LUA_PATH="${CMAKE_CURRENT_BINARY_DIR}/?.lua\;\;"
> > + # LUA_INIT="package.path='?\;'..package.path"
> > + # So use less preferable way for tests.
> > + LUA_PATH="${LUA_PATH}\;\;"
> > + ${LUAJIT_TEST_COMMAND} ${TEST_RUNNER}
> > + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
> > +)
> > +
>
> Consider the fixup patch on my branch[1] and below. I don't run CI for
> this revision, since the queue is overloaded. I'll push the bump in
> Tarantool repo a bit later.
Thanks for the fixup-patch. I've applied it with some modifications:
* drop LUA_CPATH variable as far as it is redundant
* split changes into two separate patches
* add stripping of "-Wl,--no-undefined" linker option (see rationale
below in the patch
* add comments
Side note: I've fixed missed dot at the end of sentence in a comment, so
the gihtub-ci checked a little bit different commits hashes :).
There is the iterative patch for the first commit:
===================================================================
diff --git a/test/PUC-Lua-5.1-tests/CMakeLists.txt b/test/PUC-Lua-5.1-tests/CMakeLists.txt
index 08bee36..066f668 100644
--- a/test/PUC-Lua-5.1-tests/CMakeLists.txt
+++ b/test/PUC-Lua-5.1-tests/CMakeLists.txt
@@ -1,88 +1,44 @@
# Test suite that has been added from PUC-Rio Lua 5.1 test archive
-# in scope of https://github.com/tarantool/tarantool/issues/4473.
+# in scope of https://github.com/tarantool/tarantool/issues/5845.
# See the rationale in the root CMakeLists.txt.
cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
-set(TEST_RUNNER ${CMAKE_CURRENT_SOURCE_DIR}/all.lua)
-set(LIB_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/libs)
-set(TESTLIB_PATH ${CMAKE_CURRENT_BINARY_DIR}/libs)
-
-# XXX: -fPIC is required to linking with static library.
-if(NOT BUILDMODE STREQUAL "static")
- # Build additional C libraries for tests.
- macro(build_lib lib sources)
- add_library(${lib} SHARED EXCLUDE_FROM_ALL ${sources})
- target_include_directories(${lib} PRIVATE
- ${LUAJIT_SOURCE_DIR}
- )
- set_target_properties(${lib} PROPERTIES
- LIBRARY_OUTPUT_DIRECTORY "${TESTLIB_PATH}"
- PREFIX ""
- )
- if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
- set_target_properties(${lib} PROPERTIES
- LINK_FLAGS "-undefined dynamic_lookup"
- )
- endif()
- target_link_libraries(${lib} PRIVATE libluajit_shared)
- list(APPEND TESTLIBS ${lib})
- endmacro()
-
- build_lib(lib1 ${LIB_SOURCES}/lib1.c)
- build_lib(lib11 ${LIB_SOURCES}/lib1.c ${LIB_SOURCES}/lib11.c)
- build_lib(lib2 ${LIB_SOURCES}/lib2.c)
- build_lib(lib21 ${LIB_SOURCES}/lib2.c ${LIB_SOURCES}/lib21.c)
-
- set(LIB2COPY "${TESTLIB_PATH}/lib2${CMAKE_SHARED_LIBRARY_SUFFIX}")
- set(LIB_COPY "${TESTLIB_PATH}/-lib2${CMAKE_SHARED_LIBRARY_SUFFIX}")
-
- add_custom_command(
- COMMENT "Copping lib2 to -lib2 for PUC-Rio Lua 5.1 tests"
- OUTPUT ${LIB_COPY}
- DEPENDS ${TESTLIBS}
- COMMAND ${CMAKE_COMMAND} -E copy ${LIB2COPY} ${LIB_COPY}
- WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
- )
- list(APPEND TESTLIBS ${LIB_COPY})
-endif()
-
-set(CUSTOM_TEST_DIR ${TESTLIB_PATH}/P1)
-add_custom_command(
- COMMENT "Create directory for PUC-Rio Lua 5.1 tests"
- OUTPUT ${CUSTOM_TEST_DIR}
- COMMAND ${CMAKE_COMMAND} -E make_directory ${CUSTOM_TEST_DIR}
- WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
-)
-
+# XXX: There are two ways to set up the proper environment
+# described in the suite's README:
+# * set LUA_PATH to "?;./?.lua"
+# * or, better yet, set LUA_PATH to "./?.lua;;" and LUA_INIT to
+# "package.path = '?;'..package.path"
+# Unfortunately, Tarantool doesn't support LUA_INIT and most
+# likely it never will. For more info, see
+# https://github.com/tarantool/tarantool/issues/5744.
+# Hence, there is no way other than set LUA_PATH environment
+# variable as proposed in the first case.
set(LUA_PATH "?\;${CMAKE_CURRENT_SOURCE_DIR}/?.lua")
+# Set PUC-Lua-5.1-tests-prepare target that creates <libs/P1>
+# subdirectory.
+add_subdirectory(libs)
+
# TODO: PUC-Rio Lua 5.1 test suite also has special header
# <ltests.h> and <ltests.c> translation unit to check some
# internal behaviour of the Lua implementation (see etc/
# directory). It modifies realloc function to check memory
# consistency and also contains tests for yield in hooks
# and for the Lua C API.
-# But, unfortunately, <ltests.c> depends on specific PUC-Rio Lua 5.1
-# internal headers and should be adopted for LuaJIT.
+# But, unfortunately, <ltests.c> depends on specific PUC-Rio
+# Lua 5.1 internal headers and should be adapted for LuaJIT.
add_custom_target(PUC-Lua-5.1-tests
- DEPENDS ${LUAJIT_TEST_BINARY} ${TESTLIBS} ${CUSTOM_TEST_DIR}
+ DEPENDS ${LUAJIT_TEST_BINARY} PUC-Lua-5.1-tests-prepare
)
add_custom_command(TARGET PUC-Lua-5.1-tests
COMMENT "Running PUC-Rio Lua 5.1 tests"
COMMAND
env
- # Tarantool doesn't support LUA_INIT and most likely it
- # never will.
- # See https://github.com/tarantool/tarantool/issues/5744
- # for more info.
- # LUA_PATH="${CMAKE_CURRENT_BINARY_DIR}/?.lua\;\;"
- # LUA_INIT="package.path='?\;'..package.path"
- # So use less preferable way for tests.
LUA_PATH="${LUA_PATH}\;\;"
- ${LUAJIT_TEST_COMMAND} ${TEST_RUNNER}
+ ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/all.lua
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
)
diff --git a/test/PUC-Lua-5.1-tests/libs/CMakeLists.txt b/test/PUC-Lua-5.1-tests/libs/CMakeLists.txt
new file mode 100644
index 0000000..f24e7f3
--- /dev/null
+++ b/test/PUC-Lua-5.1-tests/libs/CMakeLists.txt
@@ -0,0 +1,18 @@
+# Test suite that has been added from PUC-Rio Lua 5.1 test archive
+# in scope of https://github.com/tarantool/tarantool/issues/5845.
+
+# See the rationale in the root CMakeLists.txt.
+cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
+
+# The original tarball contains subdirectory "libs" with an empty
+# subdirectory "libs/P1", to be used by tests.
+# Instead of tracking empty directory with some anchor-file for
+# git, create this directory via CMake.
+add_custom_target(PUC-Lua-5.1-tests-prepare)
+add_custom_command(TARGET PUC-Lua-5.1-tests-prepare
+ COMMENT "Create directory for PUC-Rio Lua 5.1 tests"
+ COMMAND ${CMAKE_COMMAND} -E make_directory P1
+ WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+)
+
+# vim: expandtab tabstop=2 shiftwidth=2
diff --git a/test/PUC-Lua-5.1-tests/libs/lib1.c b/test/PUC-Lua-5.1-tests/libs/lib1.c
index 22fe6de..812bb9a 100644
--- a/test/PUC-Lua-5.1-tests/libs/lib1.c
+++ b/test/PUC-Lua-5.1-tests/libs/lib1.c
@@ -14,7 +14,7 @@ static int id (lua_State *L) {
}
-static const struct luaL_Reg funcs[] = {
+static const struct luaL_reg funcs[] = {
{"id", id},
{NULL, NULL}
};
diff --git a/test/PUC-Lua-5.1-tests/libs/lib2.c b/test/PUC-Lua-5.1-tests/libs/lib2.c
index 876a212..9972cbe 100644
--- a/test/PUC-Lua-5.1-tests/libs/lib2.c
+++ b/test/PUC-Lua-5.1-tests/libs/lib2.c
@@ -12,7 +12,7 @@ static int id (lua_State *L) {
}
-static const struct luaL_Reg funcs[] = {
+static const struct luaL_reg funcs[] = {
{"id", id},
{NULL, NULL}
};
===================================================================
And here is the second patch to add lib\d+ compilation and tests.
===================================================================
commit 54f575819a7790abc932b992e2ff13c3e6177d81
Author: Sergey Kaplun <skaplun@tarantool.org>
Date: Wed Mar 17 19:28:47 2021 +0300
test: add compiling for C libs from PUC-Rio-Lua5.1
This patch adds commands to create additional LuaC libraries for tests
in <attrib.lua>. Also, it renames `luaL_reg` to `luaL_Reg` in <lib1.c>
and <lib2.c> to be consistent with the current LuaJIT's LuaC API.
Part of tarantool/tarantool#5845
Part of tarantool/tarantool#4473
diff --git a/test/PUC-Lua-5.1-tests/CMakeLists.txt b/test/PUC-Lua-5.1-tests/CMakeLists.txt
index 066f668..2015936 100644
--- a/test/PUC-Lua-5.1-tests/CMakeLists.txt
+++ b/test/PUC-Lua-5.1-tests/CMakeLists.txt
@@ -16,7 +16,8 @@ cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
# variable as proposed in the first case.
set(LUA_PATH "?\;${CMAKE_CURRENT_SOURCE_DIR}/?.lua")
-# Set PUC-Lua-5.1-tests-prepare target that creates <libs/P1>
+# Set PUC-Lua-5.1-tests-prepare target that contains rules
+# for <libs/*> libraries compilation and creates <libs/P1>
# subdirectory.
add_subdirectory(libs)
diff --git a/test/PUC-Lua-5.1-tests/libs/CMakeLists.txt b/test/PUC-Lua-5.1-tests/libs/CMakeLists.txt
index f24e7f3..aa64a44 100644
--- a/test/PUC-Lua-5.1-tests/libs/CMakeLists.txt
+++ b/test/PUC-Lua-5.1-tests/libs/CMakeLists.txt
@@ -4,11 +4,57 @@
# See the rationale in the root CMakeLists.txt.
cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
+# Build additional C libraries for tests.
+macro(BuildTestCLib lib sources)
+ add_library(${lib} SHARED EXCLUDE_FROM_ALL ${sources})
+ target_include_directories(${lib} PRIVATE
+ ${LUAJIT_SOURCE_DIR}
+ )
+ set_target_properties(${lib} PROPERTIES
+ LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
+ PREFIX ""
+ )
+ # XXX: The dynamic libraries are loaded with LuaJIT binary and
+ # use symbols from it. So it is totally OK to have unresolved
+ # symbols at build time.
+ if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
+ set_target_properties(${lib} PROPERTIES
+ LINK_FLAGS "-undefined dynamic_lookup"
+ )
+ elseif(CMAKE_SYSTEM_NAME STREQUAL "Linux")
+ # XXX: This is necessary mostly for openSUSE builds, see also
+ # https://bugzilla.suse.com/show_bug.cgi?id=1012388.
+ # Just strip out the linker flag to suppress this linker
+ # option.
+ string(REPLACE "-Wl,--no-undefined" ""
+ CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}"
+ )
+ endif()
+ list(APPEND TESTLIBS ${lib})
+endmacro()
+
+BuildTestCLib(lib1 lib1.c)
+BuildTestCLib(lib11 lib1.c lib11.c)
+BuildTestCLib(lib2 lib2.c)
+BuildTestCLib(lib21 lib2.c lib21.c)
+
+# Create exact copy of the lib2 library for tests in attrib.lua.
+set(LIB2ORIG "${CMAKE_CURRENT_BINARY_DIR}/lib2${CMAKE_SHARED_LIBRARY_SUFFIX}")
+set(LIB2COPY "${CMAKE_CURRENT_BINARY_DIR}/-lib2${CMAKE_SHARED_LIBRARY_SUFFIX}")
+add_custom_command(
+ OUTPUT ${LIB2COPY}
+ COMMENT "Copying lib2 to -lib2 for PUC-Rio Lua 5.1 tests"
+ COMMAND ${CMAKE_COMMAND} -E copy ${LIB2ORIG} ${LIB2COPY}
+ DEPENDS ${TESTLIBS}
+ WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+)
+list(APPEND TESTLIBS ${LIB2COPY})
+
# The original tarball contains subdirectory "libs" with an empty
# subdirectory "libs/P1", to be used by tests.
# Instead of tracking empty directory with some anchor-file for
# git, create this directory via CMake.
-add_custom_target(PUC-Lua-5.1-tests-prepare)
+add_custom_target(PUC-Lua-5.1-tests-prepare DEPENDS ${TESTLIBS})
add_custom_command(TARGET PUC-Lua-5.1-tests-prepare
COMMENT "Create directory for PUC-Rio Lua 5.1 tests"
COMMAND ${CMAKE_COMMAND} -E make_directory P1
diff --git a/test/PUC-Lua-5.1-tests/libs/lib1.c b/test/PUC-Lua-5.1-tests/libs/lib1.c
index 812bb9a..22fe6de 100644
--- a/test/PUC-Lua-5.1-tests/libs/lib1.c
+++ b/test/PUC-Lua-5.1-tests/libs/lib1.c
@@ -14,7 +14,7 @@ static int id (lua_State *L) {
}
-static const struct luaL_reg funcs[] = {
+static const struct luaL_Reg funcs[] = {
{"id", id},
{NULL, NULL}
};
diff --git a/test/PUC-Lua-5.1-tests/libs/lib2.c b/test/PUC-Lua-5.1-tests/libs/lib2.c
index 9972cbe..876a212 100644
--- a/test/PUC-Lua-5.1-tests/libs/lib2.c
+++ b/test/PUC-Lua-5.1-tests/libs/lib2.c
@@ -12,7 +12,7 @@ static int id (lua_State *L) {
}
-static const struct luaL_reg funcs[] = {
+static const struct luaL_Reg funcs[] = {
{"id", id},
{NULL, NULL}
};
===================================================================
>
> ================================================================================
<snipped>
> ================================================================================
>
> > +# vim: expandtab tabstop=2 shiftwidth=2
>
> <snipped>
>
> > --
> > 2.28.0
> >
>
> [1]: https://github.com/tarantool/luajit/commit/c0c53e8
>
> --
> Best regards,
> IM
--
Best regards,
Sergey Kaplun
next prev parent reply other threads:[~2021-03-19 11:27 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-12 5:36 [Tarantool-patches] [PATCH luajit 0/3] Adapt " Sergey Kaplun via Tarantool-patches
2021-03-12 5:36 ` [Tarantool-patches] [PATCH luajit 1/3] test: add " Sergey Kaplun via Tarantool-patches
2021-03-16 13:35 ` Sergey Ostanevich via Tarantool-patches
2021-03-17 15:17 ` Igor Munkin via Tarantool-patches
2021-03-19 11:30 ` Sergey Kaplun via Tarantool-patches
2021-03-17 14:55 ` Igor Munkin via Tarantool-patches
2021-03-19 11:26 ` Sergey Kaplun via Tarantool-patches [this message]
2021-03-12 5:36 ` [Tarantool-patches] [PATCH luajit 2/3] test: adapt PUC-Rio Lua 5.1 test suite for LuaJIT Sergey Kaplun via Tarantool-patches
2021-03-13 19:04 ` Sergey Ostanevich via Tarantool-patches
2021-03-22 18:52 ` Igor Munkin via Tarantool-patches
2021-03-26 8:26 ` Sergey Kaplun via Tarantool-patches
2021-03-12 5:36 ` [Tarantool-patches] [PATCH luajit 3/3] test: adapt Lua 5.1 test suite for Tarantool Sergey Kaplun via Tarantool-patches
2021-03-13 19:07 ` Sergey Ostanevich via Tarantool-patches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210319112637.GC28016@root \
--to=tarantool-patches@dev.tarantool.org \
--cc=imun@tarantool.org \
--cc=skaplun@tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH luajit 1/3] test: add PUC-Rio Lua 5.1 test suite' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox