Make llvm-libunwind a per-target option#93604

tmandry · 2022-02-02T22:49:50Z

Fuchsia doesn't ship libunwind in its SDK, so we must provide it statically.

rust-highfive · 2022-02-02T22:49:53Z

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

library/unwind/src/libunwind.rs

petrochenkov · 2022-02-05T03:43:32Z

cc @12101111

src/bootstrap/compile.rs

Mark-Simulacrum · 2022-02-06T02:02:47Z

config.toml.example

I would not expect this to override the target default? Unless I'm misunderstanding, I would expect us to have:

target.llvm-libunwind (if set)

(global) llvm-libunwind (if set)

target.llvm-libunwind (default)

(global) llvm-unwind (default)

as the rough order.

That's the intended order. I'm trying to point out the relative ordering between #2 and #3 in your comment. Can you think of wording that would be clearer?

Maybe just essentially inlining my list as-is? Maybe saying something like "Note that the target-specific option llvm-libunwind will override this if set" would work too. "Overrides target default" to me feels a little unclear -- I think it can only mean one thing on a second reading, but the initial implication seems confusing, maybe because it is saying something that sounds obviously true to me :)

Went with your second suggestion, is this more clear?

Mark-Simulacrum · 2022-03-10T02:56:20Z

Yes, seems great.

@bors r+ rollup

bors · 2022-03-10T02:56:22Z

📌 Commit 42624ba has been approved by Mark-Simulacrum

…r=Mark-Simulacrum Use in-tree libunwind by default on Fuchsia Fuchsia doesn't ship libunwind in its SDK, so we must provide it statically.

Dylan-DPC · 2022-03-10T18:53:08Z

@bors r-

failed in rollup

bors · 2022-03-13T23:17:58Z

☔ The latest upstream changes (presumably #94916) made this pull request unmergeable. Please resolve the merge conflicts.

tmandry · 2022-05-24T23:04:24Z

Since this hit a snag building llvm-libunwind in Rust CI, I've taken out the Fuchsia-specific code from this PR so we can land the per-target logic.

tmandry · 2022-05-24T23:08:48Z

@bors r=Mark-Simulacrum

bors · 2022-05-24T23:08:50Z

📌 Commit f1e3d40 has been approved by Mark-Simulacrum

Rollup of 5 pull requests Successful merges: - rust-lang#93604 (Make llvm-libunwind a per-target option) - rust-lang#97026 (Change orderings of `Debug` for the Atomic types to `Relaxed`.) - rust-lang#97105 (Add tests for lint on type dependent on consts) - rust-lang#97323 (Introduce stricter checks for might_permit_raw_init under a debug flag ) - rust-lang#97379 (Add aliases for `current_dir`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…, r=tmandry Use llvm-libunwind="in-tree" for Fuchsia targets With updates to Fuchsia CI's Zircon libraries rust-lang#99833, we can introduce `llvm-libunwind="in-tree"` for Fuchsia targets. This PR restores functionality removed from rust-lang#93604 (comment). cc `@tmandry` `@djkoloski`

rust-highfive assigned Mark-Simulacrum Feb 2, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 2, 2022

Mark-Simulacrum reviewed Feb 4, 2022

View reviewed changes

library/unwind/src/libunwind.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

tmandry added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 5, 2022

Mark-Simulacrum added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 5, 2022

Mark-Simulacrum reviewed Feb 5, 2022

View reviewed changes

src/bootstrap/compile.rs Outdated Show resolved Hide resolved

Mark-Simulacrum reviewed Feb 6, 2022

View reviewed changes

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 6, 2022

tmandry force-pushed the libunwind-fuchsia-default branch from 1e70caa to 42624ba Compare March 10, 2022 02:08

tmandry added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 10, 2022

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 10, 2022

m-ou-se mentioned this pull request Mar 10, 2022

Rollup of 7 pull requests #94801

Closed

Dylan-DPC mentioned this pull request Mar 10, 2022

Rollup of 7 pull requests #94807

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 10, 2022

tmandry force-pushed the libunwind-fuchsia-default branch from 42624ba to 9d857d9 Compare March 18, 2022 22:01

apiraino added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label May 19, 2022

Make llvm-libunwind a per-target option

f1e3d40

tmandry force-pushed the libunwind-fuchsia-default branch from 9d857d9 to f1e3d40 Compare May 24, 2022 23:00

tmandry changed the title ~~Use in-tree libunwind by default on Fuchsia~~ Make llvm-libunwind a per-target option May 24, 2022

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 24, 2022

Dylan-DPC mentioned this pull request May 25, 2022

Rollup of 5 pull requests #97382

Merged

bors merged commit bbb88ea into rust-lang:master May 25, 2022

rustbot added this to the 1.63.0 milestone May 25, 2022

tmandry deleted the libunwind-fuchsia-default branch May 25, 2022 18:30

tmandry mentioned this pull request Jul 28, 2022

Add Fuchsia platform support documentation #99831

Merged

andrewpollack mentioned this pull request Aug 9, 2022

Use llvm-libunwind="in-tree" for Fuchsia targets #100341

Merged

Uh oh!

Conversation

tmandry commented Feb 2, 2022

Uh oh!

rust-highfive commented Feb 2, 2022

Uh oh!

Uh oh!

This comment has been minimized.

petrochenkov commented Feb 5, 2022

Uh oh!

Uh oh!

Mark-Simulacrum Feb 6, 2022

Choose a reason for hiding this comment

Uh oh!

tmandry Feb 9, 2022

Choose a reason for hiding this comment

Uh oh!

Mark-Simulacrum Feb 9, 2022

Choose a reason for hiding this comment

Uh oh!

tmandry Mar 10, 2022

Choose a reason for hiding this comment

Uh oh!

Mark-Simulacrum commented Mar 10, 2022

Uh oh!

bors commented Mar 10, 2022

Uh oh!

Dylan-DPC commented Mar 10, 2022

Uh oh!

bors commented Mar 13, 2022

Uh oh!

tmandry commented May 24, 2022

Uh oh!

tmandry commented May 24, 2022

Uh oh!

bors commented May 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants