Skip to content
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

Force use of nightly with rust-toolchain.toml #1

Merged
merged 4 commits into from
Aug 22, 2023

Conversation

morr0ne
Copy link
Contributor

@morr0ne morr0ne commented Aug 21, 2023

This pr adds a rust-toolchain.toml file that ensures the project is compiled using rust nightly.

@morr0ne
Copy link
Contributor Author

morr0ne commented Aug 21, 2023

The ci was failing because rustfmt was missing, I added it to the toolchain components. Everything should pass now

@morr0ne
Copy link
Contributor Author

morr0ne commented Aug 21, 2023

I am not sure why it's failing, maybe it wants every component to be specified?

@sunfishcode
Copy link
Owner

I'm not sure. Does it need a targets = declaration, to declare the set of targets we test?

@morr0ne
Copy link
Contributor Author

morr0ne commented Aug 21, 2023

I think I may have found the issue, the ci manually adds rust-src as a component. When it runs the build then it already has a toolchain and just install rustfmt. When its time to build it only finds rustfmt and rust-src but no core or std. The solution i think is to specify all components, including rust-src and remove that step from the ci

@morr0ne
Copy link
Contributor Author

morr0ne commented Aug 21, 2023

As I suspected everything now works. However it is using a pinned version of nightly in both rust-toolchain.toml and the ci. I don't think that's a good idea as they would need to be kept in sync. I think a better approach would be to just pin it in the rust-toolchain.toml fail and let the ci pick the version from there.

@morr0ne
Copy link
Contributor Author

morr0ne commented Aug 21, 2023

Looking more closely at the ci I feel like it could be refactored further but that is def the place for another pr. This can be merged as is.

@sunfishcode
Copy link
Owner

Sounds good.

@sunfishcode sunfishcode merged commit 6319a83 into sunfishcode:main Aug 22, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants