-
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
base: master
Are you sure you want to change the base?
Update to enable stable toolchain #2194
Conversation
This commit makes a small number of updates necessary to adopt the stable toolchain: * Sets the stable toolchain as the default by config. Invoking nightly will require using `+nightly-2025-07-20` per oxidecomputer#2169. * Removes references to nightly requirements in the FAQ, and adds a note about enabling stack size checking being the only thing that requires nightly. * Removes code that passes the `-Z` flag to cargo to enable nightly features. * Updates `#[naked]` and `asm!` usage to stable convention, which is `#[unsafe(naked)]` and `naked_asm!`. Any affected assembly sections are expected to diverge (which was previously handled by `options(noreturn)`) or end with a trap (in the case of `_start`). * Replaces `used(linker)` with `used`, which is [equivalent]. * Adds a code path in the `dist` build task to build endoscope if the task being built has a name `drv_lpc55_swd`, upon which it depends. This is because [artifact dependencies] are not stable, but `lcp55-swd` depends on having the endoscope binary available to its build script. * Gates stack size emission and check behind a `rustc_version` channel check. Passing `+nightly-2025-07-20` to `cargo` (when invoking xtask) will enable the check. [equivalent]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/attrs/data_structures/enum.UsedBy.html [artifact dependencies]: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#artifact-dependencies
if version_meta().unwrap().channel == Channel::Nightly { | ||
// Check stack sizes and resolve task slots in our linked files | ||
let mut possible_stack_overflow = vec![]; | ||
for task_name in cfg.toml.tasks.keys() { | ||
if tasks_to_build.contains(task_name.as_str()) { | ||
if task_can_overflow(&cfg.toml, task_name, verbose)? { | ||
possible_stack_overflow.push(task_name); | ||
} | ||
|
||
resolve_task_slots(&cfg, task_name, image_name)?; | ||
resolve_task_slots(&cfg, task_name, image_name)?; | ||
} | ||
} | ||
if !possible_stack_overflow.is_empty() { | ||
bail!( | ||
"tasks may overflow: {possible_stack_overflow:?}; \ | ||
see logs above" | ||
); |
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
?
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:
- The build method in dist.rs uses RUSTFLAGS to cause the
rustc
it invokes to emit stack size info. This requires thatrustc
binary to be a nightly version. - The package method in dist.rs loops over every task and checks it for overflow. The
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. The rustc
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 to rustc
. The BuildConfig
used to locate rustc
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 in RUSTFLAGS
, and then building another binary without setting RUSTC_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.
PR content
This commit makes a small number of updates necessary to adopt the stable toolchain:
+nightly-2025-07-20
per Update to nightly-2025-07-20 #2169.-Z
flag to cargo to enable nightly features.#[naked]
andasm!
usage to stable convention, which is#[unsafe(naked)]
andnaked_asm!
. Any affected assembly sections are expected to diverge (which was previously handled byoptions(noreturn)
) or end with a trap (in the case of_start
).used(linker)
withused
, which is equivalent.dist
build task to build endoscope if the task being built has a namedrv_lpc55_swd
, upon which it depends. This is because artifact dependencies are not stable, butlcp55-swd
depends on having the endoscope binary available to its build script.rustc_version
channel check. Passing+nightly-2025-07-20
tocargo
(when invoking xtask) will enable the check.Fixes #1927
Open questions
2025-07-20
is the blessed nightly version.