[Tarantool-patches] [PATCH 0/2] Fix typos and enable checkpatch

[Tarantool-patches] [PATCH 0/2] Fix typos and enable checkpatch

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <sergeyb@tarantool.org>

Sometimes we do mistakes and typos. Reviewers spend a lot of
time in proofreading of comments, commit messages and code itself and
reports typos to authors. The idea is to automate a part of work made by reviewers
and highlight about all typos and code styles problems in continuous integration,
before sending patches to review.

Patches fixes typos in our own source code and enables checkpatch in CI.

Branch: https://github.com/tarantool/luajit/tree/ligurio/gh-xxxx-spellchecking

Sergey Bronnikov (2):
  test: fix typos
  ci: enable checkpatch

 .github/actions/checkpatch/action.yml           | 17 +++++++++++++++++
 .github/workflows/lint.yml                      | 14 ++++++++++++++
 src/luajit-gdb.py                               |  6 +++---
 src/luajit_lldb.py                              |  6 +++---
 test/LuaJIT-tests/src/ctest.c                   |  2 +-
 test/tarantool-tests/CMakeLists.txt             |  2 +-
 test/tarantool-tests/fix-emit-rma.test.lua      |  2 +-
 test/tarantool-tests/gh-4199-gc64-fuse.test.lua |  2 +-
 ...gh-6096-external-unwinding-on-arm64.test.lua |  2 +-
 test/tarantool-tests/gh-6163-min-max.test.lua   |  2 +-
 .../lj-416-xor-before-jcc.test.lua              |  2 +-
 ...426-arm64-incorrect-check-closed-uv.test.lua |  2 +-
 .../lj-512-profiler-hook-finalizers.test.lua    |  4 ++--
 .../lj-603-err-snap-restore.test.lua            |  2 +-
 .../misclib-memprof-lapi.test.lua               |  6 +++---
 test/tarantool-tests/tap.lua                    |  4 ++--
 test/tarantool-tests/unit-jit-parse.test.lua    |  2 +-
 tools/memprof/parse.lua                         |  4 ++--
 tools/memprof/process.lua                       |  4 ++--
 19 files changed, 58 insertions(+), 27 deletions(-)
 create mode 100644 .github/actions/checkpatch/action.yml

-- 
2.34.1

[Tarantool-patches] [PATCH 1/2] test: fix typos

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <sergeyb@tarantool.org>

Fix typos found with codespell in a files with our own source code.
---
 src/luajit-gdb.py                                           | 6 +++---
 src/luajit_lldb.py                                          | 6 +++---
 test/LuaJIT-tests/src/ctest.c                               | 2 +-
 test/tarantool-tests/CMakeLists.txt                         | 2 +-
 test/tarantool-tests/fix-emit-rma.test.lua                  | 2 +-
 test/tarantool-tests/gh-4199-gc64-fuse.test.lua             | 2 +-
 .../gh-6096-external-unwinding-on-arm64.test.lua            | 2 +-
 test/tarantool-tests/gh-6163-min-max.test.lua               | 2 +-
 test/tarantool-tests/lj-416-xor-before-jcc.test.lua         | 2 +-
 .../lj-426-arm64-incorrect-check-closed-uv.test.lua         | 2 +-
 .../lj-512-profiler-hook-finalizers.test.lua                | 4 ++--
 test/tarantool-tests/lj-603-err-snap-restore.test.lua       | 2 +-
 test/tarantool-tests/misclib-memprof-lapi.test.lua          | 6 +++---
 test/tarantool-tests/tap.lua                                | 4 ++--
 test/tarantool-tests/unit-jit-parse.test.lua                | 2 +-
 tools/memprof/parse.lua                                     | 4 ++--
 tools/memprof/process.lua                                   | 4 ++--
 17 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
index 96ee2289..afdfecd9 100644
--- a/src/luajit-gdb.py
+++ b/src/luajit-gdb.py
@@ -268,7 +268,7 @@ def strdata(obj):
     try:
         return str(cast('char *', cast('GCstr *', obj) + 1))[len(PADDING):]
     except UnicodeEncodeError:
-        return "<luajit-gdb: error occured while rendering non-ascii slot>"
+        return "<luajit-gdb: error occurred while rendering non-ascii slot>"
 
 def itypemap(o):
     if LJ_64 and not LJ_GC64:
@@ -606,7 +606,7 @@ class LJDumpTable(LJBase):
     '''
 lj-tab <GCtab *>
 
-The command receives a GCtab adress and dumps the table contents:
+The command receives a GCtab address and dumps the table contents:
 * Metatable address whether the one is set
 * Array part <asize> slots:
   <aslot ptr>: [<index>]: <tv>
@@ -677,7 +677,7 @@ coroutine guest stack:
     + CP: Protected C frame
     + PP: VM performs a call as a result of executinig pcall or xpcall
 
-If L is ommited the main coroutine is used.
+If L is omitted the main coroutine is used.
     '''
 
     def invoke(self, arg, from_tty):
