From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 1EFE36E454; Wed, 2 Mar 2022 10:55:15 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 1EFE36E454 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1646207715; bh=q/oQIyVEan0eD4q4KRzOOGzfhy5Jy1Z5Qnw9qm6aGc4=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=et0NqEhx+EueWoilnLjtOrFaVurnhSFKyArsql1iYKVG9Icl8hUrey4fOHjexD4qn 5rxFrjWJhY0lV1bVBn0o4zNXZZk7AbQ7jmVWaPE8awteejozVOexhFMxUJ81r+WAYa d+5yCACLL4zWQKqTx8uSTXQX0q6WHVFr8ecxTYiI= Received: from smtpng3.i.mail.ru (smtpng3.i.mail.ru [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 014C96E454 for ; Wed, 2 Mar 2022 10:55:13 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 014C96E454 Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1nPJpJ-0004Xh-0H; Wed, 02 Mar 2022 10:55:13 +0300 Date: Wed, 2 Mar 2022 10:53:05 +0300 To: Igor Munkin Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD9C7814344C8C501C8D444D897409990D29C6C8F8CDCE90E97182A05F538085040E18EB0C010E9881198DB01AFF5EF294EA053D66A6B00CF33F7A21FBA0C770DDE X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE704BA85F3D5A9F85BEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637457C2806CBF873EDEA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BEBC5CAB6D411FFA691217C0219039560585518695C9A4EDECC7F00164DA146DAFE8445B8C89999728AA50765F790063783E00425F71A4181389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC82FFDA4F57982C5F4F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8BDCE939D40DBB93CA75ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-8FC586DF: 6EFBBC1D9D64D975 X-C1DE0DAB: 0D63561A33F958A5C91A5D0F750EC116B6D10490F703066ABDCEBA6498EE1E4FD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75F78D6440C3F49C15410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34A9A0A0BF1A2CAC62BB801117B4E4A5CA1A4E1246C7DEAC1C21AAAC4E6CD43345F6E4E920603B7C6F1D7E09C32AA3244C475E8FF428115275C01B2730176F25393A92A9747B6CC886FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojEE37XCRm9PDXQjM9ZApOuQ== X-Mailru-Sender: 689FA8AB762F739339CABD9B3CA9A7D6E504C0AC9C3A0C8A35C319E89AFB70590FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] ci: introduce GitHub Actions X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "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 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 > --- > .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 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)" > + > + > +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 @@ 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