[Tarantool-patches] (no subject)

[Tarantool-patches] (no subject)

[Timur Safin]

Subject: [PATCH] luajit - avoid hardcode of paths to luajit test

We try to avoid hardcode of paths to LuaJIT tests, which
live in the 3rd party repository and might get updated
without our notice.

- to simplify cmake code we introduce symlink inside of `test`
  directory which is now pointing to `third_party/luajit/test`
  subdirectory
- and we use `file(GLOB...)` statement now to propagate list
  of all available test directories we found inside of luajit
  submodule.

Closes #5425
---
 test/CMakeLists.txt | 14 +++++++++++---
 test/luajit         |  1 +
 2 files changed, 12 insertions(+), 3 deletions(-)
 create mode 120000 test/luajit

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 7b28e2bd4..8e36cba6c 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -20,6 +20,16 @@ function(build_lualib lib sources)
     endif(TARGET_OS_DARWIN)
 endfunction()
 
+macro(add_subdirectories base_directory)
+    file(GLOB entries RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} "${base_directory}/*")
+    foreach(entry ${entries})
+        if (IS_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/${entry})
+            add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${entry}
+                             ${PROJECT_BINARY_DIR}/${entry})
+        endif()
+    endforeach()
+endmacro()
+
 add_compile_flags("C;CXX"
     "-Wno-unused-parameter")
 
@@ -75,9 +85,7 @@ add_subdirectory(app-tap)
 add_subdirectory(box)
 add_subdirectory(box-tap)
 add_subdirectory(unit)
-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)
-add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/luajit/test/misclib-getmetrics-capi ${PROJECT_BINARY_DIR}/third_party/luajit/test/misclib-getmetrics-capi)
+add_subdirectories(luajit)
 
 # Move tarantoolctl config
 if (NOT ${PROJECT_BINARY_DIR} STREQUAL ${PROJECT_SOURCE_DIR})
diff --git a/test/luajit b/test/luajit
new file mode 120000
index 000000000..c83b4ed65
--- /dev/null
+++ b/test/luajit
@@ -0,0 +1 @@
+../third_party/luajit/test
\ No newline at end of file
-- 
2.20.1

Re: [Tarantool-patches] (no subject)

[Timur Safin]

Sorry for not subject - will fix it soon.

: -----Original Message-----
: From: Timur Safin <tsafin@tarantool.org>
: Sent: Thursday, October 15, 2020 7:20 PM
: To: imun@tarantool.org; alexander.turenko@tarantool.org
: Cc: Timur Safin <tsafin@tarantool.org>; tarantool-
: patches@dev.tarantool.org
: Subject:
: 
: Subject: [PATCH] luajit - avoid hardcode of paths to luajit test
: 
: We try to avoid hardcode of paths to LuaJIT tests, which
: live in the 3rd party repository and might get updated
: without our notice.
: 
: - to simplify cmake code we introduce symlink inside of `test`
:   directory which is now pointing to `third_party/luajit/test`
:   subdirectory
: - and we use `file(GLOB...)` statement now to propagate list
:   of all available test directories we found inside of luajit
:   submodule.
: 
: Closes #5425
: ---
:  test/CMakeLists.txt | 14 +++++++++++---
:  test/luajit         |  1 +
:  2 files changed, 12 insertions(+), 3 deletions(-)
:  create mode 120000 test/luajit
: 
: diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
: index 7b28e2bd4..8e36cba6c 100644
: --- a/test/CMakeLists.txt
: +++ b/test/CMakeLists.txt
: @@ -20,6 +20,16 @@ function(build_lualib lib sources)
:      endif(TARGET_OS_DARWIN)
:  endfunction()
: 
: +macro(add_subdirectories base_directory)
: +    file(GLOB entries RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
: "${base_directory}/*")
: +    foreach(entry ${entries})
: +        if (IS_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/${entry})
: +            add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${entry}
: +                             ${PROJECT_BINARY_DIR}/${entry})
: +        endif()
: +    endforeach()
: +endmacro()
: +
:  add_compile_flags("C;CXX"
:      "-Wno-unused-parameter")
: 
: @@ -75,9 +85,7 @@ add_subdirectory(app-tap)
:  add_subdirectory(box)
:  add_subdirectory(box-tap)
:  add_subdirectory(unit)
: -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)
: -add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/luajit/test/misclib-
: getmetrics-capi ${PROJECT_BINARY_DIR}/third_party/luajit/test/misclib-
: getmetrics-capi)
: +add_subdirectories(luajit)
: 
:  # Move tarantoolctl config
:  if (NOT ${PROJECT_BINARY_DIR} STREQUAL ${PROJECT_SOURCE_DIR})
: diff --git a/test/luajit b/test/luajit
: new file mode 120000
: index 000000000..c83b4ed65
: --- /dev/null
: +++ b/test/luajit
: @@ -0,0 +1 @@
: +../third_party/luajit/test
: \ No newline at end of file
: --
: 2.20.1

Re: [Tarantool-patches] [PATCH] luajit - avoid hardcode of paths to luajit test

[Igor Munkin]

Timur,

Thanks for your patch! It's worth to mention the fact we are working on
self-sufficient LuaJIT testing[1], so this mess will be gone in a while.

Please, consider the comments regarding your patch are below.

At first, CI is red[2], so unfortunately, the patch doesn't work...

Please adjust the commit subject consdering our guidelines[3]. I propose
the following: s/luajit -/test:/.

On 15.10.20, Timur Safin wrote:
> We try to avoid hardcode of paths to LuaJIT tests, which
> live in the 3rd party repository and might get updated
> without our notice.

Well, your approach definitely simplifies the maintenance, *but* the
patch is not related to th original problem we faced recently.

> 
> - to simplify cmake code we introduce symlink inside of `test`
>   directory which is now pointing to `third_party/luajit/test`
>   subdirectory

I prefer the way the symlink to <small> tests is created. However, IIRC
such approach contradicts with <add_subdirectory> behaviour, since there
is not such path at the proper CMake stage. Feel free to correct me if
I'm wrong. It would be perfect, if the symlink to the test directory
can be created at the configuration step.

There is also another approach: we can add the root CMakeLists.txt in
the LuaJIT test directory to enclose all necessary activity there.
This one look much more CMake-way, doesn't it?

> - and we use `file(GLOB...)` statement now to propagate list
>   of all available test directories we found inside of luajit
>   submodule.
> 
> Closes #5425
> ---
>  test/CMakeLists.txt | 14 +++++++++++---
>  test/luajit         |  1 +
>  2 files changed, 12 insertions(+), 3 deletions(-)
>  create mode 120000 test/luajit
> 

<snipped>

> -- 
> 2.20.1
> 

[1]: https://github.com/tarantool/tarantool/issues/4862
[2]: https://gitlab.com/tarantool/tarantool/-/pipelines/203157106
[3]: https://www.tarantool.io/en/doc/latest/dev_guide/developer_guidelines/#how-to-write-a-commit-message

-- 
Best regards,
IM