[Tarantool-patches] [PATCH v2 0/2] Enable LuaJIT tests written in C

[Tarantool-patches] [PATCH v2 0/2] Enable LuaJIT tests written in C

[Igor Munkin]

This series prepares the existing testing machinery to run LuaJIT tests
requiring libraries implemented in C and enables the existing ones:
* gh-4427-ffi-sandwich
* lj-flush-on-trace

Branch: https://github.com/tarantool/tarantool/tree/imun/ffi-sandwich

Changes in v2:
* Reworked tests considering Vlad's comments.

Igor Munkin (2):
  luajit: bump new version
  test: adjust luajit-tap testing machinery

 test/CMakeLists.txt | 17 +++++++++++------
 third_party/luajit  |  2 +-
 2 files changed, 12 insertions(+), 7 deletions(-)

-- 
2.25.0

[Tarantool-patches] [PATCH v2 1/2] luajit: bump new version

[Igor Munkin]

- jit: abort trace execution on JIT mode change
- jit: abort trace recording and execution for C API

Signed-off-by: Igor Munkin <imun@tarantool.org>
---

This patch is just a reminder to bump luajit submodule.

 third_party/luajit | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/third_party/luajit b/third_party/luajit
index cf8475858..865a884d4 160000
--- a/third_party/luajit
+++ b/third_party/luajit
@@ -1 +1 @@
-Subproject commit cf8475858861d28d1ea6d4de15e146370b3a24f2
+Subproject commit 865a884d4d7dde8a03812578b7f248cfeb43c5c0
-- 
2.25.0

[Tarantool-patches] [PATCH v2 2/2] test: adjust luajit-tap testing machinery

[Igor Munkin]

This changeset makes possible to run luajit-tap tests requiring
libraries implemented in C:
* symlink to luajit test is created on configuration phase instead of
  build one.
* introduced a CMake function for building shared libraries required for
  luajit tests.

Furthermore this commit enables CMake build for the following luajit-tap
tests:
* gh-4427-ffi-sandwich
* lj-flush-on-trace

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 test/CMakeLists.txt | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 9b5df7dc5..0ae3843e3 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -13,6 +13,13 @@ function(build_module module files)
     endif(TARGET_OS_DARWIN)
 endfunction()
 
+function(build_lualib lib sources)
+    add_library(${lib} SHARED ${sources})
+    set_target_properties(${lib} PROPERTIES PREFIX "")
+    if(TARGET_OS_DARWIN)
+        set_target_properties(${lib} PROPERTIES LINK_FLAGS "-undefined dynamic_lookup")
+    endif(TARGET_OS_DARWIN)
+endfunction()
 
 add_compile_flags("C;CXX"
     "-Wno-unused-parameter")
@@ -21,13 +28,9 @@ if(POLICY CMP0037)
     cmake_policy(SET CMP0037 OLD)
 endif(POLICY CMP0037)
 
-add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/luajit-tap
-    COMMAND ${CMAKE_COMMAND} -E create_symlink
+execute_process(COMMAND ${CMAKE_COMMAND} -E create_symlink
         ${PROJECT_SOURCE_DIR}/third_party/luajit/test
-        ${CMAKE_CURRENT_BINARY_DIR}/luajit-tap
-        COMMENT Create a symlink for luajit test dir to fix out-of-source tests)
-add_custom_target(symlink_luajit_tests ALL
-   DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/luajit-tap)
+        ${CMAKE_CURRENT_BINARY_DIR}/luajit-tap)
 
 add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/small
     COMMAND ${CMAKE_COMMAND} -E create_symlink
@@ -56,6 +59,8 @@ add_subdirectory(app)
 add_subdirectory(app-tap)
 add_subdirectory(box)
 add_subdirectory(unit)
+add_subdirectory(luajit-tap/gh-4427-ffi-sandwich)
+add_subdirectory(luajit-tap/lj-flush-on-trace)
 
 # Move tarantoolctl config
 if (NOT ${PROJECT_BINARY_DIR} STREQUAL ${PROJECT_SOURCE_DIR})
-- 
2.25.0

Re: [Tarantool-patches] [PATCH v2 0/2] Enable LuaJIT tests written in C

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

LGTM. To extent at which I know what is happening here (still low).

Re: [Tarantool-patches] [PATCH v2 0/2] Enable LuaJIT tests written in C

[Igor Munkin]

Vlad,

Thanks a lot for your review! You're always welcome with the
LuaJIT-related questions :)

