PSA notice for upcoming Cortex-M breakage by japaric · Pull Request #196 · rust-embedded/wg (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation17 Commits3 Checks0 Files changed
Conversation
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
[ Show hidden characters]({{ revealButtonHref }})
This is the PSA we'll put up when the default linker of the Cortex-M targets
changes.
I'm putting it up for early review. It's OK to merge it before the breaking
change occurs.
r? @rust-embedded/resources
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, good writeup!
| - `thumbv7em-none-eabi` |
|---|
| - `thumbv7em-none-eabihf` |
| This will break the builds of *binaries* and *cdylibs* that you were using the |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if instead of that?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally wrote this using "if you were .." but wanted to change it into passive voice and failed. I'll fix it up.
| -- *and* that were passing extra flags to the linker using any of these rustc |
|---|
| flags: `-C link-arg`, `-C link-args`, `-Z pre-link-arg` or `-Z pre-link-args`. |
| Building libraries (`rlib`s and `staticlib`s) is not affected by this change. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not quite accurate: The real issue here (i.e. causing the breakage) is abusing gcc to call the linker hence requiring the use of -Wl, passthrough arguments. If one was using ld directly then in most cases the arguments are compatible with those of ldd.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
abusing gcc to call the linker
That's the default though. And the text says that it's the default mode (using gcc) the one that's getting broken.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that there is an "and" as well. So they condition requires both gcc and extra linker flags. Using the default linker (gcc) without extra flags will not break.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, after reading that complicated paragraph another time I can see what you mean.
| This will break the builds of *binaries* and *cdylibs* that are using the |
| default linker -- that is `-C linker` is not being passed to rustc to change the |
| linker -- *and* that pass extra flags to the linker using any of these rustc |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and that pass
I'd lose the "that" here.
@therealprof I changed the wording. Do you think it's easier to understand now?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much better, LGTM now. Thanks!
I think this has enough reviews so I'm going to merge it now.
bors r+
@rust-embedded/resources please keep an eye out for the upcoming breaking change. Once it's out publish this PSA to the users.rust-lang.org forum, and cross post to reddit and twitter; then report back here.
bors bot added a commit that referenced this pull request
196: PSA notice for upcoming Cortex-M breakage r=japaric a=japaric
This is the PSA we'll put up when the default linker of the Cortex-M targets changes.
I'm putting it up for early review. It's OK to merge it before the breaking change occurs.
r? @rust-embedded/resources
Co-authored-by: Jorge Aparicio jorge@japaric.io
@japaric Are there official accounts for the WG? If so, who has access to them?
I'll go ahead and tweet about this. I am also happy to share the password with anyone on @rust-embedded/resources (or really anyone on the WG).
bors added a commit to rust-lang/rust that referenced this pull request
change the default linker of the ARM Cortex-M targets
to rust-lld so users won't need an external linker to build programs
This will break nightly builds.
We discussed this within the embedded WG and with the embedded community in rust-embedded/wg#160 and there was consensus in that this breaking change is worthwhile and that we should do it now before it becomes impossible to do without breaking stable builds.
We have already written an announcement (see rust-embedded/wg#196) that explains
the breakage and instructs the users how to fix their builds. The TL;DR is that
they can switch to the old behavior by passing the -C linker flag to rustc.
We'll post the announcement as soon as this change makes into nightly.
closes rust-embedded/wg#160
Heads up the PR has landed. The change will be available in the next nightly (tomorrow hopefully).
Reviewers
