Merged
Conversation
Contributor
|
Can I question why this includes |
Contributor
|
A number of other patches was applied to our version of libbacktrace after #30908 if you look at git history, Mach-O support in particular. |
Contributor
Author
|
@petrochenkov thanks, good call. I figured there may be more; I hadn't fully checked into it yet. I'll go take a look now that it did build successfully. |
Contributor
Author
|
I've reapplied some patches, and disregarded some others. A double check here would be great. Spelling fixes, that I've disregarded so that we closer track upstream: Backports that should be included with the update: e8121b3 added support for apple platforms, but then 8581b59 disabled it, so I've just not bothered. The original patch was sent upstream, but was ignored and eventually closed. For aa6bd11, the file was completely deleted upstream, and so I don't think applies anymore. |
Member
|
(I'd prefer us to transition to a gimli-based library eventually so I'm 👎 to merging this huge PR.) 8581b59 (#45866) only disabled mmap support for macOS, not the entire functionality. The upstream patch ianlancetaylor/libbacktrace#2 was closed because the maintainer simply ignored the PR without any further communication. It doesn't mean the patch is not working. BTW is it possible we just turn libbacktrace into a submodule? +18846/-8114 isn't a fun thing to deal with. |
Member
|
A submodule seems like a good idea. |
Member
Member
|
I believe at the time that this was originally added the repo didn't exist or I couldn't find it, but I definitely agree that we should use a submodule now. If anything it'll help us track how we diverge! To that end I've now created https://github.com/rust-lang-nursery/libbacktrace Ok and to make sure we don't lose anything, let's take a look at the current history of the "submodule". The commits we're dealing with are:
And I believe that's it. 5f57121, the next element in the history, is where we reset to a revision of upstream gcc, so it's sync'd with the original source. I agree with @steveklabnik to leave out typos, which leaves us with:
and I believe that's everything? The stack of commits is the tip of the @kennytm I agree that eventually we want to move away from libbacktrace, but unfortunately that's a much larger change so for the time being I think an incremental update moving us towards the submodule should probably be sufficient @steveklabnik do you want to update this PR to add a submodule to the repo I've created? |
Member
|
@steveklabnik also while you're at it, can you update the backtrace and backtrace-sys crates? |
Contributor
Author
|
@alexcrichton I can do it tomorrow; if you want to just get it done, feel free to close this and do it yourself :)
… On May 22, 2018, at 3:59 PM, Alex Crichton ***@***.***> wrote:
@steveklabnik also while you're at it, can you update the backtrace and backtrace-sys crates?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
3ec7aa5 to
6691dae
Compare
Member
|
@bors: r+ No worries! I've pushed the changes here and let's see what bors thinks of this. |
Collaborator
|
📌 Commit 6691dae has been approved by |
Contributor
|
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 |
Member
|
@bors: r- |
6691dae to
b3ee81e
Compare
Member
|
@bors: r+ |
Collaborator
|
📌 Commit b3ee81e has been approved by |
b3ee81e to
2a33e9b
Compare
Contributor
Author
|
Ugh, pinged @alexcrichton on chat about this; I didn't see that he updated the PR, so just did it and force pushed my version. Oops. Hopefully he can restore soon. |
Collaborator
|
⌛ Testing commit 9604a55326e1fddf812d76ebbfd0574267b1abf9 with merge 69935db6206d17be654db5dc8ac8444352edb32b... |
Collaborator
|
💔 Test failed - status-travis |
This comment has been minimized.
This comment has been minimized.
Member
|
@bors: r+ |
Collaborator
|
📌 Commit f0f0533 has been approved by |
Collaborator
|
🔒 Merge conflict |
Member
|
@bors: r+ |
Collaborator
|
📌 Commit 0a6f6e2 has been approved by |
Collaborator
|
⌛ Testing commit 0a6f6e206dc717f0198fd3ba3c5843fadaa3e754 with merge 8c5f93608ed645c1978343a9dc9fc1346ff9e63c... |
Collaborator
|
💔 Test failed - status-travis |
Contributor
|
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 |
While we're at it update the `backtrace` crate from crates.io. It turns out that the submodule's configure script has gotten a lot more finnicky as of late so also switch over to using the `cc` crate manually which allows to avoid some hacks around the configure script as well
Member
|
@bors: r+ |
Collaborator
|
📌 Commit 7c14a54 has been approved by |
Member
|
@bors p=162 |
Collaborator
Collaborator
|
☀️ Test successful - status-appveyor, status-travis |
This was referenced May 30, 2018
Closed
Contributor
|
Since this commit I'm getting 150 ui test failures (also compile-fail, anything involving failures really). Some of them are segfaults (sample backtrace: https://gist.github.com/nikic/0b752e8aecf12930e488bbfc9bfbd7f1). Most are "panicked while panicking", with the partial trace looking something like In both cases the trace contains a weird low-address function (0x1 for the segfault, 0x3 for the panic). |
Member
|
@nikic Could you file a new issue about this? |
Contributor
6 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
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.
We haven't updated libbacktrace in two years. This is just blindly updating to the latest HEAD; I'd like to see what travis says. It at least builds on my machine, running some tests...
This perpetuates the patches from #30908