-
Notifications
You must be signed in to change notification settings - Fork 207
Update to enable stable toolchain #2194
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
Open
thenewwazoo
wants to merge
2
commits into
oxidecomputer:master
Choose a base branch
from
thenewwazoo:bmatt/stable-toolchain
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,5 @@ | ||
[alias] | ||
xtask = "run --package xtask --" | ||
|
||
[build] | ||
# The purpose of this flag is to block crates using version_detect from "detecting" | ||
# features that are no longer supported by the toolchain, because despite its name, | ||
# version_detect is basically "if nightly { return true; }". This setting gets | ||
# overridden within xtask for Hubris programs, so this only affects host tools like | ||
# xtask. | ||
rustflags = ["-Zallow-features=proc_macro_diagnostic,used_with_arg"] | ||
|
||
[unstable] | ||
bindeps = true |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Ooooh this is really a deal breaker right now I think :/
We really need the stack size checking feature as stackoverflows are one of the biggest hubris pain points
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.
That's okay - the work is done, and going from "here is a list of nightly features we use" to "here is the one thing we use nightly for" is an improvement. and my goal was to knock out an easy task to familiarise myself with hubris, so: mission accomplished.
lmk if I should carve this change out slash refactor this PR as getting 90% there, or if there's more internal discussion to be had.
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.
I confess I haven't looked too closely at the mechanism or how it is implemented in practice, but I wonder if we need it in production? Or could we make it part of a CI pipeline with
RUSTC_BOOTSTRAP=1
?Uh oh!
There was an error while loading. Please reload this page.
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.
There are two aspects to the stack size check vis-a-vis compilation:
rustc
it invokes to emit stack size info. This requires thatrustc
binary to be a nightly version.get_max_stack
function does not actually require nightly, but will explode the packaging process if the stack info is not found.The first aspect is the tricker one. Right now, my current design expects
cargo
to be invoked with+nightly-...
at the topmost level (i.e.cargo +nightly-... xtask ...
) in order to implicitly select the nightly compiler. Therustc
façade will take a toolchain spec, so the toolchain selection could potentially be pushed into the dist task by conditionally adding a+nightly-...
argument torustc
. TheBuildConfig
used to locaterustc
appears to support using a façade, though its default construction specifies the sysroot, which I think implies deeper plumbing would be necessary. The net result of this would be a stable compiler building a dist build task which invokes a nightly compiler to compile firmware tasks.The second aspect is easier. If we shouldn't expect to find stack size info, just eat the error and move on.
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.
Thinking about this a bit more, the stack checking is not something that ends up in the production image, if that makes sense. You could use nightly to build a binary, emitting stack size info as a side-effect, and then throw that binary away in favor of a stable-toolchain-built binary, assuming all else is equal (... a big assumption, maybe).
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.
Yeah, that was kind of what I was going for with my earlier comment about only doing the stack checks with in CI, and not production. I apologize for being terribly vague there, however.
But a two-step process, where one builds a binary with the stack size data via setting
RUSTC_BOOTSTRAP=1
and setting the magic-Z
flag inRUSTFLAGS
, and then building another binary without settingRUSTC_BOOTSTRAP
, was what I meant.I presume we can use the same compiler binary for both invocations, and that we don't need an actual nightly compiler binary for either, as long as we set
RUSTC_BOOTSTRAP=1
for the case where we need the stack data.