diff --git a/src/luajit_lldb.py b/src/luajit_lldb.py
index 9ee10269..4d7bef81 100644
--- a/src/luajit_lldb.py
+++ b/src/luajit_lldb.py
@@ -490,7 +490,7 @@ def strdata(obj):
         ptr = cast('char *', obj + 1)
         return ptr.summary
     except UnicodeEncodeError:
-        return "<luajit-lldb: error occured while rendering non-ascii slot>"
+        return "<luajit-lldb: error occurred while rendering non-ascii slot>"
 
 def itype(o):
     return tou32(o.it64 >> 47) if LJ_GC64 else o.it
@@ -909,7 +909,7 @@ class LJDumpTable(Command):
     '''
 lj-tab <GCtab *>
 
-The command receives a GCtab adress and dumps the table contents:
+The command receives a GCtab address and dumps the table contents:
 * Metatable address whether the one is set
 * Array part <asize> slots:
   <aslot ptr>: [<index>]: <tv>
@@ -979,7 +979,7 @@ coroutine guest stack:
     + CP: Protected C frame
     + PP: VM performs a call as a result of executinig pcall or xpcall
 
-If L is ommited the main coroutine is used.
+If L is omitted the main coroutine is used.
     '''
     def execute(self, debugger, args, result):
         l = self.parse(args)
diff --git a/test/LuaJIT-tests/src/ctest.c b/test/LuaJIT-tests/src/ctest.c
index d257567b..e99f2306 100644
--- a/test/LuaJIT-tests/src/ctest.c
+++ b/test/LuaJIT-tests/src/ctest.c
@@ -234,7 +234,7 @@ static int costatus(lua_State *L, lua_State *co) {
       else
 	return CO_SUS;  /* initial state */
     }
-    default:  /* some error occured */
+    default:  /* some error occurred */
       return CO_DEAD;
   }
 }
diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 6218f76a..f0bf8efb 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -132,7 +132,7 @@ else()
 endif()
 
 # LUA_CPATH and LD_LIBRARY_PATH variables and also TESTLIBS list
-# with dependecies are set in scope of BuildTestLib macro.
+# with dependencies are set in scope of BuildTestLib macro.
 add_custom_target(tarantool-tests
   DEPENDS ${LUAJIT_TEST_BINARY} ${TESTLIBS}
 )
diff --git a/test/tarantool-tests/fix-emit-rma.test.lua b/test/tarantool-tests/fix-emit-rma.test.lua
index 1f8e66f4..7d1e5295 100644
--- a/test/tarantool-tests/fix-emit-rma.test.lua
+++ b/test/tarantool-tests/fix-emit-rma.test.lua
@@ -4,7 +4,7 @@ local test = tap.test('fix-emit-rma'):skipcond({
   ['Test requires GC64 mode enabled'] = not require('ffi').abi('gc64'),
 })
 
--- Need to test 2 cases of `emit_rma()` particulary on x64:
+-- Need to test 2 cases of `emit_rma()` particularly on x64:
 --   * `IR_LDEXP` with `fld` instruction for loading constant
 --      number `TValue` by address.
 --   * `IR_OBAR` with the corresponding `test` instruction on
diff --git a/test/tarantool-tests/gh-4199-gc64-fuse.test.lua b/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
index 4513d43b..1847e5e4 100644
--- a/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
+++ b/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
@@ -44,7 +44,7 @@ for n = 1, 100 do
       for i = 1, 5 do
         -- This constant fusion leads to the test failure.
         a[i] = 0
-        -- This summ is not necessarry but decreases the amount of
+        -- This summ is not necessary but decreases the amount of
         -- iterations.
         a[i] = a[i] + x + y
         -- Use all FPR registers and one value from the memory
diff --git a/test/tarantool-tests/gh-6096-external-unwinding-on-arm64.test.lua b/test/tarantool-tests/gh-6096-external-unwinding-on-arm64.test.lua
index cdeea441..e43750af 100644
--- a/test/tarantool-tests/gh-6096-external-unwinding-on-arm64.test.lua
+++ b/test/tarantool-tests/gh-6096-external-unwinding-on-arm64.test.lua
@@ -1,6 +1,6 @@
 local tap = require('tap')
 
--- Test file to check correctnes of external unwinding
+-- Test file to check correctness of external unwinding
 -- in LuaJIT.
 -- See also https://github.com/LuaJIT/LuaJIT/issues/698,
 -- https://github.com/LuaJIT/LuaJIT/pull/757.
