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 9C4EB58004E; Mon, 7 Aug 2023 15:31:44 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 9C4EB58004E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1691411504; bh=ip788Vq1Ucu9WJF4YdgtU46msWMT3YAVxlZyblBwukQ=; 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=PhKZAEoL5ImVq5CeeVzRBJBmxDCM96dN2DPU5FiFt9kg51ZAAT7uFKVFfAL8zFOfh pylFBHlSIT0EzfuFNymixt2fEh5SRpSo07YKch9LZr7DMIVGr6UZBlhxxf847ZI6Ia P59j0t+jRLXoSdapEb9NnhTIRY/Udgs8OtJRO8f0= 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 0E8E157FBB4 for ; Mon, 7 Aug 2023 15:31:44 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 0E8E157FBB4 Received: by smtp42.i.mail.ru with esmtpa (envelope-from ) id 1qSzOh-004JOn-0m; Mon, 07 Aug 2023 15:31:43 +0300 Date: Mon, 7 Aug 2023 12:17:53 +0000 To: Sergey Bronnikov Message-ID: References: <2f20f91c-1336-b34b-6681-3947e96b486d@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <2f20f91c-1336-b34b-6681-3947e96b486d@tarantool.org> X-Clacks-Overhead: GNU Terry Pratchett X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD969E04B5EED670DC804E38A5F9341E5D8C5E4A5EF3DB2E749182A05F5380850403F4E1C22D3AFEE44BA4D38F606722E346BBB8365A3B58AEEBA48044AFEE6107A X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE73870E1FF9A049D91EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637C5BAFDCCAC60DA968638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8B28D7A2E75383B53C909EBCE7E2162FD117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC3A703B70628EAD7BA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F4460429728776938767073520CCD848CCB6FE560CF04B652EEC242312D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EEB1593CA6EC85F86DFCE65BE3358055BDD8FC6C240DEA76429C9F4D5AE37F343AA9539A8B242431040A6AB1C7CE11FEE3457EE4B4996FC54603F1AB874ED89028C4224003CC836476E2F48590F00D11D6E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F79006373BA04B6A498D0BA4731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A531EFB1CEEAAE951C158D37FFA043F2BB55B2F8746AAC91C1F87CCE6106E1FC07E67D4AC08A07B9B0A6C7FFFE744CA7FBCB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFC01FCEF69B27B9CD1535F4F8B95ABB4F890E53C163E528028AE40CF7A1017B0CAE7801A017AB3E4A6CFA265C340F28BAC462C17EB3675544AC95CA38E59F5F59A74DFFEFA5DC0E7F02C26D483E81D6BEECAEF3E2CCC1ED8C383653B6C8D9AE0FD16FCAA6493B703A X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj/xSHLV4ZQPsdVoPCUe9dZw== X-Mailru-Sender: 2FEBA92C8E508479FE7B9A1DF348D531D73FB19E007CDFAA7A1057539B2F0A5451C05F07CCC97CB22326FE6F2A341ACE0FB9F97486540B4CD9E8847AB8CFED4D9ABF8A61C016C2CFB0DAF586E7D11B3E67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 15/15] test: run flake8 static analysis 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" Sergey, Thanks for your review! On 03.08.23, Sergey Bronnikov wrote: > Hi, Igor > > > thanks for the patch! see my comments > > > Sergey > > On 8/3/23 10:30, Igor Munkin wrote: > > This patch introduces a separate target to run flake8 against all Python > > scripts within LuaJIT repository (i.e. debugger extensions). There are > > some tweaks in .flake8rc regarding our style: one can find more info in > > the config file. > > > > The new target is a dependency for the new target, that > > joins both luacheck and flake8 linter runs. CI job with linters is > > adjusted respectively. > > > > Signed-off-by: Igor Munkin > > --- > > .flake8rc | 12 ++++++++++++ > > .github/workflows/lint.yml | 4 ++-- > > test/CMakeLists.txt | 28 ++++++++++++++++++++++++++++ > > 3 files changed, 42 insertions(+), 2 deletions(-) > > create mode 100644 .flake8rc > > > > diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt > > index 47296a22..17ac5cac 100644 > > --- a/test/CMakeLists.txt > > +++ b/test/CMakeLists.txt > > @@ -42,6 +42,34 @@ else() > > ) > > endif() > > +find_program(FLAKE8 flake8) > > +if(FLAKE8) > > + get_filename_component(FLAKE8_SOURCE_DIR "${PROJECT_SOURCE_DIR}" REALPATH) > Nit: name FLAKE8_SOURCE_DIR is confused, because dir has nothing common with > flake8. May be "REAL_SOURCE_DIR"? The semantics are the same as for LUACHECK_SOURCE_DIR: it should be read as "the directory with sources to be checked via flake8". Ignoring. > > + set(FLAKE8_RC ${FLAKE8_SOURCE_DIR}/.flake8rc) > > + file(GLOB_RECURSE FLAKE8_DEPS ${FLAKE8_SOURCE_DIR}/*.py) > > + add_custom_target(${PROJECT_NAME}-flake8 > > + DEPENDS ${FLAKE8_DEPS} > > + ) > > + add_custom_command(TARGET ${PROJECT_NAME}-flake8 > > + COMMENT "Running flake8 static analysis" > > + COMMAND > > + ${FLAKE8} ${FLAKE8_DEPS} > > + --config ${FLAKE8_RC} > > + --jobs ${CMAKE_BUILD_PARALLEL_LEVEL} > > + WORKING_DIRECTORY ${FLAKE8_SOURCE_DIR} > > + ) > > +else() > > + add_custom_target(${PROJECT_NAME}-flake8) > > + add_custom_command(TARGET ${PROJECT_NAME}-flake8 > > + COMMENT "`flake8' is not found, so ${PROJECT_NAME}-flake8 target is dummy" > > Please add a command to a dummy target: > > |COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red "||`flake8' is not > found, so ${PROJECT_NAME}-flake8 target is dummy" with added COMMAND target > will print a message | Well, 0 days since "CMake is doing something unexpected". COMMENT is totally fine, the problem is in Ninja generated artefacts: I'm not an expert in Ninja but looks like CMake generates kinda nop if COMMAND is omitted. Here is the dump: | $ cmake --version | cmake version 3.26.4 | | CMake suite maintained and supported by Kitware (kitware.com/cmake). | $ rm -f CMakeCache.txt | $ cmake . -G Ninja | -- The C compiler identification is GNU 13.2.0 | -- Detecting C compiler ABI info | -- Detecting C compiler ABI info - done | -- Check for working C compiler: /usr/bin/cc - skipped | -- Detecting C compile features | -- Detecting C compile features - done | -- [SetVersion] Reading version from VCS: v2.1.0-beta3-355-g6dd0b0e2 | -- [SetBuildParallelLevel] CMAKE_BUILD_PARALLEL_LEVEL is 4 | -- The ASM compiler identification is GNU | -- Found assembler: /usr/bin/cc | -- Configuring done (0.3s) | -- Generating done (0.0s) | -- Build files have been written to: /home/imun/projects/tarantool-luajit | $ cmake --build . --target LuaJIT-flake8 | ninja: no work to do. | $ rm -f CMakeCache.txt | $ cmake . | -- The C compiler identification is GNU 13.2.0 | -- Detecting C compiler ABI info | -- Detecting C compiler ABI info - done | -- Check for working C compiler: /usr/bin/cc - skipped | -- Detecting C compile features | -- Detecting C compile features - done | -- [SetVersion] Reading version from VCS: v2.1.0-beta3-355-g6dd0b0e2 | -- [SetBuildParallelLevel] CMAKE_BUILD_PARALLEL_LEVEL is 4 | -- The ASM compiler identification is GNU | -- Found assembler: /usr/bin/cc | -- Configuring done (0.3s) | -- Generating done (0.1s) | -- Build files have been written to: /home/imun/projects/tarantool-luajit | $ cmake --build . --target LuaJIT-flake8 | `flake8' is not found, so LuaJIT-flake8 target is dummy | Built target LuaJIT-flake8 Hence, I suggest to leave everything as is (see luacheck-related part), but in scope of the CTest series implement another way of interaction with user. Does it work for you? > > > + ) > > +endif() > > + > > +add_custom_target(${PROJECT_NAME}-lint DEPENDS > > + ${PROJECT_NAME}-luacheck > > + ${PROJECT_NAME}-flake8 > > +) > > + > > set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]") > > separate_arguments(LUAJIT_TEST_COMMAND) > > You've introduced a new target LuaJIT-lint, that includes LuaJIT-luacheck > and LuaJIT-flake8. > > I suppose we need replace dependence "LuaJIT-luacheck" to "LuaJIT-lint" for > a target "test". Oops, my bad, thanks! Fixed. ================================================================================ diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 17ac5cac..58cba5ba 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -102,6 +102,6 @@ if(LUAJIT_USE_TEST) add_custom_target(test DEPENDS ${PROJECT_NAME}-test - ${PROJECT_NAME}-luacheck + ${PROJECT_NAME}-lint ) endif() ================================================================================ -- Best regards, IM