-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
github-ci: update Fedora 39 jobs to 41 #12342
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #12342 +/- ##
========================================
Coverage 83.23% 83.24%
========================================
Files 912 912
Lines 257647 258005 +358
========================================
+ Hits 214450 214769 +319
- Misses 43197 43236 +39
Flags with carried forward coverage won't be shown. Click here to find out more. |
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.
There remains a comment cf grep 39 .github/
.github/workflows/builds.yml: # Fedora 39 build using GCC.
But otherwise CI green, good commit message, so ok
@@ -718,7 +718,7 @@ jobs: | |||
zlib-devel | |||
# packaged Rust version has no profiler support built in, so get from rustup | |||
- name: Install Rust | |||
run: curl https://sh.rustup.rs -sSf | sh -s -- --default-toolchain 1.67.1 -y | |||
run: curl https://sh.rustup.rs -sSf | sh -s -- --default-toolchain 1.83 -y |
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.
MSRV still tested other places, so ok
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.
yeah the point here was to make the coverage work. IIRC it needs the rust version to match the OS llvm version somehow. The older version failed, this works 🤷♂️
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.
I wonder if it would makes sense to use the system package here? The Rust RPM appears to depend on llvm-libs, where the rustup version embeds it own. Might be a way to keep them in sync. Or do that next time they are out of sync.
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.
I think rustc and the C compiler need to have the same profraw version INSTR_PROF_RAW_VERSION
(like 9 or 10 today) you can take a look at google/oss-fuzz#12365
@@ -859,10 +859,10 @@ jobs: | |||
- run: src/suricata --build-info | grep -E "Systemd support:\s+yes" &> /dev/null | |||
|
|||
# Fedora 39 build using GCC. |
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.
Comment could be updated to Fedora 41
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.
Ok, comment can be fixed later.
No description provided.