Drop llvm 19 support#811
Conversation
mcbarton
force-pushed
the
remove-llvm-19-support
branch
from
6639a95 to
a4fc6ef
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #811 +/- ##
=======================================
Coverage 81.67% 81.67%
=======================================
Files 13 13
Lines 4928 4928
=======================================
Hits 4025 4025
Misses 903 903
🚀 New features to boost your workflow:
|
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
To be squashed an merged in after the next CppInterOp release to free up some space in the cache. If I can get approval I will merge after the release. |
| @@ -1,384 +1,352 @@ | |||
| name: Native Builds | |||
There was a problem hiding this comment.
Why did we end up rewriting this entire file?
There was a problem hiding this comment.
This commit on main ab4ca85 has caused nothing but issues with diffs (never understood why it got merged in to begin with, since it had nothing to do with the PR it got merged in with https://github.com/compiler-research/CppInterOp/pull/827/commits). When fixing the merge conflict, I chose main.yml on main, and removed the llvm 19 jobs. No idea why it is showing such a big diff.
There was a problem hiding this comment.
Not sure if you use Windows for development, but that file was CRLF encoded. The diff is not in the characters themselves but the encoding. Please ensure this PR does not re-introduce that.
There was a problem hiding this comment.
The ai may be explaining why you chose to make that change to the yml file, but what has this change had to do with Use dated ROOT/llvm tag to build Cling 1.3? it had nothing to do with that. It was a very disruptive commit which causes all other PRs editing the ci to have a merge conflict.
Again, I don't know why the big diff for that file. I used Github to fix the merge conflict. I used main.yml as it was, and just removed the llvm 19 jobs. That might say Githubs default is CRLF maybe?
There was a problem hiding this comment.
Now everytime someone contributes the ci from a Windows platform (or uses Github since that is where I made this change) which still uses /r/n we just supposed to say sorry, we cannot have your change. Seems very strange.
aaronj0
left a comment
aaronj0
left a comment
There was a problem hiding this comment.
Please revert CRLF encoding on main.yml
|
Should we land the llvm22 support first so that we always support at least 3 versions? |
I can get the support for llvm 22 for the non oop jobs relatively quickly, depending on how disruptive this missing header error turns out to be #812 (comment) , which was introduced by the cuda work. |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
For some reason the CI decided to rebuild caches and I've cancelled the builds as they seem irrelevant for what this PR is doing. |
3 participants
We plan to only support 3 llvms at any one time. This PR is not needed right now, but is preparation for when we get llvm 22 support. Setting to draft until CppInterOp is compatible with llvm 22.