On 19.04.20, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> LGTM. To extent at which I know what is happening here (still low).

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH v2 2/2] test: adjust luajit-tap testing machinery

[Sergey Ostanevich]

Hi!

LGTM.

Sergos

On 15 апр 03:36, Igor Munkin wrote:
> This changeset makes possible to run luajit-tap tests requiring
> libraries implemented in C:
> * symlink to luajit test is created on configuration phase instead of
>   build one.
> * introduced a CMake function for building shared libraries required for
>   luajit tests.
> 
> Furthermore this commit enables CMake build for the following luajit-tap
> tests:
> * gh-4427-ffi-sandwich
> * lj-flush-on-trace
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  test/CMakeLists.txt | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
> index 9b5df7dc5..0ae3843e3 100644
> --- a/test/CMakeLists.txt
> +++ b/test/CMakeLists.txt
> @@ -13,6 +13,13 @@ function(build_module module files)
>      endif(TARGET_OS_DARWIN)
>  endfunction()
>  
> +function(build_lualib lib sources)
> +    add_library(${lib} SHARED ${sources})
> +    set_target_properties(${lib} PROPERTIES PREFIX "")
> +    if(TARGET_OS_DARWIN)
> +        set_target_properties(${lib} PROPERTIES LINK_FLAGS "-undefined dynamic_lookup")
> +    endif(TARGET_OS_DARWIN)
> +endfunction()
>  
>  add_compile_flags("C;CXX"
>      "-Wno-unused-parameter")
> @@ -21,13 +28,9 @@ if(POLICY CMP0037)
>      cmake_policy(SET CMP0037 OLD)
>  endif(POLICY CMP0037)
>  
> -add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/luajit-tap
> -    COMMAND ${CMAKE_COMMAND} -E create_symlink
> +execute_process(COMMAND ${CMAKE_COMMAND} -E create_symlink
>          ${PROJECT_SOURCE_DIR}/third_party/luajit/test
> -        ${CMAKE_CURRENT_BINARY_DIR}/luajit-tap
> -        COMMENT Create a symlink for luajit test dir to fix out-of-source tests)
> -add_custom_target(symlink_luajit_tests ALL
> -   DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/luajit-tap)
> +        ${CMAKE_CURRENT_BINARY_DIR}/luajit-tap)
>  
>  add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/small
>      COMMAND ${CMAKE_COMMAND} -E create_symlink
> @@ -56,6 +59,8 @@ add_subdirectory(app)
>  add_subdirectory(app-tap)
>  add_subdirectory(box)
>  add_subdirectory(unit)
> +add_subdirectory(luajit-tap/gh-4427-ffi-sandwich)
> +add_subdirectory(luajit-tap/lj-flush-on-trace)
>  
>  # Move tarantoolctl config
>  if (NOT ${PROJECT_BINARY_DIR} STREQUAL ${PROJECT_SOURCE_DIR})
> -- 
> 2.25.0
> 

Re: [Tarantool-patches] [PATCH v2 0/2] Enable LuaJIT tests written in C

[Sergey Ostanevich]

Hi!

Thanks, LGTM.

Sergos

On 15 апр 03:36, Igor Munkin wrote:
> This series prepares the existing testing machinery to run LuaJIT tests
> requiring libraries implemented in C and enables the existing ones:
> * gh-4427-ffi-sandwich
> * lj-flush-on-trace
> 
> Branch: https://github.com/tarantool/tarantool/tree/imun/ffi-sandwich
> 
> Changes in v2:
> * Reworked tests considering Vlad's comments.
> 
> Igor Munkin (2):
>   luajit: bump new version
>   test: adjust luajit-tap testing machinery
> 
>  test/CMakeLists.txt | 17 +++++++++++------
>  third_party/luajit  |  2 +-
>  2 files changed, 12 insertions(+), 7 deletions(-)
> 
> -- 
> 2.25.0
> 

Re: [Tarantool-patches] [PATCH v2 0/2] Enable LuaJIT tests written in C

[Igor Munkin]

Sergos,

Thanks a log for you review!

Added both yours and Vlad's tags.

