[Tarantool-patches] [PATCH luajit] ci: introduce GitHub Actions

[Tarantool-patches] [PATCH luajit] ci: introduce GitHub Actions

[Igor Munkin via Tarantool-patches]

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
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.

Signed-off-by: Igor Munkin <imun@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
+  # 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@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
+          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)"
+
+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@v2.3.4
+        with:
+          fetch-depth: 0
+          submodules: recursive
+      - name: setup
+        run: |
+          sudo apt -y update
+          sudo apt -y install 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/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
@@ -0,0 +1,57 @@
+name: "LuaJIT test workflow (Linux/x86_64)"
+
+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-x86_64:
+    runs-on: ubuntu-20.04-self-hosted
+    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@v2.3.4
+        with:
+          fetch-depth: 0
+          submodules: recursive
+      - name: setup
+        run: |
+          sudo apt -y update
+          sudo apt -y install 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-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)"
+
+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-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@v2.3.4
+        with:
+          fetch-depth: 0
+          submodules: recursive
+      - name: setup
+        run: |
+          # Install brew using 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 . ${{ 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 @@
+name: "LuaJIT test workflow (macOS/x86_64)"
+
+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-macos-x86_64:
+    runs-on: macos-11
+    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@v2.3.4
+        with:
+          fetch-depth: 0
+          submodules: recursive
+      - name: setup
+        run: |
+          # Install brew using 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 . ${{ matrix.BUILDTYPE }} ${{ matrix.GC64 }}
+      - name: build
+        run: make -j
+      - name: test
+        run: make -j test
-- 
2.34.0

Re: [Tarantool-patches] [PATCH luajit] ci: introduce GitHub Actions

[Sergey Kaplun via Tarantool-patches]

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@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@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@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@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

Re: [Tarantool-patches] [PATCH luajit] ci: introduce GitHub Actions

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for your review! I hold your LGTM a bit, until I resolve all your
"nits" (that actually are quite vital comments to the patch).

