github: build with fedora 40 and add 2 build combinations back #2248
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
a sample run can be found at https://github.com/tchaikov/seastar/actions/runs/9123831484/job/25086931644 . which tests a branch which includes both the this PR and #2246 |
|
@nyh hi Nadav, in case you are interested in this. |
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 <[email protected]>
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 <[email protected]>
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 scylladb#2247 Signed-off-by: Kefu Chai <[email protected]>
|
@scylladb/seastar-maint hello maintainers, could you help review this change? |
|
the test failure due to "Error: Failed to install the clang++-18" was spotted again at https://github.com/scylladb/seastar/actions/runs/9169644191/job/25210438413?pr=2256 |
nyh
approved these changes
May 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
in order to address the infra issues caused by unreliable connection to llvm's apt repo, and to bring the two missing build combinations back, in this series, we switch from
setup-cppaction to fedora 40, and extract the test job into a reusable workflow.