[Tarantool-patches] [PATCH luajit] ci: introduce GitHub Actions
Sergey Kaplun
skaplun at tarantool.org
Wed Mar 2 10:53:05 MSK 2022
Hi, Igor!
Thanks for the patch!
LGTM (considering I'm not the specialist in GitHub-CI), except a few
nits and ignorable comments below:
On 01.03.22, Igor Munkin wrote:
> This patch introduces the following testing matrix for our LuaJIT fork:
>
> +--------------------------------+------------------+-------------+
> | | Linux | macOS |
> | +--------+---------+--------+----+
> | | x86_64 | aarch64 | x86_64 | M1 |
> +---------------+----------------+--------+---------+--------+----+
> | | RelWithDebInfo | + | + | + | + |
> + GC64 disabled +----------------+--------+---------+--------+----+
> | | Debug(*) | + | + | + | + |
> +---------------+----------------+--------+---------+--------+----+
> | | RelWithDebInfo | + | + | + | + |
> + GC64 enabled +----------------+--------+---------+--------+----+
> | | Debug(*) | + | + | + | + |
> +---------------+----------------+--------+---------+--------+----+
>
> (*) Debug build also uses LuaJIT internal assertions.
>
> For each column there is a separate workflow file, since it requires a
> specific runner and setup routine. For each row workflow toggles GC64
> mode and build type flags and triggers the run with the chosen
> configuration options.
>
> There is one more workflow running only static analysis check. This
> division is caused by specific requirements to be installed (i.e.
> luarocks and luacheck). For other workflows <LuaJIT-luacheck> rule is
Typo: s/other/others/
> just a noop step (see test/CMakeLists.txt for more info).
>
> All workflows are not triggered for the branches with "-notest" suffix
> and also "upstream-" prefix (but they are unlikely to be merged into the
> latter ones anyway). Nothing is also triggered for all tags.
>
> There is also one nit to be mentioned: actively running workflows on all
> branches except the long-term ones (i.e. tarantool, tarantool-1.10, etc)
> are canceled if "force push" occurs.
Side note: This is great!
>
> Signed-off-by: Igor Munkin <imun at tarantool.org>
> ---
> .github/workflows/lint.yml | 47 ++++++++++++++++++++
> .github/workflows/linux-aarch64.yml | 57 +++++++++++++++++++++++++
> .github/workflows/linux-x86_64.yml | 57 +++++++++++++++++++++++++
> .github/workflows/macos-m1.yml | 66 +++++++++++++++++++++++++++++
> .github/workflows/macos-x86_64.yml | 66 +++++++++++++++++++++++++++++
> 5 files changed, 293 insertions(+)
> create mode 100644 .github/workflows/lint.yml
> create mode 100644 .github/workflows/linux-aarch64.yml
> create mode 100644 .github/workflows/linux-x86_64.yml
> create mode 100644 .github/workflows/macos-m1.yml
> create mode 100644 .github/workflows/macos-x86_64.yml
>
> diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
> new file mode 100644
> index 00000000..e0cb8eff
> --- /dev/null
> +++ b/.github/workflows/lint.yml
> @@ -0,0 +1,47 @@
> +name: "Static analysis"
> +
> +on:
> + push:
> + branches-ignore:
> + - '**-notest'
> + - 'upstream-**'
> + tags-ignore:
> + - '**'
> +
> +concurrency:
> + # Update of a developer branch cancels the previously scheduled
Typo: s/Update/An update/
Here and below.
> + # 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-luacheck:
> + runs-on: ubuntu-20.04-self-hosted
> + steps:
> + - uses: actions/checkout at v2.3.4
> + with:
> + fetch-depth: 0
> + submodules: recursive
> + - name: setup
> + run: |
> + sudo apt -y update
> + sudo apt -y install cmake make lua5.1 luarocks
Side note: Do we need to install Lua for luacheck? We can use LuaJIT
(because it slightly faster).
| time lua /usr/bin/luacheck --codes --config ./.luacheckrc . | tail -n 1
| Total: 0 warnings / 0 errors in 34 files
|
| real 0m0.236s
| user 0m0.223s
| sys 0m0.015s
| time luajit /usr/bin/luacheck --codes --config ./.luacheckrc . | tail -n 1
| Total: 0 warnings / 0 errors in 34 files
|
| real 0m0.204s
| user 0m0.176s
| sys 0m0.030s
> + sudo luarocks install luacheck
> + - name: configure
> + run: cmake .
> + - name: test
> + run: make -j LuaJIT-luacheck
> diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
> new file mode 100644
> index 00000000..a43d22ac
> --- /dev/null
> +++ b/.github/workflows/linux-aarch64.yml
> @@ -0,0 +1,57 @@
> +name: "LuaJIT test workflow (Linux/AArch64)"
I suggest decrease the name here. It's too long. For example, I can't
see the difference between these two jobs [1][2] without reading
configuring section.
As for me, obviously that all run workflows are about LuaJIT testing, so
we can just drop this part of the description. Linux-AArch64 looks nice.
> +
> +on:
> + push:
> + branches-ignore:
> + - '**-notest'
> + - 'upstream-**'
> + tags-ignore:
> + - '**'
> +
> +concurrency:
> + # 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-linux-aarch64:
> + runs-on: graviton
> + strategy:
> + matrix:
> + BUILDTYPE:
> + - -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
> + - -DCMAKE_BUILD_TYPE=RelWithDebInfo
> + GC64:
> + - -DLUAJIT_ENABLE_GC64=ON
> + - -DLUAJIT_ENABLE_GC64=OFF
> +
> + steps:
> + - uses: actions/checkout at v2.3.4
> + with:
> + fetch-depth: 0
> + submodules: recursive
> + - name: setup
> + run: |
> + sudo apt -y update
> + sudo apt -y install cmake gcc make perl
Side note: Will we add clang compiler in future CI jobs?
> + - name: configure
> + run: cmake . ${{ matrix.BUILDTYPE }} ${{ matrix.GC64 }}
When we run Debug job, we know, that it always sets -DLUA_USE_ASSERT=ON
-DLUA_USE_APICHECK=ON in cmake flags. Can we use just change names
Debug/Release and GC64-ON/GC64-OFF for matrix entries and provide the
necessary information in a build via matrix environment variables[3]?
So instead dreaded job name
| test-linux-aarch64 (-DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON -DLUAJIT_ENABLE_GC64=ON)
we will have (AFAIU)
| test-linux-aarch64 (Debug GC64-ON)
IMO, it makes easier finding job we are interested in.
> + - name: build
> + run: make -j
> + - name: test
> + run: make -j test
IINM, as far as strategy.fail-fast[4] field is set in `true` by default,
if one job from matrix fails, the others will be canceled. IMO, it's
inconvenient to stop all jobs if Debug-GC64 job is failed, so I suggest to
set this value to `false` manually.
Also, you may notice, that all workflows are quite similar. They differ
by name, 'runs-on' field and (for macOS/Linux) setup step.
So maybe we can use matrix to cover all these workflows as showing
here[5], using environment variables to determine setup step for the
particular OS (macOS or Linux)?
But maybe I am overcomplicating things, so feel free to ignore. :)
> diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux-x86_64.yml
> new file mode 100644
> index 00000000..0f32e56d
> --- /dev/null
> +++ b/.github/workflows/linux-x86_64.yml
<snipped> as far as diffierence with the previous one is minimal.
> diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
> new file mode 100644
> index 00000000..c34953be
> --- /dev/null
> +++ b/.github/workflows/macos-m1.yml
> @@ -0,0 +1,66 @@
> +name: "LuaJIT test workflow (macOS/M1)"
> +
<snipped>
> +
> +jobs:
> + test-macos-m1:
> + runs-on: macos-11-m1
> + strategy:
> + matrix:
> + BUILDTYPE:
> + - -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
> + - -DCMAKE_BUILD_TYPE=RelWithDebInfo
> + GC64:
> + - -DLUAJIT_ENABLE_GC64=ON
> + - -DLUAJIT_ENABLE_GC64=OFF
> +
> + steps:
> + - uses: actions/checkout at v2.3.4
> + with:
> + fetch-depth: 0
> + submodules: recursive
> + - name: setup
> + run: |
> + # Install brew using command from Homebrew repository instructions:
Typo: /command/the command/
> + # 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
Typo: s/try to/Try to/
Typo? s/fails/failing/
> + # if the package already exists with the previous version
Typo: s/version/version./
> + brew install --force cmake gcc make perl ||
> + brew upgrade cmake gcc make perl
> + - name: configure
> + run: cmake . ${{ matrix.BUILDTYPE }} ${{ matrix.GC64 }}
> + - name: build
> + run: make -j
> + - name: test
> + run: make -j test
> diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos-x86_64.yml
> new file mode 100644
> index 00000000..cbf8a285
> --- /dev/null
> +++ b/.github/workflows/macos-x86_64.yml
> @@ -0,0 +1,66 @@
<snipped> as far as diffierence with the previous one is minimal.
> --
> 2.34.0
>
[1]: https://github.com/tarantool/luajit/runs/5375573436?check_suite_focus=true
[2]: https://github.com/tarantool/luajit/runs/5375573350?check_suite_focus=true
[3]: https://docs.github.com/en/actions/using-jobs/using-a-build-matrix-for-your-jobs#using-environment-variables-in-a-matrix
[4]: https://docs.github.com/en/actions/using-jobs/using-a-build-matrix-for-your-jobs#canceling-remaining-jobs-if-a-matrix-job-fails
[5]: https://docs.github.com/en/actions/using-jobs/using-a-build-matrix-for-your-jobs#example-running-with-multiple-operating-systems
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list