On 02.03.22, Sergey Kaplun wrote:
> Hi, Igor!
> 
> Thanks for the patch!
> 
> LGTM (considering I'm not the specialist in GitHub-CI), except a few
> nits and ignorable comments below:

Considering the changes made below, here is the new commit message:
| ci: introduce GitHub Actions
|
| This patch introduces the following testing matrix for our LuaJIT fork:
|
| +---------------------+------------------+-------------+
| |                     |      Linux       |    macOS    |
| |                     +--------+---------+--------+----+
| |                     | x86_64 | aarch64 | x86_64 | M1 |
| +----------+----------+--------+---------+--------+----+
| |          | Release  |   +    |    +    |   +    | +  |
| + GC64 on  +----------+--------+---------+--------+----+
| |          | Debug(*) |   +    |    +    |   +    | +  |
| +----------+----------+--------+---------+--------+----+
| |          | Release  |   +    |    +    |   +    | +  |
| + GC64 off +----------+--------+---------+--------+----+
| |          | Debug(*) |   +    |    +    |   +    | +  |
| +----------+----------+--------+---------+--------+----+
|
| (*) Debug build also uses LuaJIT internal assertions.
|
| For each OS there is a separate workflow file, since it requires a
| OS-specific setup routine. For each column particular runner family is
| chosen based on the architecture name from <matrix.ARCH> options. 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
| 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.

Here is the new CI results[1] (unfortunately, I was kinda in rage and
deleted the runs from the previous version... hope nobody needs them).

> 
> 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/

Nope, it's not. Ignoring.

> 
> > 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@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.

Fixed for all entries. Diff is below:

================================================================================

diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index e0cb8eff..4d53171c 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -9,10 +9,10 @@ on:
       - '**'
 
 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.
+  # 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
diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
index a43d22ac..4799e8bc 100644
--- a/.github/workflows/linux-aarch64.yml
+++ b/.github/workflows/linux-aarch64.yml
@@ -9,10 +9,10 @@ on:
       - '**'
 
 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.
+  # 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
diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux-x86_64.yml
index 0f32e56d..656fb515 100644
--- a/.github/workflows/linux-x86_64.yml
+++ b/.github/workflows/linux-x86_64.yml
@@ -9,10 +9,10 @@ on:
       - '**'
 
 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.
+  # 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
diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
index c34953be..b8c43bf6 100644
--- a/.github/workflows/macos-m1.yml
+++ b/.github/workflows/macos-m1.yml
@@ -9,10 +9,10 @@ on:
       - '**'
 
 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.
+  # 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
diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos-x86_64.yml
index cbf8a285..911ee66c 100644
--- a/.github/workflows/macos-x86_64.yml
+++ b/.github/workflows/macos-x86_64.yml
@@ -9,10 +9,10 @@ on:
       - '**'
 
 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.
+  # 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

================================================================================

> 
> > +  # 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@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).

Well... We both know the situations with LuaJIT distribution, so I
prefer to use Lua here even if we spend a little more time for
bootstrap. Ignoring.

> 
> | 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.

This is name of the workflow, not the job. Job naming issue is fixed
below (with your help). I prefer to leave workflow name untouched.
Ignoring.

> 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@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?

I guess we will later (e.g. when sanitizers support is introduced).

> 
> > +      - 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.

Thanks a lot for your inputs and help (include section makes workflow
file much more fancy): I've fixed the workflow the way you've suggested.
I also change 'runs-on' value to the template, so we can choose the
runner respecting the <strategy.matrix.ARCH> field. As a result amount
of workflow files are reduced by two. Diff is below:

================================================================================

diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux-x86_64.yml
deleted file mode 100644
index 656fb515..00000000
--- a/.github/workflows/linux-x86_64.yml
+++ /dev/null
@@ -1,57 +0,0 @@
-name: "LuaJIT test workflow (Linux/x86_64)"
-
-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-linux-x86_64:
-    runs-on: ubuntu-20.04-self-hosted
-    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@v2.3.4
-        with:
-          fetch-depth: 0
-          submodules: recursive
-      - name: setup
-        run: |
-          sudo apt -y update
-          sudo apt -y install 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/linux-aarch64.yml b/.github/workflows/linux.yml
similarity index 65%
rename from .github/workflows/linux-aarch64.yml
rename to .github/workflows/linux.yml
index 4799e8bc..54500f6e 100644
--- a/.github/workflows/linux-aarch64.yml
+++ b/.github/workflows/linux.yml
@@ -1,4 +1,4 @@
-name: "LuaJIT test workflow (Linux/AArch64)"
+name: "LuaJIT test workflow (Linux)"
 
 on:
   push:
@@ -29,17 +29,23 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  test-linux-aarch64:
-    runs-on: graviton
+  test-linux:
+    runs-on: ${{ matrix.RUNNER }}
     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
-
+        ARCH: [x86_64, aarch64]
+        BUILDTYPE: [Debug, Release]
+        GC64: [ON, OFF]
+        include:
+          - ARCH: x86_64
+            RUNNER: ubuntu-20.04-self-hosted
+          - ARCH: aarch64
+            RUNNER: graviton
+          - BUILDTYPE: Debug
+            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
+          - BUILDTYPE: Release
+            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
+    name: Linux/${{ matrix.ARCH }} ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
     steps:
       - uses: actions/checkout@v2.3.4
         with:
@@ -50,7 +56,7 @@ jobs:
           sudo apt -y update
           sudo apt -y install cmake gcc make perl
       - name: configure
-        run: cmake . ${{ matrix.BUILDTYPE }} ${{ matrix.GC64 }}
+        run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
       - name: build
         run: make -j
       - name: test
diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos-x86_64.yml
deleted file mode 100644
index 911ee66c..00000000
--- a/.github/workflows/macos-x86_64.yml
+++ /dev/null
@@ -1,66 +0,0 @@
-name: "LuaJIT test workflow (macOS/x86_64)"
-
-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-macos-x86_64:
-    runs-on: macos-11
-    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@v2.3.4
-        with:
-          fetch-depth: 0
-          submodules: recursive
-      - name: setup
-        run: |
-          # Install brew using 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 . ${{ matrix.BUILDTYPE }} ${{ matrix.GC64 }}
-      - name: build
-        run: make -j
-      - name: test
-        run: make -j test
diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos.yml
similarity index 74%
rename from .github/workflows/macos-m1.yml
rename to .github/workflows/macos.yml
index b8c43bf6..fa770650 100644
--- a/.github/workflows/macos-m1.yml
+++ b/.github/workflows/macos.yml
@@ -1,4 +1,4 @@
-name: "LuaJIT test workflow (macOS/M1)"
+name: "LuaJIT test workflow (macOS)"
 
 on:
   push:
@@ -29,17 +29,23 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  test-macos-m1:
-    runs-on: macos-11-m1
+  test-macos:
+    runs-on: ${{ matrix.RUNNER }}
     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
-
+        ARCH: [x86_64, m1]
+        BUILDTYPE: [Debug, Release]
+        GC64: [ON, OFF]
+        include:
+          - ARCH: x86_64
+            RUNNER: macos-11
+          - ARCH: m1
+            RUNNER: macos-11-m1
+          - BUILDTYPE: Debug
+            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
+          - BUILDTYPE: Release
+            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
+    name: macOS/${{ matrix.ARCH }} ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
     steps:
       - uses: actions/checkout@v2.3.4
         with:
@@ -59,7 +65,7 @@ jobs:
           brew install --force cmake gcc make perl ||
             brew upgrade cmake gcc make perl
       - name: configure
-        run: cmake . ${{ matrix.BUILDTYPE }} ${{ matrix.GC64 }}
+        run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
       - name: build
         run: make -j
       - name: test

================================================================================

> 
> > +      - 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.

I didn't know this option (and such behaviour consequence for the matrix
strategy). Fixed. Diff is below:

