Re: [Tarantool-patches] [PATCH luajit 15/15] test: run flake8 static analysis via CMake

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Igor,


On 8/7/23 15:17, Igor Munkin wrote:

<snipped>

>>> +  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?

Actually it doesn't work for me even with Ninja generator.

However, I don't want to block this patch due to this.


LGTM

>
>>> +  )
>>> +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()
>
> ================================================================================
>