Tarantool development patches archive
 help / color / mirror / Atom feed
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

  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