On 19.04.20, Sergey Ostanevich wrote:
> Hi!
> 
> Thanks, LGTM.
> 
> Sergos
> 
> On 15 апр 03:36, Igor Munkin wrote:
> > This series prepares the existing testing machinery to run LuaJIT tests
> > requiring libraries implemented in C and enables the existing ones:
> > * gh-4427-ffi-sandwich
> > * lj-flush-on-trace
> > 
> > Branch: https://github.com/tarantool/tarantool/tree/imun/ffi-sandwich
> > 
> > Changes in v2:
> > * Reworked tests considering Vlad's comments.
> > 
> > Igor Munkin (2):
> >   luajit: bump new version
> >   test: adjust luajit-tap testing machinery
> > 
> >  test/CMakeLists.txt | 17 +++++++++++------
> >  third_party/luajit  |  2 +-
> >  2 files changed, 12 insertions(+), 7 deletions(-)
> > 
> > -- 
> > 2.25.0
> > 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH v2 0/2] Enable LuaJIT tests written in C

[Igor Munkin]

Kirill,

I have no idea what's wrong with Travis, but Gitlab CI is green. Please
proceed with the series.

On 15.04.20, Igor Munkin wrote:
> This series prepares the existing testing machinery to run LuaJIT tests
> requiring libraries implemented in C and enables the existing ones:
> * gh-4427-ffi-sandwich
> * lj-flush-on-trace
> 
> Branch: https://github.com/tarantool/tarantool/tree/imun/ffi-sandwich
> 
> Changes in v2:
> * Reworked tests considering Vlad's comments.
> 
> Igor Munkin (2):
>   luajit: bump new version
>   test: adjust luajit-tap testing machinery
> 
>  test/CMakeLists.txt | 17 +++++++++++------
>  third_party/luajit  |  2 +-
>  2 files changed, 12 insertions(+), 7 deletions(-)
> 
> -- 
> 2.25.0
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH v2 0/2] Enable LuaJIT tests written in C

[Kirill Yukhin]

Hello,

On 15 апр 03:36, Igor Munkin wrote:
> This series prepares the existing testing machinery to run LuaJIT tests
> requiring libraries implemented in C and enables the existing ones:
> * gh-4427-ffi-sandwich
> * lj-flush-on-trace
> 
> Branch: https://github.com/tarantool/tarantool/tree/imun/ffi-sandwich

I've checked your patch into 1.10, 2.2, 2.3 and master.

--
Regards, Kirill Yukhin

Re: [Tarantool-patches] [PATCH v2 0/2] Enable LuaJIT tests written in C

[Igor Munkin]

There is an masked error in test/CmakeLists.txt that doesn't arise for
in-source build bootstrap. However it occurs when out-of-source build
being bootstrapped. Here is a patch with the diff:

================================================================================

From: Igor Munkin <imun@tarantool.org>
Date: Mon, 20 Apr 2020 11:29:23 +0300
Subject: [PATCH] build: temporary fix for luajit-tap tests cmake

Fixes the regression from 335f80a04a2c3541cc1de8758758d9e1ea683cc6
('test: adjust luajit-tap testing machinery').

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 test/CMakeLists.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 0ae3843e3..37cc099b7 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -59,8 +59,8 @@ add_subdirectory(app)
 add_subdirectory(app-tap)
 add_subdirectory(box)
 add_subdirectory(unit)
-add_subdirectory(luajit-tap/gh-4427-ffi-sandwich)
-add_subdirectory(luajit-tap/lj-flush-on-trace)
+add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/luajit/test/gh-4427-ffi-sandwich ${PROJECT_BINARY_DIR}/third_party/luajit/test/gh-4427-ffi-sandwich)
+add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/luajit/test/lj-flush-on-trace ${PROJECT_BINARY_DIR}/third_party/luajit/test/lj-flush-on-trace)
 
 # Move tarantoolctl config
 if (NOT ${PROJECT_BINARY_DIR} STREQUAL ${PROJECT_SOURCE_DIR})
-- 
2.25.0

================================================================================

The fix is applied to 1.10, 2.2, 2.3 and master branches by Kirill.

On 15.04.20, Igor Munkin wrote:
> This series prepares the existing testing machinery to run LuaJIT tests
> requiring libraries implemented in C and enables the existing ones:
> * gh-4427-ffi-sandwich
> * lj-flush-on-trace
> 
> Branch: https://github.com/tarantool/tarantool/tree/imun/ffi-sandwich
> 
> Changes in v2:
> * Reworked tests considering Vlad's comments.
> 
> Igor Munkin (2):
>   luajit: bump new version
>   test: adjust luajit-tap testing machinery
> 
>  test/CMakeLists.txt | 17 +++++++++++------
>  third_party/luajit  |  2 +-
>  2 files changed, 12 insertions(+), 7 deletions(-)
> 
> -- 
> 2.25.0
> 

-- 
Best regards,
IM