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 00E9270310; Mon, 8 Feb 2021 19:29:40 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 00E9270310 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1612801780; bh=+UvRcUe7j7gXJGlk6LvriX5jFGZNZBnJX1JhrRm+NDU=; 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=uULtfBK+fknbIICusSzqNcZQK71BbjqOKgMSYVl4LEFI8mJ0PlWHpED16SK9glP8C +KAsKkt0fmAUWtGYwaCumYItNhfaJxR157upUt2gZHsutKc8JdL2u4qboqO1jfO1c5 TY3zcflFPKXrZHWUbn9T9CTi6pY45xr6Nsf/AWxo= Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 3256970310 for ; Mon, 8 Feb 2021 19:29:37 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 3256970310 Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1l99Pn-0002uG-A0; Mon, 08 Feb 2021 19:29:31 +0300 Date: Mon, 8 Feb 2021 19:29:30 +0300 To: Timur Safin Message-ID: <20210208162930.GF5448@tarantool.org> References: <6a03d693204cacc5791c75e1003efc150abb2979.1612291495.git.imun@tarantool.org> <01d401d6fe2b$dd80c160$98824420$@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <01d401d6fe2b$dd80c160$98824420$@tarantool.org> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.10.1 (2018-07-13) X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD953AC099BC0052A9CAEF2BF42A2A7729330F8028A4C0D8125182A05F5380850402E4D9E400CC6E8018E6DBD8380AEC73A444641C9AFCE74BF6FC8BCA635A822F5 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7FBCED7D376B82B5EEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637BBCE7257090F96C9EA1F7E6F0F101C674E70A05D1297E1BBC6CDE5D1141D2B1C7770272E9536556DAF9568910ADC63885015D58BB0CABC769FA2833FD35BB23D9E625A9149C048EE33AC447995A7AD18C26CFBAC0749D213D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BF1175FABE1C0F9B6A471835C12D1D977C4224003CC836476EC64975D915A344093EC92FD9297F6718AA50765F79006376D49742057570F4AA7F4EDE966BC389F395957E7521B51C24C7702A67D5C33162DBA43225CD8A89F0A35B161A8BF67C1156CCFE7AF13BCA4B5C8C57E37DE458B4C7702A67D5C3316FA3894348FB808DB48C21F01D89DB561574AF45C6390F7469DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A55DB73059CD435B17C6B42A32E67ABB4C3290ACD1C6C9DE97D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75448CF9D3A7B2C848410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34DFB7A809FB537087117F4B5368F72A6D06A5971E938AAD5B30552513F8C7BED056A73C716C5A21D91D7E09C32AA3244CC052F8C76E4B61BC608B15841714BF18B038C9161EF167A1927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojg67HQLCbniJEhW+mdXEecA== X-Mailru-Sender: 689FA8AB762F73936BC43F508A063822EC9FA48B587CE4CD71AD98B6C8C1C4BBA7C8D0F45F857DBFE9F1EFEE2F478337FB559BB5D741EB964C8C2C849690F8E70A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 3/5] test: run LuaJIT tests via CMake 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: Igor Munkin via Tarantool-patches Reply-To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Timur, Thanks for your review! On 08.02.21, Timur Safin wrote: > Some code changes proposals below... > > : From: Igor Munkin > : Subject: [PATCH luajit 3/5] test: run LuaJIT tests via CMake > : > : diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool- > : tests/CMakeLists.txt > : new file mode 100644 > : index 0000000..0be4b34 > : --- /dev/null > : +++ b/test/tarantool-tests/CMakeLists.txt > : @@ -0,0 +1,92 @@ > : +# Test suite that has been moved from Tarantool repository in > : +# scope of https://github.com/tarantool/tarantool/issues/4478. > : + > : +# See the rationale in the root CMakeLists.txt. > : +cmake_minimum_required(VERSION 3.1 FATAL_ERROR) > : + > : +find_program(PROVE prove) > : +if(NOT PROVE) > : + message(WARNING "`prove' is not found, so tarantool-tests target is not > : generated") > : + return() > : +endif() > : + > : +macro(BuildTestLib lib sources) > : + add_library(${lib} SHARED EXCLUDE_FROM_ALL ${sources}) > : + target_include_directories(${lib} PRIVATE > : + ${LUAJIT_SOURCE_DIR} > : + ${CMAKE_CURRENT_SOURCE_DIR} > : + ) > ... > : + # XXX: Append the lib to be built to the dependecy list. > : + # Unfortunately, CMake is a crap and there is no other way to > : + # extend the list in parent scope but join two strings with > : + # semicolon. If one finds the normal way to make it work, feel > : + # free to reach me. > : + set(TESTLIBS "${lib};${TESTLIBS}" PARENT_SCOPE) > > I don't like this. It reminds me of bad examples of this note > in the libev code like "this is so uncontrollably lame" which > actually distract users. We should rather put comments in more > neutral way (IMVHO). Unfortunately... CMake is a crap and I can't fix it. Well, honestly I don't want to fix it. But you're right: I can fix all these comments (athough I don't really want to). I have no idea how to express this in a more neutral way, so I propose the following: 1. s/CMake is a crap/there is no convenient way to make it in CMake/g. 2. Leave everything else unchanged. This is not only emotions. Such comments prevent one from unintentional refactoring of such fragile places with no failures and also from bothering the oldies with the questions kinda "dude, why is this done so badly". In other words, primarily I have left this for the history. If you're OK, then I'll fix the way described above. > > : + # Add the directory where the lib is built to the LUA_CPATH > : + # environment variable, so interpreter can find and load it. > : + # XXX: Here we see the other side of the coin. If one joins two > : + # strings with semicolon, the value automatically becomes the > : + # list. I have no idea what is wrong with this tool, but I found > : + # a single working solution to make LUA_CPATH be a string via > : + # "escaping" the semicolon right in string interpolation. > : + set(LUA_CPATH > : "${CMAKE_CURRENT_BINARY_DIR}/?${CMAKE_SHARED_LIBRARY_SUFFIX}\;${LUA_CPATH}" > : PARENT_SCOPE) > : + # Also add this directory to LD_LIBRARY_PATH environment > : + # variable, so FFI machinery can find and load it. > : + set(LD_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}:${LD_LIBRARY_PATH}" > : PARENT_SCOPE) > : +endmacro() > : + > : +add_subdirectory(gh-4427-ffi-sandwich) > : +add_subdirectory(lj-flush-on-trace) > : +add_subdirectory(misclib-getmetrics-capi) > > I liked you introduced globs for test files addition (blow), > but unfortunately you didn't complete this with subdirectory addition I have thought about it for some time, and I have a strong rationale (at least for me) to not introducing such change: this doesn't work whether there are directories inside containing no tests. Furthermore, we're going to re-implement several C tests, so I would like to leave everything as it is (if you don't mind) and return to this place later. > > Please see my proposed patch (with reworded comments and new macro) here https://gist.github.com/tsafin/6c7505c0c764ab2b474667bf0d65fb45. > > > Regards, > Timur > -- Best regards, IM