diff --git a/test/tarantool-tests/gh-6163-min-max.test.lua b/test/tarantool-tests/gh-6163-min-max.test.lua
index 882cb1a0..219d1456 100644
--- a/test/tarantool-tests/gh-6163-min-max.test.lua
+++ b/test/tarantool-tests/gh-6163-min-max.test.lua
@@ -53,7 +53,7 @@ local x = 1
 
 jit.opt.start('hotloop=1')
 
--- XXX: Looping over the operations and their arguments breakes the
+-- XXX: Looping over the operations and their arguments breaks the
 -- semantics of some optimization tests below. The cases are
 -- copy-pasted to preserve optimization semantics.
 
diff --git a/test/tarantool-tests/lj-416-xor-before-jcc.test.lua b/test/tarantool-tests/lj-416-xor-before-jcc.test.lua
index 861114e8..7181fb4f 100644
--- a/test/tarantool-tests/lj-416-xor-before-jcc.test.lua
+++ b/test/tarantool-tests/lj-416-xor-before-jcc.test.lua
@@ -41,7 +41,7 @@ local testxor = ffi.load('libtestxor')
 -- pressure and specific registers allocations.
 local handler = setmetatable({}, {
   __newindex = function ()
-    -- 0 and nil are suggested as differnt constant-zero values
+    -- 0 and nil are suggested as different constant-zero values
     -- for the call and occupied different registers.
     testxor.test_xor_func(0, 0, 0, 0, 0, 0, nil, 0)
   end
diff --git a/test/tarantool-tests/lj-426-arm64-incorrect-check-closed-uv.test.lua b/test/tarantool-tests/lj-426-arm64-incorrect-check-closed-uv.test.lua
index 4cdf1211..6e58370d 100644
--- a/test/tarantool-tests/lj-426-arm64-incorrect-check-closed-uv.test.lua
+++ b/test/tarantool-tests/lj-426-arm64-incorrect-check-closed-uv.test.lua
@@ -14,7 +14,7 @@ do
   -- The function's prototype is created with the following
   -- constants at chunk parsing. After adding this constant to
   -- the function's prototype it will be marked as gray during
-  -- propogate phase.
+  -- propagate phase.
   local function usets() uv = '' end
   _G.usets = usets
 end
diff --git a/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua b/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
index f7ee344f..9a3da644 100644
--- a/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
+++ b/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
@@ -20,9 +20,9 @@ local finish = os.clock()
 
 profile.stop()
 
--- XXX: The bug is occured as stopping of callbacks invocation,
+-- XXX: The bug is occurred as stopping of callbacks invocation,
 -- when a new tick strikes inside `gc_call_finalizer()`.
--- The amount of successfull callbacks isn't stable (2-15).
+-- The amount of successful callbacks isn't stable (2-15).
 -- So, assume that amount of profiling samples should be at least
 -- more than 0.5 intervals of time during sampling.
 test:ok(nsamples >= 0.5 * (finish - start) * 1e3 / INTERVAL,
diff --git a/test/tarantool-tests/lj-603-err-snap-restore.test.lua b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
index a6c831ed..1c17f0d8 100644
--- a/test/tarantool-tests/lj-603-err-snap-restore.test.lua
+++ b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
@@ -11,7 +11,7 @@ local function do_test()
   local recursive_f
   local function errfunc()
     xpcall(recursive_f, errfunc)
-    -- Since this error is occured on snapshot restoration and can
+    -- Since this error is occurred on snapshot restoration and can
     -- be handled by compiler itself, we shouldn't bother a user
     -- with it.
     handler_is_called = true
diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index eae20893..c8d6b879 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -52,13 +52,13 @@ local function generate_output(filename, payload)
   collectgarbage()
 
   local res, err = misc.memprof.start(filename)
-  -- Should start succesfully.
+  -- Should start successfully.
   assert(res, err)
 
   payload()
 
   res, err = misc.memprof.stop()
-  -- Should stop succesfully.
+  -- Should stop successfully.
   assert(res, err)
 end
 
@@ -188,7 +188,7 @@ test:test("output", function(subtest)
   -- Check allocation reports. The second argument is a line
   -- number of the allocation event itself. The third is a line
   -- number of the corresponding function definition. The last
-  -- one is the number of allocations. 1 event - alocation of
+  -- one is the number of allocations. 1 event - allocation of
   -- table by itself + 1 allocation of array part as far it is
   -- bigger than LJ_MAX_COLOSIZE (16).
   subtest:ok(check_alloc_report(alloc, { line = 37, linedefined = 35 }, 2))
diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua
index 47a8fe87..b91ca335 100644
--- a/test/tarantool-tests/tap.lua
+++ b/test/tarantool-tests/tap.lua
@@ -1,6 +1,6 @@
 --- tap.lua internal file.
 ---
---- The Test Anything Protocol vesion 13 producer.
+--- The Test Anything Protocol version 13 producer.
 ---
 
 -- Initializer FFI for <iscdata> check.
@@ -79,7 +79,7 @@ local function ok(test, cond, message, extra)
   io.write(tindent, ("line:\t%s\n"):format(trace[#trace].line))
   for frameno, frame in ipairs(trace) do
     io.write(tindent, ("frame #%d\n"):format(frameno))
-    -- XXX: Use "half indent" to dump <frame> fiels.
+    -- XXX: Use "half indent" to dump <frame> fields.
     local findent = indent(0.5) .. tindent
     for key, value in pairs(frame) do
       io.write(findent, ("%s:\t%s\n"):format(key, value))
diff --git a/test/tarantool-tests/unit-jit-parse.test.lua b/test/tarantool-tests/unit-jit-parse.test.lua
index 24216e87..34c38701 100644
--- a/test/tarantool-tests/unit-jit-parse.test.lua
+++ b/test/tarantool-tests/unit-jit-parse.test.lua
@@ -39,7 +39,7 @@ local loop_trace = traces[1]
 for irnum = 1, N_TESTS do
   local ir_pattern = expected_irs[irnum]
   local irref = loop_trace:has_ir(ir_pattern)
-  test:ok(irref, 'find IR refernce by pattern: ' .. ir_pattern)
+  test:ok(irref, 'find IR reference by pattern: ' .. ir_pattern)
 end
 
 os.exit(test:check() and 0 or 1)
diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
index d865267b..2c09db90 100644
--- a/tools/memprof/parse.lua
+++ b/tools/memprof/parse.lua
@@ -52,14 +52,14 @@ local function link_to_previous(heap_chunk, e, nsize)
     if not e.primary[heap_chunk[2]] then
       e.primary[heap_chunk[2]] = {
         loc = heap_chunk[3],
-        alloced = 0,
+        allocated = 0,
         freed = 0,
         count = 0,
       }
     end
     -- Save information about delta for memory heap.
     local location_data = e.primary[heap_chunk[2]]
-    location_data.alloced = location_data.alloced + nsize
+    location_data.allocated = location_data.allocated + nsize
     location_data.freed = location_data.freed + heap_chunk[1]
     location_data.count = location_data.count + 1
   end
diff --git a/tools/memprof/process.lua b/tools/memprof/process.lua
index 0bcb965b..9dc202ae 100644
--- a/tools/memprof/process.lua
+++ b/tools/memprof/process.lua
@@ -39,8 +39,8 @@ function M.form_heap_delta(events, symbols)
       for _, heap_chunk in pairs(event.primary) do
         local ev_line = symtab.demangle(symbols, heap_chunk.loc)
 
-        if (heap_chunk.alloced > 0) then
-          dheap[ev_line].dbytes = dheap[ev_line].dbytes + heap_chunk.alloced
+        if (heap_chunk.allocated > 0) then
+          dheap[ev_line].dbytes = dheap[ev_line].dbytes + heap_chunk.allocated
           dheap[ev_line].nalloc = dheap[ev_line].nalloc + heap_chunk.count
         end
 
-- 
2.34.1

[Tarantool-patches] [PATCH 2/2] ci: enable checkpatch

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <sergeyb@tarantool.org>

Patch enables checkpatch [1] for checking patch on a pre-commit stage.
In Tarantool we use our own fork of checkpatch [2] with additional check
types. It's logical to use it in a LuaJIT development. We don't need
check tags in commit messages like NO_DOC, NO_CHANGELOG, NO_TEST and
others, so to be able to customize command-line options Github Action, provided
by checkpatch repository [3], was added to the repository.

See documentation for used checkpatch in [4].

1. https://docs.kernel.org/dev-tools/checkpatch.html
2. https://github.com/tarantool/checkpatch
3. https://github.com/tarantool/checkpatch/blob/master/.github/actions/checkpatch/action.yml
4. https://github.com/tarantool/checkpatch/blob/master/doc/checkpatch.rst
---
 .github/actions/checkpatch/action.yml | 17 +++++++++++++++++
 .github/workflows/lint.yml            | 14 ++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 .github/actions/checkpatch/action.yml

diff --git a/.github/actions/checkpatch/action.yml b/.github/actions/checkpatch/action.yml
new file mode 100644
index 00000000..df2e2a2b
--- /dev/null
+++ b/.github/actions/checkpatch/action.yml
@@ -0,0 +1,17 @@
+name: Checkpatch
+description: Check patches against LuaJIT development guidelines
+inputs:
+  revision-range:
+    description: Git revision range to check
+    required: true
+runs:
+  using: composite
+  steps:
+    - uses: actions/checkout@v3
+      with:
+        repository: tarantool/checkpatch
+        path: 'checkpatch'
+    - run: apt install -y codespell
+      shell: bash
+    - run: checkpatch/checkpatch.pl --codespell --color=always --show-types --git ${{ inputs.revision-range }} --codespellfile /usr/lib/python3/dist-packages/codespell_lib/data/dictionary.txt --ignore NO_CHANGELOG,NO_DOC,NO_TEST,COMMIT_LOG_LONG_LINE
+      shell: bash
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 71ceee9a..d28aa15b 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -53,3 +53,17 @@ jobs:
       - name: test
         run: cmake --build . --target LuaJIT-luacheck
         working-directory: ${{ env.BUILDDIR }}
+
+  checkpatch:
+    runs-on: [self-hosted, lightweight, Linux, x86_64]
+
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+          # ref: ${{ github.event.pull_request.head.sha }}
+          submodules: recursive
+      - name: checkpatch
+        uses: ./.github/actions/checkpatch
+        with:
+          revision-range: HEAD~${{ github.event.pull_request.commits }}..HEAD
-- 
2.34.1

Re: [Tarantool-patches] [PATCH 1/2] test: fix typos

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!
Thanks for the patch!
LGTM, except some comments below:

1) I suppose it should be named `refactoring: fix typos`, since we also
changed debugger scripts and tools files. Feel free to split this commit
into three for each subsystem, but I see no need in this.

On 11.07.23, Sergey Bronnikov wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> Fix typos found with codespell in a files with our own source code.

Typo: s/a files/files/

> ---
>  src/luajit-gdb.py                                           | 6 +++---
>  src/luajit_lldb.py                                          | 6 +++---
>  test/LuaJIT-tests/src/ctest.c                               | 2 +-
>  test/tarantool-tests/CMakeLists.txt                         | 2 +-
>  test/tarantool-tests/fix-emit-rma.test.lua                  | 2 +-
>  test/tarantool-tests/gh-4199-gc64-fuse.test.lua             | 2 +-
>  .../gh-6096-external-unwinding-on-arm64.test.lua            | 2 +-
>  test/tarantool-tests/gh-6163-min-max.test.lua               | 2 +-
>  test/tarantool-tests/lj-416-xor-before-jcc.test.lua         | 2 +-
>  .../lj-426-arm64-incorrect-check-closed-uv.test.lua         | 2 +-
>  .../lj-512-profiler-hook-finalizers.test.lua                | 4 ++--
>  test/tarantool-tests/lj-603-err-snap-restore.test.lua       | 2 +-
>  test/tarantool-tests/misclib-memprof-lapi.test.lua          | 6 +++---
>  test/tarantool-tests/tap.lua                                | 4 ++--
>  test/tarantool-tests/unit-jit-parse.test.lua                | 2 +-
>  tools/memprof/parse.lua                                     | 4 ++--
>  tools/memprof/process.lua                                   | 4 ++--
>  17 files changed, 27 insertions(+), 27 deletions(-)
> 

<snipped>

> diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
> index d865267b..2c09db90 100644
> --- a/tools/memprof/parse.lua
> +++ b/tools/memprof/parse.lua
> @@ -52,14 +52,14 @@ local function link_to_previous(heap_chunk, e, nsize)
>      if not e.primary[heap_chunk[2]] then
>        e.primary[heap_chunk[2]] = {
>          loc = heap_chunk[3],
> -        alloced = 0,
> +        allocated = 0,
>          freed = 0,
>          count = 0,
>        }
>      end
>      -- Save information about delta for memory heap.
>      local location_data = e.primary[heap_chunk[2]]
> -    location_data.alloced = location_data.alloced + nsize
> +    location_data.allocated = location_data.allocated + nsize
>      location_data.freed = location_data.freed + heap_chunk[1]
>      location_data.count = location_data.count + 1
>    end
> diff --git a/tools/memprof/process.lua b/tools/memprof/process.lua
> index 0bcb965b..9dc202ae 100644
> --- a/tools/memprof/process.lua
> +++ b/tools/memprof/process.lua
> @@ -39,8 +39,8 @@ function M.form_heap_delta(events, symbols)
>        for _, heap_chunk in pairs(event.primary) do
>          local ev_line = symtab.demangle(symbols, heap_chunk.loc)
>  
> -        if (heap_chunk.alloced > 0) then
> -          dheap[ev_line].dbytes = dheap[ev_line].dbytes + heap_chunk.alloced
> +        if (heap_chunk.allocated > 0) then
> +          dheap[ev_line].dbytes = dheap[ev_line].dbytes + heap_chunk.allocated
>            dheap[ev_line].nalloc = dheap[ev_line].nalloc + heap_chunk.count
>          end

2) Can we avoid such changes in the not-string/non-comment chunks?

>  
> -- 
> 2.34.1
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH 1/2] test: fix typos

[Sergey Bronnikov via Tarantool-patches]

Thanks for review!

Updated branch force-pushed.


On 7/14/23 14:53, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the patch!
> LGTM, except some comments below:
>
> 1) I suppose it should be named `refactoring: fix typos`, since we also
> changed debugger scripts and tools files. Feel free to split this commit
> into three for each subsystem, but I see no need in this.

Agreed offline to change to "codehealth".


> On 11.07.23, Sergey Bronnikov wrote:
>> From: Sergey Bronnikov <sergeyb@tarantool.org>
>>
>> Fix typos found with codespell in a files with our own source code.
> Typo: s/a files/files/
Fixed!
<snipped>
>> diff --git a/tools/memprof/process.lua b/tools/memprof/process.lua
>> index 0bcb965b..9dc202ae 100644
>> --- a/tools/memprof/process.lua
>> +++ b/tools/memprof/process.lua
>> @@ -39,8 +39,8 @@ function M.form_heap_delta(events, symbols)
>>         for _, heap_chunk in pairs(event.primary) do
>>           local ev_line = symtab.demangle(symbols, heap_chunk.loc)
>>   
>> -        if (heap_chunk.alloced > 0) then
>> -          dheap[ev_line].dbytes = dheap[ev_line].dbytes + heap_chunk.alloced
>> +        if (heap_chunk.allocated > 0) then
>> +          dheap[ev_line].dbytes = dheap[ev_line].dbytes + heap_chunk.allocated
>>             dheap[ev_line].nalloc = dheap[ev_line].nalloc + heap_chunk.count
>>           end
> 2) Can we avoid such changes in the not-string/non-comment chunks?

I suppose no in checkpatch. These typos were found by manual codespell run.

Re: [Tarantool-patches] [PATCH 2/2] ci: enable checkpatch

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!
Thanks for the patch!
Please, consider my comments below.

On 11.07.23, Sergey Bronnikov wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> Patch enables checkpatch [1] for checking patch on a pre-commit stage.

Minor: Strictly saying it's not a precommit stage (since it's not a
pre-commit hook). But this is a prerequisite for other testing workflow.

> In Tarantool we use our own fork of checkpatch [2] with additional check
> types. It's logical to use it in a LuaJIT development. We don't need

Also, it is good to have some checks for our C code like:
* <src/lib_misc.c>
* <src/lj_mapi.c>
* <src/lj_memprof.[ch]>
* <src/lj_symtab.[ch]>
* <src/lj_sysprof.[ch]>
* <src/lj_utils.h>
* <src/lj_utils_leb128.c>
* <src/lj_wbuf.[ch]>
* <src/lmisclib.h>
But they are contradicting with Tarantool's guidelines as far as they
are written in LuaJIT's style. Have we some way to check them too?

> check tags in commit messages like NO_DOC, NO_CHANGELOG, NO_TEST and

Side note: Not so sure, that NO_TEST check is a good thing, but I'm OK
with it, since it may be useful for backporting refactoring changes.

> others, so to be able to customize command-line options Github Action, provided
> by checkpatch repository [3], was added to the repository.
> 
> See documentation for used checkpatch in [4].
> 
> 1. https://docs.kernel.org/dev-tools/checkpatch.html
> 2. https://github.com/tarantool/checkpatch
> 3. https://github.com/tarantool/checkpatch/blob/master/.github/actions/checkpatch/action.yml
> 4. https://github.com/tarantool/checkpatch/blob/master/doc/checkpatch.rst

Also, is it possible to create the make target similar to
LuaJIT-luacheck? It should:
0) Be dummy if there is no chekcpatch or codespell installed.
1) Be included in the make test check.
It is useful for local spellcheck without any push to remote CI.

