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 patch and fixes!
Generally LGTM, but please clarify my questions below.

On 06.08.26, Evgeniy Temirgaleev wrote:
> From: Temir Galeev <temir.galeev@bk.ru>
> 
> The arm64 and x86_64 architectures with clang/gcc compiler were added
> to the matrix.
> ---
>  .../README.md                                 |   0
>  .../action.yml                                |   8 +-
>  .../actions/setup-sanitizers-macos/README.md  |  15 +++
>  .../actions/setup-sanitizers-macos/action.yml |  73 +++++++++++++
>  .github/workflows/sanitizers-testing.yml      | 101 ++++++++++++++++--
>  5 files changed, 186 insertions(+), 11 deletions(-)
>  rename .github/actions/{setup-sanitizers => setup-sanitizers-linux}/README.md (100%)
>  rename .github/actions/{setup-sanitizers => setup-sanitizers-linux}/action.yml (76%)
>  create mode 100644 .github/actions/setup-sanitizers-macos/README.md
>  create mode 100644 .github/actions/setup-sanitizers-macos/action.yml
> 
> diff --git a/.github/actions/setup-sanitizers/README.md b/.github/actions/setup-sanitizers-linux/README.md
> similarity index 100%
> rename from .github/actions/setup-sanitizers/README.md
> rename to .github/actions/setup-sanitizers-linux/README.md
> diff --git a/.github/actions/setup-sanitizers/action.yml 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?

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

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


> -- 
> 2.49.0
> 

-- 
Best regards,
Sergey Kaplun