-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add a basic rust-toolchain.toml #176
base: master
Are you sure you want to change the base?
Conversation
Matches the one in Zebra, although I think pinning to a specific release (e.g., `channel = "1.82"`) would help with consistency, perhaps avoiding issues like ZcashFoundation#174 (comment)
Ah, good. This captures that the build failures are due to rustc 1.81, and not anything in #174. And also a reason to pin to a specific rustc version – changes outside of the repo shouldn’t cause a working repo to start failing. |
2a75da1
to
62a3d09
Compare
a07c8e7
to
c1eb1e4
Compare
[actions-rs/toolchain doesn’t support TOML-formatted rust-toolchain files](actions-rs/toolchain#126), but it’s unnecessary anyway. - actions-rs/cargo will pick up the rust-toolchain.toml, so we usually don’t need to mention the toolchain at all; - the Windows build just runs `rustup target add x86_64-pc-windows-msvc` directly; and - where we want to build with multiple toolchains (in a matrix), there are some slightly-awkward conditionals. This also makes some other changes: - `fail-fast` is disabled because it hides useful & distinct build results; and - `rustup component add` for clippy and rustfmt are removed because they’re in the rust-toolchain.toml toolchain, and we want to make sure they are, so that they’re available to developers.
c1eb1e4
to
b4ccb08
Compare
@conradoplg The issue from #174 (comment) occurs on master, this PR tries to pin down rustc a bit to avoid it.
So, the problem seems to be fixed with just a bit of waiting … however I can’t find anything obvious in the 1.82 or 1.85 changelogs that would indicate why this started/stopped happening. I’ve currently pinned this to 1.81 (and I would ideally pin to a specific version even if there wasn’t a bug, because it’s better for consistency). Zebra would need to be similarly pinned for it to depend on this before mid-Feb. However (as you can see in the failing check) I think everything would pass if I pinned to 1.85, but
Maybe I can pin to today’s date? |
fcf210a
to
8c332fc
Compare
Newer versions until 1.85 (current nightly) have some breakage wrt C++ linking.
It should match the version in rust-toolchain.toml. Unfortunately, it’s not possible to reference that directly, so this adds comments to remind contributors to update them together.
8c332fc
to
f6857f1
Compare
I understand why this failure is happening. The What’s confusing me now is that the Cargo.lock file pins a version of bindgen (0.69.4) that is too old to generate rustc 1.82-specific code, but one of the CI jobs (“Package (rust_toolchain.toml)”) appears to ignore the Cargo.toml and pull a newer version of bindgen that does support 1.82, which is why that was the only job that was failing. For now, I am setting the |
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.
utACK f6857f1
Matches the one in Zebra, although I think pinning to a specific release (e.g.,
channel = "1.82"
) would help with consistency, perhaps avoiding issues like#174 (comment)