Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements
@ 2022-08-11 11:17 Igor Munkin via Tarantool-patches
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 1/8] test: introduce LUAJIT_TEST_VARDIR variable Igor Munkin via Tarantool-patches
                   ` (8 more replies)
  0 siblings, 9 replies; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-11 11:17 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

This patchset contains several enhancements:
* The first patch introduces LUAJIT_TEST_VARDIR required by #7472[1].
* The three next patches fix Tarantool testing suite to be run out of
  LuaJIT source tree and replace in source build in GitHub Actions.
* The fifth patch removes the excess line from macOS M1 workflow.
* The sixth patch removes arch prefix for macOS M1 workflow.
* The last two patches merge all testing workflows into a single one.

Branch: https://github.com/tarantool/luajit/tree/imun/tweak-tests

Igor Munkin (8):
  test: introduce LUAJIT_TEST_VARDIR variable
  test: introduce MakeLuaPath.cmake helper
  test: fix tarantool suite for out of source build
  ci: use out of source build in GitHub Actions
  ci: remove excess parallel level setup
  ci: remove arch prefix for macOS M1 workflow
  ci: merge x86_64 and ARM64 workflows
  ci: merge Linux and macOS workflows

 .github/actions/environment/action.yml        |  13 --
 .github/actions/setup-linux/README.md         |  12 ++
 .github/actions/setup-linux/action.yml        |  19 ++
 .github/actions/setup-macos/README.md         |  12 ++
 .github/actions/setup-macos/action.yml        |  29 +++
 .../actions/{environment => setup}/README.md  |   5 +-
 .github/actions/setup/action.yml              |  10 +
 .github/workflows/lint.yml                    |  11 +-
 .github/workflows/linux-aarch64.yml           |  79 -------
 .github/workflows/linux-x86_64-ninja.yml      |  12 +-
 .github/workflows/linux-x86_64.yml            |  98 ---------
 .github/workflows/macos-m1.yml                |  94 --------
 .github/workflows/macos-x86_64.yml            | 107 ----------
 .github/workflows/testing.yml                 | 200 ++++++++++++++++++
 cmake/MakeLuaPath.cmake                       |  46 ++++
 test/CMakeLists.txt                           |   2 +
 test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt     |   8 +-
 test/lua-Harness-tests/CMakeLists.txt         |  16 +-
 test/tarantool-tests/CMakeLists.txt           |  37 ++--
 .../gh-5813-resolving-of-c-symbols.test.lua   |   6 +-
 .../misclib-memprof-lapi.test.lua             |  22 +-
 .../misclib-sysprof-lapi.test.lua             |   8 +-
 test/tarantool-tests/utils.lua                |  12 ++
 23 files changed, 423 insertions(+), 435 deletions(-)
 delete mode 100644 .github/actions/environment/action.yml
 create mode 100644 .github/actions/setup-linux/README.md
 create mode 100644 .github/actions/setup-linux/action.yml
 create mode 100644 .github/actions/setup-macos/README.md
 create mode 100644 .github/actions/setup-macos/action.yml
 rename .github/actions/{environment => setup}/README.md (72%)
 create mode 100644 .github/actions/setup/action.yml
 delete mode 100644 .github/workflows/linux-aarch64.yml
 delete mode 100644 .github/workflows/linux-x86_64.yml
 delete mode 100644 .github/workflows/macos-m1.yml
 delete mode 100644 .github/workflows/macos-x86_64.yml
 create mode 100644 .github/workflows/testing.yml
 create mode 100644 cmake/MakeLuaPath.cmake

-- 
2.34.0


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Tarantool-patches] [PATCH luajit 1/8] test: introduce LUAJIT_TEST_VARDIR variable
  2022-08-11 11:17 [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements Igor Munkin via Tarantool-patches
@ 2022-08-11 11:17 ` Igor Munkin via Tarantool-patches
  2022-08-15 12:08   ` Sergey Bronnikov via Tarantool-patches
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 2/8] test: introduce MakeLuaPath.cmake helper Igor Munkin via Tarantool-patches
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-11 11:17 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

Before the patch both memprof and sysprof artefacts are generated within
the binary artefacts tree (i.e. in the same directory LuaJIT binary is
generated). However, more convenient way is producing these temporary
profiles in a separate directory (e.g. located on the partition with
more strict space limits). As a result of the patch all memprof and
sysprof test chunks consider LUAJIT_TEST_VARDIR environment variable to
set the directory where test profiles are generated. For the case when
LUAJIT_TEST_VARDIR is not set, everything works as before.

Part of tarantool/tarantool#7472

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 .../gh-5813-resolving-of-c-symbols.test.lua   |  6 +++--
 .../misclib-memprof-lapi.test.lua             | 22 ++++++++++---------
 .../misclib-sysprof-lapi.test.lua             |  8 ++++---
 test/tarantool-tests/utils.lua                | 12 ++++++++++
 4 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
index d589dddf..e0b6d86d 100644
--- a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
+++ b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
@@ -1,5 +1,7 @@
 -- Memprof is implemented for x86 and x64 architectures only.
-require("utils").skipcond(
+local utils = require("utils")
+
+utils.skipcond(
   jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux",
   jit.arch.." architecture or "..jit.os..
   " OS is NIY for memprof c symbols resolving"
@@ -18,7 +20,7 @@ local testboth = require "resboth"
 local testhash = require "reshash"
 local testgnuhash = require "resgnuhash"
 
-local TMP_BINFILE = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%.%1.memprofdata.tmp.bin")
+local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin")
 
 local function tree_contains(node, name)
   if node == nil then
diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index a11f0be1..bae0c27c 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -1,5 +1,7 @@
 -- Memprof is implemented for x86 and x64 architectures only.
-require("utils").skipcond(
+local utils = require("utils")
+
+utils.skipcond(
   jit.arch ~= "x86" and jit.arch ~= "x64",
   jit.arch.." architecture is NIY for memprof"
 )
@@ -26,8 +28,8 @@ local memprof = require "memprof.parse"
 local process = require "memprof.process"
 local symtab = require "utils.symtab"
 
-local TMP_BINFILE = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%.%1.memprofdata.tmp.bin")
-local BAD_PATH = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%1/memprofdata.tmp.bin")
+local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin")
+local BAD_PATH = utils.profilename("memprofdata/tmp.bin")
 local SRC_PATH = "@"..arg[0]
 
 local function default_payload()
@@ -189,9 +191,9 @@ test:test("output", function(subtest)
   -- one is the number of allocations. 1 event - alocation 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 = 35, linedefined = 33 }, 2))
+  subtest:ok(check_alloc_report(alloc, { line = 37, linedefined = 35 }, 2))
   -- 20 strings allocations.
-  subtest:ok(check_alloc_report(alloc, { line = 40, linedefined = 33 }, 20))
+  subtest:ok(check_alloc_report(alloc, { line = 42, linedefined = 35 }, 20))
 
   -- Collect all previous allocated objects.
   subtest:ok(free.INTERNAL.num == 22)
@@ -199,8 +201,8 @@ test:test("output", function(subtest)
   -- Tests for leak-only option.
   -- See also https://github.com/tarantool/tarantool/issues/5812.
   local heap_delta = process.form_heap_delta(events, symbols)
-  local tab_alloc_stats = heap_delta[form_source_line(35)]
-  local str_alloc_stats = heap_delta[form_source_line(40)]
+  local tab_alloc_stats = heap_delta[form_source_line(37)]
+  local str_alloc_stats = heap_delta[form_source_line(42)]
   subtest:ok(tab_alloc_stats.nalloc == tab_alloc_stats.nfree)
   subtest:ok(tab_alloc_stats.dbytes == 0)
   subtest:ok(str_alloc_stats.nalloc == str_alloc_stats.nfree)
@@ -291,10 +293,10 @@ test:test("jit-output", function(subtest)
   -- 10 allocations in interpreter mode, 1 allocation for a trace
   -- recording and assembling and next 9 allocations will happen
   -- while running the trace.
-  subtest:ok(check_alloc_report(alloc, { line = 40, linedefined = 33 }, 11))
-  subtest:ok(check_alloc_report(alloc, { traceno = 1, line = 38 }, 9))
+  subtest:ok(check_alloc_report(alloc, { line = 42, linedefined = 35 }, 11))
+  subtest:ok(check_alloc_report(alloc, { traceno = 1, line = 40 }, 9))
   -- See same checks with jit.off().
-  subtest:ok(check_alloc_report(alloc, { line = 35, linedefined = 33 }, 2))
+  subtest:ok(check_alloc_report(alloc, { line = 37, linedefined = 35 }, 2))
 
   -- Restore default JIT settings.
   jit.opt.start(unpack(jit_opt_default))
diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
index 9e0a8a77..dbff41b0 100644
--- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
@@ -1,6 +1,8 @@
 -- Sysprof is implemented for x86 and x64 architectures only.
 local ffi = require("ffi")
-require("utils").skipcond(
+local utils = require("utils")
+
+utils.skipcond(
   jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux"
     or ffi.abi("gc64"),
   jit.arch.." architecture or "..jit.os..
@@ -19,8 +21,8 @@ local bufread = require("utils.bufread")
 local symtab = require("utils.symtab")
 local sysprof = require("sysprof.parse")
 
-local TMP_BINFILE = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%.%1.sysprofdata.tmp.bin")
-local BAD_PATH = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%1/sysprofdata.tmp.bin")
+local TMP_BINFILE = utils.profilename("sysprofdata.tmp.bin")
+local BAD_PATH = utils.profilename("sysprofdata/tmp.bin")
 
 local function payload()
   local function fib(n)
diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
index a1906498..87f7ff15 100644
--- a/test/tarantool-tests/utils.lua
+++ b/test/tarantool-tests/utils.lua
@@ -123,4 +123,16 @@ function M.hasbc(f, bytecode)
   return hasbc
 end
 
+function M.profilename(name)
+  local vardir = os.getenv('LUAJIT_TEST_VARDIR')
+  -- Replace pattern will change directory name of the generated
+  -- profile to LUAJIT_TEST_VARDIR if it is set in the process
+  -- environment. Otherwise, the original dirname is left intact.
+  -- As a basename for this profile the test name is concatenated
+  -- with the name given as an argument.
+  local replacepattern = ('%s/%s-%s'):format(vardir or '%1', '%2', name)
+  -- XXX: return only the resulting string.
+  return (arg[0]:gsub('^(.+)/([^/]+)%.test%.lua$', replacepattern))
+end
+
 return M
-- 
2.34.0


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Tarantool-patches] [PATCH luajit 2/8] test: introduce MakeLuaPath.cmake helper
  2022-08-11 11:17 [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements Igor Munkin via Tarantool-patches
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 1/8] test: introduce LUAJIT_TEST_VARDIR variable Igor Munkin via Tarantool-patches
@ 2022-08-11 11:17 ` Igor Munkin via Tarantool-patches
  2022-08-15 12:08   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18  9:37   ` Sergey Kaplun via Tarantool-patches
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 3/8] test: fix tarantool suite for out of source build Igor Munkin via Tarantool-patches
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-11 11:17 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

While extending test suites it is often required to append additional
path where Lua or Lua-C auxiliary modules are located to LUA_PATH or
LUA_CPATH environment variables. Due to insane semicolon interpolation
in CMake strings (that converts such string to a list as a result), we
need to escape semicolon in LUA_PATH/LUA_CPATH strings while building
the resulting value.

After the years of struggling MakeLuaPath.cmake module is introduced to
make LUA_PATH and LUA_CPATH definition convenient with <make_lua_path>
helper. This function takes all paths given as a variable list argument,
joins them in a reverse order by a semicolon and yields the resulting
string to a specified CMake variable.

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 cmake/MakeLuaPath.cmake                   | 46 +++++++++++++++++++++++
 test/CMakeLists.txt                       |  2 +
 test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt |  8 +++-
 test/lua-Harness-tests/CMakeLists.txt     | 16 +++++---
 test/tarantool-tests/CMakeLists.txt       | 28 ++++++++------
 5 files changed, 81 insertions(+), 19 deletions(-)
 create mode 100644 cmake/MakeLuaPath.cmake

diff --git a/cmake/MakeLuaPath.cmake b/cmake/MakeLuaPath.cmake
new file mode 100644
index 00000000..9a5a3bb8
--- /dev/null
+++ b/cmake/MakeLuaPath.cmake
@@ -0,0 +1,46 @@
+# make_lua_path provides a convenient way to define LUA_PATH and
+# LUA_CPATH variables.
+#
+# Example usage:
+#
+#   make_lua_path(LUA_PATH
+#     PATH
+#       ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
+#       ${CMAKE_BINARY_DIR}/?.lua
+#       ./?.lua
+#   )
+#
+# This will give you the string:
+#    "./?.lua;${CMAKE_BINARY_DIR}/?.lua;${CMAKE_CURRENT_SOURCE_DIR}/?.lua;;"
+# XXX: Mind the reverse order of the entries in the result string.
+
+function(make_lua_path path)
+  set(prefix ARG)
+  set(noValues)
+  set(singleValues)
+  set(multiValues PATHS)
+
+  # FIXME: if we update to CMake >= 3.5, can remove this line.
+  include(CMakeParseArguments)
+  cmake_parse_arguments(${prefix}
+                        "${noValues}"
+                        "${singleValues}"
+                        "${multiValues}"
+                        ${ARGN})
+
+  # XXX: This is the sentinel semicolon having special meaning
+  # for LUA_PATH and LUA_CPATH variables. For more info, see the
+  # link below:
+  # https://www.lua.org/manual/5.1/manual.html#pdf-LUA_PATH
+  set(result "\;")
+
+  foreach(inc ${ARG_PATHS})
+    # XXX: If one joins two strings with semicolon, the value
+    # automatically becomes a list. I found a single working
+    # solution to make result variable be a string via "escaping"
+    # the semicolon right in string interpolation.
+    set(result "${inc}\;${result}")
+  endforeach()
+
+  set(${path} "${result}" PARENT_SCOPE)
+endfunction()
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index ba25af54..a8262b12 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -3,6 +3,8 @@
 # See the rationale in the root CMakeLists.txt.
 cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
 
+include(MakeLuaPath)
+
 find_program(LUACHECK luacheck)
 if(LUACHECK)
   # XXX: The tweak below relates to luacheck problem with paths.
diff --git a/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt b/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
index 8e825f55..b95e7852 100644
--- a/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
+++ b/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
@@ -14,7 +14,11 @@ cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
 # https://github.com/tarantool/tarantool/issues/5744
 # Hence, there is no way other than set LUA_PATH environment
 # variable as proposed in the first case.
-set(LUA_PATH "?\;${CMAKE_CURRENT_SOURCE_DIR}/?.lua")
+make_lua_path(LUA_PATH
+  PATHS
+    ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
+    "?"
+)
 
 # Establish PUC-Rio-Lua-5.1-tests-prepare target that contains
 # rules for <libs/*> libraries compilation and creates <libs/P1>
@@ -38,7 +42,7 @@ add_custom_command(TARGET PUC-Rio-Lua-5.1-tests
   COMMENT "Running PUC-Rio Lua 5.1 tests"
   COMMAND
   env
-    LUA_PATH="${LUA_PATH}\;\;"
+    LUA_PATH="${LUA_PATH}"
     ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/all.lua
   WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
 )
diff --git a/test/lua-Harness-tests/CMakeLists.txt b/test/lua-Harness-tests/CMakeLists.txt
index 12476171..a57104a5 100644
--- a/test/lua-Harness-tests/CMakeLists.txt
+++ b/test/lua-Harness-tests/CMakeLists.txt
@@ -10,10 +10,16 @@ if(NOT PROVE)
   return()
 endif()
 
-# Tests create temporary files (see 303-package.t and 411-luajit.t for
-# example) to require. Also, they require some files from original test 
-# source directory.
-set(LUA_PATH "./?.lua\;${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${LUAJIT_SOURCE_DIR}/?.lua\;${LUAJIT_BINARY_DIR}/?.lua")
+# Tests create temporary files (see 303-package.t and 411-luajit.t
+# for example) to be required. Also, they require some files from
+# the original test source directory.
+make_lua_path(LUA_PATH
+  PATHS
+    ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
+    ${LUAJIT_BINARY_DIR}/?.lua
+    ${LUAJIT_SOURCE_DIR}/?.lua
+    ./?.lua
+)
 set(LUA_TEST_FLAGS --failures --shuffle)
 
 if(CMAKE_VERBOSE_MAKEFILE)
@@ -25,7 +31,7 @@ add_custom_command(TARGET lua-Harness-tests
   COMMENT "Running lua-Harness tests"
   COMMAND
   env
-    LUA_PATH="${LUA_PATH}\;"
+    LUA_PATH="${LUA_PATH}"
     ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR}
       --exec '${LUAJIT_TEST_COMMAND} -l profile_luajit21'
       --jobs ${CMAKE_BUILD_PARALLEL_LEVEL}
diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index ecda2e63..27866869 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -43,14 +43,11 @@ macro(BuildTestCLib lib sources)
   # semicolon. If one finds the normal way to make it work, feel
   # free to reach me.
   set(TESTLIBS "${lib};${TESTLIBS}" PARENT_SCOPE)
-  # Add the directory where the lib is built to the LUA_CPATH
-  # environment variable, so LuaJIT can find and load it.
-  # XXX: Here we see the other side of the coin. If one joins two
-  # strings with semicolon, the value automatically becomes a
-  # list. I found a single working solution to make LUA_CPATH be
-  # a string via "escaping" the semicolon right in string
-  # interpolation.
-  set(LUA_CPATH "${CMAKE_CURRENT_BINARY_DIR}/?${CMAKE_SHARED_LIBRARY_SUFFIX}\;${LUA_CPATH}" PARENT_SCOPE)
+  # Add the directory where the lib is built to the list with
+  # entries for LUA_CPATH environment variable, so LuaJIT can find
+  # and load it. See the comment about extending the list in the
+  # parent scope few lines above.
+  set(LUA_CPATHS "${CMAKE_CURRENT_BINARY_DIR}/?${CMAKE_SHARED_LIBRARY_SUFFIX};${LUA_CPATHS}" PARENT_SCOPE)
   # Also add this directory to LD_LIBRARY_PATH environment
   # variable, so FFI machinery can find and load it.
   set(LD_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}:${LD_LIBRARY_PATH}" PARENT_SCOPE)
@@ -76,14 +73,21 @@ add_subdirectory(misclib-sysprof-capi)
 # in src/ directory and auxiliary tests-related modules are
 # located in the current directory (but tests are run in the
 # binary directory), so LUA_PATH need to be updated.
-set(LUA_PATH
-  "${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${PROJECT_SOURCE_DIR}/tools/?.lua\;${PROJECT_SOURCE_DIR}/src/?.lua"
+make_lua_path(LUA_PATH
+  PATHS
+    ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
+    ${PROJECT_SOURCE_DIR}/tools/?.lua
+    ${PROJECT_SOURCE_DIR}/src/?.lua
 )
+# Update LUA_CPATH with the library paths collected within
+# <BuildTestLib> macro.
+make_lua_path(LUA_CPATH PATHS ${LUA_CPATHS})
+
 set(LUA_TEST_SUFFIX .test.lua)
 set(LUA_TEST_FLAGS --failures --shuffle)
 set(LUA_TEST_ENV
-  "LUA_PATH=\"${LUA_PATH}\;\;\""
-  "LUA_CPATH=\"${LUA_CPATH}\;\;\""
+  "LUA_PATH=\"${LUA_PATH}\""
+  "LUA_CPATH=\"${LUA_CPATH}\""
 )
 
 if(CMAKE_VERBOSE_MAKEFILE)
-- 
2.34.0


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Tarantool-patches] [PATCH luajit 3/8] test: fix tarantool suite for out of source build
  2022-08-11 11:17 [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements Igor Munkin via Tarantool-patches
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 1/8] test: introduce LUAJIT_TEST_VARDIR variable Igor Munkin via Tarantool-patches
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 2/8] test: introduce MakeLuaPath.cmake helper Igor Munkin via Tarantool-patches
@ 2022-08-11 11:17 ` Igor Munkin via Tarantool-patches
  2022-08-15 12:10   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18  9:49   ` Sergey Kaplun via Tarantool-patches
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 4/8] ci: use out of source build in GitHub Actions Igor Munkin via Tarantool-patches
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-11 11:17 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

jit/vmdef.lua is autogenerated file, so it's put to src/ directory
located in scope of the binary artefacts tree. Before the patch LUA_PATH
lacks this path, so tarantool-tests target fails due to jit/vmdef.lua
misseek. As a result of this change src/ directory in scope of the
binary tree is included to LUA_PATH as well as the one from the source
tree has been.

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 test/tarantool-tests/CMakeLists.txt | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 27866869..97c23670 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -70,14 +70,17 @@ add_subdirectory(misclib-sysprof-capi)
 
 # The part of the memory profiler toolchain is located in tools
 # directory, jit, profiler, and bytecode toolchains are located
-# in src/ directory and auxiliary tests-related modules are
-# located in the current directory (but tests are run in the
-# binary directory), so LUA_PATH need to be updated.
+# in src/ directory, jit/vmdef.lua is autogenerated file also
+# located in src/ directory, but in scope of the binary artefacts
+# tree and auxiliary tests-related modules are located in the
+# current directory (but tests are run in the binary directory),
+# so LUA_PATH need to be updated.
 make_lua_path(LUA_PATH
   PATHS
     ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
     ${PROJECT_SOURCE_DIR}/tools/?.lua
     ${PROJECT_SOURCE_DIR}/src/?.lua
+    ${PROJECT_BINARY_DIR}/src/?.lua
 )
 # Update LUA_CPATH with the library paths collected within
 # <BuildTestLib> macro.
-- 
2.34.0


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Tarantool-patches] [PATCH luajit 4/8] ci: use out of source build in GitHub Actions
  2022-08-11 11:17 [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements Igor Munkin via Tarantool-patches
                   ` (2 preceding siblings ...)
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 3/8] test: fix tarantool suite for out of source build Igor Munkin via Tarantool-patches
@ 2022-08-11 11:17 ` Igor Munkin via Tarantool-patches
  2022-08-15 12:13   ` Sergey Bronnikov via Tarantool-patches
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 5/8] ci: remove excess parallel level setup Igor Munkin via Tarantool-patches
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-11 11:17 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

Use out of source build configuration for LuaJIT testing jobs in all
GitHub workflows. For this build type configuration unique subdirectory
(using GitHub run ID) within runner temporary directory is used as a
binary artefacts tree.

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 .github/actions/environment/action.yml   | 5 +++++
 .github/workflows/lint.yml               | 3 ++-
 .github/workflows/linux-aarch64.yml      | 6 +++++-
 .github/workflows/linux-x86_64-ninja.yml | 6 +++++-
 .github/workflows/linux-x86_64.yml       | 7 ++++++-
 .github/workflows/macos-m1.yml           | 6 +++++-
 .github/workflows/macos-x86_64.yml       | 7 ++++++-
 7 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/.github/actions/environment/action.yml b/.github/actions/environment/action.yml
index 43323bc7..7fb2625f 100644
--- a/.github/actions/environment/action.yml
+++ b/.github/actions/environment/action.yml
@@ -11,3 +11,8 @@ runs:
         NPROC=$(sysctl -n hw.logicalcpu 2>/dev/null || nproc)
         echo CMAKE_BUILD_PARALLEL_LEVEL=$(($NPROC + 1)) | tee -a $GITHUB_ENV
       shell: bash
+    - run: |
+        # Set BUILDDIR environment variable to specify LuaJIT
+        # build directory.
+        echo "BUILDDIR=${{ runner.temp }}/build-${{ github.run_id }}" | tee -a $GITHUB_ENV
+      shell: bash
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 28dc6be6..64e8f992 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -45,6 +45,7 @@ jobs:
           sudo apt -y install cmake make lua5.1 luarocks
           sudo luarocks install luacheck
       - name: configure
-        run: cmake .
+        run: cmake -S . -B ${{ env.BUILDDIR }}
       - name: test
         run: cmake --build . --target LuaJIT-luacheck
+        working-directory: ${{ env.BUILDDIR }}
diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
index 8c8dcff1..21d86764 100644
--- a/.github/workflows/linux-aarch64.yml
+++ b/.github/workflows/linux-aarch64.yml
@@ -53,11 +53,15 @@ jobs:
           sudo apt -y update
           sudo apt -y install cmake gcc make perl
       - name: configure
-        run: cmake . ${{ matrix.CMAKEFLAGS }}
+        run: >
+          cmake -S . -B ${{ env.BUILDDIR }}
+          ${{ matrix.CMAKEFLAGS }}
       - name: build
         run: cmake --build . --parallel
+        working-directory: ${{ env.BUILDDIR }}
       - name: test
         run: cmake --build . --parallel --target test
+        working-directory: ${{ env.BUILDDIR }}
 
   test-tarantool-debug-w-GC64:
     name: Tarantool Debug GC64:ON
diff --git a/.github/workflows/linux-x86_64-ninja.yml b/.github/workflows/linux-x86_64-ninja.yml
index 2877d2f6..72d56d54 100644
--- a/.github/workflows/linux-x86_64-ninja.yml
+++ b/.github/workflows/linux-x86_64-ninja.yml
@@ -44,8 +44,12 @@ jobs:
           sudo apt -y update
           sudo apt -y install cmake gcc ninja-build perl
       - name: configure
-        run: cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -G Ninja
+        run: >
+          cmake -S . -B ${{ env.BUILDDIR }}
+          -DCMAKE_BUILD_TYPE=RelWithDebInfo -G Ninja
       - name: build
         run: cmake --build . --parallel
+        working-directory: ${{ env.BUILDDIR }}
       - name: test
         run: cmake --build . --parallel --target test
+        working-directory: ${{ env.BUILDDIR }}
diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux-x86_64.yml
index 44dcce98..4c3ad4c7 100644
--- a/.github/workflows/linux-x86_64.yml
+++ b/.github/workflows/linux-x86_64.yml
@@ -54,11 +54,16 @@ jobs:
           sudo apt -y update
           sudo apt -y install cmake gcc make perl
       - name: configure
-        run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
+        run: >
+          cmake -S . -B ${{ env.BUILDDIR }}
+          ${{ matrix.CMAKEFLAGS }}
+          -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
       - name: build
         run: cmake --build . --parallel
+        working-directory: ${{ env.BUILDDIR }}
       - name: test
         run: cmake --build . --parallel --target test
