Skip to content
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

Changed toolchain to stable for coverage #1987

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Raghav-Bell
Copy link

It fixes #1902

changed nightly to stable tool chain in ci.yml for coverage.
Copy link

codecov bot commented Mar 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.80%. Comparing base (ee1e217) to head (6d493b1).
Report is 64 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1987      +/-   ##
==========================================
+ Coverage   96.32%   96.80%   +0.48%     
==========================================
  Files         137      143       +6     
  Lines       20704    20434     -270     
  Branches      226      226              
==========================================
- Hits        19943    19782     -161     
+ Misses        728      618     -110     
- Partials       33       34       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@briansmith
Copy link
Owner

From the CI run:

qemu: uncaught target signal 11 (Segmentation fault) - core dumped
/home/runner/work/ring/ring/mk/runner: line 21:  6499 Segmentation fault      (core dumped) $*
error: test failed, to rerun pass `-p ring --lib`

This kind of crash seems to happen when the clang version doesn't match the LLVM version that Rust uses. It seems like we upgraded clang in CI to what Nightly Rust uses. In order to move forward with this change, we may either need to downgrade to the earlier version of clang that matches stable Rust's LLVM version, or wait until the next stable Rust is released.

You might try using channel beta to see if the beta channel is using the new version of LLVM to get an idea of how long we'd need to wait.

@Raghav-Bell
Copy link
Author

image

@briansmith You are right
stable needs LLVM $\geq$ 16
beta & nightly are compatible with LLVM ==18.
I will push it to beta for now, later we can move it to stable. Thanks

checking beta channel for LLVM compatibility.
@briansmith
Copy link
Owner

@Raghav-Bell Do you want to try again now?

@briansmith
Copy link
Owner

OK, this seems to be working now on stable. Could you please do the following?:

  1. Rebase this on top of the latest main. Your PR branch is 64 commits behind the main branch, so it is hard to compare its effect on code coverage measurement as it is. If you rebase, then we should get a useful report from codecov.io.
  2. Squash the three commits into one.
  3. Improve the commit message. Something like this:

CI: Use stable Rust toolchain for code coverage.

Use the stable Rust toolchain for code coverage in CI. This should make coverage measurements more stable day-to-day and also make it easier for people to replicate coverage measurements on locally.

@briansmith
Copy link
Owner

Since PR #2056 was merged, we'll need to wait for Rust 1.80 (IIUC) to become stable, or else add a wokaround that forces the use of nightly for the armv7 target.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: Use stable Rust for source-based code coverage measurement
2 participants