From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 38344469719 for ; Thu, 15 Oct 2020 23:52:14 +0300 (MSK) Date: Thu, 15 Oct 2020 23:41:36 +0300 From: Igor Munkin Message-ID: <20201015204136.GI32659@tarantool.org> References: <11457da6c5f5f29b511a809cbc0908b19458d284.1602778378.git.tsafin@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <11457da6c5f5f29b511a809cbc0908b19458d284.1602778378.git.tsafin@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] luajit - avoid hardcode of paths to luajit test List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Timur Safin Cc: tarantool-patches@dev.tarantool.org, alexander.turenko@tarantool.org Timur, Thanks for your patch! It's worth to mention the fact we are working on self-sufficient LuaJIT testing[1], so this mess will be gone in a while. Please, consider the comments regarding your patch are below. At first, CI is red[2], so unfortunately, the patch doesn't work... Please adjust the commit subject consdering our guidelines[3]. I propose the following: s/luajit -/test:/. On 15.10.20, Timur Safin wrote: > We try to avoid hardcode of paths to LuaJIT tests, which > live in the 3rd party repository and might get updated > without our notice. Well, your approach definitely simplifies the maintenance, *but* the patch is not related to th original problem we faced recently. > > - to simplify cmake code we introduce symlink inside of `test` > directory which is now pointing to `third_party/luajit/test` > subdirectory I prefer the way the symlink to tests is created. However, IIRC such approach contradicts with behaviour, since there is not such path at the proper CMake stage. Feel free to correct me if I'm wrong. It would be perfect, if the symlink to the test directory can be created at the configuration step. There is also another approach: we can add the root CMakeLists.txt in the LuaJIT test directory to enclose all necessary activity there. This one look much more CMake-way, doesn't it? > - and we use `file(GLOB...)` statement now to propagate list > of all available test directories we found inside of luajit > submodule. > > Closes #5425 > --- > test/CMakeLists.txt | 14 +++++++++++--- > test/luajit | 1 + > 2 files changed, 12 insertions(+), 3 deletions(-) > create mode 120000 test/luajit > > -- > 2.20.1 > [1]: https://github.com/tarantool/tarantool/issues/4862 [2]: https://gitlab.com/tarantool/tarantool/-/pipelines/203157106 [3]: https://www.tarantool.io/en/doc/latest/dev_guide/developer_guidelines/#how-to-write-a-commit-message -- Best regards, IM