+        working-directory: ${{ env.BUILDDIR }}
 
   test-tarantool-debug-wo-GC64:
     name: Tarantool Debug GC64:OFF
diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
index e0269d60..e3b6dcda 100644
--- a/.github/workflows/macos-m1.yml
+++ b/.github/workflows/macos-m1.yml
@@ -68,11 +68,15 @@ jobs:
             ${ARCH} brew upgrade cmake gcc make perl
           ${ARCH} echo "CMAKE_BUILD_PARALLEL_LEVEL=$(($(sysctl -n hw.logicalcpu) + 1))" >> $GITHUB_ENV
       - name: configure
-        run: ${ARCH} cmake . ${{ matrix.CMAKEFLAGS }}
+        run: >
+          ${ARCH} cmake -S . -B ${{ env.BUILDDIR }}
+          ${{ matrix.CMAKEFLAGS }}
       - name: build
         run: ${ARCH} cmake --build . --parallel
+        working-directory: ${{ env.BUILDDIR }}
       - name: test
         run: ${ARCH} cmake --build . --parallel --target test
+        working-directory: ${{ env.BUILDDIR }}
 
   test-tarantool-debug-w-GC64:
     name: Tarantool Debug GC64:ON
diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos-x86_64.yml
index 840806e3..3d2cf581 100644
--- a/.github/workflows/macos-x86_64.yml
+++ b/.github/workflows/macos-x86_64.yml
@@ -63,11 +63,16 @@ jobs:
           brew install --force cmake gcc make perl ||
             brew upgrade cmake gcc make perl
       - name: configure
-        run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
+        run: >
+          cmake -S . -B ${{ env.BUILDDIR }}
+          ${{ matrix.CMAKEFLAGS }}
+          -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
       - name: build
         run: cmake --build . --parallel
+        working-directory: ${{ env.BUILDDIR }}
       - name: test
         run: cmake --build . --parallel --target test
+        working-directory: ${{ env.BUILDDIR }}
 
   test-tarantool-debug-wo-GC64:
     name: Tarantool Debug GC64:OFF
-- 
2.34.0


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Tarantool-patches] [PATCH luajit 5/8] ci: remove excess parallel level setup
  2022-08-11 11:17 [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements Igor Munkin via Tarantool-patches
                   ` (3 preceding siblings ...)
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 4/8] ci: use out of source build in GitHub Actions Igor Munkin via Tarantool-patches
@ 2022-08-11 11:17 ` Igor Munkin via Tarantool-patches
  2022-08-15 12:14   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18 10:09   ` Sergey Kaplun via Tarantool-patches
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 6/8] ci: remove arch prefix for macOS M1 workflow Igor Munkin via Tarantool-patches
                   ` (3 subsequent siblings)
  8 siblings, 2 replies; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-11 11:17 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

This patch removes the artefact line left as a result of squashing
several fixup commits into 36b4b80cbdcf55f5ffdbcea0097c4dc315638c50
("ci: introduce GitHub action for environment setup").

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 .github/workflows/macos-m1.yml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
index e3b6dcda..d5b0e0f1 100644
--- a/.github/workflows/macos-m1.yml
+++ b/.github/workflows/macos-m1.yml
@@ -66,7 +66,6 @@ jobs:
           # if the package already exists with the previous version.
           ${ARCH} brew install --force cmake gcc make perl ||
             ${ARCH} brew upgrade cmake gcc make perl
-          ${ARCH} echo "CMAKE_BUILD_PARALLEL_LEVEL=$(($(sysctl -n hw.logicalcpu) + 1))" >> $GITHUB_ENV
       - name: configure
         run: >
           ${ARCH} cmake -S . -B ${{ env.BUILDDIR }}
-- 
2.34.0


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Tarantool-patches] [PATCH luajit 6/8] ci: remove arch prefix for macOS M1 workflow
  2022-08-11 11:17 [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements Igor Munkin via Tarantool-patches
                   ` (4 preceding siblings ...)
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 5/8] ci: remove excess parallel level setup Igor Munkin via Tarantool-patches
@ 2022-08-11 11:17 ` Igor Munkin via Tarantool-patches
  2022-08-15 12:17   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18 10:14   ` Sergey Kaplun via Tarantool-patches
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 7/8] ci: merge x86_64 and ARM64 workflows Igor Munkin via Tarantool-patches
                   ` (2 subsequent siblings)
  8 siblings, 2 replies; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-11 11:17 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

Finally self-hosted GitHub Actions runners support Apple M1 hardware. As
a result there is no need to explicitly add `arch -arm64` prefix for all
commands being run in scope of macOS M1 workflows.

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 .github/workflows/macos-m1.yml | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
index d5b0e0f1..4e1275f7 100644
--- a/.github/workflows/macos-m1.yml
+++ b/.github/workflows/macos-m1.yml
@@ -28,11 +28,6 @@ concurrency:
     format('{0}-{1}', github.workflow, github.ref) }}
   cancel-in-progress: true
 
-env:
-  # XXX: Github Actions agent uses x86_64 simulation, so we have
-  # to explicitly make it use native environment for job steps.
-  ARCH: arch -arm64
-
 jobs:
   test-luajit:
     runs-on: macos-11-m1
@@ -60,21 +55,21 @@ jobs:
           # XXX: 'echo' command below is required since brew installation
           # script obliges the one to enter a newline for confirming the
           # installation via Ruby script.
-          ${ARCH} brew update ||
-            echo | ${ARCH} /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
+          brew update ||
+            echo | /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
           # Try to install the packages either upgrade it to avoid of fails
           # if the package already exists with the previous version.
-          ${ARCH} brew install --force cmake gcc make perl ||
-            ${ARCH} brew upgrade cmake gcc make perl
+          brew install --force cmake gcc make perl ||
+            brew upgrade cmake gcc make perl
       - name: configure
         run: >
-          ${ARCH} cmake -S . -B ${{ env.BUILDDIR }}
+          cmake -S . -B ${{ env.BUILDDIR }}
           ${{ matrix.CMAKEFLAGS }}
       - name: build
-        run: ${ARCH} cmake --build . --parallel
+        run: cmake --build . --parallel
         working-directory: ${{ env.BUILDDIR }}
       - name: test
-        run: ${ARCH} cmake --build . --parallel --target test
+        run: cmake --build . --parallel --target test
         working-directory: ${{ env.BUILDDIR }}
 
   test-tarantool-debug-w-GC64:
-- 
2.34.0


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Tarantool-patches] [PATCH luajit 7/8] ci: merge x86_64 and ARM64 workflows
  2022-08-11 11:17 [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements Igor Munkin via Tarantool-patches
                   ` (5 preceding siblings ...)
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 6/8] ci: remove arch prefix for macOS M1 workflow Igor Munkin via Tarantool-patches
@ 2022-08-11 11:17 ` Igor Munkin via Tarantool-patches
  2022-08-15 12:22   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18 10:21   ` Sergey Kaplun via Tarantool-patches
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 8/8] ci: merge Linux and macOS workflows Igor Munkin via Tarantool-patches
  2022-11-11  8:56 ` [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements Igor Munkin via Tarantool-patches
  8 siblings, 2 replies; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-11 11:17 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

As a result of the previous commit, there is no difference left between
x86_64 and ARM64 workflows except the runner where workflow is run.
Hence there is no reason to have a separate workflow file for each
hardware architecture.

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 .github/workflows/linux-aarch64.yml           | 83 -----------------
 .../workflows/{linux-x86_64.yml => linux.yml} | 52 ++++++++---
 .github/workflows/macos-m1.yml                | 92 -------------------
 .../workflows/{macos-x86_64.yml => macos.yml} | 52 ++++++++---
 4 files changed, 82 insertions(+), 197 deletions(-)
 delete mode 100644 .github/workflows/linux-aarch64.yml
 rename .github/workflows/{linux-x86_64.yml => linux.yml} (69%)
 delete mode 100644 .github/workflows/macos-m1.yml
 rename .github/workflows/{macos-x86_64.yml => macos.yml} (73%)

diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
deleted file mode 100644
index 21d86764..00000000
--- a/.github/workflows/linux-aarch64.yml
+++ /dev/null
@@ -1,83 +0,0 @@
-name: "Linux/aarch64 test workflow"
-
-on:
-  push:
-    branches-ignore:
-      - '**-notest'
-      - 'upstream-**'
-    tags-ignore:
-      - '**'
-
-concurrency:
-  # An update of a developer branch cancels the previously
-  # scheduled workflow run for this branch. However, the default
-  # branch, and long-term branch (tarantool-1.10, tarantool-2.8,
-  # etc.) workflow runs are never canceled.
-  #
-  # We use a trick here: define the concurrency group as 'workflow
-  # run ID' + # 'workflow run attempt' because it is a unique
-  # combination for any run. So it effectively discards grouping.
-  #
-  # XXX: we cannot use `github.sha` as a unique identifier because
-  # pushing a tag may cancel a run that works on a branch push
-  # event.
-  group: ${{ (
-    github.ref == 'refs/heads/tarantool' ||
-    startsWith(github.ref, 'refs/heads/tarantool-')) &&
-    format('{0}-{1}', github.run_id, github.run_attempt) ||
-    format('{0}-{1}', github.workflow, github.ref) }}
-  cancel-in-progress: true
-
-jobs:
-  test-luajit:
-    runs-on: graviton
-    strategy:
-      fail-fast: false
-      matrix:
-        BUILDTYPE: [Debug, Release]
-        include:
-          - BUILDTYPE: Debug
-            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
-          - BUILDTYPE: Release
-            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
-    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:ON
-    steps:
-      - uses: actions/checkout@v2.3.4
-        with:
-          fetch-depth: 0
-          submodules: recursive
-      - name: environment
-        uses: ./.github/actions/environment
-      - name: setup
-        run: |
-          sudo apt -y update
-          sudo apt -y install cmake gcc make perl
-      - name: configure
-        run: >
-          cmake -S . -B ${{ env.BUILDDIR }}
-          ${{ matrix.CMAKEFLAGS }}
-      - name: build
-        run: cmake --build . --parallel
-        working-directory: ${{ env.BUILDDIR }}
-      - name: test
-        run: cmake --build . --parallel --target test
-        working-directory: ${{ env.BUILDDIR }}
-
-  test-tarantool-debug-w-GC64:
-    name: Tarantool Debug GC64:ON
-    needs: test-luajit
-    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
-    with:
-      GC64: ON
-      buildtype: Debug
-      host: graviton
-      revision: ${{ github.sha }}
-  test-tarantool-release-w-GC64:
-    name: Tarantool Release GC64:ON
-    needs: test-luajit
-    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
-    with:
-      GC64: ON
-      buildtype: RelWithDebInfo
-      host: graviton
-      revision: ${{ github.sha }}
diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux.yml
similarity index 69%
rename from .github/workflows/linux-x86_64.yml
rename to .github/workflows/linux.yml
index 4c3ad4c7..125c8708 100644
--- a/.github/workflows/linux-x86_64.yml
+++ b/.github/workflows/linux.yml
@@ -1,4 +1,4 @@
-name: "Linux/x86_64 test workflow"
+name: "Linux test workflow"
 
 on:
   push:
@@ -30,18 +30,30 @@ concurrency:
 
 jobs:
   test-luajit:
-    runs-on: ubuntu-20.04-self-hosted
     strategy:
       fail-fast: false
       matrix:
+        ARCH: [aarch64, x86_64]
         BUILDTYPE: [Debug, Release]
         GC64: [ON, OFF]
         include:
+          - ARCH: aarch64
+            RUNNER: graviton
+          - ARCH: x86_64
+            RUNNER: ubuntu-20.04-self-hosted
           - BUILDTYPE: Debug
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
           - BUILDTYPE: Release
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
-    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
+        exclude:
+          - ARCH: aarch64
+            GC64: OFF
+    runs-on: ${{ matrix.RUNNER }}
+    name: >
+      LuaJIT
+      ${{ matrix.ARCH }}
+      ${{ matrix.BUILDTYPE }}
+      GC64:${{ matrix.GC64 }}
     steps:
       - uses: actions/checkout@v2.3.4
         with:
@@ -65,8 +77,8 @@ jobs:
         run: cmake --build . --parallel --target test
         working-directory: ${{ env.BUILDDIR }}
 
-  test-tarantool-debug-wo-GC64:
-    name: Tarantool Debug GC64:OFF
+  test-tarantool-x86_64-debug-wo-GC64:
+    name: Tarantool x86_64 Debug GC64:OFF
     needs: test-luajit
     uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
     with:
@@ -74,8 +86,8 @@ jobs:
       buildtype: Debug
       host: ubuntu-20.04-self-hosted
       revision: ${{ github.sha }}
-  test-tarantool-debug-w-GC64:
-    name: Tarantool Debug GC64:ON
+  test-tarantool-x86_64-debug-w-GC64:
+    name: Tarantool x86_64 Debug GC64:ON
     needs: test-luajit
     uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
     with:
@@ -83,8 +95,8 @@ jobs:
       buildtype: Debug
       host: ubuntu-20.04-self-hosted
       revision: ${{ github.sha }}
-  test-tarantool-release-wo-GC64:
-    name: Tarantool Release GC64:OFF
+  test-tarantool-x86_64-release-wo-GC64:
+    name: Tarantool x86_64 Release GC64:OFF
     needs: test-luajit
     uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
     with:
@@ -92,8 +104,8 @@ jobs:
       buildtype: RelWithDebInfo
       host: ubuntu-20.04-self-hosted
       revision: ${{ github.sha }}
-  test-tarantool-release-w-GC64:
-    name: Tarantool Release GC64:ON
+  test-tarantool-x86_64-release-w-GC64:
+    name: Tarantool x86_64 Release GC64:ON
     needs: test-luajit
     uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
     with:
@@ -101,3 +113,21 @@ jobs:
       buildtype: RelWithDebInfo
       host: ubuntu-20.04-self-hosted
       revision: ${{ github.sha }}
+  test-tarantool-aarch64-debug-w-GC64:
+    name: Tarantool aarch64 Debug GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: Debug
+      host: graviton
+      revision: ${{ github.sha }}
+  test-tarantool-aarch64-release-w-GC64:
+    name: Tarantool aarch64 Release GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: RelWithDebInfo
+      host: graviton
+      revision: ${{ github.sha }}
diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
deleted file mode 100644
index 4e1275f7..00000000
--- a/.github/workflows/macos-m1.yml
+++ /dev/null
@@ -1,92 +0,0 @@
-name: "macOS/m1 test workflow"
-
-on:
-  push:
-    branches-ignore:
-      - '**-notest'
-      - 'upstream-**'
-    tags-ignore:
-      - '**'
-
-concurrency:
-  # An update of a developer branch cancels the previously
-  # scheduled workflow run for this branch. However, the default
-  # branch, and long-term branch (tarantool-1.10, tarantool-2.8,
-  # etc.) workflow runs are never canceled.
-  #
-  # We use a trick here: define the concurrency group as 'workflow
-  # run ID' + # 'workflow run attempt' because it is a unique
-  # combination for any run. So it effectively discards grouping.
-  #
-  # XXX: we cannot use `github.sha` as a unique identifier because
-  # pushing a tag may cancel a run that works on a branch push
-  # event.
-  group: ${{ (
-    github.ref == 'refs/heads/tarantool' ||
-    startsWith(github.ref, 'refs/heads/tarantool-')) &&
-    format('{0}-{1}', github.run_id, github.run_attempt) ||
-    format('{0}-{1}', github.workflow, github.ref) }}
-  cancel-in-progress: true
-
-jobs:
-  test-luajit:
-    runs-on: macos-11-m1
-    strategy:
-      fail-fast: false
-      matrix:
-        BUILDTYPE: [Debug, Release]
-        include:
-          - BUILDTYPE: Debug
-            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
-          - BUILDTYPE: Release
-            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
-    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:ON
-    steps:
-      - uses: actions/checkout@v2.3.4
-        with:
-          fetch-depth: 0
-          submodules: recursive
-      - name: environment
-        uses: ./.github/actions/environment
-      - name: setup
-        run: |
-          # Install brew using the command from Homebrew repository
-          # instructions: https://github.com/Homebrew/install.
-          # XXX: 'echo' command below is required since brew installation
-          # script obliges the one to enter a newline for confirming the
-          # installation via Ruby script.
-          brew update ||
-            echo | /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
-          # Try to install the packages either upgrade it to avoid of fails
-          # if the package already exists with the previous version.
-          brew install --force cmake gcc make perl ||
-            brew upgrade cmake gcc make perl
-      - name: configure
-        run: >
-          cmake -S . -B ${{ env.BUILDDIR }}
-          ${{ matrix.CMAKEFLAGS }}
-      - name: build
-        run: cmake --build . --parallel
-        working-directory: ${{ env.BUILDDIR }}
-      - name: test
-        run: cmake --build . --parallel --target test
-        working-directory: ${{ env.BUILDDIR }}
-
-  test-tarantool-debug-w-GC64:
-    name: Tarantool Debug GC64:ON
-    needs: test-luajit
-    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
-    with:
-      GC64: ON
-      buildtype: Debug
-      host: macos-11-m1
-      revision: ${{ github.sha }}
-  test-tarantool-release-w-GC64:
-    name: Tarantool Release GC64:ON
-    needs: test-luajit
-    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
-    with:
-      GC64: ON
-      buildtype: RelWithDebInfo
-      host: macos-11-m1
-      revision: ${{ github.sha }}
diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos.yml
similarity index 73%
rename from .github/workflows/macos-x86_64.yml
rename to .github/workflows/macos.yml
index 3d2cf581..2fa97f1e 100644
--- a/.github/workflows/macos-x86_64.yml
+++ b/.github/workflows/macos.yml
@@ -1,4 +1,4 @@
-name: "macOS/x86_64 test workflow"
+name: "macOS test workflow"
 
 on:
   push:
@@ -30,18 +30,30 @@ concurrency:
 
 jobs:
   test-luajit:
-    runs-on: macos-11
     strategy:
       fail-fast: false
       matrix:
+        ARCH: [M1, x86_64]
         BUILDTYPE: [Debug, Release]
         GC64: [ON, OFF]
         include:
+          - ARCH: M1
+            RUNNER: macos-11-m1
+          - ARCH: x86_64
+            RUNNER: macos-11
           - BUILDTYPE: Debug
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
           - BUILDTYPE: Release
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
-    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
+        exclude:
+          - ARCH: M1
+            GC64: OFF
+    runs-on: ${{ matrix.RUNNER }}
+    name: >
+      LuaJIT
+      ${{ matrix.ARCH }}
+      ${{ matrix.BUILDTYPE }}
+      GC64:${{ matrix.GC64 }}
     steps:
       - uses: actions/checkout@v2.3.4
         with:
@@ -74,8 +86,8 @@ jobs:
         run: cmake --build . --parallel --target test
         working-directory: ${{ env.BUILDDIR }}
 
-  test-tarantool-debug-wo-GC64:
-    name: Tarantool Debug GC64:OFF
+  test-tarantool-x86_64-debug-wo-GC64:
+    name: Tarantool x86_64 Debug GC64:OFF
     needs: test-luajit
     uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
     with:
@@ -83,8 +95,8 @@ jobs:
       buildtype: Debug
       host: macos-11
       revision: ${{ github.sha }}
-  test-tarantool-debug-w-GC64:
-    name: Tarantool Debug GC64:ON
+  test-tarantool-x86_64-debug-w-GC64:
+    name: Tarantool x86_64 Debug GC64:ON
     needs: test-luajit
     uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
     with:
@@ -92,8 +104,8 @@ jobs:
       buildtype: Debug
       host: macos-11
       revision: ${{ github.sha }}
-  test-tarantool-release-wo-GC64:
-    name: Tarantool Release GC64:OFF
+  test-tarantool-x86_64-release-wo-GC64:
+    name: Tarantool x86_64 Release GC64:OFF
     needs: test-luajit
     uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
     with:
@@ -101,8 +113,8 @@ jobs:
       buildtype: RelWithDebInfo
       host: macos-11
       revision: ${{ github.sha }}
-  test-tarantool-release-w-GC64:
-    name: Tarantool Release GC64:ON
+  test-tarantool-x86_64-release-w-GC64:
+    name: Tarantool x86_64 Release GC64:ON
     needs: test-luajit
     uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
     with:
@@ -110,3 +122,21 @@ jobs:
       buildtype: RelWithDebInfo
       host: macos-11
       revision: ${{ github.sha }}
+  test-tarantool-m1-debug-w-GC64:
+    name: Tarantool M1 Debug GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: Debug
+      host: macos-11-m1
+      revision: ${{ github.sha }}
+  test-tarantool-m1-release-w-GC64:
+    name: Tarantool M1 Release GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: RelWithDebInfo
+      host: macos-11-m1
+      revision: ${{ github.sha }}
-- 
2.34.0


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Tarantool-patches] [PATCH luajit 8/8] ci: merge Linux and macOS workflows
  2022-08-11 11:17 [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements Igor Munkin via Tarantool-patches
                   ` (6 preceding siblings ...)
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 7/8] ci: merge x86_64 and ARM64 workflows Igor Munkin via Tarantool-patches
@ 2022-08-11 11:17 ` Igor Munkin via Tarantool-patches
  2022-08-15 12:27   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18 10:32   ` Sergey Kaplun via Tarantool-patches
  2022-11-11  8:56 ` [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements Igor Munkin via Tarantool-patches
  8 siblings, 2 replies; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-11 11:17 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

As a result of two previous commits, there is no difference left between
Linux and macOS workflows except the environment setup (environment
variables, dependencies installation). Hence there is no reason to have a
separate workflow file for each OS type.

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 .github/actions/environment/action.yml        |  18 ---
 .github/actions/setup-linux/README.md         |  12 ++
 .github/actions/setup-linux/action.yml        |  19 +++
 .github/actions/setup-macos/README.md         |  12 ++
 .github/actions/setup-macos/action.yml        |  29 ++++
 .../actions/{environment => setup}/README.md  |   5 +-
 .github/actions/setup/action.yml              |  10 ++
 .github/workflows/lint.yml                    |   8 +-
 .github/workflows/linux-x86_64-ninja.yml      |   6 +-
 .github/workflows/macos.yml                   | 142 ------------------
 .github/workflows/{linux.yml => testing.yml}  | 113 +++++++++++---
 11 files changed, 186 insertions(+), 188 deletions(-)
 delete mode 100644 .github/actions/environment/action.yml
 create mode 100644 .github/actions/setup-linux/README.md
 create mode 100644 .github/actions/setup-linux/action.yml
 create mode 100644 .github/actions/setup-macos/README.md
 create mode 100644 .github/actions/setup-macos/action.yml
 rename .github/actions/{environment => setup}/README.md (72%)
 create mode 100644 .github/actions/setup/action.yml
 delete mode 100644 .github/workflows/macos.yml
 rename .github/workflows/{linux.yml => testing.yml} (52%)

diff --git a/.github/actions/environment/action.yml b/.github/actions/environment/action.yml
deleted file mode 100644
index 7fb2625f..00000000
--- a/.github/actions/environment/action.yml
+++ /dev/null
@@ -1,18 +0,0 @@
-name: 'Setup CI environment'
-description: 'Common part to tweak CI runner environment'
-runs:
-  using: 'composite'
-  steps:
-    - run: |
-        # Set CMAKE_BUILD_PARALLEL_LEVEL environment variable to
-        # limit the number of parallel jobs for build/test step.
-        # Try the exotic Darwin way at first and fallback to nproc
-        # that is the default for the normal systems.
-        NPROC=$(sysctl -n hw.logicalcpu 2>/dev/null || nproc)
-        echo CMAKE_BUILD_PARALLEL_LEVEL=$(($NPROC + 1)) | tee -a $GITHUB_ENV
-      shell: bash
-    - run: |
-        # Set BUILDDIR environment variable to specify LuaJIT
-        # build directory.
-        echo "BUILDDIR=${{ runner.temp }}/build-${{ github.run_id }}" | tee -a $GITHUB_ENV
-      shell: bash
diff --git a/.github/actions/setup-linux/README.md b/.github/actions/setup-linux/README.md
new file mode 100644
index 00000000..48ae9e01
--- /dev/null
+++ b/.github/actions/setup-linux/README.md
@@ -0,0 +1,12 @@
+# Setup environment on Linux
+
+Action setups the environment on Linux runners (install requirements, setup the
+workflow environment, etc).
+
+## How to use Github Action from Github workflow
+
+Add the following code to the running steps before LuaJIT configuration:
+```
+- uses: ./.github/actions/setup-linux
+  if: ${{ matrix.OS == 'Linux' }}
+```
diff --git a/.github/actions/setup-linux/action.yml b/.github/actions/setup-linux/action.yml
new file mode 100644
index 00000000..a13f143c
--- /dev/null
+++ b/.github/actions/setup-linux/action.yml
@@ -0,0 +1,19 @@
+name: Setup CI environment on Linux
+description: Common part to tweak Linux CI runner environment
+runs:
+  using: composite
+  steps:
+    - name: Setup CI environment
+      uses: ./.github/actions/setup
+    - name: Set CMAKE_BUILD_PARALLEL_LEVEL
+      run: |
+        # Set CMAKE_BUILD_PARALLEL_LEVEL environment variable to
+        # limit the number of parallel jobs for build/test step.
+        NPROC=$(nproc)
+        echo CMAKE_BUILD_PARALLEL_LEVEL=$(($NPROC + 1)) | tee -a $GITHUB_ENV
+      shell: bash
+    - name: Install build and test dependencies
+      run: |
+        apt -y update
+        apt -y install cmake gcc make perl
+      shell: bash
diff --git a/.github/actions/setup-macos/README.md b/.github/actions/setup-macos/README.md
new file mode 100644
index 00000000..ae70cb56
--- /dev/null
+++ b/.github/actions/setup-macos/README.md
@@ -0,0 +1,12 @@
+# Setup environment on macOS
+
+Action setups the environment on macOS runners (install requirements, setup the
+workflow environment, etc).
+
+## How to use Github Action from Github workflow
+
+Add the following code to the running steps before LuaJIT configuration:
+```
+- uses: ./.github/actions/setup-macos
+  if: ${{ matrix.OS == 'macOS' }}
+```
diff --git a/.github/actions/setup-macos/action.yml b/.github/actions/setup-macos/action.yml
new file mode 100644
index 00000000..7a98065c
--- /dev/null
+++ b/.github/actions/setup-macos/action.yml
@@ -0,0 +1,29 @@
+name: Setup CI environment on macOS
+description: Common part to tweak macOS CI runner environment
+runs:
+  using: composite
+  steps:
+    - name: Setup CI environment
+      uses: ./.github/actions/setup
+    - name: Set CMAKE_BUILD_PARALLEL_LEVEL
+      run: |
+        # Set CMAKE_BUILD_PARALLEL_LEVEL environment variable to
+        # limit the number of parallel jobs for build/test step.
+        NPROC=$(sysctl -n hw.logicalcpu 2>/dev/null)
+        echo CMAKE_BUILD_PARALLEL_LEVEL=$(($NPROC + 1)) | tee -a $GITHUB_ENV
+      shell: bash
+    - name: Install build and test dependencies
+      run: |
+        # Install brew using the command from Homebrew repository
+        # instructions: https://github.com/Homebrew/install.
+        # XXX: 'echo' command below is required since brew
+        # installation script obliges the one to enter a newline
+        # for confirming the installation via Ruby script.
+        brew update ||
+          echo | /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
+        # Try to install the packages either upgrade it to avoid
+        # of fails if the package already exists with the previous
+        # version.
+        brew install --force cmake gcc make perl ||
+          brew upgrade cmake gcc make perl
+      shell: bash
diff --git a/.github/actions/environment/README.md b/.github/actions/setup/README.md
similarity index 72%
rename from .github/actions/environment/README.md
rename to .github/actions/setup/README.md
index 1ed0d0ca..87b5bf8d 100644
--- a/.github/actions/environment/README.md
+++ b/.github/actions/setup/README.md
@@ -6,7 +6,8 @@ It is used as a common environment setup for the different testing workflows.
 
 ## How to use Github Action from Github workflow
 
-Add the following code to the running steps after checkout is finished:
+Add the following code to the running steps or OS-specific GitHub Actions after
+checkout step is finished:
 ```
-- uses: ./.github/actions/environment
+- uses: ./.github/actions/setup
 ```
diff --git a/.github/actions/setup/action.yml b/.github/actions/setup/action.yml
new file mode 100644
index 00000000..f153b0d9
--- /dev/null
+++ b/.github/actions/setup/action.yml
@@ -0,0 +1,10 @@
+name: Setup CI environment
+description: Common part to tweak CI runner environment
+runs:
+  using: composite
+  steps:
+    - run: |
+        # Set BUILDDIR environment variable to specify LuaJIT
+        # build directory.
+        echo BUILDDIR=${{ runner.temp }}/build-${{ github.run_id }} | tee -a $GITHUB_ENV
+      shell: bash
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 64e8f992..04b810f2 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -1,4 +1,4 @@
-name: "LuaJIT static analysis"
+name: Static analysis
 
 on:
   push:
@@ -38,12 +38,16 @@ jobs:
           fetch-depth: 0
           submodules: recursive
       - name: environment
-        uses: ./.github/actions/environment
+        uses: ./.github/actions/setup
       - name: setup
         run: |
+          # TODO: Move this step to a separate action.
           sudo apt -y update
           sudo apt -y install cmake make lua5.1 luarocks
           sudo luarocks install luacheck
+          # Set CMAKE_BUILD_PARALLEL_LEVEL environment variable to
+          # limit the number of parallel jobs for build/test step.
+          echo CMAKE_BUILD_PARALLEL_LEVEL=$(($(nproc) + 1)) | tee -a $GITHUB_ENV
       - name: configure
         run: cmake -S . -B ${{ env.BUILDDIR }}
       - name: test
diff --git a/.github/workflows/linux-x86_64-ninja.yml b/.github/workflows/linux-x86_64-ninja.yml
index 72d56d54..a0422d09 100644
--- a/.github/workflows/linux-x86_64-ninja.yml
+++ b/.github/workflows/linux-x86_64-ninja.yml
@@ -38,11 +38,15 @@ jobs:
           fetch-depth: 0
           submodules: recursive
       - name: environment
-        uses: ./.github/actions/environment
+        uses: ./.github/actions/setup
       - name: setup
         run: |
+          # TODO: Move this step to a separate action.
           sudo apt -y update
           sudo apt -y install cmake gcc ninja-build perl
+          # Set CMAKE_BUILD_PARALLEL_LEVEL environment variable to
+          # limit the number of parallel jobs for build/test step.
+          echo CMAKE_BUILD_PARALLEL_LEVEL=$(($(nproc) + 1)) | tee -a $GITHUB_ENV
       - name: configure
         run: >
           cmake -S . -B ${{ env.BUILDDIR }}
diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml
deleted file mode 100644
index 2fa97f1e..00000000
--- a/.github/workflows/macos.yml
+++ /dev/null
@@ -1,142 +0,0 @@
-name: "macOS test workflow"
-
-on:
-  push:
-    branches-ignore:
-      - '**-notest'
-      - 'upstream-**'
-    tags-ignore:
-      - '**'
-
-concurrency:
-  # An update of a developer branch cancels the previously
-  # scheduled workflow run for this branch. However, the default
-  # branch, and long-term branch (tarantool-1.10, tarantool-2.8,
-  # etc.) workflow runs are never canceled.
-  #
-  # We use a trick here: define the concurrency group as 'workflow
-  # run ID' + # 'workflow run attempt' because it is a unique
-  # combination for any run. So it effectively discards grouping.
-  #
-  # XXX: we cannot use `github.sha` as a unique identifier because
-  # pushing a tag may cancel a run that works on a branch push
-  # event.
-  group: ${{ (
-    github.ref == 'refs/heads/tarantool' ||
-    startsWith(github.ref, 'refs/heads/tarantool-')) &&
-    format('{0}-{1}', github.run_id, github.run_attempt) ||
-    format('{0}-{1}', github.workflow, github.ref) }}
-  cancel-in-progress: true
-
-jobs:
-  test-luajit:
-    strategy:
-      fail-fast: false
-      matrix:
-        ARCH: [M1, x86_64]
-        BUILDTYPE: [Debug, Release]
-        GC64: [ON, OFF]
-        include:
-          - ARCH: M1
-            RUNNER: macos-11-m1
-          - ARCH: x86_64
-            RUNNER: macos-11
-          - BUILDTYPE: Debug
-            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
-          - BUILDTYPE: Release
-            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
-        exclude:
-          - ARCH: M1
-            GC64: OFF
-    runs-on: ${{ matrix.RUNNER }}
-    name: >
-      LuaJIT
-      ${{ matrix.ARCH }}
-      ${{ matrix.BUILDTYPE }}
-      GC64:${{ matrix.GC64 }}
-    steps:
-      - uses: actions/checkout@v2.3.4
-        with:
-          fetch-depth: 0
-          submodules: recursive
-      - name: environment
-        uses: ./.github/actions/environment
-      - name: setup
-        run: |
-          # Install brew using the command from Homebrew repository
-          # instructions: https://github.com/Homebrew/install.
-          # XXX: 'echo' command below is required since brew installation
-          # script obliges the one to enter a newline for confirming the
-          # installation via Ruby script.
-          brew update ||
-            echo | /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
-          # Try to install the packages either upgrade it to avoid of fails
-          # if the package already exists with the previous version.
-          brew install --force cmake gcc make perl ||
-            brew upgrade cmake gcc make perl
-      - name: configure
-        run: >
-          cmake -S . -B ${{ env.BUILDDIR }}
-          ${{ matrix.CMAKEFLAGS }}
-          -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
-      - name: build
-        run: cmake --build . --parallel
-        working-directory: ${{ env.BUILDDIR }}
-      - name: test
-        run: cmake --build . --parallel --target test
-        working-directory: ${{ env.BUILDDIR }}
-
-  test-tarantool-x86_64-debug-wo-GC64:
-    name: Tarantool x86_64 Debug GC64:OFF
-    needs: test-luajit
-    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
-    with:
-      GC64: OFF
-      buildtype: Debug
-      host: macos-11
-      revision: ${{ github.sha }}
-  test-tarantool-x86_64-debug-w-GC64:
-    name: Tarantool x86_64 Debug GC64:ON
-    needs: test-luajit
-    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
-    with:
-      GC64: ON
-      buildtype: Debug
-      host: macos-11
-      revision: ${{ github.sha }}
-  test-tarantool-x86_64-release-wo-GC64:
-    name: Tarantool x86_64 Release GC64:OFF
-    needs: test-luajit
-    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
-    with:
-      GC64: OFF
-      buildtype: RelWithDebInfo
-      host: macos-11
-      revision: ${{ github.sha }}
-  test-tarantool-x86_64-release-w-GC64:
-    name: Tarantool x86_64 Release GC64:ON
-    needs: test-luajit
-    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
-    with:
-      GC64: ON
-      buildtype: RelWithDebInfo
-      host: macos-11
-      revision: ${{ github.sha }}
-  test-tarantool-m1-debug-w-GC64:
-    name: Tarantool M1 Debug GC64:ON
-    needs: test-luajit
-    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
-    with:
-      GC64: ON
-      buildtype: Debug
-      host: macos-11-m1
-      revision: ${{ github.sha }}
-  test-tarantool-m1-release-w-GC64:
-    name: Tarantool M1 Release GC64:ON
-    needs: test-luajit
-    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
-    with:
-      GC64: ON
-      buildtype: RelWithDebInfo
-      host: macos-11-m1
-      revision: ${{ github.sha }}
diff --git a/.github/workflows/linux.yml b/.github/workflows/testing.yml
similarity index 52%
rename from .github/workflows/linux.yml
rename to .github/workflows/testing.yml
index 125c8708..c4847335 100644
--- a/.github/workflows/linux.yml
+++ b/.github/workflows/testing.yml
@@ -1,4 +1,4 @@
-name: "Linux test workflow"
+name: Testing
 
 on:
   push:
@@ -33,25 +33,38 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        ARCH: [aarch64, x86_64]
+        ARCH: [ARM64, x86_64]
         BUILDTYPE: [Debug, Release]
         GC64: [ON, OFF]
+        OS: [Linux, macOS]
         include:
-          - ARCH: aarch64
+          - ARCH: ARM64
+            OS: Linux
+            NAME: Linux/aarch64
             RUNNER: graviton
           - ARCH: x86_64
+            OS: Linux
+            NAME: Linux/x86_64
             RUNNER: ubuntu-20.04-self-hosted
+          - ARCH: ARM64
+            OS: macOS
+            NAME: macOS/M1
+            RUNNER: macos-11-m1
+          - ARCH: x86_64
+            OS: macOS
+            NAME: macOS/x86_64
+            RUNNER: macos-11
           - BUILDTYPE: Debug
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
           - BUILDTYPE: Release
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
         exclude:
-          - ARCH: aarch64
+          - ARCH: ARM64
             GC64: OFF
     runs-on: ${{ matrix.RUNNER }}
     name: >
       LuaJIT
-      ${{ matrix.ARCH }}
+      (${{ matrix.NAME }})
       ${{ matrix.BUILDTYPE }}
       GC64:${{ matrix.GC64 }}
     steps:
@@ -59,12 +72,12 @@ jobs:
         with:
           fetch-depth: 0
           submodules: recursive
-      - name: environment
-        uses: ./.github/actions/environment
-      - name: setup
-        run: |
-          sudo apt -y update
-          sudo apt -y install cmake gcc make perl
+      - name: setup Linux
+        uses: ./.github/actions/setup-linux
+        if: ${{ matrix.OS == 'Linux' }}
+      - name: setup macOS
+        uses: ./.github/actions/setup-macos
+        if: ${{ matrix.OS == 'macOS' }}
       - name: configure
         run: >
           cmake -S . -B ${{ env.BUILDDIR }}
@@ -77,8 +90,8 @@ jobs:
         run: cmake --build . --parallel --target test
         working-directory: ${{ env.BUILDDIR }}
 
-  test-tarantool-x86_64-debug-wo-GC64:
-    name: Tarantool x86_64 Debug GC64:OFF
+  test-tarantool-linux-x86_64-debug-wo-GC64:
+    name: Tarantool (Linux/x86_64) Debug GC64:OFF
     needs: test-luajit
     uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
     with:
@@ -86,8 +99,8 @@ jobs:
       buildtype: Debug
       host: ubuntu-20.04-self-hosted
       revision: ${{ github.sha }}
-  test-tarantool-x86_64-debug-w-GC64:
-    name: Tarantool x86_64 Debug GC64:ON
+  test-tarantool-linux-x86_64-debug-w-GC64:
+    name: Tarantool (Linux/x86_64) Debug GC64:ON
     needs: test-luajit
     uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
     with:
@@ -95,8 +108,8 @@ jobs:
       buildtype: Debug
       host: ubuntu-20.04-self-hosted
       revision: ${{ github.sha }}
-  test-tarantool-x86_64-release-wo-GC64:
-    name: Tarantool x86_64 Release GC64:OFF
+  test-tarantool-linux-x86_64-release-wo-GC64:
+    name: Tarantool (Linux/x86_64) Release GC64:OFF
     needs: test-luajit
     uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
     with:
@@ -104,8 +117,8 @@ jobs:
       buildtype: RelWithDebInfo
       host: ubuntu-20.04-self-hosted
       revision: ${{ github.sha }}
-  test-tarantool-x86_64-release-w-GC64:
-    name: Tarantool x86_64 Release GC64:ON
+  test-tarantool-linux-x86_64-release-w-GC64:
+    name: Tarantool (Linux/x86_64) Release GC64:ON
     needs: test-luajit
     uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
     with:
@@ -113,8 +126,8 @@ jobs:
       buildtype: RelWithDebInfo
       host: ubuntu-20.04-self-hosted
       revision: ${{ github.sha }}
-  test-tarantool-aarch64-debug-w-GC64:
-    name: Tarantool aarch64 Debug GC64:ON
+  test-tarantool-linux-aarch64-debug-w-GC64:
+    name: Tarantool (Linux/aarch64) Debug GC64:ON
     needs: test-luajit
     uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
     with:
@@ -122,8 +135,8 @@ jobs:
       buildtype: Debug
       host: graviton
       revision: ${{ github.sha }}
-  test-tarantool-aarch64-release-w-GC64:
-    name: Tarantool aarch64 Release GC64:ON
+  test-tarantool-linux-aarch64-release-w-GC64:
+    name: Tarantool (Linux/aarch64) Release GC64:ON
     needs: test-luajit
     uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
     with:
@@ -131,3 +144,57 @@ jobs:
       buildtype: RelWithDebInfo
       host: graviton
       revision: ${{ github.sha }}
+  test-tarantool-macos-x86_64-debug-wo-GC64:
+    name: Tarantool (macOS/x86_64) Debug GC64:OFF
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: OFF
+      buildtype: Debug
+      host: macos-11
+      revision: ${{ github.sha }}
+  test-tarantool-macos-x86_64-debug-w-GC64:
+    name: Tarantool (macOS/x86_64) Debug GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: Debug
+      host: macos-11
+      revision: ${{ github.sha }}
+  test-tarantool-macos-x86_64-release-wo-GC64:
+    name: Tarantool (macOS/x86_64) Release GC64:OFF
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: OFF
+      buildtype: RelWithDebInfo
+      host: macos-11
+      revision: ${{ github.sha }}
+  test-tarantool-macos-x86_64-release-w-GC64:
+    name: Tarantool (macOS/x86_64) Release GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: RelWithDebInfo
+      host: macos-11
+      revision: ${{ github.sha }}
+  test-tarantool-macos-m1-debug-w-GC64:
+    name: Tarantool (macOS/M1) Debug GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: Debug
+      host: macos-11-m1
+      revision: ${{ github.sha }}
+  test-tarantool-macos-m1-release-w-GC64:
+    name: Tarantool (macOS/M1) Release GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: RelWithDebInfo
+      host: macos-11-m1
+      revision: ${{ github.sha }}
-- 
2.34.0


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 1/8] test: introduce LUAJIT_TEST_VARDIR variable
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 1/8] test: introduce LUAJIT_TEST_VARDIR variable Igor Munkin via Tarantool-patches
@ 2022-08-15 12:08   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18  8:27     ` Sergey Kaplun via Tarantool-patches
  2022-08-31 14:53     ` Igor Munkin via Tarantool-patches
  0 siblings, 2 replies; 39+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-08-15 12:08 UTC (permalink / raw)
  To: Igor Munkin, Sergey Kaplun; +Cc: tarantool-patches

Igor,

  thanks for the patch. See my comments inline.

On 11.08.2022 14:17, Igor Munkin wrote:
> Before the patch both memprof and sysprof artefacts are generated within
> the binary artefacts tree (i.e. in the same directory LuaJIT binary is
> generated). However, more convenient way is producing these temporary
> profiles in a separate directory (e.g. located on the partition with
> more strict space limits). As a result of the patch all memprof and
> sysprof test chunks consider LUAJIT_TEST_VARDIR environment variable to
> set the directory where test profiles are generated. For the case when
> LUAJIT_TEST_VARDIR is not set, everything works as before.

Commit message is inconsistent a bit with a patch itself.

As far as I understand many hunks are not related to introducing 
LUAJIT_TEST_VARDIR.

Probably it is better to change commit one-line message from "test: 
introduce LUAJIT_TEST_VARDIR variable"

to something like "test: refactoring and introduce LUAJIT_TEST_VARDIR 
variable". It allows to keep patch

as is and change expectations for those who will look at your patch.


>
> Part of tarantool/tarantool#7472
>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>   .../gh-5813-resolving-of-c-symbols.test.lua   |  6 +++--
>   .../misclib-memprof-lapi.test.lua             | 22 ++++++++++---------
>   .../misclib-sysprof-lapi.test.lua             |  8 ++++---
>   test/tarantool-tests/utils.lua                | 12 ++++++++++
>   4 files changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> index d589dddf..e0b6d86d 100644
> --- a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> +++ b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> @@ -1,5 +1,7 @@
>   -- Memprof is implemented for x86 and x64 architectures only.
> -require("utils").skipcond(
> +local utils = require("utils")
> +
> +utils.skipcond(
>     jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux",
>     jit.arch.." architecture or "..jit.os..
>     " OS is NIY for memprof c symbols resolving"
> @@ -18,7 +20,7 @@ local testboth = require "resboth"
>   local testhash = require "reshash"
>   local testgnuhash = require "resgnuhash"
>   
> -local TMP_BINFILE = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%.%1.memprofdata.tmp.bin")
> +local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin")
>   
>   local function tree_contains(node, name)
>     if node == nil then
> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> index a11f0be1..bae0c27c 100644
> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> @@ -1,5 +1,7 @@
>   -- Memprof is implemented for x86 and x64 architectures only.
> -require("utils").skipcond(
> +local utils = require("utils")
> +
> +utils.skipcond(
>     jit.arch ~= "x86" and jit.arch ~= "x64",
>     jit.arch.." architecture is NIY for memprof"
>   )
> @@ -26,8 +28,8 @@ local memprof = require "memprof.parse"
>   local process = require "memprof.process"
>   local symtab = require "utils.symtab"
>   
> -local TMP_BINFILE = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%.%1.memprofdata.tmp.bin")
> -local BAD_PATH = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%1/memprofdata.tmp.bin")
> +local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin")
> +local BAD_PATH = utils.profilename("memprofdata/tmp.bin")
>   local SRC_PATH = "@"..arg[0]
>   
>   local function default_payload()
> @@ -189,9 +191,9 @@ test:test("output", function(subtest)
>     -- one is the number of allocations. 1 event - alocation 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 = 35, linedefined = 33 }, 2))
> +  subtest:ok(check_alloc_report(alloc, { line = 37, linedefined = 35 }, 2))
>     -- 20 strings allocations.
> -  subtest:ok(check_alloc_report(alloc, { line = 40, linedefined = 33 }, 20))
> +  subtest:ok(check_alloc_report(alloc, { line = 42, linedefined = 35 }, 20))
What are these magic numbers mean and why we should change them for 
introducing LUAJIT_TEST_VARDIR?
>   
>     -- Collect all previous allocated objects.
>     subtest:ok(free.INTERNAL.num == 22)
> @@ -199,8 +201,8 @@ test:test("output", function(subtest)
>     -- Tests for leak-only option.
>     -- See also https://github.com/tarantool/tarantool/issues/5812.
>     local heap_delta = process.form_heap_delta(events, symbols)
> -  local tab_alloc_stats = heap_delta[form_source_line(35)]
> -  local str_alloc_stats = heap_delta[form_source_line(40)]
> +  local tab_alloc_stats = heap_delta[form_source_line(37)]
> +  local str_alloc_stats = heap_delta[form_source_line(42)]
>     subtest:ok(tab_alloc_stats.nalloc == tab_alloc_stats.nfree)
>     subtest:ok(tab_alloc_stats.dbytes == 0)
>     subtest:ok(str_alloc_stats.nalloc == str_alloc_stats.nfree)
> @@ -291,10 +293,10 @@ test:test("jit-output", function(subtest)
>     -- 10 allocations in interpreter mode, 1 allocation for a trace
>     -- recording and assembling and next 9 allocations will happen
>     -- while running the trace.
> -  subtest:ok(check_alloc_report(alloc, { line = 40, linedefined = 33 }, 11))
> -  subtest:ok(check_alloc_report(alloc, { traceno = 1, line = 38 }, 9))
> +  subtest:ok(check_alloc_report(alloc, { line = 42, linedefined = 35 }, 11))
> +  subtest:ok(check_alloc_report(alloc, { traceno = 1, line = 40 }, 9))
>     -- See same checks with jit.off().
> -  subtest:ok(check_alloc_report(alloc, { line = 35, linedefined = 33 }, 2))
> +  subtest:ok(check_alloc_report(alloc, { line = 37, linedefined = 35 }, 2))
>   
>     -- Restore default JIT settings.
>     jit.opt.start(unpack(jit_opt_default))
> diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
> index 9e0a8a77..dbff41b0 100644
> --- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
> @@ -1,6 +1,8 @@
>   -- Sysprof is implemented for x86 and x64 architectures only.
>   local ffi = require("ffi")
> -require("utils").skipcond(
> +local utils = require("utils")
> +
> +utils.skipcond(
>     jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux"
>       or ffi.abi("gc64"),
>     jit.arch.." architecture or "..jit.os..
> @@ -19,8 +21,8 @@ local bufread = require("utils.bufread")
>   local symtab = require("utils.symtab")
>   local sysprof = require("sysprof.parse")
>   
> -local TMP_BINFILE = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%.%1.sysprofdata.tmp.bin")
> -local BAD_PATH = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%1/sysprofdata.tmp.bin")
> +local TMP_BINFILE = utils.profilename("sysprofdata.tmp.bin")
> +local BAD_PATH = utils.profilename("sysprofdata/tmp.bin")
>   
>   local function payload()
>     local function fib(n)
> diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> index a1906498..87f7ff15 100644
> --- a/test/tarantool-tests/utils.lua
> +++ b/test/tarantool-tests/utils.lua
> @@ -123,4 +123,16 @@ function M.hasbc(f, bytecode)
>     return hasbc
>   end
>   
> +function M.profilename(name)
> +  local vardir = os.getenv('LUAJIT_TEST_VARDIR')
> +  -- Replace pattern will change directory name of the generated
> +  -- profile to LUAJIT_TEST_VARDIR if it is set in the process
> +  -- environment. Otherwise, the original dirname is left intact.
> +  -- As a basename for this profile the test name is concatenated
> +  -- with the name given as an argument.
> +  local replacepattern = ('%s/%s-%s'):format(vardir or '%1', '%2', name)
> +  -- XXX: return only the resulting string.
> +  return (arg[0]:gsub('^(.+)/([^/]+)%.test%.lua$', replacepattern))
> +end
> +
>   return M

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 2/8] test: introduce MakeLuaPath.cmake helper
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 2/8] test: introduce MakeLuaPath.cmake helper Igor Munkin via Tarantool-patches
@ 2022-08-15 12:08   ` Sergey Bronnikov via Tarantool-patches
  2022-08-31 15:07     ` Igor Munkin via Tarantool-patches
  2022-08-18  9:37   ` Sergey Kaplun via Tarantool-patches
  1 sibling, 1 reply; 39+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-08-15 12:08 UTC (permalink / raw)
  To: Igor Munkin, Sergey Kaplun; +Cc: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 8565 bytes --]

Igor, thanks for the patch! See my comments inline.

On 11.08.2022 14:17, Igor Munkin wrote:
> While extending test suites it is often required to append additional
> path where Lua or Lua-C auxiliary modules are located to LUA_PATH or
> LUA_CPATH environment variables. Due to insane semicolon interpolation
> in CMake strings (that converts such string to a list as a result), we
> need to escape semicolon in LUA_PATH/LUA_CPATH strings while building
> the resulting value.
>
> After the years of struggling MakeLuaPath.cmake module is introduced to
> make LUA_PATH and LUA_CPATH definition convenient with <make_lua_path>
> helper. This function takes all paths given as a variable list argument,
> joins them in a reverse order by a semicolon and yields the resulting
> string to a specified CMake variable.
>
> Signed-off-by: Igor Munkin<imun@tarantool.org>
> ---
>   cmake/MakeLuaPath.cmake                   | 46 +++++++++++++++++++++++
>   test/CMakeLists.txt                       |  2 +
>   test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt |  8 +++-
>   test/lua-Harness-tests/CMakeLists.txt     | 16 +++++---
>   test/tarantool-tests/CMakeLists.txt       | 28 ++++++++------
>   5 files changed, 81 insertions(+), 19 deletions(-)
>   create mode 100644 cmake/MakeLuaPath.cmake
>
> diff --git a/cmake/MakeLuaPath.cmake b/cmake/MakeLuaPath.cmake
> new file mode 100644
> index 00000000..9a5a3bb8
> --- /dev/null
> +++ b/cmake/MakeLuaPath.cmake
> @@ -0,0 +1,46 @@
> +# make_lua_path provides a convenient way to define LUA_PATH and
> +# LUA_CPATH variables.
> +#
> +# Example usage:
> +#
> +#   make_lua_path(LUA_PATH
> +#     PATH
> +#       ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
> +#       ${CMAKE_BINARY_DIR}/?.lua
> +#       ./?.lua
> +#   )
> +#
> +# This will give you the string:
> +#    "./?.lua;${CMAKE_BINARY_DIR}/?.lua;${CMAKE_CURRENT_SOURCE_DIR}/?.lua;;"
> +# XXX: Mind the reverse order of the entries in the result string.
> +
> +function(make_lua_path path)
> +  set(prefix ARG)
> +  set(noValues)
> +  set(singleValues)
> +  set(multiValues PATHS)
> +
> +  # FIXME: if we update to CMake >= 3.5, can remove this line.

I would replace comment with condition that will check CMake version

and will fail when CMake >= 3.5:

|if (CMAKE_VERSION VERSION_GREATER_EQUAL 35) |

    message(FATAL_ERROR "Please remove snippet for CMake < 3.5")

endif()


> +  include(CMakeParseArguments)
> +  cmake_parse_arguments(${prefix}
> +                        "${noValues}"
> +                        "${singleValues}"
> +                        "${multiValues}"
> +                        ${ARGN})
> +
> +  # XXX: This is the sentinel semicolon having special meaning
> +  # for LUA_PATH and LUA_CPATH variables. For more info, see the
> +  # link below:
> +  #https://www.lua.org/manual/5.1/manual.html#pdf-LUA_PATH
> +  set(result "\;")
> +
> +  foreach(inc ${ARG_PATHS})
> +    # XXX: If one joins two strings with semicolon, the value
> +    # automatically becomes a list. I found a single working
> +    # solution to make result variable be a string via "escaping"
> +    # the semicolon right in string interpolation.
> +    set(result "${inc}\;${result}")
> +  endforeach()
> +
> +  set(${path} "${result}" PARENT_SCOPE)
> +endfunction()
> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
> index ba25af54..a8262b12 100644
> --- a/test/CMakeLists.txt
> +++ b/test/CMakeLists.txt
> @@ -3,6 +3,8 @@
>   # See the rationale in the root CMakeLists.txt.
>   cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
>   
> +include(MakeLuaPath)
> +
>   find_program(LUACHECK luacheck)
>   if(LUACHECK)
>     # XXX: The tweak below relates to luacheck problem with paths.
> diff --git a/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt b/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
> index 8e825f55..b95e7852 100644
> --- a/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
> +++ b/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
> @@ -14,7 +14,11 @@ cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
>   #https://github.com/tarantool/tarantool/issues/5744
>   # Hence, there is no way other than set LUA_PATH environment
>   # variable as proposed in the first case.
> -set(LUA_PATH "?\;${CMAKE_CURRENT_SOURCE_DIR}/?.lua")
> +make_lua_path(LUA_PATH
> +  PATHS
> +    ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
> +    "?"
> +)
>   
>   # Establish PUC-Rio-Lua-5.1-tests-prepare target that contains
>   # rules for <libs/*> libraries compilation and creates <libs/P1>
> @@ -38,7 +42,7 @@ add_custom_command(TARGET PUC-Rio-Lua-5.1-tests
>     COMMENT "Running PUC-Rio Lua 5.1 tests"
>     COMMAND
>     env
> -    LUA_PATH="${LUA_PATH}\;\;"
> +    LUA_PATH="${LUA_PATH}"
>       ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/all.lua
>     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
>   )
> diff --git a/test/lua-Harness-tests/CMakeLists.txt b/test/lua-Harness-tests/CMakeLists.txt
> index 12476171..a57104a5 100644
> --- a/test/lua-Harness-tests/CMakeLists.txt
> +++ b/test/lua-Harness-tests/CMakeLists.txt
> @@ -10,10 +10,16 @@ if(NOT PROVE)
>     return()
>   endif()
>   
> -# Tests create temporary files (see 303-package.t and 411-luajit.t for
> -# example) to require. Also, they require some files from original test
> -# source directory.
> -set(LUA_PATH "./?.lua\;${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${LUAJIT_SOURCE_DIR}/?.lua\;${LUAJIT_BINARY_DIR}/?.lua")
> +# Tests create temporary files (see 303-package.t and 411-luajit.t
> +# for example) to be required. Also, they require some files from
> +# the original test source directory.
> +make_lua_path(LUA_PATH
> +  PATHS
> +    ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
> +    ${LUAJIT_BINARY_DIR}/?.lua
> +    ${LUAJIT_SOURCE_DIR}/?.lua
> +    ./?.lua
> +)
>   set(LUA_TEST_FLAGS --failures --shuffle)
>   
>   if(CMAKE_VERBOSE_MAKEFILE)
> @@ -25,7 +31,7 @@ add_custom_command(TARGET lua-Harness-tests
>     COMMENT "Running lua-Harness tests"
>     COMMAND
>     env
> -    LUA_PATH="${LUA_PATH}\;"
> +    LUA_PATH="${LUA_PATH}"
>       ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR}
>         --exec '${LUAJIT_TEST_COMMAND} -l profile_luajit21'
>         --jobs ${CMAKE_BUILD_PARALLEL_LEVEL}
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index ecda2e63..27866869 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -43,14 +43,11 @@ macro(BuildTestCLib lib sources)
>     # semicolon. If one finds the normal way to make it work, feel
>     # free to reach me.
>     set(TESTLIBS "${lib};${TESTLIBS}" PARENT_SCOPE)
> -  # Add the directory where the lib is built to the LUA_CPATH
> -  # environment variable, so LuaJIT can find and load it.
> -  # XXX: Here we see the other side of the coin. If one joins two
> -  # strings with semicolon, the value automatically becomes a
> -  # list. I found a single working solution to make LUA_CPATH be
> -  # a string via "escaping" the semicolon right in string
> -  # interpolation.
> -  set(LUA_CPATH "${CMAKE_CURRENT_BINARY_DIR}/?${CMAKE_SHARED_LIBRARY_SUFFIX}\;${LUA_CPATH}" PARENT_SCOPE)
> +  # Add the directory where the lib is built to the list with
> +  # entries for LUA_CPATH environment variable, so LuaJIT can find
> +  # and load it. See the comment about extending the list in the
> +  # parent scope few lines above.
> +  set(LUA_CPATHS "${CMAKE_CURRENT_BINARY_DIR}/?${CMAKE_SHARED_LIBRARY_SUFFIX};${LUA_CPATHS}" PARENT_SCOPE)
>     # Also add this directory to LD_LIBRARY_PATH environment
>     # variable, so FFI machinery can find and load it.
>     set(LD_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}:${LD_LIBRARY_PATH}" PARENT_SCOPE)
> @@ -76,14 +73,21 @@ add_subdirectory(misclib-sysprof-capi)
>   # in src/ directory and auxiliary tests-related modules are
>   # located in the current directory (but tests are run in the
>   # binary directory), so LUA_PATH need to be updated.
> -set(LUA_PATH
> -  "${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${PROJECT_SOURCE_DIR}/tools/?.lua\;${PROJECT_SOURCE_DIR}/src/?.lua"
> +make_lua_path(LUA_PATH
> +  PATHS
> +    ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
> +    ${PROJECT_SOURCE_DIR}/tools/?.lua
> +    ${PROJECT_SOURCE_DIR}/src/?.lua
>   )
> +# Update LUA_CPATH with the library paths collected within
> +# <BuildTestLib> macro.
> +make_lua_path(LUA_CPATH PATHS ${LUA_CPATHS})
> +
>   set(LUA_TEST_SUFFIX .test.lua)
>   set(LUA_TEST_FLAGS --failures --shuffle)
>   set(LUA_TEST_ENV
> -  "LUA_PATH=\"${LUA_PATH}\;\;\""
> -  "LUA_CPATH=\"${LUA_CPATH}\;\;\""
> +  "LUA_PATH=\"${LUA_PATH}\""
> +  "LUA_CPATH=\"${LUA_CPATH}\""
>   )
>   
>   if(CMAKE_VERBOSE_MAKEFILE)

[-- Attachment #2: Type: text/html, Size: 10564 bytes --]

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 3/8] test: fix tarantool suite for out of source build
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 3/8] test: fix tarantool suite for out of source build Igor Munkin via Tarantool-patches
@ 2022-08-15 12:10   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18  9:49   ` Sergey Kaplun via Tarantool-patches
  1 sibling, 0 replies; 39+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-08-15 12:10 UTC (permalink / raw)
  To: Igor Munkin, Sergey Kaplun; +Cc: tarantool-patches

LGTM, thanks!

On 11.08.2022 14:17, Igor Munkin wrote:
> jit/vmdef.lua is autogenerated file, so it's put to src/ directory
> located in scope of the binary artefacts tree. Before the patch LUA_PATH
> lacks this path, so tarantool-tests target fails due to jit/vmdef.lua
> misseek. As a result of this change src/ directory in scope of the
> binary tree is included to LUA_PATH as well as the one from the source
> tree has been.
>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>   test/tarantool-tests/CMakeLists.txt | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 27866869..97c23670 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -70,14 +70,17 @@ add_subdirectory(misclib-sysprof-capi)
>   
>   # The part of the memory profiler toolchain is located in tools
>   # directory, jit, profiler, and bytecode toolchains are located
> -# in src/ directory and auxiliary tests-related modules are
> -# located in the current directory (but tests are run in the
> -# binary directory), so LUA_PATH need to be updated.
> +# in src/ directory, jit/vmdef.lua is autogenerated file also
> +# located in src/ directory, but in scope of the binary artefacts
> +# tree and auxiliary tests-related modules are located in the
> +# current directory (but tests are run in the binary directory),
> +# so LUA_PATH need to be updated.
>   make_lua_path(LUA_PATH
>     PATHS
>       ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
>       ${PROJECT_SOURCE_DIR}/tools/?.lua
>       ${PROJECT_SOURCE_DIR}/src/?.lua
> +    ${PROJECT_BINARY_DIR}/src/?.lua
>   )
>   # Update LUA_CPATH with the library paths collected within
>   # <BuildTestLib> macro.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 4/8] ci: use out of source build in GitHub Actions
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 4/8] ci: use out of source build in GitHub Actions Igor Munkin via Tarantool-patches
@ 2022-08-15 12:13   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18  9:58     ` Sergey Kaplun via Tarantool-patches
  2022-08-31 15:33     ` Igor Munkin via Tarantool-patches
  0 siblings, 2 replies; 39+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-08-15 12:13 UTC (permalink / raw)
  To: Igor Munkin, Sergey Kaplun; +Cc: tarantool-patches

Igor, please see my comment below.

On 11.08.2022 14:17, Igor Munkin wrote:
> Use out of source build configuration for LuaJIT testing jobs in all
> GitHub workflows. For this build type configuration unique subdirectory
> (using GitHub run ID) within runner temporary directory is used as a
> binary artefacts tree.
>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>   .github/actions/environment/action.yml   | 5 +++++
>   .github/workflows/lint.yml               | 3 ++-
>   .github/workflows/linux-aarch64.yml      | 6 +++++-
>   .github/workflows/linux-x86_64-ninja.yml | 6 +++++-
>   .github/workflows/linux-x86_64.yml       | 7 ++++++-
>   .github/workflows/macos-m1.yml           | 6 +++++-
>   .github/workflows/macos-x86_64.yml       | 7 ++++++-
>   7 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/.github/actions/environment/action.yml b/.github/actions/environment/action.yml
> index 43323bc7..7fb2625f 100644
> --- a/.github/actions/environment/action.yml
> +++ b/.github/actions/environment/action.yml
> @@ -11,3 +11,8 @@ runs:
>           NPROC=$(sysctl -n hw.logicalcpu 2>/dev/null || nproc)
>           echo CMAKE_BUILD_PARALLEL_LEVEL=$(($NPROC + 1)) | tee -a $GITHUB_ENV
>         shell: bash
> +    - run: |
> +        # Set BUILDDIR environment variable to specify LuaJIT
> +        # build directory.
> +        echo "BUILDDIR=${{ runner.temp }}/build-${{ github.run_id }}" | tee -a $GITHUB_ENV
> +      shell: bash
> diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
> index 28dc6be6..64e8f992 100644
> --- a/.github/workflows/lint.yml
> +++ b/.github/workflows/lint.yml
> @@ -45,6 +45,7 @@ jobs:
>             sudo apt -y install cmake make lua5.1 luarocks
>             sudo luarocks install luacheck
>         - name: configure
> -        run: cmake .
> +        run: cmake -S . -B ${{ env.BUILDDIR }}
>         - name: test
>           run: cmake --build . --target LuaJIT-luacheck
> +        working-directory: ${{ env.BUILDDIR }}
> diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
> index 8c8dcff1..21d86764 100644
> --- a/.github/workflows/linux-aarch64.yml
> +++ b/.github/workflows/linux-aarch64.yml
> @@ -53,11 +53,15 @@ jobs:
>             sudo apt -y update
>             sudo apt -y install cmake gcc make perl
>         - name: configure
> -        run: cmake . ${{ matrix.CMAKEFLAGS }}
> +        run: >
> +          cmake -S . -B ${{ env.BUILDDIR }}
> +          ${{ matrix.CMAKEFLAGS }}
>         - name: build
>           run: cmake --build . --parallel
> +        working-directory: ${{ env.BUILDDIR }}
>         - name: test
>           run: cmake --build . --parallel --target test
> +        working-directory: ${{ env.BUILDDIR }}

1. I don't get an idea to use current dir for CMake and specify a 
working-directory in a job step.

Why not "cmake --build ${{ env.BUILDDIR }}" as above?

>   
>     test-tarantool-debug-w-GC64:
>       name: Tarantool Debug GC64:ON
> diff --git a/.github/workflows/linux-x86_64-ninja.yml b/.github/workflows/linux-x86_64-ninja.yml
> index 2877d2f6..72d56d54 100644
> --- a/.github/workflows/linux-x86_64-ninja.yml
> +++ b/.github/workflows/linux-x86_64-ninja.yml
> @@ -44,8 +44,12 @@ jobs:
>             sudo apt -y update
>             sudo apt -y install cmake gcc ninja-build perl
>         - name: configure
> -        run: cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -G Ninja
> +        run: >
> +          cmake -S . -B ${{ env.BUILDDIR }}
> +          -DCMAKE_BUILD_TYPE=RelWithDebInfo -G Ninja
>         - name: build
>           run: cmake --build . --parallel
> +        working-directory: ${{ env.BUILDDIR }}
>         - name: test
>           run: cmake --build . --parallel --target test
> +        working-directory: ${{ env.BUILDDIR }}
> diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux-x86_64.yml
> index 44dcce98..4c3ad4c7 100644
> --- a/.github/workflows/linux-x86_64.yml
> +++ b/.github/workflows/linux-x86_64.yml
> @@ -54,11 +54,16 @@ jobs:
>             sudo apt -y update
>             sudo apt -y install cmake gcc make perl
>         - name: configure
> -        run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
> +        run: >
> +          cmake -S . -B ${{ env.BUILDDIR }}
> +          ${{ matrix.CMAKEFLAGS }}
> +          -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
>         - name: build
>           run: cmake --build . --parallel
> +        working-directory: ${{ env.BUILDDIR }}
>         - name: test
>           run: cmake --build . --parallel --target test
> +        working-directory: ${{ env.BUILDDIR }}
>   
>     test-tarantool-debug-wo-GC64:
>       name: Tarantool Debug GC64:OFF
> diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
> index e0269d60..e3b6dcda 100644
> --- a/.github/workflows/macos-m1.yml
> +++ b/.github/workflows/macos-m1.yml
> @@ -68,11 +68,15 @@ jobs:
>               ${ARCH} brew upgrade cmake gcc make perl
>             ${ARCH} echo "CMAKE_BUILD_PARALLEL_LEVEL=$(($(sysctl -n hw.logicalcpu) + 1))" >> $GITHUB_ENV
>         - name: configure
> -        run: ${ARCH} cmake . ${{ matrix.CMAKEFLAGS }}
> +        run: >
> +          ${ARCH} cmake -S . -B ${{ env.BUILDDIR }}
> +          ${{ matrix.CMAKEFLAGS }}
>         - name: build
>           run: ${ARCH} cmake --build . --parallel
> +        working-directory: ${{ env.BUILDDIR }}
>         - name: test
>           run: ${ARCH} cmake --build . --parallel --target test
> +        working-directory: ${{ env.BUILDDIR }}
>   
>     test-tarantool-debug-w-GC64:
>       name: Tarantool Debug GC64:ON
> diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos-x86_64.yml
> index 840806e3..3d2cf581 100644
> --- a/.github/workflows/macos-x86_64.yml
> +++ b/.github/workflows/macos-x86_64.yml
> @@ -63,11 +63,16 @@ jobs:
>             brew install --force cmake gcc make perl ||
>               brew upgrade cmake gcc make perl
>         - name: configure
> -        run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
> +        run: >
> +          cmake -S . -B ${{ env.BUILDDIR }}
> +          ${{ matrix.CMAKEFLAGS }}
> +          -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
>         - name: build
>           run: cmake --build . --parallel
> +        working-directory: ${{ env.BUILDDIR }}
>         - name: test
>           run: cmake --build . --parallel --target test
> +        working-directory: ${{ env.BUILDDIR }}
>   
>     test-tarantool-debug-wo-GC64:
>       name: Tarantool Debug GC64:OFF

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 5/8] ci: remove excess parallel level setup
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 5/8] ci: remove excess parallel level setup Igor Munkin via Tarantool-patches
@ 2022-08-15 12:14   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18 10:09   ` Sergey Kaplun via Tarantool-patches
  1 sibling, 0 replies; 39+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-08-15 12:14 UTC (permalink / raw)
  To: Igor Munkin, Sergey Kaplun; +Cc: tarantool-patches

LGTM, thanks

On 11.08.2022 14:17, Igor Munkin wrote:
> This patch removes the artefact line left as a result of squashing
> several fixup commits into 36b4b80cbdcf55f5ffdbcea0097c4dc315638c50
> ("ci: introduce GitHub action for environment setup").
>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>   .github/workflows/macos-m1.yml | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
> index e3b6dcda..d5b0e0f1 100644
> --- a/.github/workflows/macos-m1.yml
> +++ b/.github/workflows/macos-m1.yml
> @@ -66,7 +66,6 @@ jobs:
>             # if the package already exists with the previous version.
>             ${ARCH} brew install --force cmake gcc make perl ||
>               ${ARCH} brew upgrade cmake gcc make perl
> -          ${ARCH} echo "CMAKE_BUILD_PARALLEL_LEVEL=$(($(sysctl -n hw.logicalcpu) + 1))" >> $GITHUB_ENV
>         - name: configure
>           run: >
>             ${ARCH} cmake -S . -B ${{ env.BUILDDIR }}

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 6/8] ci: remove arch prefix for macOS M1 workflow
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 6/8] ci: remove arch prefix for macOS M1 workflow Igor Munkin via Tarantool-patches
@ 2022-08-15 12:17   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18 10:14   ` Sergey Kaplun via Tarantool-patches
  1 sibling, 0 replies; 39+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-08-15 12:17 UTC (permalink / raw)
  To: Igor Munkin, Sergey Kaplun; +Cc: tarantool-patches

LGTM, thanks

On 11.08.2022 14:17, Igor Munkin wrote:
> Finally self-hosted GitHub Actions runners support Apple M1 hardware. As
> a result there is no need to explicitly add `arch -arm64` prefix for all
> commands being run in scope of macOS M1 workflows.
>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>   .github/workflows/macos-m1.yml | 19 +++++++------------
>   1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
> index d5b0e0f1..4e1275f7 100644
> --- a/.github/workflows/macos-m1.yml
> +++ b/.github/workflows/macos-m1.yml
> @@ -28,11 +28,6 @@ concurrency:
>       format('{0}-{1}', github.workflow, github.ref) }}
>     cancel-in-progress: true
>   
> -env:
> -  # XXX: Github Actions agent uses x86_64 simulation, so we have
> -  # to explicitly make it use native environment for job steps.
> -  ARCH: arch -arm64
> -
>   jobs:
>     test-luajit:
>       runs-on: macos-11-m1
> @@ -60,21 +55,21 @@ jobs:
>             # XXX: 'echo' command below is required since brew installation
>             # script obliges the one to enter a newline for confirming the
>             # installation via Ruby script.
> -          ${ARCH} brew update ||
> -            echo | ${ARCH} /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
> +          brew update ||
> +            echo | /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
>             # Try to install the packages either upgrade it to avoid of fails
>             # if the package already exists with the previous version.
> -          ${ARCH} brew install --force cmake gcc make perl ||
> -            ${ARCH} brew upgrade cmake gcc make perl
> +          brew install --force cmake gcc make perl ||
> +            brew upgrade cmake gcc make perl
>         - name: configure
>           run: >
> -          ${ARCH} cmake -S . -B ${{ env.BUILDDIR }}
> +          cmake -S . -B ${{ env.BUILDDIR }}
>             ${{ matrix.CMAKEFLAGS }}
>         - name: build
> -        run: ${ARCH} cmake --build . --parallel
> +        run: cmake --build . --parallel
>           working-directory: ${{ env.BUILDDIR }}
>         - name: test
> -        run: ${ARCH} cmake --build . --parallel --target test
> +        run: cmake --build . --parallel --target test
>           working-directory: ${{ env.BUILDDIR }}
>   
>     test-tarantool-debug-w-GC64:

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 7/8] ci: merge x86_64 and ARM64 workflows
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 7/8] ci: merge x86_64 and ARM64 workflows Igor Munkin via Tarantool-patches
@ 2022-08-15 12:22   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18 10:21   ` Sergey Kaplun via Tarantool-patches
  1 sibling, 0 replies; 39+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-08-15 12:22 UTC (permalink / raw)
  To: Igor Munkin, Sergey Kaplun; +Cc: tarantool-patches

Looks good, LGTM

On 11.08.2022 14:17, Igor Munkin wrote:
> As a result of the previous commit, there is no difference left between
> x86_64 and ARM64 workflows except the runner where workflow is run.
> Hence there is no reason to have a separate workflow file for each
> hardware architecture.
>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>   .github/workflows/linux-aarch64.yml           | 83 -----------------
>   .../workflows/{linux-x86_64.yml => linux.yml} | 52 ++++++++---
>   .github/workflows/macos-m1.yml                | 92 -------------------
>   .../workflows/{macos-x86_64.yml => macos.yml} | 52 ++++++++---
>   4 files changed, 82 insertions(+), 197 deletions(-)
>   delete mode 100644 .github/workflows/linux-aarch64.yml
>   rename .github/workflows/{linux-x86_64.yml => linux.yml} (69%)
>   delete mode 100644 .github/workflows/macos-m1.yml
>   rename .github/workflows/{macos-x86_64.yml => macos.yml} (73%)
>
> diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
> deleted file mode 100644
> index 21d86764..00000000
> --- a/.github/workflows/linux-aarch64.yml
> +++ /dev/null
> @@ -1,83 +0,0 @@
> -name: "Linux/aarch64 test workflow"
> -
> -on:
> -  push:
> -    branches-ignore:
> -      - '**-notest'
> -      - 'upstream-**'
> -    tags-ignore:
> -      - '**'
> -
> -concurrency:
> -  # An update of a developer branch cancels the previously
> -  # scheduled workflow run for this branch. However, the default
> -  # branch, and long-term branch (tarantool-1.10, tarantool-2.8,
> -  # etc.) workflow runs are never canceled.
> -  #
> -  # We use a trick here: define the concurrency group as 'workflow
> -  # run ID' + # 'workflow run attempt' because it is a unique
> -  # combination for any run. So it effectively discards grouping.
> -  #
> -  # XXX: we cannot use `github.sha` as a unique identifier because
> -  # pushing a tag may cancel a run that works on a branch push
> -  # event.
> -  group: ${{ (
> -    github.ref == 'refs/heads/tarantool' ||
> -    startsWith(github.ref, 'refs/heads/tarantool-')) &&
> -    format('{0}-{1}', github.run_id, github.run_attempt) ||
> -    format('{0}-{1}', github.workflow, github.ref) }}
> -  cancel-in-progress: true
> -
> -jobs:
> -  test-luajit:
> -    runs-on: graviton
> -    strategy:
> -      fail-fast: false
> -      matrix:
> -        BUILDTYPE: [Debug, Release]
> -        include:
> -          - BUILDTYPE: Debug
> -            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
> -          - BUILDTYPE: Release
> -            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> -    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:ON
> -    steps:
> -      - uses: actions/checkout@v2.3.4
> -        with:
> -          fetch-depth: 0
> -          submodules: recursive
> -      - name: environment
> -        uses: ./.github/actions/environment
> -      - name: setup
> -        run: |
> -          sudo apt -y update
> -          sudo apt -y install cmake gcc make perl
> -      - name: configure
> -        run: >
> -          cmake -S . -B ${{ env.BUILDDIR }}
> -          ${{ matrix.CMAKEFLAGS }}
> -      - name: build
> -        run: cmake --build . --parallel
> -        working-directory: ${{ env.BUILDDIR }}
> -      - name: test
> -        run: cmake --build . --parallel --target test
> -        working-directory: ${{ env.BUILDDIR }}
> -
> -  test-tarantool-debug-w-GC64:
> -    name: Tarantool Debug GC64:ON
> -    needs: test-luajit
> -    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> -    with:
> -      GC64: ON
> -      buildtype: Debug
> -      host: graviton
> -      revision: ${{ github.sha }}
> -  test-tarantool-release-w-GC64:
> -    name: Tarantool Release GC64:ON
> -    needs: test-luajit
> -    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> -    with:
> -      GC64: ON
> -      buildtype: RelWithDebInfo
> -      host: graviton
> -      revision: ${{ github.sha }}
> diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux.yml
> similarity index 69%
> rename from .github/workflows/linux-x86_64.yml
> rename to .github/workflows/linux.yml
> index 4c3ad4c7..125c8708 100644
> --- a/.github/workflows/linux-x86_64.yml
> +++ b/.github/workflows/linux.yml
> @@ -1,4 +1,4 @@
> -name: "Linux/x86_64 test workflow"
> +name: "Linux test workflow"
>   
>   on:
>     push:
> @@ -30,18 +30,30 @@ concurrency:
>   
>   jobs:
>     test-luajit:
> -    runs-on: ubuntu-20.04-self-hosted
>       strategy:
>         fail-fast: false
>         matrix:
> +        ARCH: [aarch64, x86_64]
>           BUILDTYPE: [Debug, Release]
>           GC64: [ON, OFF]
>           include:
> +          - ARCH: aarch64
> +            RUNNER: graviton
> +          - ARCH: x86_64
> +            RUNNER: ubuntu-20.04-self-hosted
>             - BUILDTYPE: Debug
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
>             - BUILDTYPE: Release
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> -    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
> +        exclude:
> +          - ARCH: aarch64
> +            GC64: OFF
> +    runs-on: ${{ matrix.RUNNER }}
> +    name: >
> +      LuaJIT
> +      ${{ matrix.ARCH }}
> +      ${{ matrix.BUILDTYPE }}
> +      GC64:${{ matrix.GC64 }}
>       steps:
>         - uses: actions/checkout@v2.3.4
>           with:
> @@ -65,8 +77,8 @@ jobs:
>           run: cmake --build . --parallel --target test
>           working-directory: ${{ env.BUILDDIR }}
>   
> -  test-tarantool-debug-wo-GC64:
> -    name: Tarantool Debug GC64:OFF
> +  test-tarantool-x86_64-debug-wo-GC64:
> +    name: Tarantool x86_64 Debug GC64:OFF
>       needs: test-luajit
>       uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>       with:
> @@ -74,8 +86,8 @@ jobs:
>         buildtype: Debug
>         host: ubuntu-20.04-self-hosted
>         revision: ${{ github.sha }}
> -  test-tarantool-debug-w-GC64:
> -    name: Tarantool Debug GC64:ON
> +  test-tarantool-x86_64-debug-w-GC64:
> +    name: Tarantool x86_64 Debug GC64:ON
>       needs: test-luajit
>       uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>       with:
> @@ -83,8 +95,8 @@ jobs:
>         buildtype: Debug
>         host: ubuntu-20.04-self-hosted
>         revision: ${{ github.sha }}
> -  test-tarantool-release-wo-GC64:
> -    name: Tarantool Release GC64:OFF
> +  test-tarantool-x86_64-release-wo-GC64:
> +    name: Tarantool x86_64 Release GC64:OFF
>       needs: test-luajit
>       uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>       with:
> @@ -92,8 +104,8 @@ jobs:
>         buildtype: RelWithDebInfo
>         host: ubuntu-20.04-self-hosted
>         revision: ${{ github.sha }}
> -  test-tarantool-release-w-GC64:
> -    name: Tarantool Release GC64:ON
> +  test-tarantool-x86_64-release-w-GC64:
> +    name: Tarantool x86_64 Release GC64:ON
>       needs: test-luajit
>       uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>       with:
> @@ -101,3 +113,21 @@ jobs:
>         buildtype: RelWithDebInfo
>         host: ubuntu-20.04-self-hosted
>         revision: ${{ github.sha }}
> +  test-tarantool-aarch64-debug-w-GC64:
> +    name: Tarantool aarch64 Debug GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: Debug
> +      host: graviton
> +      revision: ${{ github.sha }}
> +  test-tarantool-aarch64-release-w-GC64:
> +    name: Tarantool aarch64 Release GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: RelWithDebInfo
> +      host: graviton
> +      revision: ${{ github.sha }}
> diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
> deleted file mode 100644
> index 4e1275f7..00000000
> --- a/.github/workflows/macos-m1.yml
> +++ /dev/null
> @@ -1,92 +0,0 @@
> -name: "macOS/m1 test workflow"
> -
> -on:
> -  push:
> -    branches-ignore:
> -      - '**-notest'
> -      - 'upstream-**'
> -    tags-ignore:
> -      - '**'
> -
> -concurrency:
> -  # An update of a developer branch cancels the previously
> -  # scheduled workflow run for this branch. However, the default
> -  # branch, and long-term branch (tarantool-1.10, tarantool-2.8,
> -  # etc.) workflow runs are never canceled.
> -  #
> -  # We use a trick here: define the concurrency group as 'workflow
> -  # run ID' + # 'workflow run attempt' because it is a unique
> -  # combination for any run. So it effectively discards grouping.
> -  #
> -  # XXX: we cannot use `github.sha` as a unique identifier because
> -  # pushing a tag may cancel a run that works on a branch push
> -  # event.
> -  group: ${{ (
> -    github.ref == 'refs/heads/tarantool' ||
> -    startsWith(github.ref, 'refs/heads/tarantool-')) &&
> -    format('{0}-{1}', github.run_id, github.run_attempt) ||
> -    format('{0}-{1}', github.workflow, github.ref) }}
> -  cancel-in-progress: true
> -
> -jobs:
> -  test-luajit:
> -    runs-on: macos-11-m1
> -    strategy:
> -      fail-fast: false
> -      matrix:
> -        BUILDTYPE: [Debug, Release]
> -        include:
> -          - BUILDTYPE: Debug
> -            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
> -          - BUILDTYPE: Release
> -            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> -    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:ON
> -    steps:
> -      - uses: actions/checkout@v2.3.4
> -        with:
> -          fetch-depth: 0
> -          submodules: recursive
> -      - name: environment
> -        uses: ./.github/actions/environment
> -      - name: setup
> -        run: |
> -          # Install brew using the command from Homebrew repository
> -          # instructions: https://github.com/Homebrew/install.
> -          # XXX: 'echo' command below is required since brew installation
> -          # script obliges the one to enter a newline for confirming the
> -          # installation via Ruby script.
> -          brew update ||
> -            echo | /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
> -          # Try to install the packages either upgrade it to avoid of fails
> -          # if the package already exists with the previous version.
> -          brew install --force cmake gcc make perl ||
> -            brew upgrade cmake gcc make perl
> -      - name: configure
> -        run: >
> -          cmake -S . -B ${{ env.BUILDDIR }}
> -          ${{ matrix.CMAKEFLAGS }}
> -      - name: build
> -        run: cmake --build . --parallel
> -        working-directory: ${{ env.BUILDDIR }}
> -      - name: test
> -        run: cmake --build . --parallel --target test
> -        working-directory: ${{ env.BUILDDIR }}
> -
> -  test-tarantool-debug-w-GC64:
> -    name: Tarantool Debug GC64:ON
> -    needs: test-luajit
> -    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> -    with:
> -      GC64: ON
> -      buildtype: Debug
> -      host: macos-11-m1
> -      revision: ${{ github.sha }}
> -  test-tarantool-release-w-GC64:
> -    name: Tarantool Release GC64:ON
> -    needs: test-luajit
> -    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> -    with:
> -      GC64: ON
> -      buildtype: RelWithDebInfo
> -      host: macos-11-m1
> -      revision: ${{ github.sha }}
> diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos.yml
> similarity index 73%
> rename from .github/workflows/macos-x86_64.yml
> rename to .github/workflows/macos.yml
> index 3d2cf581..2fa97f1e 100644
> --- a/.github/workflows/macos-x86_64.yml
> +++ b/.github/workflows/macos.yml
> @@ -1,4 +1,4 @@
> -name: "macOS/x86_64 test workflow"
> +name: "macOS test workflow"
>   
>   on:
>     push:
> @@ -30,18 +30,30 @@ concurrency:
>   
>   jobs:
>     test-luajit:
> -    runs-on: macos-11
>       strategy:
>         fail-fast: false
>         matrix:
> +        ARCH: [M1, x86_64]
>           BUILDTYPE: [Debug, Release]
>           GC64: [ON, OFF]
>           include:
> +          - ARCH: M1
> +            RUNNER: macos-11-m1
> +          - ARCH: x86_64
> +            RUNNER: macos-11
>             - BUILDTYPE: Debug
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
>             - BUILDTYPE: Release
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> -    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
> +        exclude:
> +          - ARCH: M1
> +            GC64: OFF
> +    runs-on: ${{ matrix.RUNNER }}
> +    name: >
> +      LuaJIT
> +      ${{ matrix.ARCH }}
> +      ${{ matrix.BUILDTYPE }}
> +      GC64:${{ matrix.GC64 }}
>       steps:
>         - uses: actions/checkout@v2.3.4
>           with:
> @@ -74,8 +86,8 @@ jobs:
>           run: cmake --build . --parallel --target test
>           working-directory: ${{ env.BUILDDIR }}
>   
> -  test-tarantool-debug-wo-GC64:
> -    name: Tarantool Debug GC64:OFF
> +  test-tarantool-x86_64-debug-wo-GC64:
> +    name: Tarantool x86_64 Debug GC64:OFF
>       needs: test-luajit
>       uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>       with:
> @@ -83,8 +95,8 @@ jobs:
>         buildtype: Debug
>         host: macos-11
>         revision: ${{ github.sha }}
> -  test-tarantool-debug-w-GC64:
> -    name: Tarantool Debug GC64:ON
> +  test-tarantool-x86_64-debug-w-GC64:
> +    name: Tarantool x86_64 Debug GC64:ON
>       needs: test-luajit
>       uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>       with:
> @@ -92,8 +104,8 @@ jobs:
>         buildtype: Debug
>         host: macos-11
>         revision: ${{ github.sha }}
> -  test-tarantool-release-wo-GC64:
> -    name: Tarantool Release GC64:OFF
> +  test-tarantool-x86_64-release-wo-GC64:
> +    name: Tarantool x86_64 Release GC64:OFF
>       needs: test-luajit
>       uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>       with:
> @@ -101,8 +113,8 @@ jobs:
>         buildtype: RelWithDebInfo
>         host: macos-11
>         revision: ${{ github.sha }}
> -  test-tarantool-release-w-GC64:
> -    name: Tarantool Release GC64:ON
> +  test-tarantool-x86_64-release-w-GC64:
> +    name: Tarantool x86_64 Release GC64:ON
>       needs: test-luajit
>       uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>       with:
> @@ -110,3 +122,21 @@ jobs:
>         buildtype: RelWithDebInfo
>         host: macos-11
>         revision: ${{ github.sha }}
> +  test-tarantool-m1-debug-w-GC64:
> +    name: Tarantool M1 Debug GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: Debug
> +      host: macos-11-m1
> +      revision: ${{ github.sha }}
> +  test-tarantool-m1-release-w-GC64:
> +    name: Tarantool M1 Release GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: RelWithDebInfo
> +      host: macos-11-m1
> +      revision: ${{ github.sha }}

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 8/8] ci: merge Linux and macOS workflows
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 8/8] ci: merge Linux and macOS workflows Igor Munkin via Tarantool-patches
@ 2022-08-15 12:27   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18 10:32   ` Sergey Kaplun via Tarantool-patches
  1 sibling, 0 replies; 39+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-08-15 12:27 UTC (permalink / raw)
  To: Igor Munkin, Sergey Kaplun; +Cc: tarantool-patches

