-
Notifications
You must be signed in to change notification settings - Fork 132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: test gcc versions 11 and 12 and llvm version 17 and 18 #541
base: master
Are you sure you want to change the base?
Conversation
Note: #483 comes from a compiler bug known to be fixed in GCC12.4 and GCC13.3 (both of them upcoming releases). Should also be fixed in GCC-14. |
Great! I would suggest we merge this PR with the tests disabled, and we can enable them when we know GCC 12.4 is shipping in Ubuntu repositories. |
Thanks again for the PR! In principle we are never against more testing and it sounds like the way forward, but we need to be conscious of resources and maintenance cost. We are worried that as we increase the number of tested parameters (OS-es, compilers, LTO/no-LTO, shared/static libs, ...) the workflow will progressively become unmaintainable and possibly exceed github runners quotas. |
Hi! Sorry to keep you waiting here. We might be able to integrate tests for more compilers if we re-organise and re-parametrise workflows in such a way that expensive workflows like test-cross are only triggered periodically (as opposed to on commit). That means non-native tests will happen less often, but can be made more thorough (more os, compilers, configurations, ...). Let's try to merge this PR first and re-organise in the coming weeks. |
build-native: | ||
build-x86_64: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets keep the original name build-native
, I think it's fairly clear that this is x86 in most workflows. We are going to get aarch64 runners by the end of the year, so we need a generic name.
arch: [aarch64, armhf, ppc64el, s390x, riscv64] | ||
compiler: [gcc, llvm] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a shame to lose this to a very long list that is difficult to read/maintain, and it gets worse for test-cross.
This looks quite drastic for simply changing the compiler version.
Does it make it easier to include the versions in the compiler
variable directly and keep a structure similar to the previous one, that seemed more contained?
It might not be very idiomatic but can we filter non-functional compilers and versions in a different way that doesn't involve (just) includes and excludes, maybe with basic conditionals and additional steps?
If not is there any better way to display this list of includes/excludes?
aarch64-gcc-12, ppc64el-gcc-12, and s390x-gcc-12 are not enabled as they are failing with #483