-
Notifications
You must be signed in to change notification settings - Fork 329
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 rust-toolchain file #154
Conversation
This instructs cargo and rustc to use the proper toolchain version, as per https://github.com/rust-lang/rustup#the-toolchain-file, we can make this more specific (following https://github.com/rust-lang/rustup#toolchain-specification) if that turns out useful.
Seems to be working!
However, to help me understand better what exactly are we doing with this PR, two remarks:
Thanks! |
Sorry for not providing enough context and motivation here. btw, I should note that I'm not %100 sure we want this, but it seemed useful to me. MotivationIn general, I believe it is best practice to strive for fully configured dev environments and fully reproducible builds. This improves the security and reliability of software deployment and the velocity of development. The latter is mainly achieved due to reduced cognitive overhead from having to track the system environment and dependency chain in one's head, and reduced friction from "but it works on my system" failures. It was my understanding that this project is being developed on
I would contend that we can and should take advantage of this pinning, even though we are not pinning to nightly! We have a lower bound on rustc stated in the readme, and clear evidence that builds are not reliable on nightly, so I think it would be wise to pin the devenv to a version of the toolchain that is considered standard. This is also follows ANSII-FR's (National Cybersecurity Agency of France) guide to secure rust development:
If we are not specifying the toolchain in the dev env, then we are not doing anything to enforce development on stable, since developers are just left to use whatever happens to work on their machine. Addressing questions
Not if we keep the correct CI configuration. Currently every step explicitly specifies whether it should be run in stable or nightly, and according to the rust-toolchain action's documentation and the evident failure of the nightly check on this PR, that overrides the setting in this file.
No. Using the ANSII-FR recommended practice, devs can easily opt-in to running things on nightly (or some more specific toolchain version), when needed. E.g.: [sf@comp ibc-rs]$ cargo --version
cargo 1.44.1 (88ba85757 2020-06-11)
[sf@comp ibc-rs]$ cargo +nightly --version
cargo 1.46.0-nightly (4f74d9b2a 2020-07-08) |
Wow, very well researched! I guess it's a nice warning/prevention for users with sloppy settings. I just set my cargo default to nightly this morning and indeed I think it's going to be slightly confusing for nightly users but we don't want to implement nightly features as far as I understand. |
This instructs cargo and rustc to use the proper toolchain version, as per https://github.com/rust-lang/rustup#the-toolchain-file, we can make this more specific (following https://github.com/rust-lang/rustup#toolchain-specification) if that turns out useful.
Description
This instructs cargo and rustc to use the proper toolchain version, as
per https://github.com/rust-lang/rustup#the-toolchain-file, we can make
this more specific (following
https://github.com/rust-lang/rustup#toolchain-specification) if that
turns out useful.
For contributor use:
docs/
) and code commentsFiles changed
in the Github PR explorer