Re: [Tarantool-patches] [PATCH luajit 3/4][v2] cmake: introduce target with codespell

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

Hi, Sergey

On 10/23/23 12:20, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the patch!
> Please, consider my comments below.
>
> On 11.10.23, Sergey Bronnikov wrote:
>> From: Sergey Bronnikov <sergeyb@tarantool.org>
>>
>> The patch introduces a new CMake target: "LuaJIT-codespell", that
>> spellchecks files specified in a whitelist by codespell [1].
>>
>> 1. https://github.com/codespell-project/codespell
>> ---
>>   CMakeLists.txt                   |  1 +
>>   cmake/CodeSpell.cmake            | 36 ++++++++++++++++++++++++++++++++
>>   test/CMakeLists.txt              |  1 +
>>   tools/codespell-ignore-words.txt |  3 +++
>>   4 files changed, 41 insertions(+)
>>   create mode 100644 cmake/CodeSpell.cmake
>>   create mode 100644 tools/codespell-ignore-words.txt
>>
>> diff --git a/CMakeLists.txt b/CMakeLists.txt
> <snipped>
>
>> diff --git a/cmake/CodeSpell.cmake b/cmake/CodeSpell.cmake
>> new file mode 100644
>> index 00000000..c4d3555d
>> --- /dev/null
>> +++ b/cmake/CodeSpell.cmake
>> @@ -0,0 +1,36 @@
>> +find_program(CODESPELL codespell)
>> +
>> +list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/src/lj_mapi.c)
>> +list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/src/lj_sysprof.c)
>> +list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/src/lj_utils_leb128.c)
>> +list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/src/lj_wbuf.c)
>> +list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/src/luajit-gdb.py)
>> +list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/src/luajit_lldb.py)
>> +list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/test/CMakeLists.txt)
>> +list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/test/tarantool-c-tests)
>> +list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/test/tarantool-tests)
>> +list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/tools)
> Also, the following files should be checked:
>
> * src/lib_misc.c
> * src/lj_memprof.c
> * src/lj_memprof.h
> * src/lj_sysprof.h
> * src/lj_utils.h
> * src/lj_wbuf.h
> * src/lmisclib.h
> * test/LuaJIT-tests/CMakeLists.txt
> * test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
> * test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
> * test/PUC-Rio-Lua-5.1-tests/libs/CMakeLists.txt
> * test/lua-Harness-tests/CMakeLists.txt


Added:

--- a/cmake/CodeSpell.cmake
+++ b/cmake/CodeSpell.cmake
@@ -1,13 +1,25 @@
  find_program(CODESPELL codespell)

  string(JOIN "," CODESPELL_WHITELIST
+  ${PROJECT_SOURCE_DIR}/src/lib_misc.c
    ${PROJECT_SOURCE_DIR}/src/lj_mapi.c
+  ${PROJECT_SOURCE_DIR}/src/lj_memprof.c
+  ${PROJECT_SOURCE_DIR}/src/lj_memprof.h
    ${PROJECT_SOURCE_DIR}/src/lj_sysprof.c
+  ${PROJECT_SOURCE_DIR}/src/lj_sysprof.h
+  ${PROJECT_SOURCE_DIR}/src/lj_utils.h
    ${PROJECT_SOURCE_DIR}/src/lj_utils_leb128.c
    ${PROJECT_SOURCE_DIR}/src/lj_wbuf.c
+  ${PROJECT_SOURCE_DIR}/src/lj_wbuf.h
+  ${PROJECT_SOURCE_DIR}/src/lmisclib.h
    ${PROJECT_SOURCE_DIR}/src/luajit-gdb.py
    ${PROJECT_SOURCE_DIR}/src/luajit_lldb.py
    ${PROJECT_SOURCE_DIR}/test/CMakeLists.txt
+  ${PROJECT_SOURCE_DIR}/test/lua-Harness-tests/CMakeLists.txt
+  ${PROJECT_SOURCE_DIR}/test/LuaJIT-tests/CMakeLists.txt
+  ${PROJECT_SOURCE_DIR}/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
+  ${PROJECT_SOURCE_DIR}/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
+ ${PROJECT_SOURCE_DIR}/test/PUC-Rio-Lua-5.1-tests/libs/CMakeLists.txt
    ${PROJECT_SOURCE_DIR}/test/tarantool-c-tests
    ${PROJECT_SOURCE_DIR}/test/tarantool-tests
    ${PROJECT_SOURCE_DIR}/tools

>
>> +
>> +set(IGNORE_WORDS ${PROJECT_SOURCE_DIR}/tools/codespell-ignore-words.txt)
>> +
>> +add_custom_target(${PROJECT_NAME}-codespell)
>> +if (CODESPELL)
>> +  add_custom_command(TARGET ${PROJECT_NAME}-codespell
>> +    COMMENT "Running codespell"
>> +    COMMAND
>> +      ${CODESPELL}
>> +        --ignore-words ${IGNORE_WORDS}
>> +        --skip ${IGNORE_WORDS}
>> +        --ignore-words-list fpr
>> +        --check-filenames
>> +        ${CODESPELL_WHITELIST}
>> +    WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
>> +  )
>> +else ()
>> +  set(WARN_MSG "`codespell' is not found, "
>> +               "so ${PROJECT_NAME}-codespell target is dummy")
> The output is splitted (thanks CMake). I found the only way to join
> lines via `string(CONCAT WARN_MSG <str1> <str2>)`.

Fixed:

[1] ~/sources/MRG/tarantool/third_party/luajit$ cmake --build build 
--parallel -t LuaJIT-codespell
codespell is not found, so LuaJIT-codespell target is dummy
Built target LuaJIT-codespell


@@ -28,8 +40,9 @@ if (CODESPELL)
      WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
    )
  else ()
-  set(WARN_MSG "codespell is not found, "
-               "so ${PROJECT_NAME}-codespell target is dummy")
+  set(STR1 "codespell is not found,")
+  set(STR2 "so ${PROJECT_NAME}-codespell target is dummy")
+  string(CONCAT WARN_MSG "${STR1} ${STR2}")
    add_custom_command(TARGET ${PROJECT_NAME}-codespell
      COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red ${WARN_MSG}
      COMMENT ${MSG}

>
>> +  add_custom_command(TARGET ${PROJECT_NAME}-codespell
>> +    COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red ${MSG}
>> +    COMMENT ${MSG}
> The ${MSG} is undefined, so this line does nothing and can be deleted.
Fixed.
>
>> +  )
>> +endif (CODESPELL)
> <snipped>
>
>> diff --git a/tools/codespell-ignore-words.txt b/tools/codespell-ignore-words.txt
>> new file mode 100644
>> index 00000000..ceeed47c
>> --- /dev/null
>> +++ b/tools/codespell-ignore-words.txt
>> @@ -0,0 +1,3 @@
>> +mmaped
> Should it be mmapped instead (like map -> mapped)?
removed this work from exclusion list, I don't know why but codespell is 
not warns about it.
>
>> +isnt
>> +FPR
>> -- 
>> 2.34.1
>>