LGTM, thanks

I'm glad to see lesser number of GH workflows :)

On 11.08.2022 14:17, Igor Munkin wrote:
> As a result of two previous commits, there is no difference left between
> Linux and macOS workflows except the environment setup (environment
> variables, dependencies installation). Hence there is no reason to have a
> separate workflow file for each OS type.
>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>   .github/actions/environment/action.yml        |  18 ---
>   .github/actions/setup-linux/README.md         |  12 ++
>   .github/actions/setup-linux/action.yml        |  19 +++
>   .github/actions/setup-macos/README.md         |  12 ++
>   .github/actions/setup-macos/action.yml        |  29 ++++
>   .../actions/{environment => setup}/README.md  |   5 +-
>   .github/actions/setup/action.yml              |  10 ++
>   .github/workflows/lint.yml                    |   8 +-
>   .github/workflows/linux-x86_64-ninja.yml      |   6 +-
>   .github/workflows/macos.yml                   | 142 ------------------
>   .github/workflows/{linux.yml => testing.yml}  | 113 +++++++++++---
>   11 files changed, 186 insertions(+), 188 deletions(-)
>   delete mode 100644 .github/actions/environment/action.yml
>   create mode 100644 .github/actions/setup-linux/README.md
>   create mode 100644 .github/actions/setup-linux/action.yml
>   create mode 100644 .github/actions/setup-macos/README.md
>   create mode 100644 .github/actions/setup-macos/action.yml
>   rename .github/actions/{environment => setup}/README.md (72%)
>   create mode 100644 .github/actions/setup/action.yml
>   delete mode 100644 .github/workflows/macos.yml
>   rename .github/workflows/{linux.yml => testing.yml} (52%)
>
> diff --git a/.github/actions/environment/action.yml b/.github/actions/environment/action.yml
> deleted file mode 100644
> index 7fb2625f..00000000
> --- a/.github/actions/environment/action.yml
> +++ /dev/null
> @@ -1,18 +0,0 @@
> -name: 'Setup CI environment'
> -description: 'Common part to tweak CI runner environment'
> -runs:
> -  using: 'composite'
> -  steps:
> -    - run: |
> -        # Set CMAKE_BUILD_PARALLEL_LEVEL environment variable to
> -        # limit the number of parallel jobs for build/test step.
> -        # Try the exotic Darwin way at first and fallback to nproc
> -        # that is the default for the normal systems.
> -        NPROC=$(sysctl -n hw.logicalcpu 2>/dev/null || nproc)
> -        echo CMAKE_BUILD_PARALLEL_LEVEL=$(($NPROC + 1)) | tee -a $GITHUB_ENV
> -      shell: bash
> -    - run: |
> -        # Set BUILDDIR environment variable to specify LuaJIT
> -        # build directory.
> -        echo "BUILDDIR=${{ runner.temp }}/build-${{ github.run_id }}" | tee -a $GITHUB_ENV
> -      shell: bash
> diff --git a/.github/actions/setup-linux/README.md b/.github/actions/setup-linux/README.md
> new file mode 100644
> index 00000000..48ae9e01
> --- /dev/null
> +++ b/.github/actions/setup-linux/README.md
> @@ -0,0 +1,12 @@
> +# Setup environment on Linux
> +
> +Action setups the environment on Linux runners (install requirements, setup the
> +workflow environment, etc).
> +
> +## How to use Github Action from Github workflow
> +
> +Add the following code to the running steps before LuaJIT configuration:
> +```
> +- uses: ./.github/actions/setup-linux
> +  if: ${{ matrix.OS == 'Linux' }}
> +```
> diff --git a/.github/actions/setup-linux/action.yml b/.github/actions/setup-linux/action.yml
> new file mode 100644
> index 00000000..a13f143c
> --- /dev/null
> +++ b/.github/actions/setup-linux/action.yml
> @@ -0,0 +1,19 @@
> +name: Setup CI environment on Linux
> +description: Common part to tweak Linux CI runner environment
> +runs:
> +  using: composite
> +  steps:
> +    - name: Setup CI environment
> +      uses: ./.github/actions/setup
> +    - name: Set CMAKE_BUILD_PARALLEL_LEVEL
> +      run: |
> +        # Set CMAKE_BUILD_PARALLEL_LEVEL environment variable to
> +        # limit the number of parallel jobs for build/test step.
> +        NPROC=$(nproc)
> +        echo CMAKE_BUILD_PARALLEL_LEVEL=$(($NPROC + 1)) | tee -a $GITHUB_ENV
> +      shell: bash
> +    - name: Install build and test dependencies
> +      run: |
> +        apt -y update
> +        apt -y install cmake gcc make perl
> +      shell: bash
> diff --git a/.github/actions/setup-macos/README.md b/.github/actions/setup-macos/README.md
> new file mode 100644
> index 00000000..ae70cb56
> --- /dev/null
> +++ b/.github/actions/setup-macos/README.md
> @@ -0,0 +1,12 @@
> +# Setup environment on macOS
> +
> +Action setups the environment on macOS runners (install requirements, setup the
> +workflow environment, etc).
> +
> +## How to use Github Action from Github workflow
> +
> +Add the following code to the running steps before LuaJIT configuration:
> +```
> +- uses: ./.github/actions/setup-macos
> +  if: ${{ matrix.OS == 'macOS' }}
> +```
> diff --git a/.github/actions/setup-macos/action.yml b/.github/actions/setup-macos/action.yml
> new file mode 100644
> index 00000000..7a98065c
> --- /dev/null
> +++ b/.github/actions/setup-macos/action.yml
> @@ -0,0 +1,29 @@
> +name: Setup CI environment on macOS
> +description: Common part to tweak macOS CI runner environment
> +runs:
> +  using: composite
> +  steps:
> +    - name: Setup CI environment
> +      uses: ./.github/actions/setup
> +    - name: Set CMAKE_BUILD_PARALLEL_LEVEL
> +      run: |
> +        # Set CMAKE_BUILD_PARALLEL_LEVEL environment variable to
> +        # limit the number of parallel jobs for build/test step.
> +        NPROC=$(sysctl -n hw.logicalcpu 2>/dev/null)
> +        echo CMAKE_BUILD_PARALLEL_LEVEL=$(($NPROC + 1)) | tee -a $GITHUB_ENV
> +      shell: bash
> +    - name: Install build and test dependencies
> +      run: |
> +        # Install brew using the command from Homebrew repository
> +        # instructions: https://github.com/Homebrew/install.
> +        # XXX: 'echo' command below is required since brew
> +        # installation script obliges the one to enter a newline
> +        # for confirming the installation via Ruby script.
> +        brew update ||
> +          echo | /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
> +        # Try to install the packages either upgrade it to avoid
> +        # of fails if the package already exists with the previous
> +        # version.
> +        brew install --force cmake gcc make perl ||
> +          brew upgrade cmake gcc make perl
> +      shell: bash
> diff --git a/.github/actions/environment/README.md b/.github/actions/setup/README.md
> similarity index 72%
> rename from .github/actions/environment/README.md
> rename to .github/actions/setup/README.md
> index 1ed0d0ca..87b5bf8d 100644
> --- a/.github/actions/environment/README.md
> +++ b/.github/actions/setup/README.md
> @@ -6,7 +6,8 @@ It is used as a common environment setup for the different testing workflows.
>   
>   ## How to use Github Action from Github workflow
>   
> -Add the following code to the running steps after checkout is finished:
> +Add the following code to the running steps or OS-specific GitHub Actions after
> +checkout step is finished:
>   ```
> -- uses: ./.github/actions/environment
> +- uses: ./.github/actions/setup
>   ```
> diff --git a/.github/actions/setup/action.yml b/.github/actions/setup/action.yml
> new file mode 100644
> index 00000000..f153b0d9
> --- /dev/null
> +++ b/.github/actions/setup/action.yml
> @@ -0,0 +1,10 @@
> +name: Setup CI environment
> +description: Common part to tweak CI runner environment
> +runs:
> +  using: composite
> +  steps:
> +    - run: |
> +        # Set BUILDDIR environment variable to specify LuaJIT
> +        # build directory.
> +        echo BUILDDIR=${{ runner.temp }}/build-${{ github.run_id }} | tee -a $GITHUB_ENV
> +      shell: bash
> diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
> index 64e8f992..04b810f2 100644
> --- a/.github/workflows/lint.yml
> +++ b/.github/workflows/lint.yml
> @@ -1,4 +1,4 @@
> -name: "LuaJIT static analysis"
> +name: Static analysis
>   
>   on:
>     push:
> @@ -38,12 +38,16 @@ jobs:
>             fetch-depth: 0
>             submodules: recursive
>         - name: environment
> -        uses: ./.github/actions/environment
> +        uses: ./.github/actions/setup
>         - name: setup
>           run: |
> +          # TODO: Move this step to a separate action.
>             sudo apt -y update
>             sudo apt -y install cmake make lua5.1 luarocks
>             sudo luarocks install luacheck
> +          # Set CMAKE_BUILD_PARALLEL_LEVEL environment variable to
> +          # limit the number of parallel jobs for build/test step.
> +          echo CMAKE_BUILD_PARALLEL_LEVEL=$(($(nproc) + 1)) | tee -a $GITHUB_ENV
>         - name: configure
>           run: cmake -S . -B ${{ env.BUILDDIR }}
>         - name: test
> diff --git a/.github/workflows/linux-x86_64-ninja.yml b/.github/workflows/linux-x86_64-ninja.yml
> index 72d56d54..a0422d09 100644
> --- a/.github/workflows/linux-x86_64-ninja.yml
> +++ b/.github/workflows/linux-x86_64-ninja.yml
> @@ -38,11 +38,15 @@ jobs:
>             fetch-depth: 0
>             submodules: recursive
>         - name: environment
> -        uses: ./.github/actions/environment
> +        uses: ./.github/actions/setup
>         - name: setup
>           run: |
> +          # TODO: Move this step to a separate action.
>             sudo apt -y update
>             sudo apt -y install cmake gcc ninja-build perl
> +          # Set CMAKE_BUILD_PARALLEL_LEVEL environment variable to
> +          # limit the number of parallel jobs for build/test step.
> +          echo CMAKE_BUILD_PARALLEL_LEVEL=$(($(nproc) + 1)) | tee -a $GITHUB_ENV
>         - name: configure
>           run: >
>             cmake -S . -B ${{ env.BUILDDIR }}
> diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml
> deleted file mode 100644
> index 2fa97f1e..00000000
> --- a/.github/workflows/macos.yml
> +++ /dev/null
> @@ -1,142 +0,0 @@
> -name: "macOS test workflow"
> -
> -on:
> -  push:
> -    branches-ignore:
> -      - '**-notest'
> -      - 'upstream-**'
> -    tags-ignore:
> -      - '**'
> -
> -concurrency:
> -  # An update of a developer branch cancels the previously
> -  # scheduled workflow run for this branch. However, the default
> -  # branch, and long-term branch (tarantool-1.10, tarantool-2.8,
> -  # etc.) workflow runs are never canceled.
> -  #
> -  # We use a trick here: define the concurrency group as 'workflow
> -  # run ID' + # 'workflow run attempt' because it is a unique
> -  # combination for any run. So it effectively discards grouping.
> -  #
> -  # XXX: we cannot use `github.sha` as a unique identifier because
> -  # pushing a tag may cancel a run that works on a branch push
> -  # event.
> -  group: ${{ (
> -    github.ref == 'refs/heads/tarantool' ||
> -    startsWith(github.ref, 'refs/heads/tarantool-')) &&
> -    format('{0}-{1}', github.run_id, github.run_attempt) ||
> -    format('{0}-{1}', github.workflow, github.ref) }}
> -  cancel-in-progress: true
> -
> -jobs:
> -  test-luajit:
> -    strategy:
> -      fail-fast: false
> -      matrix:
> -        ARCH: [M1, x86_64]
> -        BUILDTYPE: [Debug, Release]
> -        GC64: [ON, OFF]
> -        include:
> -          - ARCH: M1
> -            RUNNER: macos-11-m1
> -          - ARCH: x86_64
> -            RUNNER: macos-11
> -          - BUILDTYPE: Debug
> -            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
> -          - BUILDTYPE: Release
> -            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> -        exclude:
> -          - ARCH: M1
> -            GC64: OFF
> -    runs-on: ${{ matrix.RUNNER }}
> -    name: >
> -      LuaJIT
> -      ${{ matrix.ARCH }}
> -      ${{ matrix.BUILDTYPE }}
> -      GC64:${{ matrix.GC64 }}
> -    steps:
> -      - uses: actions/checkout@v2.3.4
> -        with:
> -          fetch-depth: 0
> -          submodules: recursive
> -      - name: environment
> -        uses: ./.github/actions/environment
> -      - name: setup
> -        run: |
> -          # Install brew using the command from Homebrew repository
> -          # instructions: https://github.com/Homebrew/install.
> -          # XXX: 'echo' command below is required since brew installation
> -          # script obliges the one to enter a newline for confirming the
> -          # installation via Ruby script.
> -          brew update ||
> -            echo | /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
> -          # Try to install the packages either upgrade it to avoid of fails
> -          # if the package already exists with the previous version.
> -          brew install --force cmake gcc make perl ||
> -            brew upgrade cmake gcc make perl
> -      - name: configure
> -        run: >
> -          cmake -S . -B ${{ env.BUILDDIR }}
> -          ${{ matrix.CMAKEFLAGS }}
> -          -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
> -      - name: build
> -        run: cmake --build . --parallel
> -        working-directory: ${{ env.BUILDDIR }}
> -      - name: test
> -        run: cmake --build . --parallel --target test
> -        working-directory: ${{ env.BUILDDIR }}
> -
> -  test-tarantool-x86_64-debug-wo-GC64:
> -    name: Tarantool x86_64 Debug GC64:OFF
> -    needs: test-luajit
> -    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> -    with:
> -      GC64: OFF
> -      buildtype: Debug
> -      host: macos-11
> -      revision: ${{ github.sha }}
> -  test-tarantool-x86_64-debug-w-GC64:
> -    name: Tarantool x86_64 Debug GC64:ON
> -    needs: test-luajit
> -    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> -    with:
> -      GC64: ON
> -      buildtype: Debug
> -      host: macos-11
> -      revision: ${{ github.sha }}
> -  test-tarantool-x86_64-release-wo-GC64:
> -    name: Tarantool x86_64 Release GC64:OFF
> -    needs: test-luajit
> -    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> -    with:
> -      GC64: OFF
> -      buildtype: RelWithDebInfo
> -      host: macos-11
> -      revision: ${{ github.sha }}
> -  test-tarantool-x86_64-release-w-GC64:
> -    name: Tarantool x86_64 Release GC64:ON
> -    needs: test-luajit
> -    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> -    with:
> -      GC64: ON
> -      buildtype: RelWithDebInfo
> -      host: macos-11
> -      revision: ${{ github.sha }}
> -  test-tarantool-m1-debug-w-GC64:
> -    name: Tarantool M1 Debug GC64:ON
> -    needs: test-luajit
> -    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> -    with:
> -      GC64: ON
> -      buildtype: Debug
> -      host: macos-11-m1
> -      revision: ${{ github.sha }}
> -  test-tarantool-m1-release-w-GC64:
> -    name: Tarantool M1 Release GC64:ON
> -    needs: test-luajit
> -    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> -    with:
> -      GC64: ON
> -      buildtype: RelWithDebInfo
> -      host: macos-11-m1
> -      revision: ${{ github.sha }}
> diff --git a/.github/workflows/linux.yml b/.github/workflows/testing.yml
> similarity index 52%
> rename from .github/workflows/linux.yml
> rename to .github/workflows/testing.yml
> index 125c8708..c4847335 100644
> --- a/.github/workflows/linux.yml
> +++ b/.github/workflows/testing.yml
> @@ -1,4 +1,4 @@
> -name: "Linux test workflow"
> +name: Testing
>   
>   on:
>     push:
> @@ -33,25 +33,38 @@ jobs:
>       strategy:
>         fail-fast: false
>         matrix:
> -        ARCH: [aarch64, x86_64]
> +        ARCH: [ARM64, x86_64]
>           BUILDTYPE: [Debug, Release]
>           GC64: [ON, OFF]
> +        OS: [Linux, macOS]
>           include:
> -          - ARCH: aarch64
> +          - ARCH: ARM64
> +            OS: Linux
> +            NAME: Linux/aarch64
>               RUNNER: graviton
>             - ARCH: x86_64
> +            OS: Linux
> +            NAME: Linux/x86_64
>               RUNNER: ubuntu-20.04-self-hosted
> +          - ARCH: ARM64
> +            OS: macOS
> +            NAME: macOS/M1
> +            RUNNER: macos-11-m1
> +          - ARCH: x86_64
> +            OS: macOS
> +            NAME: macOS/x86_64
> +            RUNNER: macos-11
>             - BUILDTYPE: Debug
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
>             - BUILDTYPE: Release
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
>           exclude:
> -          - ARCH: aarch64
> +          - ARCH: ARM64
>               GC64: OFF
>       runs-on: ${{ matrix.RUNNER }}
>       name: >
>         LuaJIT
> -      ${{ matrix.ARCH }}
> +      (${{ matrix.NAME }})
>         ${{ matrix.BUILDTYPE }}
>         GC64:${{ matrix.GC64 }}
>       steps:
> @@ -59,12 +72,12 @@ jobs:
>           with:
>             fetch-depth: 0
>             submodules: recursive
> -      - name: environment
> -        uses: ./.github/actions/environment
> -      - name: setup
> -        run: |
> -          sudo apt -y update
> -          sudo apt -y install cmake gcc make perl
> +      - name: setup Linux
> +        uses: ./.github/actions/setup-linux
> +        if: ${{ matrix.OS == 'Linux' }}
> +      - name: setup macOS
> +        uses: ./.github/actions/setup-macos
> +        if: ${{ matrix.OS == 'macOS' }}
>         - name: configure
>           run: >
>             cmake -S . -B ${{ env.BUILDDIR }}
> @@ -77,8 +90,8 @@ jobs:
>           run: cmake --build . --parallel --target test
>           working-directory: ${{ env.BUILDDIR }}
>   
> -  test-tarantool-x86_64-debug-wo-GC64:
> -    name: Tarantool x86_64 Debug GC64:OFF
> +  test-tarantool-linux-x86_64-debug-wo-GC64:
> +    name: Tarantool (Linux/x86_64) Debug GC64:OFF
>       needs: test-luajit
>       uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>       with:
> @@ -86,8 +99,8 @@ jobs:
>         buildtype: Debug
>         host: ubuntu-20.04-self-hosted
>         revision: ${{ github.sha }}
> -  test-tarantool-x86_64-debug-w-GC64:
> -    name: Tarantool x86_64 Debug GC64:ON
> +  test-tarantool-linux-x86_64-debug-w-GC64:
> +    name: Tarantool (Linux/x86_64) Debug GC64:ON
>       needs: test-luajit
>       uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>       with:
> @@ -95,8 +108,8 @@ jobs:
>         buildtype: Debug
>         host: ubuntu-20.04-self-hosted
>         revision: ${{ github.sha }}
> -  test-tarantool-x86_64-release-wo-GC64:
> -    name: Tarantool x86_64 Release GC64:OFF
> +  test-tarantool-linux-x86_64-release-wo-GC64:
> +    name: Tarantool (Linux/x86_64) Release GC64:OFF
>       needs: test-luajit
>       uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>       with:
> @@ -104,8 +117,8 @@ jobs:
>         buildtype: RelWithDebInfo
>         host: ubuntu-20.04-self-hosted
>         revision: ${{ github.sha }}
> -  test-tarantool-x86_64-release-w-GC64:
> -    name: Tarantool x86_64 Release GC64:ON
> +  test-tarantool-linux-x86_64-release-w-GC64:
> +    name: Tarantool (Linux/x86_64) Release GC64:ON
>       needs: test-luajit
>       uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>       with:
> @@ -113,8 +126,8 @@ jobs:
>         buildtype: RelWithDebInfo
>         host: ubuntu-20.04-self-hosted
>         revision: ${{ github.sha }}
> -  test-tarantool-aarch64-debug-w-GC64:
> -    name: Tarantool aarch64 Debug GC64:ON
> +  test-tarantool-linux-aarch64-debug-w-GC64:
> +    name: Tarantool (Linux/aarch64) Debug GC64:ON
>       needs: test-luajit
>       uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>       with:
> @@ -122,8 +135,8 @@ jobs:
>         buildtype: Debug
>         host: graviton
>         revision: ${{ github.sha }}
> -  test-tarantool-aarch64-release-w-GC64:
> -    name: Tarantool aarch64 Release GC64:ON
> +  test-tarantool-linux-aarch64-release-w-GC64:
> +    name: Tarantool (Linux/aarch64) Release GC64:ON
>       needs: test-luajit
>       uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>       with:
> @@ -131,3 +144,57 @@ jobs:
>         buildtype: RelWithDebInfo
>         host: graviton
>         revision: ${{ github.sha }}
> +  test-tarantool-macos-x86_64-debug-wo-GC64:
> +    name: Tarantool (macOS/x86_64) Debug GC64:OFF
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: OFF
> +      buildtype: Debug
> +      host: macos-11
> +      revision: ${{ github.sha }}
> +  test-tarantool-macos-x86_64-debug-w-GC64:
> +    name: Tarantool (macOS/x86_64) Debug GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: Debug
> +      host: macos-11
> +      revision: ${{ github.sha }}
> +  test-tarantool-macos-x86_64-release-wo-GC64:
> +    name: Tarantool (macOS/x86_64) Release GC64:OFF
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: OFF
> +      buildtype: RelWithDebInfo
> +      host: macos-11
> +      revision: ${{ github.sha }}
> +  test-tarantool-macos-x86_64-release-w-GC64:
> +    name: Tarantool (macOS/x86_64) Release GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: RelWithDebInfo
> +      host: macos-11
> +      revision: ${{ github.sha }}
> +  test-tarantool-macos-m1-debug-w-GC64:
> +    name: Tarantool (macOS/M1) Debug GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: Debug
> +      host: macos-11-m1
> +      revision: ${{ github.sha }}
> +  test-tarantool-macos-m1-release-w-GC64:
> +    name: Tarantool (macOS/M1) Release GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: RelWithDebInfo
> +      host: macos-11-m1
> +      revision: ${{ github.sha }}

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 1/8] test: introduce LUAJIT_TEST_VARDIR variable
  2022-08-15 12:08   ` Sergey Bronnikov via Tarantool-patches
@ 2022-08-18  8:27     ` Sergey Kaplun via Tarantool-patches
  2022-08-31 14:53     ` Igor Munkin via Tarantool-patches
  1 sibling, 0 replies; 39+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-08-18  8:27 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Hi, Igor!

Thanks for the patch!

LGTM, after fixes for commit message mentioned by Sergey.

On 15.08.22, Sergey Bronnikov wrote:
> Igor,
> 
>   thanks for the patch. See my comments inline.
> 
> On 11.08.2022 14:17, Igor Munkin wrote:
> > Before the patch both memprof and sysprof artefacts are generated within
> > the binary artefacts tree (i.e. in the same directory LuaJIT binary is
> > generated). However, more convenient way is producing these temporary
> > profiles in a separate directory (e.g. located on the partition with
> > more strict space limits). As a result of the patch all memprof and
> > sysprof test chunks consider LUAJIT_TEST_VARDIR environment variable to
> > set the directory where test profiles are generated. For the case when
> > LUAJIT_TEST_VARDIR is not set, everything works as before.
> 
> Commit message is inconsistent a bit with a patch itself.
> 
> As far as I understand many hunks are not related to introducing 
> LUAJIT_TEST_VARDIR.
> 
> Probably it is better to change commit one-line message from "test: 
> introduce LUAJIT_TEST_VARDIR variable"
> 
> to something like "test: refactoring and introduce LUAJIT_TEST_VARDIR 
> variable". It allows to keep patch

Agree with Sergey here. This patch is more about prerequisites for this
variable.

> 
> as is and change expectations for those who will look at your patch.
> 
> 
> >
> > Part of tarantool/tarantool#7472
> >
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> >   .../gh-5813-resolving-of-c-symbols.test.lua   |  6 +++--
> >   .../misclib-memprof-lapi.test.lua             | 22 ++++++++++---------
> >   .../misclib-sysprof-lapi.test.lua             |  8 ++++---
> >   test/tarantool-tests/utils.lua                | 12 ++++++++++
> >   4 files changed, 33 insertions(+), 15 deletions(-)
> >
> > diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> > index d589dddf..e0b6d86d 100644
> > --- a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> > +++ b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> > @@ -1,5 +1,7 @@
> >   -- Memprof is implemented for x86 and x64 architectures only.
> > -require("utils").skipcond(
> > +local utils = require("utils")
> > +
> > +utils.skipcond(
> >     jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux",
> >     jit.arch.." architecture or "..jit.os..
> >     " OS is NIY for memprof c symbols resolving"
> > @@ -18,7 +20,7 @@ local testboth = require "resboth"
> >   local testhash = require "reshash"
> >   local testgnuhash = require "resgnuhash"
> >   
> > -local TMP_BINFILE = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%.%1.memprofdata.tmp.bin")
> > +local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin")
> >   
> >   local function tree_contains(node, name)
> >     if node == nil then
> > diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > index a11f0be1..bae0c27c 100644
> > --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > @@ -1,5 +1,7 @@
> >   -- Memprof is implemented for x86 and x64 architectures only.
> > -require("utils").skipcond(
> > +local utils = require("utils")
> > +
> > +utils.skipcond(
> >     jit.arch ~= "x86" and jit.arch ~= "x64",
> >     jit.arch.." architecture is NIY for memprof"
> >   )
> > @@ -26,8 +28,8 @@ local memprof = require "memprof.parse"
> >   local process = require "memprof.process"
> >   local symtab = require "utils.symtab"
> >   
> > -local TMP_BINFILE = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%.%1.memprofdata.tmp.bin")
> > -local BAD_PATH = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%1/memprofdata.tmp.bin")
> > +local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin")
> > +local BAD_PATH = utils.profilename("memprofdata/tmp.bin")
> >   local SRC_PATH = "@"..arg[0]
> >   
> >   local function default_payload()
> > @@ -189,9 +191,9 @@ test:test("output", function(subtest)
> >     -- one is the number of allocations. 1 event - alocation 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 = 35, linedefined = 33 }, 2))
> > +  subtest:ok(check_alloc_report(alloc, { line = 37, linedefined = 35 }, 2))
> >     -- 20 strings allocations.
> > -  subtest:ok(check_alloc_report(alloc, { line = 40, linedefined = 33 }, 20))
> > +  subtest:ok(check_alloc_report(alloc, { line = 42, linedefined = 35 }, 20))
> What are these magic numbers mean and why we should change them for 
> introducing LUAJIT_TEST_VARDIR?

Yes, this is nasty problems with the lines where allocation happens. As
far as we add new lines before tested function these numbers change.

> >   
> >     -- Collect all previous allocated objects.
> >     subtest:ok(free.INTERNAL.num == 22)

<snipped>

> >   return M

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 2/8] test: introduce MakeLuaPath.cmake helper
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 2/8] test: introduce MakeLuaPath.cmake helper Igor Munkin via Tarantool-patches
  2022-08-15 12:08   ` Sergey Bronnikov via Tarantool-patches
