-
Notifications
You must be signed in to change notification settings - Fork 96
Upgrade cxx dependency from 1.0.144 to 1.0.176 #1393
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1393 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 75 75
Lines 13124 13124
=========================================
Hits 13124 13124 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Cargo.toml
Outdated
| cxx = "1.0.192" | ||
| cxx-build = { version = "1.0.192", features = [ "parallel" ] } |
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.
think the first version of CXX with that fix would be 1.0.161? dtolnay/cxx@8531c59
As wonder if that version will work with the existing MSRV
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.
1.0.144 (current version) requires Rust 1.73, and the latest version of cxx to require 1.73 is 1.0.176. So I'll set it to that instead.
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.
Huh. @ahayzen-kdab That didn't work, but only because of a subdependency:
package
indexmap v2.13.0 cannot be built because it requires rustc 1.82 or newer, while the currently active rustc version is 1.77.2
However, cxx only requires indexmap v2.9.0. This makes me wonder if the real culprit here is requiring the GitHub runners to use the Cargo.lock file, and indeed having a lockfile at all rather than gitignoring it. I think the runners would otherwise select the latest version of the library compatible with their Rust version? I'm not sure, though.
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.
No, they don't, and specifically they get stuck because they don't have Rust Edition 2024. Which leaves me confused as to how someone on a much lower version of Rust would be able to add this library to a project in the first place, because I think it would try to install subdependencies that required Rust 2024, right? I think I'm out of my depth here.
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.
So the reason for the lockfile in git and CI is that we need to pin the dependencies to the same version otherwise caches aren't hit and CI takes ages :-D
So we would need to figure out a way for this to work with the lockfile. It might be that locally you need to manually run some cargo commands to "install" the exact right version of things so that the lockfile happens to be taken at the moment when everything was the right version.
Regarding Rust editions i thought you could mix and match between dependencies, but is the problem more that the compiler is too old ? ... at some point we'll need to raise the MSRV but if it's possible to somehow keep it and move CXX along a bit then that would be best.
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.
This is the error I was seeing:
The package requires the Cargo feature called
edition2024, but that feature is not stabilized in this version of Cargo (1.77.2 (e52e36006 2024-03-26))
So the issue is that the user (or the CI in this case) goes to install dependencies, but some of those (sub)dependencies require the 2024 Edition, which requires a higher MSRV. So it seems like it would be very difficult for someone to be developing in Rust at all these days if they were still stuck at 1.77.2.
e289f53 to
4dfbe96
Compare
Currently, the latest version of Rust emits this warning on every method binding created by cxx-qt:
This is caused by a mistake in the cxx library that was subsequently fixed, so upgrading the dependency resolves the issue.
Note: cxx cannot be upgraded to its latest version (1.0.192) because that requires the Rust version to be at least 1.82.