================================================================================

diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml
index 54500f6e..7e77a93a 100644
--- a/.github/workflows/linux.yml
+++ b/.github/workflows/linux.yml
@@ -32,6 +32,7 @@ jobs:
   test-linux:
     runs-on: ${{ matrix.RUNNER }}
     strategy:
+      fail-fast: false
       matrix:
         ARCH: [x86_64, aarch64]
         BUILDTYPE: [Debug, Release]
diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml
index fa770650..4a8b5893 100644
--- a/.github/workflows/macos.yml
+++ b/.github/workflows/macos.yml
@@ -32,6 +32,7 @@ jobs:
   test-macos:
     runs-on: ${{ matrix.RUNNER }}
     strategy:
+      fail-fast: false
       matrix:
         ARCH: [x86_64, m1]
         BUILDTYPE: [Debug, Release]

================================================================================

> 
> 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)?

Setup steps differs a lot on macOS and Linux, and moving everything into
<strategy.matrix.include> section makes workflow file totally
unreadable. Ignoring.

> 
> But maybe I am overcomplicating things, so feel free to ignore. :)

Now workflow is much prettier, thanks a lot for your inputs and help!

> 

<snipped>

> > 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>

> > +      - name: setup
> > +        run: |
> > +          # Install brew using command from Homebrew repository instructions:
> 
> Typo: /command/the command/

Fixed. Diff is below:

================================================================================

diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml
index 4a8b5893..711a8bd9 100644
--- a/.github/workflows/macos.yml
+++ b/.github/workflows/macos.yml
@@ -54,8 +54,8 @@ jobs:
           submodules: recursive
       - name: setup
         run: |
-          # Install brew using command from Homebrew repository instructions:
-          #   https://github.com/Homebrew/install
+          # 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.

================================================================================

> 
> > +          #   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/

Fixed. Diff is below:

================================================================================

diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml
index 711a8bd9..1299cfd4 100644
--- a/.github/workflows/macos.yml
+++ b/.github/workflows/macos.yml
@@ -61,7 +61,7 @@ jobs:
           # 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
+          # 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

================================================================================

> Typo? s/fails/failing/

Nope, it's not. Ignoring.

