-
Notifications
You must be signed in to change notification settings - Fork 281
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
Fix ci #705
Fix ci #705
Conversation
This is a legacy constant and it's better to just use `i32::MAX`. Note that one cannot `use` an associated constant so this just removed the import. This is better anyway since it's only used once and it didn't provide meaningful line length reduction.
Rust is now checking cfg attributes for typos but this interferes with our cfgs that rustc/cargo don't recognize. This whitelists them so they no longer produce warnings.
The newest nightly stabilized `PanicMessage` with a slightly different API. This updates the API and removes the `#![feature()]` attribute.
Despite using the `#![feature()]` attribute rustc still warns about it being unstable. Changing it to `libc::abort` gets rid of the annoying message.
CI failure looks like rust-lang/cc-rs#852 which we've fixed elsewhere by pinning |
Ah, when we compile the no_std_test using Maybe the simplest thing is to commit |
They also suggest we use resolver v2. I believe that's a more principled approach and given we've bumped MSRV anyway we should use it. |
It looks like that's not it (though updating edition doesn't hurt). |
Previously we had dependency problems that were resolved by resolver v2. We want to activate it just in case it happens again but even better, bump the edition. This was probably forgotten when other crates were migrated.
After trying to enable unwind I got his message:
And indeed, it's referenced in core. I guess we need to recompile it. |
The `no_std` test disables `std`, so unwinding is unsupported, so we use `panic = "abort"` but the `core` library is compiled with unwind by default which breaks the build. Xargo can handle this by recompiling `core` with `panic = "abort"` so we use it.
This can help debug CI issues.
f20c127
to
b0fa740
Compare
Since downgrading isn't helping, I think the windows issue is caused by something using This can be solved by using a custom dockerfile which I'm not keen on doing. |
Cross uses an old image by default and there's a problem that is resolved in the newest wine version, so this commit upgrades the image.
Thanks for digging into this! I had my dates confused and thought that our MSRV was at the beginning of 2018, not the beginning of 2021. But apparently three extra years have happened.. |
I'll go ahead and ACK/merge this since it gets CI passing and is easy to whitelist on my end, but I see one more clippy nit:
which we can address in another PR. |
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.
ACK 33a1893
Updated deprecated item and fixed cfg lints.