> ---
>  .github/actions/checkpatch/action.yml | 17 +++++++++++++++++
>  .github/workflows/lint.yml            | 14 ++++++++++++++
>  2 files changed, 31 insertions(+)
>  create mode 100644 .github/actions/checkpatch/action.yml
> 
> diff --git a/.github/actions/checkpatch/action.yml b/.github/actions/checkpatch/action.yml
> new file mode 100644
> index 00000000..df2e2a2b
> --- /dev/null
> +++ b/.github/actions/checkpatch/action.yml
> @@ -0,0 +1,17 @@
> +name: Checkpatch
> +description: Check patches against LuaJIT development guidelines
> +inputs:
> +  revision-range:
> +    description: Git revision range to check
> +    required: true
> +runs:
> +  using: composite
> +  steps:
> +    - uses: actions/checkout@v3
> +      with:
> +        repository: tarantool/checkpatch
> +        path: 'checkpatch'
> +    - run: apt install -y codespell
> +      shell: bash
> +    - run: checkpatch/checkpatch.pl --codespell --color=always --show-types --git ${{ inputs.revision-range }} --codespellfile /usr/lib/python3/dist-packages/codespell_lib/data/dictionary.txt --ignore NO_CHANGELOG,NO_DOC,NO_TEST,COMMIT_LOG_LONG_LINE