> 
> > +          # if the package already exists with the previous version
> 
> Typo: s/version/version./

Fixed. Diff is below:

================================================================================

diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml
index 1299cfd4..45908ff2 100644
--- a/.github/workflows/macos.yml
+++ b/.github/workflows/macos.yml
@@ -62,7 +62,7 @@ jobs:
           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
+          # if the package already exists with the previous version.
           brew install --force cmake gcc make perl ||
             brew upgrade cmake gcc make perl
       - name: configure

================================================================================

> 

<snipped>

> 
> [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

[1]: https://github.com/tarantool/luajit/commit/ef0c6ce

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit] ci: introduce GitHub Actions

[Maxim Kokryashkin via Tarantool-patches]

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


Hi!
 
Thanks for the patch!
LGTM, except for comments by Sergey and a few of mine:
>
>> + # 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
IMO, it’s better to use a combination of `github.sha` + `github.event`,
since all you want is to suppress cancelation caused by tag push.
It is essentially the same, but at least it preserves the semantics.
>
>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. :)
I think that in our case it is actually better to have a lot of workflow configs.
From time to time we encounter some issues only on certain operating
systems, so it is great to have the ability to fine-tune workflow config only for
one particular OS.
 
Best regards,
Maxim Kokryashkin
 

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

Re: [Tarantool-patches] [PATCH luajit] ci: introduce GitHub Actions

[Igor Munkin via Tarantool-patches]

Max,

Thanks for your review!

On 02.03.22, Maxim Kokryashkin wrote:
> 
> Hi!
>  
> Thanks for the patch!
> LGTM, except for comments by Sergey and a few of mine:

Added your tag:
| Reviewed-by: Maxim Kokryashkin <m.kokryashkin@tarantool.org>

> >
> >> + # 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
> IMO, it’s better to use a combination of `github.sha` + `github.event`,
> since all you want is to suppress cancelation caused by tag push.
> It is essentially the same, but at least it preserves the semantics.

I use the same recipe as in Tarantool workflows, so I leave it
unchanged. Anyway, I saved your inputs for our DevX team, thanks!

> >
> >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. :)
> I think that in our case it is actually better to have a lot of workflow configs.
> From time to time we encounter some issues only on certain operating
> systems, so it is great to have the ability to fine-tune workflow config only for
> one particular OS.

Such cases should be handled in tests, not in workflows (consider
Tarantool workflow using LuaJIT tests), so I've left one workflow per
each supported OS (i.e. Linux and macOS). Ignoring.

>  
> Best regards,
> Maxim Kokryashkin
>  

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit] ci: introduce GitHub Actions

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!

Thanks for the fixes!
LGTM.

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] ci: introduce GitHub Actions

[Igor Munkin via Tarantool-patches]

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

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
> 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.
> 
> Signed-off-by: Igor Munkin <imun@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
> 

<snipped>

> -- 
> 2.34.0
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit] ci: introduce GitHub Actions

[Igor Munkin via Tarantool-patches]

Hey, it's me again.

On 02.03.22, Igor Munkin wrote:
> Max,
> 
> Thanks for your review!
> 

<snipped>

> > >
> > >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. :)
> > I think that in our case it is actually better to have a lot of workflow configs.
> > From time to time we encounter some issues only on certain operating
> > systems, so it is great to have the ability to fine-tune workflow config only for
> > one particular OS.
> 
> Such cases should be handled in tests, not in workflows (consider
> Tarantool workflow using LuaJIT tests), so I've left one workflow per
> each supported OS (i.e. Linux and macOS). Ignoring.

Well, I was wrong, you were right -- there was a big mistake to merge
workflows for x86_64 and aarch64. At first, there is no support for the
latter in 1.10 and 2.8. Furthermore, macOS runners on m1 are using
Rosetta for Github Actions' workflows, so we need use `arch -arm64`
prefix for each command and I've failed to make it respect <matrix.ARCH>
variable.

So, I've reverted the related changes. Again, thanks for your inputs!

> 
> >  
> > Best regards,
> > Maxim Kokryashkin
> >  
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
IM