Re: [Tarantool-patches] [PATCH luajit v3 2/3] ci: enabled sanitizer tests for macOS

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

Hi, Evgeniy!
Thanks for the answers!
LGTM, with minor suggestion below.

On 18.08.26, Evgeniy Temirgaleev wrote:
> Hi, Sergey! Thanks for review!
> 
> > 
> > From: Sergey Kaplun <skaplun@tarantool.org>

<snipped>

> > b/.github/actions/setup-sanitizers-linux/action.yml
> > > similarity index 76%
> > > rename from .github/actions/setup-sanitizers/action.yml
> > > rename to .github/actions/setup-sanitizers-linux/action.yml
> > > index 8642d553..18f5a75d 100644
> > > --- a/.github/actions/setup-sanitizers/action.yml
> > > +++ b/.github/actions/setup-sanitizers-linux/action.yml
> > > @@ -20,13 +20,17 @@ runs:
> > > - name: Install build and test dependencies
> > > run: |
> > > apt -y update
> > > + echo Available compilers:
> > > + export CC_FAMILY=`echo ${CC_NAME} | sed 's/-.*$//'`
> > > + apt list | grep -Pe "^${CC_FAMILY}-[0-9]+/"
> > > + # Try to install
> > > apt -y install ${CC_NAME} libstdc++-10-dev cmake ninja-build make perl
> > > shell: bash
> > > env:
> > > CC_NAME: ${{ inputs.cc_name }}
> > > - - name: Set specific C compiler as a default toolchain
> > > + - name: Set specific C compiler as a default toolchain for cmake
> > > run: |
> > > - echo CC=${CC_NAME} | tee -a $GITHUB_ENV
> > > + echo CMAKE_C_COMPILER=${CC_NAME} | tee -a $GITHUB_ENV
> > > shell: bash
> > > env:
> > > CC_NAME: ${{ inputs.cc_name }}
> > 
> > These changes look like debugging of the workflow. Are they necessary?
> > 
> 
> I think it’s a useful information: Which compiler we can select just now with the current runner? For example, it used to answer your question below.
> I added it advisedly, as it done for macOS action.

Got it. Please add the comment and the note about it in the commit
message to avoid confusion.

> 
> > 
> > 
> > > diff --git a/.github/actions/setup-sanitizers-macos/README.md
> > b/.github/actions/setup-sanitizers-macos/README.md
> > > new file mode 100644
> > > index 00000000..9ac5eb38
> > > --- /dev/null
> > > +++ b/.github/actions/setup-sanitizers-macos/README.md
> > 
> > <snipped>
> > 
> > > diff --git a/.github/actions/setup-sanitizers-macos/action.yml
> > b/.github/actions/setup-sanitizers-macos/action.yml
> > > new file mode 100644
> > > index 00000000..9836ea03
> > > --- /dev/null
> > > +++ b/.github/actions/setup-sanitizers-macos/action.yml
> > > @@ -0,0 +1,73 @@
> > 
> > <snipped>
> > 
> > > + - name: Set CMAKE_BUILD_PARALLEL_LEVEL
> > > + shell: bash
> > > + run: |
> > > + # Set CMAKE_BUILD_PARALLEL_LEVEL environment variable to
> > > + # limit the number of parallel jobs for build/test step.
> > > + NPROC=$(sysctl -n hw.logicalcpu 2>/dev/null)
> > > + echo CMAKE_BUILD_PARALLEL_LEVEL=$(($NPROC + 1)) | tee -a $GITHUB_ENV
> > > + - name: Set MACOSX_DEPLOYMENT_TARGERT
> > > + shell: bash
> > > + run: |
> > > + # Set required MACOSX_DEPLOYMENT_TARGERT environment
> > > + # variable for Makefile.original build.
> > > + # See https://github.com/LuaJIT/LuaJIT/issues/484,
> > > + # https://github.com/LuaJIT/LuaJIT/issues/653.
> > > + echo MACOSX_DEPLOYMENT_TARGET=$(sw_vers -productVersion) | tee -a
> > $GITHUB_ENV
> > > + - name: Install build and test dependencies
> > > + shell: bash
> > > + run: |
> > > + # Install brew using the command from Homebrew repository
> > > + # instructions: https://github.com/Homebrew/install.
> > > + # XXX: 'echo' command below is required since brew
> > > + # installation script obliges the one to enter a newline
> > > + # for confirming the installation via Ruby script.
> > > + brew update ||
> > > + echo | /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install
> > )"
> > > + # Try to install the packages either upgrade it to avoid
> > > + # of fails if the package already exists with the previous
> > > + # version.
> > > + brew install --force ${CC_FORMULA} cmake make ninja perl ||
> > > + brew upgrade ${CC_FORMULA} cmake make ninja perl
> > 
> > Can we reuse setup-macos action instead these 3 steps?
> > 
> 
> Yes, we can do it on top of the «ci: unused gcc removed from macOS setup» patch.
> So, we need to merge it first.

