[Tarantool-patches] [PATCH v2 luajit 01/26] cmake: introduce AddTestLib macro

[Sergey Bronnikov]

Hi, Sergey


thanks for the patch! LGTM


On 1/29/24 13:45, Sergey Kaplun wrote:
> This patch removes copy-pasting in macros used to add a new library for
> testing and places this in the include test file.
>
> Needed for tarantool/tarantool#9398
> ---
>   test/CMakeLists.txt                           |  3 ++
>   .../PUC-Rio-Lua-5.1-tests/libs/CMakeLists.txt | 25 +----------------
>   test/cmake/AddTestLib.cmake                   | 28 +++++++++++++++++++
>   test/tarantool-tests/CMakeLists.txt           | 27 +-----------------
>   4 files changed, 33 insertions(+), 50 deletions(-)
>   create mode 100644 test/cmake/AddTestLib.cmake
>
> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
> index 8afc42df..3ad5d15f 100644
> --- a/test/CMakeLists.txt
> +++ b/test/CMakeLists.txt
> @@ -74,6 +74,9 @@ add_custom_target(${PROJECT_NAME}-lint DEPENDS
>   set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]")
>   separate_arguments(LUAJIT_TEST_COMMAND)
>   
> +set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
> +include(AddTestLib)
> +
>   add_subdirectory(LuaJIT-tests)
>   add_subdirectory(PUC-Rio-Lua-5.1-tests)
>   add_subdirectory(lua-Harness-tests)
> diff --git a/test/PUC-Rio-Lua-5.1-tests/libs/CMakeLists.txt b/test/PUC-Rio-Lua-5.1-tests/libs/CMakeLists.txt
> index 8501b767..e68e6aef 100644
> --- a/test/PUC-Rio-Lua-5.1-tests/libs/CMakeLists.txt
> +++ b/test/PUC-Rio-Lua-5.1-tests/libs/CMakeLists.txt
> @@ -6,30 +6,7 @@ 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()
> +  AddTestLib(${lib} ${sources})
>     list(APPEND TESTLIBS ${lib})
>   endmacro()
>   
> diff --git a/test/cmake/AddTestLib.cmake b/test/cmake/AddTestLib.cmake
> new file mode 100644
> index 00000000..3764ee5c
> --- /dev/null
> +++ b/test/cmake/AddTestLib.cmake
> @@ -0,0 +1,28 @@
> +macro(AddTestLib lib sources)
> +  add_library(${lib} SHARED EXCLUDE_FROM_ALL ${sources})
> +  target_include_directories(${lib} PRIVATE
> +    ${LUAJIT_SOURCE_DIR}
> +    ${CMAKE_CURRENT_SOURCE_DIR}
> +  )
> +  set_target_properties(${lib} PROPERTIES
> +    LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
> +    PREFIX ""
> +  )
> +
> +  # XXX: This change affects the current cmake variable scope and
> +  # so a user should care to don't use it in a top level scope.
> +  # 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"
> +    )
> +  else()
> +    # FIXME: Unfortunately, there is no another way to suppress
> +    # this linker option, so just strip it out from the flags.
> +    string(REPLACE "-Wl,--no-undefined" ""
> +      CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}"
> +    )
> +  endif()
> +endmacro()
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 6d686876..80d94a78 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -11,32 +11,7 @@ if(NOT PROVE)
>   endif()
>   
>   macro(BuildTestCLib lib sources)
> -  add_library(${lib} SHARED EXCLUDE_FROM_ALL ${sources})
> -  target_include_directories(${lib} PRIVATE
> -    ${LUAJIT_SOURCE_DIR}
> -    ${CMAKE_CURRENT_SOURCE_DIR}
> -  )
> -  set_target_properties(${lib} PROPERTIES
> -    LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
> -    PREFIX ""
> -  )
> -
> -  # XXX: This change affects the current cmake variable scope and
> -  # so a user should care to don't use it in a top level scope.
> -  # 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"
> -    )
> -  else()
> -    # FIXME: Unfortunately, there is no another way to suppress
> -    # this linker option, so just strip it out from the flags.
> -    string(REPLACE "-Wl,--no-undefined" ""
> -      CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}"
> -    )
> -  endif()
> +  AddTestLib(${lib} ${sources})
>     # XXX: Append the lib to be built to the dependency list.
>     # Unfortunately, there is no convenient way in CMake to extend
>     # the list in parent scope other than join two strings with