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 950AA9E35C3; Tue, 6 Feb 2024 14:28:27 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 950AA9E35C3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1707218907; bh=1CT7nvmsQ7FuAo318YwsvwrKwQP4Knm6ep3rfkKx9ZI=; 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=UO4ya4fbB4w4WgyfQ4nppshM9LOdpApcF4E1+ISYQuxeeyz8bKL1pYBO67yRhOCjT 6+h6cjbS9Frs7H122dOfRGtOceVAgnZt+CxJ8CTWeCruuiYDH78UOoWyqR56N8GfZN Is8DFnBP+rdWvZZWYwwr0058gE9nXoX6l7rY+0Bk= Received: from smtp42.i.mail.ru (smtp42.i.mail.ru [95.163.41.65]) (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 284549E35C2 for ; Tue, 6 Feb 2024 14:27:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 284549E35C2 Received: by smtp42.i.mail.ru with esmtpa (envelope-from ) id 1rXJcL-00000001ven-213S; Tue, 06 Feb 2024 14:27:57 +0300 Date: Tue, 6 Feb 2024 14:24:00 +0300 To: Maxim Kokryashkin 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: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD94C460F083DF69F519C7FE39648913355F55DE976E2925CCD182A05F5380850400399DF67F106CDEF9487ABAC94A94B54A583E7841E95803744A232330862CEEA3B9FC492F4C8D7B4 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7081BBE264C6D7F42EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637F10F1F3256FD32E28638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8F1E5B4412CABD39E5EC8F55D25551BDB7158338FCBACFAB9CC7F00164DA146DAFE8445B8C89999728AA50765F7900637D0FEED2715E18529389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC82FFDA4F57982C5F4F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE33AC447995A7AD182BEBFE083D3B9BA73A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E7352629B07FD02F83A6C4224003CC83647689D4C264860C145E X-C1DE0DAB: 0D63561A33F958A5362D57382705484D5002B1117B3ED696EE7062EF2D483EB61E49B01306B5E3AD823CB91A9FED034534781492E4B8EEADA7AC412AE061D850BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFF9858E8971D4E320A661A4F8EAE9D6DE585C8E1CEF88796B89F593CE0BADF5F4A5CD7B9A90500322807D692F32DC70320B23218E09E4FE20D27B674673BB2DAF7A38AB2D2038560F5F4332CA8FE04980913E6812662D5F2A5EAB5682573093F7837F15F2B5E4A70B33F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojh/iSjD2txnMQ/FJzSqBITQ== X-DA7885C5: 9780F9922ECA740CF255D290C0D534F9233D964992E18CC0C5E477B21CD73901799DD0EE4B9043F45B1A4C17EAA7BC4BEF2421ABFA55128DAF83EF9164C44C7E X-Mailru-Sender: 689FA8AB762F7393590D8C940224AE33E5AD6F12D61A47A52FBE6EBA85DE6F468FC746582E98D4B7E49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F 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: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Maxim! Thanks for the review! Fixed your comments and force-pushed the branch. On 31.01.24, Maxim Kokryashkin wrote: > 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. Renamed. =================================================================== diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt index 8ecfe3f6..2c933a11 100644 --- a/test/LuaJIT-tests/CMakeLists.txt +++ b/test/LuaJIT-tests/CMakeLists.txt @@ -4,7 +4,7 @@ cmake_minimum_required(VERSION 3.1 FATAL_ERROR) add_subdirectory(src) add_custom_target(LuaJIT-tests - DEPENDS ${LUAJIT_TEST_BINARY} LuaJIT-tests-prepare + DEPENDS ${LUAJIT_TEST_BINARY} LuaJIT-tests-libs ) make_lua_path(LUA_CPATH diff --git a/test/LuaJIT-tests/src/CMakeLists.txt b/test/LuaJIT-tests/src/CMakeLists.txt index 0855088f..2f90da86 100644 --- a/test/LuaJIT-tests/src/CMakeLists.txt +++ b/test/LuaJIT-tests/src/CMakeLists.txt @@ -6,4 +6,4 @@ AddTestLib(libctest libctest.c) enable_language(CXX) AddTestLib(libcpptest libcpptest.cpp) -add_custom_target(LuaJIT-tests-prepare DEPENDS ${TESTLIBS}) +add_custom_target(LuaJIT-tests-libs DEPENDS ${TESTLIBS}) =================================================================== > > +) > > + > > +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. Without them, CMake just add ${LUA_CPATH} as a string (without internal quotes). Hence, they are needed. > > +) > > + > > 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. Fixed within the patch below. > > + > > +# 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. Fixed: see the iterative patch below. =================================================================== diff --git a/test/LuaJIT-tests/src/CMakeLists.txt b/test/LuaJIT-tests/src/CMakeLists.txt index fb14d6d9..0855088f 100644 --- a/test/LuaJIT-tests/src/CMakeLists.txt +++ b/test/LuaJIT-tests/src/CMakeLists.txt @@ -1,17 +1,9 @@ # 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() - # Use `lib` prefix for loading via FFI and `require()`. -BuildTestLib(libctest libctest.c) +AddTestLib(libctest libctest.c) enable_language(CXX) -BuildTestLib(libcpptest libcpptest.cpp) +AddTestLib(libcpptest libcpptest.cpp) add_custom_target(LuaJIT-tests-prepare DEPENDS ${TESTLIBS}) - -# vim: expandtab tabstop=2 shiftwidth=2 =================================================================== > > > > > 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. Changed to the `key`: =================================================================== diff --git a/test/LuaJIT-tests/test.lua b/test/LuaJIT-tests/test.lua index f2450222..ab178331 100644 --- a/test/LuaJIT-tests/test.lua +++ b/test/LuaJIT-tests/test.lua @@ -297,9 +297,9 @@ local function append_tree_to_plan(test_tree, opts, plan, prefix) end local function seal_globals() - local sealed_mt = {__newindex = function(_, k) + local sealed_mt = {__newindex = function(_, key) -- Allow to load C/C++ libraries for the test. - if k ~= "libctest" and k ~= "libcpptest" then + if key ~= "libctest" and key ~= "libcpptest" then error("Tests should not mutate global state", 3) end end} =================================================================== > > + error("Tests should not mutate global state", 3) > > + end > > end} > > local function seal(t) > > if getmetatable(t) then return end > > -- > > 2.43.0 > > -- Best regards, Sergey Kaplun