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 E77AE562369; Thu, 3 Aug 2023 17:23:31 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E77AE562369 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1691072612; bh=TxORNOsgvOO+C6d0RAaRBoSjbU7P4SsM6GgcCuAVjOg=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=oEYboXwnvLYkjrgUJ8LvUcbgkmhYaeJPeyblMRT+UC3CySwuQYNZCln05GW9F1mAt bJu3YxAkj8rZDcUl+NxrsQ4JOl6HJ1LfHgtmFHh0Ptjg2g+gtT9jtwe4qKJsZREV2b GpDrgMqZpDb71sCteNZwaH/5ziMvK/v0ovbBOrps= Received: from smtpng2.i.mail.ru (smtpng2.i.mail.ru [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 38E8A562369 for ; Thu, 3 Aug 2023 17:23:30 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 38E8A562369 Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1qRZEf-0002Uo-47; Thu, 03 Aug 2023 17:23:29 +0300 Content-Type: multipart/alternative; boundary="------------DBr0lk4Ph1Uas7fN2WRW0xDt" Message-ID: <2f20f91c-1336-b34b-6681-3947e96b486d@tarantool.org> Date: Thu, 3 Aug 2023 17:23:28 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Content-Language: en-US To: Igor Munkin , Maxim Kokryashkin Cc: tarantool-patches@dev.tarantool.org References: In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD969E04B5EED670DC804E38A5F9341E5D89B81E0241E25E490182A05F5380850400B95420CDA15B63C7B109A75DEB68E074C0A0C4A4335FDEDEBB3905A729EC349 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE79A02CDD1178524C2EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006371E92E38DA2D50EB18638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D895A708BC44DC84043C478B22F4E0BBE1117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC3A703B70628EAD7BA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F44604297287769387670735200AC5B80A05675ACDC26CFBAC0749D213D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8B8C7ADC89C2F0B2A5AAAE862A0553A39223F8577A6DFFEA7C4DB04CA562B6C7F843847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A5A2EFFFAC2266FC3470F7242BBC144F95C7D02C5A921944C2F87CCE6106E1FC07E67D4AC08A07B9B0AD74539164518AE5CB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF2AACF16C7486FDAACDDC913BB9B0E7174A8FD8C26416F4B2BE6ACF5980E7BE23212E5196DFD6A13F8553022043D0BC9A60B1132FBE3D015B29256A5CE4CD3E79E48CAC7CA610320002C26D483E81D6BE0DBAE6F56676BC7117BB6831D7356A2DEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojcir52QaMQ800NBt0u4vGwQ== X-DA7885C5: D41D888AD0C7B98534D27B680EC4980DF3B9A3153D1D8DBE99149BDEE6E51F84262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F73930F533AC2B33E986BB2EE0CEB50087AE1C160423E9105D59DDD788429FD8613638ED9BB8B05EE7B3AFB559BB5D741EB96D19CD4E7312BAA970A04DAD6CC59E3365FEEDEB644C299C0ED14614B50AE0675 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: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------DBr0lk4Ph1Uas7fN2WRW0xDt Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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/.flake8rc b/.flake8rc > new file mode 100644 > index 00000000..b6f7ad48 > --- /dev/null > +++ b/.flake8rc > @@ -0,0 +1,12 @@ > +[flake8] > +extend-ignore = > + # TODO: I have no idea, how to fix this. flake8 suggests nothing > + # (like clang-format or checkpatch.pl do). Help needed. > + E131, > + # TODO: I have no idea, how to fix this. flake8 suggests nothing > + # (like clang-format or checkpatch.pl do). Help needed. > + E501, > + # XXX: Suppress F821, since we have autogenerated names for > + # 'ptr' type complements in luajit_lldb.py. > + F821 > +max-line-length = 80 > diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml > index 70c98104..6b420f68 100644 > --- a/.github/workflows/lint.yml > +++ b/.github/workflows/lint.yml > @@ -41,7 +41,7 @@ jobs: > run: | > # TODO: Move this step to a separate action. > sudo apt -y update > - sudo apt -y install cmake ninja-build lua5.1 luarocks > + sudo apt -y install cmake ninja-build lua5.1 luarocks flake8 > sudo luarocks install luacheck > # Set CMAKE_BUILD_PARALLEL_LEVEL environment variable to > # limit the number of parallel jobs for build/test step. > @@ -49,5 +49,5 @@ jobs: > - name: configure > run: cmake -S . -B ${{ env.BUILDDIR }} -G Ninja > - name: test > - run: cmake --build . --target LuaJIT-luacheck > + run: cmake --build . --target LuaJIT-lint > working-directory: ${{ env.BUILDDIR }} > 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"? > + 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 | > + ) > +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". --------------DBr0lk4Ph1Uas7fN2WRW0xDt Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

Hi, Igor


thanks for the patch! see my comments


Sergey

On 8/3/23 10:30, Igor Munkin wrote: 
This patch introduces a sepa=
rate 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 <LuaJIT-lint> target, th=
at
joins both luacheck and flake8 linter runs. CI job with linters is
adjusted respectively.

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 .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/.flake8rc b/.flake8rc
new file mode 100644
index 00000000..b6f7ad48
--- /dev/null
+++ b/.flake8rc
@@ -0,0 +1,12 @@
+[flake8]
+extend-ignore =3D
+  # TODO: I have no idea, how to fix this. flake8 suggests nothing
+  # (like clang-format or checkpatch.pl do). Help needed.
+  E131,
+  # TODO: I have no idea, how to fix this. flake8 suggests nothing
+  # (like clang-format or checkpatch.pl do). Help needed.
+  E501,
+  # XXX: Suppress F821, since we have autogenerated names for
+  # 'ptr' type complements in luajit_lldb.py.
+  F821
+max-line-length =3D 80
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 70c98104..6b420f68 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -41,7 +41,7 @@ jobs:
         run: |
           # TODO: Move this step to a separate action.
           sudo apt -y update
-          sudo apt -y install cmake ninja-build lua5.1 luarocks
+          sudo apt -y install cmake ninja-build lua5.1 luarocks flake8
           sudo luarocks install luacheck
           # Set CMAKE_BUILD_PARALLEL_LEVEL environment variable to
           # limit the number of parallel jobs for build/test step.
@@ -49,5 +49,5 @@ jobs:
       - name: configure
         run: cmake -S . -B ${{ env.BUILDDIR }} -G Ninja
       - name: test
-        run: cmake --build . --target LuaJIT-luacheck
+        run: cmake --build . --target LuaJIT-lint
         working-directory: ${{ env.BUILDDIR }}
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()
=20
+find_program(FLAKE8 flake8)
+if(FLAKE8)
+  get_filename_component(FLAKE8_SOURCE_DIR "${PROJECT_SOURCE_DIR}" REALP=
ATH)
Nit: name FLAKE8_SOURCE_DIR is confused, because dir has nothing common with flake8. May be "REAL_SOURCE_DIR"?
+  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


+  )
+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".


    

  


--------------DBr0lk4Ph1Uas7fN2WRW0xDt--