Re: [Tarantool-patches] [PATCH luajit 2/3] ci: introduce workflow for exotic builds

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergos!

Thanks for the review!

On 27.01.23, sergos wrote:
> Hi!
> 
> Thanks for the patch!
> 
> Some question on build options.
> 
> Sergos
> 
> > On 18 Jan 2023, at 23:16, Sergey Kaplun <skaplun@tarantool.org> wrote:
> > 
> > This workflow is well-suited for test specific builds like dual-number
> > mode on x86_x64, build with disabled JIT or FFI, etc.
> > 
> > For now, just the dual-number mode is added, due to the need to test
> > future commits. If there is need to add a new build type `FLAVOR` column
>                              ^                            ^---------
>                              a                           then(?) the
> 
> > of CI matrix should be exposed. Also, just Linux x86_64 is tested,
>                         extended?      xxxxx                      ^ only
> 

Thanks, fixed!
The new commit message is the following:

| ci: introduce workflow for exotic builds
|
| This workflow is well-suited for test specific builds like dual-number
| mode on x86_x64, build with disabled JIT or FFI, etc.
|
| For now, just the dual-number mode is added, due to the need to test
| future commits. If there is a need to add a new build type, then the
| `FLAVOR` column of CI matrix should be extended. Also, only Linux x86_64
| is tested, because we want to decrease resource usage in CI.

> > because we want to decrease resource usage in CI.
> > ---
> > .github/workflows/exotic-builds-testing.yml | 71 +++++++++++++++++++++
> > 1 file changed, 71 insertions(+)
> > create mode 100644 .github/workflows/exotic-builds-testing.yml
> > 
> > diff --git a/.github/workflows/exotic-builds-testing.yml b/.github/workflows/exotic-builds-testing.yml
> > new file mode 100644
> > index 00000000..cd0c14d7
> > --- /dev/null
> > +++ b/.github/workflows/exotic-builds-testing.yml
> > @@ -0,0 +1,71 @@
> > +name: "Exotic builds testing"
> > +
> > +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-x86_64-exotic:
> > +    strategy:
> > +      fail-fast: false
> > +      matrix:
> > +        BUILDTYPE: [Debug, Release]
> > +        GC64: [ON, OFF]
> > +        FLAVOR: [dualnum]
> > +        include:
> > +          - BUILDTYPE: Debug
> > +            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
> > +          - BUILDTYPE: Release
> > +            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> 
> Shall we add the -DLUAJIT_ENABLE_CHECKHOOK and -DLUAJIT_SMART_STRINGS=1 since they are in Tarantool by default?

-DLUAJIT_SMART_STRINGS=1 is set by default, so it's OK not to set it
mannually.

OTOH, -DLUAJIT_ENABLE_CHECKHOOK is not set by default.
I prefer not to set it for this particular workflow, because I plan to
use it for very specific builds (even impossible for Tarantool) for
example with -DLUAJIT_DISABLE_JIT=ON, -DLUAJIT_DISABLE_FFI=ON flags.
Also, this flag is not set for default LuaJIT testing, so I tried to
make those workflows look similar.
But if we want to add this flag for default build or testing we should
add it for this workflow too.
CC-ed Igor to hear his opinion.

> 
> > +          - FLAVOR: dualnum
> > +            FLAVORFLAGS: -DLUAJIT_NUMMODE=2
> > +    runs-on: [self-hosted, regular, Linux, x86_64]
> > +    name: >
> > +      LuaJIT ${{ matrix.FLAVOR }}
> > +      (Linux/x86_64)
> > +      ${{ matrix.BUILDTYPE }}
> > +      GC64:${{ matrix.GC64 }}
> > +    steps:
> > +      - uses: actions/checkout@v2.3.4
> > +        with:
> > +          fetch-depth: 0
> > +          submodules: recursive
> > +      - name: setup Linux
> > +        uses: ./.github/actions/setup-linux
> > +      - name: configure
> > +        run: >
> > +          cmake -S . -B ${{ env.BUILDDIR }}
> > +          -G Ninja
> > +          ${{ matrix.CMAKEFLAGS }}
> > +          ${{ matrix.FLAVORFLAGS }}
> > +          -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 }}
> > -- 
> > 2.34.1
> > 
> 

-- 
Best regards,
Sergey Kaplun