-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add lldb to the build #52716
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add lldb to the build #52716
Conversation
r? @aturon (rust_highfive has picked a reviewer for you, use r? to override) |
|
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'll send a patch to tidy. |
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 |
My current theory is that the new dist rule is using the wrong test in |
src/bootstrap/dist.rs
Outdated
|
||
// Do nothing if lldb was not built. This is difficult to | ||
// determine in should_run because the target is not available | ||
// at that point. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can omit this comment -- this is the expected way of doing things, should_run is very high-level.
let libdir = builder.llvm_out(target).join("lib"); | ||
let dst = image.join("lib"); | ||
t!(fs::create_dir_all(&dst)); | ||
for entry in t!(fs::read_dir(&libdir)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll probably want to add a if self.config.dry_run { return; }
somewhere in here since the read_dir will fail on dry run builds otherwise.
Is this waiting on me for something? I believe the changes look good. |
Yes, I think so. Edit: I should have said, I think @nrc switched the review to you, so my belief is that if you think it is ok, then you can merge it. However you're likely to know better than I do. |
📌 Commit 14b38bfce0fbbb5f9d58421ebee69d37b287b51f has been approved by |
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message
|
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 don't understand that failure but I will ask around about it tomorrow. The |
Maybe it's just because the Cargo.lock change somehow didn't survive the rebase. |
@bors r+ |
📌 Commit 1d43f25 has been approved by |
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message
|
@bors: r+ |
📌 Commit dd4cf7f9a76e605871578fc368d566b9e6823591 has been approved by |
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message
|
This optionally adds lldb (and clang, which it needs) to the build. Because rust uses LLVM 7, and because clang 7 is not yet released, a recent git master version of clang is used. The lldb that is used includes the Rust plugin. lldb is only built when asked for, or when doing a nightly build on macOS. Only macOS is done for now due to difficulties with the Python dependency.
Rebased again. |
@bors: r+ |
📌 Commit 6e3a4f4 has been approved by |
Add lldb to the build This optionally adds lldb (and clang, which it needs) to the build. Because rust uses LLVM 7, and because clang 7 is not yet released, a recent git master version of clang is used. The lldb that is used includes the Rust plugin. lldb is only built when asked for, or when doing a nightly build on macOS. Only macOS is done for now due to difficulties with the Python dependency.
☀️ Test successful - status-appveyor, status-travis |
This optionally adds lldb (and clang, which it needs) to the build.
Because rust uses LLVM 7, and because clang 7 is not yet released, a
recent git master version of clang is used.
The lldb that is used includes the Rust plugin.
lldb is only built when asked for, or when doing a nightly build on
macOS. Only macOS is done for now due to difficulties with the Python
dependency.