From c911d904ee865c46d66fdbd0c4a6e6c08244a19e Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Fri, 17 May 2024 10:12:52 +0800 Subject: [PATCH 1/3] github: extract test.yaml into a resusable workflow github's matrix machinery does not allow us to attach extra elements to an existing matrix, for instance, we cannot add the combination of `{compiler: clang-18, standard: 23, mode: release, enables: dpdk}` to a matrix of `{compiler: [clang-18, gcc-13], mode: [debug, release], standard: [20, 23]}`, without overwriting the combination of `{compiler: clang-18, standard: 23, mode: release}` generated by the matrix above. as the combination with "enables: dpdk" matches with it. to address this issue, we use the approach that we were using when defining the circleci workflow: to define a parameterized workflow, so that we can customize the behavior of the caller workflow using either a matrix or a hardwired combination. this change just extract the reuseable workflow out, and call it in the parent workflow. we will replace the unrolled `inputs` list with 3 different jobs, with which we will have - 12 combinations generated with the matrix - 1 combination for dpdk build - 1 combination for C++20 build instead of: - 10 combinations - 1 combination for dpdk build - 1 combination for C++20 build Signed-off-by: Kefu Chai --- .github/workflows/test.yaml | 84 ++++++++++++++++++++++++++++++++++++ .github/workflows/tests.yaml | 57 +++--------------------- 2 files changed, 91 insertions(+), 50 deletions(-) create mode 100644 .github/workflows/test.yaml diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml new file mode 100644 index 00000000000..bf90f0696f1 --- /dev/null +++ b/.github/workflows/test.yaml @@ -0,0 +1,84 @@ +name: Test + +permissions: + contents: read + +on: + workflow_call: + inputs: + compiler: + # only the compilers supported by setup-cpp can be used + description: 'the C++ compiler to use' + type: string + required: true + standard: + description: 'the C++ standard to use' + type: number + required: true + mode: + description: 'build mode (debug, dev or release)' + type: string + required: true + enables: + description: 'the --enable-* option passed to configure.py' + type: string + default: '' + required: false + options: + description: 'additional options passed to configure.py' + type: string + default: '' + required: false + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + submodules: "${{ contains(inputs.enables, 'dpdk') }}" + + - name: Install build dependencies + run: | + sudo ./install-dependencies.sh + + - name: Install ${{ inputs.compiler }} + uses: aminya/setup-cpp@master + with: + compiler: ${{ inputs.compiler }} + ccache: true + # ubuntu:latest comes with CMake 3.29, so we just need to install + # ninja. see + # https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md#tools + ninja: "${{ contains(inputs.enables, 'cxx-modules') }}" + + - name: Setup ccache + uses: hendrikmuhs/ccache-action@v1 + with: + key: ${{ inputs.compiler }}-${{ inputs.standard }}-${{ inputs.mode }}-${{ inputs.enables }} + + - name: Configure + run: > + ./configure.py + --ccache + --c++-standard ${{ inputs.standard }} + --compiler $CXX + --c-compiler $CC + --mode ${{ inputs.mode }} + ${{ inputs.options }} + ${{ inputs.enables }} ; + + - name: Build + run: cmake --build build/${{inputs.mode}} + + - name: Check Header + if: ${{ inputs.mode == 'dev' && inputs.compiler == 'clang++-18' }} + run: cmake --build build/${{ inputs.mode }} --target checkheaders + + - name: Build with C++20 modules + if: ${{ contains(inputs.enables, 'cxx-modules') }} + run: cmake --build build/${{ inputs.mode }} --target hello_cxx_module + + - name: Test + if: ${{ ! contains(inputs.enables, 'cxx-modules') }} + run: ./test.py --mode=${{ inputs.mode }} diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 90748f660cc..84d2a6089a3 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -12,7 +12,7 @@ concurrency: jobs: test: name: "Tests (${{ matrix.compiler }}, C++${{ matrix.standard}}, ${{ matrix.mode }}, ${{ matrix.enables }})" - runs-on: ubuntu-latest + uses: ./.github/workflows/test.yaml strategy: fail-fast: false matrix: @@ -66,52 +66,9 @@ jobs: standard: 23 mode: debug enables: --enable-cxx-modules - steps: - - uses: actions/checkout@v4 - with: - submodules: "${{ contains(matrix.enables, 'dpdk') }}" - - - name: Install build dependencies - run: | - sudo ./install-dependencies.sh - - - name: Install ${{ matrix.compiler }} - uses: aminya/setup-cpp@master - with: - compiler: ${{ matrix.compiler }} - ccache: true - # ubuntu:latest comes with CMake 3.29, so we just need to install - # ninja. see - # https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md#tools - ninja: "${{ contains(matrix.enables, 'cxx-modules') }}" - - - name: Setup ccache - uses: hendrikmuhs/ccache-action@v1 - with: - key: ${{ matrix.compiler }}-${{ matrix.standard }}-${{ matrix.mode }}-${{ matrix.enables }} - - - name: Configure - run: > - ./configure.py - --ccache - --c++-standard ${{ matrix.standard }} - --compiler $CXX - --c-compiler $CC - --mode ${{ matrix.mode }} - ${{ matrix.options }} - ${{ matrix.enables }} ; - - - name: Build - run: cmake --build build/${{matrix.mode}} - - - name: Check Header - if: ${{ matrix.mode == 'dev' && matrix.compiler == 'clang++-18' }} - run: cmake --build build/${{ matrix.mode }} --target checkheaders - - - name: Build with C++20 modules - if: ${{ contains(matrix.enables, 'cxx-modules') }} - run: cmake --build build/${{ matrix.mode }} --target hello_cxx_module - - - name: Test - if: ${{ ! contains(matrix.enables, 'cxx-modules') }} - run: ./test.py --mode=${{ matrix.mode }} + with: + compiler: ${{ matrix.compiler }} + standard: ${{ matrix.standard }} + mode: ${{ matrix.mode }} + enables: ${{ matrix.enables }} + options: ${{ matrix.options }} From 5a9565514fe9d1697c563678d31a0a2e45b728af Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Fri, 17 May 2024 10:40:28 +0800 Subject: [PATCH 2/3] github: add 2 testing combinations back to the matrix this change adds the combinations of - {compiler: clang++-18, standard: 23, mode: release} - {compiler: clang++-18, standard: 23, mode: debug} back. before this change, the `include` keyword instructs github workflow to override these two combinations with the ones which builds with dpdk and with C++20 modules. this hurts the coverage of the test matrix. after this change, instead of using `include` keyword, we just add two more jobs which are dedicated for testing these two combinations without overriding the existing ones. Signed-off-by: Kefu Chai --- .github/workflows/tests.yaml | 71 ++++++++++++------------------------ 1 file changed, 23 insertions(+), 48 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 84d2a6089a3..aafba28a9d7 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -10,8 +10,8 @@ concurrency: cancel-in-progress: ${{ startsWith(github.ref, 'refs/pull/') }} jobs: - test: - name: "Tests (${{ matrix.compiler }}, C++${{ matrix.standard}}, ${{ matrix.mode }}, ${{ matrix.enables }})" + regular_test: + name: "Test (${{ matrix.compiler }}, C++${{ matrix.standard}}, ${{ matrix.mode }})" uses: ./.github/workflows/test.yaml strategy: fail-fast: false @@ -20,55 +20,30 @@ jobs: compiler: [clang++-18, gcc-13] standard: [20, 23] mode: [dev, debug, release] - include: - - compiler: clang++-18 - standard: 20 - mode: dev - - compiler: clang++-18 - standard: 20 - mode: debug - - compiler: clang++-18 - standard: 20 - mode: release - - compiler: clang++-18 - standard: 23 - mode: dev - - compiler: clang++-18 - standard: 23 - mode: debug - - compiler: clang++-18 - standard: 23 - mode: release - - compiler: gcc-13 - standard: 20 - mode: dev - - compiler: gcc-13 - standard: 20 - mode: debug - - compiler: gcc-13 - standard: 20 - mode: release - - compiler: gcc-13 - standard: 23 - mode: dev - - compiler: gcc-13 - standard: 23 - mode: debug - - compiler: gcc-13 - standard: 23 - mode: release - - compiler: clang++-18 - standard: 23 - mode: release - options: --cook dpdk --dpdk-machine haswell - enables: --enable-dpdk - - compiler: clang++-18 - standard: 23 - mode: debug - enables: --enable-cxx-modules with: compiler: ${{ matrix.compiler }} standard: ${{ matrix.standard }} mode: ${{ matrix.mode }} enables: ${{ matrix.enables }} options: ${{ matrix.options }} + build_with_dpdk: + name: "Test with DPDK enabled" + uses: ./.github/workflows/test.yaml + strategy: + fail-fast: false + with: + compiler: clang++-18 + standard: 23 + mode: release + enables: --enable-dpdk + options: --cook dpdk --dpdk-machine haswell + build_with_cxx_modules: + name: "Test with C++20 modules enabled" + uses: ./.github/workflows/test.yaml + strategy: + fail-fast: false + with: + compiler: clang++-18 + standard: 23 + mode: debug + enables: --enable-cxx-modules From 28b05551f605a0f772ea08d6eed054b6758e5b35 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Fri, 17 May 2024 11:41:29 +0800 Subject: [PATCH 3/3] github: use fedora:40 image for testing instead of using ubuntu:jammy and setup-cpp action for prepearing the building toolchain, use fedora:40 container for building and testing. after switching to the github workflow based CI, we've been seeing test failures due to networking issue: ``` Failed to install llvm via system package manager Error: Command failed with exit code 35: curl -LJO https://apt.llvm.org/llvm.sh % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 0 0 0 0 0 0 0 0 --:--:-- 0:04:19 --:--:-- 0 curl: (35) OpenSSL SSL_connect: Broken pipe in connection to apt.llvm.org:443 ``` since fedora 40 comes with all the dependencies we need, let's build and test in a container with the fedora:40 image. with, hopefully the better CDN of the docker, and more reliable mirrors of fedora repositories, and the package retrievial machinary built into fedora's package management tools, we should have a more resilient CI. please note, in this change, we also * install git before checkout the repo. the reason is that, unlike the github-hosted runner, the fedora:40 image does not have `git` installed, so we have to install it manually before using "actions/checkout" action. * install clang-tools-extra when building with C++ modules enabled, because cmake and clang depend on clang-scan-deps to analyze the dependencies in betweener of C++20 modules. * use static library in "dev" build mode. this is to work around the issue where seastar allocator causes infinite recursive call if seastar is compiled as a shared library. this only happens when the tree is compiled with newer glibc. see also #2247 Signed-off-by: Kefu Chai --- .github/workflows/test.yaml | 51 ++++++++++++++++++++++-------------- .github/workflows/tests.yaml | 7 +++-- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index bf90f0696f1..f9f8b5083b2 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -7,7 +7,6 @@ on: workflow_call: inputs: compiler: - # only the compilers supported by setup-cpp can be used description: 'the C++ compiler to use' type: string required: true @@ -33,7 +32,12 @@ on: jobs: test: runs-on: ubuntu-latest + container: fedora:40 steps: + - name: Install Git + run: | + sudo dnf -y install git + - uses: actions/checkout@v4 with: submodules: "${{ contains(inputs.enables, 'dpdk') }}" @@ -42,15 +46,19 @@ jobs: run: | sudo ./install-dependencies.sh - - name: Install ${{ inputs.compiler }} - uses: aminya/setup-cpp@master - with: - compiler: ${{ inputs.compiler }} - ccache: true - # ubuntu:latest comes with CMake 3.29, so we just need to install - # ninja. see - # https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md#tools - ninja: "${{ contains(inputs.enables, 'cxx-modules') }}" + - name: Install clang++ + if: ${{ inputs.compiler == 'clang++' }} + run: | + sudo dnf -y install clang + + - name: Install clang-scan-deps + if: ${{ contains(inputs.enables, 'cxx-modules') }} + run: | + sudo dnf -y install clang-tools-extra + + - name: Install ccache + run: | + sudo dnf -y install ccache - name: Setup ccache uses: hendrikmuhs/ccache-action@v1 @@ -58,15 +66,20 @@ jobs: key: ${{ inputs.compiler }}-${{ inputs.standard }}-${{ inputs.mode }}-${{ inputs.enables }} - name: Configure - run: > - ./configure.py - --ccache - --c++-standard ${{ inputs.standard }} - --compiler $CXX - --c-compiler $CC - --mode ${{ inputs.mode }} - ${{ inputs.options }} - ${{ inputs.enables }} ; + run: | + if [ ${{ inputs.compiler }} = "clang++" ]; then + CC=clang + else + CC=gcc + fi + ./configure.py \ + --ccache \ + --c++-standard ${{ inputs.standard }} \ + --compiler ${{ inputs.compiler }} \ + --c-compiler $CC \ + --mode ${{ inputs.mode }} \ + ${{ inputs.options }} \ + ${{ inputs.enables }} - name: Build run: cmake --build build/${{inputs.mode}} diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index aafba28a9d7..5debb533902 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -16,8 +16,7 @@ jobs: strategy: fail-fast: false matrix: - # only the compilers supported by setup-cpp - compiler: [clang++-18, gcc-13] + compiler: [clang++, g++] standard: [20, 23] mode: [dev, debug, release] with: @@ -32,7 +31,7 @@ jobs: strategy: fail-fast: false with: - compiler: clang++-18 + compiler: clang++ standard: 23 mode: release enables: --enable-dpdk @@ -43,7 +42,7 @@ jobs: strategy: fail-fast: false with: - compiler: clang++-18 + compiler: clang++ standard: 23 mode: debug enables: --enable-cxx-modules