From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 89FEF9E0365; Wed, 31 Jan 2024 14:48:54 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 89FEF9E0365 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1706701734; bh=+eIiVd9HEnpLX3rbWv126n5qKnjuzDa1w+BemsnpMGY=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=FTutZIgn+J0OnNfj6ipfIML2BCVmKp6YadBkhap7jvzDKbxJb/YvM1pazB9F+kcFG ynuf8Leyi/uq8nYCv2iTo+mRgNZzE1dL5PXlOMReOQ1ktDp+J6c2j5NwkhTFikMT0K WxqccFBsSV6XuNqfpRmD/EKLFM6tiDXJS15cT7gU= Received: from smtp46.i.mail.ru (smtp46.i.mail.ru [95.163.41.84]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 75CD39E0365 for ; Wed, 31 Jan 2024 14:48:53 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 75CD39E0365 Received: by smtp46.i.mail.ru with esmtpa (envelope-from ) id 1rV95I-00000006h40-3OrI; Wed, 31 Jan 2024 14:48:53 +0300 Date: Wed, 31 Jan 2024 14:48:52 +0300 To: Sergey Kaplun Message-ID: References: <5967cedfd0e3e597b34d9b873fba883c7a209518.1706520765.git.skaplun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5967cedfd0e3e597b34d9b873fba883c7a209518.1706520765.git.skaplun@tarantool.org> X-Mailru-Src: smtp X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9DF1FFD52FB7F4A9538A030C200D600810B8213672AABC1EC00894C459B0CD1B9BF0D0A6BB3FF9F167FBBE522E338CBBDB2DFC33A0EF4FB67DF8987F3915C1C364F193C0F926A29E4 X-C1DE0DAB: 0D63561A33F958A563C5342EDE3183705002B1117B3ED69699FA745154224DD3A13BD6A4B0E00B96823CB91A9FED034534781492E4B8EEAD97DCCBFEAAA0BC6ABDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742D3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF2F28128057368030521532233D09F6EF11EB3FA677C83EC9351B2CF6D176A2115B8C288B04B80C1CBAF4CF3B87E910B199BAB91916184F05327EEA726D8C1B0C97C1E0B1DBB0AA7A5F4332CA8FE04980913E6812662D5F2A54F6898A6FDCBDC72A617DFBE5FEC2C6383653B6C8D9AE0FD16FCAA6493B703A X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojqJaWtPsRtykTr27fe8AeJg== X-Mailru-Sender: 11C2EC085EDE56FA38FD4C59F7EFE4075F4CC713AE4CF950B951B70A5BD4BD8E736E831CF0BCD99FD776CA2DFBE07C7904C9FB44FCBCE9EE92D99EB8CC7091A7ECEABDC5717908DEF544888E8238EB4872D6B4FCE48DF648AE208404248635DF X-Mras: OK Subject: Re: [Tarantool-patches] [PATCH v2 luajit 02/26] test: prepare lauxilarily libs for LuaJIT-tests X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the patch! Please consider my comments below. On Mon, Jan 29, 2024 at 01:45:02PM +0300, Sergey Kaplun wrote: > This patch adds rules to build C and C++ libs from the > directory. Also, it allows these libraries to be > loaded via `require()` (since `luaL_openlib()` evolves global state). > The name of "ctest" library is changed to "libctest" to allow it to be > loaded via both `ffi.load()` and `require()`. "cpptest" library is > renamed to "libcpptest" for consistency. > > Part of tarantool/tarantool#9398 > --- > test/LuaJIT-tests/CMakeLists.txt | 29 +++++++++++++++++-- > test/LuaJIT-tests/src/CMakeLists.txt | 17 +++++++++++ > .../src/{cpptest.cpp => libcpptest.cpp} | 4 +-- > test/LuaJIT-tests/src/{ctest.c => libctest.c} | 4 +-- > test/LuaJIT-tests/test.lua | 7 +++-- > 5 files changed, 52 insertions(+), 9 deletions(-) > create mode 100644 test/LuaJIT-tests/src/CMakeLists.txt > rename test/LuaJIT-tests/src/{cpptest.cpp => libcpptest.cpp} (96%) > rename test/LuaJIT-tests/src/{ctest.c => libctest.c} (99%) > > diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt > index 9cd76ee9..c52bcc71 100644 > --- a/test/LuaJIT-tests/CMakeLists.txt > +++ b/test/LuaJIT-tests/CMakeLists.txt > @@ -1,12 +1,35 @@ > # See the rationale in the root CMakeLists.txt > cmake_minimum_required(VERSION 3.1 FATAL_ERROR) > > -add_custom_target(LuaJIT-tests DEPENDS ${LUAJIT_TEST_BINARY}) > +add_subdirectory(src) > + > +add_custom_target(LuaJIT-tests > + DEPENDS ${LUAJIT_TEST_BINARY} LuaJIT-tests-prepare I think something like `LuaJIT-tests-libs` is better name for the target. > +) > + > +make_lua_path(LUA_CPATH > + PATHS > + ${CMAKE_CURRENT_BINARY_DIR}/src/?${CMAKE_SHARED_LIBRARY_SUFFIX} > +) > + > +set(LUAJIT_TESTS_ENV > + "LUA_CPATH=\"${LUA_CPATH}\"" IINM, outer quotes are redundant here. > +) > + > +set(LD_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}/src/:") > + > +if(CMAKE_SYSTEM_NAME STREQUAL "Darwin") > + list(APPEND LUAJIT_TESTS_ENV DYLD_LIBRARY_PATH="${LD_LIBRARY_PATH}") > +else() > + list(APPEND LUAJIT_TESTS_ENV LD_LIBRARY_PATH="${LD_LIBRARY_PATH}") > +endif() > > add_custom_command(TARGET LuaJIT-tests > COMMENT "Running LuaJIT-tests" > COMMAND > - ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/test.lua > - +slow +ffi +bit +jit > + env > + ${LUAJIT_TESTS_ENV} > + ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/test.lua > + +slow +ffi +bit +jit > WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} > ) > diff --git a/test/LuaJIT-tests/src/CMakeLists.txt b/test/LuaJIT-tests/src/CMakeLists.txt > new file mode 100644 > index 00000000..fb14d6d9 > --- /dev/null > +++ b/test/LuaJIT-tests/src/CMakeLists.txt > @@ -0,0 +1,17 @@ > +# See the rationale in the root CMakeLists.txt. > +cmake_minimum_required(VERSION 3.1 FATAL_ERROR) > + > +# Build additional C/C++ libraries for tests. > +macro(BuildTestLib lib sources) > + AddTestLib(${lib} ${sources}) > + list(APPEND TESTLIBS ${lib}) > +endmacro() See the comment about this macro in the previous patch. > + > +# Use `lib` prefix for loading via FFI and `require()`. > +BuildTestLib(libctest libctest.c) > +enable_language(CXX) > +BuildTestLib(libcpptest libcpptest.cpp) > + > +add_custom_target(LuaJIT-tests-prepare DEPENDS ${TESTLIBS}) > + > +# vim: expandtab tabstop=2 shiftwidth=2 Editor configuration should not be there. > diff --git a/test/LuaJIT-tests/src/cpptest.cpp b/test/LuaJIT-tests/src/libcpptest.cpp > similarity index 96% > rename from test/LuaJIT-tests/src/cpptest.cpp > rename to test/LuaJIT-tests/src/libcpptest.cpp > index a5893ed6..cdfb5e32 100644 > --- a/test/LuaJIT-tests/src/cpptest.cpp > +++ b/test/LuaJIT-tests/src/libcpptest.cpp > @@ -121,9 +121,9 @@ static luaL_Reg ct_funcs[] = { > }; > > extern "C" { > -LUA_API int luaopen_cpptest(lua_State *L) > +LUA_API int luaopen_libcpptest(lua_State *L) > { > - luaL_register(L, "cpptest", ct_funcs); > + luaL_register(L, "libcpptest", ct_funcs); > return 1; > } > } > diff --git a/test/LuaJIT-tests/src/ctest.c b/test/LuaJIT-tests/src/libctest.c > similarity index 99% > rename from test/LuaJIT-tests/src/ctest.c > rename to test/LuaJIT-tests/src/libctest.c > index e99f2306..aa95b57b 100644 > --- a/test/LuaJIT-tests/src/ctest.c > +++ b/test/LuaJIT-tests/src/libctest.c > @@ -332,8 +332,8 @@ static luaL_Reg ct_funcs[] = { > {NULL, NULL} > }; > > -LUA_API int luaopen_ctest(lua_State *L) > +LUA_API int luaopen_libctest(lua_State *L) > { > - luaL_register(L, "ctest", ct_funcs); > + luaL_register(L, "libctest", ct_funcs); > return 1; > } > diff --git a/test/LuaJIT-tests/test.lua b/test/LuaJIT-tests/test.lua > index b064eff7..f2450222 100644 > --- a/test/LuaJIT-tests/test.lua > +++ b/test/LuaJIT-tests/test.lua > @@ -297,8 +297,11 @@ local function append_tree_to_plan(test_tree, opts, plan, prefix) > end > > local function seal_globals() > - local sealed_mt = {__newindex = function() > - error("Tests should not mutate global state", 3) > + local sealed_mt = {__newindex = function(_, k) > + -- Allow to load C/C++ libraries for the test. > + if k ~= "libctest" and k ~= "libcpptest" then Let's change the `k` here to a more clear name. > + error("Tests should not mutate global state", 3) > + end > end} > local function seal(t) > if getmetatable(t) then return end > -- > 2.43.0 >