[Tarantool-patches] [PATCH luajit 3/4][v2] cmake: introduce target with codespell
Sergey Kaplun
skaplun at tarantool.org
Mon Oct 23 17:38:44 MSK 2023
Hi, Sergey!
Thanks for the fixes!
LGTM, just minor nits below.
On 23.10.23, Sergey Bronnikov wrote:
> 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 at 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
Side note: within vim's (sort 'l' is greater then 'L', see `man ascii`),
but I'm OK with this order too.
>
> >
> >> +
> >> +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}")
Oh, may be just the following?
| string(CONCAT WARN_MSG
| "codespell is not found,"
| "so ${PROJECT_NAME}-codespell target is dummy"
| )
> add_custom_command(TARGET ${PROJECT_NAME}-codespell
> COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red ${WARN_MSG}
> COMMENT ${MSG}
>
> >
<snipped>
> >> +++ 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.
I suppose, that some files aren't covered...
| codespell --ignore-words tools/codespell-ignore-words.txt
| ...
| ./test/tarantool-c-tests/lj-49-bad-lightuserdata.test.c:42: mmaped ==> mapped
| ./test/tarantool-c-tests/lj-49-bad-lightuserdata.test.c:44: mmaped ==> mapped
| ./test/tarantool-c-tests/lj-49-bad-lightuserdata.test.c:45: mmaped ==> mapped
| ./test/tarantool-c-tests/lj-49-bad-lightuserdata.test.c:46: mmaped ==> mapped
| ./test/tarantool-c-tests/lj-49-bad-lightuserdata.test.c:47: mmaped ==> mapped
So, maybe we should use mmapped and also add it to the ignore-words?
> >
> >> +isnt
> >> +FPR
> >> --
> >> 2.34.1
> >>
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list