Do we? Don't understand why it is required. The gcc instalation and
upgrade looks harmless, IINM.

>                                 And then I shall update this patch.
> Thanks for the notice.
> 
> > 
> > 
> > <snipped>
> > 
> > > diff --git a/.github/workflows/sanitizers-testing.yml
> > b/.github/workflows/sanitizers-testing.yml
> > > index 4bf7d023..fe550b81 100644
> > > --- a/.github/workflows/sanitizers-testing.yml
> > > +++ b/.github/workflows/sanitizers-testing.yml
> > 
> > 
> > <snipped>
> > 
> > > + exclude:
> > 
> > <snipped>
> > 
> > > + - OS: Linux
> > > + CC: gcc-15
> > > + - OS: Linux
> > > + CC: clang-21
> > 
> > May we bump gcc and clang instead for Linux as well (I suppose its
> > about time :)) ?
> > 
> > I suppose it helps to avoid too many excludes. Also, there is no need
> > for testing these specific compilers instead of modern ones in
> > sanitizers build.
> > 
> > If there are any issues when upgrading compilers for sanitizer builds
> > (infrastructure or tests failures) -- feel free to ignore.
> > 
> > <snipped>
> > 
> 
> I tried to use the top versions available for macOS and for Linux. We can’t use the same versions with our current setup.
> 
> With the macOS runner we have:
> Available formulas: ... gcc gcc@10 gcc@11 gcc@12 gcc@13 gcc@14 gcc@15 gcc@9 …
> Available formulas: ... llvm llvm@14 llvm@15 llvm@16 llvm@17 llvm@18 llvm@19 llvm@20 llvm@21 ...
> 
> With the Linux runner we have:
> 
> gcc-10/focal-updates,now 10.5.0-1ubuntu1~20.04 amd64 [installed]
> gcc-7/focal 7.5.0-6ubuntu2 amd64
> gcc-8/focal 8.4.0-3ubuntu2 amd64
> gcc-9/focal-updates,now 9.4.0-1ubuntu1~20.04.2 amd64 [installed,automatic]
> 
> clang-10/focal,now 1:10.0.0-4ubuntu1 amd64 [installed,automatic]
> clang-11/focal-updates,now 1:11.0.0-2~ubuntu20.04.1 amd64 [installed]
> clang-12/focal-updates 1:12.0.0-3ubuntu1~20.04.5 amd64
> clang-18/focal-updates 1:18.1.8-11~20.04.2 amd64
> clang-7/focal 1:7.0.1-12 amd64
> clang-8/focal 1:8.0.1-9 amd64
> clang-9/focal 1:9.0.1-12 amd64
> 
> The clang18 have broken dependencies with the current version of setup-sanitizers-linux. So I leave the current version in the context of ‘fix sanitizers for macOS patch’.

Ignore then. Thanks for the explanation!

> 
> > 
> > 
> > 
> > > --
> > > 2.49.0
> > >
> > 
> > --
> > Best regards,
> > Sergey Kaplun
> > 
> 
> Best regards,
> Evgeniy Temirgaleev

-- 
Best regards,
Sergey Kaplun