[WIP]: Remove libbacktrace #51408
Closed
[WIP]: Remove libbacktrace #51408
Conversation
This file contains 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
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
est31
force-pushed
the
libbacktrace_riir
branch
2 times, most recently
from
de853d2
to
55527be
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
joshtriplett
reviewed
Jun 7, 2018
src/libstd/build.rs
Outdated
| @@ -19,6 +19,7 @@ use std::fs::File; | |||
|
|
|||
| fn main() { | |||
| let target = env::var("TARGET").expect("TARGET was not set"); | |||
| /* | |||
There was a problem hiding this comment.
I assume the final version will actually delete code, rather than commenting it out?
|
@est31 Could I get an acronym expansion on "RIIR"? "Re-Implementation In Rust"? |
kennytm
added
S-waiting-on-author
|
@joshtriplett Rewrite It In Rust. |
est31
force-pushed
the
libbacktrace_riir
branch
2 times, most recently
from
81c18b6
to
9dda8d4
Compare
| @@ -840,7 +840,13 @@ impl<'a> Builder<'a> { | |||
| // already-passed -C metadata flag with our own. Our rustc.rs | |||
| // wrapper around the actual rustc will detect -C metadata being | |||
| // passed and frob it with this extra string we're passing in. | |||
There was a problem hiding this comment.
So the wording above was correct where the compiler was the only crate graph depending on crates.io, but since now std itself also does that, we need to apply this metadata mangling to differ std's and rustc's symbols as well. Let me know if you want further explanation or you think that's enough to update the wording.
src/bootstrap/builder.rs
Outdated
| Mode::Std => "rustc-std", | ||
| Mode::Test => "rustc-test", | ||
| Mode::Codegen | Mode::Rustc => "rustc-rustc", | ||
| Mode::ToolStd | Mode::ToolTest | Mode::ToolRustc => "rustc-tools", |
There was a problem hiding this comment.
Let's stick an unreachable in this arm to be a little clearer (this is in an if statement where we've checked for tool already).
|
|
||
| // If this was output in the `deps` dir then this is a precise file | ||
| // name (hash included) so we start tracking it. | ||
| if filename.starts_with(&target_deps_dir) { |
There was a problem hiding this comment.
Update this comment to state something along the lines of "We've already added this in the previous loop iteration"
| let filename = Path::new(&*filename); | ||
|
|
||
| // If this was an output file in the "host dir" we don't actually | ||
| // worry about it, it's not relevant for us. |
There was a problem hiding this comment.
If the host dir contains a file that wasn't in the target dir, also promote it. This primarily is the case for things like proc macros, where if std depends on that, we'll need to make sure that the proc macro artifact is also promoted -- but since it's host only, it won't be in the target dir.
| } | ||
|
|
||
| let filename = Path::new(&*filename); | ||
| let filename = Path::new(&**filename); | ||
|
|
||
| // If this was an output file in the "host dir" we don't actually | ||
| // worry about it, it's not relevant for us. |
There was a problem hiding this comment.
Let's change this comment to basically refer to this same condition below and say see why there.
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Sooo.... below a mountain of rustbuild changes, and a bunch of patches to For a simple crate While this is the output of Rust nightly: I'm not sure how much of this is an addr2line problem, and how much of this is a problem of my usage of it. @philipc do you have any ideas? |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
It probably needs to take relocation into account, such as in the addr2line tests. And related to this, we need to handle debuginfo in shared libraries too, but that shouldn't be needed for what you are testing here. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
I've chatted today again with @Mark-Simulacrum about the build system changes that this PR needs/does. They fixed some issues that this PR caused and sent me a patch which I included in this PR. They also found further issues. It turns out that the |
Mark-Simulacrum
added
S-blocked
|
If fixing derives is too hard then we could change the |
|
☔ The latest upstream changes (presumably #50698) made this pull request unmergeable. Please resolve the merge conflicts. |
|
The period of my contributions to Rust upstream has reached an end. Thus I'm unable to continue my work on this. I still think something like this is a great addition. I urge anyone interested in this change to adopt and continue it from here on. Thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Remove libbacktrace in favour of the pure Rust library addr2line. See issue #46439.
Right now this PR is only a testbed as I'm trying to find out how to fix all the problems.
This PR currently bases on #51440 as it requires its changes. I want to enable review by different people of the two changes thus I've filed two PRs.
Blockers:
addr2linesuitability for std inclusion (integration into libstd gimli-rs/addr2line#89)findshlibssuitability for std inclusion (#![no_std] support gimli-rs/findshlibs#20)Very WIP. DO NOT MERGE