@ 2022-08-18  9:37   ` Sergey Kaplun via Tarantool-patches
  2022-08-31 15:19     ` Igor Munkin via Tarantool-patches
  1 sibling, 1 reply; 39+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-08-18  9:37 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Hi, Igor!

Thanks for the patch!

Please consider my comments below.

On 11.08.22, Igor Munkin wrote:
> While extending test suites it is often required to append additional
> path where Lua or Lua-C auxiliary modules are located to LUA_PATH or
> LUA_CPATH environment variables. Due to insane semicolon interpolation
> in CMake strings (that converts such string to a list as a result), we
> need to escape semicolon in LUA_PATH/LUA_CPATH strings while building
> the resulting value.
> 
> After the years of struggling MakeLuaPath.cmake module is introduced to

Minor: s/After/Over/

> make LUA_PATH and LUA_CPATH definition convenient with <make_lua_path>
> helper. This function takes all paths given as a variable list argument,
> joins them in a reverse order by a semicolon and yields the resulting
> string to a specified CMake variable.
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  cmake/MakeLuaPath.cmake                   | 46 +++++++++++++++++++++++
>  test/CMakeLists.txt                       |  2 +
>  test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt |  8 +++-
>  test/lua-Harness-tests/CMakeLists.txt     | 16 +++++---
>  test/tarantool-tests/CMakeLists.txt       | 28 ++++++++------
>  5 files changed, 81 insertions(+), 19 deletions(-)
>  create mode 100644 cmake/MakeLuaPath.cmake
> 
> diff --git a/cmake/MakeLuaPath.cmake b/cmake/MakeLuaPath.cmake
> new file mode 100644
> index 00000000..9a5a3bb8
> --- /dev/null
> +++ b/cmake/MakeLuaPath.cmake
> @@ -0,0 +1,46 @@
> +# make_lua_path provides a convenient way to define LUA_PATH and
> +# LUA_CPATH variables.
> +#
> +# Example usage:
> +#
> +#   make_lua_path(LUA_PATH
> +#     PATH
> +#       ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
> +#       ${CMAKE_BINARY_DIR}/?.lua
> +#       ./?.lua
> +#   )
> +#
> +# This will give you the string:
> +#    "./?.lua;${CMAKE_BINARY_DIR}/?.lua;${CMAKE_CURRENT_SOURCE_DIR}/?.lua;;"
> +# XXX: Mind the reverse order of the entries in the result string.

