Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update RLS and rustfmt #81768

Merged
merged 2 commits into from
Feb 10, 2021
Merged

Conversation

calebcartwright
Copy link
Member

Fixes #81582 and fixes #81583

r? @Xanewok

I was originally surprised by the size of lockfile diff, though after looking at the RLS changes it makes a bit more sense to me now

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • These commits modify submodules.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 5, 2021
@rust-log-analyzer

This comment has been minimized.

Cargo.lock Outdated
Comment on lines 5582 to 5584
[[patch.unused]]
name = "backtrace"
version = "0.3.55"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since RLS has removed the backtrace dep, this is now coming from

rust/Cargo.toml

Lines 109 to 113 in dab3a80

# This crate's integration with libstd is a bit wonky, so we use a submodule
# instead of a crates.io dependency. Make sure everything else in the repo is
# also using the submodule, however, so we can avoid duplicate copies of the
# source code for this crate.
backtrace = { path = "library/backtrace" }

I wasn't sure if that entry should be removed from the root Cargo.toml given the presence of the submodule and a few other references (looked like the RA submodule has it, but feature gated) so left in for now and just removed it from the permitted dep lists in the tidy check to get rid of the error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine to leave it for now, especially if RA still uses it conditionally (I assume we dist it without this feature enabled?).

cc @alexcrichton just in case (06d565c)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If nothing in rust-lang/rust is using the backtrace crate then this should be fine to remove. I thought Cargo at least would be using it through anyhow, but I was mistaken.

I think everything is using the dependency through the standard library now so it should be safe to remove.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we dist it without this feature enabled?

Actually I guess it's completely disabled (at least at the moment)

https://github.com/rust-analyzer/rust-analyzer/blob/80ab753d7e0bf59b81df317d6ddda43cb919ec83/crates/stdx/Cargo.toml#L12-L19

[dependencies]
backtrace = { version = "0.3.44", optional = true }
always-assert = { version = "0.1.2", features = ["log"] }
# Think twice before adding anything here

[features]
# Uncomment to enable for the whole crate graph
# default = [ "backtrace" ]

I think everything is using the dependency through the standard library now so it should be safe to remove.

SGTM 👍 if something changes in that RA crate or elsewhere which brings backtrace back then the patch can be restored accordingly

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like bors will get to this before I get back to my dev machine. If so will open a followup PR to remove the patch

@calebcartwright
Copy link
Member Author

It's unclear to me why the other two jobs were cancelled, seemingly mid-run with everything succeeding up to that point 🤷‍♂️

@Xanewok
Copy link
Member

Xanewok commented Feb 5, 2021

Looks like an instance of #81783.

I think it's worth getting it into the queue:

@bors r+ p=1 (sweeping lockfile changes)

@bors
Copy link
Contributor

bors commented Feb 5, 2021

📌 Commit d43ffff3e6b3722a3f773164d231b405481aa3e9 has been approved by Xanewok

@bors 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 Feb 5, 2021
@bors
Copy link
Contributor

bors commented Feb 5, 2021

⌛ Testing commit d43ffff3e6b3722a3f773164d231b405481aa3e9 with merge 0fcf134521aba2dbfa8344e7e470f348c24634bc...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 5, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 5, 2021
@calebcartwright
Copy link
Member Author

Looks like we got caught between the stabilization of int_bits_const and the subsequent reversion in #81727. Will have to restart the auto publish crate update cycle

@bors
Copy link
Contributor

bors commented Feb 10, 2021

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

@calebcartwright
Copy link
Member Author

Rebased and updated to the v705 crate versions 🤞 I opted to keep the removal of the backtrace references in a separate commit on the off chance it needs to be reverted, but please lmk if you think everything should be squashed into one.

@Xanewok
Copy link
Member

Xanewok commented Feb 10, 2021

Looks good, thanks for doing this!

@bors r+

@bors
Copy link
Contributor

bors commented Feb 10, 2021

📌 Commit 0a47a38 has been approved by Xanewok

@bors 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 Feb 10, 2021
@bors
Copy link
Contributor

bors commented Feb 10, 2021

⌛ Testing commit 0a47a38 with merge 218bf8d...

@bors
Copy link
Contributor

bors commented Feb 10, 2021

☀️ Test successful - checks-actions
Approved by: Xanewok
Pushing 218bf8d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 10, 2021
@bors bors merged commit 218bf8d into rust-lang:master Feb 10, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 10, 2021
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #81768!

Tested on commit 218bf8d.
Direct link to PR: #81768

🎉 rls on windows: build-fail → test-pass (cc @Xanewok).
🎉 rls on linux: build-fail → test-pass (cc @Xanewok).
🎉 rustfmt on windows: build-fail → test-pass (cc @topecongiro @calebcartwright).
🎉 rustfmt on linux: build-fail → test-pass (cc @topecongiro @calebcartwright).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Feb 10, 2021
Tested on commit rust-lang/rust@218bf8d.
Direct link to PR: <rust-lang/rust#81768>

🎉 rls on windows: build-fail → test-pass (cc @Xanewok).
🎉 rls on linux: build-fail → test-pass (cc @Xanewok).
🎉 rustfmt on windows: build-fail → test-pass (cc @topecongiro @calebcartwright).
🎉 rustfmt on linux: build-fail → test-pass (cc @topecongiro @calebcartwright).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustfmt no longer builds after rust-lang/rust#81578 rls no longer builds after rust-lang/rust#81578
7 participants