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
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
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
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
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
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
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
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
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
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
[-- 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 --]
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.
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
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 }}
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:
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 }}
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 }}
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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>
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>
>
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>
>
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
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