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.

Sign up for GitHub

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

Jump to bottom

Update rustfmt to 0.3.6 #47454

Merged
merged 1 commit into from
Jan 19, 2018
Merged

Update rustfmt to 0.3.6 #47454

merged 1 commit into from
Jan 19, 2018

Conversation

topecongiro
Copy link
Contributor

r? @nrc

@topecongiro
Copy link
Contributor Author

Currently rustfmt and rls is missing from the nightly component. This PR brings those two back.

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 17, 2018
kennytm
kennytm reviewed Jan 17, 2018
View reviewed changes
src/Cargo.lock
@@ -1723,6 +1723,72 @@ dependencies = [
"syntax_pos 0.0.0",
]

[[package]]
name = "rustc-ap-rustc_cratesio_shim"
version = "12.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

Since we are building rustfmt in the compiler, it's strange we in turn depend on an external, older copy of these crates; besides, these will almost always be outdated (the current version is 14, for instance).

The advantage of this is that compiler changes will less likely to break rustfmt on nightly.

Not sure if we want to r+ as-is or require a [patch] section.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also wonder how this affects build times.

Copy link
Member

@kennytm kennytm Jan 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@petrochenkov It shouldn't affect build times since those two builds don't touch rustfmt/rls.

Copy link
Member

Choose a reason for hiding this comment

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

@kennytm same thing will happen to rls, however rls uses a locally-patched version of rustfmt, so I assume it'll work as intended, provided that rustfmt compiles as expected in-tree, with [patch]ed deps or not, no?

Copy link
Member

Choose a reason for hiding this comment

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

It is intentional to depend on an older version of the crate, it means that breakage in libsyntax doesn't break rustfmt (and the RLS), which is the whole reason for doing this. Using [patch] would prevent that happening.

@Xanewok Xanewok mentioned this pull request Jan 17, 2018
Merged
@nrc
Copy link
Member

nrc commented Jan 18, 2018

@bors: r+ p=1

@bors
Copy link
Contributor

bors commented Jan 18, 2018

📌 Commit 1f0f46e has been approved by nrc

@topecongiro
Copy link
Contributor Author

@nrc rustfmt 0.3.5 has a bug which is likely to appear in many places (cc rust-lang/rustfmt#2370 and rust-lang/rustfmt#2374). As such, I am going to publish 0.3.6 and use it instead.

@kennytm
Copy link
Member

kennytm commented Jan 18, 2018

@bors r-

Waiting for author to update to 0.3.6

@kennytm kennytm added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 18, 2018
@topecongiro topecongiro changed the title Update rustfmt to 0.3.5 Update rustfmt to 0.3.6 Jan 18, 2018
@topecongiro
Copy link
Contributor Author

Updated rustfmt to 0.3.6.

@kennytm kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Jan 18, 2018
@kennytm
Copy link
Member

kennytm commented Jan 18, 2018

@bors r=nrc

@bors
Copy link
Contributor

bors commented Jan 18, 2018

📌 Commit 8057443 has been approved by nrc

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 18, 2018
@kennytm
Copy link
Member

kennytm commented Jan 18, 2018 •
edited
Loading

@bors r-

@topecongiro Could you rebase on the latest master to resolve the Cargo.lock conflict? Thanks.

@bors
Copy link
Contributor

bors commented Jan 18, 2018

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

@topecongiro
Copy link
Contributor Author

Rebased on the latest master.

@kennytm kennytm 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 18, 2018
@kennytm
Copy link
Member

kennytm commented Jan 18, 2018

@bors r=nrc

@bors
Copy link
Contributor

bors commented Jan 18, 2018

📌 Commit 5ae8fa9 has been approved by nrc

Xanewok added a commit to Xanewok/rls that referenced this pull request Jan 18, 2018
@Xanewok
Update rustfmt to 0.3.6
abd741e 
Fixes a possible bug from 0.3.5
(rust-lang/rust#47454 (comment))
and prepares the RLS repo to be updated in Rust tree.
@Xanewok Xanewok mentioned this pull request Jan 18, 2018
Merged
@bors
Copy link
Contributor

bors commented Jan 18, 2018

⌛ Testing commit 5ae8fa9 with merge f703782c49196ffa110c97ad262a8498570b5e0a...

@bors
Copy link
Contributor

bors commented Jan 18, 2018

💔 Test failed - status-appveyor

@kennytm
Copy link
Member

kennytm commented Jan 19, 2018

@bors retry 3 hour timeout.

@bors
Copy link
Contributor

bors commented Jan 19, 2018

⌛ Testing commit 5ae8fa9 with merge b71cbd8...

bors added a commit that referenced this pull request Jan 19, 2018
@bors
Auto merge of #47454 - topecongiro:update-rustfmt, r=nrc
b71cbd8 
Update rustfmt to 0.3.6

r? @nrc
@bors
Copy link
Contributor

bors commented Jan 19, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nrc
Pushing b71cbd8 to master...

@bors bors merged commit 5ae8fa9 into rust-lang:master Jan 19, 2018
kennytm-githubbot added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Jan 19, 2018
@kennytm-githubbot
📣 Toolstate changed by rust-lang/rust#47454!
748fcc3 
Tested on commit rust-lang/rust@b71cbd8.

🎉 rls on windows: build-fail → test-pass.
🎉 rls on linux: build-fail → test-pass.
🎉 rustfmt on windows: build-fail → test-pass.
🎉 rustfmt on linux: build-fail → test-pass.
@topecongiro topecongiro deleted the update-rustfmt branch January 19, 2018 10:21
alexheretic pushed a commit to alexheretic/rls that referenced this pull request Jan 20, 2018
@Xanewok @alexheretic
Update rustfmt to 0.3.6
0b614ec 
Fixes a possible bug from 0.3.5
(rust-lang/rust#47454 (comment))
and prepares the RLS repo to be updated in Rust tree.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kennytm kennytm kennytm left review comments

@nrc nrc nrc left review comments

@Xanewok Xanewok Xanewok left review comments

@petrochenkov petrochenkov petrochenkov left review comments

Assignees

@nrc nrc

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@topecongiro @nrc @bors @kennytm @Xanewok @petrochenkov