From d7dd3f79dad266b371b590e8e573f260affabe97 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Mon, 9 Dec 2024 11:42:00 +0300 Subject: [PATCH 1/5] cmake: remove arguments in endif() The arguments to `else()` and `endif()` were required before version 2.6.0. As of CMake 2.6.0 the `else()` and `endif()` constructs can be empty. The same is true for closing constructs on `endmacro()`, `endfunction()`, and `endforeach()`. If you require 2.4.x compatibility, CMake 2.4.3 or greater recognizes the `CMAKE_ALLOW_LOOSE_LOOP_CONSTRUCTS` option (which is superfluous in 2.6.0). See [1] and [2]. The patch removes arguments in `endif()`. 1. https://cmake.org/cmake/help/v3.6/command/endif.html 2. https://stackoverflow.com/questions/29959126/why-does-cmake-syntax-have-redundant-parentheses-everywhere/29967305#29967305 --- CMakeLists.txt | 4 ++-- cmake/CodeCoverage.cmake | 4 ++-- libluamut/CMakeLists.txt | 2 +- tests/CMakeLists.txt | 2 +- tests/capi/CMakeLists.txt | 12 ++++++------ 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 18a14cb0..1a5915b0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -34,7 +34,7 @@ include(SetHardwareArch) if (ENABLE_LUAJIT_RANDOM_RA AND NOT USE_LUAJIT) message(FATAL_ERROR "Option ENABLE_LUAJIT_RANDOM_RA is LuaJIT-specific.") -endif (ENABLE_LUAJIT_RANDOM_RA AND NOT USE_LUAJIT) +endif() if (USE_LUA AND NOT LUA_VERSION) set(LUA_VERSION "master") @@ -70,7 +70,7 @@ endif() find_package(Protobuf) if (NOT Protobuf_FOUND) set(ENABLE_BUILD_PROTOBUF ON) -endif (NOT Protobuf_FOUND) +endif() SetBuildParallelLevel(CMAKE_BUILD_PARALLEL_LEVEL) diff --git a/cmake/CodeCoverage.cmake b/cmake/CodeCoverage.cmake index d5fce4a4..919adf05 100644 --- a/cmake/CodeCoverage.cmake +++ b/cmake/CodeCoverage.cmake @@ -42,7 +42,7 @@ if(USE_LUAJIT) # Exclude buildvm source code, it's a project's build infrastructure. set(GCOVR_OPTIONS ${GCOVR_OPTIONS} --exclude ".*/host/") set(GCOVR_OPTIONS ${GCOVR_OPTIONS} --object-directory ${LUA_SOURCE_DIR}/src) -endif (USE_LUAJIT) +endif () file(MAKE_DIRECTORY ${COVERAGE_DIR}) add_custom_target(${target_name}) @@ -61,7 +61,7 @@ if(USE_LUAJIT) # CMake cannot remove recursively by globbing. # Files 'src/host/*.gcda' are not used for building coverage report. set(GCDA_FILES "${LUA_SOURCE_DIR}/src/*.gcda") -endif(USE_LUAJIT) +endif() add_custom_target(coverage-reset COMMENT "Reset code coverage counters" COMMAND ${CMAKE_COMMAND} -E rm -f ${GCDA_FILES} diff --git a/libluamut/CMakeLists.txt b/libluamut/CMakeLists.txt index 0b6888e7..b93f19f5 100644 --- a/libluamut/CMakeLists.txt +++ b/libluamut/CMakeLists.txt @@ -5,7 +5,7 @@ if (ENABLE_COV) -fcoverage-mapping -ftest-coverage) set(LDFLAGS ${LDFLAGS} -fprofile-instr-generate -fprofile-arcs -fcoverage-mapping -ftest-coverage) -endif (ENABLE_COV) +endif() set(LIB_LUA_MUTATE lua_mutate) add_library(${LIB_LUA_MUTATE} STATIC mutate.c) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index ada0fcc3..9dd70ade 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,4 +1,4 @@ add_subdirectory(capi) if (ENABLE_BONUS_TESTS) add_subdirectory(lapi) -endif (ENABLE_BONUS_TESTS) +endif() diff --git a/tests/capi/CMakeLists.txt b/tests/capi/CMakeLists.txt index cb24ef14..a73e0c03 100644 --- a/tests/capi/CMakeLists.txt +++ b/tests/capi/CMakeLists.txt @@ -29,19 +29,19 @@ target_link_libraries( if (CMAKE_BUILD_TYPE STREQUAL "Debug") set(LDFLAGS "${LDFLAGS} ${CMAKE_C_FLAGS_DEBUG}") -endif (CMAKE_BUILD_TYPE STREQUAL "Debug") +endif() if (ENABLE_ASAN) set(LDFLAGS "${LDFLAGS} -fsanitize=address") -endif (ENABLE_ASAN) +endif() if (ENABLE_UBSAN) set(LDFLAGS "${LDFLAGS} -fsanitize=undefined") -endif (ENABLE_UBSAN) +endif() if (ENABLE_COV) set(LDFLAGS "${LDFLAGS} -fprofile-instr-generate -fcoverage-mapping") -endif (ENABLE_COV) +endif() set(DEFAULT_RUNS_NUMBER 5) @@ -96,14 +96,14 @@ function(create_test) set_tests_properties(${test_name} PROPERTIES ENVIRONMENT "ASAN_OPTIONS='detect_invalid_pointer_pairs=2'" ) - endif (USE_LUA) + endif() set_tests_properties(${test_name} PROPERTIES LABELS capi ) if (USE_LUAJIT) target_compile_definitions(${test_name} PUBLIC LUAJIT) - endif (USE_LUAJIT) + endif() endfunction() # These Lua C functions are unsupported by LuaJIT. From a14b1a52793767cbe0603431b32ef5aa37720779 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Fri, 29 Nov 2024 16:45:55 +0300 Subject: [PATCH 2/5] cmake: get rid of LUA_TARGET Required for setting a Lua library outside. --- cmake/BuildLua.cmake | 11 ++++++++--- cmake/BuildLuaJIT.cmake | 12 ++++++++---- libluamut/CMakeLists.txt | 4 ++-- tests/capi/CMakeLists.txt | 2 +- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/cmake/BuildLua.cmake b/cmake/BuildLua.cmake index 40a15104..8d275f15 100644 --- a/cmake/BuildLua.cmake +++ b/cmake/BuildLua.cmake @@ -59,7 +59,7 @@ macro(build_lua LUA_VERSION) include(ExternalProject) - set(LUA_LIBRARIES ${PROJECT_BINARY_DIR}/lua-${LUA_VERSION}/source/liblua.a) + set(LUA_LIBRARY ${PROJECT_BINARY_DIR}/lua-${LUA_VERSION}/source/liblua.a) set(LUA_EXECUTABLE ${LUA_SOURCE_DIR}/lua) ExternalProject_Add(patched-lua-${LUA_VERSION} @@ -82,12 +82,17 @@ macro(build_lua LUA_VERSION) LDFLAGS=${LDFLAGS} INSTALL_COMMAND "" - BUILD_BYPRODUCTS ${LUA_LIBRARIES} ${LUA_EXECUTABLE} + BUILD_BYPRODUCTS ${LUA_LIBRARY} ${LUA_EXECUTABLE} ) + add_library(bundled-liblua STATIC IMPORTED GLOBAL) + set_target_properties(bundled-liblua PROPERTIES + IMPORTED_LOCATION ${LUA_LIBRARY}) + add_dependencies(bundled-liblua patched-lua-${LUA_VERSION}) + + set(LUA_LIBRARIES bundled-liblua) set(LUA_INCLUDE_DIR ${PROJECT_BINARY_DIR}/lua-${LUA_VERSION}/source/) set(LUA_VERSION_STRING "PUC Rio Lua ${LUA_VERSION}") - set(LUA_TARGET patched-lua-${LUA_VERSION}) unset(LUA_BINARY_DIR) unset(LUA_PATCH_PATH) diff --git a/cmake/BuildLuaJIT.cmake b/cmake/BuildLuaJIT.cmake index 62d054e2..1e2419d1 100644 --- a/cmake/BuildLuaJIT.cmake +++ b/cmake/BuildLuaJIT.cmake @@ -83,7 +83,7 @@ macro(build_luajit LJ_VERSION) include(ExternalProject) - set(LUA_LIBRARIES ${LJ_SOURCE_DIR}/src/libluajit.a) + set(LUA_LIBRARY ${LJ_SOURCE_DIR}/src/libluajit.a) set(LUA_EXECUTABLE ${LJ_SOURCE_DIR}/src/luajit) ExternalProject_Add(patched-luajit-${LJ_VERSION} @@ -110,13 +110,17 @@ macro(build_luajit LJ_VERSION) -C src INSTALL_COMMAND "" - BUILD_BYPRODUCTS ${LUA_LIBRARIES} ${LUA_EXECUTABLE} + BUILD_BYPRODUCTS ${LUA_LIBRARY} ${LUA_EXECUTABLE} ) - set(LUA_SOURCE_DIR ${LJ_SOURCE_DIR}) + add_library(bundled-liblua STATIC IMPORTED GLOBAL) + set_target_properties(bundled-liblua PROPERTIES + IMPORTED_LOCATION ${LUA_LIBRARY}) + add_dependencies(bundled-liblua patched-luajit-${LJ_VERSION}) + + set(LUA_LIBRARIES bundled-liblua) set(LUA_INCLUDE_DIR ${LJ_SOURCE_DIR}/src/) set(LUA_VERSION_STRING "LuaJIT ${LJ_VERSION}") - set(LUA_TARGET patched-luajit-${LJ_VERSION}) unset(LJ_SOURCE_DIR) unset(LJ_BINARY_DIR) diff --git a/libluamut/CMakeLists.txt b/libluamut/CMakeLists.txt index b93f19f5..2c57df6f 100644 --- a/libluamut/CMakeLists.txt +++ b/libluamut/CMakeLists.txt @@ -12,14 +12,14 @@ add_library(${LIB_LUA_MUTATE} STATIC mutate.c) target_link_libraries(${LIB_LUA_MUTATE} PRIVATE ${LUA_LIBRARIES} ${LDFLAGS}) target_include_directories(${LIB_LUA_MUTATE} PRIVATE ${LUA_INCLUDE_DIR}) target_compile_options(${LIB_LUA_MUTATE} PRIVATE ${CFLAGS}) -add_dependencies(${LIB_LUA_MUTATE} ${LUA_TARGET}) +add_dependencies(${LIB_LUA_MUTATE} ${LUA_LIBRARIES}) set(LIB_LUA_CROSSOVER lua_crossover) add_library(${LIB_LUA_CROSSOVER} STATIC crossover.c) target_link_libraries(${LIB_LUA_CROSSOVER} PRIVATE ${LUA_LIBRARIES} ${LDFLAGS}) target_include_directories(${LIB_LUA_CROSSOVER} PRIVATE ${LUA_INCLUDE_DIR}) target_compile_options(${LIB_LUA_CROSSOVER} PRIVATE ${CFLAGS}) -add_dependencies(${LIB_LUA_CROSSOVER} ${LUA_TARGET}) +add_dependencies(${LIB_LUA_CROSSOVER} ${LUA_LIBRARIES}) if (ENABLE_INTERNAL_TESTS) add_subdirectory(tests) diff --git a/tests/capi/CMakeLists.txt b/tests/capi/CMakeLists.txt index a73e0c03..d5b6974e 100644 --- a/tests/capi/CMakeLists.txt +++ b/tests/capi/CMakeLists.txt @@ -71,7 +71,7 @@ function(create_test) target_link_libraries(${test_name} PUBLIC fuzzer_config ${FUZZ_LIBRARIES} ${LUA_LIBRARIES} ${LDFLAGS}) target_include_directories(${test_name} PRIVATE ${LUA_INCLUDE_DIR}) target_compile_options(${test_name} PRIVATE -Wall -Wextra -Wpedantic -Wno-unused-parameter -g) - add_dependencies(${test_name} ${LUA_TARGET}) + add_dependencies(${test_name} ${LUA_LIBRARIES}) string(REPLACE "_test" "" test_prefix ${test_name}) set(LIBFUZZER_OPTS "${LIBFUZZER_OPTS} -artifact_prefix=${test_name}_") if (USE_LUAJIT AND (${test_name} STREQUAL "lua_dump_test")) From d2b823ccfe9ae6ddb177ba68e775e62eb6846236 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Fri, 29 Nov 2024 18:33:45 +0300 Subject: [PATCH 3/5] cmake: introduce IS_LUAJIT The patch introduce a CMake flag `IS_LUAJIT` that should be enabled when passed `LUA_LIBRARIES` and `LUA_INCLUDE_DIR` point to a LuaJIT implementation. Required for setting a Lua library outside. --- CMakeLists.txt | 3 ++- cmake/CodeCoverage.cmake | 4 ++-- tests/capi/CMakeLists.txt | 10 +++++----- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1a5915b0..c95ce19a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -32,7 +32,7 @@ set(CMAKE_INCLUDE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake" ${CMAKE_INCLUDE_PATH} include(SetBuildParallelLevel) include(SetHardwareArch) -if (ENABLE_LUAJIT_RANDOM_RA AND NOT USE_LUAJIT) +if (ENABLE_LUAJIT_RANDOM_RA AND NOT IS_LUAJIT) message(FATAL_ERROR "Option ENABLE_LUAJIT_RANDOM_RA is LuaJIT-specific.") endif() @@ -50,6 +50,7 @@ if (USE_LUA) elseif (USE_LUAJIT) include(BuildLuaJIT) build_luajit(${LUA_VERSION}) + set(IS_LUAJIT TRUE) else () message(FATAL_ERROR "No Lua is specified.") endif () diff --git a/cmake/CodeCoverage.cmake b/cmake/CodeCoverage.cmake index 919adf05..0ec887f4 100644 --- a/cmake/CodeCoverage.cmake +++ b/cmake/CodeCoverage.cmake @@ -36,7 +36,7 @@ if(USE_LUA) set(GCOVR_OPTIONS ${GCOVR_OPTIONS} --object-directory ${LUA_SOURCE_DIR}) endif () -if(USE_LUAJIT) +if(IS_LUAJIT) # Exclude DynASM files, that contain a low-level VM code for CPUs. set(GCOVR_OPTIONS ${GCOVR_OPTIONS} --exclude ".*\.dasc") # Exclude buildvm source code, it's a project's build infrastructure. @@ -56,7 +56,7 @@ add_custom_command(TARGET ${target_name} # object files built with the GCC -fprofile-arcs option is executed. # https://gcc.gnu.org/onlinedocs/gcc/Gcov-Data-Files.html set(GCDA_FILES "${LUA_SOURCE_DIR}/*.gcda") -if(USE_LUAJIT) +if(IS_LUAJIT) # Files 'src/host/*.gcda' are not removed, because # CMake cannot remove recursively by globbing. # Files 'src/host/*.gcda' are not used for building coverage report. diff --git a/tests/capi/CMakeLists.txt b/tests/capi/CMakeLists.txt index d5b6974e..670cfdc0 100644 --- a/tests/capi/CMakeLists.txt +++ b/tests/capi/CMakeLists.txt @@ -74,10 +74,10 @@ function(create_test) add_dependencies(${test_name} ${LUA_LIBRARIES}) string(REPLACE "_test" "" test_prefix ${test_name}) set(LIBFUZZER_OPTS "${LIBFUZZER_OPTS} -artifact_prefix=${test_name}_") - if (USE_LUAJIT AND (${test_name} STREQUAL "lua_dump_test")) + if (IS_LUAJIT AND (${test_name} STREQUAL "lua_dump_test")) set(LIBFUZZER_OPTS "${LIBFUZZER_OPTS} -only_ascii=1") endif () - if (USE_LUAJIT AND (${test_name} STREQUAL "lua_load_test")) + if (IS_LUAJIT AND (${test_name} STREQUAL "lua_load_test")) set(LIBFUZZER_OPTS "${LIBFUZZER_OPTS} -only_ascii=1") endif () set(dict_path ${PROJECT_SOURCE_DIR}/corpus/${test_name}.dict) @@ -101,7 +101,7 @@ function(create_test) LABELS capi ) - if (USE_LUAJIT) + if (IS_LUAJIT) target_compile_definitions(${test_name} PUBLIC LUAJIT) endif() endfunction() @@ -121,7 +121,7 @@ list(APPEND LUAJIT_BLACKLIST_TESTS "luaL_loadstring_test") file(GLOB tests LIST_DIRECTORIES false ${CMAKE_CURRENT_SOURCE_DIR} *.c *.cc) foreach(filename ${tests}) get_filename_component(test_name ${filename} NAME_WE) - if (USE_LUAJIT AND (${test_name} IN_LIST LUAJIT_BLACKLIST_TESTS)) + if (IS_LUAJIT AND (${test_name} IN_LIST LUAJIT_BLACKLIST_TESTS)) continue() endif () if ((${test_name} IN_LIST BLACKLIST_TESTS)) @@ -134,6 +134,6 @@ endforeach() include(ProtobufMutator) add_subdirectory(luaL_loadbuffer_proto) -if(USE_LUAJIT) +if(IS_LUAJIT) add_subdirectory(ffi_cdef_proto) endif () From 962f4b66cdff5a122e7ef5c2837f9f338acccd01 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Wed, 18 Dec 2024 15:24:47 +0300 Subject: [PATCH 4/5] tests: fix leading whitespace in linker flags Building a project as ExternalProject in Tarantool is failed because initial LDFLAGS is empty and after adding an option a final value has a leading whitespace, but should not [1]: CMake Error at tests/capi/CMakeLists.txt:69 (add_executable): Target "ffi_cdef_proto_test" links to item " -g" which has leading or trailing whitespace. This is now an error according to policy CMP0004. Call Stack (most recent call first): tests/capi/ffi_cdef_proto/CMakeLists.txt:16 (create_test) The patch add a macro `AppendFlags` that allow to avoid such issues. Needed by https://github.com/tarantool/tarantool/pull/10911 1. https://cmake.org/cmake/help/latest/policy/CMP0004.html --- cmake/utils.cmake | 10 ++++++++++ tests/capi/CMakeLists.txt | 17 +++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/cmake/utils.cmake b/cmake/utils.cmake index 27669a82..fdf793bc 100644 --- a/cmake/utils.cmake +++ b/cmake/utils.cmake @@ -32,3 +32,13 @@ function(lua_source varname filename symbolname) set(var ${${varname}}) set(${varname} ${var} ${dstfile} PARENT_SCOPE) endfunction() + +macro(AppendFlags flags) + foreach(flag ${ARGN}) + if (${flags}) + set(${flags} "${${flags}} ${flag}") + else() + set(${flags} "${flag}") + endif() + endforeach() +endmacro() diff --git a/tests/capi/CMakeLists.txt b/tests/capi/CMakeLists.txt index 670cfdc0..70b76dee 100644 --- a/tests/capi/CMakeLists.txt +++ b/tests/capi/CMakeLists.txt @@ -27,20 +27,29 @@ target_link_libraries( > ) +# The following condition looks unnecessary, it was added to fix +# an issue for Sydr: in the Lua external project in the build +# system the explicit build command is used: +# BUILD_COMMAND cd && make -j CC=${CMAKE_C_COMPILER} CFLAGS=${CFLAGS} LDFLAGS=${LDFLAGS}, +# which use only flags for Lua build, which were explicitly +# declared earlier, so we need to add `-g` to CFLAGS by ourselves. +# See [1]. +# +# 1. https://github.com/ligurio/lua-c-api-tests/pull/6#discussion_r1185003511 if (CMAKE_BUILD_TYPE STREQUAL "Debug") - set(LDFLAGS "${LDFLAGS} ${CMAKE_C_FLAGS_DEBUG}") + AppendFlags(LDFLAGS ${CMAKE_C_FLAGS_DEBUG}) endif() if (ENABLE_ASAN) - set(LDFLAGS "${LDFLAGS} -fsanitize=address") + AppendFlags(LDFLAGS -fsanitize=address) endif() if (ENABLE_UBSAN) - set(LDFLAGS "${LDFLAGS} -fsanitize=undefined") + AppendFlags(LDFLAGS -fsanitize=undefined) endif() if (ENABLE_COV) - set(LDFLAGS "${LDFLAGS} -fprofile-instr-generate -fcoverage-mapping") + AppendFlags(LDFLAGS -fprofile-instr-generate -fcoverage-mapping) endif() set(DEFAULT_RUNS_NUMBER 5) From 1a7a167276ea4880b501623b6d3b73af6ece5d30 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Fri, 29 Nov 2024 16:13:52 +0300 Subject: [PATCH 5/5] cmake: allow to set a Lua library outside The project builds Lua libraries for two the most popular Lua implementations: PUC Rio Lua and LuaJIT. However, integration with other implementations are interesting as well (like Tarantool, LuaVela, OpenResty etc.). The patch enables integration with other Lua libraries by setting `LUA_INCLUDE_DIR`, `LUA_LIBRARIES` and `LUA_EXECUTABLE` (yes, it is required too). Needed for tarantool/tarantool#10911 --- CMakeLists.txt | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index c95ce19a..5a0cbb4e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -51,6 +51,19 @@ elseif (USE_LUAJIT) include(BuildLuaJIT) build_luajit(${LUA_VERSION}) set(IS_LUAJIT TRUE) +elseif (LUA_INCLUDE_DIR AND LUA_LIBRARIES AND LUA_EXECUTABLE) + message(STATUS "Lua library passed outside:") + message(STATUS "LUA_INCLUDE_DIR: ${LUA_INCLUDE_DIR}") + message(STATUS "LUA_LIBRARIES: ${LUA_LIBRARIES}") + message(STATUS "LUA_EXECUTABLE: ${LUA_EXECUTABLE}") + + # When a path to a Lua library is passed outside, we should + # mimic a real CMake library to don't break code that depends on + # LUA_LIBRARIES. + add_library(bundled-liblua STATIC IMPORTED GLOBAL) + set_target_properties(bundled-liblua PROPERTIES + IMPORTED_LOCATION ${LUA_LIBRARIES}) + set(LUA_LIBRARIES bundled-liblua) else () message(FATAL_ERROR "No Lua is specified.") endif ()