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

chore: add channel to rust-toolchain #43

Merged
merged 9 commits into from
Nov 15, 2022

Conversation

dariuszparys
Copy link
Contributor

Fixes #42

Description

The current approach was failing on Windows machines due to possible CRLF issues. The change in the rust-toolchain file allows now to correctly use the provided channel by the rust toolchain without the override command

Dariusz Parys added 2 commits November 15, 2022 08:58

Verified

This commit was signed with the committer’s verified signature.
makenowjust Hiroya Fujinami
The current approach was failing on Windows machines due to possible
CRLF issues. The change in the rust-toolchain file allows now to
correctly use the provided channel by the rust toolchain without
the override command
Updating CI pipeline to read out the value
of the channel key
Copy link
Contributor

@atifaziz atifaziz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might as well use this opportunity to get rid of the legacy name rust-toolchain and rename to rust-toolchain.toml (since we changed the format to TOML). From the docs:

In these cases the toolchain can be named in the project's directory in a file called rust-toolchain.toml or rust-toolchain. If both files are present in a directory, the latter is used for backwards compatibility.

For backwards compatibility, rust-toolchain files also support a legacy format that only contains a toolchain name without any TOML encoding, …

Dariusz Parys added 3 commits November 15, 2022 10:36
As suggested in the PR review renaming rust-toolchain and adjusting
corresponding usage
This reverts commit 101b46f.

It seems that action-rs/toolchain doesn't support .toml as
extension for rust-toolchain files
Update triggers to run pipelines when rust-toolchain
updates
@atifaziz
Copy link
Contributor

atifaziz commented Nov 15, 2022

It seems that action-rs/toolchain doesn't support .toml as extension for rust-toolchain files

Yes, this is an open issue, but do we really need a whole action just to run rustup update (or just rustup show)? I wouldn't keep a file in the wrong format because of the action.

Dariusz Parys added 3 commits November 15, 2022 13:07
As the github-action is not leveraging the toml format of the
rust toolchain, reverting back to the rust-toolchain old format.

Still override is removed from the dockerfile as it doesn't need
to be explicitly called in order to use the toolchain provided in
the file.
Using now rustup show instead of the dedicated action-rs/toolchain
github action. This allows using the .toml extension. Thanks
@atifaziz for pointing out
actions-rs/toolchain#126 (comment)
@dariuszparys
Copy link
Contributor Author

I wouldn't keep a file in the wrong format because of the action.

Yes, I agree. I updated this in 6d476ae

Replaced the remaining github action commands for the rust
toolchain.
@atifaziz atifaziz merged commit b850c22 into eclipse-chariott:main Nov 15, 2022
@dariuszparys dariuszparys deleted the fix/rust-toolchain branch November 15, 2022 13:54
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.

Dev container errors
2 participants