Why do we need this behaviour? May be it is better to save strict order
of entries?

> +
> +function(make_lua_path path)
> +  set(prefix ARG)
> +  set(noValues)
> +  set(singleValues)
> +  set(multiValues PATHS)
> +
> +  # FIXME: if we update to CMake >= 3.5, can remove this line.

So, we can just check CMake version as Sergey suggested.

> +  include(CMakeParseArguments)
> +  cmake_parse_arguments(${prefix}
> +                        "${noValues}"
> +                        "${singleValues}"
> +                        "${multiValues}"
> +                        ${ARGN})
> +
> +  # XXX: This is the sentinel semicolon having special meaning

Nit: It's not single semicolon, but two of them. This ";;" special value
is created during concatenation at the next lines.

> +  # for LUA_PATH and LUA_CPATH variables. For more info, see the
> +  # link below:
> +  # https://www.lua.org/manual/5.1/manual.html#pdf-LUA_PATH
> +  set(result "\;")
> +
> +  foreach(inc ${ARG_PATHS})
> +    # XXX: If one joins two strings with semicolon, the value

Typo: s/semicolon/the semicolon/

> +    # automatically becomes a list. I found a single working
> +    # solution to make result variable be a string via "escaping"

Minor: s/be/stay/ or just s/be//
Feel free to ignore.

> +    # the semicolon right in string interpolation.
> +    set(result "${inc}\;${result}")
> +  endforeach()
> +
> +  set(${path} "${result}" PARENT_SCOPE)
> +endfunction()
> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
> index ba25af54..a8262b12 100644
> --- a/test/CMakeLists.txt
> +++ b/test/CMakeLists.txt
> @@ -3,6 +3,8 @@
>  # See the rationale in the root CMakeLists.txt.
>  cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
>  
> +include(MakeLuaPath)
> +

Minor: It would be nice to drop the comment that this will be used in
all test suites by transitivity.
Feel free to ignore.

>  find_program(LUACHECK luacheck)
>  if(LUACHECK)
>    # XXX: The tweak below relates to luacheck problem with paths.
> diff --git a/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt b/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
> index 8e825f55..b95e7852 100644
> --- a/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
> +++ b/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt

<snipped>

> diff --git a/test/lua-Harness-tests/CMakeLists.txt b/test/lua-Harness-tests/CMakeLists.txt
> index 12476171..a57104a5 100644
> --- a/test/lua-Harness-tests/CMakeLists.txt
> +++ b/test/lua-Harness-tests/CMakeLists.txt

<snipped>

> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index ecda2e63..27866869 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -43,14 +43,11 @@ macro(BuildTestCLib lib sources)
>    # semicolon. If one finds the normal way to make it work, feel
>    # free to reach me.
>    set(TESTLIBS "${lib};${TESTLIBS}" PARENT_SCOPE)
> -  # Add the directory where the lib is built to the LUA_CPATH
> -  # environment variable, so LuaJIT can find and load it.
> -  # XXX: Here we see the other side of the coin. If one joins two
> -  # strings with semicolon, the value automatically becomes a
> -  # list. I found a single working solution to make LUA_CPATH be
> -  # a string via "escaping" the semicolon right in string
> -  # interpolation.
> -  set(LUA_CPATH "${CMAKE_CURRENT_BINARY_DIR}/?${CMAKE_SHARED_LIBRARY_SUFFIX}\;${LUA_CPATH}" PARENT_SCOPE)
> +  # Add the directory where the lib is built to the list with
> +  # entries for LUA_CPATH environment variable, so LuaJIT can find
> +  # and load it. See the comment about extending the list in the
> +  # parent scope few lines above.
> +  set(LUA_CPATHS "${CMAKE_CURRENT_BINARY_DIR}/?${CMAKE_SHARED_LIBRARY_SUFFIX};${LUA_CPATHS}" PARENT_SCOPE)
>    # Also add this directory to LD_LIBRARY_PATH environment
>    # variable, so FFI machinery can find and load it.
>    set(LD_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}:${LD_LIBRARY_PATH}" PARENT_SCOPE)
> @@ -76,14 +73,21 @@ add_subdirectory(misclib-sysprof-capi)
>  # in src/ directory and auxiliary tests-related modules are
>  # located in the current directory (but tests are run in the
>  # binary directory), so LUA_PATH need to be updated.
> -set(LUA_PATH
> -  "${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${PROJECT_SOURCE_DIR}/tools/?.lua\;${PROJECT_SOURCE_DIR}/src/?.lua"
> +make_lua_path(LUA_PATH
> +  PATHS
> +    ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
> +    ${PROJECT_SOURCE_DIR}/tools/?.lua
> +    ${PROJECT_SOURCE_DIR}/src/?.lua
>  )
> +# Update LUA_CPATH with the library paths collected within
> +# <BuildTestLib> macro.
> +make_lua_path(LUA_CPATH PATHS ${LUA_CPATHS})

Side note: Looks like this part of patching is unnecessary -- we can
just leave it as is (isntead manipulation with ; later in
`make_lua_path()`). But it is good for consistency of working with env
variables, so feel free to ignore.

> +
>  set(LUA_TEST_SUFFIX .test.lua)
>  set(LUA_TEST_FLAGS --failures --shuffle)
>  set(LUA_TEST_ENV
> -  "LUA_PATH=\"${LUA_PATH}\;\;\""
> -  "LUA_CPATH=\"${LUA_CPATH}\;\;\""
> +  "LUA_PATH=\"${LUA_PATH}\""
> +  "LUA_CPATH=\"${LUA_CPATH}\""
>  )
>  
>  if(CMAKE_VERBOSE_MAKEFILE)
> -- 
> 2.34.0
> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 3/8] test: fix tarantool suite for out of source build
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 3/8] test: fix tarantool suite for out of source build Igor Munkin via Tarantool-patches
  2022-08-15 12:10   ` Sergey Bronnikov via Tarantool-patches
@ 2022-08-18  9:49   ` Sergey Kaplun via Tarantool-patches
  2022-08-31 17:20     ` Igor Munkin via Tarantool-patches
  1 sibling, 1 reply; 39+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-08-18  9:49 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Hi, Igor!

Thanks for the patch!
This is really nice to fix this pebble in the shoe!
LGTM, except a few typos in comment and the commit message.

On 11.08.22, Igor Munkin wrote:
> jit/vmdef.lua is autogenerated file, so it's put to src/ directory
> located in scope of the binary artefacts tree. Before the patch LUA_PATH
> lacks this path, so tarantool-tests target fails due to jit/vmdef.lua

Side note: IINM tarantool-tests are affected due to `jit.bc` usage in
the test suite.

> misseek. As a result of this change src/ directory in scope of the

Typo: s/in scope/in the scope/

> binary tree is included to LUA_PATH as well as the one from the source
> tree has been.
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  test/tarantool-tests/CMakeLists.txt | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 27866869..97c23670 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -70,14 +70,17 @@ add_subdirectory(misclib-sysprof-capi)
>  
>  # The part of the memory profiler toolchain is located in tools
>  # directory, jit, profiler, and bytecode toolchains are located
> -# in src/ directory and auxiliary tests-related modules are
> -# located in the current directory (but tests are run in the
> -# binary directory), so LUA_PATH need to be updated.
> +# in src/ directory, jit/vmdef.lua is autogenerated file also
> +# located in src/ directory, but in scope of the binary artefacts

Typo: s/in scope/in the scope/

> +# tree and auxiliary tests-related modules are located in the
> +# current directory (but tests are run in the binary directory),
> +# so LUA_PATH need to be updated.
>  make_lua_path(LUA_PATH
>    PATHS
>      ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
>      ${PROJECT_SOURCE_DIR}/tools/?.lua
>      ${PROJECT_SOURCE_DIR}/src/?.lua
> +    ${PROJECT_BINARY_DIR}/src/?.lua
>  )
>  # Update LUA_CPATH with the library paths collected within
>  # <BuildTestLib> macro.
> -- 
> 2.34.0
> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 4/8] ci: use out of source build in GitHub Actions
  2022-08-15 12:13   ` Sergey Bronnikov via Tarantool-patches
@ 2022-08-18  9:58     ` Sergey Kaplun via Tarantool-patches
  2022-08-31 15:34       ` Igor Munkin via Tarantool-patches
  2022-08-31 15:33     ` Igor Munkin via Tarantool-patches
  1 sibling, 1 reply; 39+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-08-18  9:58 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Hi, Igor!

Thanks for the patch!

LGTM in general, but have the same question as Sergey.

On 15.08.22, Sergey Bronnikov wrote:
> Igor, please see my comment below.
> 
> On 11.08.2022 14:17, Igor Munkin wrote:
> > Use out of source build configuration for LuaJIT testing jobs in all
> > GitHub workflows. For this build type configuration unique subdirectory
> > (using GitHub run ID) within runner temporary directory is used as a
> > binary artefacts tree.
> >
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> >   .github/actions/environment/action.yml   | 5 +++++
> >   .github/workflows/lint.yml               | 3 ++-
> >   .github/workflows/linux-aarch64.yml      | 6 +++++-
> >   .github/workflows/linux-x86_64-ninja.yml | 6 +++++-
> >   .github/workflows/linux-x86_64.yml       | 7 ++++++-
> >   .github/workflows/macos-m1.yml           | 6 +++++-
> >   .github/workflows/macos-x86_64.yml       | 7 ++++++-
> >   7 files changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/.github/actions/environment/action.yml b/.github/actions/environment/action.yml
> > index 43323bc7..7fb2625f 100644
> > --- a/.github/actions/environment/action.yml
> > +++ b/.github/actions/environment/action.yml

<snipped>

> > diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
> > index 8c8dcff1..21d86764 100644
> > --- a/.github/workflows/linux-aarch64.yml
> > +++ b/.github/workflows/linux-aarch64.yml
> > @@ -53,11 +53,15 @@ jobs:
> >             sudo apt -y update
> >             sudo apt -y install cmake gcc make perl
> >         - name: configure
> > -        run: cmake . ${{ matrix.CMAKEFLAGS }}
> > +        run: >
> > +          cmake -S . -B ${{ env.BUILDDIR }}
> > +          ${{ matrix.CMAKEFLAGS }}
> >         - name: build
> >           run: cmake --build . --parallel
> > +        working-directory: ${{ env.BUILDDIR }}
> >         - name: test
> >           run: cmake --build . --parallel --target test
> > +        working-directory: ${{ env.BUILDDIR }}
> 
> 1. I don't get an idea to use current dir for CMake and specify a 
> working-directory in a job step.
> 
> Why not "cmake --build ${{ env.BUILDDIR }}" as above?

I don't get the idea too :).

> 
> >   
> >     test-tarantool-debug-w-GC64:
> >       name: Tarantool Debug GC64:ON
> > diff --git a/.github/workflows/linux-x86_64-ninja.yml b/.github/workflows/linux-x86_64-ninja.yml
> > index 2877d2f6..72d56d54 100644
> > --- a/.github/workflows/linux-x86_64-ninja.yml
> > +++ b/.github/workflows/linux-x86_64-ninja.yml

<snipped>

> >       name: Tarantool Debug GC64:OFF

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 5/8] ci: remove excess parallel level setup
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 5/8] ci: remove excess parallel level setup Igor Munkin via Tarantool-patches
  2022-08-15 12:14   ` Sergey Bronnikov via Tarantool-patches
@ 2022-08-18 10:09   ` Sergey Kaplun via Tarantool-patches
  1 sibling, 0 replies; 39+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-08-18 10:09 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Hi, Igor!

Thanks for the patch!

LGTM!

On 11.08.22, Igor Munkin wrote:
> This patch removes the artefact line left as a result of squashing
> several fixup commits into 36b4b80cbdcf55f5ffdbcea0097c4dc315638c50
> ("ci: introduce GitHub action for environment setup").
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  .github/workflows/macos-m1.yml | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
> index e3b6dcda..d5b0e0f1 100644
> --- a/.github/workflows/macos-m1.yml
> +++ b/.github/workflows/macos-m1.yml
> @@ -66,7 +66,6 @@ jobs:
>            # if the package already exists with the previous version.
>            ${ARCH} brew install --force cmake gcc make perl ||
>              ${ARCH} brew upgrade cmake gcc make perl
> -          ${ARCH} echo "CMAKE_BUILD_PARALLEL_LEVEL=$(($(sysctl -n hw.logicalcpu) + 1))" >> $GITHUB_ENV
>        - name: configure
>          run: >
>            ${ARCH} cmake -S . -B ${{ env.BUILDDIR }}
> -- 
> 2.34.0
> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 6/8] ci: remove arch prefix for macOS M1 workflow
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 6/8] ci: remove arch prefix for macOS M1 workflow Igor Munkin via Tarantool-patches
  2022-08-15 12:17   ` Sergey Bronnikov via Tarantool-patches
@ 2022-08-18 10:14   ` Sergey Kaplun via Tarantool-patches
  2022-08-31 15:55     ` Igor Munkin via Tarantool-patches
  1 sibling, 1 reply; 39+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-08-18 10:14 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Hi, Igor!

Thanks for the patch!

LGTM, except a single nit regarding the commit message.

On 11.08.22, Igor Munkin wrote:
> Finally self-hosted GitHub Actions runners support Apple M1 hardware. As

Side note: Is there any link or an announcement of it?

> a result there is no need to explicitly add `arch -arm64` prefix for all

Typo: s/As a result/As a result,/

> commands being run in scope of macOS M1 workflows.
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  .github/workflows/macos-m1.yml | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
> index d5b0e0f1..4e1275f7 100644
> --- a/.github/workflows/macos-m1.yml
> +++ b/.github/workflows/macos-m1.yml

<snipped>

> -- 
> 2.34.0
> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 7/8] ci: merge x86_64 and ARM64 workflows
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 7/8] ci: merge x86_64 and ARM64 workflows Igor Munkin via Tarantool-patches
  2022-08-15 12:22   ` Sergey Bronnikov via Tarantool-patches
@ 2022-08-18 10:21   ` Sergey Kaplun via Tarantool-patches
  2022-08-31 16:02     ` Igor Munkin via Tarantool-patches
  1 sibling, 1 reply; 39+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-08-18 10:21 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Hi, Igor!

Thanks for the patch!
LGTM!

On 11.08.22, Igor Munkin wrote:
> As a result of the previous commit, there is no difference left between
> x86_64 and ARM64 workflows except the runner where workflow is run.
> Hence there is no reason to have a separate workflow file for each
> hardware architecture.
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  .github/workflows/linux-aarch64.yml           | 83 -----------------
>  .../workflows/{linux-x86_64.yml => linux.yml} | 52 ++++++++---
>  .github/workflows/macos-m1.yml                | 92 -------------------
>  .../workflows/{macos-x86_64.yml => macos.yml} | 52 ++++++++---
>  4 files changed, 82 insertions(+), 197 deletions(-)
>  delete mode 100644 .github/workflows/linux-aarch64.yml
>  rename .github/workflows/{linux-x86_64.yml => linux.yml} (69%)
>  delete mode 100644 .github/workflows/macos-m1.yml
>  rename .github/workflows/{macos-x86_64.yml => macos.yml} (73%)
> 
> diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
> deleted file mode 100644
> index 21d86764..00000000
> --- a/.github/workflows/linux-aarch64.yml
> +++ /dev/null
> @@ -1,83 +0,0 @@

<snipped>

> diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux.yml
> similarity index 69%
> rename from .github/workflows/linux-x86_64.yml
> rename to .github/workflows/linux.yml
> index 4c3ad4c7..125c8708 100644
> --- a/.github/workflows/linux-x86_64.yml
> +++ b/.github/workflows/linux.yml

<snipped>

> @@ -65,8 +77,8 @@ jobs:
>          run: cmake --build . --parallel --target test
>          working-directory: ${{ env.BUILDDIR }}
>  
> -  test-tarantool-debug-wo-GC64:
> -    name: Tarantool Debug GC64:OFF
> +  test-tarantool-x86_64-debug-wo-GC64:

Side note: Can we use the same matrix approach for integration
workflows?

> +    name: Tarantool x86_64 Debug GC64:OFF
>      needs: test-luajit
>      uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>      with:
> @@ -74,8 +86,8 @@ jobs:
>        buildtype: Debug
>        host: ubuntu-20.04-self-hosted
>        revision: ${{ github.sha }}
> -  test-tarantool-debug-w-GC64:
> -    name: Tarantool Debug GC64:ON
> +  test-tarantool-x86_64-debug-w-GC64:
> +    name: Tarantool x86_64 Debug GC64:ON
>      needs: test-luajit
>      uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>      with:
> @@ -83,8 +95,8 @@ jobs:
>        buildtype: Debug
>        host: ubuntu-20.04-self-hosted
>        revision: ${{ github.sha }}
> -  test-tarantool-release-wo-GC64:
> -    name: Tarantool Release GC64:OFF
> +  test-tarantool-x86_64-release-wo-GC64:
> +    name: Tarantool x86_64 Release GC64:OFF
>      needs: test-luajit
>      uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>      with:
> @@ -92,8 +104,8 @@ jobs:
>        buildtype: RelWithDebInfo
>        host: ubuntu-20.04-self-hosted
>        revision: ${{ github.sha }}
> -  test-tarantool-release-w-GC64:
> -    name: Tarantool Release GC64:ON
> +  test-tarantool-x86_64-release-w-GC64:
> +    name: Tarantool x86_64 Release GC64:ON
>      needs: test-luajit
>      uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
>      with:
> @@ -101,3 +113,21 @@ jobs:

<snipped>

> diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
> deleted file mode 100644
> index 4e1275f7..00000000
> --- a/.github/workflows/macos-m1.yml
> +++ /dev/null
> @@ -1,92 +0,0 @@

<snipped>

> -- 
> 2.34.0
> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 8/8] ci: merge Linux and macOS workflows
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 8/8] ci: merge Linux and macOS workflows Igor Munkin via Tarantool-patches
  2022-08-15 12:27   ` Sergey Bronnikov via Tarantool-patches
@ 2022-08-18 10:32   ` Sergey Kaplun via Tarantool-patches
  1 sibling, 0 replies; 39+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-08-18 10:32 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Hi, Igor!

Thanks for the patch!
LGTM!

On 11.08.22, Igor Munkin wrote:
> As a result of two previous commits, there is no difference left between
> Linux and macOS workflows except the environment setup (environment
> variables, dependencies installation). Hence there is no reason to have a
> separate workflow file for each OS type.
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  .github/actions/environment/action.yml        |  18 ---
>  .github/actions/setup-linux/README.md         |  12 ++
>  .github/actions/setup-linux/action.yml        |  19 +++
>  .github/actions/setup-macos/README.md         |  12 ++
>  .github/actions/setup-macos/action.yml        |  29 ++++
>  .../actions/{environment => setup}/README.md  |   5 +-
>  .github/actions/setup/action.yml              |  10 ++
>  .github/workflows/lint.yml                    |   8 +-
>  .github/workflows/linux-x86_64-ninja.yml      |   6 +-
>  .github/workflows/macos.yml                   | 142 ------------------
>  .github/workflows/{linux.yml => testing.yml}  | 113 +++++++++++---
>  11 files changed, 186 insertions(+), 188 deletions(-)
>  delete mode 100644 .github/actions/environment/action.yml
>  create mode 100644 .github/actions/setup-linux/README.md
>  create mode 100644 .github/actions/setup-linux/action.yml
>  create mode 100644 .github/actions/setup-macos/README.md
>  create mode 100644 .github/actions/setup-macos/action.yml
>  rename .github/actions/{environment => setup}/README.md (72%)
>  create mode 100644 .github/actions/setup/action.yml
>  delete mode 100644 .github/workflows/macos.yml
>  rename .github/workflows/{linux.yml => testing.yml} (52%)
> 

<snipped>

> -- 
> 2.34.0
> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 1/8] test: introduce LUAJIT_TEST_VARDIR variable
  2022-08-15 12:08   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18  8:27     ` Sergey Kaplun via Tarantool-patches
@ 2022-08-31 14:53     ` Igor Munkin via Tarantool-patches
  2022-09-02 12:06       ` Sergey Bronnikov via Tarantool-patches
  1 sibling, 1 reply; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-31 14:53 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Sergey,

Thanks for your review!

On 15.08.22, Sergey Bronnikov wrote:
> Igor,
> 
>   thanks for the patch. See my comments inline.
> 
> On 11.08.2022 14:17, Igor Munkin wrote:
> > Before the patch both memprof and sysprof artefacts are generated within
> > the binary artefacts tree (i.e. in the same directory LuaJIT binary is
> > generated). However, more convenient way is producing these temporary
> > profiles in a separate directory (e.g. located on the partition with
> > more strict space limits). As a result of the patch all memprof and
> > sysprof test chunks consider LUAJIT_TEST_VARDIR environment variable to
> > set the directory where test profiles are generated. For the case when
> > LUAJIT_TEST_VARDIR is not set, everything works as before.
> 
> Commit message is inconsistent a bit with a patch itself.
> 
> As far as I understand many hunks are not related to introducing 
> LUAJIT_TEST_VARDIR.
> 
> Probably it is better to change commit one-line message from "test: 
> introduce LUAJIT_TEST_VARDIR variable"
> 
> to something like "test: refactoring and introduce LUAJIT_TEST_VARDIR 
> variable". It allows to keep patch
> 
> as is and change expectations for those who will look at your patch.

I believe your commit subject is too long, so I propose another
solution. Since the only thing affected by LUAJIT_TEST_VARDIR is
<utils.profilename>, so I can write something like "test: introduce
utils.profilename helper" and describe its rationale and functions
within the commit message. Are you OK with it?

> 
> 
> >
> > Part of tarantool/tarantool#7472
> >
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> >   .../gh-5813-resolving-of-c-symbols.test.lua   |  6 +++--
> >   .../misclib-memprof-lapi.test.lua             | 22 ++++++++++---------
> >   .../misclib-sysprof-lapi.test.lua             |  8 ++++---
> >   test/tarantool-tests/utils.lua                | 12 ++++++++++
> >   4 files changed, 33 insertions(+), 15 deletions(-)
> >

<snipped>

> > @@ -189,9 +191,9 @@ test:test("output", function(subtest)
> >     -- one is the number of allocations. 1 event - alocation 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 = 35, linedefined = 33 }, 2))
> > +  subtest:ok(check_alloc_report(alloc, { line = 37, linedefined = 35 }, 2))
> >     -- 20 strings allocations.
> > -  subtest:ok(check_alloc_report(alloc, { line = 40, linedefined = 33 }, 20))
> > +  subtest:ok(check_alloc_report(alloc, { line = 42, linedefined = 35 }, 20))
> What are these magic numbers mean and why we should change them for 
> introducing LUAJIT_TEST_VARDIR?

These are artefacts from not such elegant memprof tests...

> >   
> >     -- Collect all previous allocated objects.
> >     subtest:ok(free.INTERNAL.num == 22)

<snipped>

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 2/8] test: introduce MakeLuaPath.cmake helper
  2022-08-15 12:08   ` Sergey Bronnikov via Tarantool-patches
@ 2022-08-31 15:07     ` Igor Munkin via Tarantool-patches
  2022-09-02 12:09       ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 1 reply; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-31 15:07 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Sergey,

Thanks for you review!

On 15.08.22, Sergey Bronnikov wrote:
> Igor, thanks for the patch! See my comments inline.
> 
> On 11.08.2022 14:17, Igor Munkin wrote:
> > While extending test suites it is often required to append additional
> > path where Lua or Lua-C auxiliary modules are located to LUA_PATH or
> > LUA_CPATH environment variables. Due to insane semicolon interpolation
> > in CMake strings (that converts such string to a list as a result), we
> > need to escape semicolon in LUA_PATH/LUA_CPATH strings while building
> > the resulting value.
> >
> > After the years of struggling MakeLuaPath.cmake module is introduced to
> > make LUA_PATH and LUA_CPATH definition convenient with <make_lua_path>
> > helper. This function takes all paths given as a variable list argument,
> > joins them in a reverse order by a semicolon and yields the resulting
> > string to a specified CMake variable.
> >
> > Signed-off-by: Igor Munkin<imun@tarantool.org>
> > ---
> >   cmake/MakeLuaPath.cmake                   | 46 +++++++++++++++++++++++
> >   test/CMakeLists.txt                       |  2 +
> >   test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt |  8 +++-
> >   test/lua-Harness-tests/CMakeLists.txt     | 16 +++++---
> >   test/tarantool-tests/CMakeLists.txt       | 28 ++++++++------
> >   5 files changed, 81 insertions(+), 19 deletions(-)
> >   create mode 100644 cmake/MakeLuaPath.cmake
> >
> > diff --git a/cmake/MakeLuaPath.cmake b/cmake/MakeLuaPath.cmake
> > new file mode 100644
> > index 00000000..9a5a3bb8
> > --- /dev/null
> > +++ b/cmake/MakeLuaPath.cmake

<snipped>

> > +
> > +  # FIXME: if we update to CMake >= 3.5, can remove this line.
> 
> I would replace comment with condition that will check CMake version
> 
> and will fail when CMake >= 3.5:
> 
> |if (CMAKE_VERSION VERSION_GREATER_EQUAL 35) |
> 
>     message(FATAL_ERROR "Please remove snippet for CMake < 3.5")
> 
> endif()

I doubt this is a working solution. The comment means, that there is no
need to include a separate module if CMake to be used is greater than
3.5. With your code, fatal error is raised in that case. I have CMake
3.20 on my local machine and LuaJIT fails to be built with your check
added (I've fixed the typo in the version number).

Ignoring for now.

> 
> 
> > +  include(CMakeParseArguments)

<snipped>

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 2/8] test: introduce MakeLuaPath.cmake helper
  2022-08-18  9:37   ` Sergey Kaplun via Tarantool-patches
@ 2022-08-31 15:19     ` Igor Munkin via Tarantool-patches
  2022-09-01 10:16       ` Sergey Kaplun via Tarantool-patches
  0 siblings, 1 reply; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-31 15:19 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

Thanks for your review!

On 18.08.22, Sergey Kaplun wrote:
> Hi, Igor!
> 
> Thanks for the patch!
> 
> Please consider my comments below.
> 
> On 11.08.22, Igor Munkin wrote:
> > While extending test suites it is often required to append additional
> > path where Lua or Lua-C auxiliary modules are located to LUA_PATH or
> > LUA_CPATH environment variables. Due to insane semicolon interpolation
> > in CMake strings (that converts such string to a list as a result), we
> > need to escape semicolon in LUA_PATH/LUA_CPATH strings while building
> > the resulting value.
> > 
> > After the years of struggling MakeLuaPath.cmake module is introduced to
> 
> Minor: s/After/Over/

No it's not. I mean that as a result of the years of struggling the
module is introduced. It was not being introduced over the years of
struggling.

> 
> > make LUA_PATH and LUA_CPATH definition convenient with <make_lua_path>
> > helper. This function takes all paths given as a variable list argument,
> > joins them in a reverse order by a semicolon and yields the resulting
> > string to a specified CMake variable.
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> >  cmake/MakeLuaPath.cmake                   | 46 +++++++++++++++++++++++
> >  test/CMakeLists.txt                       |  2 +
> >  test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt |  8 +++-
> >  test/lua-Harness-tests/CMakeLists.txt     | 16 +++++---
> >  test/tarantool-tests/CMakeLists.txt       | 28 ++++++++------
> >  5 files changed, 81 insertions(+), 19 deletions(-)
> >  create mode 100644 cmake/MakeLuaPath.cmake
> > 
> > diff --git a/cmake/MakeLuaPath.cmake b/cmake/MakeLuaPath.cmake
> > new file mode 100644
> > index 00000000..9a5a3bb8
> > --- /dev/null
> > +++ b/cmake/MakeLuaPath.cmake
> > @@ -0,0 +1,46 @@
> > +# make_lua_path provides a convenient way to define LUA_PATH and
> > +# LUA_CPATH variables.
> > +#
> > +# Example usage:
> > +#
> > +#   make_lua_path(LUA_PATH
> > +#     PATH
> > +#       ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
> > +#       ${CMAKE_BINARY_DIR}/?.lua
> > +#       ./?.lua
> > +#   )
> > +#
> > +# This will give you the string:
> > +#    "./?.lua;${CMAKE_BINARY_DIR}/?.lua;${CMAKE_CURRENT_SOURCE_DIR}/?.lua;;"
> > +# XXX: Mind the reverse order of the entries in the result string.
> 
> Why do we need this behaviour? May be it is better to save strict order
> of entries?

The code will become more complex as a result and in the 99.999% of the
cases the order of LUA_PATH entries doesn't bother you. So I don't see
we should make the code more complex to save the order of the entries.

> 
> > +
> > +function(make_lua_path path)
> > +  set(prefix ARG)
> > +  set(noValues)
> > +  set(singleValues)
> > +  set(multiValues PATHS)
> > +
> > +  # FIXME: if we update to CMake >= 3.5, can remove this line.
> 
> So, we can just check CMake version as Sergey suggested.

No, we can't (see the comment in the Sergey's thread).

> 
> > +  include(CMakeParseArguments)
> > +  cmake_parse_arguments(${prefix}
> > +                        "${noValues}"
> > +                        "${singleValues}"
> > +                        "${multiValues}"
> > +                        ${ARGN})
> > +
> > +  # XXX: This is the sentinel semicolon having special meaning
> 
> Nit: It's not single semicolon, but two of them. This ";;" special value
> is created during concatenation at the next lines.

Yes, it is. But strictly saying only the second semicolon has a special
meaning, the first one is a separator; otherwise it should be three of
them :)

> 
> > +  # for LUA_PATH and LUA_CPATH variables. For more info, see the
> > +  # link below:
> > +  # https://www.lua.org/manual/5.1/manual.html#pdf-LUA_PATH
> > +  set(result "\;")
> > +
> > +  foreach(inc ${ARG_PATHS})
> > +    # XXX: If one joins two strings with semicolon, the value
> 
> Typo: s/semicolon/the semicolon/

Fixed.

> 
> > +    # automatically becomes a list. I found a single working
> > +    # solution to make result variable be a string via "escaping"
> 
> Minor: s/be/stay/ or just s/be//
> Feel free to ignore.

I don't get why. Ignoring.

> 
> > +    # the semicolon right in string interpolation.
> > +    set(result "${inc}\;${result}")
> > +  endforeach()
> > +
> > +  set(${path} "${result}" PARENT_SCOPE)
> > +endfunction()
> > diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
> > index ba25af54..a8262b12 100644
> > --- a/test/CMakeLists.txt
> > +++ b/test/CMakeLists.txt
> > @@ -3,6 +3,8 @@
> >  # See the rationale in the root CMakeLists.txt.
> >  cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
> >  
> > +include(MakeLuaPath)
> > +
> 
> Minor: It would be nice to drop the comment that this will be used in
> all test suites by transitivity.
> Feel free to ignore.

There are some includes made in the root CMakeLists.txt that are used
project-wide. There are no comments nearby. Ignoring for this case too.

> 
> >  find_program(LUACHECK luacheck)
> >  if(LUACHECK)
> >    # XXX: The tweak below relates to luacheck problem with paths.

<snipped>

> > diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> > index ecda2e63..27866869 100644
> > --- a/test/tarantool-tests/CMakeLists.txt
> > +++ b/test/tarantool-tests/CMakeLists.txt
> > @@ -43,14 +43,11 @@ macro(BuildTestCLib lib sources)
> >    # semicolon. If one finds the normal way to make it work, feel
> >    # free to reach me.
> >    set(TESTLIBS "${lib};${TESTLIBS}" PARENT_SCOPE)
> > -  # Add the directory where the lib is built to the LUA_CPATH
> > -  # environment variable, so LuaJIT can find and load it.
> > -  # XXX: Here we see the other side of the coin. If one joins two
> > -  # strings with semicolon, the value automatically becomes a
> > -  # list. I found a single working solution to make LUA_CPATH be
> > -  # a string via "escaping" the semicolon right in string
> > -  # interpolation.
> > -  set(LUA_CPATH "${CMAKE_CURRENT_BINARY_DIR}/?${CMAKE_SHARED_LIBRARY_SUFFIX}\;${LUA_CPATH}" PARENT_SCOPE)
> > +  # Add the directory where the lib is built to the list with
> > +  # entries for LUA_CPATH environment variable, so LuaJIT can find
> > +  # and load it. See the comment about extending the list in the
> > +  # parent scope few lines above.
> > +  set(LUA_CPATHS "${CMAKE_CURRENT_BINARY_DIR}/?${CMAKE_SHARED_LIBRARY_SUFFIX};${LUA_CPATHS}" PARENT_SCOPE)
> >    # Also add this directory to LD_LIBRARY_PATH environment
> >    # variable, so FFI machinery can find and load it.
> >    set(LD_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}:${LD_LIBRARY_PATH}" PARENT_SCOPE)
> > @@ -76,14 +73,21 @@ add_subdirectory(misclib-sysprof-capi)
> >  # in src/ directory and auxiliary tests-related modules are
> >  # located in the current directory (but tests are run in the
> >  # binary directory), so LUA_PATH need to be updated.
> > -set(LUA_PATH
> > -  "${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${PROJECT_SOURCE_DIR}/tools/?.lua\;${PROJECT_SOURCE_DIR}/src/?.lua"
> > +make_lua_path(LUA_PATH
> > +  PATHS
> > +    ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
> > +    ${PROJECT_SOURCE_DIR}/tools/?.lua
> > +    ${PROJECT_SOURCE_DIR}/src/?.lua
> >  )
> > +# Update LUA_CPATH with the library paths collected within
> > +# <BuildTestLib> macro.
> > +make_lua_path(LUA_CPATH PATHS ${LUA_CPATHS})
> 
> Side note: Looks like this part of patching is unnecessary -- we can
> just leave it as is (isntead manipulation with ; later in
> `make_lua_path()`). But it is good for consistency of working with env
> variables, so feel free to ignore.

Yes, I did it for consistency of working with env variables (otherwise
there would be a mess with semicolons everywhere). Ignoring.

> 
> > +
> >  set(LUA_TEST_SUFFIX .test.lua)
> >  set(LUA_TEST_FLAGS --failures --shuffle)
> >  set(LUA_TEST_ENV
> > -  "LUA_PATH=\"${LUA_PATH}\;\;\""
> > -  "LUA_CPATH=\"${LUA_CPATH}\;\;\""
> > +  "LUA_PATH=\"${LUA_PATH}\""
> > +  "LUA_CPATH=\"${LUA_CPATH}\""
> >  )
> >  
> >  if(CMAKE_VERBOSE_MAKEFILE)
> > -- 
> > 2.34.0
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 4/8] ci: use out of source build in GitHub Actions
  2022-08-15 12:13   ` Sergey Bronnikov via Tarantool-patches
  2022-08-18  9:58     ` Sergey Kaplun via Tarantool-patches
@ 2022-08-31 15:33     ` Igor Munkin via Tarantool-patches
  2022-09-02 12:09       ` Sergey Bronnikov via Tarantool-patches
  1 sibling, 1 reply; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-31 15:33 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Sergey,

Thanks for your review!

On 15.08.22, Sergey Bronnikov wrote:
> Igor, please see my comment below.
> 
> On 11.08.2022 14:17, Igor Munkin wrote:
> > Use out of source build configuration for LuaJIT testing jobs in all
> > GitHub workflows. For this build type configuration unique subdirectory
> > (using GitHub run ID) within runner temporary directory is used as a
> > binary artefacts tree.
> >
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> >   .github/actions/environment/action.yml   | 5 +++++
> >   .github/workflows/lint.yml               | 3 ++-
> >   .github/workflows/linux-aarch64.yml      | 6 +++++-
> >   .github/workflows/linux-x86_64-ninja.yml | 6 +++++-
> >   .github/workflows/linux-x86_64.yml       | 7 ++++++-
> >   .github/workflows/macos-m1.yml           | 6 +++++-
> >   .github/workflows/macos-x86_64.yml       | 7 ++++++-
> >   7 files changed, 34 insertions(+), 6 deletions(-)
> >

<snipped>

> > diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
> > index 8c8dcff1..21d86764 100644
> > --- a/.github/workflows/linux-aarch64.yml
> > +++ b/.github/workflows/linux-aarch64.yml
> > @@ -53,11 +53,15 @@ jobs:
> >             sudo apt -y update
> >             sudo apt -y install cmake gcc make perl
> >         - name: configure
> > -        run: cmake . ${{ matrix.CMAKEFLAGS }}
> > +        run: >
> > +          cmake -S . -B ${{ env.BUILDDIR }}
> > +          ${{ matrix.CMAKEFLAGS }}
> >         - name: build
> >           run: cmake --build . --parallel
> > +        working-directory: ${{ env.BUILDDIR }}
> >         - name: test
> >           run: cmake --build . --parallel --target test
> > +        working-directory: ${{ env.BUILDDIR }}
> 
> 1. I don't get an idea to use current dir for CMake and specify a 
> working-directory in a job step.
> 
> Why not "cmake --build ${{ env.BUILDDIR }}" as above?

I tried to not touch the original command. Configuration step can't be
left intact, but build and test -- can and that makes diff more clear
(IMHO). Furhermore, it looks more explicit to me: the command is run in
the current directory, and the current directory is specified
separately from the command itself.

> 
> >   
> >     test-tarantool-debug-w-GC64:
> >       name: Tarantool Debug GC64:ON

<snipped>

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 4/8] ci: use out of source build in GitHub Actions
  2022-08-18  9:58     ` Sergey Kaplun via Tarantool-patches
@ 2022-08-31 15:34       ` Igor Munkin via Tarantool-patches
  0 siblings, 0 replies; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-31 15:34 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

Thanks for your review!

On 18.08.22, Sergey Kaplun wrote:
> Hi, Igor!
> 
> Thanks for the patch!
> 
> LGTM in general, but have the same question as Sergey.
> 
> On 15.08.22, Sergey Bronnikov wrote:
> > Igor, please see my comment below.
> > 
> > On 11.08.2022 14:17, Igor Munkin wrote:
> > > Use out of source build configuration for LuaJIT testing jobs in all
> > > GitHub workflows. For this build type configuration unique subdirectory
> > > (using GitHub run ID) within runner temporary directory is used as a
> > > binary artefacts tree.
> > >
> > > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > > ---
> > >   .github/actions/environment/action.yml   | 5 +++++
> > >   .github/workflows/lint.yml               | 3 ++-
> > >   .github/workflows/linux-aarch64.yml      | 6 +++++-
> > >   .github/workflows/linux-x86_64-ninja.yml | 6 +++++-
> > >   .github/workflows/linux-x86_64.yml       | 7 ++++++-
> > >   .github/workflows/macos-m1.yml           | 6 +++++-
> > >   .github/workflows/macos-x86_64.yml       | 7 ++++++-
> > >   7 files changed, 34 insertions(+), 6 deletions(-)
> > >

<snipped>

> > > diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
> > > index 8c8dcff1..21d86764 100644
> > > --- a/.github/workflows/linux-aarch64.yml
> > > +++ b/.github/workflows/linux-aarch64.yml
> > > @@ -53,11 +53,15 @@ jobs:
> > >             sudo apt -y update
> > >             sudo apt -y install cmake gcc make perl
> > >         - name: configure
> > > -        run: cmake . ${{ matrix.CMAKEFLAGS }}
> > > +        run: >
> > > +          cmake -S . -B ${{ env.BUILDDIR }}
> > > +          ${{ matrix.CMAKEFLAGS }}
> > >         - name: build
> > >           run: cmake --build . --parallel
> > > +        working-directory: ${{ env.BUILDDIR }}
> > >         - name: test
> > >           run: cmake --build . --parallel --target test
> > > +        working-directory: ${{ env.BUILDDIR }}
> > 
> > 1. I don't get an idea to use current dir for CMake and specify a 
> > working-directory in a job step.
> > 
> > Why not "cmake --build ${{ env.BUILDDIR }}" as above?
> 
> I don't get the idea too :).

I've answered to Sergey's thread.

> 

<snipped>

> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 6/8] ci: remove arch prefix for macOS M1 workflow
  2022-08-18 10:14   ` Sergey Kaplun via Tarantool-patches
@ 2022-08-31 15:55     ` Igor Munkin via Tarantool-patches
  0 siblings, 0 replies; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-31 15:55 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

Thanks for your review! I've fixed all your comments, see the new commit
message at the bottom.

On 18.08.22, Sergey Kaplun wrote:
> Hi, Igor!
> 
> Thanks for the patch!
> 
> LGTM, except a single nit regarding the commit message.
> 
> On 11.08.22, Igor Munkin wrote:
> > Finally self-hosted GitHub Actions runners support Apple M1 hardware. As
> 
> Side note: Is there any link or an announcement of it?

Added.

> 
> > a result there is no need to explicitly add `arch -arm64` prefix for all
> 
> Typo: s/As a result/As a result,/

Fixed.

> 
> > commands being run in scope of macOS M1 workflows.
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> >  .github/workflows/macos-m1.yml | 19 +++++++------------
> >  1 file changed, 7 insertions(+), 12 deletions(-)
> > 
> > diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
> > index d5b0e0f1..4e1275f7 100644
> > --- a/.github/workflows/macos-m1.yml
> > +++ b/.github/workflows/macos-m1.yml
> 
> <snipped>
> 
> > -- 
> > 2.34.0
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

| ci: remove arch prefix for macOS M1 workflow
|
| Finally self-hosted GitHub Actions runners support Apple M1 hardware[1].
| As a result, there is no need to explicitly add `arch -arm64` prefix for
| all commands being run in scope of macOS M1 workflows.
|
| [1]: https://github.blog/changelog/2022-08-09-github-actions-self-hosted-runners-now-support-apple-m1-hardware/

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 7/8] ci: merge x86_64 and ARM64 workflows
  2022-08-18 10:21   ` Sergey Kaplun via Tarantool-patches
@ 2022-08-31 16:02     ` Igor Munkin via Tarantool-patches
  0 siblings, 0 replies; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-31 16:02 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

Thanks for your review!

On 18.08.22, Sergey Kaplun wrote:
> Hi, Igor!
> 
> Thanks for the patch!
> LGTM!
> 
> On 11.08.22, Igor Munkin wrote:
> > As a result of the previous commit, there is no difference left between
> > x86_64 and ARM64 workflows except the runner where workflow is run.
> > Hence there is no reason to have a separate workflow file for each
> > hardware architecture.
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> >  .github/workflows/linux-aarch64.yml           | 83 -----------------
> >  .../workflows/{linux-x86_64.yml => linux.yml} | 52 ++++++++---
> >  .github/workflows/macos-m1.yml                | 92 -------------------
> >  .../workflows/{macos-x86_64.yml => macos.yml} | 52 ++++++++---
> >  4 files changed, 82 insertions(+), 197 deletions(-)
> >  delete mode 100644 .github/workflows/linux-aarch64.yml
> >  rename .github/workflows/{linux-x86_64.yml => linux.yml} (69%)
> >  delete mode 100644 .github/workflows/macos-m1.yml
> >  rename .github/workflows/{macos-x86_64.yml => macos.yml} (73%)
> > 

<snipped>

> > @@ -65,8 +77,8 @@ jobs:
> >          run: cmake --build . --parallel --target test
> >          working-directory: ${{ env.BUILDDIR }}
> >  
> > -  test-tarantool-debug-wo-GC64:
> > -    name: Tarantool Debug GC64:OFF
> > +  test-tarantool-x86_64-debug-wo-GC64:
> 
> Side note: Can we use the same matrix approach for integration
> workflows?

Unfortunately, we can't: reusable workflows doesn't support matrix
approach (as I've already mentioned here[1]).

> 
> > +    name: Tarantool x86_64 Debug GC64:OFF
> >      needs: test-luajit
> >      uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> >      with:

<snipped>

> > -- 
> > 2.34.0
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

[1]: https://lists.tarantool.org/tarantool-patches/YrzPiTa7FyNFKbbW@tarantool.org/

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 3/8] test: fix tarantool suite for out of source build
  2022-08-18  9:49   ` Sergey Kaplun via Tarantool-patches
@ 2022-08-31 17:20     ` Igor Munkin via Tarantool-patches
  0 siblings, 0 replies; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-08-31 17:20 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

Thanks for your review!

On 18.08.22, Sergey Kaplun wrote:
> Hi, Igor!
> 
> Thanks for the patch!
> This is really nice to fix this pebble in the shoe!
> LGTM, except a few typos in comment and the commit message.
> 
> On 11.08.22, Igor Munkin wrote:
> > jit/vmdef.lua is autogenerated file, so it's put to src/ directory
> > located in scope of the binary artefacts tree. Before the patch LUA_PATH
> > lacks this path, so tarantool-tests target fails due to jit/vmdef.lua
> 
> Side note: IINM tarantool-tests are affected due to `jit.bc` usage in
> the test suite.
> 
> > misseek. As a result of this change src/ directory in scope of the
> 
> Typo: s/in scope/in the scope/

Fixed.

> 
> > binary tree is included to LUA_PATH as well as the one from the source
> > tree has been.
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> >  test/tarantool-tests/CMakeLists.txt | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> > index 27866869..97c23670 100644
> > --- a/test/tarantool-tests/CMakeLists.txt
> > +++ b/test/tarantool-tests/CMakeLists.txt
> > @@ -70,14 +70,17 @@ add_subdirectory(misclib-sysprof-capi)
> >  
> >  # The part of the memory profiler toolchain is located in tools
> >  # directory, jit, profiler, and bytecode toolchains are located
> > -# in src/ directory and auxiliary tests-related modules are
> > -# located in the current directory (but tests are run in the
> > -# binary directory), so LUA_PATH need to be updated.
> > +# in src/ directory, jit/vmdef.lua is autogenerated file also
> > +# located in src/ directory, but in scope of the binary artefacts
> 
> Typo: s/in scope/in the scope/

Fixed.

> 
> > +# tree and auxiliary tests-related modules are located in the
> > +# current directory (but tests are run in the binary directory),
> > +# so LUA_PATH need to be updated.
> >  make_lua_path(LUA_PATH
> >    PATHS
> >      ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
> >      ${PROJECT_SOURCE_DIR}/tools/?.lua
> >      ${PROJECT_SOURCE_DIR}/src/?.lua
> > +    ${PROJECT_BINARY_DIR}/src/?.lua
> >  )
> >  # Update LUA_CPATH with the library paths collected within
> >  # <BuildTestLib> macro.
> > -- 
> > 2.34.0
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 2/8] test: introduce MakeLuaPath.cmake helper
  2022-08-31 15:19     ` Igor Munkin via Tarantool-patches
@ 2022-09-01 10:16       ` Sergey Kaplun via Tarantool-patches
  0 siblings, 0 replies; 39+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-09-01 10:16 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Hi, Igor!

Thanks for the fixes!

On 31.08.22, Igor Munkin wrote:
> Sergey,
> 
> Thanks for your review!
> 
> On 18.08.22, Sergey Kaplun wrote:

<snipped>

> > > +# XXX: Mind the reverse order of the entries in the result string.
> > 
> > Why do we need this behaviour? May be it is better to save strict order
> > of entries?
> 
> The code will become more complex as a result and in the 99.999% of the
> cases the order of LUA_PATH entries doesn't bother you.

But it bothers me for the lua-Harness tests already. We get the
following snippet from <303-package.t>:

|   f = io.open('syntax.lua', 'w')
|   f:write [[?syntax error?]]
|   f:close()
|   local r, e = pcall(require, 'syntax')
|   error_matches(function () require('syntax') end,
|           "^error loading module 'syntax' from file '%.[/\\]syntax%.lua':",
|           "function require (syntax error)")

As we can see `error_matches()` expects "file './syntax.lua'" string
from the output, but if "./.?lua" path isn't the first one, this test
will fail due to the whole path in the output.

>                                                         So I don't see
> we should make the code more complex to save the order of the entries.

As far as I don't usually read from bottom to top (except mcode encoding)
it is more convenient to me to use strict order.

Also, as for me the following patch (feel free to modify it as you want)
doesn't make the code too complex:

===================================================================
diff --git a/cmake/MakeLuaPath.cmake b/cmake/MakeLuaPath.cmake
index 9a5a3bb8..f92bdb19 100644
--- a/cmake/MakeLuaPath.cmake
+++ b/cmake/MakeLuaPath.cmake
@@ -28,19 +28,23 @@ function(make_lua_path path)
                         "${multiValues}"
                         ${ARGN})
 
-  # XXX: This is the sentinel semicolon having special meaning
-  # for LUA_PATH and LUA_CPATH variables. For more info, see the
-  # link below:
-  # https://www.lua.org/manual/5.1/manual.html#pdf-LUA_PATH
-  set(result "\;")
-
   foreach(inc ${ARG_PATHS})
     # XXX: If one joins two strings with semicolon, the value
     # automatically becomes a list. I found a single working
     # solution to make result variable be a string via "escaping"
     # the semicolon right in string interpolation.
-    set(result "${inc}\;${result}")
+    if(result)
+      set(result "${result}\;${inc}")
+    else()
+      set(result "${inc}")
+    endif()
   endforeach()
 
+  # XXX: This is the sentinel semicolon having special meaning
+  # for LUA_PATH and LUA_CPATH variables. For more info, see the
+  # link below:
+  # https://www.lua.org/manual/5.1/manual.html#pdf-LUA_PATH
+  set(result "${result}\;\;")
+
   set(${path} "${result}" PARENT_SCOPE)
 endfunction()
===================================================================

> 
> > 
> > > +
> > > +function(make_lua_path path)

<snipped>

> > 
> > -- 
> > Best regards,
> > Sergey Kaplun
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 1/8] test: introduce LUAJIT_TEST_VARDIR variable
  2022-08-31 14:53     ` Igor Munkin via Tarantool-patches
@ 2022-09-02 12:06       ` Sergey Bronnikov via Tarantool-patches
  2022-10-05 19:51         ` Igor Munkin via Tarantool-patches
  0 siblings, 1 reply; 39+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-09-02 12:06 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Hi,

On 31.08.2022 17:53, Igor Munkin wrote:
> Sergey,
>
> Thanks for your review!
>
> On 15.08.22, Sergey Bronnikov wrote:
>> Igor,
>>
>>    thanks for the patch. See my comments inline.
>>
>> On 11.08.2022 14:17, Igor Munkin wrote:
>>> Before the patch both memprof and sysprof artefacts are generated within
>>> the binary artefacts tree (i.e. in the same directory LuaJIT binary is
>>> generated). However, more convenient way is producing these temporary
>>> profiles in a separate directory (e.g. located on the partition with
>>> more strict space limits). As a result of the patch all memprof and
>>> sysprof test chunks consider LUAJIT_TEST_VARDIR environment variable to
>>> set the directory where test profiles are generated. For the case when
>>> LUAJIT_TEST_VARDIR is not set, everything works as before.
>> Commit message is inconsistent a bit with a patch itself.
>>
>> As far as I understand many hunks are not related to introducing
>> LUAJIT_TEST_VARDIR.
>>
>> Probably it is better to change commit one-line message from "test:
>> introduce LUAJIT_TEST_VARDIR variable"
>>
>> to something like "test: refactoring and introduce LUAJIT_TEST_VARDIR
>> variable". It allows to keep patch
>>
>> as is and change expectations for those who will look at your patch.
> I believe your commit subject is too long, so I propose another
> solution. Since the only thing affected by LUAJIT_TEST_VARDIR is
> <utils.profilename>, so I can write something like "test: introduce
> utils.profilename helper" and describe its rationale and functions
> within the commit message. Are you OK with it?

I'm ok!

<snipped>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 4/8] ci: use out of source build in GitHub Actions
  2022-08-31 15:33     ` Igor Munkin via Tarantool-patches
@ 2022-09-02 12:09       ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 0 replies; 39+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-09-02 12:09 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Hi,

LGTM

On 31.08.2022 18:33, Igor Munkin wrote:
> <snipped>
>>> diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
>>> index 8c8dcff1..21d86764 100644
>>> --- a/.github/workflows/linux-aarch64.yml
>>> +++ b/.github/workflows/linux-aarch64.yml
>>> @@ -53,11 +53,15 @@ jobs:
>>>              sudo apt -y update
>>>              sudo apt -y install cmake gcc make perl
>>>          - name: configure
>>> -        run: cmake . ${{ matrix.CMAKEFLAGS }}
>>> +        run: >
>>> +          cmake -S . -B ${{ env.BUILDDIR }}
>>> +          ${{ matrix.CMAKEFLAGS }}
>>>          - name: build
>>>            run: cmake --build . --parallel
>>> +        working-directory: ${{ env.BUILDDIR }}
>>>          - name: test
>>>            run: cmake --build . --parallel --target test
>>> +        working-directory: ${{ env.BUILDDIR }}
>> 1. I don't get an idea to use current dir for CMake and specify a
>> working-directory in a job step.
>>
>> Why not "cmake --build ${{ env.BUILDDIR }}" as above?
> I tried to not touch the original command. Configuration step can't be
> left intact, but build and test -- can and that makes diff more clear
> (IMHO). Furhermore, it looks more explicit to me: the command is run in
> the current directory, and the current directory is specified
> separately from the command itself.
>
>>>    
>>>      test-tarantool-debug-w-GC64:
>>>        name: Tarantool Debug GC64:ON
> <snipped>
>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 2/8] test: introduce MakeLuaPath.cmake helper
  2022-08-31 15:07     ` Igor Munkin via Tarantool-patches
@ 2022-09-02 12:09       ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 0 replies; 39+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-09-02 12:09 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

LGTM

On 31.08.2022 18:07, Igor Munkin wrote:
> Sergey,
>
> Thanks for you review!
>
> On 15.08.22, Sergey Bronnikov wrote:
>> Igor, thanks for the patch! See my comments inline.
>>
>> On 11.08.2022 14:17, Igor Munkin wrote:
>>> While extending test suites it is often required to append additional
>>> path where Lua or Lua-C auxiliary modules are located to LUA_PATH or
>>> LUA_CPATH environment variables. Due to insane semicolon interpolation
>>> in CMake strings (that converts such string to a list as a result), we
>>> need to escape semicolon in LUA_PATH/LUA_CPATH strings while building
>>> the resulting value.
>>>
>>> After the years of struggling MakeLuaPath.cmake module is introduced to
>>> make LUA_PATH and LUA_CPATH definition convenient with <make_lua_path>
>>> helper. This function takes all paths given as a variable list argument,
>>> joins them in a reverse order by a semicolon and yields the resulting
>>> string to a specified CMake variable.
>>>
>>> Signed-off-by: Igor Munkin<imun@tarantool.org>
>>> ---
>>>    cmake/MakeLuaPath.cmake                   | 46 +++++++++++++++++++++++
>>>    test/CMakeLists.txt                       |  2 +
>>>    test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt |  8 +++-
>>>    test/lua-Harness-tests/CMakeLists.txt     | 16 +++++---
>>>    test/tarantool-tests/CMakeLists.txt       | 28 ++++++++------
>>>    5 files changed, 81 insertions(+), 19 deletions(-)
>>>    create mode 100644 cmake/MakeLuaPath.cmake
>>>
>>> diff --git a/cmake/MakeLuaPath.cmake b/cmake/MakeLuaPath.cmake
>>> new file mode 100644
>>> index 00000000..9a5a3bb8
>>> --- /dev/null
>>> +++ b/cmake/MakeLuaPath.cmake
> <snipped>
>
>>> +
>>> +  # FIXME: if we update to CMake >= 3.5, can remove this line.
>> I would replace comment with condition that will check CMake version
>>
>> and will fail when CMake >= 3.5:
>>
>> |if (CMAKE_VERSION VERSION_GREATER_EQUAL 35) |
>>
>>      message(FATAL_ERROR "Please remove snippet for CMake < 3.5")
>>
>> endif()
> I doubt this is a working solution. The comment means, that there is no
> need to include a separate module if CMake to be used is greater than
> 3.5. With your code, fatal error is raised in that case. I have CMake
> 3.20 on my local machine and LuaJIT fails to be built with your check
> added (I've fixed the typo in the version number).
>
> Ignoring for now.
>
>>
>>> +  include(CMakeParseArguments)
> <snipped>
>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 1/8] test: introduce LUAJIT_TEST_VARDIR variable
  2022-09-02 12:06       ` Sergey Bronnikov via Tarantool-patches
@ 2022-10-05 19:51         ` Igor Munkin via Tarantool-patches
  0 siblings, 0 replies; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-10-05 19:51 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Sergey,

On 02.09.22, Sergey Bronnikov wrote:
> Hi,
> 
> On 31.08.2022 17:53, Igor Munkin wrote:
> > Sergey,
> >
> > Thanks for your review!
> >
> > On 15.08.22, Sergey Bronnikov wrote:
> >> Igor,
> >>
> >>    thanks for the patch. See my comments inline.
> >>
> >> On 11.08.2022 14:17, Igor Munkin wrote:
> >>> Before the patch both memprof and sysprof artefacts are generated within
> >>> the binary artefacts tree (i.e. in the same directory LuaJIT binary is
> >>> generated). However, more convenient way is producing these temporary
> >>> profiles in a separate directory (e.g. located on the partition with
> >>> more strict space limits). As a result of the patch all memprof and
> >>> sysprof test chunks consider LUAJIT_TEST_VARDIR environment variable to
> >>> set the directory where test profiles are generated. For the case when
> >>> LUAJIT_TEST_VARDIR is not set, everything works as before.
> >> Commit message is inconsistent a bit with a patch itself.
> >>
> >> As far as I understand many hunks are not related to introducing
> >> LUAJIT_TEST_VARDIR.
> >>
> >> Probably it is better to change commit one-line message from "test:
> >> introduce LUAJIT_TEST_VARDIR variable"
> >>
> >> to something like "test: refactoring and introduce LUAJIT_TEST_VARDIR
> >> variable". It allows to keep patch
> >>
> >> as is and change expectations for those who will look at your patch.
> > I believe your commit subject is too long, so I propose another
> > solution. Since the only thing affected by LUAJIT_TEST_VARDIR is
> > <utils.profilename>, so I can write something like "test: introduce
> > utils.profilename helper" and describe its rationale and functions
> > within the commit message. Are you OK with it?
> 
> I'm ok!

I've rewritten the commit message the following way:
| test: introduce utils.profilename helper
|
| Before the patch both memprof and sysprof artefacts were generated
| within the binary artefacts tree (i.e. in the same directory LuaJIT
| binary is generated). However, more convenient way is producing these
| temporary profiles in a separate directory (e.g. located on the
| partition with more strict space limits).
|
| As a result of the patch <utils.profilename> helper is introduced and
| all memprof and sysprof test chunks are patched to use it. The new
| helper takes the basename of the profile to be created and also
| considers LUAJIT_TEST_VARDIR environment variable to set the directory,
| where test profiles are generated. For the case when LUAJIT_TEST_VARDIR
| is not set, everything works as before.
|
| Part of tarantool/tarantool#7472

> 
> <snipped>

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements
  2022-08-11 11:17 [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements Igor Munkin via Tarantool-patches
                   ` (7 preceding siblings ...)
  2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 8/8] ci: merge Linux and macOS workflows Igor Munkin via Tarantool-patches
@ 2022-11-11  8:56 ` Igor Munkin via Tarantool-patches
  8 siblings, 0 replies; 39+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-11-11  8:56 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

I've checked the patch into all long-term branches in tarantool/luajit
and bumped a new version in master, 2.10 and 1.10.

On 11.08.22, Igor Munkin wrote:
> This patchset contains several enhancements:
> * The first patch introduces LUAJIT_TEST_VARDIR required by #7472[1].
> * The three next patches fix Tarantool testing suite to be run out of
>   LuaJIT source tree and replace in source build in GitHub Actions.
> * The fifth patch removes the excess line from macOS M1 workflow.
> * The sixth patch removes arch prefix for macOS M1 workflow.
> * The last two patches merge all testing workflows into a single one.
> 
> Branch: https://github.com/tarantool/luajit/tree/imun/tweak-tests
> 
> Igor Munkin (8):
>   test: introduce LUAJIT_TEST_VARDIR variable
>   test: introduce MakeLuaPath.cmake helper
>   test: fix tarantool suite for out of source build
>   ci: use out of source build in GitHub Actions
>   ci: remove excess parallel level setup
>   ci: remove arch prefix for macOS M1 workflow
>   ci: merge x86_64 and ARM64 workflows
>   ci: merge Linux and macOS workflows
> 
>  .github/actions/environment/action.yml        |  13 --
>  .github/actions/setup-linux/README.md         |  12 ++
>  .github/actions/setup-linux/action.yml        |  19 ++
>  .github/actions/setup-macos/README.md         |  12 ++
>  .github/actions/setup-macos/action.yml        |  29 +++
>  .../actions/{environment => setup}/README.md  |   5 +-
>  .github/actions/setup/action.yml              |  10 +
>  .github/workflows/lint.yml                    |  11 +-
>  .github/workflows/linux-aarch64.yml           |  79 -------
>  .github/workflows/linux-x86_64-ninja.yml      |  12 +-
>  .github/workflows/linux-x86_64.yml            |  98 ---------
>  .github/workflows/macos-m1.yml                |  94 --------
>  .github/workflows/macos-x86_64.yml            | 107 ----------
>  .github/workflows/testing.yml                 | 200 ++++++++++++++++++
>  cmake/MakeLuaPath.cmake                       |  46 ++++
>  test/CMakeLists.txt                           |   2 +
>  test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt     |   8 +-
>  test/lua-Harness-tests/CMakeLists.txt         |  16 +-
>  test/tarantool-tests/CMakeLists.txt           |  37 ++--
>  .../gh-5813-resolving-of-c-symbols.test.lua   |   6 +-
>  .../misclib-memprof-lapi.test.lua             |  22 +-
>  .../misclib-sysprof-lapi.test.lua             |   8 +-
>  test/tarantool-tests/utils.lua                |  12 ++
>  23 files changed, 423 insertions(+), 435 deletions(-)
>  delete mode 100644 .github/actions/environment/action.yml
>  create mode 100644 .github/actions/setup-linux/README.md
>  create mode 100644 .github/actions/setup-linux/action.yml
>  create mode 100644 .github/actions/setup-macos/README.md
>  create mode 100644 .github/actions/setup-macos/action.yml
>  rename .github/actions/{environment => setup}/README.md (72%)
>  create mode 100644 .github/actions/setup/action.yml
>  delete mode 100644 .github/workflows/linux-aarch64.yml
>  delete mode 100644 .github/workflows/linux-x86_64.yml
>  delete mode 100644 .github/workflows/macos-m1.yml
>  delete mode 100644 .github/workflows/macos-x86_64.yml
>  create mode 100644 .github/workflows/testing.yml
>  create mode 100644 cmake/MakeLuaPath.cmake
> 
> -- 
> 2.34.0
> 

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2022-11-11  9:10 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-11 11:17 [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements Igor Munkin via Tarantool-patches
2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 1/8] test: introduce LUAJIT_TEST_VARDIR variable Igor Munkin via Tarantool-patches
2022-08-15 12:08   ` Sergey Bronnikov via Tarantool-patches
2022-08-18  8:27     ` Sergey Kaplun via Tarantool-patches
2022-08-31 14:53     ` Igor Munkin via Tarantool-patches
2022-09-02 12:06       ` Sergey Bronnikov via Tarantool-patches
2022-10-05 19:51         ` Igor Munkin via Tarantool-patches
2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 2/8] test: introduce MakeLuaPath.cmake helper Igor Munkin via Tarantool-patches
2022-08-15 12:08   ` Sergey Bronnikov via Tarantool-patches
2022-08-31 15:07     ` Igor Munkin via Tarantool-patches
2022-09-02 12:09       ` Sergey Bronnikov via Tarantool-patches
2022-08-18  9:37   ` Sergey Kaplun via Tarantool-patches
2022-08-31 15:19     ` Igor Munkin via Tarantool-patches
2022-09-01 10:16       ` Sergey Kaplun via Tarantool-patches
2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 3/8] test: fix tarantool suite for out of source build Igor Munkin via Tarantool-patches
2022-08-15 12:10   ` Sergey Bronnikov via Tarantool-patches
2022-08-18  9:49   ` Sergey Kaplun via Tarantool-patches
2022-08-31 17:20     ` Igor Munkin via Tarantool-patches
2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 4/8] ci: use out of source build in GitHub Actions Igor Munkin via Tarantool-patches
2022-08-15 12:13   ` Sergey Bronnikov via Tarantool-patches
2022-08-18  9:58     ` Sergey Kaplun via Tarantool-patches
2022-08-31 15:34       ` Igor Munkin via Tarantool-patches
2022-08-31 15:33     ` Igor Munkin via Tarantool-patches
2022-09-02 12:09       ` Sergey Bronnikov via Tarantool-patches
2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 5/8] ci: remove excess parallel level setup Igor Munkin via Tarantool-patches
2022-08-15 12:14   ` Sergey Bronnikov via Tarantool-patches
2022-08-18 10:09   ` Sergey Kaplun via Tarantool-patches
2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 6/8] ci: remove arch prefix for macOS M1 workflow Igor Munkin via Tarantool-patches
2022-08-15 12:17   ` Sergey Bronnikov via Tarantool-patches
2022-08-18 10:14   ` Sergey Kaplun via Tarantool-patches
2022-08-31 15:55     ` Igor Munkin via Tarantool-patches
2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 7/8] ci: merge x86_64 and ARM64 workflows Igor Munkin via Tarantool-patches
2022-08-15 12:22   ` Sergey Bronnikov via Tarantool-patches
2022-08-18 10:21   ` Sergey Kaplun via Tarantool-patches
2022-08-31 16:02     ` Igor Munkin via Tarantool-patches
2022-08-11 11:17 ` [Tarantool-patches] [PATCH luajit 8/8] ci: merge Linux and macOS workflows Igor Munkin via Tarantool-patches
2022-08-15 12:27   ` Sergey Bronnikov via Tarantool-patches
2022-08-18 10:32   ` Sergey Kaplun via Tarantool-patches
2022-11-11  8:56 ` [Tarantool-patches] [PATCH luajit 0/8] LuaJIT tests and CI enhancements Igor Munkin via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox