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

README: document more building tricks #270

Merged
merged 6 commits into from
Jul 2, 2023
Merged

README: document more building tricks #270

merged 6 commits into from
Jul 2, 2023

Conversation

woodruffw
Copy link
Member

No description provided.

@woodruffw woodruffw self-assigned this Jul 2, 2023
woodruffw added 4 commits July 2, 2023 10:32
We don't support 1.68+ yet, due to their use of LLVM 16.

See: #267.

Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Copy link
Member Author

Choose a reason for hiding this comment

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

This is unfortunate, but IMO produces a better user experience than our current one: the current UX is that cargo build or cargo install will complete successfully, but will fail to analyze any IR produced by versions of rustc newer than 1.67.

Comment on lines 88 to 92
let rsup_default = Command::new("rustup")
.arg("default")
.args(["show", "active-toolchain"])
.output()
.expect("failed to run rustup to configure toolchain");
let utf8_toolchain = std::str::from_utf8(&rsup_default.stdout)
Copy link
Member Author

Choose a reason for hiding this comment

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

We now use rustup show active-toolchain to determine the toolchain to use. This should be slightly more reliable than rustup default, in the sense that it'll both respect user overrides and should (?) respect the local rust-toolchain.toml in any crate.

Copy link
Collaborator

@smoelius smoelius left a comment

Choose a reason for hiding this comment

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

I can't find anything in this to disagree with.

@woodruffw woodruffw merged commit c17b049 into master Jul 2, 2023
@woodruffw woodruffw deleted the ww/docs branch July 2, 2023 18:33
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.

2 participants