Please, split this line into several.

> +      shell: bash
> diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
> index 71ceee9a..d28aa15b 100644
> --- a/.github/workflows/lint.yml
> +++ b/.github/workflows/lint.yml
> @@ -53,3 +53,17 @@ jobs:
>        - name: test
>          run: cmake --build . --target LuaJIT-luacheck
>          working-directory: ${{ env.BUILDDIR }}
> +
> +  checkpatch:
> +    runs-on: [self-hosted, lightweight, Linux, x86_64]
> +
> +    steps:
> +      - uses: actions/checkout@v3
> +        with:
> +          fetch-depth: 0
> +          # ref: ${{ github.event.pull_request.head.sha }}

Minor: Why do we need this comment?

> +          submodules: recursive
> +      - name: checkpatch
> +        uses: ./.github/actions/checkpatch
> +        with:
> +          revision-range: HEAD~${{ github.event.pull_request.commits }}..HEAD

There is something wrong with this definition; in CI [1] it changes to the
following:

| checkpatch/checkpatch.pl --codespell --color=always --show-types --git HEAD~..HEAD --ignore NO_CHANGELOG,NO_DOC,NO_TEST,COMMIT_LOG_LONG_LINE

So, only the top commit is verified (instead of 2 -- this is why there
is a typo in the previous commit message).

Also, is it possible to do this activity on push instead, depending on
difference with tarantool/master HEAD?

Also, how can we avoid some errors here, if this will show incorrect
spelling in the backported patches?

> -- 
> 2.34.1
> 

[1]: https://github.com/tarantool/luajit/actions/runs/5519618350/jobs/10065116711#step:3:143

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH 2/2] ci: enable checkpatch

[Sergey Bronnikov via Tarantool-patches]

Hi, Sergey!

thanks for review!

Fixes applied and force-pushed.


On 7/14/23 15:45, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the patch!
> Please, consider my comments below.
>
> On 11.07.23, Sergey Bronnikov wrote:
>> From: Sergey Bronnikov <sergeyb@tarantool.org>
>>
>> Patch enables checkpatch [1] for checking patch on a pre-commit stage.
> Minor: Strictly saying it's not a precommit stage (since it's not a
> pre-commit hook). But this is a prerequisite for other testing workflow.
>
>> In Tarantool we use our own fork of checkpatch [2] with additional check
>> types. It's logical to use it in a LuaJIT development. We don't need
> Also, it is good to have some checks for our C code like:
> * <src/lib_misc.c>
> * <src/lj_mapi.c>
> * <src/lj_memprof.[ch]>
> * <src/lj_symtab.[ch]>
> * <src/lj_sysprof.[ch]>
> * <src/lj_utils.h>
> * <src/lj_utils_leb128.c>
> * <src/lj_wbuf.[ch]>
> * <src/lmisclib.h>
> But they are contradicting with Tarantool's guidelines as far as they
> are written in LuaJIT's style. Have we some way to check them too?


New findings:

diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c
index 2e9ed9b3..745eb1a3 100644
--- a/src/lj_sysprof.c
+++ b/src/lj_sysprof.c
@@ -485,7 +485,7 @@ int lj_sysprof_stop(lua_State *L)
    if (SPS_HALT == sp->state) {
      errno = sp->saved_errno;
      sp->state = SPS_IDLE;
-    /* wbuf was terminated when error occured. */
+    /* wbuf was terminated when error occurred. */
      return PROFILE_ERRIO;
    }

diff --git a/src/lj_wbuf.h b/src/lj_wbuf.h
index 9eaa5e51..33ec8463 100644
--- a/src/lj_wbuf.h
+++ b/src/lj_wbuf.h
@@ -75,7 +75,7 @@ void lj_wbuf_addn(struct lj_wbuf *buf, const void 
*src, size_t n);
  /* Write string to the buffer. */
  void LJ_FASTCALL lj_wbuf_addstring(struct lj_wbuf *buf, const char *s);

-/* Immediatly flush the buffer. */
+/* Immediately flush the buffer. */
  void LJ_FASTCALL lj_wbuf_flush(struct lj_wbuf *buf);

  /* Check flags. */


new fixes were added (amended) to a commit with typos fixes.


>
>> check tags in commit messages like NO_DOC, NO_CHANGELOG, NO_TEST and
> Side note: Not so sure, that NO_TEST check is a good thing, but I'm OK
> with it, since it may be useful for backporting refactoring changes.

I don't remember a case when someone missed a test for the patch with fix,

so I would leave this options disabled.

(Let's not cure what does not hurt.)


>
>> others, so to be able to customize command-line options Github Action, provided
>> by checkpatch repository [3], was added to the repository.
>>
>> See documentation for used checkpatch in [4].
>>
>> 1. https://docs.kernel.org/dev-tools/checkpatch.html
>> 2. https://github.com/tarantool/checkpatch
>> 3. https://github.com/tarantool/checkpatch/blob/master/.github/actions/checkpatch/action.yml
>> 4. https://github.com/tarantool/checkpatch/blob/master/doc/checkpatch.rst
> Also, is it possible to create the make target similar to
> LuaJIT-luacheck? It should:
> 0) Be dummy if there is no chekcpatch or codespell installed.
Added.
> 1) Be included in the make test check.

Didn't get it. What make target did you mean? "test" or "check"?

I've added a new target "check" and added LuaJIT-luacheck and 
LuaJIT-checkpatch there.


> It is useful for local spellcheck without any push to remote CI.
>
>> ---
>>   .github/actions/checkpatch/action.yml | 17 +++++++++++++++++
>>   .github/workflows/lint.yml            | 14 ++++++++++++++
>>   2 files changed, 31 insertions(+)
>>   create mode 100644 .github/actions/checkpatch/action.yml
>>
>> diff --git a/.github/actions/checkpatch/action.yml b/.github/actions/checkpatch/action.yml
>> new file mode 100644
>> index 00000000..df2e2a2b
>> --- /dev/null
>> +++ b/.github/actions/checkpatch/action.yml
>> @@ -0,0 +1,17 @@
>> +name: Checkpatch
>> +description: Check patches against LuaJIT development guidelines
>> +inputs:
>> +  revision-range:
>> +    description: Git revision range to check
>> +    required: true
>> +runs:
>> +  using: composite
>> +  steps:
>> +    - uses: actions/checkout@v3
>> +      with:
>> +        repository: tarantool/checkpatch
>> +        path: 'checkpatch'
>> +    - run: apt install -y codespell
>> +      shell: bash
>> +    - run: checkpatch/checkpatch.pl --codespell --color=always --show-types --git ${{ inputs.revision-range }} --codespellfile /usr/lib/python3/dist-packages/codespell_lib/data/dictionary.txt --ignore NO_CHANGELOG,NO_DOC,NO_TEST,COMMIT_LOG_LONG_LINE
> Please, split this line into several.

Done.


--- a/.github/actions/checkpatch/action.yml
+++ b/.github/actions/checkpatch/action.yml
@@ -13,5 +13,12 @@ runs:
          path: 'checkpatch'
      - run: apt install -y codespell
        shell: bash
-    - run: checkpatch/checkpatch.pl --codespell --color=always 
--show-types --git ${{ inputs.revision-range }} --ignore NO_CHANGELOG,NO_DOC
,NO_TEST,COMMIT_LOG_LONG_LINE
+    - run: checkpatch/checkpatch.pl --codespell \
+                                    --color=always \
+                                    --show-types \
+                                    --git ${{ inputs.revision-range }} \
+                                    --ignore NO_CHANGELOG \
+                                    --ignore NO_DOC \
+                                    --ignore NO_TEST \
+                                    --ignore COMMIT_LOG_LONG_LINE
        shell: bash


>
>> +      shell: bash
>> diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
>> index 71ceee9a..d28aa15b 100644
>> --- a/.github/workflows/lint.yml
>> +++ b/.github/workflows/lint.yml
>> @@ -53,3 +53,17 @@ jobs:
>>         - name: test
>>           run: cmake --build . --target LuaJIT-luacheck
>>           working-directory: ${{ env.BUILDDIR }}
>> +
>> +  checkpatch:
>> +    runs-on: [self-hosted, lightweight, Linux, x86_64]
>> +
>> +    steps:
>> +      - uses: actions/checkout@v3
>> +        with:
>> +          fetch-depth: 0
>> +          # ref: ${{ github.event.pull_request.head.sha }}
> Minor: Why do we need this comment?
Forgot to remove, removed.
>
>> +          submodules: recursive
>> +      - name: checkpatch
>> +        uses: ./.github/actions/checkpatch
>> +        with:
>> +          revision-range: HEAD~${{ github.event.pull_request.commits }}..HEAD
> There is something wrong with this definition; in CI [1] it changes to the
> following:
>
> | checkpatch/checkpatch.pl --codespell --color=always --show-types --git HEAD~..HEAD --ignore NO_CHANGELOG,NO_DOC,NO_TEST,COMMIT_LOG_LONG_LINE
>
> So, only the top commit is verified (instead of 2 -- this is why there
> is a typo in the previous commit message).
Fixed. Now checkpatch always checks a range tarantool/master..HEAD.
>
> Also, is it possible to do this activity on push instead, depending on
> difference with tarantool/master HEAD?
Yes, fixed.
>
> Also, how can we avoid some errors here, if this will show incorrect
> spelling in the backported patches?

Should we avoid these errors?

If yes, then, I suppose, we can ignore them.

>> -- 
>> 2.34.1
>>
> [1]: https://github.com/tarantool/luajit/actions/runs/5519618350/jobs/10065116711#step:3:143