Remove backtrace header text #69042
Merged
Remove backtrace header text #69042
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
|
r? @dtolnay |
dtolnay
requested changes
Feb 10, 2020
There was a problem hiding this comment.
I think we want to keep the add_context call. BacktraceFmt::add_context is documented as:
This is required on some platforms for backtraces to be fully symbolicated later, and otherwise this should just be the first method you call after creating a
BacktraceFmt.
That sounds like a behavior that we want to keep.
I think all we want for this item in #65280 is to remove the "stack backtrace:" printed by add_context.
Okay, I figured I'd try this first because @alexcrichton doesn't want to change the format of Do you think it would be sufficient to add a new #[cfg(target_os = "fuchsia")]
fuchsia::print_dso_context(self.fmt)?;
Ok(()) |
|
Do you have a link to that discussion with Alex? I haven't seen that and I am not sure what all the constraints are. In any case, we don't need to change the Display or Debug behavior of the external backtrace::Backtrace, just std::backtrace::Backtrace -- inside the backtrace crate we can move the "stack backtrace:" header from add_context out to the Debug impl. In the worst case it would also be reasonable to have std instantiate BacktraceFmt with a Formatter that discards "stack backtrace:" if it's the first thing printed. |
|
Oh sorry for the confusion, but to clarify, I intened that the general format of the current output shouldn't change in the |
After I started working on this I realized that, so that will be pushed shortly along with a PR for backtrace-rs to add that and bump the version.
Okay, I'll just remove it in backtrace-rs rather than moving it and then reduce this change to just bumping the dependency version once that is done. |
yaahc
force-pushed
the
backtrace-header
branch
from
60223e5
to
4cf0365
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 |
|
@bors try |
|
⌛ Trying commit 4cf0365 with merge 6f3697084e8993f5df429500a196baedfbe696de... |
|
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 |
|
💔 Test failed - checks-azure |
bors
added
S-waiting-on-author
|
@Dylan-DPC this wont work until the PR to backtrace-rs is merged. |
|
rust-lang/backtrace-rs#286 is the PR in question |
|
It is merged now @bors try |
|
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 |
|
💔 Test failed - checks-azure |
dtolnay
requested changes
Feb 11, 2020
There was a problem hiding this comment.
Failure seems legit.
error: the lock file /checkout/Cargo.lock needs to be updated but --locked was passed to prevent this
If you want to try to generate the lock file without accessing the network, use the --offline flag.
Build completed unsuccessfully in 0:00:25
make: *** [prepare] Error 1Needs a Cargo.lock update.
|
@bors r+ |
|
📌 Commit d2b08c7 has been approved by |
bors
added
S-waiting-on-bors
|
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 |
|
the ui tests for backtraces were failing so I decided to fix them in two ways
|
|
@bors r+ |
|
📌 Commit 090a157 has been approved by |
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Feb 12, 2020
Remove backtrace header text Fixes point 3 from rust-lang#65280 related to rust-lang#53487 This should probably be double checked by someone who works on fuschia because theres some extra fuschia specific output in `add_context` that is also removed by this change.
bors
added a commit
that referenced
this pull request
Feb 12, 2020
Rollup of 8 pull requests Successful merges: - #67585 (Improve `char::is_ascii_*` codegen) - #68914 (Speed up `SipHasher128`.) - #68994 (rustbuild: include channel in sanitizers installed name) - #69032 (ICE in nightly-2020-02-08: handle TerminatorKind::Yield in librustc_mir::transform::promote_consts::Validator method) - #69034 (parser: Remove `Parser::prev_token_kind`) - #69042 (Remove backtrace header text) - #69059 (Remove a few unused objects) - #69089 (Properly use the darwin archive format on Apple targets) Failed merges: r? @ghost
|
☔ The latest upstream changes (presumably #69094) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
S-waiting-on-author
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.
Fixes point 3 from #65280
related to #53487
This should probably be double checked by someone who works on fuschia because theres some extra fuschia specific output in
add_contextthat is also removed by this change.